Don't forward transactions that are expired or failed signature check #4199
Conversation
pgarg66
requested review from
mvines,
aeyakovenko,
carllin,
sagar-solana and
garious
core/src/banking_stage.rs
Outdated
| @@ -514,9 +543,10 @@ impl BankingStage { | |||
|
|
|||
| let tx_len = verified_transactions.len(); | |||
|
|
|||
| let (processed, unprocessed_txs) = f(bank, &verified_transactions, poh)?; | |||
| let (processed, unprocessed_txs) = fn_proc(bank, &verified_transactions, poh)?; | |||
There was a problem hiding this comment.
This code looks unnecessarily awkward. It looks like breaking it up would make it possible to write without closures and minimal copypasta. What if we moved the code above into its own function? Then the caller would invoke that new function and use it to create (processed, unprocessed_txs) and valid_unprocessed_txs directly.
There was a problem hiding this comment.
The latest patch should untangle this function.
core/src/banking_stage.rs
Outdated
| @@ -480,15 +481,43 @@ impl BankingStage { | |||
| Ok((chunk_start, unprocessed_txs)) | |||
| } | |||
|
|
|||
| fn process_received_packets_using_closure<'a, F>( | |||
| fn filter_out_invalid_transactions( | |||
There was a problem hiding this comment.
This function can use a couple tests and a code comment. Probably a better name too (filter_valid_transaction_indexes?)
There was a problem hiding this comment.
Reworked the function and added some tests.
Codecov Report
@@ Coverage Diff @@
## master #4199 +/- ##
========================================
+ Coverage 78.6% 78.6% +<.1%
========================================
Files 164 164
Lines 28263 28296 +33
========================================
+ Hits 22234 22268 +34
+ Misses 6029 6028 -1 |
|
@sagar-solana we should also drop anything that can't afford the fee (could be done in a different PR though) |
Problem
The old leader can potentially forward expired transactions to the new leader. This puts more pressure on leader's pipeline stages (e.g. SigVerify and BankingStage), and create a backlog of transaction. If this backlog grows, the node can run out of memory.
Summary of Changes
Drop the transactions that have expired (blockhash is too old).
Fixes #3959