From c628cc4fe79954e1bf9ccad4088aa39ed558bf02 Mon Sep 17 00:00:00 2001 From: joshieDo Date: Thu, 28 Mar 2024 14:51:42 +0100 Subject: [PATCH 1/5] add recover_signer_unchecked_with_buffer to TransactionSignedNoHash --- crates/primitives/src/transaction/mod.rs | 27 +++++++++++++++++++++ crates/stages/src/stages/sender_recovery.rs | 8 +++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 92d55b8d82b0..67991e31a0f3 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -856,10 +856,37 @@ impl TransactionSignedNoHash { /// /// Returns `None` if the transaction's signature is invalid, see also [Self::recover_signer]. pub fn recover_signer(&self) -> Option
{ + // Optimism's Deposit transaction does not have a signature. Directly return the + // `from` address. + #[cfg(feature = "optimism")] + if let Transaction::Deposit(TxDeposit { from, .. }) = self.transaction { + return Some(from) + } + let signature_hash = self.signature_hash(); self.signature.recover_signer(signature_hash) } + /// Recover signer from signature and hash _without ensuring that the signature has a low `s` + /// value_. + /// + /// Re-uses a given buffer to avoid numerous reallocations when recovering batches. + /// + /// Returns `None` if the transaction's signature is invalid, see also + /// [Signature::recover_signer_unchecked]. + pub fn recover_signer_unchecked_with_buffer(&self, rlp_buf: &mut Vec) -> Option
{ + // Optimism's Deposit transaction does not have a signature. Directly return the + // `from` address. + #[cfg(feature = "optimism")] + if let Transaction::Deposit(TxDeposit { from, .. }) = self.transaction { + return Some(from) + } + + self.transaction.encode_without_signature(rlp_buf); + + self.signature.recover_signer_unchecked(keccak256(rlp_buf)) + } + /// Converts into a transaction type with its hash: [`TransactionSigned`]. /// /// Note: This will recalculate the hash of the transaction. diff --git a/crates/stages/src/stages/sender_recovery.rs b/crates/stages/src/stages/sender_recovery.rs index 4648f9732443..c85784454640 100644 --- a/crates/stages/src/stages/sender_recovery.rs +++ b/crates/stages/src/stages/sender_recovery.rs @@ -11,7 +11,8 @@ use reth_interfaces::consensus; use reth_primitives::{ keccak256, stage::{EntitiesCheckpoint, StageCheckpoint, StageId}, - Address, PruneSegment, StaticFileSegment, TransactionSignedNoHash, TxNumber, + Address, PruneSegment, StaticFileSegment, Transaction, TransactionSignedNoHash, TxDeposit, + TxNumber, }; use reth_provider::{ BlockReader, DatabaseProviderRW, HeaderProvider, ProviderError, PruneCheckpointReader, @@ -229,16 +230,13 @@ fn recover_sender( (tx_id, tx): (TxNumber, TransactionSignedNoHash), rlp_buf: &mut Vec, ) -> Result<(u64, Address), Box> { - tx.transaction.encode_without_signature(rlp_buf); - // We call [Signature::recover_signer_unchecked] because transactions run in the pipeline are // known to be valid - this means that we do not need to check whether or not the `s` 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 - .signature - .recover_signer_unchecked(keccak256(rlp_buf)) + .recover_signer_unchecked_with_buffer(rlp_buf) .ok_or(SenderRecoveryStageError::FailedRecovery(FailedSenderRecoveryError { tx: tx_id }))?; Ok((tx_id, sender)) From 409c5eb3c87ba475d60ef7f2db5ec78b6bd6a124 Mon Sep 17 00:00:00 2001 From: joshieDo Date: Thu, 28 Mar 2024 14:59:39 +0100 Subject: [PATCH 2/5] clippy --- crates/primitives/src/transaction/mod.rs | 4 ++-- crates/stages/src/stages/sender_recovery.rs | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 67991e31a0f3..910df1d32331 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -868,8 +868,8 @@ impl TransactionSignedNoHash { } /// Recover signer from signature and hash _without ensuring that the signature has a low `s` - /// value_. - /// + /// value_. + /// /// Re-uses a given buffer to avoid numerous reallocations when recovering batches. /// /// Returns `None` if the transaction's signature is invalid, see also diff --git a/crates/stages/src/stages/sender_recovery.rs b/crates/stages/src/stages/sender_recovery.rs index c85784454640..1fb5ca4bf8d7 100644 --- a/crates/stages/src/stages/sender_recovery.rs +++ b/crates/stages/src/stages/sender_recovery.rs @@ -9,10 +9,8 @@ use reth_db::{ }; use reth_interfaces::consensus; use reth_primitives::{ - keccak256, stage::{EntitiesCheckpoint, StageCheckpoint, StageId}, - Address, PruneSegment, StaticFileSegment, Transaction, TransactionSignedNoHash, TxDeposit, - TxNumber, + Address, PruneSegment, StaticFileSegment, TransactionSignedNoHash, TxNumber, }; use reth_provider::{ BlockReader, DatabaseProviderRW, HeaderProvider, ProviderError, PruneCheckpointReader, From 211da78667322084b6d930400dfe904a5c4381b8 Mon Sep 17 00:00:00 2001 From: joshieDo Date: Thu, 28 Mar 2024 15:32:00 +0100 Subject: [PATCH 3/5] clear buffer before use --- crates/primitives/src/transaction/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 910df1d32331..28951ba30ba0 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -870,11 +870,12 @@ impl TransactionSignedNoHash { /// Recover signer from signature and hash _without ensuring that the signature has a low `s` /// value_. /// - /// Re-uses a given buffer to avoid numerous reallocations when recovering batches. + /// Re-uses a given buffer to avoid numerous reallocations when recovering batches. **Clears the + /// buffer before use.** /// /// Returns `None` if the transaction's signature is invalid, see also /// [Signature::recover_signer_unchecked]. - pub fn recover_signer_unchecked_with_buffer(&self, rlp_buf: &mut Vec) -> Option
{ + pub fn recover_signer_unchecked_with_buffer(&self, buffer: &mut Vec) -> Option
{ // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. #[cfg(feature = "optimism")] @@ -882,9 +883,10 @@ impl TransactionSignedNoHash { return Some(from) } - self.transaction.encode_without_signature(rlp_buf); + buffer.clear(); + self.transaction.encode_without_signature(buffer); - self.signature.recover_signer_unchecked(keccak256(rlp_buf)) + self.signature.recover_signer_unchecked(keccak256(buffer)) } /// Converts into a transaction type with its hash: [`TransactionSigned`]. From 39f0827212b13cad94f07625d1ae3edebe49a527 Mon Sep 17 00:00:00 2001 From: joshieDo Date: Thu, 28 Mar 2024 17:19:26 +0100 Subject: [PATCH 4/5] rename to encode_and_recover_unchecked --- crates/primitives/src/transaction/mod.rs | 2 +- crates/stages/src/stages/sender_recovery.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 28951ba30ba0..d36f7e04b8da 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -875,7 +875,7 @@ impl TransactionSignedNoHash { /// /// Returns `None` if the transaction's signature is invalid, see also /// [Signature::recover_signer_unchecked]. - pub fn recover_signer_unchecked_with_buffer(&self, buffer: &mut Vec) -> Option
{ + pub fn encode_and_recover_unchecked(&self, buffer: &mut Vec) -> Option
{ // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. #[cfg(feature = "optimism")] diff --git a/crates/stages/src/stages/sender_recovery.rs b/crates/stages/src/stages/sender_recovery.rs index 1fb5ca4bf8d7..afb65c560605 100644 --- a/crates/stages/src/stages/sender_recovery.rs +++ b/crates/stages/src/stages/sender_recovery.rs @@ -234,7 +234,7 @@ fn recover_sender( // large `s` values, so using [Signature::recover_signer] here would not be // backwards-compatible. let sender = tx - .recover_signer_unchecked_with_buffer(rlp_buf) + .encode_and_recover_unchecked(rlp_buf) .ok_or(SenderRecoveryStageError::FailedRecovery(FailedSenderRecoveryError { tx: tx_id }))?; Ok((tx_id, sender)) From 36cbac50e9f5038acd4cc12666c596c5e01006ca Mon Sep 17 00:00:00 2001 From: joshieDo Date: Thu, 28 Mar 2024 22:38:29 +0100 Subject: [PATCH 5/5] encode TxDeposit on encode_and_recover_unchecked --- crates/primitives/src/transaction/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index d36f7e04b8da..426607583221 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -876,6 +876,9 @@ impl TransactionSignedNoHash { /// Returns `None` if the transaction's signature is invalid, see also /// [Signature::recover_signer_unchecked]. pub fn encode_and_recover_unchecked(&self, buffer: &mut Vec) -> Option
{ + buffer.clear(); + self.transaction.encode_without_signature(buffer); + // Optimism's Deposit transaction does not have a signature. Directly return the // `from` address. #[cfg(feature = "optimism")] @@ -883,9 +886,6 @@ impl TransactionSignedNoHash { return Some(from) } - buffer.clear(); - self.transaction.encode_without_signature(buffer); - self.signature.recover_signer_unchecked(keccak256(buffer)) }