From f22aac1603341dd6989098865cf9ea429ba8aa12 Mon Sep 17 00:00:00 2001 From: sean Date: Sun, 5 Feb 2023 17:26:36 -0500 Subject: [PATCH] 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,