Make sure to ban invalid transactions. (#615)

polkadot/transaction-pool/src/lib.rs

@@ -421,6 +421,12 @@ A: PolkadotApi,
 
 	/// Remove a set of transactions idenitified by hashes.
 	pub fn remove(&self, hashes: &[Hash], is_valid: bool) -> Vec<Option<Arc<VerifiedTransaction>>> {
+		// temporarily ban invalid transactions
+		if !is_valid {
+			debug!(target: "transaction-pool", "Banning invalid transactions: {:?}", hashes);
+			self.rotator.ban(&Instant::now(), hashes);
+		}
+
 		self.inner.remove(hashes, is_valid)
 	}
 }
@@ -763,6 +769,24 @@ mod tests {
 
 		let pending: Vec<_> = pool.cull_and_get_pending(BlockId::number(1), |p| p.map(|a| (a.sender(), a.index())).collect()).unwrap();
 		assert_eq!(pending, vec![]);
+	}
+
+	#[test]
+	fn should_ban_invalid_transactions() {
+		let api = TestPolkadotApi::default();
+		let pool = pool(&api);
+		let uxt = uxt(Alice, 209, true);
+		let hash = *pool.import_unchecked_extrinsic(BlockId::number(0), uxt.clone()).unwrap().hash();
+		pool.remove(&[hash], true);
+		pool.import_unchecked_extrinsic(BlockId::number(0), uxt.clone()).unwrap();
+
+		// when
+		pool.remove(&[hash], false);
+		let pending: Vec<_> = pool.cull_and_get_pending(BlockId::number(0), |p| p.map(|a| (a.sender(), a.index())).collect()).unwrap();
+		assert_eq!(pending, vec![]);
 
+		// then
+		pool.import_unchecked_extrinsic(BlockId::number(0), uxt.clone()).unwrap_err();
 	}
+
 }

polkadot/transaction-pool/src/rotator.rs

@@ -56,24 +56,32 @@ impl PoolRotator {
 		self.banned_until.read().contains_key(hash)
 	}
 
-	/// Bans extrinsic if it's stale.
-	///
-	/// Returns `true` if extrinsic is stale and got banned.
-	pub fn ban_if_stale(&self, now: &Instant, tx: &VerifiedTransaction) -> bool {
-		if &tx.valid_till > now {
-			return false;
+	/// Bans given set of hashes.
+	pub fn ban(&self, now: &Instant, hashes: &[Hash]) {
+		let mut banned = self.banned_until.write();
+
+		for hash in hashes {
+			banned.insert(*hash, *now + self.ban_time);
 		}
 
-		let mut banned = self.banned_until.write();
-		banned.insert(*tx.hash(), *now + self.ban_time);
 		if banned.len() > 2 * EXPECTED_SIZE {
 			while banned.len() > EXPECTED_SIZE {
 				if let Some(key) = banned.keys().next().cloned() {
 					banned.remove(&key);
 				}
 			}
 		}
+	}
+
+	/// Bans extrinsic if it's stale.
+	///
+	/// Returns `true` if extrinsic is stale and got banned.
+	pub fn ban_if_stale(&self, now: &Instant, tx: &VerifiedTransaction) -> bool {
+		if &tx.valid_till > now {
+			return false;
+		}
 
+		self.ban(now, &[*tx.hash()]);
 		true
 	}