diff --git a/Cargo.lock b/Cargo.lock index 0bec6c9e0..49c56bc1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2116,6 +2116,7 @@ dependencies = [ "ethrex-common", "ethrex-p2p", "ethrex-rlp", + "ethrex-trie", "futures", "hex", "libp2p", diff --git a/crates/blockchain/state_transition/src/beacon/gossip/mod.rs b/crates/blockchain/state_transition/src/beacon/gossip/mod.rs index 701fa705c..1d5019069 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/mod.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/mod.rs @@ -321,6 +321,10 @@ pub enum RejectReason { BuilderIndexMismatch, /// A gloas envelope's payload block hash is not its bid's. BlockHashMismatch, + /// An execution payload's `block_hash` is not the hash of the execution + /// block header rebuilt from the payload's own fields. A check stricter + /// than the specification's, run in p2p: see `docs/spec_deviations.md`. + PayloadBlockHashMismatch, /// The root of a gloas envelope's execution requests is not its bid's. ExecutionRequestsRootMismatch, /// A gloas envelope's payload carries more withdrawals than its limit. @@ -384,6 +388,7 @@ impl RejectReason { Self::PayloadInvalid => "payload_invalid", Self::BuilderIndexMismatch => "builder_index_mismatch", Self::BlockHashMismatch => "block_hash_mismatch", + Self::PayloadBlockHashMismatch => "payload_block_hash_mismatch", Self::ExecutionRequestsRootMismatch => "execution_requests_root_mismatch", Self::TooManyWithdrawals => "too_many_withdrawals", Self::PreGloasSlot => "pre_gloas_slot", diff --git a/crates/net/p2p/Cargo.toml b/crates/net/p2p/Cargo.toml index 22154e141..e386426eb 100644 --- a/crates/net/p2p/Cargo.toml +++ b/crates/net/p2p/Cargo.toml @@ -70,6 +70,9 @@ rand = "0.8" # against with nothing in the manifest saying so. ethrex-p2p = { git = "https://github.com/lambdaclass/ethrex", tag = "v25.0.0" } ethrex-rlp = { git = "https://github.com/lambdaclass/ethrex", tag = "v25.0.0" } +# The ordered trie behind a payload's transactions root, which `ethrex-common` +# only builds from decoded transactions, not the raw bytes a payload carries. +ethrex-trie = { git = "https://github.com/lambdaclass/ethrex", tag = "v25.0.0" } ethrex-common = { git = "https://github.com/lambdaclass/ethrex", tag = "v25.0.0" } # `rand` seeds the throwaway signing keys the ENR tests build records with. diff --git a/crates/net/p2p/src/beacon/envelope_checks.rs b/crates/net/p2p/src/beacon/envelope_checks.rs index 5dbbd43de..0690e378b 100644 --- a/crates/net/p2p/src/beacon/envelope_checks.rs +++ b/crates/net/p2p/src/beacon/envelope_checks.rs @@ -33,6 +33,7 @@ use ethlambda_types::beacon::containers::gloas::SignedExecutionPayloadEnvelope; use tracing::{debug, error, warn}; use crate::P2PServer; +use crate::beacon::payload_hash; /// Whether a fetched envelope goes to the chain actor. pub(crate) fn forwards(outcome: &Outcome) -> bool { @@ -48,10 +49,14 @@ fn judge(store: &Store, envelope: &SignedExecutionPayloadEnvelope) -> Outcome { // A fresh, empty cache: a fetched envelope is not a gossip duplicate of // anything, so the cache's one rule never fires. let seen = SeenEnvelopes::new(NonZeroUsize::MIN); - match cheap_checks(&seen, store, envelope) { - Ok(()) => stateful_checks(store, envelope), - Err(outcome) => outcome, + if let Err(outcome) = cheap_checks(&seen, store, envelope) { + return outcome; } + // The same stricter-than-the-specification rule the gossip topic runs. + if let Err(outcome) = payload_hash::reject_envelope_on_mismatch(envelope) { + return outcome; + } + stateful_checks(store, envelope) } /// Run the gossip rules on `envelopes` and send the chain actor the ones that @@ -235,7 +240,13 @@ pub(crate) mod tests { } pub(crate) fn envelope(slot: u64, byte: u8) -> SignedExecutionPayloadEnvelope { - crate::beacon::encoding::test_support::envelope(H256::repeat_byte(byte), slot, H256::ZERO) + // Sealed: the fetched-envelope rules refuse a block hash that is not + // the payload's header's. + payload_hash::seal_envelope(crate::beacon::encoding::test_support::envelope( + H256::repeat_byte(byte), + slot, + H256::ZERO, + )) } #[test] diff --git a/crates/net/p2p/src/beacon/mod.rs b/crates/net/p2p/src/beacon/mod.rs index 33944b4f7..dce6adf2d 100644 --- a/crates/net/p2p/src/beacon/mod.rs +++ b/crates/net/p2p/src/beacon/mod.rs @@ -21,6 +21,7 @@ pub mod envelope_checks; pub mod fork_schedule; pub mod handler; pub mod messages; +pub mod payload_hash; pub mod protocols; pub mod scoring; pub mod subnets; diff --git a/crates/net/p2p/src/beacon/payload_hash.rs b/crates/net/p2p/src/beacon/payload_hash.rs new file mode 100644 index 000000000..b0e33b905 --- /dev/null +++ b/crates/net/p2p/src/beacon/payload_hash.rs @@ -0,0 +1,365 @@ +//! The execution block hash an execution payload claims, checked against the +//! header rebuilt from the payload's own fields. +//! +//! A payload carries its `block_hash` next to the fields that hash is taken +//! over, so a node can recompute it without an execution client: the header is +//! the payload's fields plus the transactions, withdrawals and requests tries, +//! RLP-encoded and keccak-hashed. The Engine API answers a mismatch with +//! `INVALID` and a null `latestValidHash` (execution-apis `shanghai.md`), which +//! is indistinguishable from a payload that executed badly and whose last valid +//! ancestor is unknown. Catching the structural case here keeps such a payload +//! from ever reaching fork choice. It is a rule stricter than the consensus +//! specification's gossip rules; see `docs/spec_deviations.md`. +//! +//! The header's shape per fork: +//! +//! | fork | adds to the header | +//! |-----------|-----------------------------------------------------------| +//! | bellatrix | the London fields, with `base_fee_per_gas` | +//! | capella | `withdrawals_root` | +//! | deneb | `blob_gas_used`, `excess_blob_gas`, `parent_beacon_block_root` | +//! | electra | `requests_hash` (fulu is the same) | +//! | gloas | `block_access_list_hash`, `slot_number` | +//! +//! Fields the execution layer fixes after the merge are constants here: no +//! ommers, difficulty zero, a zero nonce, `prev_randao` in the mix hash slot. +//! The header is encoded here rather than through ethrex's `BlockHeader`, +//! which holds `base_fee_per_gas` as a `u64` where the payload has 256 bits. + +use ethlambda_state_transition::beacon::gossip::{Outcome, RejectReason}; +use ethlambda_state_transition::beacon::stf::{electra, gloas as gloas_stf}; +use ethlambda_types::ShortRoot; +use ethlambda_types::beacon::containers::{SignedBeaconBlock, capella, gloas}; +use ethlambda_types::beacon::primitives::Root; +use ethrex_common::types::requests::{EncodedRequests, compute_requests_hash}; +use ethrex_common::types::{Withdrawal, compute_withdrawals_root}; +use ethrex_common::{Address, Bloom, Bytes, H256, NativeCrypto, U256}; +use ethrex_rlp::encode::RLPEncode; +use ethrex_rlp::structs::Encoder; +use ethrex_trie::Trie; +use tracing::warn; + +use crate::metrics; + +/// `keccak256(rlp([]))`: the ommers hash of every post-merge block. +const EMPTY_OMMERS_HASH: [u8; 32] = [ + 0x1d, 0xcc, 0x4d, 0xe8, 0xde, 0xc7, 0x5d, 0x7a, 0xab, 0x85, 0xb5, 0x67, 0xb6, 0xcc, 0xd4, 0x1a, + 0xd3, 0x12, 0x45, 0x1b, 0x94, 0x8a, 0x74, 0x13, 0xf0, 0xa1, 0x42, 0xfd, 0x40, 0xd4, 0x93, 0x47, +]; + +/// The hash the payload claims against the one its fields give. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct HashMismatch { + pub claimed: [u8; 32], + pub computed: [u8; 32], +} + +/// What a payload and its surroundings supply to the header, whatever the fork. +struct Fields<'a> { + parent_hash: [u8; 32], + fee_recipient: [u8; 20], + state_root: [u8; 32], + receipts_root: [u8; 32], + logs_bloom: &'a [u8], + prev_randao: [u8; 32], + block_number: u64, + gas_limit: u64, + gas_used: u64, + timestamp: u64, + extra_data: &'a [u8], + /// 32 little-endian bytes, as SSZ carries it. + base_fee_per_gas: [u8; 32], + claimed_hash: [u8; 32], + transactions: Vec<&'a [u8]>, + /// `None` before capella. + withdrawals: Option<&'a [capella::Withdrawal]>, + /// `None` before deneb. + blob_gas: Option<(u64, u64)>, + /// `None` before deneb. + parent_beacon_block_root: Option<[u8; 32]>, + /// The Engine API's request list; `None` before electra. + requests: Option>>, + /// The payload's block access list bytes and slot number; `None` before gloas. + amsterdam: Option<(&'a [u8], u64)>, +} + +impl Fields<'_> { + /// `keccak256(rlp(header))`. + fn computed_hash(&self) -> [u8; 32] { + // The trie roots are over opaque bytes: a transaction is already + // `type || rlp(tx)` on the wire, so it is never decoded. + let transactions = self + .transactions + .iter() + .enumerate() + .map(|(index, tx)| (index.encode_to_vec(), tx.to_vec())); + let transactions_root = Trie::compute_hash_from_unsorted_iter(transactions, &NativeCrypto); + let withdrawals_root = self.withdrawals.map(|withdrawals| { + let withdrawals: Vec = withdrawals + .iter() + .map(|withdrawal| Withdrawal { + index: withdrawal.index, + validator_index: withdrawal.validator_index, + address: Address::from(withdrawal.address.0), + amount: withdrawal.amount, + }) + .collect(); + compute_withdrawals_root(&withdrawals, &NativeCrypto) + }); + let requests_hash = self.requests.as_ref().map(|requests| { + let requests: Vec = requests + .iter() + .map(|request| EncodedRequests(Bytes::copy_from_slice(request))) + .collect(); + compute_requests_hash(&requests) + }); + // EIP-7928: the header commits to the keccak of the RLP bytes the + // payload carries, whatever they decode to. + let block_access_list_hash = self + .amsterdam + .map(|(bal, _)| H256(ethrex_crypto_keccak(bal))); + let slot_number = self.amsterdam.map(|(_, slot)| slot); + let base_fee_per_gas = U256::from_little_endian(&self.base_fee_per_gas); + + let mut buf = Vec::with_capacity(700); + Encoder::new(&mut buf) + .encode_field(&H256(self.parent_hash)) + .encode_field(&H256(EMPTY_OMMERS_HASH)) + .encode_field(&Address::from(self.fee_recipient)) + .encode_field(&H256(self.state_root)) + .encode_field(&transactions_root) + .encode_field(&H256(self.receipts_root)) + .encode_field(&Bloom::from_slice(self.logs_bloom)) + .encode_field(&U256::zero()) + .encode_field(&self.block_number) + .encode_field(&self.gas_limit) + .encode_field(&self.gas_used) + .encode_field(&self.timestamp) + .encode_field(&Bytes::copy_from_slice(self.extra_data)) + .encode_field(&H256(self.prev_randao)) + .encode_field(&[0u8; 8]) + .encode_field(&base_fee_per_gas) + .encode_optional_field(&withdrawals_root) + .encode_optional_field(&self.blob_gas.map(|(used, _)| used)) + .encode_optional_field(&self.blob_gas.map(|(_, excess)| excess)) + .encode_optional_field(&self.parent_beacon_block_root.map(H256)) + .encode_optional_field(&requests_hash) + .encode_optional_field(&block_access_list_hash) + .encode_optional_field(&slot_number) + .finish(); + ethrex_crypto_keccak(&buf) + } + + fn verify(&self) -> Result<(), HashMismatch> { + let computed = self.computed_hash(); + if computed == self.claimed_hash { + Ok(()) + } else { + Err(HashMismatch { + claimed: self.claimed_hash, + computed, + }) + } + } +} + +fn ethrex_crypto_keccak(data: &[u8]) -> [u8; 32] { + ethrex_common::utils::keccak(data).0 +} + +/// The block's payload hash against its header, for every fork that carries +/// the payload in the block body. +/// +/// A block before bellatrix, a bellatrix block from before the merge (its +/// payload is the empty default), and a gloas block (its payload arrives in a +/// separate envelope) have nothing to check. +pub fn verify_block(block: &SignedBeaconBlock) -> Result<(), HashMismatch> { + match block { + SignedBeaconBlock::Bellatrix(block) => { + let payload = &block.message.body.execution_payload; + if payload.block_hash.0 == [0; 32] && payload.parent_hash.0 == [0; 32] { + return Ok(()); + } + Fields { + parent_hash: payload.parent_hash.0, + fee_recipient: payload.fee_recipient.0, + state_root: payload.state_root.0, + receipts_root: payload.receipts_root.0, + logs_bloom: payload.logs_bloom.as_ref(), + prev_randao: payload.prev_randao.0, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data.as_ref(), + base_fee_per_gas: payload.base_fee_per_gas.0, + claimed_hash: payload.block_hash.0, + transactions: payload.transactions.iter().map(|tx| &tx[..]).collect(), + withdrawals: None, + blob_gas: None, + parent_beacon_block_root: None, + requests: None, + amsterdam: None, + } + .verify() + } + SignedBeaconBlock::Capella(block) => { + let payload = &block.message.body.execution_payload; + Fields { + parent_hash: payload.parent_hash.0, + fee_recipient: payload.fee_recipient.0, + state_root: payload.state_root.0, + receipts_root: payload.receipts_root.0, + logs_bloom: payload.logs_bloom.as_ref(), + prev_randao: payload.prev_randao.0, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data.as_ref(), + base_fee_per_gas: payload.base_fee_per_gas.0, + claimed_hash: payload.block_hash.0, + transactions: payload.transactions.iter().map(|tx| &tx[..]).collect(), + withdrawals: Some(payload.withdrawals.as_ref()), + blob_gas: None, + parent_beacon_block_root: None, + requests: None, + amsterdam: None, + } + .verify() + } + // Deneb, and electra and fulu on top of it: one payload container, + // with the requests list the only addition. + SignedBeaconBlock::Deneb(_) + | SignedBeaconBlock::Electra(_) + | SignedBeaconBlock::Fulu(_) => { + let parent_root = match block { + SignedBeaconBlock::Deneb(block) => block.message.parent_root.0, + SignedBeaconBlock::Electra(block) | SignedBeaconBlock::Fulu(block) => { + block.message.parent_root.0 + } + _ => unreachable!("narrowed by the outer match"), + }; + let (payload, requests) = match block { + SignedBeaconBlock::Deneb(block) => (&block.message.body.execution_payload, None), + SignedBeaconBlock::Electra(block) | SignedBeaconBlock::Fulu(block) => ( + &block.message.body.execution_payload, + Some(electra::get_execution_requests_list( + &block.message.body.execution_requests, + )), + ), + _ => unreachable!("narrowed by the outer match"), + }; + Fields { + parent_hash: payload.parent_hash.0, + fee_recipient: payload.fee_recipient.0, + state_root: payload.state_root.0, + receipts_root: payload.receipts_root.0, + logs_bloom: payload.logs_bloom.as_ref(), + prev_randao: payload.prev_randao.0, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data.as_ref(), + base_fee_per_gas: payload.base_fee_per_gas.0, + claimed_hash: payload.block_hash.0, + transactions: payload.transactions.iter().map(|tx| &tx[..]).collect(), + withdrawals: Some(payload.withdrawals.as_ref()), + blob_gas: Some((payload.blob_gas_used, payload.excess_blob_gas)), + parent_beacon_block_root: Some(parent_root), + requests, + amsterdam: None, + } + .verify() + } + SignedBeaconBlock::Phase0(_) + | SignedBeaconBlock::Altair(_) + | SignedBeaconBlock::Gloas(_) + | SignedBeaconBlock::Lean(_) => Ok(()), + } +} + +/// A gloas envelope's payload hash against its header. +pub fn verify_envelope( + envelope: &gloas::SignedExecutionPayloadEnvelope, +) -> Result<(), HashMismatch> { + let message = &envelope.message; + let payload = &message.payload; + Fields { + parent_hash: payload.parent_hash.0, + fee_recipient: payload.fee_recipient.0, + state_root: payload.state_root.0, + receipts_root: payload.receipts_root.0, + logs_bloom: payload.logs_bloom.as_ref(), + prev_randao: payload.prev_randao.0, + block_number: payload.block_number, + gas_limit: payload.gas_limit, + gas_used: payload.gas_used, + timestamp: payload.timestamp, + extra_data: payload.extra_data.as_ref(), + base_fee_per_gas: payload.base_fee_per_gas.0, + claimed_hash: payload.block_hash.0, + transactions: payload.transactions.iter().map(|tx| &tx[..]).collect(), + withdrawals: Some(payload.withdrawals.as_ref()), + blob_gas: Some((payload.blob_gas_used, payload.excess_blob_gas)), + parent_beacon_block_root: Some(message.parent_beacon_block_root.0), + requests: Some(gloas_stf::get_execution_requests_list( + &message.execution_requests, + )), + amsterdam: Some((payload.block_access_list.as_ref(), payload.slot_number)), + } + .verify() +} + +/// `envelope` with its payload's `block_hash` set to the header's, for tests +/// that build a structurally valid envelope out of arbitrary fields. +#[cfg(test)] +pub(crate) fn seal_envelope( + mut envelope: gloas::SignedExecutionPayloadEnvelope, +) -> gloas::SignedExecutionPayloadEnvelope { + if let Err(mismatch) = verify_envelope(&envelope) { + envelope.message.payload.block_hash = mismatch.computed.into(); + } + envelope +} + +/// [`verify_block`] as a gossip verdict: `Err(Reject)` on a mismatch, counted +/// and logged. +pub(crate) fn reject_block_on_mismatch( + block: &SignedBeaconBlock, + block_root: Root, +) -> Result<(), Outcome> { + verify_block(block).map_err(|mismatch| refuse("block", block.slot(), block_root, mismatch)) +} + +/// [`verify_envelope`] as a verdict, shared by the gossip topic and the +/// fetched-envelope checks. +pub(crate) fn reject_envelope_on_mismatch( + envelope: &gloas::SignedExecutionPayloadEnvelope, +) -> Result<(), Outcome> { + verify_envelope(envelope).map_err(|mismatch| { + refuse( + "envelope", + envelope.message.payload.slot_number, + envelope.message.beacon_block_root, + mismatch, + ) + }) +} + +fn refuse(kind: &str, slot: u64, root: Root, mismatch: HashMismatch) -> Outcome { + metrics::inc_beacon_payload_block_hash_mismatch(kind); + warn!( + kind, + slot, + root = %ShortRoot(&root.0), + claimed = %ShortRoot(&mismatch.claimed), + computed = %ShortRoot(&mismatch.computed), + "Refusing an execution payload whose block hash is not its header's" + ); + Outcome::Reject(RejectReason::PayloadBlockHashMismatch) +} + +#[cfg(test)] +mod tests; diff --git a/crates/net/p2p/src/beacon/payload_hash/tests.rs b/crates/net/p2p/src/beacon/payload_hash/tests.rs new file mode 100644 index 000000000..500be82c8 --- /dev/null +++ b/crates/net/p2p/src/beacon/payload_hash/tests.rs @@ -0,0 +1,233 @@ +//! Every fork the node follows, against a payload taken from a real chain. +//! The vectors are SSZ as the Beacon API served it +//! (`beacon/testdata/payload_hash/`): +//! +//! | fork | chain | object | +//! |-----------|-------------|----------------------| +//! | bellatrix | Sepolia | block, slot 500000 | +//! | capella | Sepolia | block, slot 1900000 | +//! | deneb | Hoodi | block, slot 2003 | +//! | electra | Hoodi | block, slot 70001 | +//! | fulu | mainnet | block, slot 15576000 | +//! | gloas | Plataberget | envelope, slot 593101 | + +use ethlambda_types::beacon::containers::electra; +use ethlambda_types::beacon::fork::ForkName; +use libssz::SszDecode as _; + +use super::*; + +const BELLATRIX: &[u8] = + include_bytes!("../testdata/payload_hash/sepolia_bellatrix_slot_500000.ssz"); +const CAPELLA: &[u8] = include_bytes!("../testdata/payload_hash/sepolia_capella_slot_1900000.ssz"); +const DENEB: &[u8] = include_bytes!("../testdata/payload_hash/hoodi_deneb_slot_2003.ssz"); +const ELECTRA: &[u8] = include_bytes!("../testdata/payload_hash/hoodi_electra_slot_70001.ssz"); +const FULU: &[u8] = include_bytes!("../testdata/payload_hash/mainnet_fulu_slot_15576000.ssz"); +const GLOAS: &[u8] = + include_bytes!("../testdata/payload_hash/plataberget_gloas_envelope_slot_593101.ssz"); + +fn block(fork: ForkName, bytes: &[u8]) -> SignedBeaconBlock { + SignedBeaconBlock::from_ssz(fork, bytes).expect("a real block decodes") +} + +fn envelope() -> gloas::SignedExecutionPayloadEnvelope { + gloas::SignedExecutionPayloadEnvelope::from_ssz_bytes(GLOAS).expect("a real envelope decodes") +} + +fn bellatrix_payload( + block: &mut SignedBeaconBlock, +) -> &mut ethlambda_types::beacon::containers::bellatrix::ExecutionPayload { + let SignedBeaconBlock::Bellatrix(inner) = block else { + panic!("bellatrix"); + }; + &mut inner.message.body.execution_payload +} + +/// An all-zero request: only its presence matters to the requests hash. +fn consolidation() -> electra::ConsolidationRequest { + electra::ConsolidationRequest::from_ssz_bytes(&[0; 116]).expect("a fixed-size container") +} + +fn withdrawal() -> capella::Withdrawal { + capella::Withdrawal::from_ssz_bytes(&[0; 44]).expect("a fixed-size container") +} + +fn assert_refused(result: Result<(), HashMismatch>, what: &str) { + assert!(result.is_err(), "{what}: the hash must no longer match"); +} + +#[test] +fn every_real_payload_matches_its_hash() { + for (fork, bytes) in [ + (ForkName::Bellatrix, BELLATRIX), + (ForkName::Capella, CAPELLA), + (ForkName::Deneb, DENEB), + (ForkName::Electra, ELECTRA), + (ForkName::Fulu, FULU), + ] { + let block = block(fork, bytes); + assert_eq!(verify_block(&block), Ok(()), "{fork:?}"); + } + assert_eq!(verify_envelope(&envelope()), Ok(()), "gloas"); +} + +#[test] +fn the_vectors_exercise_what_each_fork_adds() { + let SignedBeaconBlock::Capella(capella) = block(ForkName::Capella, CAPELLA) else { + panic!("capella"); + }; + assert!( + !capella + .message + .body + .execution_payload + .withdrawals + .is_empty(), + "capella's vector must carry withdrawals" + ); + let SignedBeaconBlock::Fulu(fulu) = block(ForkName::Fulu, FULU) else { + panic!("fulu"); + }; + let SignedBeaconBlock::Bellatrix(bellatrix) = block(ForkName::Bellatrix, BELLATRIX) else { + panic!("bellatrix"); + }; + // Past the merge: a pre-merge block carries the empty default payload. + assert_ne!( + bellatrix.message.body.execution_payload.block_hash.0, + [0; 32] + ); + assert!(!fulu.message.body.execution_payload.transactions.is_empty()); + assert!(!envelope().message.payload.transactions.is_empty()); +} + +#[test] +fn a_changed_bellatrix_field_is_refused() { + let mut changed = block(ForkName::Bellatrix, BELLATRIX); + bellatrix_payload(&mut changed).gas_used += 1; + assert_refused(verify_block(&changed), "gas_used"); + + let mut changed = block(ForkName::Bellatrix, BELLATRIX); + bellatrix_payload(&mut changed).block_hash.0[0] ^= 1; + assert_refused(verify_block(&changed), "block_hash"); +} + +#[test] +fn a_changed_capella_withdrawal_is_refused() { + let mut changed = block(ForkName::Capella, CAPELLA); + let SignedBeaconBlock::Capella(inner) = &mut changed else { + panic!("capella"); + }; + inner.message.body.execution_payload.withdrawals[0].amount += 1; + assert_refused(verify_block(&changed), "withdrawal amount"); +} + +#[test] +fn a_changed_deneb_field_is_refused() { + for what in ["excess_blob_gas", "parent_root", "timestamp"] { + let mut changed = block(ForkName::Deneb, DENEB); + let SignedBeaconBlock::Deneb(inner) = &mut changed else { + panic!("deneb"); + }; + match what { + "excess_blob_gas" => inner.message.body.execution_payload.excess_blob_gas += 1, + // The header commits to the parent beacon block root, which the + // payload does not carry: it comes from the block. + "parent_root" => inner.message.parent_root.0[0] ^= 1, + _ => inner.message.body.execution_payload.timestamp += 1, + } + assert_refused(verify_block(&changed), what); + } +} + +#[test] +fn a_changed_electra_or_fulu_field_is_refused() { + for (fork, bytes) in [(ForkName::Electra, ELECTRA), (ForkName::Fulu, FULU)] { + for what in ["transaction", "parent_root", "request", "extra_data"] { + let mut changed = block(fork, bytes); + let (SignedBeaconBlock::Electra(inner) | SignedBeaconBlock::Fulu(inner)) = &mut changed + else { + panic!("electra or fulu"); + }; + match what { + "transaction" => { + let payload = &mut inner.message.body.execution_payload; + let last = payload.transactions.len() - 1; + let tx = &mut payload.transactions[last]; + let end = tx.len() - 1; + tx[end] ^= 1; + } + "parent_root" => inner.message.parent_root.0[0] ^= 1, + // The requests hash covers a request however empty its + // payload: one more consolidation moves it. + "request" => inner + .message + .body + .execution_requests + .consolidations + .push(consolidation()) + .expect("room for one more request"), + _ => inner + .message + .body + .execution_payload + .extra_data + .push(1) + .expect("room for one more byte"), + } + assert_refused(verify_block(&changed), &format!("{fork:?} {what}")); + } + } +} + +#[test] +fn a_changed_gloas_field_is_refused() { + for what in [ + "slot_number", + "block_access_list", + "parent_beacon_block_root", + "execution_requests", + "withdrawal", + "transaction", + "base_fee_per_gas", + ] { + let mut changed = envelope(); + let message = &mut changed.message; + match what { + "slot_number" => message.payload.slot_number += 1, + "block_access_list" => message.payload.block_access_list.push(0), + "parent_beacon_block_root" => message.parent_beacon_block_root.0[0] ^= 1, + "execution_requests" => message + .execution_requests + .consolidations + .push(consolidation()), + "withdrawal" => message.payload.withdrawals.push(withdrawal()), + "transaction" => { + let last = message.payload.transactions.len() - 1; + let tx = &mut message.payload.transactions[last]; + let end = tx.len() - 1; + tx[end] ^= 1; + } + _ => message.payload.base_fee_per_gas.0[31] ^= 1, + } + assert_refused(verify_envelope(&changed), what); + } +} + +#[test] +fn a_pre_merge_bellatrix_payload_has_nothing_to_check() { + // Before the merge a block carries the empty default payload. + let mut empty = block(ForkName::Bellatrix, BELLATRIX); + let payload = bellatrix_payload(&mut empty); + payload.block_hash = Default::default(); + payload.parent_hash = Default::default(); + assert_eq!(verify_block(&empty), Ok(())); +} + +#[test] +fn a_mismatch_is_a_reject_with_its_own_reason() { + let mut changed = envelope(); + changed.message.payload.gas_used += 1; + let outcome = reject_envelope_on_mismatch(&changed).expect_err("a changed envelope"); + assert_eq!(outcome.labels(), ("reject", "payload_block_hash_mismatch")); + assert!(reject_envelope_on_mismatch(&envelope()).is_ok()); +} diff --git a/crates/net/p2p/src/beacon/testdata/payload_hash/hoodi_deneb_slot_2003.ssz b/crates/net/p2p/src/beacon/testdata/payload_hash/hoodi_deneb_slot_2003.ssz new file mode 100644 index 000000000..0520bc7e8 Binary files /dev/null and b/crates/net/p2p/src/beacon/testdata/payload_hash/hoodi_deneb_slot_2003.ssz differ diff --git a/crates/net/p2p/src/beacon/testdata/payload_hash/hoodi_electra_slot_70001.ssz b/crates/net/p2p/src/beacon/testdata/payload_hash/hoodi_electra_slot_70001.ssz new file mode 100644 index 000000000..9d745df6c Binary files /dev/null and b/crates/net/p2p/src/beacon/testdata/payload_hash/hoodi_electra_slot_70001.ssz differ diff --git a/crates/net/p2p/src/beacon/testdata/payload_hash/mainnet_fulu_slot_15576000.ssz b/crates/net/p2p/src/beacon/testdata/payload_hash/mainnet_fulu_slot_15576000.ssz new file mode 100644 index 000000000..3f2355c3d Binary files /dev/null and b/crates/net/p2p/src/beacon/testdata/payload_hash/mainnet_fulu_slot_15576000.ssz differ diff --git a/crates/net/p2p/src/beacon/testdata/payload_hash/plataberget_gloas_envelope_slot_593101.ssz b/crates/net/p2p/src/beacon/testdata/payload_hash/plataberget_gloas_envelope_slot_593101.ssz new file mode 100644 index 000000000..b541030e9 Binary files /dev/null and b/crates/net/p2p/src/beacon/testdata/payload_hash/plataberget_gloas_envelope_slot_593101.ssz differ diff --git a/crates/net/p2p/src/beacon/testdata/payload_hash/sepolia_bellatrix_slot_500000.ssz b/crates/net/p2p/src/beacon/testdata/payload_hash/sepolia_bellatrix_slot_500000.ssz new file mode 100644 index 000000000..3f721faea Binary files /dev/null and b/crates/net/p2p/src/beacon/testdata/payload_hash/sepolia_bellatrix_slot_500000.ssz differ diff --git a/crates/net/p2p/src/beacon/testdata/payload_hash/sepolia_capella_slot_1900000.ssz b/crates/net/p2p/src/beacon/testdata/payload_hash/sepolia_capella_slot_1900000.ssz new file mode 100644 index 000000000..e8edef812 Binary files /dev/null and b/crates/net/p2p/src/beacon/testdata/payload_hash/sepolia_capella_slot_1900000.ssz differ diff --git a/crates/net/p2p/src/gossipsub/handler.rs b/crates/net/p2p/src/gossipsub/handler.rs index d6b90bce0..bc06df996 100644 --- a/crates/net/p2p/src/gossipsub/handler.rs +++ b/crates/net/p2p/src/gossipsub/handler.rs @@ -36,6 +36,7 @@ use super::{ }; use crate::beacon::constants::ATTESTATION_SUBNET_COUNT; use crate::beacon::decode::BeaconGossip; +use crate::beacon::payload_hash; use crate::beacon::verdict::{self, Dispatch, GossipId, Validated}; use crate::beacon::{BeaconWire, decode as beacon_decode, topics as beacon_topics}; use crate::{P2PServer, metrics}; @@ -323,6 +324,11 @@ fn triage_block(server: &P2PServer, wire: &BeaconWire, payload: &[u8]) -> Dispat { return Dispatch::Report(outcome); } + // Stricter than the specification: see `beacon::payload_hash`. After the + // rules that drop most messages, since it hashes every transaction. + if let Err(outcome) = payload_hash::reject_block_on_mismatch(&block, block_root) { + return Dispatch::Report(outcome); + } Dispatch::Validate(Validated::Block { block: Box::new(block), block_root, @@ -529,6 +535,9 @@ fn triage_envelope(server: &P2PServer, payload: &[u8]) -> Dispatch { { return Dispatch::Report(outcome); } + if let Err(outcome) = payload_hash::reject_envelope_on_mismatch(&envelope) { + return Dispatch::Report(outcome); + } Dispatch::Validate(Validated::Envelope(Box::new(envelope))) } @@ -1762,7 +1771,7 @@ mod tests { #[tokio::test] async fn an_envelope_passes_triage_unless_its_key_was_seen() { let mut server = unconnected_beacon_server(Config::mainnet(), 0).await; - let envelope = crate::test_support::envelope(1, 3); + let envelope = payload_hash::seal_envelope(crate::test_support::envelope(1, 3)); let payload = envelope.to_ssz(); assert!(matches!( @@ -1780,6 +1789,20 @@ mod tests { )); } + /// An envelope whose block hash is not its payload header's is rejected + /// in triage, before any state is read, and counted. + #[tokio::test] + async fn an_envelope_with_a_wrong_block_hash_is_rejected_in_triage() { + let server = unconnected_beacon_server(Config::mainnet(), 0).await; + let mut envelope = payload_hash::seal_envelope(crate::test_support::envelope(1, 3)); + envelope.message.payload.gas_used += 1; + + assert!(matches!( + triage_envelope(&server, &envelope.to_ssz()), + Dispatch::Report(Outcome::Reject(RejectReason::PayloadBlockHashMismatch)) + )); + } + /// A payload attestation for a slot before gloas is rejected by the cheap /// checks; mainnet's gloas epoch is not zero, so slot zero is before it. #[tokio::test] diff --git a/crates/net/p2p/src/metrics.rs b/crates/net/p2p/src/metrics.rs index 643300967..a38546377 100644 --- a/crates/net/p2p/src/metrics.rs +++ b/crates/net/p2p/src/metrics.rs @@ -474,6 +474,24 @@ pub fn inc_beacon_gossip_verdict_expired(kind: &str) { .inc(); } +static LEAN_BEACON_PAYLOAD_BLOCK_HASH_MISMATCH_TOTAL: LazyLock = LazyLock::new( + || { + register_int_counter_vec!( + "lean_beacon_payload_block_hash_mismatch_total", + "Execution payloads refused because their block hash is not the hash of the header rebuilt from their fields, by kind (block, envelope)", + &["kind"] + ) + .unwrap() + }, +); + +/// Count one payload refused for a block hash that is not its header's. +pub fn inc_beacon_payload_block_hash_mismatch(kind: &str) { + LEAN_BEACON_PAYLOAD_BLOCK_HASH_MISMATCH_TOTAL + .with_label_values(&[kind]) + .inc(); +} + pub fn inc_beacon_status_digest_mismatch() { LEAN_BEACON_STATUS_DIGEST_MISMATCH_TOTAL.inc(); } diff --git a/crates/net/p2p/src/req_resp/envelope_client.rs b/crates/net/p2p/src/req_resp/envelope_client.rs index 1714ce3dc..e10f16529 100644 --- a/crates/net/p2p/src/req_resp/envelope_client.rs +++ b/crates/net/p2p/src/req_resp/envelope_client.rs @@ -384,7 +384,13 @@ mod tests { use crate::{BACKOFF_MULTIPLIER, ConnectionDirection, INITIAL_BACKOFF_MS, MAX_FETCH_RETRIES}; fn envelope_for(slot: u64, block_root: H256) -> SignedExecutionPayloadEnvelope { - crate::beacon::encoding::test_support::envelope(block_root, slot, H256::ZERO) + // Sealed: the fetched-envelope rules refuse a block hash that is not + // the payload's header's. + crate::beacon::payload_hash::seal_envelope(crate::beacon::encoding::test_support::envelope( + block_root, + slot, + H256::ZERO, + )) } fn fulu_block_at(slot: u64) -> SignedBeaconBlock { diff --git a/docs/beacon_wire.md b/docs/beacon_wire.md index c2138785b..8d3afd997 100644 --- a/docs/beacon_wire.md +++ b/docs/beacon_wire.md @@ -212,6 +212,14 @@ an unbounded progressive bitlist, so one longer than electra's type bound is REJECTed first, reading its length without expanding it, since expanding is what the seen-set check does before any other. +One rule on `beacon_block` (bellatrix to fulu) and `execution_payload` is +stricter than the specification: the payload's `block_hash` must equal the hash +of the execution header rebuilt from the payload's own fields, or the message is +REJECTed (`beacon/payload_hash.rs`; the reason is `payload_block_hash_mismatch`, +and `lean_beacon_payload_block_hash_mismatch_total{kind}` counts it). It runs +after the cheap rules and before the stateful half. See +[spec_deviations.md](./spec_deviations.md#a-payloads-block-hash-must-be-its-headers). + Three deliberate departures from the gloas gossip rules, each for want of state this node does not keep. Where the specification rejects a message whose block failed validation, this node (having no bad-block cache) cannot tell that from @@ -603,7 +611,8 @@ not imported only briefly and in small numbers, so a batch's envelopes must arrive before the next batch's blocks do. Everything fetched goes through the same rules as an `execution_payload` gossip message (`beacon/envelope_checks.rs`) before the actor sees it; a `Queue` verdict is -forwarded too. +forwarded too. That includes the payload block hash rule above, which the +specification does not have. ### Data column sidecars @@ -806,6 +815,7 @@ as a query filter, so a `quic`-only record is invisible to it. | `lean_beacon_gossip_messages_total{topic,result}` | Gossip received, by topic and by `decoded` / `decode_failed` / `decompress_failed` / `unsupported_fork` | | `lean_beacon_gossip_validation_total{kind,outcome,reason}` | Gossip verdicts, now including `beacon_aggregate_and_proof` and `beacon_attestation`; see [metrics.md](./metrics.md#beacon-gossip-validation) | | `lean_beacon_gossip_verdict_expired_total{kind}` | Verdicts that came too late to propagate anything; should stay at zero | +| `lean_beacon_payload_block_hash_mismatch_total{kind}` | Payloads refused because `block_hash` is not the hash of the header rebuilt from their fields, by `block` / `envelope` | | `lean_beacon_status_digest_mismatch_total` | Handshakes seen from another fork digest | | `lean_beacon_fork_digest{digest}` | The digest computed at startup, as a label | | `lean_beacon_aggregate_decode_seconds` | Time spent decoding one aggregate off the wire | diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 49ee15c03..c69f24ce9 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -461,6 +461,37 @@ that the tests compare like with like. - **Equivalence:** each is the specification's answer wherever the specification has one; they differ only where it would have raised. +## A payload's block hash must be its header's + +`beacon_block` (bellatrix to fulu) and `execution_payload` gossip, and every +envelope fetched by root or range, are REJECTed when the payload's `block_hash` +is not `keccak256(rlp(header))` for the execution block header rebuilt from the +payload's own fields. + +- **The specification:** has no such rule. A hash that does not match is the + execution client's to find, when the chain actor hands it the payload. +- **ethlambda:** rebuilds the header in `ethlambda-p2p` + (`beacon/payload_hash.rs`) and refuses the message before the stateful rules + run. The header takes the payload's fields, the transactions and withdrawals + tries, the parent beacon block root (the block's `parent_root`; the + envelope's own field in gloas), the EIP-7685 requests hash (the Engine API's + request list, `get_execution_requests_list`), and in gloas the keccak of the + block access list bytes (EIP-7928) and the slot number (EIP-7843). It is run + per fork against real chain data: see the vector table in the module's tests. + A bellatrix payload that is the empty default (before the merge) is skipped. +- **Why:** since `engine_newPayloadV2` the Engine API answers a block-hash + mismatch with `INVALID` and a null `latestValidHash`, the same answer as for + a payload that executed badly and whose last valid ancestor is unknown. + Applying payloads to fork choice before the execution client answers is only + safe if the structural case cannot get that far. +- **Cost and counter:** one trie over the transactions per accepted-looking + message, after the cheap rules that drop most of them. + `lean_beacon_payload_block_hash_mismatch_total{kind}` counts refusals and a + `warn!` names the slot and root. +- **Equivalence:** a stricter filter, never a looser one: every payload it + refuses is one the execution client would call `INVALID`. Blocks fetched by + `beacon_blocks_by_range/root` are not checked here. + ## The envelope check reads the cached state root `verify_execution_payload_envelope` compares `envelope.beacon_block_root` with