transaction-status: Add return data to meta #23688
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23688 +/- ##
=========================================
- Coverage 81.7% 81.7% -0.1%
=========================================
Files 583 580 -3
Lines 158720 158428 -292
=========================================
- Hits 129767 129528 -239
+ Misses 28953 28900 -53 |
|
Another thing I was thinking about -- are there any concerns with adding fields to RPC return data? Or do all of the old versions of client libraries correctly handle unknown fields? |
sdk/src/transaction_context.rs
Outdated
| ) -> ( | ||
| Vec<TransactionAccount>, | ||
| Vec<Vec<InstructionContext>>, | ||
| TransactionReturnData, | ||
| ) { |
There was a problem hiding this comment.
I went with TransactionRecord, let me know if you prefer another name
There was a problem hiding this comment.
There's already a TransactionRecorder in solana-poh, which is pretty close 🤔
Maybe something like ExecutionRecord?
There was a problem hiding this comment.
I can get down with that -- done!
No concerns; adding new fields is acceptable per our backward compatibility policy (and yes, our clients correctly handle unknown fields). |
|
Let me know if you have any other issues, but that should be everything |
mergify
bot
dismissed
CriesofCarrots’s stale review
Pull request has been modified.
Problem
Programs can set return data, but that information isn't propagated outside of programs.
Summary of Changes
Expose all return data information by adding it in
TransactionStatusMetaandTransactionSimulationResults. For (slightly?) easier review, the first commit is just adding all of the wiring everywhere to expose in the meta, and the second commit is all about adding it to simulations.The bank only propagates return data if it's not all 0, and cuts off at the first non-zero byte starting from the end.
Fixes #