improve error handling

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<reqwest::Error> for Error {
@@ -94,6 +95,12 @@ impl From<ssz_types::Error> for Error {
     }
 }
 
+impl From<BlobTxConversionError> for Error {
+    fn from(e: BlobTxConversionError) -> Self {
+        Error::BlobTxConversionError(e)
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, IntoStaticStr)]
 #[strum(serialize_all = "snake_case")]
 pub enum PayloadStatusV1Status {

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,
+    ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock, Slot, Transaction,
-    Transaction as SszTransaction, Uint256,
+    Uint256,
 };
 use types::{
     ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge,
@@ -1621,7 +1622,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                 .transactions()
                 .into_iter()
                 .map(ethers_tx_to_bytes::<T>)
-                .collect::<Result<Vec<_>, ApiError>>()?,
+                .collect::<Result<Vec<_>, BlobTxConversionError>>()?,
         );
 
         let payload = match block {
@@ -2081,34 +2082,82 @@ fn noop<T: EthSpec>(_: &ExecutionLayer<T>, _: &ExecutionPayload<T>) -> Option<Ex
     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<ssz_types::Error> for BlobTxConversionError {
+    fn from(value: ssz_types::Error) -> Self {
+        Self::SszError(value)
+    }
+}
+
+impl From<serde_json::Error> 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<T: EthSpec>(
-    transaction: &Transaction,
+    transaction: &EthersTransaction,
-) -> Result<SszTransaction<T::MaxBytesPerTransaction>, ApiError> {
+) -> Result<Transaction<T::MaxBytesPerTransaction>, 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)?;
+            .ok_or(BlobTxConversionError::NoChainId)?;
-        let nonce = std::cmp::min(transaction.nonce, Uint256::from(u64::MAX)).as_u64();
+        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)?;
+            .ok_or(BlobTxConversionError::MaxFeePerGasMissing)?;
-        let gas = std::cmp::min(transaction.gas, Uint256::from(u64::MAX)).as_u64();
+        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<T: EthSpec>(
                         storage_keys: VariableList::new(access_tuple.storage_keys.clone())?,
                     })
                 })
-                .collect::<Result<Vec<AccessTuple>, ApiError>>()?,
+                .collect::<Result<Vec<AccessTuple>, 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,