From 20e003f9b4f20ecfdea4a11f9628687835c020f9 Mon Sep 17 00:00:00 2001 From: bendanzhentan <455462586@qq.com> Date: Tue, 7 Jan 2025 01:20:58 +0800 Subject: [PATCH] feat(primitives): re-export alloy Recovered (#13670) --- crates/optimism/node/src/txpool.rs | 6 +- crates/optimism/node/tests/it/priority.rs | 2 +- crates/primitives/src/lib.rs | 6 +- crates/primitives/src/transaction/mod.rs | 129 ++---------------- crates/primitives/src/transaction/pooled.rs | 42 +----- .../transaction-pool/src/test_utils/mock.rs | 7 +- crates/transaction-pool/src/traits.rs | 26 ++-- 7 files changed, 39 insertions(+), 179 deletions(-) diff --git a/crates/optimism/node/src/txpool.rs b/crates/optimism/node/src/txpool.rs index 79f41f9c0..752a78405 100644 --- a/crates/optimism/node/src/txpool.rs +++ b/crates/optimism/node/src/txpool.rs @@ -60,7 +60,7 @@ impl TryFrom> for OpPooledTransaction { fn try_from(value: RecoveredTx) -> Result { let (tx, signer) = value.into_parts(); let pooled: RecoveredTx = - RecoveredTx::from_signed_transaction(tx.try_into()?, signer); + RecoveredTx::new_unchecked(tx.try_into()?, signer); Ok(pooled.into()) } } @@ -84,7 +84,7 @@ impl PoolTransaction for OpPooledTransaction { tx: RecoveredTx, ) -> Result, Self::TryFromConsensusError> { let (tx, signer) = tx.into_parts(); - Ok(RecoveredTx::from_signed_transaction(tx.try_into()?, signer)) + Ok(RecoveredTx::new_unchecked(tx.try_into()?, signer)) } fn hash(&self) -> &TxHash { @@ -459,7 +459,7 @@ mod tests { }); let signature = Signature::test_signature(); let signed_tx = OpTransactionSigned::new_unhashed(deposit_tx, signature); - let signed_recovered = RecoveredTx::from_signed_transaction(signed_tx, signer); + let signed_recovered = RecoveredTx::new_unchecked(signed_tx, signer); let len = signed_recovered.encode_2718_len(); let pooled_tx = OpPooledTransaction::new(signed_recovered, len); let outcome = validator.validate_one(origin, pooled_tx); diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 510cd5cdb..d34dac483 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -67,7 +67,7 @@ impl OpPayloadTransactions for CustomTxPriority { ..Default::default() }; let signature = sender.sign_transaction_sync(&mut end_of_block_tx).unwrap(); - let end_of_block_tx = RecoveredTx::from_signed_transaction( + let end_of_block_tx = RecoveredTx::new_unchecked( OpTransactionSigned::new_unhashed( OpTypedTransaction::Eip1559(end_of_block_tx), signature, diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index d44391de7..4667689ae 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -44,13 +44,13 @@ pub use reth_primitives_traits::{ pub use static_file::StaticFileSegment; pub use alloy_consensus::{ - transaction::{PooledTransaction, TransactionMeta}, + transaction::{PooledTransaction, Recovered as RecoveredTx, TransactionMeta}, ReceiptWithBloom, }; pub use transaction::{ util::secp256k1::{public_key_to_address, recover_signer_unchecked, sign_message}, - InvalidTransactionError, PooledTransactionsElementEcRecovered, RecoveredTx, Transaction, - TransactionSigned, TransactionSignedEcRecovered, TxType, + InvalidTransactionError, PooledTransactionsElementEcRecovered, Transaction, TransactionSigned, + TransactionSignedEcRecovered, TxType, }; // Re-exports diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index a805c4c23..e338592f3 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -1,5 +1,6 @@ //! Transaction types. +use crate::RecoveredTx; use alloc::vec::Vec; pub use alloy_consensus::transaction::PooledTransaction; use alloy_consensus::{ @@ -16,7 +17,7 @@ use alloy_eips::{ use alloy_primitives::{ keccak256, Address, Bytes, ChainId, PrimitiveSignature as Signature, TxHash, TxKind, B256, U256, }; -use alloy_rlp::{Decodable, Encodable, Error as RlpError, Header}; +use alloy_rlp::{Decodable, Encodable, Header}; use core::hash::{Hash, Hasher}; use derive_more::{AsRef, Deref}; use once_cell as _; @@ -912,7 +913,7 @@ impl TransactionSigned { /// Returns the [`RecoveredTx`] transaction with the given sender. #[inline] pub const fn with_signer(self, signer: Address) -> RecoveredTx { - RecoveredTx::from_signed_transaction(self, signer) + RecoveredTx::new_unchecked(self, signer) } /// Consumes the type, recover signer and return [`RecoveredTx`] @@ -920,7 +921,7 @@ impl TransactionSigned { /// Returns `None` if the transaction's signature is invalid, see also [`Self::recover_signer`]. pub fn into_ecrecovered(self) -> Option> { let signer = self.recover_signer()?; - Some(RecoveredTx { signed_transaction: self, signer }) + Some(RecoveredTx::new_unchecked(self, signer)) } /// Consumes the type, recover signer and return [`RecoveredTx`] _without @@ -930,7 +931,7 @@ impl TransactionSigned { /// [`Self::recover_signer_unchecked`]. pub fn into_ecrecovered_unchecked(self) -> Option> { let signer = self.recover_signer_unchecked()?; - Some(RecoveredTx { signed_transaction: self, signer }) + Some(RecoveredTx::new_unchecked(self, signer)) } /// Tries to recover signer and return [`RecoveredTx`]. _without ensuring that @@ -941,7 +942,7 @@ impl TransactionSigned { pub fn try_into_ecrecovered_unchecked(self) -> Result, Self> { match self.recover_signer_unchecked() { None => Err(self), - Some(signer) => Ok(RecoveredTx { signed_transaction: self, signer }), + Some(signer) => Ok(RecoveredTx::new_unchecked(self, signer)), } } @@ -1184,13 +1185,13 @@ impl alloy_consensus::Transaction for TransactionSigned { impl From> for TransactionSigned { fn from(recovered: RecoveredTx) -> Self { - recovered.signed_transaction + recovered.into_tx() } } impl From> for TransactionSigned { fn from(recovered: RecoveredTx) -> Self { - recovered.signed_transaction.into() + recovered.into_tx().into() } } @@ -1521,105 +1522,12 @@ impl<'a> arbitrary::Arbitrary<'a> for TransactionSigned { /// Type alias kept for backward compatibility. pub type TransactionSignedEcRecovered = RecoveredTx; -/// Signed transaction with recovered signer. -#[derive(Debug, Clone, PartialEq, Hash, Eq, AsRef, Deref)] -pub struct RecoveredTx { - /// Signer of the transaction - signer: Address, - /// Signed transaction - #[deref] - #[as_ref] - signed_transaction: T, -} - -// === impl RecoveredTx === - -impl RecoveredTx { - /// Signer of transaction recovered from signature - pub const fn signer(&self) -> Address { - self.signer - } - - /// Reference to the signer of transaction recovered from signature - pub const fn signer_ref(&self) -> &Address { - &self.signer - } - - /// Returns a reference to [`TransactionSigned`] - pub const fn tx(&self) -> &T { - &self.signed_transaction - } - - /// Transform back to [`TransactionSigned`] - pub fn into_tx(self) -> T { - self.signed_transaction - } - - /// Dissolve Self to its component - pub fn into_parts(self) -> (T, Address) { - (self.signed_transaction, self.signer) - } - - /// Create [`RecoveredTx`] from [`TransactionSigned`] and [`Address`] of the - /// signer. - #[inline] - pub const fn from_signed_transaction(signed_transaction: T, signer: Address) -> Self { - Self { signed_transaction, signer } - } - - /// Applies the given closure to the inner transactions. - pub fn map_transaction(self, f: impl FnOnce(T) -> Tx) -> RecoveredTx { - RecoveredTx::from_signed_transaction(f(self.signed_transaction), self.signer) - } -} - -impl Encodable for RecoveredTx { - /// This encodes the transaction _with_ the signature, and an rlp header. - /// - /// Refer to docs for [`TransactionSigned::encode`] for details on the exact format. - fn encode(&self, out: &mut dyn bytes::BufMut) { - self.signed_transaction.encode(out) - } - - fn length(&self) -> usize { - self.signed_transaction.length() - } -} - -impl Decodable for RecoveredTx { - fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { - let signed_transaction = T::decode(buf)?; - let signer = signed_transaction - .recover_signer() - .ok_or(RlpError::Custom("Unable to recover decoded transaction signer."))?; - Ok(Self { signer, signed_transaction }) - } -} - -impl Encodable2718 for RecoveredTx { - fn type_flag(&self) -> Option { - self.signed_transaction.type_flag() - } - - fn encode_2718_len(&self) -> usize { - self.signed_transaction.encode_2718_len() - } - - fn encode_2718(&self, out: &mut dyn alloy_rlp::BufMut) { - self.signed_transaction.encode_2718(out) - } - - fn trie_hash(&self) -> B256 { - self.signed_transaction.trie_hash() - } -} - /// Extension trait for [`SignedTransaction`] to convert it into [`RecoveredTx`]. pub trait SignedTransactionIntoRecoveredExt: SignedTransaction { /// Tries to recover signer and return [`RecoveredTx`] by cloning the type. fn try_ecrecovered(&self) -> Option> { let signer = self.recover_signer()?; - Some(RecoveredTx { signed_transaction: self.clone(), signer }) + Some(RecoveredTx::new_unchecked(self.clone(), signer)) } /// Tries to recover signer and return [`RecoveredTx`]. @@ -1629,7 +1537,7 @@ pub trait SignedTransactionIntoRecoveredExt: SignedTransaction { fn try_into_ecrecovered(self) -> Result, Self> { match self.recover_signer() { None => Err(self), - Some(signer) => Ok(RecoveredTx { signed_transaction: self, signer }), + Some(signer) => Ok(RecoveredTx::new_unchecked(self, signer)), } } @@ -1639,12 +1547,12 @@ pub trait SignedTransactionIntoRecoveredExt: SignedTransaction { /// Returns `None` if the transaction's signature is invalid. fn into_ecrecovered_unchecked(self) -> Option> { let signer = self.recover_signer_unchecked()?; - Some(RecoveredTx::from_signed_transaction(self, signer)) + Some(RecoveredTx::new_unchecked(self, signer)) } /// Returns the [`RecoveredTx`] transaction with the given sender. fn with_signer(self, signer: Address) -> RecoveredTx { - RecoveredTx::from_signed_transaction(self, signer) + RecoveredTx::new_unchecked(self, signer) } } @@ -1895,7 +1803,7 @@ where mod tests { use crate::{ transaction::{TxEip1559, TxKind, TxLegacy}, - RecoveredTx, Transaction, TransactionSigned, + Transaction, TransactionSigned, }; use alloy_consensus::Transaction as _; use alloy_eips::eip2718::{Decodable2718, Encodable2718}; @@ -2148,17 +2056,6 @@ mod tests { assert_eq!(encoded, input); } - #[test] - fn test_decode_signed_ec_recovered_transaction() { - // random tx: - let input = hex!("02f871018302a90f808504890aef60826b6c94ddf4c5025d1a5742cf12f74eec246d4432c295e487e09c3bbcc12b2b80c080a0f21a4eacd0bf8fea9c5105c543be5a1d8c796516875710fafafdf16d16d8ee23a001280915021bb446d1973501a67f93d2b38894a514b976e7b46dc2fe54598d76"); - let tx = TransactionSigned::decode(&mut &input[..]).unwrap(); - let recovered = tx.into_ecrecovered().unwrap(); - - let decoded = RecoveredTx::decode(&mut &alloy_rlp::encode(&recovered)[..]).unwrap(); - assert_eq!(recovered, decoded) - } - #[test] fn test_decode_tx() { // some random transactions pulled from hive tests diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 35013ba31..113bc41e5 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -1,51 +1,13 @@ //! Defines the types for blob transactions, legacy, and other EIP-2718 transactions included in a //! response to `GetPooledTransactions`. -use crate::{RecoveredTx, TransactionSigned}; +use crate::RecoveredTx; use alloy_consensus::transaction::PooledTransaction; -use alloy_eips::eip4844::BlobTransactionSidecar; -use reth_primitives_traits::transaction::error::TransactionConversionError; +// TODO: remove this foreign type /// A signed pooled transaction with recovered signer. pub type PooledTransactionsElementEcRecovered = RecoveredTx; -impl PooledTransactionsElementEcRecovered { - /// Transform back to [`RecoveredTx`] - pub fn into_ecrecovered_transaction(self) -> RecoveredTx { - let (tx, signer) = self.into_parts(); - RecoveredTx::from_signed_transaction(tx.into(), signer) - } - - /// Converts from an EIP-4844 [`RecoveredTx`] to a - /// [`PooledTransactionsElementEcRecovered`] with the given sidecar. - /// - /// Returns the transaction is not an EIP-4844 transaction. - pub fn try_from_blob_transaction( - tx: RecoveredTx, - sidecar: BlobTransactionSidecar, - ) -> Result> { - let RecoveredTx { signer, signed_transaction } = tx; - let transaction = signed_transaction - .try_into_pooled_eip4844(sidecar) - .map_err(|tx| RecoveredTx { signer, signed_transaction: tx })?; - Ok(Self::from_signed_transaction(transaction, signer)) - } -} - -/// Converts a `Recovered` into a `PooledTransactionsElementEcRecovered`. -impl TryFrom> for PooledTransactionsElementEcRecovered { - type Error = TransactionConversionError; - - fn try_from(tx: RecoveredTx) -> Result { - match PooledTransaction::try_from(tx.signed_transaction) { - Ok(pooled_transaction) => { - Ok(Self::from_signed_transaction(pooled_transaction, tx.signer)) - } - Err(_) => Err(TransactionConversionError::UnsupportedForP2P), - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index 0f9243f60..74873e094 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -1047,7 +1047,8 @@ impl TryFrom> for MockTransaction { impl From for MockTransaction { fn from(tx: PooledTransactionsElementEcRecovered) -> Self { - tx.into_ecrecovered_transaction().try_into().expect( + let (tx, signer) = tx.into_parts(); + RecoveredTx::::new_unchecked(tx.into(), signer).try_into().expect( "Failed to convert from PooledTransactionsElementEcRecovered to MockTransaction", ) } @@ -1058,7 +1059,7 @@ impl From for RecoveredTx { let signed_tx = TransactionSigned::new(tx.clone().into(), Signature::test_signature(), *tx.hash()); - Self::from_signed_transaction(signed_tx, tx.sender()) + Self::new_unchecked(signed_tx, tx.sender()) } } @@ -1180,7 +1181,7 @@ impl proptest::arbitrary::Arbitrary for MockTransaction { arb::<(TransactionSigned, Address)>() .prop_map(|(signed_transaction, signer)| { - RecoveredTx::from_signed_transaction(signed_transaction, signer) + RecoveredTx::new_unchecked(signed_transaction, signer) .try_into() .expect("Failed to create an Arbitrary MockTransaction via RecoveredTx") }) diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index a63e92a47..bcac0080c 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1252,14 +1252,14 @@ impl From for EthPooledTransaction { let (tx, sig, hash) = tx.into_parts(); let (tx, blob) = tx.into_parts(); let tx = TransactionSigned::new(tx.into(), sig, hash); - let tx = RecoveredTx::from_signed_transaction(tx, signer); + let tx = RecoveredTx::new_unchecked(tx, signer); let mut pooled = Self::new(tx, encoded_length); pooled.blob_sidecar = EthBlobTransactionSidecar::Present(blob); pooled } tx => { // no blob sidecar - let tx = RecoveredTx::from_signed_transaction(tx.into(), signer); + let tx = RecoveredTx::new_unchecked(tx.into(), signer); Self::new(tx, encoded_length) } } @@ -1284,7 +1284,7 @@ impl PoolTransaction for EthPooledTransaction { let pooled = tx .try_into_pooled() .map_err(|_| TryFromRecoveredTransactionError::BlobSidecarMissing)?; - Ok(RecoveredTx::from_signed_transaction(pooled, signer)) + Ok(RecoveredTx::new_unchecked(pooled, signer)) } /// Returns hash of the transaction. @@ -1416,11 +1416,11 @@ impl EthPoolTransaction for EthPooledTransaction { self, sidecar: Arc, ) -> Option> { - PooledTransactionsElementEcRecovered::try_from_blob_transaction( - self.into_consensus(), - Arc::unwrap_or_clone(sidecar), - ) - .ok() + let (signed_transaction, signer) = self.into_consensus().into_parts(); + let pooled_transaction = + signed_transaction.try_into_pooled_eip4844(Arc::unwrap_or_clone(sidecar)).ok()?; + + Some(RecoveredTx::new_unchecked(pooled_transaction, signer)) } fn try_from_eip4844( @@ -1692,7 +1692,7 @@ mod tests { }); let signature = Signature::test_signature(); let signed_tx = TransactionSigned::new_unhashed(tx, signature); - let transaction = RecoveredTx::from_signed_transaction(signed_tx, Default::default()); + let transaction = RecoveredTx::new_unchecked(signed_tx, Default::default()); let pooled_tx = EthPooledTransaction::new(transaction.clone(), 200); // Check that the pooled transaction is created correctly @@ -1713,7 +1713,7 @@ mod tests { }); let signature = Signature::test_signature(); let signed_tx = TransactionSigned::new_unhashed(tx, signature); - let transaction = RecoveredTx::from_signed_transaction(signed_tx, Default::default()); + let transaction = RecoveredTx::new_unchecked(signed_tx, Default::default()); let pooled_tx = EthPooledTransaction::new(transaction.clone(), 200); // Check that the pooled transaction is created correctly @@ -1734,7 +1734,7 @@ mod tests { }); let signature = Signature::test_signature(); let signed_tx = TransactionSigned::new_unhashed(tx, signature); - let transaction = RecoveredTx::from_signed_transaction(signed_tx, Default::default()); + let transaction = RecoveredTx::new_unchecked(signed_tx, Default::default()); let pooled_tx = EthPooledTransaction::new(transaction.clone(), 200); // Check that the pooled transaction is created correctly @@ -1757,7 +1757,7 @@ mod tests { }); let signature = Signature::test_signature(); let signed_tx = TransactionSigned::new_unhashed(tx, signature); - let transaction = RecoveredTx::from_signed_transaction(signed_tx, Default::default()); + let transaction = RecoveredTx::new_unchecked(signed_tx, Default::default()); let pooled_tx = EthPooledTransaction::new(transaction.clone(), 300); // Check that the pooled transaction is created correctly @@ -1780,7 +1780,7 @@ mod tests { }); let signature = Signature::test_signature(); let signed_tx = TransactionSigned::new_unhashed(tx, signature); - let transaction = RecoveredTx::from_signed_transaction(signed_tx, Default::default()); + let transaction = RecoveredTx::new_unchecked(signed_tx, Default::default()); let pooled_tx = EthPooledTransaction::new(transaction.clone(), 200); // Check that the pooled transaction is created correctly