Add (preflight) simulation to BanksClient#22084
Add (preflight) simulation to BanksClient#22084mergify[bot] merged 7 commits intosolana-labs:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22084 +/- ##
=========================================
- Coverage 81.2% 81.1% -0.1%
=========================================
Files 521 521
Lines 146060 146126 +66
=========================================
+ Hits 118630 118632 +2
- Misses 27430 27494 +64 |
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Just a few thoughts, looking good so far!
CriesofCarrots
force-pushed
the
banks-simulate
branch
2 times, most recently
from
9e5cc71 to
e83f548
Compare
|
@joncinque , ready for proper review! I verified this against the case that came up in |
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks great! Just one tiny nit, which you can absolutely take or leave
CriesofCarrots
force-pushed
the
banks-simulate
branch
from
e83f548 to
00d004f
Compare
mergify
Bot
dismissed
joncinque’s stale review
Pull request has been modified.
CriesofCarrots
added
the
automerge
* Add more-legitimate conversion from legacy Transaction to SanitizedTransaction * Add Banks method with preflight checks * Expose BanksClient method with preflight checks * Unwrap simulation err * Add Bank simulation method that works on unfrozen Banks * Add simpler api * Better name: BanksTransactionResultWithSimulation (cherry picked from commit 422a095)
* Add more-legitimate conversion from legacy Transaction to SanitizedTransaction * Add Banks method with preflight checks * Expose BanksClient method with preflight checks * Unwrap simulation err * Add Bank simulation method that works on unfrozen Banks * Add simpler api * Better name: BanksTransactionResultWithSimulation (cherry picked from commit 422a095) Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Oops, I never responded to this part. I didn't have any specific test cases unfortunately |
No worries, wrote my own post-merge. |
|
This PR has been automatically locked since there has not been any activity in past 14 days after it was merged. |
2 participants
Problem
BanksServer operation is confusing or clunky for cases where a transaction does not land (successfully or with InstructionError). See #22029 for more details and examples.
Summary of Changes
Add Banks method with preflight checks, and expose via the BanksClient::process_transaction_with_preflight() method.
Consider adding stronger comments recommending this api.
Fixes #22029
cc @joncinque