diff --git a/substrate/client/transaction-pool/graph/src/base_pool.rs b/substrate/client/transaction-pool/graph/src/base_pool.rs index 445ef0adaf..9b644bbdb3 100644 --- a/substrate/client/transaction-pool/graph/src/base_pool.rs +++ b/substrate/client/transaction-pool/graph/src/base_pool.rs @@ -155,13 +155,13 @@ impl Transaction { /// every reason to be commented. That's why we `Transaction` is not `Clone`, /// but there's explicit `duplicate` method. pub fn duplicate(&self) -> Self { - Transaction { + Self { data: self.data.clone(), - bytes: self.bytes.clone(), + bytes: self.bytes, hash: self.hash.clone(), - priority: self.priority.clone(), + priority: self.priority, source: self.source, - valid_till: self.valid_till.clone(), + valid_till: self.valid_till, requires: self.requires.clone(), provides: self.provides.clone(), propagate: self.propagate, @@ -174,16 +174,9 @@ impl fmt::Debug for Transaction where Extrinsic: fmt::Debug, { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fn print_tags(fmt: &mut fmt::Formatter, tags: &[Tag]) -> fmt::Result { - let mut it = tags.iter(); - if let Some(t) = it.next() { - write!(fmt, "{}", HexDisplay::from(t))?; - } - for t in it { - write!(fmt, ",{}", HexDisplay::from(t))?; - } - Ok(()) - } + let join_tags = |tags: &[Tag]| { + tags.iter().map(|tag| HexDisplay::from(tag).to_string()).collect::>().join(", ") + }; write!(fmt, "Transaction {{ ")?; write!(fmt, "hash: {:?}, ", &self.hash)?; @@ -192,11 +185,8 @@ impl fmt::Debug for Transaction where write!(fmt, "bytes: {:?}, ", &self.bytes)?; write!(fmt, "propagate: {:?}, ", &self.propagate)?; write!(fmt, "source: {:?}, ", &self.source)?; - write!(fmt, "requires: [")?; - print_tags(fmt, &self.requires)?; - write!(fmt, "], provides: [")?; - print_tags(fmt, &self.provides)?; - write!(fmt, "], ")?; + write!(fmt, "requires: [{}], ", join_tags(&self.requires))?; + write!(fmt, "provides: [{}], ", join_tags(&self.provides))?; write!(fmt, "data: {:?}", &self.data)?; write!(fmt, "}}")?; Ok(()) @@ -239,7 +229,7 @@ impl Default for Bas impl BasePool { /// Create new pool given reject_future_transactions flag. pub fn new(reject_future_transactions: bool) -> Self { - BasePool { + Self { reject_future_transactions, future: Default::default(), ready: Default::default(), @@ -320,13 +310,8 @@ impl BasePool tx, - None => break, - }; - + // take first transaction from the list + while let Some(tx) = to_import.pop() { // find transactions in Future that it unlocks to_import.append(&mut self.future.satisfy_tags(&tx.transaction.provides)); @@ -1087,7 +1072,7 @@ mod tests { }), "Transaction { \ hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, \ -source: TransactionSource::External, requires: [03,02], provides: [04], data: [4]}".to_owned() +source: TransactionSource::External, requires: [03, 02], provides: [04], data: [4]}".to_owned() ); } diff --git a/substrate/client/transaction-pool/graph/src/future.rs b/substrate/client/transaction-pool/graph/src/future.rs index 98d49817e3..9dcfd13808 100644 --- a/substrate/client/transaction-pool/graph/src/future.rs +++ b/substrate/client/transaction-pool/graph/src/future.rs @@ -47,24 +47,22 @@ impl fmt::Debug for WaitingTransaction>().join(", "), + )?; + write!(fmt, "}}") } } impl Clone for WaitingTransaction { fn clone(&self) -> Self { - WaitingTransaction { + Self { transaction: self.transaction.clone(), missing_tags: self.missing_tags.clone(), - imported_at: self.imported_at.clone(), + imported_at: self.imported_at, } } } @@ -90,7 +88,7 @@ impl WaitingTransaction { .cloned() .collect(); - WaitingTransaction { + Self { transaction: Arc::new(transaction), missing_tags, imported_at: Instant::now(), @@ -123,7 +121,7 @@ pub struct FutureTransactions { impl Default for FutureTransactions { fn default() -> Self { - FutureTransactions { + Self { wanted_tags: Default::default(), waiting: Default::default(), } diff --git a/substrate/client/transaction-pool/graph/src/listener.rs b/substrate/client/transaction-pool/graph/src/listener.rs index d707c0a0f8..563243bf45 100644 --- a/substrate/client/transaction-pool/graph/src/listener.rs +++ b/substrate/client/transaction-pool/graph/src/listener.rs @@ -20,12 +20,14 @@ use std::{ collections::HashMap, hash, fmt::Debug, }; + use linked_hash_map::LinkedHashMap; use serde::Serialize; -use crate::{watcher, ChainApi, ExtrinsicHash, BlockHash}; use log::{debug, trace, warn}; use sp_runtime::traits; +use crate::{watcher, ChainApi, ExtrinsicHash, BlockHash}; + /// Extrinsic pool default listener. pub struct Listener { watchers: HashMap>>, @@ -37,7 +39,7 @@ const MAX_FINALITY_WATCHERS: usize = 512; impl Default for Listener { fn default() -> Self { - Listener { + Self { watchers: Default::default(), finality_watchers: Default::default(), } @@ -115,7 +117,7 @@ impl Listener { while self.finality_watchers.len() > MAX_FINALITY_WATCHERS { if let Some((hash, txs)) = self.finality_watchers.pop_front() { for tx in txs { - self.fire(&tx, |s| s.finality_timeout(hash.clone())); + self.fire(&tx, |s| s.finality_timeout(hash)); } } } diff --git a/substrate/client/transaction-pool/graph/src/pool.rs b/substrate/client/transaction-pool/graph/src/pool.rs index eee14049d4..8a60ea80bc 100644 --- a/substrate/client/transaction-pool/graph/src/pool.rs +++ b/substrate/client/transaction-pool/graph/src/pool.rs @@ -21,8 +21,6 @@ use std::{ sync::Arc, }; -use crate::{base_pool as base, watcher::Watcher}; - use futures::Future; use sp_runtime::{ generic::BlockId, @@ -35,6 +33,7 @@ use sp_transaction_pool::error; use wasm_timer::Instant; use futures::channel::mpsc::Receiver; +use crate::{base_pool as base, watcher::Watcher}; use crate::validated_pool::ValidatedPool; pub use crate::validated_pool::{IsValidator, ValidatedTransaction}; @@ -111,7 +110,7 @@ pub struct Options { impl Default for Options { fn default() -> Self { - Options { + Self { ready: base::Limit { count: 8192, total_bytes: 20 * 1024 * 1024, @@ -151,7 +150,7 @@ where impl Pool { /// Create a new transaction pool. pub fn new(options: Options, is_validator: IsValidator, api: Arc) -> Self { - Pool { + Self { validated_pool: Arc::new(ValidatedPool::new(options, is_validator, api)), } } @@ -193,7 +192,7 @@ impl Pool { res.expect("One extrinsic passed; one result returned; qed") } - /// Import a single extrinsic and starts to watch their progress in the pool. + /// Import a single extrinsic and starts to watch its progress in the pool. pub async fn submit_and_watch( &self, at: &BlockId, @@ -242,8 +241,8 @@ impl Pool { // Prune all transactions that provide given tags let prune_status = self.validated_pool.prune_tags(in_pool_tags)?; - let pruned_transactions = hashes.into_iter().cloned() - .chain(prune_status.pruned.iter().map(|tx| tx.hash.clone())); + let pruned_transactions = hashes.iter().cloned() + .chain(prune_status.pruned.iter().map(|tx| tx.hash)); self.validated_pool.fire_pruned(at, pruned_transactions) } @@ -337,7 +336,7 @@ impl Pool { // note that `known_imported_hashes` will be rejected here due to temporary ban. let pruned_hashes = prune_status.pruned .iter() - .map(|tx| tx.hash.clone()).collect::>(); + .map(|tx| tx.hash).collect::>(); let pruned_transactions = prune_status.pruned .into_iter() .map(|tx| (tx.source, tx.data.clone())); @@ -402,7 +401,7 @@ impl Pool { let ignore_banned = matches!(check, CheckBannedBeforeVerify::No); if let Err(err) = self.validated_pool.check_is_known(&hash, ignore_banned) { - return (hash.clone(), ValidatedTransaction::Invalid(hash, err.into())) + return (hash, ValidatedTransaction::Invalid(hash, err)) } let validation_result = self.validated_pool.api().validate_transaction( @@ -413,17 +412,17 @@ impl Pool { let status = match validation_result { Ok(status) => status, - Err(e) => return (hash.clone(), ValidatedTransaction::Invalid(hash, e)), + Err(e) => return (hash, ValidatedTransaction::Invalid(hash, e)), }; let validity = match status { Ok(validity) => { if validity.provides.is_empty() { - ValidatedTransaction::Invalid(hash.clone(), error::Error::NoTagsProvided.into()) + ValidatedTransaction::Invalid(hash, error::Error::NoTagsProvided.into()) } else { ValidatedTransaction::valid_at( block_number.saturated_into::(), - hash.clone(), + hash, source, xt, bytes, @@ -432,9 +431,9 @@ impl Pool { } }, Err(TransactionValidityError::Invalid(e)) => - ValidatedTransaction::Invalid(hash.clone(), error::Error::InvalidTransaction(e).into()), + ValidatedTransaction::Invalid(hash, error::Error::InvalidTransaction(e).into()), Err(TransactionValidityError::Unknown(e)) => - ValidatedTransaction::Unknown(hash.clone(), error::Error::UnknownTransaction(e).into()), + ValidatedTransaction::Unknown(hash, error::Error::UnknownTransaction(e).into()), }; (hash, validity) diff --git a/substrate/client/transaction-pool/graph/src/ready.rs b/substrate/client/transaction-pool/graph/src/ready.rs index c2af4f9cb9..4ede9241d8 100644 --- a/substrate/client/transaction-pool/graph/src/ready.rs +++ b/substrate/client/transaction-pool/graph/src/ready.rs @@ -50,7 +50,7 @@ pub struct TransactionRef { impl Clone for TransactionRef { fn clone(&self) -> Self { - TransactionRef { + Self { transaction: self.transaction.clone(), insertion_id: self.insertion_id, } @@ -93,7 +93,7 @@ pub struct ReadyTx { impl Clone for ReadyTx { fn clone(&self) -> Self { - ReadyTx { + Self { transaction: self.transaction.clone(), unlocks: self.unlocks.clone(), requires_offset: self.requires_offset, @@ -128,7 +128,7 @@ impl tracked_map::Size for ReadyTx { impl Default for ReadyTransactions { fn default() -> Self { - ReadyTransactions { + Self { insertion_id: Default::default(), provided_tags: Default::default(), ready: Default::default(), @@ -259,7 +259,7 @@ impl ReadyTransactions { /// (i.e. the entire subgraph that this transaction is a start of will be removed). /// All removed transactions are returned. pub fn remove_subtree(&mut self, hashes: &[Hash]) -> Vec>> { - let to_remove = hashes.iter().cloned().collect::>(); + let to_remove = hashes.to_vec(); self.remove_subtree_with_tag_filter(to_remove, None) } diff --git a/substrate/client/transaction-pool/graph/src/rotator.rs b/substrate/client/transaction-pool/graph/src/rotator.rs index 3d9b359fd3..4c800c7671 100644 --- a/substrate/client/transaction-pool/graph/src/rotator.rs +++ b/substrate/client/transaction-pool/graph/src/rotator.rs @@ -48,7 +48,7 @@ pub struct PoolRotator { impl Default for PoolRotator { fn default() -> Self { - PoolRotator { + Self { ban_time: Duration::from_secs(60 * 30), banned_until: Default::default(), } @@ -78,7 +78,6 @@ impl PoolRotator { } } - /// Bans extrinsic if it's stale. /// /// Returns `true` if extrinsic is stale and got banned. diff --git a/substrate/client/transaction-pool/graph/src/tracked_map.rs b/substrate/client/transaction-pool/graph/src/tracked_map.rs index 9cd6ad84b4..98fd9e21b3 100644 --- a/substrate/client/transaction-pool/graph/src/tracked_map.rs +++ b/substrate/client/transaction-pool/graph/src/tracked_map.rs @@ -22,7 +22,7 @@ use std::{ }; use parking_lot::{RwLock, RwLockWriteGuard, RwLockReadGuard}; -/// Something that can report it's size. +/// Something that can report its size. pub trait Size { fn size(&self) -> usize; } @@ -64,14 +64,14 @@ impl TrackedMap { } /// Lock map for read. - pub fn read<'a>(&'a self) -> TrackedMapReadAccess<'a, K, V> { + pub fn read(&self) -> TrackedMapReadAccess { TrackedMapReadAccess { inner_guard: self.index.read(), } } /// Lock map for write. - pub fn write<'a>(&'a self) -> TrackedMapWriteAccess<'a, K, V> { + pub fn write(&self) -> TrackedMapWriteAccess { TrackedMapWriteAccess { inner_guard: self.index.write(), bytes: &self.bytes, @@ -90,7 +90,7 @@ where K: Eq + std::hash::Hash { /// Lock map for read. - pub fn read<'a>(&'a self) -> TrackedMapReadAccess<'a, K, V> { + pub fn read(&self) -> TrackedMapReadAccess { TrackedMapReadAccess { inner_guard: self.0.read(), } @@ -136,10 +136,10 @@ where let new_bytes = val.size(); self.bytes.fetch_add(new_bytes as isize, AtomicOrdering::Relaxed); self.length.fetch_add(1, AtomicOrdering::Relaxed); - self.inner_guard.insert(key, val).and_then(|old_val| { + self.inner_guard.insert(key, val).map(|old_val| { self.bytes.fetch_sub(old_val.size() as isize, AtomicOrdering::Relaxed); self.length.fetch_sub(1, AtomicOrdering::Relaxed); - Some(old_val) + old_val }) } @@ -186,4 +186,4 @@ mod tests { assert_eq!(map.bytes(), 1); assert_eq!(map.len(), 1); } -} \ No newline at end of file +} diff --git a/substrate/client/transaction-pool/graph/src/validated_pool.rs b/substrate/client/transaction-pool/graph/src/validated_pool.rs index c02aab47d8..6042189e87 100644 --- a/substrate/client/transaction-pool/graph/src/validated_pool.rs +++ b/substrate/client/transaction-pool/graph/src/validated_pool.rs @@ -22,12 +22,7 @@ use std::{ sync::Arc, }; -use crate::base_pool as base; -use crate::listener::Listener; -use crate::rotator::PoolRotator; -use crate::watcher::Watcher; use serde::Serialize; - use parking_lot::{Mutex, RwLock}; use sp_runtime::{ generic::BlockId, @@ -39,7 +34,10 @@ use wasm_timer::Instant; use futures::channel::mpsc::{channel, Sender}; use retain_mut::RetainMut; -use crate::base_pool::PruneStatus; +use crate::base_pool::{self as base, PruneStatus}; +use crate::listener::Listener; +use crate::rotator::PoolRotator; +use crate::watcher::Watcher; use crate::pool::{ EventStream, Options, ChainApi, BlockHash, ExtrinsicHash, ExtrinsicFor, TransactionFor, }; @@ -95,13 +93,13 @@ pub struct IsValidator(Box bool + Send + Sync>); impl From for IsValidator { fn from(is_validator: bool) -> Self { - IsValidator(Box::new(move || is_validator)) + Self(Box::new(move || is_validator)) } } impl From bool + Send + Sync>> for IsValidator { fn from(is_validator: Box bool + Send + Sync>) -> Self { - IsValidator(is_validator) + Self(is_validator) } } @@ -134,7 +132,7 @@ impl ValidatedPool { /// Create a new transaction pool. pub fn new(options: Options, is_validator: IsValidator, api: Arc) -> Self { let base_pool = base::BasePool::new(options.reject_future_transactions); - ValidatedPool { + Self { is_validator, options, listener: Default::default(), @@ -168,7 +166,7 @@ impl ValidatedPool { if !ignore_banned && self.is_banned(tx_hash) { Err(error::Error::TemporarilyBanned.into()) } else if self.pool.read().is_imported(tx_hash) { - Err(error::Error::AlreadyImported(Box::new(tx_hash.clone())).into()) + Err(error::Error::AlreadyImported(Box::new(*tx_hash)).into()) } else { Ok(()) } @@ -209,7 +207,7 @@ impl ValidatedPool { if let base::Imported::Ready { ref hash, .. } = imported { self.import_notification_sinks.lock() .retain_mut(|sink| { - match sink.try_send(hash.clone()) { + match sink.try_send(*hash) { Ok(()) => true, Err(e) => { if e.is_full() { @@ -225,15 +223,15 @@ impl ValidatedPool { let mut listener = self.listener.write(); fire_events(&mut *listener, &imported); - Ok(imported.hash().clone()) + Ok(*imported.hash()) }, ValidatedTransaction::Invalid(hash, err) => { self.rotator.ban(&Instant::now(), std::iter::once(hash)); - Err(err.into()) + Err(err) }, ValidatedTransaction::Unknown(hash, err) => { self.listener.write().invalid(&hash, false); - Err(err.into()) + Err(err) }, } } @@ -258,9 +256,9 @@ impl ValidatedPool { let removed = { let mut pool = self.pool.write(); let removed = pool.enforce_limits(ready_limit, future_limit) - .into_iter().map(|x| x.hash.clone()).collect::>(); + .into_iter().map(|x| x.hash).collect::>(); // ban all removed transactions - self.rotator.ban(&Instant::now(), removed.iter().map(|x| x.clone())); + self.rotator.ban(&Instant::now(), removed.iter().copied()); removed }; if !removed.is_empty() { @@ -295,9 +293,9 @@ impl ValidatedPool { }, ValidatedTransaction::Invalid(hash, err) => { self.rotator.ban(&Instant::now(), std::iter::once(hash)); - Err(err.into()) + Err(err) }, - ValidatedTransaction::Unknown(_, err) => Err(err.into()), + ValidatedTransaction::Unknown(_, err) => Err(err), } } @@ -327,9 +325,9 @@ impl ValidatedPool { // note we are not considering tx with hash invalid here - we just want // to remove it along with dependent transactions and `remove_subtree()` // does exactly what we need - let removed = pool.remove_subtree(&[hash.clone()]); + let removed = pool.remove_subtree(&[hash]); for removed_tx in removed { - let removed_hash = removed_tx.hash.clone(); + let removed_hash = removed_tx.hash; let updated_transaction = updated_transactions.remove(&removed_hash); let tx_to_resubmit = if let Some(updated_tx) = updated_transaction { updated_tx @@ -343,7 +341,7 @@ impl ValidatedPool { ValidatedTransaction::Valid(transaction) }; - initial_statuses.insert(removed_hash.clone(), Status::Ready); + initial_statuses.insert(removed_hash, Status::Ready); txs_to_resubmit.push((removed_hash, tx_to_resubmit)); } // make sure to remove the hash even if it's not present in the pool any more. @@ -370,7 +368,7 @@ impl ValidatedPool { final_statuses.insert(hash, Status::Failed); } for tx in removed { - final_statuses.insert(tx.hash.clone(), Status::Dropped); + final_statuses.insert(tx.hash, Status::Dropped); } }, base::Imported::Future { .. } => { @@ -400,7 +398,7 @@ impl ValidatedPool { // queue, updating final statuses as required if reject_future_transactions { for future_tx in pool.clear_future() { - final_statuses.insert(future_tx.hash.clone(), Status::Dropped); + final_statuses.insert(future_tx.hash, Status::Dropped); } } @@ -428,7 +426,7 @@ impl ValidatedPool { self.pool.read().by_hashes(&hashes) .into_iter() .map(|existing_in_pool| existing_in_pool - .map(|transaction| transaction.provides.iter().cloned().collect())) + .map(|transaction| transaction.provides.to_vec())) .collect() } @@ -477,7 +475,7 @@ impl ValidatedPool { .into_iter() .enumerate() .filter_map(|(idx, r)| match r.map_err(error::IntoPoolError::into_pool_error) { - Err(Ok(error::Error::InvalidTransaction(_))) => Some(pruned_hashes[idx].clone()), + Err(Ok(error::Error::InvalidTransaction(_))) => Some(pruned_hashes[idx]), _ => None, }); // Fire `pruned` notifications for collected hashes and make sure to include @@ -498,7 +496,7 @@ impl ValidatedPool { hashes: impl Iterator>, ) -> Result<(), B::Error> { let header_hash = self.api.block_id_to_hash(at)? - .ok_or_else(|| error::Error::InvalidBlockId(format!("{:?}", at)).into())?; + .ok_or_else(|| error::Error::InvalidBlockId(format!("{:?}", at)))?; let mut listener = self.listener.write(); let mut set = HashSet::with_capacity(hashes.size_hint().0); for h in hashes { @@ -519,13 +517,13 @@ impl ValidatedPool { /// See `prune_tags` if you want this. pub fn clear_stale(&self, at: &BlockId) -> Result<(), B::Error> { let block_number = self.api.block_id_to_number(at)? - .ok_or_else(|| error::Error::InvalidBlockId(format!("{:?}", at)).into())? + .ok_or_else(|| error::Error::InvalidBlockId(format!("{:?}", at)))? .saturated_into::(); let now = Instant::now(); let to_remove = { self.ready() .filter(|tx| self.rotator.ban_if_stale(&now, block_number, &tx)) - .map(|tx| tx.hash.clone()) + .map(|tx| tx.hash) .collect::>() }; let futures_to_remove: Vec> = { @@ -533,7 +531,7 @@ impl ValidatedPool { let mut hashes = Vec::new(); for tx in p.futures() { if self.rotator.ban_if_stale(&now, block_number, &tx) { - hashes.push(tx.hash.clone()); + hashes.push(tx.hash); } } hashes diff --git a/substrate/client/transaction-pool/src/lib.rs b/substrate/client/transaction-pool/src/lib.rs index b6f19ba376..efd5a7a143 100644 --- a/substrate/client/transaction-pool/src/lib.rs +++ b/substrate/client/transaction-pool/src/lib.rs @@ -167,7 +167,7 @@ impl BasicPool let (revalidation_queue, background_task, notifier) = revalidation::RevalidationQueue::new_test(pool_api.clone(), pool.clone()); ( - BasicPool { + Self { api: pool_api, pool, revalidation_queue: Arc::new(revalidation_queue), @@ -203,7 +203,7 @@ impl BasicPool spawner.spawn("txpool-background", background_task); } - BasicPool { + Self { api: pool_api, pool, revalidation_queue: Arc::new(revalidation_queue),