From 6d0044992ede39e13bfd1b834bef09f8a9d85c07 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 21 Aug 2023 14:37:21 -0400 Subject: [PATCH 1/2] feat: add EIP-4788 parent_beacon_block_root to Header --- crates/consensus/auto-seal/src/lib.rs | 1 + crates/consensus/beacon/src/engine/handle.rs | 8 +- crates/consensus/beacon/src/engine/message.rs | 3 + crates/consensus/beacon/src/engine/mod.rs | 30 +++--- .../consensus/beacon/src/engine/test_utils.rs | 6 +- crates/consensus/common/src/validation.rs | 8 ++ crates/interfaces/src/consensus.rs | 4 + crates/net/eth-wire/src/types/blocks.rs | 4 + crates/payload/basic/src/lib.rs | 2 + crates/primitives/src/header.rs | 98 +++++++++++++------ crates/rpc/rpc-engine-api/src/engine_api.rs | 15 ++- crates/rpc/rpc-engine-api/tests/it/payload.rs | 14 +-- crates/rpc/rpc-types/src/eth/block.rs | 7 ++ .../rpc/rpc-types/src/eth/engine/payload.rs | 77 +++++++-------- crates/rpc/rpc/src/eth/api/pending_block.rs | 1 + testing/ef-tests/src/models.rs | 3 + 16 files changed, 188 insertions(+), 93 deletions(-) diff --git a/crates/consensus/auto-seal/src/lib.rs b/crates/consensus/auto-seal/src/lib.rs index cadfb5593b7c..f68d1271a7b9 100644 --- a/crates/consensus/auto-seal/src/lib.rs +++ b/crates/consensus/auto-seal/src/lib.rs @@ -277,6 +277,7 @@ impl StorageInner { blob_gas_used: None, excess_blob_gas: None, extra_data: Default::default(), + parent_beacon_block_root: None, }; header.transactions_root = if transactions.is_empty() { diff --git a/crates/consensus/beacon/src/engine/handle.rs b/crates/consensus/beacon/src/engine/handle.rs index d2b8661b6080..176a8cbbf9de 100644 --- a/crates/consensus/beacon/src/engine/handle.rs +++ b/crates/consensus/beacon/src/engine/handle.rs @@ -5,6 +5,7 @@ use crate::{ BeaconForkChoiceUpdateError, BeaconOnNewPayloadError, }; use futures::TryFutureExt; +use reth_primitives::H256; use reth_rpc_types::engine::{ ExecutionPayload, ForkchoiceState, ForkchoiceUpdated, PayloadAttributes, PayloadStatus, }; @@ -34,9 +35,14 @@ impl BeaconConsensusEngineHandle { pub async fn new_payload( &self, payload: ExecutionPayload, + parent_beacon_block_root: Option, ) -> Result { let (tx, rx) = oneshot::channel(); - let _ = self.to_engine.send(BeaconEngineMessage::NewPayload { payload, tx }); + let _ = self.to_engine.send(BeaconEngineMessage::NewPayload { + payload, + parent_beacon_block_root, + tx, + }); rx.await.map_err(|_| BeaconOnNewPayloadError::EngineUnavailable)? } diff --git a/crates/consensus/beacon/src/engine/message.rs b/crates/consensus/beacon/src/engine/message.rs index f894603ab4b4..9e29a07d54bd 100644 --- a/crates/consensus/beacon/src/engine/message.rs +++ b/crates/consensus/beacon/src/engine/message.rs @@ -5,6 +5,7 @@ use crate::{ use futures::{future::Either, FutureExt}; use reth_interfaces::consensus::ForkchoiceState; use reth_payload_builder::error::PayloadBuilderError; +use reth_primitives::H256; use reth_rpc_types::engine::{ ExecutionPayload, ForkChoiceUpdateResult, ForkchoiceUpdateError, ForkchoiceUpdated, PayloadAttributes, PayloadId, PayloadStatus, PayloadStatusEnum, @@ -146,6 +147,8 @@ pub enum BeaconEngineMessage { NewPayload { /// The execution payload received by Engine API. payload: ExecutionPayload, + /// The parent beacon block root, if any. + parent_beacon_block_root: Option, /// The sender for returning payload status result. tx: oneshot::Sender>, }, diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 6f6dfb35dd50..7665d575ce1c 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -1049,12 +1049,13 @@ where /// /// This returns a [`PayloadStatus`] that represents the outcome of a processed new payload and /// returns an error if an internal error occurred. - #[instrument(level = "trace", skip(self, payload), fields(block_hash= ?payload.block_hash, block_number = %payload.block_number.as_u64(), is_pipeline_idle = %self.sync.is_pipeline_idle()), target = "consensus::engine")] + #[instrument(level = "trace", skip(self, payload, parent_beacon_block_root), fields(block_hash= ?payload.block_hash, block_number = %payload.block_number.as_u64(), is_pipeline_idle = %self.sync.is_pipeline_idle()), target = "consensus::engine")] fn on_new_payload( &mut self, payload: ExecutionPayload, + parent_beacon_block_root: Option, ) -> Result { - let block = match self.ensure_well_formed_payload(payload) { + let block = match self.ensure_well_formed_payload(payload, parent_beacon_block_root) { Ok(block) => block, Err(status) => return Ok(status), }; @@ -1118,9 +1119,10 @@ where fn ensure_well_formed_payload( &self, payload: ExecutionPayload, + parent_beacon_block_root: Option, ) -> Result { let parent_hash = payload.parent_hash; - let block = match SealedBlock::try_from(payload) { + let block = match payload.try_into_sealed_block(parent_beacon_block_root) { Ok(block) => block, Err(error) => { error!(target: "consensus::engine", ?error, "Invalid payload"); @@ -1725,9 +1727,9 @@ where } } } - BeaconEngineMessage::NewPayload { payload, tx } => { + BeaconEngineMessage::NewPayload { payload, parent_beacon_block_root, tx } => { this.metrics.new_payload_messages.increment(1); - let res = this.on_new_payload(payload); + let res = this.on_new_payload(payload, parent_beacon_block_root); let _ = tx.send(res); } BeaconEngineMessage::TransitionConfigurationExchanged => { @@ -1865,7 +1867,7 @@ mod tests { assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); // consensus engine is still idle because no FCUs were received - let _ = env.send_new_payload(SealedBlock::default().into()).await; + let _ = env.send_new_payload(SealedBlock::default().into(), None).await; assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); // consensus engine is still idle because pruning is running @@ -2279,14 +2281,16 @@ mod tests { let mut engine_rx = spawn_consensus_engine(consensus_engine); // Send new payload - let res = - env.send_new_payload(random_block(&mut rng, 0, None, None, Some(0)).into()).await; + let res = env + .send_new_payload(random_block(&mut rng, 0, None, None, Some(0)).into(), None) + .await; // Invalid, because this is a genesis block assert_matches!(res, Ok(result) => assert_matches!(result.status, PayloadStatusEnum::Invalid { .. })); // Send new payload - let res = - env.send_new_payload(random_block(&mut rng, 1, None, None, Some(0)).into()).await; + let res = env + .send_new_payload(random_block(&mut rng, 1, None, None, Some(0)).into(), None) + .await; let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Syncing); assert_matches!(res, Ok(result) => assert_eq!(result, expected_result)); @@ -2336,7 +2340,7 @@ mod tests { // Send new payload let result = - env.send_new_payload_retry_on_syncing(block2.clone().into()).await.unwrap(); + env.send_new_payload_retry_on_syncing(block2.clone().into(), None).await.unwrap(); let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Valid) .with_latest_valid_hash(block2.hash); assert_eq!(result, expected_result); @@ -2434,7 +2438,7 @@ mod tests { // Send new payload let block = random_block(&mut rng, 2, Some(H256::random()), None, Some(0)); - let res = env.send_new_payload(block.into()).await; + let res = env.send_new_payload(block.into(), None).await; let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Syncing); assert_matches!(res, Ok(result) => assert_eq!(result, expected_result)); @@ -2497,7 +2501,7 @@ mod tests { // Send new payload let result = - env.send_new_payload_retry_on_syncing(block2.clone().into()).await.unwrap(); + env.send_new_payload_retry_on_syncing(block2.clone().into(), None).await.unwrap(); let expected_result = PayloadStatus::from_status(PayloadStatusEnum::Invalid { validation_error: BlockValidationError::BlockPreMerge { hash: block2.hash } diff --git a/crates/consensus/beacon/src/engine/test_utils.rs b/crates/consensus/beacon/src/engine/test_utils.rs index 2aafd028da3f..287cd0f41ceb 100644 --- a/crates/consensus/beacon/src/engine/test_utils.rs +++ b/crates/consensus/beacon/src/engine/test_utils.rs @@ -69,8 +69,9 @@ impl TestEnv { pub async fn send_new_payload( &self, payload: ExecutionPayload, + parent_beacon_block_root: Option, ) -> Result { - self.engine_handle.new_payload(payload).await + self.engine_handle.new_payload(payload, parent_beacon_block_root).await } /// Sends the `ExecutionPayload` message to the consensus engine and retries if the engine @@ -78,9 +79,10 @@ impl TestEnv { pub async fn send_new_payload_retry_on_syncing( &self, payload: ExecutionPayload, + parent_beacon_block_root: Option, ) -> Result { loop { - let result = self.send_new_payload(payload.clone()).await?; + let result = self.send_new_payload(payload.clone(), parent_beacon_block_root).await?; if !result.is_syncing() { return Ok(result) } diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index 2c2bde80fa40..d7feec42c972 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -50,6 +50,8 @@ pub fn validate_header_standalone( return Err(ConsensusError::BlobGasUsedUnexpected) } else if header.excess_blob_gas.is_some() { return Err(ConsensusError::ExcessBlobGasUnexpected) + } else if header.parent_beacon_block_root.is_some() { + return Err(ConsensusError::ParentBeaconBlockRootUnexpected) } Ok(()) @@ -451,6 +453,7 @@ pub fn validate_4844_header_with_parent( /// /// * `blob_gas_used` exists as a header field /// * `excess_blob_gas` exists as a header field +/// * `parent_beacon_block_root` exists as a header field /// * `blob_gas_used` is less than or equal to `MAX_DATA_GAS_PER_BLOCK` /// * `blob_gas_used` is a multiple of `DATA_GAS_PER_BLOB` pub fn validate_4844_header_standalone(header: &SealedHeader) -> Result<(), ConsensusError> { @@ -460,6 +463,10 @@ pub fn validate_4844_header_standalone(header: &SealedHeader) -> Result<(), Cons return Err(ConsensusError::ExcessBlobGasMissing) } + if header.parent_beacon_block_root.is_none() { + return Err(ConsensusError::ParentBeaconBlockRootMissing) + } + if blob_gas_used > MAX_DATA_GAS_PER_BLOCK { return Err(ConsensusError::BlobGasUsedExceedsMaxBlobGasPerBlock { blob_gas_used, @@ -633,6 +640,7 @@ mod tests { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }; // size: 0x9b5 diff --git a/crates/interfaces/src/consensus.rs b/crates/interfaces/src/consensus.rs index 9176bd06d7f2..9274cdcbe52f 100644 --- a/crates/interfaces/src/consensus.rs +++ b/crates/interfaces/src/consensus.rs @@ -143,6 +143,10 @@ pub enum ConsensusError { ExcessBlobGasMissing, #[error("Unexpected excess blob gas")] ExcessBlobGasUnexpected, + #[error("Missing parent beacon block root")] + ParentBeaconBlockRootMissing, + #[error("Unexpected parent beacon block root")] + ParentBeaconBlockRootUnexpected, #[error("Blob gas used {blob_gas_used} exceeds maximum allowance {max_blob_gas_per_block}")] BlobGasUsedExceedsMaxBlobGasPerBlock { blob_gas_used: u64, max_blob_gas_per_block: u64 }, #[error( diff --git a/crates/net/eth-wire/src/types/blocks.rs b/crates/net/eth-wire/src/types/blocks.rs index 808c8f4a4609..2b5bfd0207f3 100644 --- a/crates/net/eth-wire/src/types/blocks.rs +++ b/crates/net/eth-wire/src/types/blocks.rs @@ -260,6 +260,7 @@ mod test { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }, ]), }.encode(&mut data); @@ -293,6 +294,7 @@ mod test { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }, ]), }; @@ -407,6 +409,7 @@ mod test { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }, ], withdrawals: None, @@ -493,6 +496,7 @@ mod test { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }, ], withdrawals: None, diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 16be06110baa..96ac6d9a6dad 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -785,6 +785,7 @@ where extra_data: extra_data.into(), blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }; // seal the block @@ -856,6 +857,7 @@ where blob_gas_used: None, excess_blob_gas: None, extra_data: extra_data.into(), + parent_beacon_block_root: None, }; let block = Block { header, body: vec![], ommers: vec![], withdrawals }; diff --git a/crates/primitives/src/header.rs b/crates/primitives/src/header.rs index bc6d75194a2b..12f4e3fca82a 100644 --- a/crates/primitives/src/header.rs +++ b/crates/primitives/src/header.rs @@ -100,6 +100,9 @@ pub struct Header { /// with above-target blob gas consumption increase this value, blocks with below-target blob /// gas consumption decrease it (bounded at 0). This was added in EIP-4844. pub excess_blob_gas: Option, + /// TODO: Docs + /// This was added in EIP-4788. + pub parent_beacon_block_root: Option, /// An arbitrary byte array containing data relevant to this block. This must be 32 bytes or /// fewer; formally Hx. pub extra_data: Bytes, @@ -127,6 +130,7 @@ impl Default for Header { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, } } } @@ -227,6 +231,7 @@ impl Header { mem::size_of::>() + // base fee per gas mem::size_of::>() + // blob gas used mem::size_of::>() + // excess blob gas + mem::size_of::>() + // parent beacon block root self.extra_data.len() // extra data } @@ -252,32 +257,42 @@ impl Header { length += U256::from(base_fee).length(); } else if self.withdrawals_root.is_some() || self.blob_gas_used.is_some() || - self.excess_blob_gas.is_some() + self.excess_blob_gas.is_some() || + self.parent_beacon_block_root.is_some() { - length += 1; // EMPTY STRING CODE + length += 1; // EMPTY LIST CODE } if let Some(root) = self.withdrawals_root { length += root.length(); - } else if self.blob_gas_used.is_some() || self.excess_blob_gas.is_some() { + } else if self.blob_gas_used.is_some() || + self.excess_blob_gas.is_some() || + self.parent_beacon_block_root.is_some() + { length += 1; // EMPTY STRING CODE } if let Some(blob_gas_used) = self.blob_gas_used { length += U256::from(blob_gas_used).length(); - } else if self.excess_blob_gas.is_some() { - length += 1; // EMPTY STRING CODE + } else if self.excess_blob_gas.is_some() || self.parent_beacon_block_root.is_some() { + length += 1; // EMPTY LIST CODE } - // Encode excess blob gas length. If new fields are added, the above pattern will need to - // be repeated and placeholder length added. Otherwise, it's impossible to tell _which_ - // fields are missing. This is mainly relevant for contrived cases where a header is - // created at random, for example: + if let Some(excess_blob_gas) = self.excess_blob_gas { + length += U256::from(excess_blob_gas).length(); + } else if self.parent_beacon_block_root.is_some() { + length += 1; // EMPTY LIST CODE + } + + // Encode parent beacon block root length. If new fields are added, the above pattern will + // need to be repeated and placeholder length added. Otherwise, it's impossible to + // tell _which_ fields are missing. This is mainly relevant for contrived cases + // where a header is created at random, for example: // * A header is created with a withdrawals root, but no base fee. Shanghai blocks are // post-London, so this is technically not valid. However, a tool like proptest would // generate a block like this. - if let Some(excess_blob_gas) = self.excess_blob_gas { - length += U256::from(excess_blob_gas).length(); + if let Some(parent_beacon_block_root) = self.parent_beacon_block_root { + length += parent_beacon_block_root.length(); } length @@ -305,42 +320,54 @@ impl Encodable for Header { self.mix_hash.encode(out); H64::from_low_u64_be(self.nonce).encode(out); - // Encode base fee. Put empty string if base fee is missing, + // Encode base fee. Put empty list if base fee is missing, // but withdrawals root is present. if let Some(ref base_fee) = self.base_fee_per_gas { U256::from(*base_fee).encode(out); } else if self.withdrawals_root.is_some() || self.blob_gas_used.is_some() || - self.excess_blob_gas.is_some() + self.excess_blob_gas.is_some() || + self.parent_beacon_block_root.is_some() { - out.put_u8(EMPTY_STRING_CODE); + out.put_u8(EMPTY_LIST_CODE); } // Encode withdrawals root. Put empty string if withdrawals root is missing, // but blob gas used is present. if let Some(ref root) = self.withdrawals_root { root.encode(out); - } else if self.blob_gas_used.is_some() || self.excess_blob_gas.is_some() { + } else if self.blob_gas_used.is_some() || + self.excess_blob_gas.is_some() || + self.parent_beacon_block_root.is_some() + { out.put_u8(EMPTY_STRING_CODE); } - // Encode blob gas used. Put empty string if blob gas used is missing, + // Encode blob gas used. Put empty list if blob gas used is missing, // but excess blob gas is present. if let Some(ref blob_gas_used) = self.blob_gas_used { U256::from(*blob_gas_used).encode(out); - } else if self.excess_blob_gas.is_some() { + } else if self.excess_blob_gas.is_some() || self.parent_beacon_block_root.is_some() { + out.put_u8(EMPTY_LIST_CODE); + } + + // Encode excess blob gas. Put empty list if excess blob gas is missing, + // but parent beacon block root is present. + if let Some(ref excess_blob_gas) = self.excess_blob_gas { + U256::from(*excess_blob_gas).encode(out); + } else if self.parent_beacon_block_root.is_some() { out.put_u8(EMPTY_LIST_CODE); } - // Encode excess blob gas. If new fields are added, the above pattern will need to be - // repeated and placeholders added. Otherwise, it's impossible to tell _which_ fields - // are missing. This is mainly relevant for contrived cases where a header is created - // at random, for example: + // Encode parent beacon block root. If new fields are added, the above pattern will need to + // be repeated and placeholders added. Otherwise, it's impossible to tell _which_ + // fields are missing. This is mainly relevant for contrived cases where a header is + // created at random, for example: // * A header is created with a withdrawals root, but no base fee. Shanghai blocks are // post-London, so this is technically not valid. However, a tool like proptest would // generate a block like this. - if let Some(ref excess_blob_gas) = self.excess_blob_gas { - U256::from(*excess_blob_gas).encode(out); + if let Some(ref parent_beacon_block_root) = self.parent_beacon_block_root { + parent_beacon_block_root.encode(out); } } @@ -379,10 +406,11 @@ impl Decodable for Header { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }; if started_len - buf.len() < rlp_head.payload_length { - if buf.first().map(|b| *b == EMPTY_STRING_CODE).unwrap_or_default() { + if buf.first().map(|b| *b == EMPTY_LIST_CODE).unwrap_or_default() { buf.advance(1) } else { this.base_fee_per_gas = Some(U256::decode(buf)?.to::()); @@ -407,15 +435,23 @@ impl Decodable for Header { } } - // Decode excess blob gas. If new fields are added, the above pattern will need to be - // repeated and placeholders decoded. Otherwise, it's impossible to tell _which_ fields are - // missing. This is mainly relevant for contrived cases where a header is created at - // random, for example: + if started_len - buf.len() < rlp_head.payload_length { + if buf.first().map(|b| *b == EMPTY_LIST_CODE).unwrap_or_default() { + buf.advance(1) + } else { + this.excess_blob_gas = Some(U256::decode(buf)?.to::()); + } + } + + // Decode parent beacon block root. If new fields are added, the above pattern will need to + // be repeated and placeholders decoded. Otherwise, it's impossible to tell _which_ + // fields are missing. This is mainly relevant for contrived cases where a header is + // created at random, for example: // * A header is created with a withdrawals root, but no base fee. Shanghai blocks are // post-London, so this is technically not valid. However, a tool like proptest would // generate a block like this. if started_len - buf.len() < rlp_head.payload_length { - this.excess_blob_gas = Some(U256::decode(buf)?.to::()); + this.parent_beacon_block_root = Some(H256::decode(buf)?); } let consumed = started_len - buf.len(); @@ -642,6 +678,7 @@ mod ethers_compat { logs_bloom: block.logs_bloom.unwrap_or_default().0.into(), blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, } } } @@ -713,6 +750,7 @@ mod tests { withdrawals_root: None, blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }; assert_eq!(header.hash_slow(), expected_hash); } @@ -836,6 +874,7 @@ mod tests { ), blob_gas_used: Some(0x020000), excess_blob_gas: Some(0), + parent_beacon_block_root: None, }; let header = Header::decode(&mut data.as_slice()).unwrap(); @@ -892,6 +931,7 @@ mod tests { H256::from_str("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421") .unwrap(), ), + parent_beacon_block_root: None, blob_gas_used: Some(0), excess_blob_gas: Some(0x1600000), }; diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index 057b3954ee25..0213ce9cda47 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -75,7 +75,7 @@ where EngineApiMessageVersion::V1, PayloadOrAttributes::from_execution_payload(&payload, None), )?; - Ok(self.inner.beacon_consensus.new_payload(payload).await?) + Ok(self.inner.beacon_consensus.new_payload(payload, None).await?) } /// See also @@ -87,7 +87,7 @@ where EngineApiMessageVersion::V2, PayloadOrAttributes::from_execution_payload(&payload, None), )?; - Ok(self.inner.beacon_consensus.new_payload(payload).await?) + Ok(self.inner.beacon_consensus.new_payload(payload, None).await?) } /// See also @@ -443,6 +443,8 @@ where Ok(EngineApi::new_payload_v2(self, payload).await?) } + /// Handler for `engine_newPayloadV3` + /// See also async fn new_payload_v3( &self, _payload: ExecutionPayload, @@ -517,6 +519,15 @@ where Ok(EngineApi::get_payload_v2(self, payload_id).await?) } + /// Handler for `engine_getPayloadV3` + /// + /// Returns the most recent version of the payload that is available in the corresponding + /// payload build process at the time of receiving this call. + /// + /// See also + /// + /// Note: + /// > Provider software MAY stop the corresponding build process after serving this call. async fn get_payload_v3(&self, _payload_id: PayloadId) -> RpcResult { Err(jsonrpsee_types::error::ErrorCode::MethodNotFound.into()) } diff --git a/crates/rpc/rpc-engine-api/tests/it/payload.rs b/crates/rpc/rpc-engine-api/tests/it/payload.rs index bc9f1f4249ae..a39b59eea5a7 100644 --- a/crates/rpc/rpc-engine-api/tests/it/payload.rs +++ b/crates/rpc/rpc-engine-api/tests/it/payload.rs @@ -57,7 +57,7 @@ fn payload_validation() { b.header.extra_data = BytesMut::zeroed(32).freeze().into(); b }); - assert_matches!(TryInto::::try_into(block_with_valid_extra_data), Ok(_)); + assert_matches!(block_with_valid_extra_data.try_into_sealed_block(None), Ok(_)); // Invalid extra data let block_with_invalid_extra_data: Bytes = BytesMut::zeroed(33).freeze(); @@ -66,7 +66,7 @@ fn payload_validation() { b }); assert_matches!( - TryInto::::try_into(invalid_extra_data_block), + invalid_extra_data_block.try_into_sealed_block(None), Err(PayloadError::ExtraData(data)) if data == block_with_invalid_extra_data ); @@ -76,7 +76,7 @@ fn payload_validation() { b }); assert_matches!( - TryInto::::try_into(block_with_zero_base_fee), + block_with_zero_base_fee.try_into_sealed_block(None), Err(PayloadError::BaseFee(val)) if val == U256::ZERO ); @@ -86,7 +86,7 @@ fn payload_validation() { *tx = Bytes::new().into(); }); assert_matches!( - TryInto::::try_into(payload_with_invalid_txs), + payload_with_invalid_txs.try_into_sealed_block(None), Err(PayloadError::Decode(DecodeError::InputTooShort)) ); @@ -96,7 +96,7 @@ fn payload_validation() { b }); assert_matches!( - TryInto::::try_into(block_with_ommers.clone()), + block_with_ommers.clone().try_into_sealed_block(None), Err(PayloadError::BlockHash { consensus, .. }) if consensus == block_with_ommers.block_hash ); @@ -107,7 +107,7 @@ fn payload_validation() { b }); assert_matches!( - TryInto::::try_into(block_with_difficulty.clone()), + block_with_difficulty.clone().try_into_sealed_block(None), Err(PayloadError::BlockHash { consensus, .. }) if consensus == block_with_difficulty.block_hash ); @@ -117,7 +117,7 @@ fn payload_validation() { b }); assert_matches!( - TryInto::::try_into(block_with_nonce.clone()), + block_with_nonce.clone().try_into_sealed_block(None), Err(PayloadError::BlockHash { consensus, .. }) if consensus == block_with_nonce.block_hash ); diff --git a/crates/rpc/rpc-types/src/eth/block.rs b/crates/rpc/rpc-types/src/eth/block.rs index 5588d55611f6..6db49cb7dee8 100644 --- a/crates/rpc/rpc-types/src/eth/block.rs +++ b/crates/rpc/rpc-types/src/eth/block.rs @@ -131,6 +131,9 @@ pub struct Header { /// Excess blob gas #[serde(rename = "excessBlobGas", skip_serializing_if = "Option::is_none")] pub excess_blob_gas: Option, + /// Parent beacon block root + #[serde(rename = "parentBeaconBlockRoot", skip_serializing_if = "Option::is_none")] + pub parent_beacon_block_root: Option, } // === impl Header === @@ -162,6 +165,7 @@ impl Header { withdrawals_root, blob_gas_used, excess_blob_gas, + parent_beacon_block_root, }, hash, } = primitive_header; @@ -187,6 +191,7 @@ impl Header { base_fee_per_gas: base_fee_per_gas.map(U256::from), blob_gas_used: blob_gas_used.map(U64::from), excess_blob_gas: excess_blob_gas.map(U64::from), + parent_beacon_block_root, } } } @@ -318,6 +323,7 @@ mod tests { base_fee_per_gas: Some(U256::from(20)), blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }, total_difficulty: Some(U256::from(100000)), uncles: vec![H256::from_low_u64_be(17)], @@ -358,6 +364,7 @@ mod tests { base_fee_per_gas: Some(U256::from(20)), blob_gas_used: None, excess_blob_gas: None, + parent_beacon_block_root: None, }, total_difficulty: Some(U256::from(100000)), uncles: vec![H256::from_low_u64_be(17)], diff --git a/crates/rpc/rpc-types/src/eth/engine/payload.rs b/crates/rpc/rpc-types/src/eth/engine/payload.rs index 84751e7f7aa2..38d330ff94ef 100644 --- a/crates/rpc/rpc-types/src/eth/engine/payload.rs +++ b/crates/rpc/rpc-types/src/eth/engine/payload.rs @@ -135,27 +135,28 @@ impl From for ExecutionPayload { } } -/// Try to construct a block from given payload. Perform addition validation of `extra_data` and -/// `base_fee_per_gas` fields. -/// -/// NOTE: The log bloom is assumed to be validated during serialization. -/// NOTE: Empty ommers, nonce and difficulty values are validated upon computing block hash and -/// comparing the value with `payload.block_hash`. -/// -/// See -impl TryFrom for SealedBlock { - type Error = PayloadError; - - fn try_from(payload: ExecutionPayload) -> Result { - if payload.extra_data.len() > MAXIMUM_EXTRA_DATA_SIZE { - return Err(PayloadError::ExtraData(payload.extra_data)) +impl ExecutionPayload { + /// Tries to create a new block from the given payload and optional parent beacon block root. + /// Perform additional validation of `extra_data` and `base_fee_per_gas` fields. + /// + /// NOTE: The log bloom is assumed to be validated during serialization. + /// NOTE: Empty ommers, nonce and difficulty values are validated upon computing block hash and + /// comparing the value with `payload.block_hash`. + /// + /// See + pub fn try_into_sealed_block( + self, + parent_beacon_block_root: Option, + ) -> Result { + if self.extra_data.len() > MAXIMUM_EXTRA_DATA_SIZE { + return Err(PayloadError::ExtraData(self.extra_data)) } - if payload.base_fee_per_gas < MIN_PROTOCOL_BASE_FEE_U256 { - return Err(PayloadError::BaseFee(payload.base_fee_per_gas)) + if self.base_fee_per_gas < MIN_PROTOCOL_BASE_FEE_U256 { + return Err(PayloadError::BaseFee(self.base_fee_per_gas)) } - let transactions = payload + let transactions = self .transactions .iter() .map(|tx| TransactionSigned::decode(&mut tx.as_ref())) @@ -163,32 +164,30 @@ impl TryFrom for SealedBlock { let transactions_root = proofs::calculate_transaction_root(&transactions); let withdrawals_root = - payload.withdrawals.as_ref().map(|w| proofs::calculate_withdrawals_root(w)); + self.withdrawals.as_ref().map(|w| proofs::calculate_withdrawals_root(w)); let header = Header { - parent_hash: payload.parent_hash, - beneficiary: payload.fee_recipient, - state_root: payload.state_root, + parent_hash: self.parent_hash, + beneficiary: self.fee_recipient, + state_root: self.state_root, transactions_root, - receipts_root: payload.receipts_root, + receipts_root: self.receipts_root, withdrawals_root, - logs_bloom: payload.logs_bloom, - number: payload.block_number.as_u64(), - gas_limit: payload.gas_limit.as_u64(), - gas_used: payload.gas_used.as_u64(), - timestamp: payload.timestamp.as_u64(), - mix_hash: payload.prev_randao, + parent_beacon_block_root, + logs_bloom: self.logs_bloom, + number: self.block_number.as_u64(), + gas_limit: self.gas_limit.as_u64(), + gas_used: self.gas_used.as_u64(), + timestamp: self.timestamp.as_u64(), + mix_hash: self.prev_randao, base_fee_per_gas: Some( - payload - .base_fee_per_gas + self.base_fee_per_gas .uint_try_to() - .map_err(|_| PayloadError::BaseFee(payload.base_fee_per_gas))?, + .map_err(|_| PayloadError::BaseFee(self.base_fee_per_gas))?, ), - blob_gas_used: payload.blob_gas_used.map(|blob_gas_used| blob_gas_used.as_u64()), - excess_blob_gas: payload - .excess_blob_gas - .map(|excess_blob_gas| excess_blob_gas.as_u64()), - extra_data: payload.extra_data, + blob_gas_used: self.blob_gas_used.map(|blob_gas_used| blob_gas_used.as_u64()), + excess_blob_gas: self.excess_blob_gas.map(|excess_blob_gas| excess_blob_gas.as_u64()), + extra_data: self.extra_data, // Defaults ommers_hash: EMPTY_LIST_HASH, difficulty: Default::default(), @@ -196,17 +195,17 @@ impl TryFrom for SealedBlock { } .seal_slow(); - if payload.block_hash != header.hash() { + if self.block_hash != header.hash() { return Err(PayloadError::BlockHash { execution: header.hash(), - consensus: payload.block_hash, + consensus: self.block_hash, }) } Ok(SealedBlock { header, body: transactions, - withdrawals: payload.withdrawals, + withdrawals: self.withdrawals, ommers: Default::default(), }) } diff --git a/crates/rpc/rpc/src/eth/api/pending_block.rs b/crates/rpc/rpc/src/eth/api/pending_block.rs index 97285309e6e4..3700de63d6f0 100644 --- a/crates/rpc/rpc/src/eth/api/pending_block.rs +++ b/crates/rpc/rpc/src/eth/api/pending_block.rs @@ -145,6 +145,7 @@ impl PendingBlockEnv { blob_gas_used: None, excess_blob_gas: None, extra_data: Default::default(), + parent_beacon_block_root: None, }; // seal the block diff --git a/testing/ef-tests/src/models.rs b/testing/ef-tests/src/models.rs index 7276174b4099..5171f4fe9922 100644 --- a/testing/ef-tests/src/models.rs +++ b/testing/ef-tests/src/models.rs @@ -82,6 +82,8 @@ pub struct Header { pub blob_gas_used: Option, /// Excess blob gas. pub excess_blob_gas: Option, + /// Parent beacon block root. + pub parent_beacon_block_root: Option, } impl From
for SealedHeader { @@ -106,6 +108,7 @@ impl From
for SealedHeader { withdrawals_root: value.withdrawals_root, blob_gas_used: value.blob_gas_used.map(|v| v.0.to::()), excess_blob_gas: value.excess_blob_gas.map(|v| v.0.to::()), + parent_beacon_block_root: value.parent_beacon_block_root, }; header.seal(value.hash) } From ab0ee675a64c8146281e4ef77b73543fccc8f6bd Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Mon, 21 Aug 2023 15:12:48 -0400 Subject: [PATCH 2/2] add parent beacon block root arg to new_payload_v3 call --- crates/rpc/rpc-engine-api/src/engine_api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index 0213ce9cda47..1a6758b86e8d 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -103,7 +103,7 @@ where )?; // TODO: validate versioned hashes and figure out what to do with parent_beacon_block_root - Ok(self.inner.beacon_consensus.new_payload(payload).await?) + Ok(self.inner.beacon_consensus.new_payload(payload, Some(parent_beacon_block_root)).await?) } /// Sends a message to the beacon consensus engine to update the fork choice _without_