diff --git a/bin/reth/src/commands/debug_cmd/build_block.rs b/bin/reth/src/commands/debug_cmd/build_block.rs index 88f2b322b..f932ae1fb 100644 --- a/bin/reth/src/commands/debug_cmd/build_block.rs +++ b/bin/reth/src/commands/debug_cmd/build_block.rs @@ -167,7 +167,7 @@ impl> Command { debug!(target: "reth::cli", bytes = ?tx_bytes, "Decoding transaction"); let transaction = TransactionSigned::decode(&mut &Bytes::from_str(tx_bytes)?[..])? .try_ecrecovered() - .ok_or_else(|| eyre::eyre!("failed to recover tx"))?; + .map_err(|e| eyre::eyre!("failed to recover tx: {e}"))?; let encoded_length = match &transaction.transaction { Transaction::Eip4844(TxEip4844 { blob_versioned_hashes, .. }) => { diff --git a/crates/engine/invalid-block-hooks/src/witness.rs b/crates/engine/invalid-block-hooks/src/witness.rs index af008470c..ad5ceb906 100644 --- a/crates/engine/invalid-block-hooks/src/witness.rs +++ b/crates/engine/invalid-block-hooks/src/witness.rs @@ -1,7 +1,6 @@ use alloy_consensus::BlockHeader; use alloy_primitives::{keccak256, B256}; use alloy_rpc_types_debug::ExecutionWitness; -use eyre::OptionExt; use pretty_assertions::Comparison; use reth_chainspec::{EthChainSpec, EthereumHardforks}; use reth_engine_primitives::InvalidBlockHook; @@ -87,7 +86,8 @@ where // Re-execute all of the transactions in the block to load all touched accounts into // the cache DB. for tx in block.body().transactions() { - let signer = tx.recover_signer().ok_or_eyre("failed to recover sender")?; + let signer = + tx.recover_signer().map_err(|_| eyre::eyre!("failed to recover sender"))?; evm.transact_commit(self.evm_config.tx_env(tx, signer))?; } diff --git a/crates/ethereum/primitives/src/transaction.rs b/crates/ethereum/primitives/src/transaction.rs index 8a80f5118..261950429 100644 --- a/crates/ethereum/primitives/src/transaction.rs +++ b/crates/ethereum/primitives/src/transaction.rs @@ -19,7 +19,7 @@ use once_cell as _; use once_cell::sync::OnceCell as OnceLock; use reth_primitives_traits::{ crypto::secp256k1::{recover_signer, recover_signer_unchecked}, - transaction::error::TransactionConversionError, + transaction::{error::TransactionConversionError, signed::RecoveryError}, FillTxEnv, InMemorySize, SignedTransaction, }; use revm_primitives::{AuthorizationList, TxEnv}; @@ -719,12 +719,15 @@ impl SignedTransaction for TransactionSigned { &self.signature } - fn recover_signer(&self) -> Option
{ + fn recover_signer(&self) -> Result { let signature_hash = self.signature_hash(); recover_signer(&self.signature, signature_hash) } - fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ + fn recover_signer_unchecked_with_buf( + &self, + buf: &mut Vec, + ) -> Result { self.encode_for_signing(buf); let signature_hash = keccak256(buf); recover_signer_unchecked(&self.signature, signature_hash) @@ -940,10 +943,10 @@ mod tests { // recover signer, expect failure let hash = *tx.tx_hash(); - assert!(recover_signer(signature, hash).is_none()); + assert!(recover_signer(signature, hash).is_err()); // use unchecked, ensure it succeeds (the signature is valid if not for EIP-2) - assert!(recover_signer_unchecked(signature, hash).is_some()); + assert!(recover_signer_unchecked(signature, hash).is_ok()); } #[test] @@ -1002,8 +1005,8 @@ mod tests { let decoded = TransactionSigned::decode_2718(&mut &tx_bytes[..]).unwrap(); assert_eq!( - decoded.recover_signer(), - Some(Address::from_str("0x95222290DD7278Aa3Ddd389Cc1E1d165CC4BAfe5").unwrap()) + decoded.recover_signer().unwrap(), + Address::from_str("0x95222290DD7278Aa3Ddd389Cc1E1d165CC4BAfe5").unwrap() ); } @@ -1018,8 +1021,10 @@ mod tests { let decoded = TransactionSigned::decode_2718(&mut raw_tx.as_slice()).unwrap(); assert!(alloy_consensus::Typed2718::is_eip4844(&decoded)); - let from = decoded.recover_signer(); - assert_eq!(from, Some(address!("A83C816D4f9b2783761a22BA6FADB0eB0606D7B2"))); + assert_eq!( + decoded.recover_signer().ok(), + Some(address!("A83C816D4f9b2783761a22BA6FADB0eB0606D7B2")) + ); let tx = decoded.transaction; @@ -1182,7 +1187,8 @@ mod tests { let mut pointer = raw.as_ref(); let tx = TransactionSigned::decode(&mut pointer).unwrap(); assert_eq!(*tx.tx_hash(), hash, "Expected same hash"); - assert_eq!(tx.recover_signer(), Some(signer), "Recovering signer should pass."); + let recovered = tx.recover_signer().expect("Recovering signer should pass"); + assert_eq!(recovered, signer); } #[test] @@ -1252,7 +1258,7 @@ mod tests { let data = hex!("f8ea0c850ba43b7400832dc6c0942935aa0a2d2fbb791622c29eb1c117b65b7a908580b884590528a9000000000000000000000001878ace42092b7f1ae1f28d16c1272b1aa80ca4670000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000d02ab486cedc0000000000000000000000000000000000000000000000000000557fe293cabc08cf1ca05bfaf3fda0a56b49cc78b22125feb5ae6a99d2b4781f00507d8b02c173771c85a0b5da0dbe6c5bc53740d0071fc83eb17ba0f709e49e9ae7df60dee625ef51afc5"); let tx = TransactionSigned::decode_2718(&mut data.as_slice()).unwrap(); let sender = tx.recover_signer(); - assert!(sender.is_none()); + assert!(sender.is_err()); let sender = tx.recover_signer_unchecked().unwrap(); assert_eq!(sender, address!("7e9e359edf0dbacf96a9952fa63092d919b0842b")); diff --git a/crates/optimism/primitives/src/transaction/signed.rs b/crates/optimism/primitives/src/transaction/signed.rs index cde794efa..a6def1d4a 100644 --- a/crates/optimism/primitives/src/transaction/signed.rs +++ b/crates/optimism/primitives/src/transaction/signed.rs @@ -26,7 +26,7 @@ use proptest as _; use reth_primitives_traits::{ crypto::secp256k1::{recover_signer, recover_signer_unchecked}, sync::OnceLock, - transaction::error::TransactionConversionError, + transaction::{error::TransactionConversionError, signed::RecoveryError}, InMemorySize, SignedTransaction, }; @@ -79,11 +79,11 @@ impl SignedTransaction for OpTransactionSigned { &self.signature } - fn recover_signer(&self) -> Option
{ + fn recover_signer(&self) -> Result { // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. if let OpTypedTransaction::Deposit(TxDeposit { from, .. }) = self.transaction { - return Some(from) + return Ok(from) } let Self { transaction, signature, .. } = self; @@ -91,11 +91,11 @@ impl SignedTransaction for OpTransactionSigned { recover_signer(signature, signature_hash) } - fn recover_signer_unchecked(&self) -> Option
{ + fn recover_signer_unchecked(&self) -> Result { // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. if let OpTypedTransaction::Deposit(TxDeposit { from, .. }) = &self.transaction { - return Some(*from) + return Ok(*from) } let Self { transaction, signature, .. } = self; @@ -103,11 +103,14 @@ impl SignedTransaction for OpTransactionSigned { recover_signer_unchecked(signature, signature_hash) } - fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ + fn recover_signer_unchecked_with_buf( + &self, + buf: &mut Vec, + ) -> Result { match &self.transaction { // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. - OpTypedTransaction::Deposit(tx) => return Some(tx.from), + OpTypedTransaction::Deposit(tx) => return Ok(tx.from), OpTypedTransaction::Legacy(tx) => tx.encode_for_signing(buf), OpTypedTransaction::Eip2930(tx) => tx.encode_for_signing(buf), OpTypedTransaction::Eip1559(tx) => tx.encode_for_signing(buf), diff --git a/crates/primitives-traits/src/block/body.rs b/crates/primitives-traits/src/block/body.rs index 01dd5df50..bbbd70654 100644 --- a/crates/primitives-traits/src/block/body.rs +++ b/crates/primitives-traits/src/block/body.rs @@ -123,11 +123,11 @@ pub trait BlockBody: } /// Recover signer addresses for all transactions in the block body. - fn recover_signers(&self) -> Option> + fn recover_signers(&self) -> Result, RecoveryError> where Self::Transaction: SignedTransaction, { - crate::transaction::recover::recover_signers(self.transactions()) + crate::transaction::recover::recover_signers(self.transactions()).map_err(|_| RecoveryError) } /// Recover signer addresses for all transactions in the block body. @@ -137,14 +137,14 @@ pub trait BlockBody: where Self::Transaction: SignedTransaction, { - self.recover_signers().ok_or(RecoveryError) + self.recover_signers() } /// Recover signer addresses for all transactions in the block body _without ensuring that the /// signature has a low `s` value_. /// /// Returns `None`, if some transaction's signature is invalid. - fn recover_signers_unchecked(&self) -> Option> + fn recover_signers_unchecked(&self) -> Result, RecoveryError> where Self::Transaction: SignedTransaction, { @@ -159,7 +159,7 @@ pub trait BlockBody: where Self::Transaction: SignedTransaction, { - self.recover_signers_unchecked().ok_or(RecoveryError) + self.recover_signers_unchecked() } } diff --git a/crates/primitives-traits/src/block/mod.rs b/crates/primitives-traits/src/block/mod.rs index f0a9c3670..67e197e15 100644 --- a/crates/primitives-traits/src/block/mod.rs +++ b/crates/primitives-traits/src/block/mod.rs @@ -16,8 +16,8 @@ use alloy_primitives::{Address, B256}; use alloy_rlp::{Decodable, Encodable}; use crate::{ - BlockBody, BlockHeader, FullBlockBody, FullBlockHeader, InMemorySize, MaybeSerde, SealedHeader, - SignedTransaction, + transaction::signed::RecoveryError, BlockBody, BlockHeader, FullBlockBody, FullBlockHeader, + InMemorySize, MaybeSerde, SealedHeader, SignedTransaction, }; /// Bincode-compatible header type serde implementations. @@ -121,7 +121,7 @@ pub trait Block: } /// Expensive operation that recovers transaction signer. - fn senders(&self) -> Option> + fn senders(&self) -> Result, RecoveryError> where ::Transaction: SignedTransaction, { @@ -158,10 +158,10 @@ pub trait Block: let senders = if self.body().transactions().len() == senders.len() { senders } else { - let Some(senders) = self.body().recover_signers_unchecked() else { return Err(self) }; + // Fall back to recovery if lengths don't match + let Ok(senders) = self.body().recover_signers_unchecked() else { return Err(self) }; senders }; - Ok(RecoveredBlock::new_unhashed(self, senders)) } @@ -173,7 +173,7 @@ pub trait Block: where ::Transaction: SignedTransaction, { - let senders = self.senders()?; + let senders = self.body().recover_signers().ok()?; Some(RecoveredBlock::new_unhashed(self, senders)) } } diff --git a/crates/primitives-traits/src/block/sealed.rs b/crates/primitives-traits/src/block/sealed.rs index 5ff43bff6..611b1ad7e 100644 --- a/crates/primitives-traits/src/block/sealed.rs +++ b/crates/primitives-traits/src/block/sealed.rs @@ -2,6 +2,7 @@ use crate::{ block::{error::BlockRecoveryError, RecoveredBlock}, + transaction::signed::RecoveryError, Block, BlockBody, GotExpected, InMemorySize, SealedHeader, }; use alloc::vec::Vec; @@ -179,7 +180,7 @@ impl SealedBlock { /// Recovers all senders from the transactions in the block. /// /// Returns `None` if any of the transactions fail to recover the sender. - pub fn senders(&self) -> Option> { + pub fn senders(&self) -> Result, RecoveryError> { self.body().recover_signers() } diff --git a/crates/primitives-traits/src/crypto.rs b/crates/primitives-traits/src/crypto.rs index 99a6521cd..0f59b97db 100644 --- a/crates/primitives-traits/src/crypto.rs +++ b/crates/primitives-traits/src/crypto.rs @@ -22,6 +22,7 @@ pub mod secp256k1 { #[cfg(feature = "secp256k1")] use super::impl_secp256k1 as imp; + use crate::transaction::signed::RecoveryError; pub use imp::{public_key_to_address, sign_message}; /// Recover signer from message hash, _without ensuring that the signature has a low `s` @@ -30,7 +31,10 @@ pub mod secp256k1 { /// Using this for signature validation will succeed, even if the signature is malleable or not /// compliant with EIP-2. This is provided for compatibility with old signatures which have /// large `s` values. - pub fn recover_signer_unchecked(signature: &Signature, hash: B256) -> Option
{ + pub fn recover_signer_unchecked( + signature: &Signature, + hash: B256, + ) -> Result { let mut sig: [u8; 65] = [0; 65]; sig[0..32].copy_from_slice(&signature.r().to_be_bytes::<32>()); @@ -39,7 +43,7 @@ pub mod secp256k1 { // NOTE: we are removing error from underlying crypto library as it will restrain primitive // errors and we care only if recovery is passing or not. - imp::recover_signer_unchecked(&sig, &hash.0).ok() + imp::recover_signer_unchecked(&sig, &hash.0).map_err(|_| RecoveryError) } /// Recover signer address from message hash. This ensures that the signature S value is @@ -47,11 +51,10 @@ pub mod secp256k1 { /// [EIP-2](https://eips.ethereum.org/EIPS/eip-2). /// /// If the S value is too large, then this will return `None` - pub fn recover_signer(signature: &Signature, hash: B256) -> Option
{ + pub fn recover_signer(signature: &Signature, hash: B256) -> Result { if signature.s() > SECP256K1N_HALF { - return None + return Err(RecoveryError) } - recover_signer_unchecked(signature, hash) } } diff --git a/crates/primitives-traits/src/transaction/recover.rs b/crates/primitives-traits/src/transaction/recover.rs index cad57bc26..a7436c3ef 100644 --- a/crates/primitives-traits/src/transaction/recover.rs +++ b/crates/primitives-traits/src/transaction/recover.rs @@ -8,7 +8,7 @@ pub use iter::*; #[cfg(feature = "rayon")] mod rayon { - use crate::SignedTransaction; + use crate::{transaction::signed::RecoveryError, SignedTransaction}; use alloc::vec::Vec; use alloy_primitives::Address; use rayon::prelude::{IntoParallelIterator, ParallelIterator}; @@ -16,7 +16,7 @@ mod rayon { /// Recovers a list of signers from a transaction list iterator. /// /// Returns `None`, if some transaction's signature is invalid - pub fn recover_signers<'a, I, T>(txes: I) -> Option> + pub fn recover_signers<'a, I, T>(txes: I) -> Result, RecoveryError> where T: SignedTransaction, I: IntoParallelIterator + IntoIterator + Send, @@ -28,7 +28,7 @@ mod rayon { /// signature has a low `s` value_. /// /// Returns `None`, if some transaction's signature is invalid. - pub fn recover_signers_unchecked<'a, I, T>(txes: I) -> Option> + pub fn recover_signers_unchecked<'a, I, T>(txes: I) -> Result, RecoveryError> where T: SignedTransaction, I: IntoParallelIterator + IntoIterator + Send, @@ -39,14 +39,14 @@ mod rayon { #[cfg(not(feature = "rayon"))] mod iter { - use crate::SignedTransaction; + use crate::{transaction::signed::RecoveryError, SignedTransaction}; use alloc::vec::Vec; use alloy_primitives::Address; /// Recovers a list of signers from a transaction list iterator. /// - /// Returns `None`, if some transaction's signature is invalid - pub fn recover_signers<'a, I, T>(txes: I) -> Option> + /// Returns `Err(RecoveryError)`, if some transaction's signature is invalid + pub fn recover_signers<'a, I, T>(txes: I) -> Result, RecoveryError> where T: SignedTransaction, I: IntoIterator + IntoIterator, @@ -57,8 +57,8 @@ mod iter { /// Recovers a list of signers from a transaction list iterator _without ensuring that the /// signature has a low `s` value_. /// - /// Returns `None`, if some transaction's signature is invalid. - pub fn recover_signers_unchecked<'a, I, T>(txes: I) -> Option> + /// Returns `Err(RecoveryError)`, if some transaction's signature is invalid. + pub fn recover_signers_unchecked<'a, I, T>(txes: I) -> Result, RecoveryError> where T: SignedTransaction, I: IntoIterator + IntoIterator, diff --git a/crates/primitives-traits/src/transaction/signed.rs b/crates/primitives-traits/src/transaction/signed.rs index bc64b4521..1370d9d59 100644 --- a/crates/primitives-traits/src/transaction/signed.rs +++ b/crates/primitives-traits/src/transaction/signed.rs @@ -62,13 +62,13 @@ pub trait SignedTransaction: /// 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. - fn recover_signer(&self) -> Option
; + fn recover_signer(&self) -> Result; /// Recover signer from signature and hash. /// /// Returns an error if the transaction's signature is invalid. fn try_recover(&self) -> Result { - self.recover_signer().ok_or(RecoveryError) + self.recover_signer().map_err(|_| RecoveryError) } /// Recover signer from signature and hash _without ensuring that the signature has a low `s` @@ -76,8 +76,8 @@ 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
{ - self.recover_signer_unchecked_with_buf(&mut Vec::new()) + fn recover_signer_unchecked(&self) -> Result { + self.recover_signer_unchecked_with_buf(&mut Vec::new()).map_err(|_| RecoveryError) } /// Recover signer from signature and hash _without ensuring that the signature has a low `s` @@ -85,12 +85,15 @@ pub trait SignedTransaction: /// /// Returns an error if the transaction's signature is invalid. fn try_recover_unchecked(&self) -> Result { - self.recover_signer_unchecked().ok_or(RecoveryError) + self.recover_signer_unchecked() } /// 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
; + fn recover_signer_unchecked_with_buf( + &self, + buf: &mut Vec, + ) -> Result; /// Calculate transaction hash, eip2728 transaction does not contain rlp header and start with /// tx type. @@ -120,12 +123,15 @@ impl SignedTransaction for PooledTransaction { } } - fn recover_signer(&self) -> Option
{ + fn recover_signer(&self) -> Result { let signature_hash = self.signature_hash(); recover_signer(self.signature(), signature_hash) } - fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ + fn recover_signer_unchecked_with_buf( + &self, + buf: &mut Vec, + ) -> Result { match self { Self::Legacy(tx) => tx.tx().encode_for_signing(buf), Self::Eip2930(tx) => tx.tx().encode_for_signing(buf), @@ -158,12 +164,15 @@ impl SignedTransaction for op_alloy_consensus::OpPooledTransaction { } } - fn recover_signer(&self) -> Option
{ + fn recover_signer(&self) -> Result { let signature_hash = self.signature_hash(); recover_signer(self.signature(), signature_hash) } - fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec) -> Option
{ + fn recover_signer_unchecked_with_buf( + &self, + buf: &mut Vec, + ) -> Result { match self { Self::Legacy(tx) => tx.tx().encode_for_signing(buf), Self::Eip2930(tx) => tx.tx().encode_for_signing(buf), @@ -178,9 +187,8 @@ impl SignedTransaction for op_alloy_consensus::OpPooledTransaction { /// Extension trait for [`SignedTransaction`] to convert it into [`Recovered`]. pub trait SignedTransactionIntoRecoveredExt: SignedTransaction { /// Tries to recover signer and return [`Recovered`] by cloning the type. - fn try_ecrecovered(&self) -> Option> { - let signer = self.recover_signer()?; - Some(Recovered::new_unchecked(self.clone(), signer)) + fn try_ecrecovered(&self) -> Result, RecoveryError> { + self.recover_signer().map(|signer| Recovered::new_unchecked(self.clone(), signer)) } /// Tries to recover signer and return [`Recovered`]. @@ -189,8 +197,8 @@ pub trait SignedTransactionIntoRecoveredExt: SignedTransaction { /// [`SignedTransaction::recover_signer`]. fn try_into_ecrecovered(self) -> Result, Self> { match self.recover_signer() { - None => Err(self), - Some(signer) => Ok(Recovered::new_unchecked(self, signer)), + Ok(signer) => Ok(Recovered::new_unchecked(self, signer)), + Err(_) => Err(self), } } @@ -198,9 +206,8 @@ pub trait SignedTransactionIntoRecoveredExt: SignedTransaction { /// ensuring that the signature has a low `s` value_ (EIP-2). /// /// Returns `None` if the transaction's signature is invalid. - fn into_ecrecovered_unchecked(self) -> Option> { - let signer = self.recover_signer_unchecked()?; - Some(Recovered::new_unchecked(self, signer)) + fn into_ecrecovered_unchecked(self) -> Result, RecoveryError> { + self.recover_signer_unchecked().map(|signer| Recovered::new_unchecked(self, signer)) } /// Returns the [`Recovered`] transaction with the given sender. diff --git a/crates/primitives/benches/recover_ecdsa_crit.rs b/crates/primitives/benches/recover_ecdsa_crit.rs index 9273d71f6..af275c72a 100644 --- a/crates/primitives/benches/recover_ecdsa_crit.rs +++ b/crates/primitives/benches/recover_ecdsa_crit.rs @@ -13,7 +13,7 @@ pub fn criterion_benchmark(c: &mut Criterion) { let raw =hex!("f88b8212b085028fa6ae00830f424094aad593da0c8116ef7d2d594dd6a63241bccfc26c80a48318b64b000000000000000000000000641c5d790f862a58ec7abcfd644c0442e9c201b32aa0a6ef9e170bca5ffb7ac05433b13b7043de667fbb0b4a5e45d3b54fb2d6efcc63a0037ec2c05c3d60c5f5f78244ce0a3859e3a18a36c61efb061b383507d3ce19d2"); let mut pointer = raw.as_ref(); let tx = TransactionSigned::decode(&mut pointer).unwrap(); - tx.recover_signer(); + tx.recover_signer().unwrap(); } ) }); diff --git a/crates/rpc/rpc-eth-api/src/helpers/transaction.rs b/crates/rpc/rpc-eth-api/src/helpers/transaction.rs index 6ab585ded..ff69fbde1 100644 --- a/crates/rpc/rpc-eth-api/src/helpers/transaction.rs +++ b/crates/rpc/rpc-eth-api/src/helpers/transaction.rs @@ -486,7 +486,7 @@ pub trait LoadTransaction: SpawnBlocking + FullEthApiTypes + RpcNodeCoreExt { // check for pre EIP-2 because this transaction could be pre-EIP-2. let transaction = tx .into_ecrecovered_unchecked() - .ok_or(EthApiError::InvalidTransactionSignature)?; + .map_err(|_| EthApiError::InvalidTransactionSignature)?; let tx = TransactionSource::Block { transaction, diff --git a/crates/rpc/rpc-eth-types/src/error/mod.rs b/crates/rpc/rpc-eth-types/src/error/mod.rs index e93a566e2..d798002f3 100644 --- a/crates/rpc/rpc-eth-types/src/error/mod.rs +++ b/crates/rpc/rpc-eth-types/src/error/mod.rs @@ -10,6 +10,7 @@ use alloy_primitives::{Address, Bytes, U256}; use alloy_rpc_types_eth::{error::EthRpcErrorCode, request::TransactionInputError, BlockError}; use alloy_sol_types::decode_revert_reason; use reth_errors::RethError; +use reth_primitives_traits::transaction::signed::RecoveryError; use reth_rpc_server_types::result::{ block_id_to_str, internal_rpc_err, invalid_params_rpc_err, rpc_err, rpc_error_with_code, }; @@ -285,6 +286,12 @@ where } } +impl From for EthApiError { + fn from(_: RecoveryError) -> Self { + Self::InvalidTransactionSignature + } +} + /// An error due to invalid transaction. /// /// The only reason this exists is to maintain compatibility with other clients de-facto standard diff --git a/crates/rpc/rpc-eth-types/src/gas_oracle.rs b/crates/rpc/rpc-eth-types/src/gas_oracle.rs index c3a7d9bc9..07f5fb1e0 100644 --- a/crates/rpc/rpc-eth-types/src/gas_oracle.rs +++ b/crates/rpc/rpc-eth-types/src/gas_oracle.rs @@ -251,7 +251,7 @@ where } // check if the sender was the coinbase, if so, ignore - if let Some(sender) = tx.recover_signer() { + if let Ok(sender) = tx.recover_signer() { if sender == block.beneficiary() { continue } diff --git a/crates/rpc/rpc-eth-types/src/receipt.rs b/crates/rpc/rpc-eth-types/src/receipt.rs index c207eb1bc..60d92acd3 100644 --- a/crates/rpc/rpc-eth-types/src/receipt.rs +++ b/crates/rpc/rpc-eth-types/src/receipt.rs @@ -1,6 +1,6 @@ //! RPC receipt response builder, extends a layer one receipt with layer two data. -use super::{EthApiError, EthResult}; +use super::EthResult; use alloy_consensus::{transaction::TransactionMeta, ReceiptEnvelope, TxReceipt}; use alloy_primitives::{Address, TxKind}; use alloy_rpc_types_eth::{Log, ReceiptWithBloom, TransactionReceipt}; @@ -21,8 +21,7 @@ where { // Note: we assume this transaction is valid, because it's mined (or part of pending block) // and we don't need to check for pre EIP-2 - let from = - transaction.recover_signer_unchecked().ok_or(EthApiError::InvalidTransactionSignature)?; + let from = transaction.recover_signer_unchecked()?; // get the previous transaction cumulative gas used let gas_used = if meta.index == 0 { diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index 03d3be817..222e584e5 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -166,23 +166,19 @@ where .body() .transactions() .iter() - .map(|tx| { - tx.recover_signer() - .ok_or(EthApiError::InvalidTransactionSignature) - .map_err(Eth::Error::from_eth_err) - }) - .collect::, Eth::Error>>()? + .map(|tx| tx.recover_signer().map_err(Eth::Error::from_eth_err)) + .collect::, _>>()? + .into_iter() + .collect() } else { block .body() .transactions() .iter() - .map(|tx| { - tx.recover_signer_unchecked() - .ok_or(EthApiError::InvalidTransactionSignature) - .map_err(Eth::Error::from_eth_err) - }) - .collect::, Eth::Error>>()? + .map(|tx| tx.recover_signer_unchecked().map_err(Eth::Error::from_eth_err)) + .collect::, _>>()? + .into_iter() + .collect() }; self.trace_block(Arc::new(block.with_senders_unchecked(senders)), evm_env, opts).await diff --git a/crates/stages/stages/src/stages/sender_recovery.rs b/crates/stages/stages/src/stages/sender_recovery.rs index 34598714a..05e0230e0 100644 --- a/crates/stages/stages/src/stages/sender_recovery.rs +++ b/crates/stages/stages/src/stages/sender_recovery.rs @@ -313,9 +313,9 @@ fn recover_sender( // 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 - .recover_signer_unchecked_with_buf(rlp_buf) - .ok_or(SenderRecoveryStageError::FailedRecovery(FailedSenderRecoveryError { tx: tx_id }))?; + let sender = tx.recover_signer_unchecked_with_buf(rlp_buf).map_err(|_| { + SenderRecoveryStageError::FailedRecovery(FailedSenderRecoveryError { tx: tx_id }) + })?; Ok((tx_id, sender)) } diff --git a/crates/storage/errors/src/provider.rs b/crates/storage/errors/src/provider.rs index 671c3d673..81961a9ec 100644 --- a/crates/storage/errors/src/provider.rs +++ b/crates/storage/errors/src/provider.rs @@ -3,7 +3,7 @@ use alloc::{boxed::Box, string::String}; use alloy_eips::{BlockHashOrNumber, HashOrNumber}; use alloy_primitives::{Address, BlockHash, BlockNumber, TxNumber, B256}; use derive_more::Display; -use reth_primitives_traits::GotExpected; +use reth_primitives_traits::{transaction::signed::RecoveryError, GotExpected}; use reth_prune_types::PruneSegmentError; use reth_static_file_types::StaticFileSegment; @@ -151,6 +151,12 @@ impl From for ProviderError { } } +impl From for ProviderError { + fn from(_: RecoveryError) -> Self { + Self::SenderRecoveryError + } +} + /// A root mismatch error at a given block height. #[derive(Clone, Debug, PartialEq, Eq, Display)] #[display("root mismatch at #{block_number} ({block_hash}): {root}")] diff --git a/crates/storage/provider/src/providers/blockchain_provider.rs b/crates/storage/provider/src/providers/blockchain_provider.rs index 92451fab1..d94f9cdd8 100644 --- a/crates/storage/provider/src/providers/blockchain_provider.rs +++ b/crates/storage/provider/src/providers/blockchain_provider.rs @@ -2640,7 +2640,7 @@ mod tests { transaction_sender, |block: &SealedBlock, tx_num: TxNumber, _: B256, _: &Vec>| ( tx_num, - block.body().transactions[test_tx_index].recover_signer() + block.body().transactions[test_tx_index].recover_signer().ok() ), u64::MAX ), diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 6fefa4d40..3cd0200ab 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -693,9 +693,7 @@ impl DatabaseProvider { match known_senders.get(&tx_num) { None => { // recover the sender from the transaction if not found - let sender = tx - .recover_signer_unchecked() - .ok_or(ProviderError::SenderRecoveryError)?; + let sender = tx.recover_signer_unchecked()?; senders.push(sender); } Some(sender) => senders.push(*sender), diff --git a/crates/storage/provider/src/providers/static_file/jar.rs b/crates/storage/provider/src/providers/static_file/jar.rs index 8a7654470..c87fc45d7 100644 --- a/crates/storage/provider/src/providers/static_file/jar.rs +++ b/crates/storage/provider/src/providers/static_file/jar.rs @@ -299,15 +299,14 @@ impl> TransactionsPr range: impl RangeBounds, ) -> ProviderResult> { let txs = self.transactions_by_tx_range(range)?; - reth_primitives_traits::transaction::recover::recover_signers(&txs) - .ok_or(ProviderError::SenderRecoveryError) + Ok(reth_primitives_traits::transaction::recover::recover_signers(&txs)?) } fn transaction_sender(&self, num: TxNumber) -> ProviderResult> { Ok(self .cursor()? .get_one::>(num.into())? - .and_then(|tx| tx.recover_signer())) + .and_then(|tx| tx.recover_signer().ok())) } } diff --git a/crates/storage/provider/src/providers/static_file/manager.rs b/crates/storage/provider/src/providers/static_file/manager.rs index 51b68f8f2..77583f0da 100644 --- a/crates/storage/provider/src/providers/static_file/manager.rs +++ b/crates/storage/provider/src/providers/static_file/manager.rs @@ -1568,12 +1568,14 @@ impl> TransactionsPr range: impl RangeBounds, ) -> ProviderResult> { let txes = self.transactions_by_tx_range(range)?; - reth_primitives_traits::transaction::recover::recover_signers(&txes) - .ok_or(ProviderError::SenderRecoveryError) + Ok(reth_primitives_traits::transaction::recover::recover_signers(&txes)?) } fn transaction_sender(&self, id: TxNumber) -> ProviderResult> { - Ok(self.transaction_by_id_unhashed(id)?.and_then(|tx| tx.recover_signer())) + match self.transaction_by_id_unhashed(id)? { + Some(tx) => Ok(tx.recover_signer().ok()), + None => Ok(None), + } } } diff --git a/crates/storage/provider/src/test_utils/mock.rs b/crates/storage/provider/src/test_utils/mock.rs index e6efe8012..d484d1e65 100644 --- a/crates/storage/provider/src/test_utils/mock.rs +++ b/crates/storage/provider/src/test_utils/mock.rs @@ -368,7 +368,11 @@ impl TransactionsProvider for MockEthProvider { .flat_map(|block| &block.body.transactions) .enumerate() .filter_map(|(tx_number, tx)| { - range.contains(&(tx_number as TxNumber)).then(|| tx.recover_signer()).flatten() + if range.contains(&(tx_number as TxNumber)) { + tx.recover_signer().ok() + } else { + None + } }) .collect(); diff --git a/crates/transaction-pool/src/maintain.rs b/crates/transaction-pool/src/maintain.rs index af8e8da33..e1f4dbf4b 100644 --- a/crates/transaction-pool/src/maintain.rs +++ b/crates/transaction-pool/src/maintain.rs @@ -577,7 +577,7 @@ where let pool_transactions = txs_signed .into_iter() - .filter_map(|tx| tx.try_ecrecovered()) + .filter_map(|tx| tx.try_ecrecovered().ok()) .filter_map(|tx| { // Filter out errors ::try_from_consensus(tx).ok()