diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 042c98afe3..beabd898de 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1026,28 +1026,25 @@ 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(); - //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)), - ); - } - - 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(), - }); + // 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)), + ); } + + 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/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 21ca2b2991..bb1f0248c4 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -1,4 +1,5 @@ use crate::engines::ForkchoiceState; +use crate::BlobTxConversionError; pub use ethers_core::types::Transaction; use ethers_core::utils::rlp::{self, Decodable, Rlp}; use http::deposit_methods::RpcError; @@ -40,8 +41,7 @@ pub enum Error { PayloadIdUnavailable, TransitionConfigurationMismatch, PayloadConversionLogicFlaw, - DeserializeTransaction(ssz_types::Error), - DeserializeTransactions(ssz_types::Error), + SszError(ssz_types::Error), DeserializeWithdrawals(ssz_types::Error), BuilderApi(builder_client::Error), IncorrectStateVariant, @@ -49,6 +49,7 @@ pub enum Error { UnsupportedForkVariant(String), BadConversion(String), RlpDecoderError(rlp::DecoderError), + BlobTxConversionError(BlobTxConversionError), } impl From for Error { @@ -88,6 +89,18 @@ impl From for Error { } } +impl From for Error { + fn from(e: ssz_types::Error) -> Self { + Error::SszError(e) + } +} + +impl From for Error { + fn from(e: BlobTxConversionError) -> Self { + Error::BlobTxConversionError(e) + } +} + #[derive(Clone, Copy, Debug, PartialEq, IntoStaticStr)] #[strum(serialize_all = "snake_case")] pub enum PayloadStatusV1Status { diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index a8b60a4fd4..5e574cf0f9 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -13,6 +13,7 @@ pub use engine_api::{http, http::deposit_methods, http::HttpJsonRpc}; use engines::{Engine, EngineError}; pub use engines::{EngineState, ForkchoiceState}; use eth2::types::{builder_bid::SignedBuilderBid, ForkVersionedResponse}; +use ethers_core::types::Transaction as EthersTransaction; use fork_choice::ForkchoiceUpdateParameters; use lru::LruCache; use payload_status::process_payload_status; @@ -21,6 +22,7 @@ use sensitive_url::SensitiveUrl; use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, trace, warn, Logger}; use slot_clock::SlotClock; +use ssz::Encode; use std::collections::HashMap; use std::fmt; use std::future::Future; @@ -35,10 +37,13 @@ use tokio::{ time::sleep, }; use tokio_stream::wrappers::WatchStream; +use types::consts::eip4844::BLOB_TX_TYPE; +use types::transaction::{AccessTuple, BlobTransaction}; use types::{AbstractExecPayload, BeaconStateError, Blob, ExecPayload, KzgCommitment}; use types::{ BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionBlockHash, ForkName, - ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock, Slot, Uint256, + ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock, Slot, Transaction, + Uint256, }; use types::{ ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge, @@ -1612,15 +1617,13 @@ impl ExecutionLayer { return Ok(None); }; - let transactions = VariableList::new( + let transactions = VariableList::from( block .transactions() - .iter() - .map(|transaction| VariableList::new(transaction.rlp().to_vec())) - .collect::>() - .map_err(ApiError::DeserializeTransaction)?, - ) - .map_err(ApiError::DeserializeTransactions)?; + .into_iter() + .map(ethers_tx_to_bytes::) + .collect::, BlobTxConversionError>>()?, + ); let payload = match block { ExecutionBlockWithTransactions::Merge(merge_block) => { @@ -1928,6 +1931,132 @@ async fn timed_future, T>(metric: &str, future: F) -> (T, (result, duration) } +#[derive(Debug)] +pub enum BlobTxConversionError { + /// The transaction type was not set. + NoTransactionType, + /// The transaction chain ID was not set. + NoChainId, + /// The transaction nonce was too large to fit in a `u64`. + NonceTooLarge, + /// The transaction gas was too large to fit in a `u64`. + GasTooHigh, + /// Missing the `max_fee_per_gas` field. + MaxFeePerGasMissing, + /// Missing the `max_priority_fee_per_gas` field. + MaxPriorityFeePerGasMissing, + /// Missing the `access_list` field. + AccessListMissing, + /// Missing the `max_fee_per_data_gas` field. + MaxFeePerDataGasMissing, + /// Missing the `blob_versioned_hashes` field. + BlobVersionedHashesMissing, + /// There was an error converting the transaction to SSZ. + SszError(ssz_types::Error), + /// There was an error converting the transaction from JSON. + SerdeJson(serde_json::Error), +} + +impl From for BlobTxConversionError { + fn from(value: ssz_types::Error) -> Self { + Self::SszError(value) + } +} + +impl From for BlobTxConversionError { + fn from(value: serde_json::Error) -> Self { + Self::SerdeJson(value) + } +} + +/// A utility function to convert a `ethers-rs` `Transaction` into the correct bytes encoding based +/// on transaction type. That means RLP encoding if this is a transaction other than a +/// `BLOB_TX_TYPE` transaction in which case, SSZ encoding will be used. +fn ethers_tx_to_bytes( + transaction: &EthersTransaction, +) -> Result, BlobTxConversionError> { + let tx_type = transaction + .transaction_type + .ok_or(BlobTxConversionError::NoTransactionType)? + .as_u64(); + let tx = if BLOB_TX_TYPE as u64 == tx_type { + let chain_id = transaction + .chain_id + .ok_or(BlobTxConversionError::NoChainId)?; + let nonce = if transaction.nonce > Uint256::from(u64::MAX) { + return Err(BlobTxConversionError::NonceTooLarge); + } else { + transaction.nonce.as_u64() + }; + let max_priority_fee_per_gas = transaction + .max_priority_fee_per_gas + .ok_or(BlobTxConversionError::MaxPriorityFeePerGasMissing)?; + let max_fee_per_gas = transaction + .max_fee_per_gas + .ok_or(BlobTxConversionError::MaxFeePerGasMissing)?; + let gas = if transaction.gas > Uint256::from(u64::MAX) { + return Err(BlobTxConversionError::GasTooHigh); + } else { + transaction.gas.as_u64() + }; + let to = transaction.to; + let value = transaction.value; + let data = VariableList::new(transaction.input.to_vec())?; + let access_list = VariableList::new( + transaction + .access_list + .as_ref() + .ok_or(BlobTxConversionError::AccessListMissing)? + .0 + .iter() + .map(|access_tuple| { + Ok(AccessTuple { + address: access_tuple.address, + storage_keys: VariableList::new(access_tuple.storage_keys.clone())?, + }) + }) + .collect::, BlobTxConversionError>>()?, + )?; + let max_fee_per_data_gas = transaction + .other + .get("maxFeePerDataGas") + .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)? + .as_str() + .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)? + .parse() + .map_err(|_| BlobTxConversionError::MaxFeePerDataGasMissing)?; + let blob_versioned_hashes = serde_json::from_str( + transaction + .other + .get("blobVersionedHashes") + .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)? + .as_str() + .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)?, + )?; + BlobTransaction { + chain_id, + nonce, + max_priority_fee_per_gas, + max_fee_per_gas, + gas, + to, + value, + data, + access_list, + max_fee_per_data_gas, + blob_versioned_hashes, + } + .as_ssz_bytes() + } else { + transaction.rlp().to_vec() + }; + VariableList::new(tx).map_err(Into::into) +} + +fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option> { + None +} + #[cfg(test)] mod test { use super::*; @@ -2075,10 +2204,6 @@ mod test { } } -fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option> { - None -} - #[cfg(test)] /// Returns the duration since the unix epoch. fn timestamp_now() -> u64 { diff --git a/consensus/types/src/kzg_commitment.rs b/consensus/types/src/kzg_commitment.rs deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/consensus/types/src/kzg_proof.rs b/consensus/types/src/kzg_proof.rs deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 91d0dd008f..c09b34f87c 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -101,6 +101,7 @@ pub mod sqlite; pub mod blobs_sidecar; pub mod signed_block_and_blobs; +pub mod transaction; use ethereum_types::{H160, H256}; diff --git a/consensus/types/src/transaction.rs b/consensus/types/src/transaction.rs new file mode 100644 index 0000000000..797ee1dae6 --- /dev/null +++ b/consensus/types/src/transaction.rs @@ -0,0 +1,31 @@ +use crate::{Hash256, Uint256, VersionedHash}; +use ethereum_types::Address; +use ssz_derive::{Decode, Encode}; +use ssz_types::typenum::U16777216; +use ssz_types::VariableList; + +pub type MaxCalldataSize = U16777216; +pub type MaxAccessListSize = U16777216; +pub type MaxVersionedHashesListSize = U16777216; +pub type MaxAccessListStorageKeys = U16777216; + +#[derive(Debug, Clone, PartialEq, Encode, Decode)] +pub struct BlobTransaction { + pub chain_id: Uint256, + pub nonce: u64, + pub max_priority_fee_per_gas: Uint256, + pub max_fee_per_gas: Uint256, + pub gas: u64, + pub to: Option
, + pub value: Uint256, + pub data: VariableList, + pub access_list: VariableList, + pub max_fee_per_data_gas: Uint256, + pub blob_versioned_hashes: VariableList, +} + +#[derive(Debug, Clone, PartialEq, Encode, Decode)] +pub struct AccessTuple { + pub address: Address, + pub storage_keys: VariableList, +}