feat: unify recover fn result type (paradigmxyz#13897)

Co-authored-by: Matthias Seitz <[email protected]>

bin/reth/src/commands/debug_cmd/build_block.rs

@@ -167,7 +167,7 @@ impl<C: ChainSpecParser<ChainSpec = ChainSpec>> Command<C> {
             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, .. }) => {

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))?;
         }
 

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<Address> {
+    fn recover_signer(&self) -> Result<Address, RecoveryError> {
         let signature_hash = self.signature_hash();
         recover_signer(&self.signature, signature_hash)
     }
 
-    fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec<u8>) -> Option<Address> {
+    fn recover_signer_unchecked_with_buf(
+        &self,
+        buf: &mut Vec<u8>,
+    ) -> Result<Address, RecoveryError> {
         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"));

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,35 +79,38 @@ impl SignedTransaction for OpTransactionSigned {
         &self.signature
     }
 
-    fn recover_signer(&self) -> Option<Address> {
+    fn recover_signer(&self) -> Result<Address, RecoveryError> {
         // 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;
         let signature_hash = signature_hash(transaction);
         recover_signer(signature, signature_hash)
     }
 
-    fn recover_signer_unchecked(&self) -> Option<Address> {
+    fn recover_signer_unchecked(&self) -> Result<Address, RecoveryError> {
         // 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;
         let signature_hash = signature_hash(transaction);
         recover_signer_unchecked(signature, signature_hash)
     }
 
-    fn recover_signer_unchecked_with_buf(&self, buf: &mut Vec<u8>) -> Option<Address> {
+    fn recover_signer_unchecked_with_buf(
+        &self,
+        buf: &mut Vec<u8>,
+    ) -> Result<Address, RecoveryError> {
         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),

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<Vec<Address>>
+    fn recover_signers(&self) -> Result<Vec<Address>, 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<Vec<Address>>
+    fn recover_signers_unchecked(&self) -> Result<Vec<Address>, 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()
     }
 }
 

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<Vec<Address>>
+    fn senders(&self) -> Result<Vec<Address>, RecoveryError>
     where
         <Self::Body as BlockBody>::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
         <Self::Body as BlockBody>::Transaction: SignedTransaction,
     {
-        let senders = self.senders()?;
+        let senders = self.body().recover_signers().ok()?;
         Some(RecoveredBlock::new_unhashed(self, senders))
     }
 }

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<B: Block> SealedBlock<B> {
     /// 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<Vec<Address>> {
+    pub fn senders(&self) -> Result<Vec<Address>, RecoveryError> {
         self.body().recover_signers()
     }
 

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<Address> {
+    pub fn recover_signer_unchecked(
+        signature: &Signature,
+        hash: B256,
+    ) -> Result<Address, RecoveryError> {
         let mut sig: [u8; 65] = [0; 65];
 
         sig[0..32].copy_from_slice(&signature.r().to_be_bytes::<32>());
@@ -39,19 +43,18 @@ 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
     /// greater than `secp256k1n / 2`, as specified in
     /// [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<Address> {
+    pub fn recover_signer(signature: &Signature, hash: B256) -> Result<Address, RecoveryError> {
         if signature.s() > SECP256K1N_HALF {
-            return None
+            return Err(RecoveryError)
         }
-
         recover_signer_unchecked(signature, hash)
     }
 }

crates/primitives-traits/src/transaction/recover.rs

@@ -8,15 +8,15 @@ 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};
 
     /// 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<Vec<Address>>
+    pub fn recover_signers<'a, I, T>(txes: I) -> Result<Vec<Address>, RecoveryError>
     where
         T: SignedTransaction,
         I: IntoParallelIterator<Item = &'a T> + IntoIterator<Item = &'a T> + 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<Vec<Address>>
+    pub fn recover_signers_unchecked<'a, I, T>(txes: I) -> Result<Vec<Address>, RecoveryError>
     where
         T: SignedTransaction,
         I: IntoParallelIterator<Item = &'a T> + IntoIterator<Item = &'a T> + 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<Vec<Address>>
+    /// Returns `Err(RecoveryError)`, if some transaction's signature is invalid
+    pub fn recover_signers<'a, I, T>(txes: I) -> Result<Vec<Address>, RecoveryError>
     where
         T: SignedTransaction,
         I: IntoIterator<Item = &'a T> + IntoIterator<Item = &'a T>,
@@ -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<Vec<Address>>
+    /// Returns `Err(RecoveryError)`, if some transaction's signature is invalid.
+    pub fn recover_signers_unchecked<'a, I, T>(txes: I) -> Result<Vec<Address>, RecoveryError>
     where
         T: SignedTransaction,
         I: IntoIterator<Item = &'a T> + IntoIterator<Item = &'a T>,