From d1f862a30822ea6ebe67549a9e332c00a5c16884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 14 Apr 2023 10:21:06 +0200 Subject: [PATCH] Unqueue invalid transactions when skipping (#13918) The check was intially added by: https://github.com/paritytech/substrate/pull/5121 But a later pr fixed it properly: https://github.com/paritytech/substrate/pull/9789 TLDR: We don't need to check for skipped anymore, as we already remove all transactions that are being unlocked by an invalid transactions and thus, we will not tag them as invalid directly because the nonce for example is incorrect. Fixes: https://github.com/paritytech/substrate/issues/13911 --- .../client/basic-authorship/src/basic_authorship.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/substrate/client/basic-authorship/src/basic_authorship.rs b/substrate/client/basic-authorship/src/basic_authorship.rs index 376278fa1b..795288d0a1 100644 --- a/substrate/client/basic-authorship/src/basic_authorship.rs +++ b/substrate/client/basic-authorship/src/basic_authorship.rs @@ -474,14 +474,6 @@ where break EndProposingReason::HitBlockWeightLimit } }, - Err(e) if skipped > 0 => { - pending_iterator.report_invalid(&pending_tx); - trace!( - "[{:?}] Ignoring invalid transaction when skipping: {}", - pending_tx_hash, - e - ); - }, Err(e) => { pending_iterator.report_invalid(&pending_tx); debug!("[{:?}] Invalid transaction: {}", pending_tx_hash, e); @@ -740,8 +732,11 @@ mod tests { ); } + // This test ensures that if one transaction of a user was rejected, because for example + // the weight limit was hit, we don't mark the other transactions of the user as invalid because + // the nonce is not matching. #[test] - fn should_not_remove_invalid_transactions_when_skipping() { + fn should_not_remove_invalid_transactions_from_the_same_sender_after_one_was_invalid() { // given let mut client = Arc::new(substrate_test_runtime_client::new()); let spawner = sp_core::testing::TaskExecutor::new();