Skip to content
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

Merged
merged 5 commits into from Dec 2, 2022
Merged

feat: the web3.js getBlock APIs now accept rewards and transactionDetails config #29000

merged 5 commits into from Dec 2, 2022

Conversation

steveluscher
Copy link
Contributor

@steveluscher steveluscher commented Nov 30, 2022

Problem

These were added to the RPC interface but never to this client library.

Summary of Changes

  • Add Typescript types for the missing params.
  • Add structs to deserialize the different kinds of responses that result when you change the transactionDetails param.

Fixes #28999.

…because it's such a PITA to share config between two methods and not have Typedoc throw a fit.
@github-actions github-actions bot added the web3.js Related to the JavaScript client label Nov 30, 2022
@steveluscher
Copy link
Contributor Author

No tests, because what would we test? All a test would test is param serialization and forwarding.

@steveluscher
Copy link
Contributor Author

j/k. Looks like tests are in order because none and accounts fail to deserialize the response.

@steveluscher steveluscher marked this pull request as draft November 30, 2022 21:57
@codecov
Copy link

codecov bot commented Nov 30, 2022

Codecov Report

Merging #29000 (e04dfce) into master (4267a15) will decrease coverage by 0.3%.
The diff coverage is n/a.

@@            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     

@steveluscher steveluscher marked this pull request as ready for review December 1, 2022 01:36
@@ -3071,6 +3071,229 @@ describe('Connection', function () {
});
}

describe('get parsed block', function () {
Copy link
Contributor Author

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.

Comment on lines +4420 to +4423
async getBlock(
slot: number,
rawConfig: GetBlockConfig & {transactionDetails: 'accounts'},
): Promise<AccountsModeBlockResponse | null>;
Copy link
Contributor Author

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.

Comment on lines +4551 to +4554
| ParsedBlockResponse
| ParsedAccountsModeBlockResponse
| ParsedNoneModeBlockResponse
| null
Copy link
Contributor Author

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.

@steveluscher
Copy link
Contributor Author

Note to self: I spot checked this branch:

Self::MissingMetadata(ref transaction) => Ok(EncodedTransactionWithStatusMeta {
version: None,
transaction: transaction.encode(encoding),
meta: None,
}),

And my conclusion is that even transactions that are missing metadata will deserialize fine because meta and version are marked as nullable and optional respectively in the JS deserializer.

transactions: array(
pick({
transaction: ConfirmedTransactionResult,
meta: nullable(ConfirmedTransactionMetaResult),
version: optional(TransactionVersionStruct),
}),
),

@steveluscher steveluscher merged commit b112d01 into solana-labs:master Dec 2, 2022
@steveluscher steveluscher deleted the update-get-block-api branch December 2, 2022 06:04
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
…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
nickfrosty pushed a commit to nickfrosty/solana that referenced this pull request Jan 4, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
web3.js Related to the JavaScript client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

web3.js: Add transactionDetails config to getBlock
1 participant