-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
rpc: add context toggle to getProgramAccounts #17399
Conversation
8d4976f
to
d575af4
Compare
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 a nice improvement, thank you!
cli/src/cluster_query.rs
Outdated
@@ -1681,6 +1681,7 @@ pub fn process_show_stakes( | |||
encoding: Some(solana_account_decoder::UiAccountEncoding::Base64), | |||
..RpcAccountInfoConfig::default() | |||
}, | |||
with_context: None, |
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.
How about this instead:
..RpcProgramAccountsConfig::default()
so we don't need to touch all these places the next time a new field is added. In some cases it looks like doing so would allow us to remove filters: None,
too
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.
Sure. Fixed
d575af4
to
76daa23
Compare
76daa23
to
745ec5c
Compare
Nice, I bet there are a few other rpc methods that could take advantage of |
Co-authored-by: Michael Vines <[email protected]>
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.
lgtm with my documentation nits picked
Co-authored-by: Michael Vines <[email protected]>
Thanks! Glad to help! |
automerge label removed due to a CI failure |
Downstream projects build fix landing via https://github.com/solana-labs/solana-program-library/pull/1766/files, once it does I'll retrigger the failed build here |
Codecov Report
@@ Coverage Diff @@
## master #17399 +/- ##
=========================================
- Coverage 82.8% 82.7% -0.1%
=========================================
Files 424 424
Lines 118541 118554 +13
=========================================
- Hits 98205 98150 -55
- Misses 20336 20404 +68 |
* fix(rpc): return context in get_program_accounts * doc(rpc): document withContext flag * fix(rpc): fix comment Co-authored-by: Michael Vines <[email protected]> * fix(rpc): fix doc Co-authored-by: Michael Vines <[email protected]> Co-authored-by: Michael Vines <[email protected]> (cherry picked from commit d41266e)
* fix(rpc): return context in get_program_accounts * doc(rpc): document withContext flag * fix(rpc): fix comment Co-authored-by: Michael Vines <[email protected]> * fix(rpc): fix doc Co-authored-by: Michael Vines <[email protected]> Co-authored-by: Michael Vines <[email protected]> (cherry picked from commit d41266e) Co-authored-by: Nikita <[email protected]>
Problem
There is no way in current implementation to get
getProgramAccounts
response with context.Summary of Changes
Added an optional flag (
with_context
) ingetProgramAccounts
config to set the format of the response. The implementation is simple: if it is present and istrue
, the response will be wrapped in RpcResponse, otherwise it defaults to array. While it feels like returning RpcResponse should be the default behavior, there is a big chance that someone relies on the current format of the API. This might seem like not the most elegant solution, but it's backwards compatible.