From 427a8395f9d89aaab8ff563d6041af34cddbf425 Mon Sep 17 00:00:00 2001 From: PatStiles <33334338+PatStiles@users.noreply.github.com> Date: Fri, 11 Aug 2023 08:34:56 -0500 Subject: [PATCH] feat(txpool): Make TransactionPool trait object safe (#4156) Co-authored-by: Matthias Seitz --- crates/consensus/auto-seal/src/task.rs | 4 +++- crates/net/network/src/transactions.rs | 6 ++---- crates/transaction-pool/src/lib.rs | 7 ++----- crates/transaction-pool/src/noop.rs | 7 ++----- crates/transaction-pool/src/pool/mod.rs | 4 ++-- crates/transaction-pool/src/pool/txpool.rs | 12 ++++++------ crates/transaction-pool/src/traits.rs | 7 ++----- 7 files changed, 19 insertions(+), 28 deletions(-) diff --git a/crates/consensus/auto-seal/src/task.rs b/crates/consensus/auto-seal/src/task.rs index ddb450acea..4abdd19b12 100644 --- a/crates/consensus/auto-seal/src/task.rs +++ b/crates/consensus/auto-seal/src/task.rs @@ -135,7 +135,9 @@ where { Ok((new_header, post_state)) => { // clear all transactions from pool - pool.remove_transactions(transactions.iter().map(|tx| tx.hash())); + pool.remove_transactions( + transactions.iter().map(|tx| tx.hash()).collect(), + ); let state = ForkchoiceState { head_block_hash: new_header.hash, diff --git a/crates/net/network/src/transactions.rs b/crates/net/network/src/transactions.rs index 7345893d1e..87850f1e4f 100644 --- a/crates/net/network/src/transactions.rs +++ b/crates/net/network/src/transactions.rs @@ -204,7 +204,7 @@ where /// complete transaction object if it is unknown to them. The dissemination of complete /// transactions to a fraction of peers usually ensures that all nodes receive the transaction /// and won't need to request it. - fn on_new_transactions(&mut self, hashes: impl IntoIterator) { + fn on_new_transactions(&mut self, hashes: Vec) { // Nothing to propagate while initially syncing if self.network.is_initially_syncing() { return @@ -372,9 +372,7 @@ where /// Handles a command received from a detached [`TransactionsHandle`] fn on_command(&mut self, cmd: TransactionsCommand) { match cmd { - TransactionsCommand::PropagateHash(hash) => { - self.on_new_transactions(std::iter::once(hash)) - } + TransactionsCommand::PropagateHash(hash) => self.on_new_transactions(vec![hash]), } } diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index ccdf752e0d..8d039f66a9 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -401,7 +401,7 @@ where fn remove_transactions( &self, - hashes: impl IntoIterator, + hashes: Vec, ) -> Vec>> { self.pool.remove_transactions(hashes) } @@ -414,10 +414,7 @@ where self.inner().get(tx_hash) } - fn get_all( - &self, - txs: impl IntoIterator, - ) -> Vec>> { + fn get_all(&self, txs: Vec) -> Vec>> { self.inner().get_all(txs) } diff --git a/crates/transaction-pool/src/noop.rs b/crates/transaction-pool/src/noop.rs index 920a98b0d6..d9c26214a5 100644 --- a/crates/transaction-pool/src/noop.rs +++ b/crates/transaction-pool/src/noop.rs @@ -135,7 +135,7 @@ impl TransactionPool for NoopTransactionPool { fn remove_transactions( &self, - _hashes: impl IntoIterator, + _hashes: Vec, ) -> Vec>> { vec![] } @@ -146,10 +146,7 @@ impl TransactionPool for NoopTransactionPool { None } - fn get_all( - &self, - _txs: impl IntoIterator, - ) -> Vec>> { + fn get_all(&self, _txs: Vec) -> Vec>> { vec![] } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 93003431c5..8279633690 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -513,7 +513,7 @@ where /// Removes and returns all matching transactions from the pool. pub(crate) fn remove_transactions( &self, - hashes: impl IntoIterator, + hashes: Vec, ) -> Vec>> { let removed = self.pool.write().remove_transactions(hashes); @@ -552,7 +552,7 @@ where /// If no transaction exists, it is skipped. pub(crate) fn get_all( &self, - txs: impl IntoIterator, + txs: Vec, ) -> Vec>> { self.pool.read().get_all(txs).collect() } diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 2922069987..92abe86c65 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -247,10 +247,10 @@ impl TxPool { } /// Returns transactions for the multiple given hashes, if they exist. - pub(crate) fn get_all<'a>( - &'a self, - txs: impl IntoIterator + 'a, - ) -> impl Iterator>> + 'a { + pub(crate) fn get_all( + &self, + txs: Vec, + ) -> impl Iterator>> + '_ { txs.into_iter().filter_map(|tx| self.get(&tx)) } @@ -409,7 +409,7 @@ impl TxPool { /// Maintenance task to apply a series of updates. /// /// This will move/discard the given transaction according to the `PoolUpdate` - fn process_updates(&mut self, updates: impl IntoIterator) -> UpdateOutcome { + fn process_updates(&mut self, updates: Vec) -> UpdateOutcome { let mut outcome = UpdateOutcome::default(); for update in updates { let PoolUpdate { id, hash, current, destination } = update; @@ -445,7 +445,7 @@ impl TxPool { /// any additional updates. pub(crate) fn remove_transactions( &mut self, - hashes: impl IntoIterator, + hashes: Vec, ) -> Vec>> { hashes.into_iter().filter_map(|hash| self.remove_transaction_by_hash(&hash)).collect() } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 5cffaa30df..e89c94385b 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -227,7 +227,7 @@ pub trait TransactionPool: Send + Sync + Clone { /// Consumer: Block production fn remove_transactions( &self, - hashes: impl IntoIterator, + hashes: Vec, ) -> Vec>>; /// Retains only those hashes that are unknown to the pool. @@ -250,10 +250,7 @@ pub trait TransactionPool: Send + Sync + Clone { /// This adheres to the expected behavior of [`GetPooledTransactions`](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#getpooledtransactions-0x09): /// The transactions must be in same order as in the request, but it is OK to skip transactions /// which are not available. - fn get_all( - &self, - txs: impl IntoIterator, - ) -> Vec>>; + fn get_all(&self, txs: Vec) -> Vec>>; /// Notify the pool about transactions that are propagated to peers. ///