Skip to content

Commit 65c2907

Browse files
authored
fix: store gossip signatures in order (#267)
This PR fixes a regression added in #253, which unordered gossip transactions, causing errors during aggregation
1 parent ed2712a commit 65c2907

1 file changed

Lines changed: 13 additions & 26 deletions

File tree

crates/storage/src/store.rs

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{HashMap, HashSet, VecDeque};
1+
use std::collections::{BTreeMap, HashMap, HashSet, VecDeque};
22
use std::sync::{Arc, LazyLock, Mutex};
33

44
/// The tree hash root of an empty block body.
@@ -205,17 +205,15 @@ impl PayloadBuffer {
205205
}
206206
}
207207

208-
/// Individual validator signature received via gossip.
209-
#[derive(Clone)]
210-
struct GossipSignatureEntry {
211-
validator_id: u64,
212-
signature: ValidatorSignature,
213-
}
214-
215208
/// Gossip signatures grouped by attestation data.
209+
///
210+
/// Signatures are stored in a `BTreeMap` keyed by validator_id to guarantee
211+
/// ascending iteration order. XMSS aggregate proofs are order-dependent:
212+
/// verification reconstructs pubkeys from the participation bitfield (low-to-high),
213+
/// so aggregation must produce them in the same ascending order.
216214
struct GossipDataEntry {
217215
data: AttestationData,
218-
signatures: Vec<GossipSignatureEntry>,
216+
signatures: BTreeMap<u64, ValidatorSignature>,
219217
}
220218

221219
/// Gossip signatures grouped by attestation data (via data_root).
@@ -260,8 +258,7 @@ pub struct Store {
260258
backend: Arc<dyn StorageBackend>,
261259
new_payloads: Arc<Mutex<PayloadBuffer>>,
262260
known_payloads: Arc<Mutex<PayloadBuffer>>,
263-
/// In-memory gossip signatures: data_root → (AttestationData, Vec<GossipSignatureEntry>).
264-
/// Transient data consumed at interval 2 aggregation.
261+
/// In-memory gossip signatures, consumed at interval 2 aggregation.
265262
gossip_signatures: Arc<Mutex<GossipSignatureMap>>,
266263
}
267264

@@ -947,7 +944,7 @@ impl Store {
947944
// Gossip signatures are individual validator signatures received via P2P.
948945
// They're transient (consumed at interval 2 aggregation) so stored in-memory.
949946
// Keyed by AttestationData (via data_root) matching the leanSpec structure:
950-
// gossip_signatures: dict[AttestationData, set[GossipSignatureEntry]]
947+
// gossip_signatures: dict[AttestationData, set[GossipSignature]]
951948

952949
/// Delete gossip entries for the given (validator_id, data_root) pairs.
953950
pub fn delete_gossip_signatures(&mut self, keys: &[(u64, H256)]) {
@@ -957,7 +954,7 @@ impl Store {
957954
let mut gossip = self.gossip_signatures.lock().unwrap();
958955
for &(vid, data_root) in keys {
959956
if let Some(entry) = gossip.get_mut(&data_root) {
960-
entry.signatures.retain(|e| e.validator_id != vid);
957+
entry.signatures.remove(&vid);
961958
if entry.signatures.is_empty() {
962959
gossip.remove(&data_root);
963960
}
@@ -974,7 +971,7 @@ impl Store {
974971
let sigs: Vec<_> = entry
975972
.signatures
976973
.iter()
977-
.map(|e| (e.validator_id, e.signature.clone()))
974+
.map(|(&vid, sig)| (vid, sig.clone()))
978975
.collect();
979976
(HashedAttestationData::new(entry.data.clone()), sigs)
980977
})
@@ -992,19 +989,9 @@ impl Store {
992989
let mut gossip = self.gossip_signatures.lock().unwrap();
993990
let entry = gossip.entry(data_root).or_insert_with(|| GossipDataEntry {
994991
data: att_data,
995-
signatures: Vec::new(),
992+
signatures: BTreeMap::new(),
996993
});
997-
// Avoid duplicates for same validator
998-
if !entry
999-
.signatures
1000-
.iter()
1001-
.any(|e| e.validator_id == validator_id)
1002-
{
1003-
entry.signatures.push(GossipSignatureEntry {
1004-
validator_id,
1005-
signature,
1006-
});
1007-
}
994+
entry.signatures.entry(validator_id).or_insert(signature);
1008995
}
1009996

1010997
// ============ Derived Accessors ============

0 commit comments

Comments
 (0)