feat(rpc): add getMultipleAccounts implementation#9068
feat(rpc): add getMultipleAccounts implementation#9068jherrera-jump merged 4 commits intofiredancer-io:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an implementation of the Solana JSON-RPC getMultipleAccounts endpoint in Firedancer’s RPC tile, enabling batch account fetches from accdb with configurable encoding/slicing behavior.
Changes:
- Implement
getMultipleAccountsrequest validation, account lookup, and JSON response streaming. - Add support for per-account data slicing and base58/base64/binary encoding output (per shared config parsing).
| int is_base58 = !strncmp( encoding_cstr, "base58", 6 ); | ||
| int is_binary = !strncmp( encoding_cstr, "binary", 6 ); | ||
|
|
There was a problem hiding this comment.
fd_rpc_validate_config allows encoding: "base64+zstd", but this implementation never performs ZSTD compression. As a result, responses may advertise "base64+zstd" while actually returning plain base64 (and it also doesn't error when ZSTD support is disabled). Add base64+zstd handling equivalent to getAccountInfo (including the FD_HAS_ZSTD gated error path), or explicitly reject base64+zstd for this method to avoid returning incorrectly encoded data.
| ulong data_sz = fd_accdb_ref_data_sz( ro ); | ||
| uchar const * data = (uchar const *)fd_accdb_ref_data_const( ro ) + fd_ulong_if( slice_offset<data_sz, slice_offset, 0UL ); | ||
| ulong snip_sz = fd_ulong_min( fd_ulong_if( slice_offset<data_sz, data_sz-slice_offset, 0UL ), slice_length ); | ||
| ulong out_sz = snip_sz; | ||
|
|
There was a problem hiding this comment.
This method duplicates the account encoding/serialization logic used by getAccountInfo. The duplication has already drifted (e.g., base64+zstd handling), which makes it easy for the two endpoints to become inconsistent over time. Consider extracting a shared helper for “emit UiAccount JSON + encoded data” so both methods stay in sync.
jherrera-jump
left a comment
jherrera-jump
left a comment
There was a problem hiding this comment.
Some nits. I'll likely approve later when I have some time to validate/tidy up missing parts in a follow-up PR.
| int is_base58 = !strncmp( encoding_cstr, "base58", 6 ); | ||
| int is_binary = !strncmp( encoding_cstr, "binary", 6 ); | ||
|
|
There was a problem hiding this comment.
encoding accepts "base64+zstd" via fd_rpc_validate_config, but this implementation never applies zstd compression and will still return the encoding string "base64+zstd". This makes the response payload inconsistent with the declared encoding (and differs from getAccountInfo, which compresses / errors when zstd is disabled). Consider adding the same base64+zstd handling as getAccountInfo or rejecting it here with a clear error.
| ulong slice_offset = 0; | ||
| cJSON const * config = cJSON_GetArrayItem( params, 1 ); | ||
| int config_valid = fd_rpc_validate_config( ctx, id, config, "struct RpcAccountInfoConfig", | ||
| 1, 1, 1, 1, |
There was a problem hiding this comment.
The fd_rpc_validate_config call uses bare 1, 1, 1, 1 flags without the inline /* has_* */ comments used elsewhere in this file (e.g., getAccountInfo). Adding the same comments would make it easier to review and prevent accidental argument-order bugs when this call is modified later.
| 1, 1, 1, 1, | |
| 1, /* has_commitment */ | |
| 1, /* has_min_context_slot */ | |
| 1, /* has_encoding */ | |
| 1, /* has_data_slice */ |
| static fd_http_server_response_t | ||
| getMultipleAccounts( fd_rpc_tile_t * ctx, | ||
| cJSON const * id, | ||
| cJSON const * params ) { | ||
| fd_http_server_response_t response; | ||
| if( FD_UNLIKELY( !fd_rpc_validate_params( ctx, id, params, 1, 2, &response ) ) ) return response; | ||
|
|
||
| cJSON const * keys_arr = cJSON_GetArrayItem( params, 0 ); | ||
| if( FD_UNLIKELY( !keys_arr || !cJSON_IsArray( keys_arr ) ) ) { | ||
| CSTR_JSON( id, id_cstr ); | ||
| return PRINTF_JSON( ctx, "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32602,\"message\":\"Invalid params: expected array of pubkeys\"},\"id\":%s}\n", id_cstr ); | ||
| } |
There was a problem hiding this comment.
getMultipleAccounts introduces a new RPC method with multiple branches (param validation, account lookup loop, encoding/dataSlice handling, error paths), but there are currently no corresponding cases in src/discof/rpc/test_rpc_tile.py (unlike getAccountInfo). Adding targeted tests (at least: success with 1-2 known accounts, mixed missing accounts returning null, base64/base58/binary, and base64+zstd or its expected error) would help prevent regressions and ensure Agave-compatibility.
No description provided.