From aff65946382d13295c253f3b69f8f6424e4417b5 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Thu, 26 Feb 2026 20:37:50 -0300 Subject: [PATCH 1/6] Split on_block() and on_gossip_attestation() into verified and unverified variants, removing the skip-signature-verification compile-time feature flag entirely. on_block() and on_gossip_attestation() now always verify signatures (safe default). on_block_without_verification() and on_gossip_attestation_without_verification() provide explicit opt-outs for tests where signatures are absent. Shared block processing logic is extracted into a private on_block_core() that takes parent_state by value to avoid cloning. --- CLAUDE.md | 11 +- Makefile | 4 +- RELEASE.md | 6 +- crates/blockchain/Cargo.toml | 5 - crates/blockchain/src/store.rs | 166 +++++++++++++----- .../blockchain/tests/forkchoice_spectests.rs | 66 +++---- 6 files changed, 161 insertions(+), 97 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 19864d7..9509c54 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,7 +74,7 @@ Fork choice head update ```bash make fmt # Format code (cargo fmt --all) make lint # Clippy with -D warnings -make test # All tests + forkchoice (with skip-signature-verification) +make test # All tests + forkchoice spec tests ``` ### Common Operations @@ -295,21 +295,22 @@ GENESIS_VALIDATORS: ### Test Categories 1. **Unit tests**: Embedded in source files 2. **Spec tests**: From `leanSpec/fixtures/consensus/` - - `forkchoice_spectests.rs` (requires `skip-signature-verification`) + - `forkchoice_spectests.rs` (uses `on_block_without_verification`) - `signature_spectests.rs` - `stf_spectests.rs` (state transition) ### Running Tests ```bash cargo test --workspace --release # All workspace tests -cargo test -p ethlambda-blockchain --features skip-signature-verification --test forkchoice_spectests -cargo test -p ethlambda-blockchain --features skip-signature-verification --test forkchoice_spectests -- --test-threads=1 # Sequential +cargo test -p ethlambda-blockchain --test forkchoice_spectests +cargo test -p ethlambda-blockchain --test forkchoice_spectests -- --test-threads=1 # Sequential ``` ## Common Gotchas ### Signature Verification -- Tests require `skip-signature-verification` feature for performance +- Fork choice tests use `on_block_without_verification()` to skip signature checks +- Signature spec tests use `on_block()` which always verifies - Crypto tests marked `#[ignore]` (slow leanVM operations) ### Storage Architecture diff --git a/Makefile b/Makefile index 36bdc13..cac407c 100644 --- a/Makefile +++ b/Makefile @@ -9,10 +9,10 @@ fmt: ## 🎨 Format all code using rustfmt lint: ## 🔍 Run clippy on all workspace crates cargo clippy --workspace --all-targets -- -D warnings -test: leanSpec/fixtures ## 🧪 Run all tests, then forkchoice tests with skip-signature-verification +test: leanSpec/fixtures ## 🧪 Run all tests, then forkchoice spec tests # Tests need to be run on release to avoid stack overflows during signature verification/aggregation cargo test --workspace --release - cargo test -p ethlambda-blockchain --features skip-signature-verification --test forkchoice_spectests + cargo test -p ethlambda-blockchain --test forkchoice_spectests GIT_COMMIT=$(shell git rev-parse HEAD) GIT_BRANCH=$(shell git rev-parse --abbrev-ref HEAD) diff --git a/RELEASE.md b/RELEASE.md index e720f6d..09dbd65 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -67,15 +67,11 @@ The Dockerfile accepts build arguments for customizing the build: | Argument | Default | Description | |----------|---------|-------------| | `BUILD_PROFILE` | `release` | Cargo build profile | -| `FEATURES` | `""` | Extra Cargo features (e.g. `skip-signature-verification`) | +| `FEATURES` | `""` | Extra Cargo features | | `RUSTFLAGS` | `""` | Extra Rust compiler flags | Example with custom args: -```bash -docker build --build-arg FEATURES=skip-signature-verification -t ethlambda:test . -``` - `GIT_COMMIT` and `GIT_BRANCH` are also available but set automatically by CI. When building locally, `vergen-git2` extracts them from the local Git repo at build time. diff --git a/crates/blockchain/Cargo.toml b/crates/blockchain/Cargo.toml index d8e0825..8f28be5 100644 --- a/crates/blockchain/Cargo.toml +++ b/crates/blockchain/Cargo.toml @@ -10,10 +10,6 @@ rust-version.workspace = true version.workspace = true autotests = false -[features] -# To skip signature verification during tests -skip-signature-verification = [] - [dependencies] ethlambda-storage.workspace = true ethlambda-state-transition.workspace = true @@ -45,7 +41,6 @@ ssz_types = "0.14.0" name = "forkchoice_spectests" path = "tests/forkchoice_spectests.rs" harness = false -required-features = ["skip-signature-verification"] [[test]] name = "signature_spectests" diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index a7208da..4901d01 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -188,7 +188,10 @@ pub fn on_tick(store: &mut Store, timestamp: u64, has_proposal: bool) { } } -/// Process a gossiped attestation. +/// Process a gossiped attestation (with signature verification). +/// +/// This is the safe default: it always verifies the validator's XMSS signature +/// and stores it for future block building. pub fn on_gossip_attestation( store: &mut Store, signed_attestation: SignedAttestation, @@ -211,24 +214,56 @@ pub fn on_gossip_attestation( .get_pubkey() .map_err(|_| StoreError::PubkeyDecodingFailed(validator_id))?; let message = attestation.data.tree_hash_root(); - if cfg!(not(feature = "skip-signature-verification")) { - use ethlambda_types::signature::ValidatorSignature; - // Use attestation.data.slot as epoch (matching what Zeam and ethlambda use for signing) - let epoch: u32 = attestation.data.slot.try_into().expect("slot exceeds u32"); - let signature = ValidatorSignature::from_bytes(&signed_attestation.signature) - .map_err(|_| StoreError::SignatureDecodingFailed)?; - if !signature.is_valid(&validator_pubkey, epoch, &message) { - return Err(StoreError::SignatureVerificationFailed); - } + + // Verify the validator's XMSS signature + let epoch: u32 = attestation.data.slot.try_into().expect("slot exceeds u32"); + let signature = ValidatorSignature::from_bytes(&signed_attestation.signature) + .map_err(|_| StoreError::SignatureDecodingFailed)?; + if !signature.is_valid(&validator_pubkey, epoch, &message) { + return Err(StoreError::SignatureVerificationFailed); } + + on_attestation(store, attestation.clone(), false)?; + + // Store signature for later lookup during block building + store.insert_gossip_signature(&attestation.data, validator_id, signature); + + metrics::inc_attestations_valid("gossip"); + + let slot = attestation.data.slot; + let target_slot = attestation.data.target.slot; + let source_slot = attestation.data.source.slot; + info!( + slot, + validator = validator_id, + target_slot, + target_root = %ShortRoot(&attestation.data.target.root.0), + source_slot, + source_root = %ShortRoot(&attestation.data.source.root.0), + "Attestation processed" + ); + + Ok(()) +} + +/// Process a gossiped attestation without signature verification. +/// +/// This skips all cryptographic checks and signature storage. Use only in tests +/// where signatures are absent or irrelevant. +pub fn on_gossip_attestation_without_verification( + store: &mut Store, + signed_attestation: SignedAttestation, +) -> Result<(), StoreError> { + let validator_id = signed_attestation.validator_id; + let attestation = Attestation { + validator_id, + data: signed_attestation.message, + }; + validate_attestation(store, &attestation) + .inspect_err(|_| metrics::inc_attestations_invalid("gossip"))?; + on_attestation(store, attestation.clone(), false)?; - if cfg!(not(feature = "skip-signature-verification")) { - // Store signature for later lookup during block building - let signature = ValidatorSignature::from_bytes(&signed_attestation.signature) - .map_err(|_| StoreError::SignatureDecodingFailed)?; - store.insert_gossip_signature(&attestation.data, validator_id, signature); - } metrics::inc_attestations_valid("gossip"); let slot = attestation.data.slot; @@ -312,23 +347,25 @@ fn on_attestation( Ok(()) } -/// Process a new block and update the forkchoice state. +/// Process a new block and update the forkchoice state (with signature verification). +/// +/// This is the safe default: it always verifies cryptographic signatures +/// and stores them for future block building. Use this for all production paths. /// /// This method integrates a block into the forkchoice store by: /// 1. Validating the block's parent exists -/// 2. Computing the post-state via the state transition function -/// 3. Processing attestations included in the block body (on-chain) -/// 4. Updating the forkchoice head -/// 5. Processing the proposer's attestation (as if gossiped) +/// 2. Verifying cryptographic signatures +/// 3. Computing the post-state via the state transition function +/// 4. Processing attestations included in the block body (on-chain) +/// 5. Updating the forkchoice head +/// 6. Processing the proposer's attestation (as if gossiped) pub fn on_block( store: &mut Store, signed_block: SignedBlockWithAttestation, ) -> Result<(), StoreError> { let _timing = metrics::time_fork_choice_block_processing(); - // Unpack block components - let block = signed_block.message.block.clone(); - let proposer_attestation = signed_block.message.proposer_attestation.clone(); + let block = &signed_block.message.block; let block_root = block.tree_hash_root(); let slot = block.slot; @@ -338,8 +375,6 @@ pub fn on_block( } // Verify parent state is available - // Note: Parent block existence is checked by the caller before calling this function. - // This check ensures the state has been computed for the parent block. let parent_state = store .get_state(&block.parent_root) @@ -349,14 +384,67 @@ pub fn on_block( })?; // Validate cryptographic signatures - // TODO: extract signature verification to a pre-checks function - // to avoid the need for this - if cfg!(not(feature = "skip-signature-verification")) { - verify_signatures(&parent_state, &signed_block)?; + verify_signatures(&parent_state, &signed_block)?; + + // Store the proposer's signature for potential future block building + let proposer_attestation = &signed_block.message.proposer_attestation; + let proposer_sig = ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature) + .map_err(|_| StoreError::SignatureDecodingFailed)?; + store.insert_gossip_signature( + &proposer_attestation.data, + proposer_attestation.validator_id, + proposer_sig, + ); + + on_block_core(store, signed_block, parent_state) +} + +/// Process a new block without signature verification. +/// +/// This skips all cryptographic checks and signature storage. Use only in tests +/// where signatures are absent or irrelevant (e.g., fork choice spec tests). +pub fn on_block_without_verification( + store: &mut Store, + signed_block: SignedBlockWithAttestation, +) -> Result<(), StoreError> { + let _timing = metrics::time_fork_choice_block_processing(); + + let block = &signed_block.message.block; + let block_root = block.tree_hash_root(); + let slot = block.slot; + + // Skip duplicate blocks (idempotent operation) + if store.has_state(&block_root) { + return Ok(()); } + // Verify parent state is available + let parent_state = + store + .get_state(&block.parent_root) + .ok_or(StoreError::MissingParentState { + parent_root: block.parent_root, + slot, + })?; + + on_block_core(store, signed_block, parent_state) +} + +/// Shared block processing logic used by both `on_block()` and `on_block_without_verification()`. +/// +/// Takes `parent_state` by value to avoid cloning (the caller already fetched it). +fn on_block_core( + store: &mut Store, + signed_block: SignedBlockWithAttestation, + parent_state: State, +) -> Result<(), StoreError> { + let block = signed_block.message.block.clone(); + let proposer_attestation = signed_block.message.proposer_attestation.clone(); + let block_root = block.tree_hash_root(); + let slot = block.slot; + // Execute state transition function to compute post-block state - let mut post_state = parent_state.clone(); + let mut post_state = parent_state; ethlambda_state_transition::state_transition(&mut post_state, &block)?; // Cache the state root in the latest block header @@ -414,22 +502,6 @@ pub fn on_block( // to prevent the proposer from gaining circular weight advantage. update_head(store); - // Process proposer attestation as if received via gossip - // The proposer's attestation should NOT affect this block's fork choice position. - // It is treated as pending until interval 3 (end of slot). - - if cfg!(not(feature = "skip-signature-verification")) { - // Store the proposer's signature for potential future block building - let proposer_sig = - ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature) - .map_err(|_| StoreError::SignatureDecodingFailed)?; - store.insert_gossip_signature( - &proposer_attestation.data, - proposer_attestation.validator_id, - proposer_sig, - ); - } - // Process proposer attestation (enters "new" stage, not "known") // TODO: validate attestations before processing let _ = on_attestation(store, proposer_attestation, false) diff --git a/crates/blockchain/tests/forkchoice_spectests.rs b/crates/blockchain/tests/forkchoice_spectests.rs index 7a9fd62..092f416 100644 --- a/crates/blockchain/tests/forkchoice_spectests.rs +++ b/crates/blockchain/tests/forkchoice_spectests.rs @@ -63,7 +63,7 @@ fn run(path: &Path) -> datatest_stable::Result<()> { // NOTE: the has_proposal argument is set to true, following the spec store::on_tick(&mut store, block_time, true); - let result = store::on_block(&mut store, signed_block); + let result = store::on_block_without_verification(&mut store, signed_block); match (result.is_ok(), step.valid) { (true, false) => { @@ -303,46 +303,46 @@ fn validate_attestation_check( })?; // Validate attestation slot if specified - if let Some(expected_slot) = check.attestation_slot { - if attestation.slot != expected_slot { - return Err(format!( - "Step {}: attestation slot mismatch for validator {}: expected {}, got {}", - step_idx, validator_id, expected_slot, attestation.slot - ) - .into()); - } + if let Some(expected_slot) = check.attestation_slot + && attestation.slot != expected_slot + { + return Err(format!( + "Step {}: attestation slot mismatch for validator {}: expected {}, got {}", + step_idx, validator_id, expected_slot, attestation.slot + ) + .into()); } - if let Some(expected_head_slot) = check.head_slot { - if attestation.head.slot != expected_head_slot { - return Err(format!( - "Step {}: attestation head slot mismatch: expected {}, got {}", - step_idx, expected_head_slot, attestation.head.slot - ) - .into()); - } + if let Some(expected_head_slot) = check.head_slot + && attestation.head.slot != expected_head_slot + { + return Err(format!( + "Step {}: attestation head slot mismatch: expected {}, got {}", + step_idx, expected_head_slot, attestation.head.slot + ) + .into()); } // Validate source slot if specified - if let Some(expected_source_slot) = check.source_slot { - if attestation.source.slot != expected_source_slot { - return Err(format!( - "Step {}: attestation source slot mismatch: expected {}, got {}", - step_idx, expected_source_slot, attestation.source.slot - ) - .into()); - } + if let Some(expected_source_slot) = check.source_slot + && attestation.source.slot != expected_source_slot + { + return Err(format!( + "Step {}: attestation source slot mismatch: expected {}, got {}", + step_idx, expected_source_slot, attestation.source.slot + ) + .into()); } // Validate target slot if specified - if let Some(expected_target_slot) = check.target_slot { - if attestation.target.slot != expected_target_slot { - return Err(format!( - "Step {}: attestation target slot mismatch: expected {}, got {}", - step_idx, expected_target_slot, attestation.target.slot - ) - .into()); - } + if let Some(expected_target_slot) = check.target_slot + && attestation.target.slot != expected_target_slot + { + return Err(format!( + "Step {}: attestation target slot mismatch: expected {}, got {}", + step_idx, expected_target_slot, attestation.target.slot + ) + .into()); } Ok(()) From c4e90026ef6bd64ad9d7bb827cac735a533cd5ec Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 27 Feb 2026 12:18:49 -0300 Subject: [PATCH 2/6] Update Makefile, remove features skip-signature-verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com> --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index cac407c..c95474c 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,6 @@ lint: ## 🔍 Run clippy on all workspace crates test: leanSpec/fixtures ## 🧪 Run all tests, then forkchoice spec tests # Tests need to be run on release to avoid stack overflows during signature verification/aggregation cargo test --workspace --release - cargo test -p ethlambda-blockchain --test forkchoice_spectests GIT_COMMIT=$(shell git rev-parse HEAD) GIT_BRANCH=$(shell git rev-parse --abbrev-ref HEAD) From b3ce70343e0d909b1303ff7c6bc5ff1c00f46113 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 27 Feb 2026 12:54:21 -0300 Subject: [PATCH 3/6] Address review comments: consolidate on_block and on_gossip_attestation duplication into shared core functions with a runtime verify flag, restore parent-state comment, remove redundant forkchoice test target from Makefile, and add BUILD_PROFILE example to RELEASE.md. --- RELEASE.md | 4 + crates/blockchain/src/store.rs | 172 ++++++++++++++------------------- 2 files changed, 75 insertions(+), 101 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 09dbd65..e5044e6 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -72,6 +72,10 @@ The Dockerfile accepts build arguments for customizing the build: Example with custom args: +```bash +docker build --build-arg BUILD_PROFILE=debug -t ethlambda:debug . +``` + `GIT_COMMIT` and `GIT_BRANCH` are also available but set automatically by CI. When building locally, `vergen-git2` extracts them from the local Git repo at build time. diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 4901d01..1143ab2 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -196,54 +196,7 @@ pub fn on_gossip_attestation( store: &mut Store, signed_attestation: SignedAttestation, ) -> Result<(), StoreError> { - let validator_id = signed_attestation.validator_id; - let attestation = Attestation { - validator_id, - data: signed_attestation.message, - }; - validate_attestation(store, &attestation) - .inspect_err(|_| metrics::inc_attestations_invalid("gossip"))?; - let target = attestation.data.target; - let target_state = store - .get_state(&target.root) - .ok_or(StoreError::MissingTargetState(target.root))?; - if validator_id >= target_state.validators.len() as u64 { - return Err(StoreError::InvalidValidatorIndex); - } - let validator_pubkey = target_state.validators[validator_id as usize] - .get_pubkey() - .map_err(|_| StoreError::PubkeyDecodingFailed(validator_id))?; - let message = attestation.data.tree_hash_root(); - - // Verify the validator's XMSS signature - let epoch: u32 = attestation.data.slot.try_into().expect("slot exceeds u32"); - let signature = ValidatorSignature::from_bytes(&signed_attestation.signature) - .map_err(|_| StoreError::SignatureDecodingFailed)?; - if !signature.is_valid(&validator_pubkey, epoch, &message) { - return Err(StoreError::SignatureVerificationFailed); - } - - on_attestation(store, attestation.clone(), false)?; - - // Store signature for later lookup during block building - store.insert_gossip_signature(&attestation.data, validator_id, signature); - - metrics::inc_attestations_valid("gossip"); - - let slot = attestation.data.slot; - let target_slot = attestation.data.target.slot; - let source_slot = attestation.data.source.slot; - info!( - slot, - validator = validator_id, - target_slot, - target_root = %ShortRoot(&attestation.data.target.root.0), - source_slot, - source_root = %ShortRoot(&attestation.data.source.root.0), - "Attestation processed" - ); - - Ok(()) + on_gossip_attestation_core(store, signed_attestation, true) } /// Process a gossiped attestation without signature verification. @@ -253,6 +206,18 @@ pub fn on_gossip_attestation( pub fn on_gossip_attestation_without_verification( store: &mut Store, signed_attestation: SignedAttestation, +) -> Result<(), StoreError> { + on_gossip_attestation_core(store, signed_attestation, false) +} + +/// Core gossip attestation processing logic. +/// +/// When `verify` is true, the validator's XMSS signature is checked and stored +/// for future block building. When false, all signature checks are skipped. +fn on_gossip_attestation_core( + store: &mut Store, + signed_attestation: SignedAttestation, + verify: bool, ) -> Result<(), StoreError> { let validator_id = signed_attestation.validator_id; let attestation = Attestation { @@ -262,7 +227,34 @@ pub fn on_gossip_attestation_without_verification( validate_attestation(store, &attestation) .inspect_err(|_| metrics::inc_attestations_invalid("gossip"))?; - on_attestation(store, attestation.clone(), false)?; + if verify { + let target = attestation.data.target; + let target_state = store + .get_state(&target.root) + .ok_or(StoreError::MissingTargetState(target.root))?; + if validator_id >= target_state.validators.len() as u64 { + return Err(StoreError::InvalidValidatorIndex); + } + let validator_pubkey = target_state.validators[validator_id as usize] + .get_pubkey() + .map_err(|_| StoreError::PubkeyDecodingFailed(validator_id))?; + let message = attestation.data.tree_hash_root(); + + // Verify the validator's XMSS signature + let epoch: u32 = attestation.data.slot.try_into().expect("slot exceeds u32"); + let signature = ValidatorSignature::from_bytes(&signed_attestation.signature) + .map_err(|_| StoreError::SignatureDecodingFailed)?; + if !signature.is_valid(&validator_pubkey, epoch, &message) { + return Err(StoreError::SignatureVerificationFailed); + } + + on_attestation(store, attestation.clone(), false)?; + + // Store signature for later lookup during block building + store.insert_gossip_signature(&attestation.data, validator_id, signature); + } else { + on_attestation(store, attestation.clone(), false)?; + } metrics::inc_attestations_valid("gossip"); @@ -351,52 +343,11 @@ fn on_attestation( /// /// This is the safe default: it always verifies cryptographic signatures /// and stores them for future block building. Use this for all production paths. -/// -/// This method integrates a block into the forkchoice store by: -/// 1. Validating the block's parent exists -/// 2. Verifying cryptographic signatures -/// 3. Computing the post-state via the state transition function -/// 4. Processing attestations included in the block body (on-chain) -/// 5. Updating the forkchoice head -/// 6. Processing the proposer's attestation (as if gossiped) pub fn on_block( store: &mut Store, signed_block: SignedBlockWithAttestation, ) -> Result<(), StoreError> { - let _timing = metrics::time_fork_choice_block_processing(); - - let block = &signed_block.message.block; - let block_root = block.tree_hash_root(); - let slot = block.slot; - - // Skip duplicate blocks (idempotent operation) - if store.has_state(&block_root) { - return Ok(()); - } - - // Verify parent state is available - let parent_state = - store - .get_state(&block.parent_root) - .ok_or(StoreError::MissingParentState { - parent_root: block.parent_root, - slot, - })?; - - // Validate cryptographic signatures - verify_signatures(&parent_state, &signed_block)?; - - // Store the proposer's signature for potential future block building - let proposer_attestation = &signed_block.message.proposer_attestation; - let proposer_sig = ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature) - .map_err(|_| StoreError::SignatureDecodingFailed)?; - store.insert_gossip_signature( - &proposer_attestation.data, - proposer_attestation.validator_id, - proposer_sig, - ); - - on_block_core(store, signed_block, parent_state) + on_block_core(store, signed_block, true) } /// Process a new block without signature verification. @@ -406,6 +357,18 @@ pub fn on_block( pub fn on_block_without_verification( store: &mut Store, signed_block: SignedBlockWithAttestation, +) -> Result<(), StoreError> { + on_block_core(store, signed_block, false) +} + +/// Core block processing logic. +/// +/// When `verify` is true, cryptographic signatures are validated and stored +/// for future block building. When false, all signature checks are skipped. +fn on_block_core( + store: &mut Store, + signed_block: SignedBlockWithAttestation, + verify: bool, ) -> Result<(), StoreError> { let _timing = metrics::time_fork_choice_block_processing(); @@ -419,6 +382,8 @@ pub fn on_block_without_verification( } // Verify parent state is available + // Note: Parent block existence is checked by the caller before calling this function. + // This check ensures the state has been computed for the parent block. let parent_state = store .get_state(&block.parent_root) @@ -427,17 +392,22 @@ pub fn on_block_without_verification( slot, })?; - on_block_core(store, signed_block, parent_state) -} + if verify { + // Validate cryptographic signatures + verify_signatures(&parent_state, &signed_block)?; + + // Store the proposer's signature for potential future block building + let proposer_attestation = &signed_block.message.proposer_attestation; + let proposer_sig = + ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature) + .map_err(|_| StoreError::SignatureDecodingFailed)?; + store.insert_gossip_signature( + &proposer_attestation.data, + proposer_attestation.validator_id, + proposer_sig, + ); + } -/// Shared block processing logic used by both `on_block()` and `on_block_without_verification()`. -/// -/// Takes `parent_state` by value to avoid cloning (the caller already fetched it). -fn on_block_core( - store: &mut Store, - signed_block: SignedBlockWithAttestation, - parent_state: State, -) -> Result<(), StoreError> { let block = signed_block.message.block.clone(); let proposer_attestation = signed_block.message.proposer_attestation.clone(); let block_root = block.tree_hash_root(); From 3f9a50a5b794ac4aad847604b7c2b270ac5178cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 27 Feb 2026 14:26:52 -0300 Subject: [PATCH 4/6] Apply suggestion from @MegaRedHand --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c95474c..ae133ca 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ fmt: ## 🎨 Format all code using rustfmt lint: ## 🔍 Run clippy on all workspace crates cargo clippy --workspace --all-targets -- -D warnings -test: leanSpec/fixtures ## 🧪 Run all tests, then forkchoice spec tests +test: leanSpec/fixtures ## 🧪 Run all tests # Tests need to be run on release to avoid stack overflows during signature verification/aggregation cargo test --workspace --release From 9b29ecb0f4746e19a3e41e201d7b9ce8617c5bea Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 27 Feb 2026 16:06:32 -0300 Subject: [PATCH 5/6] Address review comments: consolidate on_block and on_gossip_attestation duplication into shared core functions with a runtime verify flag, restore parent-state comment, keep proposer signature storage in original position, remove redundant forkchoice test target from Makefile, and add BUILD_PROFILE example to RELEASE.md. --- crates/blockchain/src/store.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 1143ab2..d52b7c9 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -395,17 +395,6 @@ fn on_block_core( if verify { // Validate cryptographic signatures verify_signatures(&parent_state, &signed_block)?; - - // Store the proposer's signature for potential future block building - let proposer_attestation = &signed_block.message.proposer_attestation; - let proposer_sig = - ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature) - .map_err(|_| StoreError::SignatureDecodingFailed)?; - store.insert_gossip_signature( - &proposer_attestation.data, - proposer_attestation.validator_id, - proposer_sig, - ); } let block = signed_block.message.block.clone(); @@ -472,6 +461,18 @@ fn on_block_core( // to prevent the proposer from gaining circular weight advantage. update_head(store); + if verify { + // Store the proposer's signature for potential future block building + let proposer_sig = + ValidatorSignature::from_bytes(&signed_block.signature.proposer_signature) + .map_err(|_| StoreError::SignatureDecodingFailed)?; + store.insert_gossip_signature( + &proposer_attestation.data, + proposer_attestation.validator_id, + proposer_sig, + ); + } + // Process proposer attestation (enters "new" stage, not "known") // TODO: validate attestations before processing let _ = on_attestation(store, proposer_attestation, false) From 0bba4c67467e634d7aae9344579c966e7ff2676c Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 27 Feb 2026 16:11:57 -0300 Subject: [PATCH 6/6] Restore original proposer attestation comment explaining why it enters the "new" stage instead of "known", addressing review feedback. --- crates/blockchain/src/store.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index d52b7c9..1a33642 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -473,7 +473,9 @@ fn on_block_core( ); } - // Process proposer attestation (enters "new" stage, not "known") + // Process proposer attestation as if received via gossip + // The proposer's attestation should NOT affect this block's fork choice position. + // It is treated as pending until interval 3 (end of slot). // TODO: validate attestations before processing let _ = on_attestation(store, proposer_attestation, false) .inspect(|_| metrics::inc_attestations_valid("gossip"))