Merge pull request #3951 from realbigsean/fix-blob-tx-ssz

Encode blob transactions as signed blob transactions

beacon_node/execution_layer/src/lib.rs

@@ -14,7 +14,9 @@ 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 ethers_core::abi::ethereum_types::FromStrRadixErr;
+use ethers_core::types::transaction::eip2930::AccessListItem;
+use ethers_core::types::{Transaction as EthersTransaction, U64};
 use fork_choice::ForkchoiceUpdateParameters;
 use lru::LruCache;
 use payload_status::process_payload_status;
@@ -39,8 +41,10 @@ use tokio::{
 };
 use tokio_stream::wrappers::WatchStream;
 use types::consts::eip4844::BLOB_TX_TYPE;
-use types::transaction::{AccessTuple, BlobTransaction};
+use types::transaction::{AccessTuple, BlobTransaction, EcdsaSignature, SignedBlobTransaction};
-use types::{AbstractExecPayload, BeaconStateError, Blob, ExecPayload, KzgCommitment};
+use types::{
+    AbstractExecPayload, BeaconStateError, Blob, ExecPayload, KzgCommitment, VersionedHash,
+};
 use types::{
     BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionBlockHash, ForkName,
     ProposerPreparationData, PublicKeyBytes, Signature, SignedBeaconBlock, Slot, Transaction,
@@ -1687,13 +1691,15 @@ impl<T: EthSpec> ExecutionLayer<T> {
             return Ok(None);
         };
 
-        let transactions = VariableList::from(
+        let convert_transactions = |transactions: Vec<EthersTransaction>| {
-            block
+            VariableList::new(
-                .transactions()
+                transactions
                     .into_iter()
                     .map(ethers_tx_to_bytes::<T>)
                     .collect::<Result<Vec<_>, BlobTxConversionError>>()?,
-        );
+            )
+            .map_err(BlobTxConversionError::SszError)
+        };
 
         let payload = match block {
             ExecutionBlockWithTransactions::Merge(merge_block) => {
@@ -1711,7 +1717,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                     extra_data: merge_block.extra_data,
                     base_fee_per_gas: merge_block.base_fee_per_gas,
                     block_hash: merge_block.block_hash,
-                    transactions,
+                    transactions: convert_transactions(merge_block.transactions)?,
                 })
             }
             ExecutionBlockWithTransactions::Capella(capella_block) => {
@@ -1737,7 +1743,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                     extra_data: capella_block.extra_data,
                     base_fee_per_gas: capella_block.base_fee_per_gas,
                     block_hash: capella_block.block_hash,
-                    transactions,
+                    transactions: convert_transactions(capella_block.transactions)?,
                     withdrawals,
                 })
             }
@@ -1765,7 +1771,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                     base_fee_per_gas: eip4844_block.base_fee_per_gas,
                     excess_data_gas: eip4844_block.excess_data_gas,
                     block_hash: eip4844_block.block_hash,
-                    transactions,
+                    transactions: convert_transactions(eip4844_block.transactions)?,
                     withdrawals,
                 })
             }
@@ -2030,10 +2036,18 @@ pub enum BlobTxConversionError {
     MaxFeePerDataGasMissing,
     /// Missing the `blob_versioned_hashes` field.
     BlobVersionedHashesMissing,
+    /// `y_parity` field was greater than one.
+    InvalidYParity,
     /// 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),
+    /// There was an error converting the transaction from hex.
+    FromHex(String),
+    /// There was an error converting the transaction from hex.
+    FromStrRadix(FromStrRadixErr),
+    /// A `versioned_hash` did not contain 32 bytes.
+    InvalidVersionedHashBytesLen,
 }
 
 impl From<ssz_types::Error> for BlobTxConversionError {
@@ -2052,67 +2066,123 @@ impl From<serde_json::Error> for BlobTxConversionError {
 /// 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: &EthersTransaction,
+    transaction: EthersTransaction,
 ) -> Result<Transaction<T::MaxBytesPerTransaction>, 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
+        let EthersTransaction {
-            .chain_id
+            hash: _,
-            .ok_or(BlobTxConversionError::NoChainId)?;
+            nonce,
-        let nonce = if transaction.nonce > Uint256::from(u64::MAX) {
+            block_hash: _,
+            block_number: _,
+            transaction_index: _,
+            from: _,
+            to,
+            value,
+            gas_price: _,
+            gas,
+            input,
+            v,
+            r,
+            s,
+            transaction_type: _,
+            access_list,
+            max_priority_fee_per_gas,
+            max_fee_per_gas,
+            chain_id,
+            other,
+        } = transaction;
+
+        // ******************** BlobTransaction fields ********************
+
+        // chainId
+        let chain_id = chain_id.ok_or(BlobTxConversionError::NoChainId)?;
+
+        // nonce
+        let nonce = if nonce > Uint256::from(u64::MAX) {
             return Err(BlobTxConversionError::NonceTooLarge);
         } else {
-            transaction.nonce.as_u64()
+            nonce.as_u64()
         };
-        let max_priority_fee_per_gas = transaction
+
-            .max_priority_fee_per_gas
+        // maxPriorityFeePerGas
-            .ok_or(BlobTxConversionError::MaxPriorityFeePerGasMissing)?;
+        let max_priority_fee_per_gas =
-        let max_fee_per_gas = transaction
+            max_priority_fee_per_gas.ok_or(BlobTxConversionError::MaxPriorityFeePerGasMissing)?;
-            .max_fee_per_gas
+
-            .ok_or(BlobTxConversionError::MaxFeePerGasMissing)?;
+        // maxFeePerGas
-        let gas = if transaction.gas > Uint256::from(u64::MAX) {
+        let max_fee_per_gas = max_fee_per_gas.ok_or(BlobTxConversionError::MaxFeePerGasMissing)?;
+
+        // gas
+        let gas = if gas > Uint256::from(u64::MAX) {
             return Err(BlobTxConversionError::GasTooHigh);
         } else {
-            transaction.gas.as_u64()
+            gas.as_u64()
         };
-        let to = transaction.to;
+
-        let value = transaction.value;
+        // data (a.k.a input)
-        let data = VariableList::new(transaction.input.to_vec())?;
+        let data = VariableList::new(input.to_vec())?;
+
+        // accessList
         let access_list = VariableList::new(
-            transaction
+            access_list
-                .access_list
-                .as_ref()
                 .ok_or(BlobTxConversionError::AccessListMissing)?
                 .0
-                .iter()
+                .into_iter()
                 .map(|access_tuple| {
+                    let AccessListItem {
+                        address,
+                        storage_keys,
+                    } = access_tuple;
                     Ok(AccessTuple {
-                        address: access_tuple.address,
+                        address,
-                        storage_keys: VariableList::new(access_tuple.storage_keys.clone())?,
+                        storage_keys: VariableList::new(storage_keys)?,
                     })
                 })
                 .collect::<Result<Vec<AccessTuple>, BlobTxConversionError>>()?,
         )?;
-        let max_fee_per_data_gas = transaction
+
-            .other
+        // ******************** BlobTransaction `other` fields ********************
+        //
+        // Here we use the `other` field in the `ethers-rs` `Transaction` type because
+        // `ethers-rs` does not yet support SSZ and therefore the blobs transaction type.
+
+        // maxFeePerDataGas
+        let max_fee_per_data_gas = Uint256::from_str_radix(
+            other
                 .get("maxFeePerDataGas")
                 .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)?
                 .as_str()
-            .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)?
+                .ok_or(BlobTxConversionError::MaxFeePerDataGasMissing)?,
-            .parse()
+            16,
-            .map_err(|_| BlobTxConversionError::MaxFeePerDataGasMissing)?;
+        )
-        let blob_versioned_hashes = serde_json::from_str(
+        .map_err(BlobTxConversionError::FromStrRadix)?;
-            transaction
+
-                .other
+        // blobVersionedHashes
+        let blob_versioned_hashes = other
             .get("blobVersionedHashes")
             .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)?
+            .as_array()
+            .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)?
+            .into_iter()
+            .map(|versioned_hash| {
+                let hash_bytes = eth2_serde_utils::hex::decode(
+                    versioned_hash
                         .as_str()
                         .ok_or(BlobTxConversionError::BlobVersionedHashesMissing)?,
-        )?;
+                )
-        BlobTransaction {
+                .map_err(BlobTxConversionError::FromHex)?;
+                if hash_bytes.len() != Hash256::ssz_fixed_len() {
+                    Err(BlobTxConversionError::InvalidVersionedHashBytesLen)
+                } else {
+                    Ok(Hash256::from_slice(&hash_bytes))
+                }
+            })
+            .collect::<Result<Vec<VersionedHash>, BlobTxConversionError>>()?;
+        let message = BlobTransaction {
             chain_id,
             nonce,
             max_priority_fee_per_gas,
@@ -2123,9 +2193,24 @@ fn ethers_tx_to_bytes<T: EthSpec>(
             data,
             access_list,
             max_fee_per_data_gas,
-            blob_versioned_hashes,
+            blob_versioned_hashes: VariableList::new(blob_versioned_hashes)?,
-        }
+        };
-        .as_ssz_bytes()
+
+        // ******************** EcdsaSignature fields ********************
+
+        let y_parity = if v == U64::zero() {
+            false
+        } else if v == U64::one() {
+            true
+        } else {
+            return Err(BlobTxConversionError::InvalidYParity);
+        };
+        let signature = EcdsaSignature { y_parity, r, s };
+
+        // The `BLOB_TX_TYPE` should prepend the SSZ encoded `SignedBlobTransaction`.
+        let mut signed_tx = SignedBlobTransaction { message, signature }.as_ssz_bytes();
+        signed_tx.insert(0, BLOB_TX_TYPE);
+        signed_tx
     } else {
         transaction.rlp().to_vec()
     };

consensus/types/src/transaction.rs

@@ -9,6 +9,12 @@ pub type MaxAccessListSize = U16777216;
 pub type MaxVersionedHashesListSize = U16777216;
 pub type MaxAccessListStorageKeys = U16777216;
 
+#[derive(Debug, Clone, PartialEq, Encode, Decode)]
+pub struct SignedBlobTransaction {
+    pub message: BlobTransaction,
+    pub signature: EcdsaSignature,
+}
+
 #[derive(Debug, Clone, PartialEq, Encode, Decode)]
 pub struct BlobTransaction {
     pub chain_id: Uint256,
@@ -29,3 +35,10 @@ pub struct AccessTuple {
     pub address: Address,
     pub storage_keys: VariableList<Hash256, MaxAccessListStorageKeys>,
 }
+
+#[derive(Debug, Clone, PartialEq, Encode, Decode)]
+pub struct EcdsaSignature {
+    pub y_parity: bool,
+    pub r: Uint256,
+    pub s: Uint256,
+}