fix(authenticator): normalize sparse indexer pubkey slots before key set validation#447
Conversation
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
75efd81 to
226b119
Compare
piohei
left a comment
There was a problem hiding this comment.
Left only minor comment there. :)
| /// | ||
| /// # Errors | ||
| /// Returns an error if the number of public keys exceeds [`MAX_AUTHENTICATOR_KEYS`]. | ||
| pub fn new(pubkeys: Option<Vec<EdDSAPublicKey>>) -> Result<Self, PrimitiveError> { |
There was a problem hiding this comment.
I think now only one test is using that new method passing something else than None. Maybe we can change it and create empty set by default when calling new?
There was a problem hiding this comment.
Good point, I'll do it on follow up PR🙏
Problem
The indexer stores authenticator addresses/pubkeys as sparse slot arrays, where removed authenticators are kept as
nullto preserve stablepubkey_idpositions.The previous mapping path compacted these arrays, dropping nulls, which could shift key indices and make gateway/authenticator logic operate on the wrong slot.
Solution
Preserved sparse slot semantics end-to-end and normalized safely before validation:
Vec<Option<U256>>(and added serde support for optional hex vectors).Nonefor removed authenticators) instead of filtering missing entries.decode_indexer_pubkeysnormalization:MAX_AUTHENTICATOR_KEYS.Note
Medium Risk
Changes affect wire formats and key-slot indexing semantics across services; mismatched deployments or edge cases in slot normalization could break clients or produce incorrect slot selection.
Overview
Fixes authenticator
pubkey_idstability by propagating sparse authenticator slot arrays end-to-end instead of compacting away removed entries.Indexer API payloads for authenticator pubkeys now return
Vec<Option<U256>>(serialized via newhex_u256_opt_vec), indexer DB mappers stop filtering outnulls, and inclusion-proof/pubkey routes validate/normalize sparse slots before constructingAuthenticatorPublicKeySet.On the authenticator side, indexer pubkeys are decoded via
decode_sparse_authenticator_pubkeys(trim trailingNone, preserve interior holes, reject out-of-bounds used slots) with a newInvalidIndexerPubkeySloterror, and insertion now reuses the first empty slot rather than always appending; tests and stubs are updated accordingly.Written by Cursor Bugbot for commit 226b119. This will update automatically on new commits. Configure here.