feat: support committing fee-only transactions #2425
Conversation
|
@apfitzge do you mind browsing through this PR to see if the direction looks reasonable? If it looks ok, I will break up the changes into smaller PRs and add testing. |
svm/src/transaction_processor.rs
Outdated
| Ok(loaded_transaction) => { | ||
| TransactionLoadResult::NotLoaded(err) => Err(err), | ||
| TransactionLoadResult::FeesOnly(fees_only_tx) => { | ||
| if enable_transaction_failure_fees { |
There was a problem hiding this comment.
This is where we opt in to the new behavior of committing fees-only transactions
Overall approach seems very solid, didn't dig into details yet but the separateion during acct loading and then again during committing makes sense to me. |
jstarry
force-pushed
the
feat/tx-fees-only
branch
3 times, most recently
from
16314bb to
3eb5127
Compare
jstarry
force-pushed
the
feat/tx-fees-only
branch
6 times, most recently
from
a420bd8 to
0ebc876
Compare
jstarry
force-pushed
the
feat/tx-fees-only
branch
from
0ebc876 to
29f2058
Compare
|
This is now ready for review. I put the implementation changes in the first commit and test changes in the second commit to help with review. |
| pub enum ProcessedTransaction { | ||
| /// Transaction was executed, but if execution failed, all account state changes | ||
| /// will be rolled back except deducted fees and any advanced nonces | ||
| Executed(Box<ExecutedTransaction>), | ||
| /// Transaction was not able to be executed but fees are able to be | ||
| /// collected and any nonces are advanceable | ||
| FeesOnly(Box<FeesOnlyTransaction>), | ||
| } |
There was a problem hiding this comment.
Outside the scope of this PR, but we may want to re-evaluate this at some later point.
Doing a box-allocation per transaction is likely bad for performance - though I'm sure there are many such sins in our current pipeline.
| ]; | ||
| let initial_balance = bank.get_balance(&leader); | ||
|
|
||
| bank.filter_program_errors_and_collect_fee(&processing_results); | ||
| bank.freeze(); | ||
| assert_eq!( | ||
| bank.get_balance(&leader), | ||
| initial_balance + bank.fee_rate_governor.burn(tx_fee * 2).0 | ||
| initial_balance + bank.fee_rate_governor.burn(tx_fee * 3).0 |
There was a problem hiding this comment.
Note for myself.
Change to 3 is because there are 2 added results:
- Err => no fees
- FeesOnly => fees
* feat: support committing fee-only transactions * update and add new tests * fix sbf test * feedback * fix new test * test cleanup
Problem
In order to implement solana-foundation/solana-improvement-documents#82, the transaction pipeline needs to support committing validated transactions that are not executable to a block.
Summary of Changes
ProcessedTransactionenum to represent transactions processed by the runtime but not necessarily executedFixes #