chore: unify recover_singer (#12881)

crates/engine/invalid-block-hooks/Cargo.toml

@@ -16,6 +16,7 @@ reth-chainspec.workspace = true
 reth-engine-primitives.workspace = true
 reth-evm.workspace = true
 reth-primitives.workspace = true
+reth-primitives-traits.workspace = true
 reth-provider.workspace = true
 reth-revm = { workspace = true, features = ["serde"] }
 reth-rpc-api = { workspace = true, features = ["client"] }

crates/engine/invalid-block-hooks/src/witness.rs

@@ -1,5 +1,3 @@
-use std::{collections::HashMap, fmt::Debug, fs::File, io::Write, path::PathBuf};
-
 use alloy_consensus::Header;
 use alloy_primitives::{keccak256, B256, U256};
 use alloy_rpc_types_debug::ExecutionWitness;
@@ -11,6 +9,7 @@ use reth_evm::{
     state_change::post_block_balance_increments, system_calls::SystemCaller, ConfigureEvm,
 };
 use reth_primitives::{Receipt, SealedBlockWithSenders, SealedHeader};
+use reth_primitives_traits::SignedTransaction;
 use reth_provider::{BlockExecutionOutput, ChainSpecProvider, StateProviderFactory};
 use reth_revm::{
     database::StateProviderDatabase,
@@ -22,6 +21,7 @@ use reth_rpc_api::DebugApiClient;
 use reth_tracing::tracing::warn;
 use reth_trie::{updates::TrieUpdates, HashedPostState, HashedStorage};
 use serde::Serialize;
+use std::{collections::HashMap, fmt::Debug, fs::File, io::Write, path::PathBuf};
 
 /// Generates a witness for the given block and saves it to a file.
 #[derive(Debug)]

crates/primitives/benches/recover_ecdsa_crit.rs

@@ -4,6 +4,7 @@ use alloy_rlp::Decodable;
 use criterion::{criterion_group, criterion_main, Criterion};
 use pprof::criterion::{Output, PProfProfiler};
 use reth_primitives::TransactionSigned;
+use reth_primitives_traits::SignedTransaction;
 
 /// Benchmarks the recovery of the public key from the ECDSA message using criterion.
 pub fn criterion_benchmark(c: &mut Criterion) {

crates/primitives/src/transaction/mod.rs

@@ -1074,42 +1074,6 @@ impl TransactionSigned {
         self.hash.get_or_init(|| self.recalculate_hash())
     }
 
-    /// Recover signer from signature and hash.
-    ///
-    /// Returns `None` if the transaction's signature is invalid following [EIP-2](https://eips.ethereum.org/EIPS/eip-2), see also [`recover_signer`].
-    ///
-    /// Note:
-    ///
-    /// This can fail for some early ethereum mainnet transactions pre EIP-2, use
-    /// [`Self::recover_signer_unchecked`] if you want to recover the signer without ensuring that
-    /// the signature has a low `s` value.
-    pub fn recover_signer(&self) -> Option<Address> {
-        // 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();
-        recover_signer(&self.signature, signature_hash)
-    }
-
-    /// Recover signer from signature and hash _without ensuring that the signature has a low `s`
-    /// value_.
-    ///
-    /// Returns `None` if the transaction's signature is invalid, see also
-    /// [`recover_signer_unchecked`].
-    pub fn recover_signer_unchecked(&self) -> Option<Address> {
-        // 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();
-        recover_signer_unchecked(&self.signature, signature_hash)
-    }
-
     /// Recovers a list of signers from a transaction list iterator.
     ///
     /// Returns `None`, if some transaction's signature is invalid, see also
@@ -1281,11 +1245,23 @@ impl SignedTransaction for TransactionSigned {
     }
 
     fn recover_signer(&self) -> Option<Address> {
+        // 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();
         recover_signer(&self.signature, signature_hash)
     }
 
     fn recover_signer_unchecked(&self) -> Option<Address> {
+        // 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();
         recover_signer_unchecked(&self.signature, signature_hash)
     }
@@ -1971,6 +1947,7 @@ mod tests {
     use alloy_rlp::{Decodable, Encodable, Error as RlpError};
     use reth_chainspec::MIN_TRANSACTION_GAS;
     use reth_codecs::Compact;
+    use reth_primitives_traits::SignedTransaction;
     use std::str::FromStr;
 
     #[test]

crates/prune/prune/src/segments/user/sender_recovery.rs

@@ -90,6 +90,7 @@ mod tests {
         Itertools,
     };
     use reth_db::tables;
+    use reth_primitives_traits::SignedTransaction;
     use reth_provider::{DatabaseProviderFactory, PruneCheckpointReader};
     use reth_prune_types::{PruneCheckpoint, PruneLimiter, PruneMode, PruneProgress, PruneSegment};
     use reth_stages::test_utils::{StorageKind, TestStageDB};

crates/rpc/rpc-eth-types/src/gas_oracle.rs

@@ -7,19 +7,21 @@ use alloy_primitives::{B256, U256};
 use alloy_rpc_types_eth::BlockId;
 use derive_more::{Deref, DerefMut, From, Into};
 use itertools::Itertools;
-use reth_rpc_server_types::constants;
+use reth_primitives_traits::SignedTransaction;
+use reth_rpc_server_types::{
+    constants,
+    constants::gas_oracle::{
+        DEFAULT_GAS_PRICE_BLOCKS, DEFAULT_GAS_PRICE_PERCENTILE, DEFAULT_IGNORE_GAS_PRICE,
+        DEFAULT_MAX_GAS_PRICE, MAX_HEADER_HISTORY, SAMPLE_NUMBER,
+    },
+};
 use reth_storage_api::BlockReaderIdExt;
 use schnellru::{ByLength, LruMap};
 use serde::{Deserialize, Serialize};
 use std::fmt::{self, Debug, Formatter};
 use tokio::sync::Mutex;
 use tracing::warn;
 
-use reth_rpc_server_types::constants::gas_oracle::{
-    DEFAULT_GAS_PRICE_BLOCKS, DEFAULT_GAS_PRICE_PERCENTILE, DEFAULT_IGNORE_GAS_PRICE,
-    DEFAULT_MAX_GAS_PRICE, MAX_HEADER_HISTORY, SAMPLE_NUMBER,
-};
-
 use super::{EthApiError, EthResult, EthStateCache, RpcInvalidTransactionError};
 
 /// The default gas limit for `eth_call` and adjacent calls. See

crates/rpc/rpc-eth-types/src/receipt.rs

@@ -1,13 +1,13 @@
 //! RPC receipt response builder, extends a layer one receipt with layer two data.
 
+use super::{EthApiError, EthResult};
 use alloy_consensus::{ReceiptEnvelope, Transaction};
 use alloy_primitives::{Address, TxKind};
 use alloy_rpc_types_eth::{Log, ReceiptWithBloom, TransactionReceipt};
 use reth_primitives::{Receipt, TransactionMeta, TransactionSigned, TxType};
+use reth_primitives_traits::SignedTransaction;
 use revm_primitives::calc_blob_gasprice;
 
-use super::{EthApiError, EthResult};
-
 /// Builds an [`TransactionReceipt`] obtaining the inner receipt envelope from the given closure.
 pub fn build_receipt<T>(
     transaction: &TransactionSigned,

crates/rpc/rpc/Cargo.toml

@@ -15,6 +15,7 @@ workspace = true
 # reth
 reth-chainspec.workspace = true
 reth-primitives = { workspace = true, features = ["secp256k1"] }
+reth-primitives-traits.workspace = true
 reth-rpc-api.workspace = true
 reth-rpc-eth-api.workspace = true
 reth-errors.workspace = true

crates/rpc/rpc/src/debug.rs

@@ -19,6 +19,7 @@ use reth_evm::{
     ConfigureEvmEnv,
 };
 use reth_primitives::{Block, BlockExt, SealedBlockWithSenders};
+use reth_primitives_traits::SignedTransaction;
 use reth_provider::{
     BlockReaderIdExt, ChainSpecProvider, HeaderProvider, StateProofProvider, StateProviderFactory,
     TransactionVariant,

crates/stages/stages/src/stages/prune.rs

@@ -171,6 +171,7 @@ mod tests {
     };
     use alloy_primitives::B256;
     use reth_primitives::SealedBlock;
+    use reth_primitives_traits::SignedTransaction;
     use reth_provider::{
         providers::StaticFileWriter, TransactionsProvider, TransactionsProviderExt,
     };

crates/stages/stages/src/stages/sender_recovery.rs

@@ -361,10 +361,16 @@ struct FailedSenderRecoveryError {
 
 #[cfg(test)]
 mod tests {
+    use super::*;
+    use crate::test_utils::{
+        stage_test_suite_ext, ExecuteStageTestRunner, StageTestRunner, StorageKind,
+        TestRunnerError, TestStageDB, UnwindStageTestRunner,
+    };
     use alloy_primitives::{BlockNumber, B256};
     use assert_matches::assert_matches;
     use reth_db_api::cursor::DbCursorRO;
     use reth_primitives::{SealedBlock, TransactionSigned};
+    use reth_primitives_traits::SignedTransaction;
     use reth_provider::{
         providers::StaticFileWriter, DatabaseProviderFactory, PruneCheckpointWriter,
         StaticFileProviderFactory, TransactionsProvider,
@@ -375,12 +381,6 @@ mod tests {
         self, random_block, random_block_range, BlockParams, BlockRangeParams,
     };
 
-    use super::*;
-    use crate::test_utils::{
-        stage_test_suite_ext, ExecuteStageTestRunner, StageTestRunner, StorageKind,
-        TestRunnerError, TestStageDB, UnwindStageTestRunner,
-    };
-
     stage_test_suite_ext!(SenderRecoveryTestRunner, sender_recovery);
 
     /// Execute a block range with a single transaction

crates/storage/provider/src/providers/blockchain_provider.rs

@@ -775,12 +775,6 @@ impl<N: ProviderNodeTypes> StateReader for BlockchainProvider2<N> {
 
 #[cfg(test)]
 mod tests {
-    use std::{
-        ops::{Range, RangeBounds},
-        sync::Arc,
-        time::Instant,
-    };
-
     use crate::{
         providers::BlockchainProvider2,
         test_utils::{
@@ -812,7 +806,7 @@ mod tests {
     use reth_primitives::{
         BlockExt, Receipt, SealedBlock, StaticFileSegment, TransactionSignedNoHash,
     };
-    use reth_primitives_traits::BlockBody as _;
+    use reth_primitives_traits::{BlockBody as _, SignedTransaction};
     use reth_storage_api::{
         BlockHashReader, BlockIdReader, BlockNumReader, BlockReader, BlockReaderIdExt, BlockSource,
         ChangeSetReader, DatabaseProviderFactory, HeaderProvider, ReceiptProvider,
@@ -824,7 +818,11 @@ mod tests {
         random_receipt, BlockParams, BlockRangeParams,
     };
     use revm::db::BundleState;
-    use std::ops::Bound;
+    use std::{
+        ops::{Bound, Range, RangeBounds},
+        sync::Arc,
+        time::Instant,
+    };
 
     const TEST_BLOCKS_COUNT: usize = 5;
 

crates/storage/provider/src/providers/database/mod.rs

@@ -670,6 +670,7 @@ mod tests {
         test_utils::{create_test_static_files_dir, ERROR_TEMPDIR},
     };
     use reth_primitives::StaticFileSegment;
+    use reth_primitives_traits::SignedTransaction;
     use reth_prune_types::{PruneMode, PruneModes};
     use reth_storage_errors::provider::ProviderError;
     use reth_testing_utils::generators::{self, random_block, random_header, BlockParams};

crates/storage/provider/src/test_utils/mock.rs

@@ -26,6 +26,7 @@ use reth_primitives::{
     Account, Block, BlockWithSenders, Bytecode, EthPrimitives, GotExpected, Receipt, SealedBlock,
     SealedBlockWithSenders, SealedHeader, TransactionMeta, TransactionSigned,
 };
+use reth_primitives_traits::SignedTransaction;
 use reth_stages_types::{StageCheckpoint, StageId};
 use reth_storage_api::{
     DatabaseProviderFactory, StageCheckpointReader, StateProofProvider, StorageRootProvider,

testing/testing-utils/Cargo.toml

@@ -24,3 +24,4 @@ secp256k1 = { workspace = true, features = ["rand"] }
 
 [dev-dependencies]
 alloy-eips.workspace = true
+reth-primitives-traits .workspace = true

testing/testing-utils/src/generators.rs

@@ -453,6 +453,7 @@ mod tests {
     use alloy_eips::eip2930::AccessList;
     use alloy_primitives::{hex, PrimitiveSignature as Signature};
     use reth_primitives::public_key_to_address;
+    use reth_primitives_traits::SignedTransaction;
     use std::str::FromStr;
 
     #[test]