diff --git a/substrate/core/transaction-pool/graph/src/pool.rs b/substrate/core/transaction-pool/graph/src/pool.rs index 97244f1cec..dcb54f710f 100644 --- a/substrate/core/transaction-pool/graph/src/pool.rs +++ b/substrate/core/transaction-pool/graph/src/pool.rs @@ -240,6 +240,7 @@ impl Pool { tags: impl IntoIterator, known_imported_hashes: impl IntoIterator> + Clone, ) -> impl Future> { + log::trace!(target: "txpool", "Pruning at {:?}", at); // Prune all transactions that provide given tags let prune_status = match self.validated_pool.prune_tags(tags) { Ok(prune_status) => prune_status, @@ -257,6 +258,7 @@ impl Pool { let pruned_transactions = prune_status.pruned.into_iter().map(|tx| tx.data.clone()); let reverify_future = self.verify(at, pruned_transactions, false); + log::trace!(target: "txpool", "Prunning at {:?}. Resubmitting transactions.", at); // And finally - submit reverified transactions back to the pool let at = at.clone(); let validated_pool = self.validated_pool.clone(); @@ -908,3 +910,4 @@ mod tests { } } } + diff --git a/substrate/core/transaction-pool/graph/src/ready.rs b/substrate/core/transaction-pool/graph/src/ready.rs index 85bb4dd783..3698bf447e 100644 --- a/substrate/core/transaction-pool/graph/src/ready.rs +++ b/substrate/core/transaction-pool/graph/src/ready.rs @@ -338,7 +338,20 @@ impl ReadyTransactions { } } - debug!(target: "txpool", "[{:?}] Pruned.", tx.hash); + // we also need to remove all other tags that this transaction provides, + // but since all the hard work is done, we only clear the provided_tag -> hash + // mapping. + let current_tag = &tag; + for tag in &tx.provides { + let removed = self.provided_tags.remove(tag); + assert_eq!( + removed.as_ref(), + if current_tag == tag { None } else { Some(&tx.hash) }, + "The pool contains exactly one transaction providing given tag; the removed transaction + claims to provide that tag, so it has to be mapped to it's hash; qed" + ); + } + removed.push(tx); } } diff --git a/substrate/core/transaction-pool/src/tests.rs b/substrate/core/transaction-pool/src/tests.rs index d1ad27dd26..60a9e0562f 100644 --- a/substrate/core/transaction-pool/src/tests.rs +++ b/substrate/core/transaction-pool/src/tests.rs @@ -27,11 +27,15 @@ use sr_primitives::{ transaction_validity::{TransactionValidity, ValidTransaction}, }; -struct TestApi; +struct TestApi { + pub modifier: Box, +} impl TestApi { fn default() -> Self { - TestApi + TestApi { + modifier: Box::new(|_| {}), + } } } @@ -54,14 +58,18 @@ impl txpool::ChainApi for TestApi { }; let provides = vec![vec![uxt.transfer().nonce as u8]]; + let mut validity = ValidTransaction { + priority: 1, + requires, + provides, + longevity: 64, + propagate: true, + }; + + (self.modifier)(&mut validity); + futures::future::ready(Ok( - Ok(ValidTransaction { - priority: 1, - requires, - provides, - longevity: 64, - propagate: true, - }) + Ok(validity) )) } @@ -181,3 +189,34 @@ fn should_ban_invalid_transactions() { // then block_on(pool.submit_one(&BlockId::number(0), uxt.clone())).unwrap_err(); } + +#[test] +fn should_correctly_prune_transactions_providing_more_than_one_tag() { + let mut api = TestApi::default(); + api.modifier = Box::new(|v: &mut ValidTransaction| { + v.provides.push(vec![155]); + }); + let pool = Pool::new(Default::default(), api); + let xt = uxt(Alice, 209); + block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + assert_eq!(pool.status().ready, 1); + + // remove the transaction that just got imported. + block_on(pool.prune_tags(&BlockId::number(1), vec![vec![209]], vec![])).expect("1. Pruned"); + assert_eq!(pool.status().ready, 0); + // it's re-imported to future + assert_eq!(pool.status().future, 1); + + // so now let's insert another transaction that also provides the 155 + let xt = uxt(Alice, 211); + block_on(pool.submit_one(&BlockId::number(2), xt.clone())).expect("2. Imported"); + assert_eq!(pool.status().ready, 1); + assert_eq!(pool.status().future, 1); + let pending: Vec<_> = pool.ready().map(|a| a.data.transfer().nonce).collect(); + assert_eq!(pending, vec![211]); + + // prune it and make sure the pool is empty + block_on(pool.prune_tags(&BlockId::number(3), vec![vec![155]], vec![])).expect("2. Pruned"); + assert_eq!(pool.status().ready, 0); + assert_eq!(pool.status().future, 2); +}