From 2b4eb8438c6b3fffe99682e49d0e8abf6f1cbd8d Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 16 Nov 2023 19:54:07 +0100 Subject: [PATCH] fix: fetch 4844 blob before reinjected reorged blob txs (#5460) --- crates/net/network/src/transactions.rs | 2 +- crates/primitives/src/transaction/mod.rs | 2 +- crates/primitives/src/transaction/pooled.rs | 41 ++++++++++++++++++- crates/rpc/rpc/src/eth/api/transactions.rs | 5 ++- crates/transaction-pool/src/maintain.rs | 29 ++++++++++++- .../transaction-pool/src/test_utils/mock.rs | 2 +- crates/transaction-pool/src/traits.rs | 10 ++--- crates/transaction-pool/src/validate/eth.rs | 5 ++- 8 files changed, 80 insertions(+), 16 deletions(-) diff --git a/crates/net/network/src/transactions.rs b/crates/net/network/src/transactions.rs index 7b35e84689..24f6f8ffb2 100644 --- a/crates/net/network/src/transactions.rs +++ b/crates/net/network/src/transactions.rs @@ -734,7 +734,7 @@ where } Entry::Vacant(entry) => { // this is a new transaction that should be imported into the pool - let pool_transaction = ::from_recovered_transaction(tx); + let pool_transaction = ::from_recovered_pooled_transaction(tx); let pool = self.pool.clone(); diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 968336c873..91cbb3c830 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -1484,7 +1484,7 @@ impl FromRecoveredTransaction for TransactionSignedEcRecovered { #[cfg(feature = "c-kzg")] pub trait FromRecoveredPooledTransaction { /// Converts to this type from the given [`PooledTransactionsElementEcRecovered`]. - fn from_recovered_transaction(tx: PooledTransactionsElementEcRecovered) -> Self; + fn from_recovered_pooled_transaction(tx: PooledTransactionsElementEcRecovered) -> Self; } /// The inverse of [`FromRecoveredTransaction`] that ensure the transaction can be sent over the diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 55302c4975..493c723a83 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -4,8 +4,9 @@ #![cfg_attr(docsrs, doc(cfg(feature = "c-kzg")))] use crate::{ - Address, BlobTransaction, Bytes, Signature, Transaction, TransactionSigned, - TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxHash, TxLegacy, B256, EIP4844_TX_TYPE_ID, + Address, BlobTransaction, BlobTransactionSidecar, Bytes, Signature, Transaction, + TransactionSigned, TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxHash, TxLegacy, B256, + EIP4844_TX_TYPE_ID, }; use alloy_rlp::{Decodable, Encodable, Error as RlpError, Header, EMPTY_LIST_CODE}; use bytes::Buf; @@ -71,6 +72,27 @@ impl PooledTransactionsElement { Ok(tx.into()) } + /// Converts from an EIP-4844 [TransactionSignedEcRecovered] to a + /// [PooledTransactionsElementEcRecovered] with the given sidecar. + /// + /// Returns the transaction is not an EIP-4844 transaction. + pub fn try_from_blob_transaction( + tx: TransactionSigned, + sidecar: BlobTransactionSidecar, + ) -> Result { + let TransactionSigned { transaction, signature, hash } = tx; + if let Transaction::Eip4844(tx) = transaction { + Ok(PooledTransactionsElement::BlobTransaction(BlobTransaction { + transaction: tx, + signature, + hash, + sidecar, + })) + } else { + Err(TransactionSigned { transaction, signature, hash }) + } + } + /// Heavy operation that return signature hash over rlp encoded transaction. /// It is only for signature signing or signer recovery. pub fn signature_hash(&self) -> B256 { @@ -575,6 +597,21 @@ impl PooledTransactionsElementEcRecovered { ) -> Self { Self { transaction, signer } } + + /// Converts from an EIP-4844 [TransactionSignedEcRecovered] to a + /// [PooledTransactionsElementEcRecovered] with the given sidecar. + /// + /// Returns the transaction is not an EIP-4844 transaction. + pub fn try_from_blob_transaction( + tx: TransactionSignedEcRecovered, + sidecar: BlobTransactionSidecar, + ) -> Result { + let TransactionSignedEcRecovered { signer, signed_transaction } = tx; + let transaction = + PooledTransactionsElement::try_from_blob_transaction(signed_transaction, sidecar) + .map_err(|tx| TransactionSignedEcRecovered { signer, signed_transaction: tx })?; + Ok(Self { transaction, signer }) + } } impl From for PooledTransactionsElementEcRecovered { diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index 4989be4219..78307ee333 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -491,7 +491,7 @@ where self.forward_to_sequencer(&tx).await?; let recovered = recover_raw_transaction(tx)?; - let pool_transaction = ::from_recovered_transaction(recovered); + let pool_transaction = ::from_recovered_pooled_transaction(recovered); // submit the transaction to the pool with a `Local` origin let hash = self.pool().add_transaction(TransactionOrigin::Local, pool_transaction).await?; @@ -578,7 +578,8 @@ where let recovered = signed_tx.into_ecrecovered().ok_or(EthApiError::InvalidTransactionSignature)?; - let pool_transaction = ::from_recovered_transaction(recovered.into()); + let pool_transaction = + ::from_recovered_pooled_transaction(recovered.into()); // submit the transaction to the pool with a `Local` origin let hash = self.pool().add_transaction(TransactionOrigin::Local, pool_transaction).await?; diff --git a/crates/transaction-pool/src/maintain.rs b/crates/transaction-pool/src/maintain.rs index 19c3d6d513..692a5177bb 100644 --- a/crates/transaction-pool/src/maintain.rs +++ b/crates/transaction-pool/src/maintain.rs @@ -12,7 +12,8 @@ use futures_util::{ }; use reth_interfaces::RethError; use reth_primitives::{ - Address, BlockHash, BlockNumber, BlockNumberOrTag, FromRecoveredTransaction, + Address, BlockHash, BlockNumber, BlockNumberOrTag, FromRecoveredPooledTransaction, + FromRecoveredTransaction, PooledTransactionsElementEcRecovered, }; use reth_provider::{ BlockReaderIdExt, BundleStateWithReceipts, CanonStateNotification, ChainSpecProvider, @@ -286,7 +287,31 @@ pub async fn maintain_transaction_pool( let pruned_old_transactions = old_blocks .transactions_ecrecovered() .filter(|tx| !new_mined_transactions.contains(&tx.hash)) - .map(

::Transaction::from_recovered_transaction) + .filter_map(|tx| { + if tx.is_eip4844() { + // reorged blobs no longer include the blob, which is necessary for + // validating the transaction. Even though the transaction could have + // been validated previously, we still need the blob in order to + // accurately set the transaction's + // encoded-length which is propagated over the network. + pool.get_blob(tx.hash) + .ok() + .flatten() + .and_then(|sidecar| { + PooledTransactionsElementEcRecovered::try_from_blob_transaction( + tx, sidecar, + ) + .ok() + }) + .map( +

::Transaction::from_recovered_pooled_transaction, + ) + } else { + Some(

::Transaction::from_recovered_transaction( + tx, + )) + } + }) .collect::>(); // update the pool first diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index a59f173ad6..d289597d2b 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -778,7 +778,7 @@ impl FromRecoveredTransaction for MockTransaction { } impl FromRecoveredPooledTransaction for MockTransaction { - fn from_recovered_transaction(tx: PooledTransactionsElementEcRecovered) -> Self { + fn from_recovered_pooled_transaction(tx: PooledTransactionsElementEcRecovered) -> Self { FromRecoveredTransaction::from_recovered_transaction(tx.into_ecrecovered_transaction()) } } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 177f462cff..91455df3d8 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -921,6 +921,10 @@ impl PoolTransaction for EthPooledTransaction { } } + fn access_list(&self) -> Option<&AccessList> { + self.transaction.access_list() + } + /// Returns the EIP-1559 Priority fee the caller is paying to the block author. /// /// This will return `None` for non-EIP1559 transactions @@ -939,10 +943,6 @@ impl PoolTransaction for EthPooledTransaction { self.transaction.max_fee_per_blob_gas() } - fn access_list(&self) -> Option<&AccessList> { - self.transaction.access_list() - } - /// Returns the effective tip for this transaction. /// /// For EIP-1559 transactions: `min(max_fee_per_gas - base_fee, max_priority_fee_per_gas)`. @@ -1029,7 +1029,7 @@ impl FromRecoveredTransaction for EthPooledTransaction { } impl FromRecoveredPooledTransaction for EthPooledTransaction { - fn from_recovered_transaction(tx: PooledTransactionsElementEcRecovered) -> Self { + fn from_recovered_pooled_transaction(tx: PooledTransactionsElementEcRecovered) -> Self { EthPooledTransaction::from(tx) } } diff --git a/crates/transaction-pool/src/validate/eth.rs b/crates/transaction-pool/src/validate/eth.rs index c8ca6891d7..169f17263e 100644 --- a/crates/transaction-pool/src/validate/eth.rs +++ b/crates/transaction-pool/src/validate/eth.rs @@ -771,8 +771,9 @@ mod tests { let data = hex::decode(raw).unwrap(); let tx = PooledTransactionsElement::decode_enveloped(data.into()).unwrap(); - let transaction = - EthPooledTransaction::from_recovered_transaction(tx.try_into_ecrecovered().unwrap()); + let transaction = EthPooledTransaction::from_recovered_pooled_transaction( + tx.try_into_ecrecovered().unwrap(), + ); let res = ensure_intrinsic_gas(&transaction, false); assert!(res.is_ok()); let res = ensure_intrinsic_gas(&transaction, true);