From ed68755dabd0348dd09f914be5799d6b5ea5a1b9 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Tue, 31 Mar 2026 13:02:18 -0300 Subject: [PATCH] Simplify signature verification and document gossip counter safety Combine validator ID collection, bounds checking, and public key lookup into a single pass in verify_all_signatures, eliminating an intermediate Vec allocation and a redundant iteration on the block verification hot path. Add a comment on gossip_signatures_count documenting that its read-then-write pattern in insert_gossip_signature is safe only because Store mutations are single-threaded (BlockChain actor). If concurrent writes are introduced, the counter update must be made atomic. --- crates/blockchain/src/store.rs | 15 ++++++--------- crates/storage/src/store.rs | 4 ++++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 3d70e08..4efbff0 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -1150,18 +1150,15 @@ fn verify_signatures( return Err(StoreError::ParticipantsMismatch); } - let validator_ids: Vec<_> = validator_indices(&attestation.aggregation_bits).collect(); - if validator_ids.iter().any(|vid| *vid >= num_validators) { - return Err(StoreError::InvalidValidatorIndex); - } - let slot: u32 = attestation.data.slot.try_into().expect("slot exceeds u32"); let message = attestation.data.tree_hash_root(); - // Collect public keys for all participating validators - let public_keys: Vec<_> = validator_ids - .iter() - .map(|&vid| { + // Collect public keys with bounds check in a single pass + let public_keys: Vec<_> = validator_indices(&attestation.aggregation_bits) + .map(|vid| { + if vid >= num_validators { + return Err(StoreError::InvalidValidatorIndex); + } validators[vid as usize] .get_pubkey() .map_err(|_| StoreError::PubkeyDecodingFailed(vid)) diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index e7bed2a..fa47e7c 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -227,6 +227,10 @@ pub struct Store { backend: Arc, new_payloads: Arc>, known_payloads: Arc>, + /// Cached count of GossipSignatures table entries, used only for metrics. + /// Safe because all Store mutations happen on the BlockChain actor's single thread. + /// If concurrent writes are ever introduced, the read-then-write in + /// insert_gossip_signature must be made atomic to prevent counter drift. gossip_signatures_count: Arc, }