Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use unchecked sender recovery when loading from disk #5919

Merged
merged 1 commit into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions crates/primitives/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,37 @@ impl Block {
///
/// 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]
pub fn with_senders_unchecked(self, senders: Vec<Address>) -> BlockWithSenders {
self.try_with_senders_unchecked(senders).expect("stored block is valid")
}

/// Transform into a [`BlockWithSenders`] 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_.
/// See also [TransactionSigned::recover_signer_unchecked]
///
/// Returns an error if a signature is invalid.
#[track_caller]
pub fn with_senders(self, senders: Vec<Address>) -> BlockWithSenders {
pub fn try_with_senders_unchecked(
self,
senders: Vec<Address>,
) -> Result<BlockWithSenders, Self> {
let senders = if self.body.len() == senders.len() {
senders
} else {
TransactionSigned::recover_signers(&self.body, self.body.len())
.expect("stored block is valid")
let Some(senders) =
TransactionSigned::recover_signers_unchecked(&self.body, self.body.len())
else {
return Err(self);
};
senders
};

BlockWithSenders { block: self, senders }
Ok(BlockWithSenders { block: self, senders })
}

/// **Expensive**. Transform into a [`BlockWithSenders`] by recovering senders in the contained
Expand Down
18 changes: 17 additions & 1 deletion crates/primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ impl TransactionSigned {
self.signature.recover_signer_unchecked(signature_hash)
}

/// Recovers a list of signers from a transaction list iterator
/// Recovers a list of signers from a transaction list iterator.
///
/// Returns `None`, if some transaction's signature is invalid, see also
/// [Self::recover_signer].
Expand All @@ -1011,6 +1011,22 @@ impl TransactionSigned {
}
}

/// 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, see also
/// [Self::recover_signer_unchecked].
pub fn recover_signers_unchecked<'a, T>(txes: T, num_txes: usize) -> Option<Vec<Address>>
where
T: IntoParallelIterator<Item = &'a Self> + IntoIterator<Item = &'a Self> + Send,
{
if num_txes < *PARALLEL_SENDER_RECOVERY_THRESHOLD {
txes.into_iter().map(|tx| tx.recover_signer_unchecked()).collect()
} else {
txes.into_par_iter().map(|tx| tx.recover_signer_unchecked()).collect()
}
}

/// Returns the [TransactionSignedEcRecovered] transaction with the given sender.
#[inline]
pub const fn with_signer(self, signer: Address) -> TransactionSignedEcRecovered {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,14 @@ impl<TX: DbTx> BlockReader for DatabaseProvider<TX> {
})
.collect();

Ok(Some(Block { header, body, ommers, withdrawals }.with_senders(senders)))
let block = Block { header, body, ommers, withdrawals };
let block = block
// 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)
.map_err(|_| ProviderError::SenderRecoveryError)?;

Ok(Some(block))
}

fn block_range(&self, range: RangeInclusive<BlockNumber>) -> ProviderResult<Vec<Block>> {
Expand Down
Loading