-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: the web3.js getBlock APIs now accept rewards
and transactionDetails
config
#29000
feat: the web3.js getBlock APIs now accept rewards
and transactionDetails
config
#29000
Conversation
…because it's such a PITA to share config between two methods and not have Typedoc throw a fit.
No tests, because what would we test? All a test would test is param serialization and forwarding. |
j/k. Looks like tests are in order because |
Codecov Report
@@ Coverage Diff @@
## master #29000 +/- ##
=========================================
- Coverage 77.1% 76.7% -0.4%
=========================================
Files 55 55
Lines 2934 3112 +178
Branches 408 460 +52
=========================================
+ Hits 2264 2390 +126
- Misses 529 558 +29
- Partials 141 164 +23 |
@@ -3071,6 +3071,229 @@ describe('Connection', function () { | |||
}); | |||
} | |||
|
|||
describe('get parsed block', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled all of these sample bits of data from the RPC today.
async getBlock( | ||
slot: number, | ||
rawConfig: GetBlockConfig & {transactionDetails: 'accounts'}, | ||
): Promise<AccountsModeBlockResponse | null>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how we should have done getParsedBlock
back in the day. This method signature overload tells Typescript what shape the return value should have in the case that this method is called with exactly accounts
as the transactionDetails
property. We could have done the same with the encoding
property, obviating the need for the getParsedBlock
method altogether.
cc/ @jordansexton, @ngundotra, @lwus, @2501babe, @timhagn, @catalinred, @nramadas for the TypeScript trivia.
| ParsedBlockResponse | ||
| ParsedAccountsModeBlockResponse | ||
| ParsedNoneModeBlockResponse | ||
| null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base signature just has to be the union of all the possible return types.
Note to self: I spot checked this branch: solana/transaction-status/src/lib.rs Lines 801 to 805 in 5850af5
And my conclusion is that even transactions that are missing metadata will deserialize fine because solana/web3.js/src/connection.ts Lines 2126 to 2132 in 5850af5
|
…Details` config (solana-labs#29000) * Add `transactionDetails` and `rewards` params to `getBlock` API of web3.js * Add the same content to the legacy call …because it's such a PITA to share config between two methods and not have Typedoc throw a fit. * Add tests to exercise block deserialization in the case that `transactionDetails` is `none` or `accounts` * Extract the annotated account key parser into a separate struct * Parse the `getBlock()` responses differently depending on the mode
…Details` config (solana-labs#29000) * Add `transactionDetails` and `rewards` params to `getBlock` API of web3.js * Add the same content to the legacy call …because it's such a PITA to share config between two methods and not have Typedoc throw a fit. * Add tests to exercise block deserialization in the case that `transactionDetails` is `none` or `accounts` * Extract the annotated account key parser into a separate struct * Parse the `getBlock()` responses differently depending on the mode
Problem
These were added to the RPC interface but never to this client library.
Summary of Changes
transactionDetails
param.Fixes #28999.