From 9d32b72d17b3d8b72a9307a0ddcfff9f5229904d Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Mon, 6 Apr 2026 17:10:45 -0300 Subject: [PATCH 1/7] Enforce single AttestationData per block (leanSpec PR #510) Add block validation rejecting duplicate AttestationData entries and a compaction step in block building that merges proofs sharing the same AttestationData into a single attestation entry. For empty proofs (skip-sig mode), participant bitfields are unioned. For real proofs, the proof with the most participants is kept until lean-multisig supports recursive aggregation. --- crates/blockchain/src/store.rs | 363 +++++++++++++++++++++++++++++++++ 1 file changed, 363 insertions(+) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 6f114cf..a5bcde7 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -519,6 +519,20 @@ fn on_block_core( slot, })?; + // Each unique AttestationData must appear at most once per block. + let attestations = &signed_block.block.block.body.attestations; + let unique_count = attestations + .iter() + .map(|att| &att.data) + .collect::>() + .len(); + if unique_count != attestations.len() { + return Err(StoreError::DuplicateAttestationData { + count: attestations.len(), + unique: unique_count, + }); + } + let sig_verification_start = std::time::Instant::now(); if verify { // Validate cryptographic signatures @@ -883,6 +897,11 @@ pub enum StoreError { attestation_id: u64, proposer_index: u64, }, + + #[error( + "Block contains duplicate AttestationData entries: {count} entries but only {unique} unique" + )] + DuplicateAttestationData { count: usize, unique: usize }, } /// Build an AggregationBits bitfield from a list of validator indices. @@ -906,6 +925,97 @@ fn aggregation_bits_from_validator_indices(bits: &[u64]) -> AggregationBits { aggregation_bits } +/// Compute the bitwise union (OR) of two AggregationBits bitfields. +fn union_aggregation_bits(a: &AggregationBits, b: &AggregationBits) -> AggregationBits { + let max_len = a.len().max(b.len()); + if max_len == 0 { + return AggregationBits::with_length(0).expect("zero-length bitlist"); + } + let mut result = AggregationBits::with_length(max_len).expect("union exceeds bitlist capacity"); + for i in 0..max_len { + if a.get(i).unwrap_or(false) || b.get(i).unwrap_or(false) { + result.set(i, true).expect("index within capacity"); + } + } + result +} + +/// Compact attestations so each AttestationData appears at most once. +/// +/// For each group of entries sharing the same AttestationData: +/// - Single entry: kept as-is. +/// - Multiple entries with empty proof_data: merged into one with unioned participants. +/// - Multiple entries with real proof_data: keeps only the proof with the most participants +/// (recursive cryptographic aggregation is not yet supported in lean-multisig). +fn compact_attestations( + attestations: Vec, + proofs: Vec, +) -> (Vec, Vec) { + debug_assert_eq!(attestations.len(), proofs.len()); + + if attestations.len() <= 1 { + return (attestations, proofs); + } + + // Group indices by AttestationData, preserving first-occurrence order + let mut order: Vec = Vec::new(); + let mut groups: HashMap> = HashMap::new(); + for (i, att) in attestations.iter().enumerate() { + groups + .entry(att.data.clone()) + .or_insert_with(|| { + order.push(att.data.clone()); + Vec::new() + }) + .push(i); + } + + // Fast path: no duplicates + if order.len() == attestations.len() { + return (attestations, proofs); + } + + let mut compacted_atts = Vec::with_capacity(order.len()); + let mut compacted_proofs = Vec::with_capacity(order.len()); + + for data in &order { + let indices = &groups[data]; + if indices.len() == 1 { + let i = indices[0]; + compacted_atts.push(attestations[i].clone()); + compacted_proofs.push(proofs[i].clone()); + continue; + } + + let all_empty = indices.iter().all(|&i| proofs[i].proof_data.is_empty()); + + if all_empty { + // Merge: union all participant bitfields, empty proof + let mut merged_bits = attestations[indices[0]].aggregation_bits.clone(); + for &idx in &indices[1..] { + merged_bits = + union_aggregation_bits(&merged_bits, &attestations[idx].aggregation_bits); + } + compacted_atts.push(AggregatedAttestation { + aggregation_bits: merged_bits.clone(), + data: data.clone(), + }); + compacted_proofs.push(AggregatedSignatureProof::empty(merged_bits)); + } else { + // Cannot merge real proofs; keep the one with the most participants + let best_idx = indices + .iter() + .copied() + .max_by_key(|&i| proofs[i].participant_indices().count()) + .expect("group is non-empty"); + compacted_atts.push(attestations[best_idx].clone()); + compacted_proofs.push(proofs[best_idx].clone()); + } + } + + (compacted_atts, compacted_proofs) +} + /// Greedily select proofs maximizing new validator coverage. /// /// For a single attestation data entry, picks proofs that cover the most @@ -1056,6 +1166,10 @@ fn build_block( } } + // Compact: ensure each AttestationData appears at most once + let (aggregated_attestations, aggregated_signatures) = + compact_attestations(aggregated_attestations, aggregated_signatures); + // Build final block let attestations: AggregatedAttestations = aggregated_attestations .try_into() @@ -1370,4 +1484,253 @@ mod tests { "source must match head state's justified checkpoint, not store-wide max" ); } + + fn make_att_data(slot: u64) -> AttestationData { + AttestationData { + slot, + head: Checkpoint::default(), + target: Checkpoint::default(), + source: Checkpoint::default(), + } + } + + fn make_bits(indices: &[usize]) -> AggregationBits { + let max = indices.iter().copied().max().unwrap_or(0); + let mut bits = AggregationBits::with_length(max + 1).unwrap(); + for &i in indices { + bits.set(i, true).unwrap(); + } + bits + } + + #[test] + fn compact_attestations_no_duplicates() { + let data_a = make_att_data(1); + let data_b = make_att_data(2); + let bits_a = make_bits(&[0]); + let bits_b = make_bits(&[1]); + + let atts = vec![ + AggregatedAttestation { + aggregation_bits: bits_a.clone(), + data: data_a.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_b.clone(), + data: data_b.clone(), + }, + ]; + let proofs = vec![ + AggregatedSignatureProof::empty(bits_a), + AggregatedSignatureProof::empty(bits_b), + ]; + + let (out_atts, out_proofs) = compact_attestations(atts.clone(), proofs.clone()); + assert_eq!(out_atts.len(), 2); + assert_eq!(out_proofs.len(), 2); + assert_eq!(out_atts[0].data, data_a); + assert_eq!(out_atts[1].data, data_b); + } + + #[test] + fn compact_attestations_merges_empty_proofs() { + let data = make_att_data(1); + let bits_a = make_bits(&[0]); + let bits_b = make_bits(&[1, 2]); + + let atts = vec![ + AggregatedAttestation { + aggregation_bits: bits_a.clone(), + data: data.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_b.clone(), + data: data.clone(), + }, + ]; + let proofs = vec![ + AggregatedSignatureProof::empty(bits_a), + AggregatedSignatureProof::empty(bits_b), + ]; + + let (out_atts, out_proofs) = compact_attestations(atts, proofs); + assert_eq!(out_atts.len(), 1, "should merge into one"); + assert_eq!(out_proofs.len(), 1); + assert_eq!(out_atts[0].data, data); + + // Merged participants should cover validators 0, 1, 2 + let merged = &out_atts[0].aggregation_bits; + assert!(merged.get(0).unwrap()); + assert!(merged.get(1).unwrap()); + assert!(merged.get(2).unwrap()); + assert!(out_proofs[0].proof_data.is_empty()); + } + + #[test] + fn compact_attestations_real_proofs_keeps_best() { + use ethlambda_types::block::ByteListMiB; + + let data = make_att_data(1); + let bits_small = make_bits(&[0]); + let bits_large = make_bits(&[1, 2, 3]); + + let dummy_proof_data: ByteListMiB = vec![0xABu8; 64].try_into().unwrap(); + + let atts = vec![ + AggregatedAttestation { + aggregation_bits: bits_small.clone(), + data: data.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_large.clone(), + data: data.clone(), + }, + ]; + let proofs = vec![ + AggregatedSignatureProof::new(bits_small, dummy_proof_data.clone()), + AggregatedSignatureProof::new(bits_large.clone(), dummy_proof_data), + ]; + + let (out_atts, out_proofs) = compact_attestations(atts, proofs); + assert_eq!(out_atts.len(), 1, "should keep only the best"); + assert_eq!(out_proofs.len(), 1); + + // Should keep the one with 3 participants (validators 1, 2, 3) + assert_eq!(out_atts[0].aggregation_bits, bits_large); + assert!(!out_proofs[0].proof_data.is_empty()); + } + + #[test] + fn compact_attestations_preserves_order() { + let data_a = make_att_data(1); + let data_b = make_att_data(2); + let data_c = make_att_data(3); + + let bits_0 = make_bits(&[0]); + let bits_1 = make_bits(&[1]); + let bits_2 = make_bits(&[2]); + + // Order: A, B, A, C - A has duplicates + let atts = vec![ + AggregatedAttestation { + aggregation_bits: bits_0.clone(), + data: data_a.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_1.clone(), + data: data_b.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_2.clone(), + data: data_a.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_0.clone(), + data: data_c.clone(), + }, + ]; + let proofs = vec![ + AggregatedSignatureProof::empty(bits_0.clone()), + AggregatedSignatureProof::empty(bits_1), + AggregatedSignatureProof::empty(bits_2), + AggregatedSignatureProof::empty(bits_0), + ]; + + let (out_atts, _) = compact_attestations(atts, proofs); + assert_eq!(out_atts.len(), 3); + // First-occurrence order: A, B, C + assert_eq!(out_atts[0].data, data_a); + assert_eq!(out_atts[1].data, data_b); + assert_eq!(out_atts[2].data, data_c); + } + + #[test] + fn on_block_rejects_duplicate_attestation_data() { + use ethlambda_storage::backend::InMemoryBackend; + use std::sync::Arc; + + let genesis_state = State::from_genesis(1000, vec![]); + let genesis_block = Block { + slot: 0, + proposer_index: 0, + parent_root: H256::ZERO, + state_root: H256::ZERO, + body: BlockBody { + attestations: AggregatedAttestations::default(), + }, + }; + let backend = Arc::new(InMemoryBackend::new()); + let mut store = Store::get_forkchoice_store(backend, genesis_state, genesis_block); + + let head_root = store.head(); + let att_data = AttestationData { + slot: 0, + head: Checkpoint { + root: head_root, + slot: 0, + }, + target: Checkpoint { + root: head_root, + slot: 0, + }, + source: Checkpoint { + root: head_root, + slot: 0, + }, + }; + + let bits_a = make_bits(&[0]); + let bits_b = make_bits(&[1]); + + // Two attestations with the SAME data - should be rejected + let attestations = AggregatedAttestations::try_from(vec![ + AggregatedAttestation { + aggregation_bits: bits_a.clone(), + data: att_data.clone(), + }, + AggregatedAttestation { + aggregation_bits: bits_b.clone(), + data: att_data.clone(), + }, + ]) + .unwrap(); + + let attestation_signatures = AttestationSignatures::try_from(vec![ + AggregatedSignatureProof::empty(bits_a), + AggregatedSignatureProof::empty(bits_b), + ]) + .unwrap(); + + let signed_block = SignedBlockWithAttestation { + block: BlockWithAttestation { + block: Block { + slot: 1, + proposer_index: 0, + parent_root: head_root, + state_root: H256::ZERO, + body: BlockBody { attestations }, + }, + proposer_attestation: Attestation { + validator_id: 0, + data: att_data, + }, + }, + signature: BlockSignatures { + attestation_signatures, + proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(), + }, + }; + + let result = on_block_without_verification(&mut store, signed_block); + assert!( + matches!( + result, + Err(StoreError::DuplicateAttestationData { + count: 2, + unique: 1, + }) + ), + "Expected DuplicateAttestationData, got: {result:?}" + ); + } } From 12b88bbc2aca9826ccb7fb4fc0e062f397f66210 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Mon, 6 Apr 2026 17:25:57 -0300 Subject: [PATCH 2/7] Eliminate clones in compact_attestations with Option::take Wrap the parallel attestation/proof vecs into Vec> so items can be moved out by index via .take() instead of cloned. Also use early-exit duplicate detection in on_block_core and avoid a double clone in the grouping loop by cloning from the entry key. --- crates/blockchain/src/store.rs | 79 +++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index a5bcde7..7dd7907 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -521,16 +521,14 @@ fn on_block_core( // Each unique AttestationData must appear at most once per block. let attestations = &signed_block.block.block.body.attestations; - let unique_count = attestations - .iter() - .map(|att| &att.data) - .collect::>() - .len(); - if unique_count != attestations.len() { - return Err(StoreError::DuplicateAttestationData { - count: attestations.len(), - unique: unique_count, - }); + let mut seen = HashSet::with_capacity(attestations.len()); + for att in attestations { + if !seen.insert(&att.data) { + return Err(StoreError::DuplicateAttestationData { + count: attestations.len(), + unique: seen.len(), + }); + } } let sig_verification_start = std::time::Instant::now(); @@ -961,13 +959,15 @@ fn compact_attestations( let mut order: Vec = Vec::new(); let mut groups: HashMap> = HashMap::new(); for (i, att) in attestations.iter().enumerate() { - groups - .entry(att.data.clone()) - .or_insert_with(|| { - order.push(att.data.clone()); - Vec::new() - }) - .push(i); + match groups.entry(att.data.clone()) { + std::collections::hash_map::Entry::Vacant(e) => { + order.push(e.key().clone()); + e.insert(vec![i]); + } + std::collections::hash_map::Entry::Occupied(mut e) => { + e.get_mut().push(i); + } + } } // Fast path: no duplicates @@ -975,41 +975,52 @@ fn compact_attestations( return (attestations, proofs); } + // Wrap in Option so we can .take() items by index without cloning + let mut items: Vec> = + attestations.into_iter().zip(proofs).map(Some).collect(); + let mut compacted_atts = Vec::with_capacity(order.len()); let mut compacted_proofs = Vec::with_capacity(order.len()); - for data in &order { - let indices = &groups[data]; + for data in order { + let indices = &groups[&data]; if indices.len() == 1 { - let i = indices[0]; - compacted_atts.push(attestations[i].clone()); - compacted_proofs.push(proofs[i].clone()); + let (att, proof) = items[indices[0]].take().expect("index used once"); + compacted_atts.push(att); + compacted_proofs.push(proof); continue; } - let all_empty = indices.iter().all(|&i| proofs[i].proof_data.is_empty()); + let all_empty = indices + .iter() + .all(|&i| items[i].as_ref().unwrap().1.proof_data.is_empty()); if all_empty { - // Merge: union all participant bitfields, empty proof - let mut merged_bits = attestations[indices[0]].aggregation_bits.clone(); - for &idx in &indices[1..] { - merged_bits = - union_aggregation_bits(&merged_bits, &attestations[idx].aggregation_bits); + // Merge: take all entries and fold their participant bitfields + let mut merged_bits = None; + for &idx in indices { + let (att, _) = items[idx].take().expect("index used once"); + merged_bits = Some(match merged_bits { + None => att.aggregation_bits, + Some(acc) => union_aggregation_bits(&acc, &att.aggregation_bits), + }); } + let merged_bits = merged_bits.expect("group is non-empty"); + compacted_proofs.push(AggregatedSignatureProof::empty(merged_bits.clone())); compacted_atts.push(AggregatedAttestation { - aggregation_bits: merged_bits.clone(), - data: data.clone(), + aggregation_bits: merged_bits, + data, }); - compacted_proofs.push(AggregatedSignatureProof::empty(merged_bits)); } else { // Cannot merge real proofs; keep the one with the most participants let best_idx = indices .iter() .copied() - .max_by_key(|&i| proofs[i].participant_indices().count()) + .max_by_key(|&i| items[i].as_ref().unwrap().1.participant_indices().count()) .expect("group is non-empty"); - compacted_atts.push(attestations[best_idx].clone()); - compacted_proofs.push(proofs[best_idx].clone()); + let (att, proof) = items[best_idx].take().expect("index used once"); + compacted_atts.push(att); + compacted_proofs.push(proof); } } From ddcae2e14ab80f09595ee8919af935fb1b64f214 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Tue, 7 Apr 2026 14:49:48 -0300 Subject: [PATCH 3/7] Revert LEAN_SPEC_COMMIT_HASH to d39d101 to fix CI The merge from devnet4 bumped the hash to ad9a322 (dual-key commit) which expects test key files with attestation_public/proposal_public fields that don't exist at that revision, causing 83 KeyError failures in the leanSpec test suite. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index dc49e18..cbb0e50 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ docker-build: ## 🐳 Build the Docker image -t ghcr.io/lambdaclass/ethlambda:$(DOCKER_TAG) . @echo -LEAN_SPEC_COMMIT_HASH:=ad9a3226f55e1ba143e0991010ff1f6c2de62941 +LEAN_SPEC_COMMIT_HASH:=d39d10195414921e979e2fdd43723d89cee13c8b leanSpec: git clone https://github.com/leanEthereum/leanSpec.git --single-branch From 172d9b099400f7e859cb325f84fe424e95285df1 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Wed, 8 Apr 2026 15:45:00 -0300 Subject: [PATCH 4/7] Advance XMSS key preparation window before signing (#261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Expose `is_prepared_for(slot)` and `advance_preparation()` on `ValidatorSecretKey`, delegating to the leansig `SignatureSchemeSecretKey` trait - Before signing in `KeyManager::sign_message()`, check if the target slot is within the prepared window and advance if needed - Return a descriptive error if the key's activation interval is fully exhausted - Add a timing test for `advance_preparation()` (526ms in release mode on Apple Silicon) ## Root Cause XMSS keys use a Top-Bottom Tree Traversal scheme where only two consecutive bottom trees are loaded in memory at any time. Each bottom tree covers `sqrt(LIFETIME) = 2^16 = 65,536` slots, so the prepared window spans `131,072` slots (~6 days at 4s/slot). The leansig library provides `advance_preparation()` to slide this window forward by computing the next bottom tree, but ethlambda's `KeyManager` never called it. When the devnet at `admin@ethlambda-1` reached slot 131,072, all 4 nodes panicked simultaneously: ``` Signing: key not yet prepared for this epoch, try calling sk.advance_preparation. ``` The fix checks the prepared interval before every sign operation and advances the window on demand. This is a lazy approach — `advance_preparation` is called at signing time rather than proactively in the tick loop — because: - It happens once every ~3 days (65,536 slots) - The computation (one bottom tree of hash leaves) takes ~526ms in release mode - It keeps the change minimal and avoids tick-loop complexity ## Test plan - [x] `make fmt` clean - [x] `make lint` clean - [x] `make test` passes (existing tests use small lifetimes or skip verification) - [x] `test_advance_preparation_duration` passes (`cargo test -p ethlambda-types test_advance_preparation_duration --release -- --ignored --nocapture`) - [ ] Deploy to devnet with fresh genesis and verify it runs past slot 131,072 without panic --- Cargo.lock | 1 + crates/blockchain/src/key_manager.rs | 18 ++++++ crates/common/types/Cargo.toml | 1 + crates/common/types/src/signature.rs | 86 +++++++++++++++++++++++++++- 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 2ca64b6..19502fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2141,6 +2141,7 @@ dependencies = [ "libssz-derive", "libssz-merkle", "libssz-types", + "rand 0.9.2", "serde", "serde_json", "serde_yaml_ng", diff --git a/crates/blockchain/src/key_manager.rs b/crates/blockchain/src/key_manager.rs index 07eddae..d16deaa 100644 --- a/crates/blockchain/src/key_manager.rs +++ b/crates/blockchain/src/key_manager.rs @@ -5,6 +5,7 @@ use ethlambda_types::{ primitives::{H256, HashTreeRoot as _}, signature::{ValidatorSecretKey, ValidatorSignature}, }; +use tracing::info; use crate::metrics; @@ -102,6 +103,23 @@ impl KeyManager { .get_mut(&validator_id) .ok_or(KeyManagerError::ValidatorKeyNotFound(validator_id))?; + // Advance XMSS key preparation window if the slot is outside the current window. + // Each bottom tree covers 65,536 slots; the window holds 2 at a time. + // Multiple advances may be needed if the node was offline for an extended period. + if !secret_key.is_prepared_for(slot) { + info!(validator_id, slot, "Advancing XMSS key preparation window"); + while !secret_key.is_prepared_for(slot) { + let before = secret_key.get_prepared_interval(); + secret_key.advance_preparation(); + if secret_key.get_prepared_interval() == before { + return Err(KeyManagerError::SigningError(format!( + "XMSS key exhausted for validator {validator_id}: \ + slot {slot} is beyond the key's activation interval" + ))); + } + } + } + let signature: ValidatorSignature = { let _timing = metrics::time_pq_sig_attestation_signing(); secret_key diff --git a/crates/common/types/Cargo.toml b/crates/common/types/Cargo.toml index 6c9d788..ccf6815 100644 --- a/crates/common/types/Cargo.toml +++ b/crates/common/types/Cargo.toml @@ -24,3 +24,4 @@ libssz-types.workspace = true [dev-dependencies] serde_json.workspace = true serde_yaml_ng.workspace = true +rand.workspace = true diff --git a/crates/common/types/src/signature.rs b/crates/common/types/src/signature.rs index d263d66..d2fd852 100644 --- a/crates/common/types/src/signature.rs +++ b/crates/common/types/src/signature.rs @@ -1,6 +1,8 @@ +use std::ops::Range; + use leansig::{ serialization::Serializable, - signature::{SignatureScheme, SigningError}, + signature::{SignatureScheme, SignatureSchemeSecretKey as _, SigningError}, }; use crate::primitives::H256; @@ -97,4 +99,86 @@ impl ValidatorSecretKey { let sig = LeanSignatureScheme::sign(&self.inner, slot, &message.0)?; Ok(ValidatorSignature { inner: sig }) } + + /// Returns true if the key is prepared to sign at the given slot. + /// + /// XMSS keys maintain a sliding window of two bottom trees. Only slots + /// within this window can be signed without advancing the preparation. + pub fn is_prepared_for(&self, slot: u32) -> bool { + self.inner.get_prepared_interval().contains(&(slot as u64)) + } + + /// Returns the slot range currently covered by the prepared window. + pub fn get_prepared_interval(&self) -> Range { + self.inner.get_prepared_interval() + } + + /// Advance the prepared window forward by one bottom tree. + /// + /// Each call slides the window by sqrt(LIFETIME) = 65,536 slots. + /// If the window is already at the end of the key's activation interval, + /// this is a no-op. + pub fn advance_preparation(&mut self) { + self.inner.advance_preparation(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use leansig::serialization::Serializable; + use rand::{SeedableRng, rngs::StdRng}; + + const LEAVES_PER_BOTTOM_TREE: u32 = 1 << 16; // 65,536 + + /// Generate a ValidatorSecretKey with 3 bottom trees so advance_preparation can be tested. + /// + /// This is slow (~minutes) because it computes 3 bottom trees of 65,536 leaves each. + fn generate_key_with_three_bottom_trees() -> ValidatorSecretKey { + let mut rng = StdRng::seed_from_u64(42); + // Request enough active epochs for 3 bottom trees (> 2 * 65,536) + let num_active_epochs = (LEAVES_PER_BOTTOM_TREE as usize) * 2 + 1; + let (_pk, sk) = LeanSignatureScheme::key_gen(&mut rng, 0, num_active_epochs); + let sk_bytes = sk.to_bytes(); + ValidatorSecretKey::from_bytes(&sk_bytes).expect("valid secret key") + } + + #[test] + #[ignore = "slow: generates production-size XMSS key (~minutes)"] + fn test_advance_preparation_duration() { + println!("Generating XMSS key with 3 bottom trees (this takes a while)..."); + let keygen_start = std::time::Instant::now(); + let mut sk = generate_key_with_three_bottom_trees(); + println!("Key generation took: {:?}", keygen_start.elapsed()); + + // Initial window covers [0, 131072) + assert!(sk.is_prepared_for(0)); + assert!(sk.is_prepared_for(LEAVES_PER_BOTTOM_TREE - 1)); + assert!(sk.is_prepared_for(2 * LEAVES_PER_BOTTOM_TREE - 1)); + assert!(!sk.is_prepared_for(2 * LEAVES_PER_BOTTOM_TREE)); + + // Time the advance_preparation call + let advance_start = std::time::Instant::now(); + sk.advance_preparation(); + let advance_duration = advance_start.elapsed(); + + println!("advance_preparation() took: {advance_duration:?}"); + + // Window should now cover [65536, 196608) + assert!(!sk.is_prepared_for(0)); + assert!(sk.is_prepared_for(LEAVES_PER_BOTTOM_TREE)); + assert!(sk.is_prepared_for(3 * LEAVES_PER_BOTTOM_TREE - 1)); + + // Verify signing works in the new window + let message = H256::from([42u8; 32]); + let slot = 2 * LEAVES_PER_BOTTOM_TREE; // slot 131,072 — the one that crashed the devnet + let sign_start = std::time::Instant::now(); + let result = sk.sign(slot, &message); + println!("Signing at slot {slot} took: {:?}", sign_start.elapsed()); + assert!( + result.is_ok(), + "signing should succeed after advance: {}", + result.err().map_or(String::new(), |e| e.to_string()) + ); + } } From f00fbb0e300a2215e73a8f304da388589238705e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 8 Apr 2026 15:56:29 -0300 Subject: [PATCH 5/7] fix: handle big messages in P2P layer (#264) This PR adds additional "big message" checks in the P2P layer: - when encoding a payload, check it doesn't go over the max uncompressed size - when responding to `BlocksByRoot` requests, skip big blocks None of these code paths should be triggered during normal operation, but they improve network stability by keeping errors local. --- crates/net/p2p/src/req_resp/codec.rs | 19 +++++++++++++++---- crates/net/p2p/src/req_resp/encoding.rs | 8 ++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/net/p2p/src/req_resp/codec.rs b/crates/net/p2p/src/req_resp/codec.rs index 9f3214f..ddb5a02 100644 --- a/crates/net/p2p/src/req_resp/codec.rs +++ b/crates/net/p2p/src/req_resp/codec.rs @@ -2,10 +2,10 @@ use std::io; use libp2p::futures::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use libssz::{SszDecode, SszEncode}; -use tracing::{debug, trace}; +use tracing::{debug, trace, warn}; use super::{ - encoding::{decode_payload, write_payload}, + encoding::{MAX_PAYLOAD_SIZE, decode_payload, write_payload}, messages::{ BLOCKS_BY_ROOT_PROTOCOL_V1, ErrorMessage, Request, Response, ResponseCode, ResponsePayload, STATUS_PROTOCOL_V1, Status, @@ -109,10 +109,21 @@ impl libp2p::request_response::Codec for Codec { write_payload(io, &encoded).await } ResponsePayload::BlocksByRoot(blocks) => { - // Write each block as separate chunk + // Write each block as a separate chunk. + // Encode first, then check size before writing the SUCCESS + // code byte. This avoids corrupting the stream if a block + // exceeds MAX_PAYLOAD_SIZE (the SUCCESS byte would already + // be on the wire with no payload following). for block in blocks { - io.write_all(&[ResponseCode::SUCCESS.into()]).await?; let encoded = block.to_ssz(); + if encoded.len() > MAX_PAYLOAD_SIZE - 1024 { + warn!( + size = encoded.len(), + "Skipping oversized block in BlocksByRoot response" + ); + continue; + } + io.write_all(&[ResponseCode::SUCCESS.into()]).await?; write_payload(io, &encoded).await?; } // Empty response if no blocks found (stream just ends) diff --git a/crates/net/p2p/src/req_resp/encoding.rs b/crates/net/p2p/src/req_resp/encoding.rs index b45d447..7a4116c 100644 --- a/crates/net/p2p/src/req_resp/encoding.rs +++ b/crates/net/p2p/src/req_resp/encoding.rs @@ -52,6 +52,14 @@ where T: AsyncWrite + Unpin, { let uncompressed_size = encoded.len(); + // Stop ourselves from sending messages our peers won't receive. + // Leave some leeway for response codes and the varint encoding of the size. + if uncompressed_size > MAX_PAYLOAD_SIZE - 1024 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "message size exceeds maximum allowed", + )); + } let mut compressor = FrameEncoder::new(encoded); let mut buf = Vec::new(); From ed2712afa58d73e7052581ef06d54f97fc620348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Wed, 8 Apr 2026 16:27:00 -0300 Subject: [PATCH 6/7] fix: do not subscribe to subnets when not aggregating (#265) This PR removes subnet subscribing when we're not an aggregator. It was already fixed in #160, but was re-introduced in #249 --- crates/net/p2p/src/lib.rs | 57 ++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/crates/net/p2p/src/lib.rs b/crates/net/p2p/src/lib.rs index e74216e..c2a36b8 100644 --- a/crates/net/p2p/src/lib.rs +++ b/crates/net/p2p/src/lib.rs @@ -205,40 +205,49 @@ pub fn build_swarm( .subscribe(&aggregation_topic) .unwrap(); - // Compute the set of subnets to subscribe to. - // Validators subscribe for gossipsub mesh health; aggregators additionally - // subscribe to any explicitly requested subnets. - let validator_subnets: HashSet = config - .validator_ids - .iter() - .map(|vid| vid % config.attestation_committee_count) - .collect(); - - let mut subscribe_subnets: HashSet = validator_subnets.clone(); - + // Aggregators subscribe to attestation subnets to receive unaggregated + // attestations. Non-aggregators don't need to subscribe; they publish via + // gossipsub fanout. if config.is_aggregator { + let mut aggregate_subnets: HashSet = config + .validator_ids + .iter() + .map(|vid| vid % config.attestation_committee_count) + .collect(); if let Some(ref explicit_ids) = config.aggregate_subnet_ids { - subscribe_subnets.extend(explicit_ids); + aggregate_subnets.extend(explicit_ids); } // Aggregator with no validators and no explicit subnets: fallback to subnet 0 - if subscribe_subnets.is_empty() { - subscribe_subnets.insert(0); + if aggregate_subnets.is_empty() { + aggregate_subnets.insert(0); + } + for &subnet_id in &aggregate_subnets { + let topic = attestation_subnet_topic(subnet_id); + swarm.behaviour_mut().gossipsub.subscribe(&topic)?; + info!(subnet_id, "Subscribed to attestation subnet"); } } - // Report lowest validator subnet for backward-compatible metric - let metric_subnet = validator_subnets.iter().copied().min().unwrap_or(0); - metrics::set_attestation_committee_subnet(metric_subnet); - - // Build topics and subscribe + // Build topic cache (avoids string allocation on every publish). + // Includes validator subnets and any explicit aggregate_subnet_ids. let mut attestation_topics: HashMap = HashMap::new(); - for &subnet_id in &subscribe_subnets { - let topic = attestation_subnet_topic(subnet_id); - swarm.behaviour_mut().gossipsub.subscribe(&topic)?; - info!(subnet_id, "Subscribed to attestation subnet"); - attestation_topics.insert(subnet_id, topic); + for &vid in &config.validator_ids { + let subnet_id = vid % config.attestation_committee_count; + attestation_topics + .entry(subnet_id) + .or_insert_with(|| attestation_subnet_topic(subnet_id)); + } + if let Some(ref explicit_ids) = config.aggregate_subnet_ids { + for &subnet_id in explicit_ids { + attestation_topics + .entry(subnet_id) + .or_insert_with(|| attestation_subnet_topic(subnet_id)); + } } + let metric_subnet = attestation_topics.keys().copied().min().unwrap_or(0); + metrics::set_attestation_committee_subnet(metric_subnet); + info!(socket=%config.listening_socket, "P2P node started"); Ok(BuiltSwarm { From ead8da30f000a9ae98f4f2b12a7166ed2eb81274 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Wed, 8 Apr 2026 18:51:27 -0300 Subject: [PATCH 7/7] Remove unnecessary real-proofs branch from compact_attestations The else branch handling multiple real proofs for the same AttestationData is not in the spec and doesn't arise in practice (skip-sig and real-proof modes are mutually exclusive). Simplify to always merge via union of participant bitfields. --- crates/blockchain/src/store.rs | 80 ++++++---------------------------- 1 file changed, 14 insertions(+), 66 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 581a223..a29564f 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -951,9 +951,7 @@ fn union_aggregation_bits(a: &AggregationBits, b: &AggregationBits) -> Aggregati /// /// For each group of entries sharing the same AttestationData: /// - Single entry: kept as-is. -/// - Multiple entries with empty proof_data: merged into one with unioned participants. -/// - Multiple entries with real proof_data: keeps only the proof with the most participants -/// (recursive cryptographic aggregation is not yet supported in lean-multisig). +/// - Multiple entries: merged into one with unioned participant bitfields. fn compact_attestations( attestations: Vec, proofs: Vec, @@ -1000,37 +998,21 @@ fn compact_attestations( continue; } - let all_empty = indices - .iter() - .all(|&i| items[i].as_ref().unwrap().1.proof_data.is_empty()); - - if all_empty { - // Merge: take all entries and fold their participant bitfields - let mut merged_bits = None; - for &idx in indices { - let (att, _) = items[idx].take().expect("index used once"); - merged_bits = Some(match merged_bits { - None => att.aggregation_bits, - Some(acc) => union_aggregation_bits(&acc, &att.aggregation_bits), - }); - } - let merged_bits = merged_bits.expect("group is non-empty"); - compacted_proofs.push(AggregatedSignatureProof::empty(merged_bits.clone())); - compacted_atts.push(AggregatedAttestation { - aggregation_bits: merged_bits, - data, + // Merge: take all entries and fold their participant bitfields + let mut merged_bits = None; + for &idx in indices { + let (att, _) = items[idx].take().expect("index used once"); + merged_bits = Some(match merged_bits { + None => att.aggregation_bits, + Some(acc) => union_aggregation_bits(&acc, &att.aggregation_bits), }); - } else { - // Cannot merge real proofs; keep the one with the most participants - let best_idx = indices - .iter() - .copied() - .max_by_key(|&i| items[i].as_ref().unwrap().1.participant_indices().count()) - .expect("group is non-empty"); - let (att, proof) = items[best_idx].take().expect("index used once"); - compacted_atts.push(att); - compacted_proofs.push(proof); } + let merged_bits = merged_bits.expect("group is non-empty"); + compacted_proofs.push(AggregatedSignatureProof::empty(merged_bits.clone())); + compacted_atts.push(AggregatedAttestation { + aggregation_bits: merged_bits, + data, + }); } (compacted_atts, compacted_proofs) @@ -1748,40 +1730,6 @@ mod tests { assert!(out_proofs[0].proof_data.is_empty()); } - #[test] - fn compact_attestations_real_proofs_keeps_best() { - use ethlambda_types::block::ByteListMiB; - - let data = make_att_data(1); - let bits_small = make_bits(&[0]); - let bits_large = make_bits(&[1, 2, 3]); - - let dummy_proof_data: ByteListMiB = vec![0xABu8; 64].try_into().unwrap(); - - let atts = vec![ - AggregatedAttestation { - aggregation_bits: bits_small.clone(), - data: data.clone(), - }, - AggregatedAttestation { - aggregation_bits: bits_large.clone(), - data: data.clone(), - }, - ]; - let proofs = vec![ - AggregatedSignatureProof::new(bits_small, dummy_proof_data.clone()), - AggregatedSignatureProof::new(bits_large.clone(), dummy_proof_data), - ]; - - let (out_atts, out_proofs) = compact_attestations(atts, proofs); - assert_eq!(out_atts.len(), 1, "should keep only the best"); - assert_eq!(out_proofs.len(), 1); - - // Should keep the one with 3 participants (validators 1, 2, 3) - assert_eq!(out_atts[0].aggregation_bits, bits_large); - assert!(!out_proofs[0].proof_data.is_empty()); - } - #[test] fn compact_attestations_preserves_order() { let data_a = make_att_data(1);