From 12ee2fa7e49fab41058a9b43f3829756806f283c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 7 Apr 2026 17:05:48 -0300 Subject: [PATCH 1/5] Add regression test for oversized blocks during stall recovery (#259) build_block() greedily includes every matching attestation from the payload pool with no size cap. During stall recovery the pool can grow large enough that the SSZ-encoded SignedBlockWithAttestation exceeds MAX_PAYLOAD_SIZE (10 MiB), causing gossip publish failures. The test simulates a stall by populating the pool with 50 entries carrying ~253 KB proofs each, producing a 12.4 MiB block. It asserts the block must stay under the limit, so it fails until a size cap is enforced in build_block. --- Cargo.lock | 2 + crates/blockchain/Cargo.toml | 2 + crates/blockchain/src/store.rs | 145 +++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 9babc5a0..2ca64b6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2000,6 +2000,8 @@ dependencies = [ "ethlambda-test-fixtures", "ethlambda-types", "hex", + "libssz", + "libssz-types", "serde", "serde_json", "spawned-concurrency 0.5.0", diff --git a/crates/blockchain/Cargo.toml b/crates/blockchain/Cargo.toml index f4e0751c..d565123d 100644 --- a/crates/blockchain/Cargo.toml +++ b/crates/blockchain/Cargo.toml @@ -33,6 +33,8 @@ ethlambda-test-fixtures.workspace = true serde = { workspace = true } serde_json = { workspace = true } hex = { workspace = true } +libssz.workspace = true +libssz-types.workspace = true datatest-stable = "0.3.3" [[test]] diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 6f114cfb..1fdf727c 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -1304,6 +1304,151 @@ mod tests { ); } + /// Regression test for https://github.com/lambdaclass/ethlambda/issues/259 + /// + /// build_block greedily includes every matching attestation from the payload + /// pool with no size cap. During stall recovery the pool can grow large + /// enough that the SSZ-encoded SignedBlockWithAttestation exceeds the + /// MAX_PAYLOAD_SIZE (10 MiB), causing gossip publish failures and peer + /// rejection. + /// + /// This test simulates a stall scenario by populating the payload pool with + /// 50 distinct attestation entries, each carrying a ~253 KB proof (realistic + /// XMSS aggregated proof size). build_block includes all of them, producing + /// a block whose wire encoding exceeds the 10 MiB limit. + #[test] + fn build_block_exceeds_max_payload_size_during_stall() { + use libssz::SszEncode; + use libssz_types::SszList; + + const MAX_PAYLOAD_SIZE: usize = 10 * 1024 * 1024; // 10 MiB (spec limit) + const PROOF_SIZE: usize = 253 * 1024; // ~253 KB realistic XMSS proof + const NUM_VALIDATORS: usize = 50; + const NUM_PAYLOAD_ENTRIES: usize = 50; + + // Create genesis state with NUM_VALIDATORS validators. + let validators: Vec<_> = (0..NUM_VALIDATORS) + .map(|i| ethlambda_types::state::Validator { + pubkey: [i as u8; 52], + index: i as u64, + }) + .collect(); + let head_state = State::from_genesis(1000, validators); + + // process_slots fills in the genesis header's state_root before + // process_block_header computes the parent hash. Simulate that here. + let mut header_for_root = head_state.latest_block_header.clone(); + header_for_root.state_root = head_state.hash_tree_root(); + let parent_root = header_for_root.hash_tree_root(); + + // Proposer for slot 1 with NUM_VALIDATORS validators: 1 % 50 = 1 + let proposer_index = 1u64; + let slot = 1u64; + + // The genesis edge case in build_block sets current_justified to: + // Checkpoint { root: parent_root, slot: 0 } + let source = Checkpoint { + root: parent_root, + slot: 0, + }; + + let mut known_block_roots = HashSet::new(); + known_block_roots.insert(parent_root); + + // Simulate a stall: populate the payload pool with many distinct entries. + // Each has a unique target (different slot) and a large proof payload. + let mut aggregated_payloads: HashMap)> = + HashMap::new(); + + for i in 0..NUM_PAYLOAD_ENTRIES { + let target_slot = (i + 1) as u64; + let att_data = AttestationData { + slot: target_slot, + head: Checkpoint { + root: parent_root, + slot: 0, + }, + target: Checkpoint { + root: H256([target_slot as u8; 32]), + slot: target_slot, + }, + source, + }; + + // Use the real hash_tree_root as the data_root key + let data_root = att_data.hash_tree_root(); + + // Create a single large proof per entry (one validator per proof) + let validator_id = i % NUM_VALIDATORS; + let mut bits = AggregationBits::with_length(NUM_VALIDATORS).unwrap(); + bits.set(validator_id, true).unwrap(); + + let proof_bytes: Vec = vec![0xAB; PROOF_SIZE]; + let proof_data = SszList::try_from(proof_bytes).expect("proof fits in ByteListMiB"); + let proof = AggregatedSignatureProof::new(bits, proof_data); + + aggregated_payloads.insert(data_root, (att_data, vec![proof])); + } + + // Build the block; this should succeed (the bug: no size guard) + let (block, signatures) = build_block( + &head_state, + slot, + proposer_index, + parent_root, + &known_block_roots, + &aggregated_payloads, + ) + .expect("build_block should succeed"); + + // The block should have included all attestation entries + assert!( + block.body.attestations.len() > 0, + "block should contain attestations from the payload pool" + ); + + // Construct the full signed block as it would be sent over gossip + let attestation_sigs: Vec = signatures; + let signed_block = SignedBlockWithAttestation { + block: BlockWithAttestation { + block, + proposer_attestation: Attestation { + validator_id: proposer_index, + data: AttestationData { + slot, + head: Checkpoint { + root: parent_root, + slot: 0, + }, + target: Checkpoint { + root: parent_root, + slot: 0, + }, + source, + }, + }, + }, + signature: BlockSignatures { + attestation_signatures: AttestationSignatures::try_from(attestation_sigs).unwrap(), + proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(), + }, + }; + + // SSZ-encode: this is exactly what publish_block does before compression + let ssz_bytes = signed_block.to_ssz(); + + // build_block must not produce blocks that exceed the gossip wire limit. + assert!( + ssz_bytes.len() <= MAX_PAYLOAD_SIZE, + "block with {} attestations is {} bytes SSZ, \ + which exceeds MAX_PAYLOAD_SIZE ({} bytes). \ + build_block must enforce a size cap (issue #259).", + signed_block.block.block.body.attestations.len(), + ssz_bytes.len(), + MAX_PAYLOAD_SIZE, + ); + } + /// Attestation source must come from the head state's justified checkpoint, /// not the store-wide global max. /// From f719c6dbbb66b37cf9ab93326f8fa29332f171b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 7 Apr 2026 17:45:07 -0300 Subject: [PATCH 2/5] Cap attestations per block to 24 to prevent oversized blocks (#259) With XMSS proofs averaging ~253 KB each, an uncapped block builder can produce blocks exceeding the 10 MiB MAX_PAYLOAD_SIZE gossip limit during stall recovery. Cap build_block to MAX_ATTESTATIONS_PER_BLOCK = 24, which yields ~6 MiB and leaves headroom for header and proposer signature. The budget is threaded into extend_proofs_greedily so it stops selecting proofs once the cap is reached. --- crates/blockchain/src/store.rs | 49 ++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 1fdf727c..3c9b0d71 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -26,6 +26,16 @@ use crate::{INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT const JUSTIFICATION_LOOKBACK_SLOTS: u64 = 3; +/// Maximum number of aggregated attestations (and corresponding signature proofs) +/// that build_block will include in a single block. +/// +/// With XMSS aggregated proofs averaging ~253 KB each, 24 attestations produce +/// a block of ~6 MiB SSZ, well within the 10 MiB MAX_PAYLOAD_SIZE gossip limit +/// while leaving headroom for the proposer signature and block header. +/// +/// See: https://github.com/lambdaclass/ethlambda/issues/259 +const MAX_ATTESTATIONS_PER_BLOCK: usize = 24; + /// Accept new aggregated payloads, promoting them to known for fork choice. fn accept_new_attestations(store: &mut Store, log_tree: bool) { store.promote_new_aggregated_payloads(); @@ -915,15 +925,17 @@ fn extend_proofs_greedily( selected_proofs: &mut Vec, attestations: &mut Vec, att_data: &AttestationData, + budget: usize, ) { - if proofs.is_empty() { + if proofs.is_empty() || budget == 0 { return; } let mut covered: HashSet = HashSet::new(); let mut remaining_indices: HashSet = (0..proofs.len()).collect(); + let mut added = 0; - while !remaining_indices.is_empty() { + while !remaining_indices.is_empty() && added < budget { // Pick proof covering the most uncovered validators (count only, no allocation) let best = remaining_indices .iter() @@ -961,6 +973,7 @@ fn extend_proofs_greedily( covered.extend(new_covered); remaining_indices.remove(&best_idx); + added += 1; } } @@ -1006,6 +1019,11 @@ fn build_block( let mut found_new = false; for &(data_root, (att_data, proofs)) in &sorted_entries { + let remaining = + MAX_ATTESTATIONS_PER_BLOCK.saturating_sub(aggregated_attestations.len()); + if remaining == 0 { + break; + } if processed_data_roots.contains(data_root) { continue; } @@ -1024,6 +1042,7 @@ fn build_block( &mut aggregated_signatures, &mut aggregated_attestations, att_data, + remaining, ); } @@ -1306,18 +1325,13 @@ mod tests { /// Regression test for https://github.com/lambdaclass/ethlambda/issues/259 /// - /// build_block greedily includes every matching attestation from the payload - /// pool with no size cap. During stall recovery the pool can grow large - /// enough that the SSZ-encoded SignedBlockWithAttestation exceeds the - /// MAX_PAYLOAD_SIZE (10 MiB), causing gossip publish failures and peer - /// rejection. - /// - /// This test simulates a stall scenario by populating the payload pool with - /// 50 distinct attestation entries, each carrying a ~253 KB proof (realistic - /// XMSS aggregated proof size). build_block includes all of them, producing - /// a block whose wire encoding exceeds the 10 MiB limit. + /// Simulates a stall scenario by populating the payload pool with 50 + /// distinct attestation entries, each carrying a ~253 KB proof (realistic + /// XMSS aggregated proof size). Without the MAX_ATTESTATIONS_PER_BLOCK cap + /// this would produce a 12.4 MiB block, exceeding the 10 MiB gossip limit. + /// Verifies that build_block respects the cap and stays under the limit. #[test] - fn build_block_exceeds_max_payload_size_during_stall() { + fn build_block_respects_max_payload_size_during_stall() { use libssz::SszEncode; use libssz_types::SszList; @@ -1401,10 +1415,11 @@ mod tests { ) .expect("build_block should succeed"); - // The block should have included all attestation entries - assert!( - block.body.attestations.len() > 0, - "block should contain attestations from the payload pool" + // The cap should have been enforced: 24 out of 50 entries included + assert_eq!( + block.body.attestations.len(), + MAX_ATTESTATIONS_PER_BLOCK, + "block should contain exactly MAX_ATTESTATIONS_PER_BLOCK attestations" ); // Construct the full signed block as it would be sent over gossip From 71d4ed31d8eca7d1310f1665f30bf33247854d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 7 Apr 2026 17:52:29 -0300 Subject: [PATCH 3/5] fmt --- crates/blockchain/src/store.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 3c9b0d71..16d17ca4 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -1371,8 +1371,10 @@ mod tests { // Simulate a stall: populate the payload pool with many distinct entries. // Each has a unique target (different slot) and a large proof payload. - let mut aggregated_payloads: HashMap)> = - HashMap::new(); + let mut aggregated_payloads: HashMap< + H256, + (AttestationData, Vec), + > = HashMap::new(); for i in 0..NUM_PAYLOAD_ENTRIES { let target_slot = (i + 1) as u64; From c7df081938c695ea12d63b974e56684511814c22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 7 Apr 2026 18:06:21 -0300 Subject: [PATCH 4/5] Switch to byte-based cap for attestation proof budget (#259) Replace the fixed count cap (MAX_ATTESTATIONS_PER_BLOCK = 24) with a byte budget (MAX_ATTESTATION_PROOF_BYTES = 9 MiB) that tracks cumulative proof_data.len() as proofs are added. This adapts automatically to varying proof sizes instead of assuming ~253 KB averages. The 9 MiB budget leaves 1 MiB margin within the 10 MiB MAX_PAYLOAD_SIZE gossip limit for block header, proposer signature, attestation metadata, and SSZ encoding overhead. --- crates/blockchain/src/store.rs | 65 +++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 16d17ca4..3c009a0f 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -26,15 +26,14 @@ use crate::{INTERVALS_PER_SLOT, MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT const JUSTIFICATION_LOOKBACK_SLOTS: u64 = 3; -/// Maximum number of aggregated attestations (and corresponding signature proofs) -/// that build_block will include in a single block. +/// Maximum bytes of attestation proof data that build_block will accumulate. /// -/// With XMSS aggregated proofs averaging ~253 KB each, 24 attestations produce -/// a block of ~6 MiB SSZ, well within the 10 MiB MAX_PAYLOAD_SIZE gossip limit -/// while leaving headroom for the proposer signature and block header. +/// Derived from the 10 MiB MAX_PAYLOAD_SIZE gossip limit with a 1 MiB margin +/// for the block header, proposer signature, attestation metadata, bitlists, +/// and SSZ encoding overhead. /// /// See: https://github.com/lambdaclass/ethlambda/issues/259 -const MAX_ATTESTATIONS_PER_BLOCK: usize = 24; +const MAX_ATTESTATION_PROOF_BYTES: usize = 9 * 1024 * 1024; /// Accept new aggregated payloads, promoting them to known for fork choice. fn accept_new_attestations(store: &mut Store, log_tree: bool) { @@ -920,22 +919,23 @@ fn aggregation_bits_from_validator_indices(bits: &[u64]) -> AggregationBits { /// /// For a single attestation data entry, picks proofs that cover the most /// uncovered validators. Each selected proof produces one AggregatedAttestation. +/// Returns the total proof_data bytes consumed. fn extend_proofs_greedily( proofs: &[AggregatedSignatureProof], selected_proofs: &mut Vec, attestations: &mut Vec, att_data: &AttestationData, - budget: usize, -) { - if proofs.is_empty() || budget == 0 { - return; + remaining_bytes: usize, +) -> usize { + if proofs.is_empty() || remaining_bytes == 0 { + return 0; } let mut covered: HashSet = HashSet::new(); let mut remaining_indices: HashSet = (0..proofs.len()).collect(); - let mut added = 0; + let mut bytes_consumed = 0; - while !remaining_indices.is_empty() && added < budget { + while !remaining_indices.is_empty() { // Pick proof covering the most uncovered validators (count only, no allocation) let best = remaining_indices .iter() @@ -955,13 +955,18 @@ fn extend_proofs_greedily( break; } + let proof = &proofs[best_idx]; + let proof_bytes = proof.proof_data.len(); + if bytes_consumed + proof_bytes > remaining_bytes { + break; + } + // Collect coverage only for the winning proof - let new_covered: Vec = proofs[best_idx] + let new_covered: Vec = proof .participant_indices() .filter(|vid| !covered.contains(vid)) .collect(); - let proof = &proofs[best_idx]; attestations.push(AggregatedAttestation { aggregation_bits: proof.participants.clone(), data: att_data.clone(), @@ -973,8 +978,10 @@ fn extend_proofs_greedily( covered.extend(new_covered); remaining_indices.remove(&best_idx); - added += 1; + bytes_consumed += proof_bytes; } + + bytes_consumed } /// Build a valid block on top of this state. @@ -995,6 +1002,7 @@ fn build_block( ) -> Result<(Block, Vec), StoreError> { let mut aggregated_attestations: Vec = Vec::new(); let mut aggregated_signatures: Vec = Vec::new(); + let mut accumulated_proof_bytes: usize = 0; if !aggregated_payloads.is_empty() { // Genesis edge case: when building on genesis (slot 0), @@ -1019,11 +1027,10 @@ fn build_block( let mut found_new = false; for &(data_root, (att_data, proofs)) in &sorted_entries { - let remaining = - MAX_ATTESTATIONS_PER_BLOCK.saturating_sub(aggregated_attestations.len()); - if remaining == 0 { + if accumulated_proof_bytes >= MAX_ATTESTATION_PROOF_BYTES { break; } + let remaining_bytes = MAX_ATTESTATION_PROOF_BYTES - accumulated_proof_bytes; if processed_data_roots.contains(data_root) { continue; } @@ -1037,16 +1044,17 @@ fn build_block( processed_data_roots.insert(*data_root); found_new = true; - extend_proofs_greedily( + let consumed = extend_proofs_greedily( proofs, &mut aggregated_signatures, &mut aggregated_attestations, att_data, - remaining, + remaining_bytes, ); + accumulated_proof_bytes += consumed; } - if !found_new { + if !found_new || accumulated_proof_bytes >= MAX_ATTESTATION_PROOF_BYTES { break; } @@ -1327,8 +1335,8 @@ mod tests { /// /// Simulates a stall scenario by populating the payload pool with 50 /// distinct attestation entries, each carrying a ~253 KB proof (realistic - /// XMSS aggregated proof size). Without the MAX_ATTESTATIONS_PER_BLOCK cap - /// this would produce a 12.4 MiB block, exceeding the 10 MiB gossip limit. + /// XMSS aggregated proof size). Without the byte budget cap this would + /// produce a 12.4 MiB block, exceeding the 10 MiB gossip limit. /// Verifies that build_block respects the cap and stays under the limit. #[test] fn build_block_respects_max_payload_size_during_stall() { @@ -1417,11 +1425,12 @@ mod tests { ) .expect("build_block should succeed"); - // The cap should have been enforced: 24 out of 50 entries included - assert_eq!( - block.body.attestations.len(), - MAX_ATTESTATIONS_PER_BLOCK, - "block should contain exactly MAX_ATTESTATIONS_PER_BLOCK attestations" + // The byte budget should have been enforced: fewer than 50 entries included + let attestation_count = block.body.attestations.len(); + assert!(attestation_count > 0, "block should contain attestations"); + assert!( + attestation_count < NUM_PAYLOAD_ENTRIES, + "byte budget should have capped attestations below the pool size" ); // Construct the full signed block as it would be sent over gossip From 7ef06750e01ff5f6695d60a39111755c35e85b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 7 Apr 2026 18:11:45 -0300 Subject: [PATCH 5/5] refactor: move let to use site --- crates/blockchain/src/store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 3c009a0f..2b841aab 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -1030,7 +1030,6 @@ fn build_block( if accumulated_proof_bytes >= MAX_ATTESTATION_PROOF_BYTES { break; } - let remaining_bytes = MAX_ATTESTATION_PROOF_BYTES - accumulated_proof_bytes; if processed_data_roots.contains(data_root) { continue; } @@ -1044,6 +1043,7 @@ fn build_block( processed_data_roots.insert(*data_root); found_new = true; + let remaining_bytes = MAX_ATTESTATION_PROOF_BYTES - accumulated_proof_bytes; let consumed = extend_proofs_greedily( proofs, &mut aggregated_signatures,