feat: unify recover fn result type (#13897)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

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,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<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,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<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>,

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<Address>;
+    fn recover_signer(&self) -> Result<Address, RecoveryError>;
 
     /// Recover signer from signature and hash.
     ///
     /// Returns an error if the transaction's signature is invalid.
     fn try_recover(&self) -> Result<Address, RecoveryError> {
-        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<Address> {
-        self.recover_signer_unchecked_with_buf(&mut Vec::new())
+    fn recover_signer_unchecked(&self) -> Result<Address, RecoveryError> {
+        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<Address, RecoveryError> {
-        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<u8>) -> Option<Address>;
+    fn recover_signer_unchecked_with_buf(
+        &self,
+        buf: &mut Vec<u8>,
+    ) -> Result<Address, RecoveryError>;
 
     /// 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<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> {
         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<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> {
         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<Recovered<Self>> {
-        let signer = self.recover_signer()?;
-        Some(Recovered::new_unchecked(self.clone(), signer))
+    fn try_ecrecovered(&self) -> Result<Recovered<Self>, 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<Recovered<Self>, 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<Recovered<Self>> {
-        let signer = self.recover_signer_unchecked()?;
-        Some(Recovered::new_unchecked(self, signer))
+    fn into_ecrecovered_unchecked(self) -> Result<Recovered<Self>, RecoveryError> {
+        self.recover_signer_unchecked().map(|signer| Recovered::new_unchecked(self, signer))
     }
 
     /// Returns the [`Recovered`] transaction with the given sender.