Add loaded-accounts-data-size field to sim result#6023
Conversation
|
If this PR represents a change to the public RPC API:
Thank you for keeping the RPC clients in sync with the server API @tao-stones. |
bcdf312 to
ba0083c
Compare
|
result of running in testnet that includes both |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6023 +/- ##
=========================================
- Coverage 82.9% 82.9% -0.1%
=========================================
Files 842 842
Lines 377389 377450 +61
=========================================
+ Hits 312874 312917 +43
- Misses 64515 64533 +18 🚀 New features to boost your workflow:
|
|
spot checked transactions in testnet, PoC RPC returns exact same simulation results except with |
|
Hi @steveluscher , this poc is to add an additional field |
0423680 to
2671a99
Compare
|
@2501babe, just a heads-up about this PR that adds loaded data size to Simulation RPC result. It relates to the SIMD-0186 changes you are working on. I don't see any direct conflicts between the two, except we probably should communicate users that once your feature is activated, they might see significantly different values in RPC results (if this PR lands). |
|
i highly support this pr! letting wallets get the size from simulation and then set the compute budget after was exactly the flow i had in mind. will review next week one thing worth noting is if you simulate a transaction and then add a compute budget instruction to it you may need to add 22 to the requested size to account for the compute budget pseudoprogram. we should report the correct size for whatever transaction we are given but whoever does the frontend code should probably take that into account we definitely want to communicate with foundation after this pr and my pr land. if end users implement a flow like "simulate, get the number, add compute budget instruction based on the number" then they should be future-proofed and it shouldnt be a problem if this goes live before simd186 activates |
steveluscher
left a comment
There was a problem hiding this comment.
Sorry for the huge delay in reviewing this. Looks great!
If you have time, please do submit a PR to Kit to add this field to the RPC response. If you don't, let me know and I'll find someone.
Thanks for kindly reminding, haven't PR-ed in Kit, but happy to give it a try. |
* Add loaded-accounts-data-size field to sim result * increase loaded_accounts_data_size visibility to public * update svm examples * update rpc tests with additional field
Problem
For issue #5992
Summary of Changes
loaded_accounts_data_sizeto simulation response (implementing proposal 2)Note for reviewers:
By adding an additional field to JSON result, it is likely to be backward compatible - most JSON parsers and clients would ignore additional field, but not guaranteed for users perform strict schema validation. Would like feedback if this is an acceptable practice.
Fixes #5992