From d9e83e6cec60e38c2b0d7160e7d5a464549c1197 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 3 Feb 2023 17:06:33 -0500 Subject: [PATCH 1/8] blob decoding --- beacon_node/execution_layer/src/engine_api.rs | 1 + beacon_node/execution_layer/src/lib.rs | 58 ++++++++++++++++++- consensus/types/src/kzg_commitment.rs | 0 consensus/types/src/kzg_proof.rs | 0 consensus/types/src/lib.rs | 1 + consensus/types/src/transaction.rs | 38 ++++++++++++ 6 files changed, 97 insertions(+), 1 deletion(-) delete mode 100644 consensus/types/src/kzg_commitment.rs delete mode 100644 consensus/types/src/kzg_proof.rs create mode 100644 consensus/types/src/transaction.rs diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 21ca2b2991..8419de588e 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -49,6 +49,7 @@ pub enum Error { UnsupportedForkVariant(String), BadConversion(String), RlpDecoderError(rlp::DecoderError), + BlobTxConversionError, } impl From for Error { diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index a8b60a4fd4..c670b14c98 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -22,12 +22,14 @@ use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, trace, warn, Logger}; use slot_clock::SlotClock; use std::collections::HashMap; +use std::default::default; use std::fmt; use std::future::Future; use std::io::Write; use std::path::PathBuf; use std::sync::Arc; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use ethers_core::types::transaction::eip2930::AccessListItem; use strum::AsRefStr; use task_executor::TaskExecutor; use tokio::{ @@ -35,6 +37,8 @@ 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, @@ -1616,7 +1620,59 @@ impl ExecutionLayer { block .transactions() .iter() - .map(|transaction| VariableList::new(transaction.rlp().to_vec())) + .map(|transaction: Transaction| { + let tx_type = transaction + .transaction_type + .ok_or(ApiError::BlobTxConversionError)?.as_u64(); + let tx = if BLOB_TX_TYPE as u64 == tx_type { + let chain_id = transaction + .chain_id + .ok_or(ApiError::BlobTxConversionError)?; + let nonce = transaction.nonce.as_u64(); + let max_priority_fee_per_gas = transaction + .max_priority_fee_per_gas + .ok_or(ApiError::BlobTxConversionError)?; + let max_fee_per_gas = transaction + .max_fee_per_gas + .ok_or(ApiError::BlobTxConversionError)?; + let gas = transaction.gas.as_u64(); + let to = transaction.to; + let value = transaction.value; + let data = VariableList::from(transaction.input.to_vec()); + let access_list = VariableList::from(transaction + .access_list + .ok_or(ApiError::BlobTxConversionError)? + .0 + .into_iter().map(|access_tuple| Ok(AccessTuple { + address: access_tuple.address, + storage_keys: VariableList::from( + access_tuple + .storage_keys, + ), + })) + .collect::, ApiError>>()?); + let max_fee_per_data_gas = transaction.other.get("max_fee_per_data_gas").ok_or(ApiError::BlobTxConversionError)?; + let data_str = max_fee_per_data_gas.as_str().ok_or(ApiError::BlobTxConversionError)?; + let blob_versioned_hashes = transaction.other.get("blob_versioned_hashes").ok_or(ApiError::BlobTxConversionError)?; + Ok(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, + }); + vec![] + } else { + transaction.rlp().to_vec() + }; + VariableList::new(tx) + }) .collect::>() .map_err(ApiError::DeserializeTransaction)?, ) 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..d1c3a797d2 --- /dev/null +++ b/consensus/types/src/transaction.rs @@ -0,0 +1,38 @@ +use crate::{Hash256, Transaction, 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, +} + +#[derive(Debug, Clone, PartialEq, Encode, Decode)] +pub struct EcdsaSignature { + y_parity: bool, + r: Uint256, + s: Uint256, +} From 94a369b9df99107370ab570e858104e474b39d8d Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 16:39:15 -0500 Subject: [PATCH 2/8] blob tx decoding --- beacon_node/execution_layer/src/lib.rs | 144 ++++++++++++++----------- consensus/types/src/transaction.rs | 2 +- 2 files changed, 83 insertions(+), 63 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index c670b14c98..85ed0e25ae 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -21,15 +21,14 @@ 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::default::default; use std::fmt; use std::future::Future; use std::io::Write; use std::path::PathBuf; use std::sync::Arc; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; -use ethers_core::types::transaction::eip2930::AccessListItem; use strum::AsRefStr; use task_executor::TaskExecutor; use tokio::{ @@ -42,7 +41,8 @@ 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 as SszTransaction, Uint256, }; use types::{ ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge, @@ -1616,67 +1616,13 @@ impl ExecutionLayer { return Ok(None); }; - let transactions = VariableList::new( + let transactions = VariableList::from( block .transactions() - .iter() - .map(|transaction: Transaction| { - let tx_type = transaction - .transaction_type - .ok_or(ApiError::BlobTxConversionError)?.as_u64(); - let tx = if BLOB_TX_TYPE as u64 == tx_type { - let chain_id = transaction - .chain_id - .ok_or(ApiError::BlobTxConversionError)?; - let nonce = transaction.nonce.as_u64(); - let max_priority_fee_per_gas = transaction - .max_priority_fee_per_gas - .ok_or(ApiError::BlobTxConversionError)?; - let max_fee_per_gas = transaction - .max_fee_per_gas - .ok_or(ApiError::BlobTxConversionError)?; - let gas = transaction.gas.as_u64(); - let to = transaction.to; - let value = transaction.value; - let data = VariableList::from(transaction.input.to_vec()); - let access_list = VariableList::from(transaction - .access_list - .ok_or(ApiError::BlobTxConversionError)? - .0 - .into_iter().map(|access_tuple| Ok(AccessTuple { - address: access_tuple.address, - storage_keys: VariableList::from( - access_tuple - .storage_keys, - ), - })) - .collect::, ApiError>>()?); - let max_fee_per_data_gas = transaction.other.get("max_fee_per_data_gas").ok_or(ApiError::BlobTxConversionError)?; - let data_str = max_fee_per_data_gas.as_str().ok_or(ApiError::BlobTxConversionError)?; - let blob_versioned_hashes = transaction.other.get("blob_versioned_hashes").ok_or(ApiError::BlobTxConversionError)?; - Ok(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, - }); - vec![] - } else { - transaction.rlp().to_vec() - }; - VariableList::new(tx) - }) - .collect::>() - .map_err(ApiError::DeserializeTransaction)?, - ) - .map_err(ApiError::DeserializeTransactions)?; + .into_iter() + .map(ethers_tx_to_bytes::) + .collect::, ApiError>>()?, + ); let payload = match block { ExecutionBlockWithTransactions::Merge(merge_block) => { @@ -2135,6 +2081,80 @@ fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option( + transaction: &Transaction, +) -> Result, ApiError> { + + let tx_type = transaction + .transaction_type + .ok_or(ApiError::BlobTxConversionError)? + .as_u64(); + let tx = if BLOB_TX_TYPE as u64 == tx_type { + let chain_id = transaction + .chain_id + .ok_or(ApiError::BlobTxConversionError)?; + let nonce = std::cmp::min(transaction.nonce, Uint256::from(u64::MAX)).as_u64(); + let max_priority_fee_per_gas = transaction + .max_priority_fee_per_gas + .ok_or(ApiError::BlobTxConversionError)?; + let max_fee_per_gas = transaction + .max_fee_per_gas + .ok_or(ApiError::BlobTxConversionError)?; + let gas = std::cmp::min(transaction.gas, Uint256::from(u64::MAX)).as_u64(); + let to = transaction.to; + let value = transaction.value; + let data = VariableList::from(transaction.input.to_vec()); + + let access_list = VariableList::from( + transaction + .access_list.as_ref() + .ok_or(ApiError::BlobTxConversionError)? + .0 + .iter() + .map(|access_tuple| { + Ok(AccessTuple { + address: access_tuple.address, + storage_keys: VariableList::from(access_tuple.storage_keys.clone()), + }) + }) + .collect::, ApiError>>()?, + ); + let max_fee_per_data_gas = transaction + .other + .get("max_fee_per_data_gas") + .ok_or(ApiError::BlobTxConversionError)? + .as_str() + .ok_or(ApiError::BlobTxConversionError)? + .parse() + .map_err(|_| ApiError::BlobTxConversionError)?; + let blob_versioned_hashes = serde_json::from_str( + transaction + .other + .get("blob_versioned_hashes") + .ok_or(ApiError::BlobTxConversionError)? + .as_str() + .ok_or(ApiError::BlobTxConversionError)?, + )?; + 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() + }; + Ok(VariableList::from(tx)) +} + #[cfg(test)] /// Returns the duration since the unix epoch. fn timestamp_now() -> u64 { diff --git a/consensus/types/src/transaction.rs b/consensus/types/src/transaction.rs index d1c3a797d2..73dee84ad3 100644 --- a/consensus/types/src/transaction.rs +++ b/consensus/types/src/transaction.rs @@ -1,4 +1,4 @@ -use crate::{Hash256, Transaction, Uint256, VersionedHash}; +use crate::{Hash256, Uint256, VersionedHash}; use ethereum_types::Address; use ssz_derive::{Decode, Encode}; use ssz_types::typenum::U16777216; From 90e25dc6cf6886a3494cd60c76942f10b76f2252 Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 16:55:55 -0500 Subject: [PATCH 3/8] from to new --- beacon_node/execution_layer/src/engine_api.rs | 9 +++++++-- beacon_node/execution_layer/src/lib.rs | 12 ++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 8419de588e..3c931a0cdb 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -40,8 +40,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, @@ -89,6 +88,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: ssz_types::Error) -> Self { + Error::SszError(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 85ed0e25ae..06e5275088 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -2084,7 +2084,6 @@ fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option( transaction: &Transaction, ) -> Result, ApiError> { - let tx_type = transaction .transaction_type .ok_or(ApiError::BlobTxConversionError)? @@ -2103,22 +2102,23 @@ fn ethers_tx_to_bytes( let gas = std::cmp::min(transaction.gas, Uint256::from(u64::MAX)).as_u64(); let to = transaction.to; let value = transaction.value; - let data = VariableList::from(transaction.input.to_vec()); + let data = VariableList::new(transaction.input.to_vec())?; - let access_list = VariableList::from( + let access_list = VariableList::new( transaction - .access_list.as_ref() + .access_list + .as_ref() .ok_or(ApiError::BlobTxConversionError)? .0 .iter() .map(|access_tuple| { Ok(AccessTuple { address: access_tuple.address, - storage_keys: VariableList::from(access_tuple.storage_keys.clone()), + storage_keys: VariableList::new(access_tuple.storage_keys.clone())?, }) }) .collect::, ApiError>>()?, - ); + )?; let max_fee_per_data_gas = transaction .other .get("max_fee_per_data_gas") From f22aac1603341dd6989098865cf9ea429ba8aa12 Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 17:26:36 -0500 Subject: [PATCH 4/8] improve error handling --- beacon_node/execution_layer/src/engine_api.rs | 9 +- beacon_node/execution_layer/src/lib.rs | 87 +++++++++++++++---- 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 3c931a0cdb..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; @@ -48,7 +49,7 @@ pub enum Error { UnsupportedForkVariant(String), BadConversion(String), RlpDecoderError(rlp::DecoderError), - BlobTxConversionError, + BlobTxConversionError(BlobTxConversionError), } impl From for Error { @@ -94,6 +95,12 @@ impl From for Error { } } +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 06e5275088..fa5f264457 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; @@ -41,8 +42,8 @@ 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, - Transaction as SszTransaction, Uint256, + ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock, Slot, Transaction, + Uint256, }; use types::{ ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge, @@ -1621,7 +1622,7 @@ impl ExecutionLayer { .transactions() .into_iter() .map(ethers_tx_to_bytes::) - .collect::, ApiError>>()?, + .collect::, BlobTxConversionError>>()?, ); let payload = match block { @@ -2081,34 +2082,82 @@ fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option 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: &Transaction, -) -> Result, ApiError> { + transaction: &EthersTransaction, +) -> Result, BlobTxConversionError> { let tx_type = transaction .transaction_type - .ok_or(ApiError::BlobTxConversionError)? + .ok_or(BlobTxConversionError::NoTransactionType)? .as_u64(); let tx = if BLOB_TX_TYPE as u64 == tx_type { let chain_id = transaction .chain_id - .ok_or(ApiError::BlobTxConversionError)?; - let nonce = std::cmp::min(transaction.nonce, Uint256::from(u64::MAX)).as_u64(); + .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(ApiError::BlobTxConversionError)?; + .ok_or(BlobTxConversionError::MaxPriorityFeePerGasMissing)?; let max_fee_per_gas = transaction .max_fee_per_gas - .ok_or(ApiError::BlobTxConversionError)?; - let gas = std::cmp::min(transaction.gas, Uint256::from(u64::MAX)).as_u64(); + .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(ApiError::BlobTxConversionError)? + .ok_or(BlobTxConversionError::AccessListMissing)? .0 .iter() .map(|access_tuple| { @@ -2117,23 +2166,23 @@ fn ethers_tx_to_bytes( storage_keys: VariableList::new(access_tuple.storage_keys.clone())?, }) }) - .collect::, ApiError>>()?, + .collect::, BlobTxConversionError>>()?, )?; let max_fee_per_data_gas = transaction .other .get("max_fee_per_data_gas") - .ok_or(ApiError::BlobTxConversionError)? + .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)? .as_str() - .ok_or(ApiError::BlobTxConversionError)? + .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)? .parse() - .map_err(|_| ApiError::BlobTxConversionError)?; + .map_err(|_| BlobTxConversionError::MaxFeePerDataGasMissing)?; let blob_versioned_hashes = serde_json::from_str( transaction .other .get("blob_versioned_hashes") - .ok_or(ApiError::BlobTxConversionError)? + .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)? .as_str() - .ok_or(ApiError::BlobTxConversionError)?, + .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)?, )?; BlobTransaction { chain_id, From 38db8d7952c70f28cf1034c85fb637c0de2326c8 Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 17:31:20 -0500 Subject: [PATCH 5/8] add back in 4844 tx consistencycheck during payload reconstruction --- beacon_node/beacon_chain/src/beacon_chain.rs | 39 +++++++++----------- 1 file changed, 18 insertions(+), 21 deletions(-) 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. From 1315098c15f44fcdac597701208a7a0635cbf1ea Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 17:56:03 -0500 Subject: [PATCH 6/8] variable list from -> new --- beacon_node/execution_layer/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index fa5f264457..902b5beec1 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -2201,7 +2201,7 @@ fn ethers_tx_to_bytes( } else { transaction.rlp().to_vec() }; - Ok(VariableList::from(tx)) + VariableList::new(tx).map_err(Into::into) } #[cfg(test)] From e5896d9b71e515ddc8b0411d8a37c1a30aae17c6 Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 18:14:24 -0500 Subject: [PATCH 7/8] re-order methods --- beacon_node/execution_layer/src/lib.rs | 251 ++++++++++++------------- 1 file changed, 125 insertions(+), 126 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 902b5beec1..64d46d4852 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -1931,6 +1931,131 @@ 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 `max_data_gas` 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("max_fee_per_data_gas") + .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("blob_versioned_hashes") + .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::*; @@ -2078,132 +2203,6 @@ mod test { } } -fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option> { - None -} - -#[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 `max_data_gas` 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("max_fee_per_data_gas") - .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("blob_versioned_hashes") - .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) -} - #[cfg(test)] /// Returns the duration since the unix epoch. fn timestamp_now() -> u64 { From 3533ed418e595a4d30555912080b7d24e0b323ec Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 7 Feb 2023 08:36:35 -0500 Subject: [PATCH 8/8] pr feedback and bugfixes --- beacon_node/execution_layer/src/lib.rs | 9 +++++---- consensus/types/src/transaction.rs | 7 ------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 64d46d4852..5e574cf0f9 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -1949,7 +1949,7 @@ pub enum BlobTxConversionError { AccessListMissing, /// Missing the `max_fee_per_data_gas` field. MaxFeePerDataGasMissing, - /// Missing the `max_data_gas` field. + /// Missing the `blob_versioned_hashes` field. BlobVersionedHashesMissing, /// There was an error converting the transaction to SSZ. SszError(ssz_types::Error), @@ -2019,7 +2019,7 @@ fn ethers_tx_to_bytes( )?; let max_fee_per_data_gas = transaction .other - .get("max_fee_per_data_gas") + .get("maxFeePerDataGas") .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)? .as_str() .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)? @@ -2028,7 +2028,7 @@ fn ethers_tx_to_bytes( let blob_versioned_hashes = serde_json::from_str( transaction .other - .get("blob_versioned_hashes") + .get("blobVersionedHashes") .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)? .as_str() .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)?, @@ -2045,7 +2045,8 @@ fn ethers_tx_to_bytes( access_list, max_fee_per_data_gas, blob_versioned_hashes, - }.as_ssz_bytes() + } + .as_ssz_bytes() } else { transaction.rlp().to_vec() }; diff --git a/consensus/types/src/transaction.rs b/consensus/types/src/transaction.rs index 73dee84ad3..797ee1dae6 100644 --- a/consensus/types/src/transaction.rs +++ b/consensus/types/src/transaction.rs @@ -29,10 +29,3 @@ pub struct AccessTuple { pub address: Address, pub storage_keys: VariableList, } - -#[derive(Debug, Clone, PartialEq, Encode, Decode)] -pub struct EcdsaSignature { - y_parity: bool, - r: Uint256, - s: Uint256, -}