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

tx-pool: Make txpool independent of primitive tx types #9916

Merged
merged 15 commits into from
Aug 1, 2024
1 change: 0 additions & 1 deletion crates/consensus/auto-seal/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ impl<Executor, Client, Pool, Engine> Future for MiningTask<Client, Pool, Executo
where
Client: StateProviderFactory + CanonChainTracker + Clone + Unpin + 'static,
Pool: TransactionPool + Unpin + 'static,
<Pool as TransactionPool>::Transaction: IntoRecoveredTransaction,
Engine: EngineTypes,
Executor: BlockExecutorProvider,
{
Expand Down
8 changes: 3 additions & 5 deletions crates/net/network/src/transactions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ use reth_network_p2p::{
};
use reth_network_peers::PeerId;
use reth_network_types::ReputationChangeKind;
use reth_primitives::{
FromRecoveredPooledTransaction, PooledTransactionsElement, TransactionSigned, TxHash, B256,
};
use reth_primitives::{PooledTransactionsElement, TransactionSigned, TxHash, B256};
use reth_tokio_util::EventStream;
use reth_transaction_pool::{
error::{PoolError, PoolResult},
Expand Down Expand Up @@ -1004,7 +1002,7 @@ where
Entry::Vacant(entry) => {
if !self.bad_imports.contains(tx.hash()) {
// this is a new transaction that should be imported into the pool
let pool_transaction = <Pool::Transaction as FromRecoveredPooledTransaction>::from_recovered_pooled_transaction(tx);
let pool_transaction: Pool::Transaction = tx.into();
new_txs.push(pool_transaction);

entry.insert(HashSet::from([peer_id]));
Expand Down Expand Up @@ -1375,7 +1373,7 @@ impl PropagateTransaction {
/// Create a new instance from a pooled transaction
fn new<T: PoolTransaction>(tx: Arc<ValidPoolTransaction<T>>) -> Self {
let size = tx.encoded_length();
let transaction = Arc::new(tx.transaction.to_recovered_transaction().into_signed());
let transaction = Arc::new(tx.transaction.clone().into().into_signed());
Self { size, transaction }
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/node/src/txpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ where
let l1_block_info = self.block_info.l1_block_info.read().clone();

let mut encoded = Vec::new();
valid_tx.transaction().to_recovered_transaction().encode_enveloped(&mut encoded);
valid_tx.transaction().clone().into().encode_enveloped(&mut encoded);

let cost_addition = match l1_block_info.l1_tx_data_fee(
&self.chain_spec(),
Expand Down
10 changes: 5 additions & 5 deletions crates/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ pub use reth_primitives_traits::{
pub use static_file::StaticFileSegment;

pub use transaction::{
BlobTransaction, BlobTransactionSidecar, FromRecoveredPooledTransaction,
PooledTransactionsElement, PooledTransactionsElementEcRecovered,
BlobTransaction, BlobTransactionSidecar, PooledTransactionsElement,
PooledTransactionsElementEcRecovered,
};

#[cfg(feature = "c-kzg")]
Expand All @@ -70,9 +70,9 @@ pub use transaction::{
util::secp256k1::{public_key_to_address, recover_signer_unchecked, sign_message},
AccessList, AccessListItem, IntoRecoveredTransaction, InvalidTransactionError, Signature,
Transaction, TransactionMeta, TransactionSigned, TransactionSignedEcRecovered,
TransactionSignedNoHash, TryFromRecoveredTransaction, TxEip1559, TxEip2930, TxEip4844,
TxEip7702, TxHashOrNumber, TxLegacy, TxType, EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID,
EIP4844_TX_TYPE_ID, EIP7702_TX_TYPE_ID, LEGACY_TX_TYPE_ID,
TransactionSignedNoHash, TxEip1559, TxEip2930, TxEip4844, TxEip7702, TxHashOrNumber, TxLegacy,
TxType, EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID, EIP4844_TX_TYPE_ID, EIP7702_TX_TYPE_ID,
LEGACY_TX_TYPE_ID,
};

// Re-exports
Expand Down
43 changes: 1 addition & 42 deletions crates/primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1580,48 +1580,7 @@ impl Decodable for TransactionSignedEcRecovered {
}
}

/// A transaction type that can be created from a [`TransactionSignedEcRecovered`] transaction.
///
/// This is a conversion trait that'll ensure transactions received via P2P can be converted to the
/// transaction type that the transaction pool uses.
pub trait TryFromRecoveredTransaction {
/// The error type returned by the transaction.
type Error;
/// Converts to this type from the given [`TransactionSignedEcRecovered`].
fn try_from_recovered_transaction(
tx: TransactionSignedEcRecovered,
) -> Result<Self, Self::Error>
where
Self: Sized;
}

// Noop conversion
impl TryFromRecoveredTransaction for TransactionSignedEcRecovered {
type Error = TryFromRecoveredTransactionError;

#[inline]
fn try_from_recovered_transaction(
tx: TransactionSignedEcRecovered,
) -> Result<Self, Self::Error> {
if tx.is_eip4844() {
Err(TryFromRecoveredTransactionError::BlobSidecarMissing)
} else {
Ok(tx)
}
}
}

/// A transaction type that can be created from a [`PooledTransactionsElementEcRecovered`]
/// transaction.
///
/// This is a conversion trait that'll ensure transactions received via P2P can be converted to the
/// transaction type that the transaction pool uses.
pub trait FromRecoveredPooledTransaction {
/// Converts to this type from the given [`PooledTransactionsElementEcRecovered`].
fn from_recovered_pooled_transaction(tx: PooledTransactionsElementEcRecovered) -> Self;
}

/// The inverse of [`TryFromRecoveredTransaction`] that ensure the transaction can be sent over the
/// Ensures the transaction can be sent over the
/// network
pub trait IntoRecoveredTransaction {
/// Converts to this type into a [`TransactionSignedEcRecovered`].
Expand Down
15 changes: 5 additions & 10 deletions crates/rpc/rpc-eth-api/src/helpers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use std::{fmt, ops::Deref, sync::Arc};
use alloy_dyn_abi::TypedData;
use futures::Future;
use reth_primitives::{
Address, BlockId, Bytes, FromRecoveredPooledTransaction, IntoRecoveredTransaction, Receipt,
SealedBlockWithSenders, TransactionMeta, TransactionSigned, TxHash, TxKind, B256, U256,
Address, BlockId, Bytes, Receipt, SealedBlockWithSenders, TransactionMeta, TransactionSigned,
TxHash, TxKind, B256, U256,
};
use reth_provider::{BlockReaderIdExt, ReceiptProvider, TransactionsProvider};
use reth_rpc_eth_types::{
Expand Down Expand Up @@ -250,10 +250,7 @@ pub trait EthTransactions: LoadTransaction {
) -> impl Future<Output = Result<B256, Self::Error>> + Send {
async move {
let recovered = recover_raw_transaction(tx.clone())?;
let pool_transaction =
<Self::Pool as TransactionPool>::Transaction::from_recovered_pooled_transaction(
recovered,
);
let pool_transaction: <Self::Pool as TransactionPool>::Transaction = recovered.into();

// On optimism, transactions are forwarded directly to the sequencer to be included in
// blocks that it builds.
Expand Down Expand Up @@ -474,7 +471,7 @@ pub trait EthTransactions: LoadTransaction {
signed_tx.into_ecrecovered().ok_or(EthApiError::InvalidTransactionSignature)?;

let pool_transaction = match recovered.try_into() {
Ok(converted) => <<Self as LoadTransaction>::Pool as TransactionPool>::Transaction::from_recovered_pooled_transaction(converted),
Ok(converted) => converted,
Err(_) => return Err(EthApiError::TransactionConversionError.into()),
};

Expand Down Expand Up @@ -610,9 +607,7 @@ pub trait LoadTransaction: SpawnBlocking {

if resp.is_none() {
// tx not found on disk, check pool
if let Some(tx) =
self.pool().get(&hash).map(|tx| tx.transaction.to_recovered_transaction())
{
if let Some(tx) = self.pool().get(&hash).map(|tx| tx.transaction.clone().into()) {
resp = Some(TransactionSource::Pool(tx));
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/rpc/rpc/src/txpool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use async_trait::async_trait;
use jsonrpsee::core::RpcResult as Result;
use reth_primitives::Address;
use reth_primitives::{Address, TransactionSignedEcRecovered};
use reth_rpc_api::TxPoolApiServer;
use reth_rpc_types::{
txpool::{TxpoolContent, TxpoolContentFrom, TxpoolInspect, TxpoolInspectSummary, TxpoolStatus},
Expand Down Expand Up @@ -38,7 +38,7 @@ where
) {
content.entry(tx.sender()).or_default().insert(
tx.nonce().to_string(),
reth_rpc_types_compat::transaction::from_recovered(tx.to_recovered_transaction()),
reth_rpc_types_compat::transaction::from_recovered(tx.clone().into()),
);
}

Expand Down Expand Up @@ -87,7 +87,7 @@ where
inspect: &mut BTreeMap<Address, BTreeMap<String, TxpoolInspectSummary>>,
) {
let entry = inspect.entry(tx.sender()).or_default();
let tx = tx.to_recovered_transaction();
let tx: TransactionSignedEcRecovered = tx.clone().into();
entry.insert(
tx.nonce().to_string(),
TxpoolInspectSummary {
Expand Down
19 changes: 6 additions & 13 deletions crates/transaction-pool/src/maintain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ use futures_util::{
use reth_execution_types::ExecutionOutcome;
use reth_fs_util::FsPathError;
use reth_primitives::{
Address, BlockHash, BlockNumber, BlockNumberOrTag, FromRecoveredPooledTransaction,
IntoRecoveredTransaction, PooledTransactionsElementEcRecovered, TransactionSigned,
TryFromRecoveredTransaction,
Address, BlockHash, BlockNumber, BlockNumberOrTag, IntoRecoveredTransaction,
PooledTransactionsElementEcRecovered, TransactionSigned,
};
use reth_provider::{
BlockReaderIdExt, CanonStateNotification, ChainSpecProvider, ProviderError,
Expand Down Expand Up @@ -332,13 +331,9 @@ pub async fn maintain_transaction_pool<Client, P, St, Tasks>(
)
.ok()
})
.map(
<P as TransactionPool>::Transaction::from_recovered_pooled_transaction,
)
.map(Into::into)
} else {
<P as TransactionPool>::Transaction::try_from_recovered_transaction(
tx,
).ok()
tx.try_into().ok()
}
})
.collect::<Vec<_>>();
Expand Down Expand Up @@ -592,7 +587,7 @@ where
.filter_map(|tx| tx.try_ecrecovered())
.filter_map(|tx| {
// Filter out errors
<P as TransactionPool>::Transaction::try_from_recovered_transaction(tx).ok()
tx.try_into().ok()
})
.collect::<Vec<_>>();

Expand Down Expand Up @@ -705,9 +700,7 @@ mod tests {
let tx_bytes = hex!("02f87201830655c2808505ef61f08482565f94388c818ca8b9251b393131c08a736a67ccb192978801049e39c4b5b1f580c001a01764ace353514e8abdfb92446de356b260e3c1225b73fc4c8876a6258d12a129a04f02294aa61ca7676061cd99f29275491218b4754b46a0248e5e42bc5091f507");
let tx = PooledTransactionsElement::decode_enveloped(&mut &tx_bytes[..]).unwrap();
let provider = MockEthProvider::default();
let transaction = EthPooledTransaction::from_recovered_pooled_transaction(
tx.try_into_ecrecovered().unwrap(),
);
let transaction: EthPooledTransaction = tx.try_into_ecrecovered().unwrap().into();
let tx_to_cmp = transaction.clone();
let sender = hex!("1f9090aaE28b8a3dCeaDf281B0F12828e676c326").into();
provider.add_account(sender, ExtendedAccount::new(42, U256::MAX));
Expand Down
8 changes: 2 additions & 6 deletions crates/transaction-pool/src/test_utils/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use rand::Rng;
use reth_chainspec::MAINNET;
use reth_primitives::{
constants::MIN_PROTOCOL_BASE_FEE, sign_message, AccessList, Address, Bytes, Transaction,
TransactionSigned, TryFromRecoveredTransaction, TxEip1559, TxEip4844, TxKind, TxLegacy, B256,
U256,
TransactionSigned, TxEip1559, TxEip4844, TxKind, TxLegacy, B256, U256,
};

/// A generator for transactions for testing purposes.
Expand Down Expand Up @@ -99,10 +98,7 @@ 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 {
EthPooledTransaction::try_from_recovered_transaction(
self.gen_eip1559().into_ecrecovered().unwrap(),
)
.unwrap()
self.gen_eip1559().into_ecrecovered().unwrap().try_into().unwrap()
}

/// Generates and returns a pooled EIP-4844 transaction with a random signer.
Expand Down
39 changes: 19 additions & 20 deletions crates/transaction-pool/src/test_utils/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ use reth_primitives::{
constants::{eip4844::DATA_GAS_PER_BLOB, MIN_PROTOCOL_BASE_FEE},
transaction::TryFromRecoveredTransactionError,
AccessList, Address, BlobTransactionSidecar, BlobTransactionValidationError, Bytes, ChainId,
FromRecoveredPooledTransaction, IntoRecoveredTransaction, PooledTransactionsElementEcRecovered,
Signature, Transaction, TransactionSigned, TransactionSignedEcRecovered,
TryFromRecoveredTransaction, TxEip1559, TxEip2930, TxEip4844, TxHash, TxKind, TxLegacy, TxType,
B256, EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID, EIP4844_TX_TYPE_ID, LEGACY_TX_TYPE_ID, U256,
PooledTransactionsElementEcRecovered, Signature, Transaction, TransactionSigned,
TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxEip4844, TxHash, TxKind, TxLegacy,
TxType, B256, EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID, EIP4844_TX_TYPE_ID, LEGACY_TX_TYPE_ID,
U256,
};
use std::{ops::Range, sync::Arc, time::Instant, vec::IntoIter};

Expand Down Expand Up @@ -558,6 +558,10 @@ impl MockTransaction {
}

impl PoolTransaction for MockTransaction {
type Consensus = TransactionSignedEcRecovered;

type Pooled = PooledTransactionsElementEcRecovered;

fn hash(&self) -> &TxHash {
match self {
Self::Legacy { hash, .. } |
Expand Down Expand Up @@ -755,12 +759,10 @@ impl EthPoolTransaction for MockTransaction {
}
}

impl TryFromRecoveredTransaction for MockTransaction {
impl TryFrom<TransactionSignedEcRecovered> for MockTransaction {
type Error = TryFromRecoveredTransactionError;

fn try_from_recovered_transaction(
tx: TransactionSignedEcRecovered,
) -> Result<Self, Self::Error> {
fn try_from(tx: TransactionSignedEcRecovered) -> Result<Self, Self::Error> {
let sender = tx.signer();
let transaction = tx.into_signed();
let hash = transaction.hash();
Expand Down Expand Up @@ -869,26 +871,23 @@ impl TryFromRecoveredTransaction for MockTransaction {
}
}

impl FromRecoveredPooledTransaction for MockTransaction {
fn from_recovered_pooled_transaction(tx: PooledTransactionsElementEcRecovered) -> Self {
TryFromRecoveredTransaction::try_from_recovered_transaction(
tx.into_ecrecovered_transaction(),
impl From<PooledTransactionsElementEcRecovered> for MockTransaction {
fn from(tx: PooledTransactionsElementEcRecovered) -> Self {
tx.into_ecrecovered_transaction().try_into().expect(
"Failed to convert from PooledTransactionsElementEcRecovered to MockTransaction",
)
.expect("Failed to convert from PooledTransactionsElementEcRecovered to MockTransaction")
}
}

impl IntoRecoveredTransaction for MockTransaction {
fn to_recovered_transaction(&self) -> TransactionSignedEcRecovered {
let tx = self.clone().into();

impl From<MockTransaction> for TransactionSignedEcRecovered {
fn from(tx: MockTransaction) -> Self {
let signed_tx = TransactionSigned {
hash: *self.hash(),
hash: *tx.hash(),
signature: Signature::default(),
transaction: tx,
transaction: tx.clone().into(),
};

TransactionSignedEcRecovered::from_signed_transaction(signed_tx, self.sender())
Self::from_signed_transaction(signed_tx, tx.sender())
}
}

Expand Down
Loading
Loading