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

Extract account-decoder-client-types #2872

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

kevinheavey
Copy link

Problem

rpc_client needs only a small subset of account_decoder. It brings in some very heavy dependencies such as spl-token-2022, which we can avoid

Summary of Changes

  • Rip the necessary types for rpc_client and rpc_client_api from acocunt_decoder into a new account_decoder_client_types crate.
  • Make zstd optional in the new crate
  • convert UiAccount::encode to a free func encode_ui_account
  • replace a bunch of From impls with free functions to avoid pulling in dependencies. These impls do not fall under the supported APIs outlined here: https://docs.solanalabs.com/backwards-compatibility#rust-crates

On my machine this improves release build time for rpc_client_api by 36%.

This PR branches off #2870 so either that should be merged first, or we merge this as one big PR and close #2870

Copy link

mergify bot commented Sep 9, 2024

If this PR represents a change to the public RPC API:

  1. Make sure it includes a complementary update to rpc-client/ (example)
  2. Open a follow-up PR to update the JavaScript client @solana/web3.js (example)

Thank you for keeping the RPC clients in sync with the server API @kevinheavey.

@kevinheavey kevinheavey marked this pull request as ready for review September 12, 2024 15:37
@kevinheavey kevinheavey force-pushed the account-decoder-client-types branch 2 times, most recently from e120ff4 to 489e4de Compare September 27, 2024 08:03
@kevinheavey kevinheavey force-pushed the account-decoder-client-types branch 4 times, most recently from 83516f1 to ff5fac0 Compare October 7, 2024 21:22
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm @kevinheavey, just the question on decode!

Comment on lines +27 to +28
all-features = true
rustdoc-args = ["--cfg=docsrs"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these extra keys necessary here? I don't see them used anywhere else in the monorepo.


impl UiAccountData {
/// Returns decoded account data in binary format if possible
pub fn decode(&self) -> Option<Vec<u8>> {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not also make this one a free function, like its encode counterpart? Then you can drop the bs_58 dependency from this crate, possibly more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants