From 6f0c80c8268590248c7ceb978d909647d05a491c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 14 Apr 2023 00:09:26 +0200 Subject: [PATCH] Unqueue invalid transactions when skipping 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/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index 376278fa1b6db..795288d0a1da8 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/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();