From 3ca60d4e379f4e3b6df75fc68bf964df07228429 Mon Sep 17 00:00:00 2001 From: Luca Provini Date: Sat, 27 Apr 2024 20:02:44 +0200 Subject: [PATCH] fix: checking if the eip1559 gas fields are not set on eip2930 check (#635) * fix: checking if the eip1559 gas fields are not set on eip2930 check * fix: some touchup to trim_conflicting --------- Co-authored-by: James --- crates/network/src/ethereum/builder.rs | 31 ++++++++++++++----- .../rpc-types/src/eth/transaction/request.rs | 13 +++++--- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/crates/network/src/ethereum/builder.rs b/crates/network/src/ethereum/builder.rs index 3890e8905cb..9c78f87a764 100644 --- a/crates/network/src/ethereum/builder.rs +++ b/crates/network/src/ethereum/builder.rs @@ -181,11 +181,25 @@ impl TransactionBuilder for TransactionRequest { #[cfg(test)] mod tests { - use alloy_consensus::{BlobTransactionSidecar, TxType, TypedTransaction}; - use alloy_primitives::Address; + use crate::{TransactionBuilder, TransactionBuilderError}; + use alloy_consensus::{BlobTransactionSidecar, TxEip1559, TxType, TypedTransaction}; + use alloy_primitives::{Address, TxKind}; use alloy_rpc_types::{AccessList, TransactionRequest}; - use crate::{TransactionBuilder, TransactionBuilderError}; + #[test] + fn from_eip1559_to_tx_req() { + let tx = TxEip1559 { + chain_id: 1, + nonce: 0, + gas_limit: 21_000, + to: TxKind::Call(Address::ZERO), + max_priority_fee_per_gas: 20e9 as u128, + max_fee_per_gas: 20e9 as u128, + ..Default::default() + }; + let tx_req: TransactionRequest = tx.into(); + tx_req.build_unsigned().unwrap(); + } #[test] fn test_4844_when_sidecar() { @@ -216,7 +230,7 @@ mod tests { .with_max_priority_fee_per_gas(0) .with_to(Address::ZERO) .with_gas_price(0) - .access_list(AccessList::default()); + .with_access_list(AccessList::default()); let tx = request.build_unsigned().unwrap(); @@ -245,7 +259,7 @@ mod tests { fn test_fail_when_sidecar_and_access_list() { let request = TransactionRequest::default() .with_blob_sidecar(BlobTransactionSidecar::default()) - .access_list(AccessList::default()); + .with_access_list(AccessList::default()); let error = request.clone().build_unsigned().unwrap_err(); @@ -290,7 +304,9 @@ mod tests { #[test] fn test_invalid_2930_fields() { - let request = TransactionRequest::default().access_list(AccessList::default()); + let request = TransactionRequest::default() + .with_access_list(AccessList::default()) + .with_gas_price(Default::default()); let error = request.clone().build_unsigned().unwrap_err(); @@ -299,11 +315,10 @@ mod tests { }; assert_eq!(tx_type, TxType::Eip2930); - assert_eq!(errors.len(), 4); + assert_eq!(errors.len(), 3); assert!(errors.contains(&"to")); assert!(errors.contains(&"nonce")); assert!(errors.contains(&"gas_limit")); - assert!(errors.contains(&"gas_price")); } #[test] diff --git a/crates/rpc-types/src/eth/transaction/request.rs b/crates/rpc-types/src/eth/transaction/request.rs index 72506007a76..23e88780440 100644 --- a/crates/rpc-types/src/eth/transaction/request.rs +++ b/crates/rpc-types/src/eth/transaction/request.rs @@ -340,23 +340,28 @@ impl TransactionRequest { /// submission via rpc. pub fn trim_conflicting_keys(&mut self) { match self.preferred_type() { - TxType::Legacy | TxType::Eip2930 => { + TxType::Legacy => { self.max_fee_per_gas = None; self.max_priority_fee_per_gas = None; self.max_fee_per_blob_gas = None; + self.blob_versioned_hashes = None; + self.sidecar = None; self.access_list = None; + } + TxType::Eip2930 => { + self.max_fee_per_gas = None; + self.max_priority_fee_per_gas = None; + self.max_fee_per_blob_gas = None; self.blob_versioned_hashes = None; self.sidecar = None; } TxType::Eip1559 => { self.gas_price = None; - self.access_list = None; self.blob_versioned_hashes = None; self.sidecar = None; } TxType::Eip4844 => { self.gas_price = None; - self.access_list = None; } } } @@ -371,7 +376,7 @@ impl TransactionRequest { pub const fn preferred_type(&self) -> TxType { if self.sidecar.is_some() || self.max_fee_per_blob_gas.is_some() { TxType::Eip4844 - } else if self.access_list.is_some() { + } else if self.access_list.is_some() && self.gas_price.is_some() { TxType::Eip2930 } else if self.gas_price.is_some() { TxType::Legacy