From 14a4de41e0cca70102805688c96885dfb404cd62 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 12 May 2026 11:47:16 +1000 Subject: [PATCH] Move payload status validation prior to mutations! --- consensus/fork_choice/src/fork_choice.rs | 78 +++++++++++------------ testing/ef_tests/src/cases/fork_choice.rs | 10 ++- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 593aa27915..19ffd55469 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -823,6 +823,45 @@ where })); } + let execution_status = if let Ok(execution_payload) = block.body().execution_payload() { + let block_hash = execution_payload.block_hash(); + + if block_hash == ExecutionBlockHash::zero() { + // The block is post-merge-fork, but pre-terminal-PoW block. We don't need to verify + // the payload. + ExecutionStatus::irrelevant() + } else { + match payload_verification_status { + PayloadVerificationStatus::Verified => ExecutionStatus::Valid(block_hash), + PayloadVerificationStatus::Optimistic => { + ExecutionStatus::Optimistic(block_hash) + } + // It would be a logic error to declare a block irrelevant if it has an + // execution payload with a non-zero block hash. + PayloadVerificationStatus::Irrelevant => { + return Err(Error::InvalidPayloadStatus { + block_slot: block.slot(), + block_root, + payload_verification_status, + }); + } + } + } + } else { + // There is no payload to verify. + ExecutionStatus::irrelevant() + }; + + let (execution_payload_parent_hash, execution_payload_block_hash) = + if let Ok(signed_bid) = block.body().signed_execution_payload_bid() { + ( + Some(signed_bid.message.parent_block_hash), + Some(signed_bid.message.block_hash), + ) + } else { + (None, None) + }; + let attestation_threshold = spec.get_attestation_due::(block.slot()); // Add proposer score boost if the block is the first timely block for this slot and its @@ -956,45 +995,6 @@ where .on_verified_block(block, block_root, state) .map_err(Error::AfterBlockFailed)?; - let execution_status = if let Ok(execution_payload) = block.body().execution_payload() { - let block_hash = execution_payload.block_hash(); - - if block_hash == ExecutionBlockHash::zero() { - // The block is post-merge-fork, but pre-terminal-PoW block. We don't need to verify - // the payload. - ExecutionStatus::irrelevant() - } else { - match payload_verification_status { - PayloadVerificationStatus::Verified => ExecutionStatus::Valid(block_hash), - PayloadVerificationStatus::Optimistic => { - ExecutionStatus::Optimistic(block_hash) - } - // It would be a logic error to declare a block irrelevant if it has an - // execution payload with a non-zero block hash. - PayloadVerificationStatus::Irrelevant => { - return Err(Error::InvalidPayloadStatus { - block_slot: block.slot(), - block_root, - payload_verification_status, - }); - } - } - } - } else { - // There is no payload to verify. - ExecutionStatus::irrelevant() - }; - - let (execution_payload_parent_hash, execution_payload_block_hash) = - if let Ok(signed_bid) = block.body().signed_execution_payload_bid() { - ( - Some(signed_bid.message.parent_block_hash), - Some(signed_bid.message.block_hash), - ) - } else { - (None, None) - }; - // This does not apply a vote to the block, it just makes fork choice aware of the block so // it can still be identified as the head even if it doesn't have any votes. self.proto_array.process_block::( diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 9a0dff0510..34d25d2e36 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -865,7 +865,7 @@ impl Tester { block_root, block_delay, &state, - PayloadVerificationStatus::Irrelevant, + PayloadVerificationStatus::Optimistic, block.message().proposer_index(), &self.harness.chain.spec, ); @@ -1249,7 +1249,13 @@ impl Tester { actual_sorted.sort(); let mut expected_sorted: Vec<(Hash256, u8, u64)> = expected .iter() - .map(|x| (x.root, x.payload_status.unwrap_or(FcPayloadStatus::Pending as u8), x.weight)) + .map(|x| { + ( + x.root, + x.payload_status.unwrap_or(FcPayloadStatus::Pending as u8), + x.weight, + ) + }) .collect(); expected_sorted.sort();