From 3b4edb0a69fc8a8ab068a71c1bd4a1bf2dd836ea Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 29 Nov 2024 10:24:11 +0400 Subject: [PATCH] feat: use generic `SignedTx` in `SenderRecoveryStage` (#12996) --- crates/cli/commands/src/db/get.rs | 15 ++++++------ .../src/transaction/signed.rs | 10 ++++++-- crates/primitives/src/transaction/mod.rs | 5 ++-- crates/primitives/src/transaction/pooled.rs | 21 +++++++++++++--- .../stages/src/stages/sender_recovery.rs | 24 +++++++++++-------- 5 files changed, 50 insertions(+), 25 deletions(-) diff --git a/crates/cli/commands/src/db/get.rs b/crates/cli/commands/src/db/get.rs index 8f9a5f1d32..13b7b70347 100644 --- a/crates/cli/commands/src/db/get.rs +++ b/crates/cli/commands/src/db/get.rs @@ -9,6 +9,7 @@ use reth_db::{ }; use reth_db_api::table::{Decompress, DupSort, Table}; use reth_db_common::DbTool; +use reth_node_api::{ReceiptTy, TxTy}; use reth_node_builder::NodeTypesWithDB; use reth_provider::{providers::ProviderNodeTypes, StaticFileProviderFactory}; use reth_static_file_types::StaticFileSegment; @@ -65,14 +66,12 @@ impl Command { StaticFileSegment::Headers => { (table_key::(&key)?, >::MASK) } - StaticFileSegment::Transactions => ( - table_key::(&key)?, - ::Value>>::MASK, - ), - StaticFileSegment::Receipts => ( - table_key::(&key)?, - ::Value>>::MASK, - ), + StaticFileSegment::Transactions => { + (table_key::(&key)?, >>::MASK) + } + StaticFileSegment::Receipts => { + (table_key::(&key)?, >>::MASK) + } }; let content = tool.provider_factory.static_file_provider().find_static_file( diff --git a/crates/primitives-traits/src/transaction/signed.rs b/crates/primitives-traits/src/transaction/signed.rs index ae9a8f0d2a..5e0a91b4da 100644 --- a/crates/primitives-traits/src/transaction/signed.rs +++ b/crates/primitives-traits/src/transaction/signed.rs @@ -1,7 +1,7 @@ //! API of a signed transaction. use crate::{FillTxEnv, InMemorySize, MaybeArbitrary, MaybeCompact, MaybeSerde, TxType}; -use alloc::fmt; +use alloc::{fmt, vec::Vec}; use alloy_eips::eip2718::{Decodable2718, Encodable2718}; use alloy_primitives::{keccak256, Address, PrimitiveSignature, TxHash, B256}; use core::hash::Hash; @@ -61,7 +61,13 @@ pub trait SignedTransaction: /// /// Returns `None` if the transaction's signature is invalid, see also /// `reth_primitives::transaction::recover_signer_unchecked`. - fn recover_signer_unchecked(&self) -> Option
; + fn recover_signer_unchecked(&self) -> Option
{ + self.recover_signer_unchecked_with_buf(&mut Vec::new()) + } + + /// Same as [`Self::recover_signer_unchecked`] but receives a buffer to operate on. This is used + /// during batch recovery to avoid allocating a new buffer for each transaction. + fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
; /// Calculate transaction hash, eip2728 transaction does not contain rlp header and start with /// tx type. diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index db789d1f6d..e1524aa1dc 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -1245,14 +1245,15 @@ impl SignedTransaction for TransactionSigned { recover_signer(&self.signature, signature_hash) } - fn recover_signer_unchecked(&self) -> Option
{ + fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. #[cfg(feature = "optimism")] if let Transaction::Deposit(TxDeposit { from, .. }) = self.transaction { return Some(from) } - let signature_hash = self.signature_hash(); + self.encode_for_signing(buf); + let signature_hash = keccak256(buf); recover_signer_unchecked(&self.signature, signature_hash) } } diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 145660f44c..979a55f273 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -8,10 +8,11 @@ use super::{ use crate::{ BlobTransaction, Transaction, TransactionSigned, TransactionSignedEcRecovered, TxType, }; +use alloc::vec::Vec; use alloy_consensus::{ constants::EIP4844_TX_TYPE_ID, transaction::{TxEip1559, TxEip2930, TxEip4844, TxLegacy}, - Signed, TxEip4844WithSidecar, + SignableTransaction, Signed, TxEip4844WithSidecar, }; use alloy_eips::{ eip2718::{Decodable2718, Eip2718Result, Encodable2718}, @@ -27,6 +28,7 @@ use bytes::Buf; use core::hash::{Hash, Hasher}; use derive_more::{AsRef, Deref}; use reth_primitives_traits::{InMemorySize, SignedTransaction}; +use revm_primitives::keccak256; use serde::{Deserialize, Serialize}; /// A response to `GetPooledTransactions`. This can include either a blob transaction, or a @@ -153,6 +155,18 @@ impl PooledTransactionsElement { } } + /// This encodes the transaction _without_ the signature, and is only suitable for creating a + /// hash intended for signing. + pub fn encode_for_signing(&self, out: &mut dyn bytes::BufMut) { + match self { + Self::Legacy(tx) => tx.tx().encode_for_signing(out), + Self::Eip2930(tx) => tx.tx().encode_for_signing(out), + Self::Eip1559(tx) => tx.tx().encode_for_signing(out), + Self::BlobTransaction(tx) => tx.tx().encode_for_signing(out), + Self::Eip7702(tx) => tx.tx().encode_for_signing(out), + } + } + /// Create [`TransactionSignedEcRecovered`] by converting this transaction into /// [`TransactionSigned`] and [`Address`] of the signer. pub fn into_ecrecovered_transaction(self, signer: Address) -> TransactionSignedEcRecovered { @@ -600,8 +614,9 @@ impl SignedTransaction for PooledTransactionsElement { recover_signer(self.signature(), signature_hash) } - fn recover_signer_unchecked(&self) -> Option
{ - let signature_hash = self.signature_hash(); + fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ + self.encode_for_signing(buf); + let signature_hash = keccak256(buf); recover_signer_unchecked(self.signature(), signature_hash) } } diff --git a/crates/stages/stages/src/stages/sender_recovery.rs b/crates/stages/stages/src/stages/sender_recovery.rs index a6c2537c18..674d035021 100644 --- a/crates/stages/stages/src/stages/sender_recovery.rs +++ b/crates/stages/stages/src/stages/sender_recovery.rs @@ -1,13 +1,14 @@ use alloy_primitives::{Address, TxNumber}; use reth_config::config::SenderRecoveryConfig; use reth_consensus::ConsensusError; -use reth_db::{static_file::TransactionMask, tables, RawValue}; +use reth_db::{static_file::TransactionMask, table::Value, tables, RawValue}; use reth_db_api::{ cursor::DbCursorRW, transaction::{DbTx, DbTxMut}, DbTxUnwindExt, }; -use reth_primitives::{GotExpected, StaticFileSegment, TransactionSignedNoHash}; +use reth_primitives::{GotExpected, NodePrimitives, StaticFileSegment}; +use reth_primitives_traits::SignedTransaction; use reth_provider::{ BlockReader, DBProvider, HeaderProvider, ProviderError, PruneCheckpointReader, StaticFileProviderFactory, StatsReader, @@ -59,7 +60,7 @@ impl Stage for SenderRecoveryStage where Provider: DBProvider + BlockReader - + StaticFileProviderFactory + + StaticFileProviderFactory> + StatsReader + PruneCheckpointReader, { @@ -233,7 +234,9 @@ fn setup_range_recovery( provider: &Provider, ) -> mpsc::Sender, RecoveryResultSender)>> where - Provider: DBProvider + HeaderProvider + StaticFileProviderFactory, + Provider: DBProvider + + HeaderProvider + + StaticFileProviderFactory>, { let (tx_sender, tx_receiver) = mpsc::channel::, RecoveryResultSender)>>(); let static_file_provider = provider.static_file_provider(); @@ -254,9 +257,9 @@ where chunk_range.clone(), |cursor, number| { Ok(cursor - .get_one::>>( - number.into(), - )? + .get_one::::SignedTx>, + >>(number.into())? .map(|tx| (number, tx))) }, |_| true, @@ -300,17 +303,18 @@ where } #[inline] -fn recover_sender( - (tx_id, tx): (TxNumber, TransactionSignedNoHash), +fn recover_sender( + (tx_id, tx): (TxNumber, T), rlp_buf: &mut Vec, ) -> Result<(u64, Address), Box> { + rlp_buf.clear(); // We call [Signature::encode_and_recover_unchecked] because transactions run in the pipeline // are known to be valid - this means that we do not need to check whether or not the `s` // value is greater than `secp256k1n / 2` if past EIP-2. There are transactions // pre-homestead which have large `s` values, so using [Signature::recover_signer] here // would not be backwards-compatible. let sender = tx - .encode_and_recover_unchecked(rlp_buf) + .recover_signer_unchecked_with_buf(rlp_buf) .ok_or(SenderRecoveryStageError::FailedRecovery(FailedSenderRecoveryError { tx: tx_id }))?; Ok((tx_id, sender))