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

fix: ensure no blob transactions in payloads pre-cancun #4779

Merged
merged 6 commits into from
Sep 26, 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
3 changes: 3 additions & 0 deletions crates/consensus/beacon/src/engine/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ pub enum BeaconOnNewPayloadError {
/// Thrown when the engine task is unavailable/stopped.
#[error("beacon consensus engine task stopped")]
EngineUnavailable,
/// Thrown when a block has blob transactions, but is not after the Cancun fork.
#[error("block has blob transactions, but is not after the Cancun fork")]
PreCancunBlockWithBlobTransactions,
/// An internal error occurred, not necessarily related to the payload.
#[error(transparent)]
Internal(Box<dyn std::error::Error + Send + Sync>),
Expand Down
44 changes: 36 additions & 8 deletions crates/consensus/beacon/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use reth_interfaces::{
use reth_payload_builder::{PayloadBuilderAttributes, PayloadBuilderHandle};
use reth_primitives::{
constants::EPOCH_SLOTS, listener::EventListeners, stage::StageId, BlockNumHash, BlockNumber,
Head, Header, SealedBlock, SealedHeader, H256, U256,
ChainSpec, Head, Header, SealedBlock, SealedHeader, H256, U256,
};
use reth_provider::{
BlockIdReader, BlockReader, BlockSource, CanonChainTracker, ChainSpecProvider, ProviderError,
Expand All @@ -33,7 +33,7 @@ use reth_rpc_types::engine::{
CancunPayloadFields, ExecutionPayload, PayloadAttributes, PayloadError, PayloadStatus,
PayloadStatusEnum, PayloadValidationError,
};
use reth_rpc_types_compat::engine::payload::try_into_sealed_block;
use reth_rpc_types_compat::payload::{try_into_block, validate_block_hash};
use reth_stages::{ControlFlow, Pipeline, PipelineError};
use reth_tasks::TaskSpawner;
use std::{
Expand Down Expand Up @@ -1079,7 +1079,7 @@ where
payload: ExecutionPayload,
cancun_fields: Option<CancunPayloadFields>,
) -> Result<PayloadStatus, BeaconOnNewPayloadError> {
let block = match self.ensure_well_formed_payload(payload, cancun_fields) {
let block = match self.ensure_well_formed_payload(payload, cancun_fields)? {
Ok(block) => block,
Err(status) => return Ok(status),
};
Expand Down Expand Up @@ -1143,16 +1143,35 @@ where
/// - incorrect hash
/// - the versioned hashes passed with the payload do not exactly match transaction
/// versioned hashes
/// - the block does not contain blob transactions if it is pre-cancun
fn ensure_well_formed_payload(
&self,
payload: ExecutionPayload,
cancun_fields: Option<CancunPayloadFields>,
) -> Result<SealedBlock, PayloadStatus> {
) -> Result<Result<SealedBlock, PayloadStatus>, BeaconOnNewPayloadError> {
let parent_hash = payload.parent_hash();
let block = match try_into_sealed_block(

let block_hash = payload.block_hash();
let block_res = match try_into_block(
payload,
cancun_fields.as_ref().map(|fields| fields.parent_beacon_block_root),
) {
Ok(block) => {
// make sure there are no blob transactions in the payload if it is pre-cancun
// we perform this check before validating the block hash because INVALID_PARAMS
// must be returned over an INVALID response.
if !self.chain_spec().is_cancun_active_at_timestamp(block.timestamp) &&
block.has_blob_transactions()
{
return Err(BeaconOnNewPayloadError::PreCancunBlockWithBlobTransactions)
}

validate_block_hash(block_hash, block)
}
Err(error) => Err(error),
};

let block = match block_res {
Ok(block) => block,
Err(error) => {
error!(target: "consensus::engine", ?error, "Invalid payload");
Expand All @@ -1166,7 +1185,7 @@ where
}
let status = PayloadStatusEnum::from(error);

return Err(PayloadStatus::new(status, latest_valid_hash))
return Ok(Err(PayloadStatus::new(status, latest_valid_hash)))
}
};

Expand All @@ -1177,9 +1196,18 @@ where
.flatten()
.collect::<Vec<_>>();

self.validate_versioned_hashes(parent_hash, block_versioned_hashes, cancun_fields)?;
if let Err(status) =
self.validate_versioned_hashes(parent_hash, block_versioned_hashes, cancun_fields)
{
return Ok(Err(status))
}

Ok(Ok(block))
}

Ok(block)
/// Returns the currently configured [ChainSpec].
fn chain_spec(&self) -> Arc<ChainSpec> {
self.blockchain.chain_spec()
}

/// Validates that the versioned hashes in the block match the versioned hashes passed in the
Expand Down
5 changes: 5 additions & 0 deletions crates/primitives/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ impl Block {
BlockWithSenders { block: self, senders }
}

/// Returns whether or not the block contains any blob transactions.
pub fn has_blob_transactions(&self) -> bool {
self.body.iter().any(|tx| tx.is_eip4844())
}

/// Calculates a heuristic for the in-memory size of the [Block].
#[inline]
pub fn size(&self) -> usize {
Expand Down
1 change: 1 addition & 0 deletions crates/rpc/rpc-engine-api/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ impl From<EngineApiError> for jsonrpsee_types::error::ErrorObject<'static> {
},
EngineApiError::NewPayload(ref err) => match err {
BeaconOnNewPayloadError::Internal(_) |
BeaconOnNewPayloadError::PreCancunBlockWithBlobTransactions => INVALID_PARAMS_CODE,
BeaconOnNewPayloadError::EngineUnavailable => INTERNAL_ERROR_CODE,
},
// Any other server error
Expand Down
56 changes: 45 additions & 11 deletions crates/rpc/rpc-types-compat/src/engine/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,19 +257,17 @@ pub fn convert_block_to_payload_input_v2(value: SealedBlock) -> 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.
/// Tries to create a new block (without a block hash) from the given payload and optional parent
/// beacon block root.
/// Performs 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 <https://github.com/ethereum/go-ethereum/blob/79a478bb6176425c2400e949890e668a3d9a3d05/core/beacon/types.go#L145>
pub fn try_into_sealed_block(
pub fn try_into_block(
value: ExecutionPayload,
parent_beacon_block_root: Option<H256>,
) -> Result<SealedBlock, PayloadError> {
let block_hash = value.block_hash();
) -> Result<Block, PayloadError> {
let mut base_payload = match value {
ExecutionPayload::V1(payload) => try_payload_v1_to_block(payload)?,
ExecutionPayload::V2(payload) => try_payload_v2_to_block(payload)?,
Expand All @@ -278,12 +276,48 @@ pub fn try_into_sealed_block(

base_payload.header.parent_beacon_block_root = parent_beacon_block_root;

let payload = base_payload.seal_slow();
Ok(base_payload)
}

/// Tries to create a new block from the given payload and optional parent beacon block root.
///
/// NOTE: Empty ommers, nonce and difficulty values are validated upon computing block hash and
/// comparing the value with `payload.block_hash`.
///
/// Uses [try_into_block] to convert from the [ExecutionPayload] to [Block] and seals the block
/// with its hash.
///
/// Uses [validate_block_hash] to validate the payload block hash and ultimately return the
/// [SealedBlock].
pub fn try_into_sealed_block(
payload: ExecutionPayload,
parent_beacon_block_root: Option<H256>,
) -> Result<SealedBlock, PayloadError> {
let block_hash = payload.block_hash();
let base_payload = try_into_block(payload, parent_beacon_block_root)?;

// validate block hash and return
validate_block_hash(block_hash, base_payload)
}

if block_hash != payload.hash() {
return Err(PayloadError::BlockHash { execution: payload.hash(), consensus: block_hash })
/// Takes the expected block hash and [Block], validating the block and converting it into a
/// [SealedBlock].
///
/// If the provided block hash does not match the block hash computed from the provided block, this
/// returns [PayloadError::BlockHash].
pub fn validate_block_hash(
expected_block_hash: H256,
block: Block,
) -> Result<SealedBlock, PayloadError> {
let sealed_block = block.seal_slow();
if expected_block_hash != sealed_block.hash() {
return Err(PayloadError::BlockHash {
execution: sealed_block.hash(),
consensus: expected_block_hash,
})
}
Ok(payload)

Ok(sealed_block)
}

/// Converts [Withdrawal] to [reth_rpc_types::engine::payload::Withdrawal]
Expand Down