From dd512cd82a69836329acb32cb90c46ed1315ea43 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 27 Jan 2023 11:39:26 +0100 Subject: [PATCH 1/3] stub out tx root check, fix block hash calculation --- beacon_node/beacon_chain/src/beacon_chain.rs | 22 ++++++++++++------- beacon_node/execution_layer/src/block_hash.rs | 8 ++++++- consensus/types/src/execution_block_header.rs | 13 ++++++++--- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 72b5b58bbe..d82e0feb48 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1037,14 +1037,20 @@ impl BeaconChain { ); } - return Err(Error::InconsistentPayloadReconstructed { - slot: blinded_block.slot(), - exec_block_hash, - canonical_payload_root: execution_payload_header.tree_hash_root(), - reconstructed_payload_root: header_from_payload.tree_hash_root(), - canonical_transactions_root: execution_payload_header.transactions_root(), - reconstructed_transactions_root: header_from_payload.transactions_root(), - }); + if execution_payload_header.transactions_root() + != header_from_payload.transactions_root() + { + //FIXME(sean) we're not decoding blobs txs correctly yet + } else { + return Err(Error::InconsistentPayloadReconstructed { + slot: blinded_block.slot(), + exec_block_hash, + canonical_payload_root: execution_payload_header.tree_hash_root(), + reconstructed_payload_root: header_from_payload.tree_hash_root(), + canonical_transactions_root: execution_payload_header.transactions_root(), + reconstructed_transactions_root: header_from_payload.transactions_root(), + }); + } } // Add the payload to the block to form a full block. diff --git a/beacon_node/execution_layer/src/block_hash.rs b/beacon_node/execution_layer/src/block_hash.rs index e9b7dcc17f..00bf4a7e8a 100644 --- a/beacon_node/execution_layer/src/block_hash.rs +++ b/beacon_node/execution_layer/src/block_hash.rs @@ -36,12 +36,15 @@ impl ExecutionLayer { None }; + let rlp_excess_data_gas = payload.excess_data_gas().ok(); + // Construct the block header. let exec_block_header = ExecutionBlockHeader::from_payload( payload, KECCAK_EMPTY_LIST_RLP.as_fixed_bytes().into(), rlp_transactions_root, rlp_withdrawals_root, + rlp_excess_data_gas, ); // Hash the RLP encoding of the block header. @@ -75,12 +78,15 @@ pub fn rlp_encode_withdrawal(withdrawal: &JsonWithdrawal) -> Vec { pub fn rlp_encode_block_header(header: &ExecutionBlockHeader) -> Vec { let mut rlp_header_stream = RlpStream::new(); rlp_header_stream.begin_unbounded_list(); - map_execution_block_header_fields_except_withdrawals!(&header, |_, field| { + map_execution_block_header_fields_except_withdrawals_excess_data_gas!(&header, |_, field| { rlp_header_stream.append(field); }); if let Some(withdrawals_root) = &header.withdrawals_root { rlp_header_stream.append(withdrawals_root); } + if let Some(excess_data_gas) = &header.excess_data_gas { + rlp_header_stream.append(excess_data_gas); + } rlp_header_stream.finalize_unbounded_list(); rlp_header_stream.out().into() } diff --git a/consensus/types/src/execution_block_header.rs b/consensus/types/src/execution_block_header.rs index b19988ff7d..6dca1f57a4 100644 --- a/consensus/types/src/execution_block_header.rs +++ b/consensus/types/src/execution_block_header.rs @@ -24,9 +24,13 @@ use metastruct::metastruct; /// /// Credit to Reth for the type definition. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -#[metastruct(mappings(map_execution_block_header_fields_except_withdrawals(exclude( - withdrawals_root -))))] +#[metastruct(mappings( + map_execution_block_header_fields_except_withdrawals(exclude(withdrawals_root)), + map_execution_block_header_fields_except_withdrawals_excess_data_gas(exclude( + withdrawals_root, + excess_data_gas + )) +))] pub struct ExecutionBlockHeader { pub parent_hash: Hash256, pub ommers_hash: Hash256, @@ -45,6 +49,7 @@ pub struct ExecutionBlockHeader { pub nonce: Hash64, pub base_fee_per_gas: Uint256, pub withdrawals_root: Option, + pub excess_data_gas: Option, } impl ExecutionBlockHeader { @@ -53,6 +58,7 @@ impl ExecutionBlockHeader { rlp_empty_list_root: Hash256, rlp_transactions_root: Hash256, rlp_withdrawals_root: Option, + rlp_excess_data_gas: Option, ) -> Self { // Most of these field mappings are defined in EIP-3675 except for `mixHash`, which is // defined in EIP-4399. @@ -74,6 +80,7 @@ impl ExecutionBlockHeader { nonce: Hash64::zero(), base_fee_per_gas: payload.base_fee_per_gas(), withdrawals_root: rlp_withdrawals_root, + excess_data_gas: rlp_excess_data_gas, } } } From 2c12200a1aeba5a245b0ff538ef9b24203869517 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 27 Jan 2023 11:41:59 +0100 Subject: [PATCH 2/3] fix compile --- beacon_node/execution_layer/src/block_hash.rs | 4 ++-- consensus/types/src/execution_block_header.rs | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/beacon_node/execution_layer/src/block_hash.rs b/beacon_node/execution_layer/src/block_hash.rs index 00bf4a7e8a..14993acac4 100644 --- a/beacon_node/execution_layer/src/block_hash.rs +++ b/beacon_node/execution_layer/src/block_hash.rs @@ -44,7 +44,7 @@ impl ExecutionLayer { KECCAK_EMPTY_LIST_RLP.as_fixed_bytes().into(), rlp_transactions_root, rlp_withdrawals_root, - rlp_excess_data_gas, + rlp_excess_data_gas.copied(), ); // Hash the RLP encoding of the block header. @@ -78,7 +78,7 @@ pub fn rlp_encode_withdrawal(withdrawal: &JsonWithdrawal) -> Vec { pub fn rlp_encode_block_header(header: &ExecutionBlockHeader) -> Vec { let mut rlp_header_stream = RlpStream::new(); rlp_header_stream.begin_unbounded_list(); - map_execution_block_header_fields_except_withdrawals_excess_data_gas!(&header, |_, field| { + map_execution_block_header_fields_except_withdrawals!(&header, |_, field| { rlp_header_stream.append(field); }); if let Some(withdrawals_root) = &header.withdrawals_root { diff --git a/consensus/types/src/execution_block_header.rs b/consensus/types/src/execution_block_header.rs index 6dca1f57a4..9c01d9a597 100644 --- a/consensus/types/src/execution_block_header.rs +++ b/consensus/types/src/execution_block_header.rs @@ -25,11 +25,7 @@ use metastruct::metastruct; /// Credit to Reth for the type definition. #[derive(Debug, Clone, PartialEq, Eq, Hash)] #[metastruct(mappings( - map_execution_block_header_fields_except_withdrawals(exclude(withdrawals_root)), - map_execution_block_header_fields_except_withdrawals_excess_data_gas(exclude( - withdrawals_root, - excess_data_gas - )) + map_execution_block_header_fields_except_withdrawals(exclude(withdrawals_root, excess_data_gas)), ))] pub struct ExecutionBlockHeader { pub parent_hash: Hash256, From 37e7c1d5c7c8862b542eb362fd41d95262fcc927 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 27 Jan 2023 17:59:40 +0100 Subject: [PATCH 3/3] keep verification of payloads pre 4844 --- beacon_node/beacon_chain/src/beacon_chain.rs | 27 +++++++++---------- .../work_reprocessing_queue.rs | 3 +-- consensus/types/src/execution_block_header.rs | 7 ++--- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 95d2b7922f..042c98afe3 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1026,22 +1026,19 @@ impl BeaconChain { //FIXME(sean) avoid the clone by comparing refs to headers (`as_execution_payload_header` method ?) let full_payload: FullPayload = execution_payload.clone().into(); - // Verify payload integrity. - let header_from_payload = full_payload.to_execution_payload_header(); - if header_from_payload != execution_payload_header { - for txn in execution_payload.transactions() { - debug!( - self.log, - "Reconstructed txn"; - "bytes" => format!("0x{}", hex::encode(&**txn)), - ); - } + //FIXME(sean) we're not decoding blobs txs correctly yet + if !matches!(execution_payload, ExecutionPayload::Eip4844(_)) { + // Verify payload integrity. + let header_from_payload = full_payload.to_execution_payload_header(); + if header_from_payload != execution_payload_header { + for txn in execution_payload.transactions() { + debug!( + self.log, + "Reconstructed txn"; + "bytes" => format!("0x{}", hex::encode(&**txn)), + ); + } - if execution_payload_header.transactions_root() - != header_from_payload.transactions_root() - { - //FIXME(sean) we're not decoding blobs txs correctly yet - } else { return Err(Error::InconsistentPayloadReconstructed { slot: blinded_block.slot(), exec_block_hash, diff --git a/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs b/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs index d7cede9ee7..4d0bdc0027 100644 --- a/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs +++ b/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs @@ -31,8 +31,7 @@ use tokio::sync::mpsc::{self, Receiver, Sender}; use tokio::time::error::Error as TimeError; use tokio_util::time::delay_queue::{DelayQueue, Key as DelayKey}; use types::{ - Attestation, EthSpec, Hash256, LightClientOptimisticUpdate, SignedAggregateAndProof, - SubnetId, + Attestation, EthSpec, Hash256, LightClientOptimisticUpdate, SignedAggregateAndProof, SubnetId, }; const TASK_NAME: &str = "beacon_processor_reprocess_queue"; diff --git a/consensus/types/src/execution_block_header.rs b/consensus/types/src/execution_block_header.rs index 9c01d9a597..c0c08795e8 100644 --- a/consensus/types/src/execution_block_header.rs +++ b/consensus/types/src/execution_block_header.rs @@ -24,9 +24,10 @@ use metastruct::metastruct; /// /// Credit to Reth for the type definition. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -#[metastruct(mappings( - map_execution_block_header_fields_except_withdrawals(exclude(withdrawals_root, excess_data_gas)), -))] +#[metastruct(mappings(map_execution_block_header_fields_except_withdrawals(exclude( + withdrawals_root, + excess_data_gas +)),))] pub struct ExecutionBlockHeader { pub parent_hash: Hash256, pub ommers_hash: Hash256,