Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add EIP-4788 parent_beacon_block_root to Header #4299

Merged
merged 2 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/consensus/auto-seal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 7 additions & 1 deletion crates/consensus/beacon/src/engine/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -34,9 +35,14 @@ impl BeaconConsensusEngineHandle {
pub async fn new_payload(
&self,
payload: ExecutionPayload,
parent_beacon_block_root: Option<H256>,
) -> Result<PayloadStatus, BeaconOnNewPayloadError> {
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)?
}

Expand Down
3 changes: 3 additions & 0 deletions crates/consensus/beacon/src/engine/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<H256>,
Comment on lines 149 to +151
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that we could introduce a new wrapper type here, but I actually prefer having a Some|None argument in all the on_new_payload functions so it is evident when we're using it

/// The sender for returning payload status result.
tx: oneshot::Sender<Result<PayloadStatus, BeaconOnNewPayloadError>>,
},
Expand Down
30 changes: 17 additions & 13 deletions crates/consensus/beacon/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<H256>,
) -> Result<PayloadStatus, BeaconOnNewPayloadError> {
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),
};
Expand Down Expand Up @@ -1118,9 +1119,10 @@ where
fn ensure_well_formed_payload(
&self,
payload: ExecutionPayload,
parent_beacon_block_root: Option<H256>,
) -> Result<SealedBlock, PayloadStatus> {
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");
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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 }
Expand Down
6 changes: 4 additions & 2 deletions crates/consensus/beacon/src/engine/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,20 @@ impl<DB> TestEnv<DB> {
pub async fn send_new_payload(
&self,
payload: ExecutionPayload,
parent_beacon_block_root: Option<H256>,
) -> Result<PayloadStatus, BeaconOnNewPayloadError> {
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
/// is syncing.
pub async fn send_new_payload_retry_on_syncing(
&self,
payload: ExecutionPayload,
parent_beacon_block_root: Option<H256>,
) -> Result<PayloadStatus, BeaconOnNewPayloadError> {
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)
}
Expand Down
8 changes: 8 additions & 0 deletions crates/consensus/common/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down Expand Up @@ -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> {
Expand All @@ -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,
Expand Down Expand Up @@ -633,6 +640,7 @@ mod tests {
withdrawals_root: None,
blob_gas_used: None,
excess_blob_gas: None,
parent_beacon_block_root: None,
};
// size: 0x9b5

Expand Down
4 changes: 4 additions & 0 deletions crates/interfaces/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions crates/net/eth-wire/src/types/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -293,6 +294,7 @@ mod test {
withdrawals_root: None,
blob_gas_used: None,
excess_blob_gas: None,
parent_beacon_block_root: None,
},
]),
};
Expand Down Expand Up @@ -407,6 +409,7 @@ mod test {
withdrawals_root: None,
blob_gas_used: None,
excess_blob_gas: None,
parent_beacon_block_root: None,
},
],
withdrawals: None,
Expand Down Expand Up @@ -493,6 +496,7 @@ mod test {
withdrawals_root: None,
blob_gas_used: None,
excess_blob_gas: None,
parent_beacon_block_root: None,
},
],
withdrawals: None,
Expand Down
2 changes: 2 additions & 0 deletions crates/payload/basic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 };
Expand Down
Loading
Loading