Skip to content

Commit

Permalink
chore!:unify trait fn naming for recovery (paradigmxyz#13981)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattsse authored Jan 24, 2025
1 parent 61ae871 commit 5dac5cf
Show file tree
Hide file tree
Showing 16 changed files with 56 additions and 58 deletions.
4 changes: 2 additions & 2 deletions bin/reth/src/commands/debug_cmd/build_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl<C: ChainSpecParser<ChainSpec = ChainSpec>> Command<C> {
provider
.block(best_number.into())?
.expect("the header for the latest block is missing, database is corrupt")
.seal(best_hash),
.seal_unchecked(best_hash),
))
}

Expand Down Expand Up @@ -166,7 +166,7 @@ impl<C: ChainSpecParser<ChainSpec = ChainSpec>> Command<C> {
for tx_bytes in &self.transactions {
debug!(target: "reth::cli", bytes = ?tx_bytes, "Decoding transaction");
let transaction = TransactionSigned::decode(&mut &Bytes::from_str(tx_bytes)?[..])?
.try_ecrecovered()
.try_clone_into_recovered()
.map_err(|e| eyre::eyre!("failed to recover tx: {e}"))?;

let encoded_length = match &transaction.transaction {
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/util/src/reorg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ where
}

// Configure the environment for the block.
let tx_recovered = tx.clone().try_into_ecrecovered().map_err(|_| {
let tx_recovered = tx.try_clone_into_recovered().map_err(|_| {
BlockExecutionError::Validation(BlockValidationError::SenderRecoveryError)
})?;
let tx_env = evm_config.tx_env(&tx_recovered, tx_recovered.signer());
Expand Down
4 changes: 2 additions & 2 deletions crates/ethereum/evm/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ mod tests {
let BlockExecutionOutput { receipts, requests, .. } = executor
.execute(
&Block { header, body: BlockBody { transactions: vec![tx], ..Default::default() } }
.with_recovered_senders()
.try_into_recovered()
.unwrap(),
)
.unwrap();
Expand Down Expand Up @@ -1074,7 +1074,7 @@ mod tests {
// Execute the block and capture the result
let exec_result = executor.execute(
&Block { header, body: BlockBody { transactions: vec![tx], ..Default::default() } }
.with_recovered_senders()
.try_into_recovered()
.unwrap(),
);

Expand Down
7 changes: 2 additions & 5 deletions crates/exex/exex/src/backfill/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::sync::Arc;
use alloy_consensus::{constants::ETH_TO_WEI, BlockHeader, Header, TxEip2930};
use alloy_genesis::{Genesis, GenesisAccount};
use alloy_primitives::{b256, Address, TxKind, U256};
use eyre::OptionExt;
use reth_chainspec::{ChainSpec, ChainSpecBuilder, EthereumHardfork, MAINNET, MIN_TRANSACTION_GAS};
use reth_evm::execute::{BatchExecutor, BlockExecutionOutput, BlockExecutorProvider, Executor};
use reth_evm_ethereum::execute::EthExecutorProvider;
Expand Down Expand Up @@ -121,8 +120,7 @@ fn blocks(
..Default::default()
},
}
.with_recovered_senders()
.ok_or_eyre("failed to recover senders")?;
.try_into_recovered()?;

// Second block resends the same transaction with increased nonce
let block2 = Block {
Expand Down Expand Up @@ -153,8 +151,7 @@ fn blocks(
..Default::default()
},
}
.with_recovered_senders()
.ok_or_eyre("failed to recover senders")?;
.try_into_recovered()?;

Ok((block1, block2))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/net/network/src/transactions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ where
let mut new_txs = Vec::with_capacity(transactions.len());
for tx in transactions {
// recover transaction
let tx = match tx.try_into_ecrecovered() {
let tx = match tx.try_into_recovered() {
Ok(tx) => tx,
Err(badtx) => {
trace!(target: "net::tx",
Expand Down
7 changes: 3 additions & 4 deletions crates/optimism/payload/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,10 +789,9 @@ where
// purely for the purposes of utilizing the `evm_config.tx_env`` function.
// Deposit transactions do not have signatures, so if the tx is a deposit, this
// will just pull in its `from` address.
let sequencer_tx =
sequencer_tx.value().clone().try_into_ecrecovered().map_err(|_| {
PayloadBuilderError::other(OpPayloadBuilderError::TransactionEcRecoverFailed)
})?;
let sequencer_tx = sequencer_tx.value().try_clone_into_recovered().map_err(|_| {
PayloadBuilderError::other(OpPayloadBuilderError::TransactionEcRecoverFailed)
})?;

// Cache the depositor account prior to the state transition for the deposit nonce.
//
Expand Down
58 changes: 29 additions & 29 deletions crates/primitives-traits/src/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use alloy_primitives::{Address, B256};
use alloy_rlp::{Decodable, Encodable};

use crate::{
transaction::signed::RecoveryError, BlockBody, BlockHeader, FullBlockBody, FullBlockHeader,
InMemorySize, MaybeSerde, SealedHeader, SignedTransaction,
block::error::BlockRecoveryError, transaction::signed::RecoveryError, BlockBody, BlockHeader,
FullBlockBody, FullBlockHeader, InMemorySize, MaybeSerde, SealedHeader, SignedTransaction,
};

/// Bincode-compatible header type serde implementations.
Expand Down Expand Up @@ -81,7 +81,7 @@ pub trait Block:
/// Seal the block with a known hash.
///
/// WARNING: This method does not perform validation whether the hash is correct.
fn seal(self, hash: B256) -> SealedBlock<Self> {
fn seal_unchecked(self, hash: B256) -> SealedBlock<Self> {
SealedBlock::new_unchecked(self, hash)
}

Expand Down Expand Up @@ -121,60 +121,60 @@ pub trait Block:
}

/// Expensive operation that recovers transaction signer.
fn senders(&self) -> Result<Vec<Address>, RecoveryError>
fn recover_signers(&self) -> Result<Vec<Address>, RecoveryError>
where
<Self::Body as BlockBody>::Transaction: SignedTransaction,
{
self.body().recover_signers()
}

/// Transform into a [`RecoveredBlock`].
///
/// # Panics
///
/// If the number of senders does not match the number of transactions in the block
/// and the signer recovery for one of the transactions fails.
///
/// Note: this is expected to be called with blocks read from disk.
#[track_caller]
fn with_senders_unchecked(self, senders: Vec<Address>) -> RecoveredBlock<Self>
where
<Self::Body as BlockBody>::Transaction: SignedTransaction,
{
self.try_with_senders_unchecked(senders).expect("stored block is valid")
}

/// Transform into a [`RecoveredBlock`] using the given senders.
/// Transform the block into a [`RecoveredBlock`] using the given senders.
///
/// If the number of senders does not match the number of transactions in the block, this falls
/// back to manually recovery, but _without ensuring that the signature has a low `s` value_.
///
/// Returns an error if a signature is invalid.
#[track_caller]
fn try_with_senders_unchecked(self, senders: Vec<Address>) -> Result<RecoveredBlock<Self>, Self>
/// Returns the block as error if a signature is invalid.
fn try_into_recovered_unchecked(
self,
senders: Vec<Address>,
) -> Result<RecoveredBlock<Self>, BlockRecoveryError<Self>>
where
<Self::Body as BlockBody>::Transaction: SignedTransaction,
{
let senders = if self.body().transactions().len() == senders.len() {
senders
} else {
// Fall back to recovery if lengths don't match
let Ok(senders) = self.body().recover_signers_unchecked() else { return Err(self) };
let Ok(senders) = self.body().recover_signers_unchecked() else {
return Err(BlockRecoveryError::new(self))
};
senders
};
Ok(RecoveredBlock::new_unhashed(self, senders))
}

/// Transform the block into a [`RecoveredBlock`] using the given signers.
///
/// Note: This method assumes the signers are correct and does not validate them.
fn into_recovered_with_signers(self, signers: Vec<Address>) -> RecoveredBlock<Self>
where
<Self::Body as BlockBody>::Transaction: SignedTransaction,
{
RecoveredBlock::new_unhashed(self, signers)
}

/// **Expensive**. Transform into a [`RecoveredBlock`] by recovering senders in the contained
/// transactions.
///
/// Returns `None` if a transaction is invalid.
fn with_recovered_senders(self) -> Option<RecoveredBlock<Self>>
/// Returns the block as error if a signature is invalid.
fn try_into_recovered(self) -> Result<RecoveredBlock<Self>, BlockRecoveryError<Self>>
where
<Self::Body as BlockBody>::Transaction: SignedTransaction,
{
let senders = self.body().recover_signers().ok()?;
Some(RecoveredBlock::new_unhashed(self, senders))
let Ok(signers) = self.body().recover_signers() else {
return Err(BlockRecoveryError::new(self))
};
Ok(RecoveredBlock::new_unhashed(self, signers))
}
}

Expand Down
8 changes: 5 additions & 3 deletions crates/primitives-traits/src/transaction/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ 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) -> Result<Recovered<Self>, RecoveryError> {
fn try_clone_into_recovered(&self) -> Result<Recovered<Self>, RecoveryError> {
self.recover_signer().map(|signer| Recovered::new_unchecked(self.clone(), signer))
}

/// Tries to recover signer and return [`Recovered`].
///
/// Returns `Err(Self)` if the transaction's signature is invalid, see also
/// [`SignedTransaction::recover_signer`].
fn try_into_ecrecovered(self) -> Result<Recovered<Self>, Self> {
fn try_into_recovered(self) -> Result<Recovered<Self>, Self> {
match self.recover_signer() {
Ok(signer) => Ok(Recovered::new_unchecked(self, signer)),
Err(_) => Err(self),
Expand All @@ -206,11 +206,13 @@ 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) -> Result<Recovered<Self>, RecoveryError> {
fn into_recovered_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.
///
/// Note: assumes the given signer is the signer of this transaction.
fn with_signer(self, signer: Address) -> Recovered<Self> {
Recovered::new_unchecked(self, signer)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-eth-api/src/helpers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ pub trait LoadTransaction: SpawnBlocking + FullEthApiTypes + RpcNodeCoreExt {
// part of pending block) and already. We don't need to
// check for pre EIP-2 because this transaction could be pre-EIP-2.
let transaction = tx
.into_ecrecovered_unchecked()
.into_recovered_unchecked()
.map_err(|_| EthApiError::InvalidTransactionSignature)?;

let tx = TransactionSource::Block {
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-eth-types/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub fn recover_raw_transaction<T: SignedTransaction>(mut data: &[u8]) -> EthResu
let transaction =
T::decode_2718(&mut data).map_err(|_| EthApiError::FailedToDecodeSignedTransaction)?;

transaction.try_into_ecrecovered().or(Err(EthApiError::InvalidTransactionSignature))
transaction.try_into_recovered().or(Err(EthApiError::InvalidTransactionSignature))
}

/// Performs a binary search within a given block range to find the desired block number.
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ where
.collect()
};

self.trace_block(Arc::new(block.with_senders_unchecked(senders)), evm_env, opts).await
self.trace_block(Arc::new(block.into_recovered_with_signers(senders)), evm_env, opts).await
}

/// Replays a block and returns the trace of each transaction.
Expand Down
4 changes: 2 additions & 2 deletions crates/storage/provider/src/providers/database/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ impl<TX: DbTx + 'static, N: NodeTypesForProvider> BlockReader for DatabaseProvid
// Note: we're using unchecked here because we know the block contains valid txs
// wrt to its height and can ignore the s value check so pre
// EIP-2 txs are allowed
.try_with_senders_unchecked(senders)
.try_into_recovered_unchecked(senders)
.map(Some)
.map_err(|_| ProviderError::SenderRecoveryError)
},
Expand Down Expand Up @@ -1274,7 +1274,7 @@ impl<TX: DbTx + 'static, N: NodeTypesForProvider> BlockReader for DatabaseProvid
|range| self.headers_range(range),
|header, body, senders| {
Self::Block::new(header, body)
.try_with_senders_unchecked(senders)
.try_into_recovered_unchecked(senders)
.map_err(|_| ProviderError::SenderRecoveryError)
},
)
Expand Down
4 changes: 2 additions & 2 deletions crates/transaction-pool/src/maintain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ where

let pool_transactions = txs_signed
.into_iter()
.filter_map(|tx| tx.try_ecrecovered().ok())
.filter_map(|tx| tx.try_clone_into_recovered().ok())
.filter_map(|tx| {
// Filter out errors
<P::Transaction as PoolTransaction>::try_from_consensus(tx).ok()
Expand Down Expand Up @@ -695,7 +695,7 @@ mod tests {
let tx_bytes = hex!("02f87201830655c2808505ef61f08482565f94388c818ca8b9251b393131c08a736a67ccb192978801049e39c4b5b1f580c001a01764ace353514e8abdfb92446de356b260e3c1225b73fc4c8876a6258d12a129a04f02294aa61ca7676061cd99f29275491218b4754b46a0248e5e42bc5091f507");
let tx = PooledTransaction::decode_2718(&mut &tx_bytes[..]).unwrap();
let provider = MockEthProvider::default();
let transaction: EthPooledTransaction = tx.try_into_ecrecovered().unwrap().into();
let transaction: EthPooledTransaction = tx.try_into_recovered().unwrap().into();
let tx_to_cmp = transaction.clone();
let sender = hex!("1f9090aaE28b8a3dCeaDf281B0F12828e676c326").into();
provider.add_account(sender, ExtendedAccount::new(42, U256::MAX));
Expand Down
4 changes: 2 additions & 2 deletions crates/transaction-pool/src/test_utils/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ impl<R: Rng> TransactionGenerator<R> {

/// Generates and returns a pooled EIP-1559 transaction with a random signer.
pub fn gen_eip1559_pooled(&mut self) -> EthPooledTransaction {
self.gen_eip1559().try_into_ecrecovered().unwrap().try_into().unwrap()
self.gen_eip1559().try_into_recovered().unwrap().try_into().unwrap()
}

/// Generates and returns a pooled EIP-4844 transaction with a random signer.
pub fn gen_eip4844_pooled(&mut self) -> EthPooledTransaction {
let tx = self.gen_eip4844().try_into_ecrecovered().unwrap();
let tx = self.gen_eip4844().try_into_recovered().unwrap();
let encoded_length = tx.encode_2718_len();
EthPooledTransaction::new(tx, encoded_length)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/transaction-pool/src/validate/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ mod tests {
let data = hex::decode(raw).unwrap();
let tx = PooledTransaction::decode_2718(&mut data.as_ref()).unwrap();

tx.try_into_ecrecovered().unwrap().into()
tx.try_into_recovered().unwrap().into()
}

// <https://github.com/paradigmxyz/reth/issues/5178>
Expand Down
2 changes: 1 addition & 1 deletion examples/custom-payload-builder/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ where
.ok_or_else(|| PayloadBuilderError::MissingParentBlock(attributes.parent()))?;

// we already know the hash, so we can seal it
block.seal(attributes.parent())
block.seal_unchecked(attributes.parent())
};
let hash = parent_block.hash();
let header = SealedHeader::new(parent_block.header().clone(), hash);
Expand Down

0 comments on commit 5dac5cf

Please sign in to comment.