Add missing fields to rpc Server.getLatestLedger response#1389
Add missing fields to rpc Server.getLatestLedger response#1389
Conversation
There was a problem hiding this comment.
Pull request overview
Updates rpc.Server.getLatestLedger() to return a richer, parsed response object (including additional fields and decoded XDR) instead of the raw RPC payload.
Changes:
- Added parsing logic for
getLatestLedgerto decodeheaderXdr/metadataXdrinto XDR objects. - Introduced a raw response type and a raw
_getLatestLedger()method. - Updated unit tests to validate the new parsed response shape and adjust mocks used by other tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/server/soroban/get_latest_ledger.test.ts | Updates expectations to match parsed getLatestLedger response, including XDR decoding. |
| test/unit/server/soroban/get_classic_entries.test.ts | Adjusts HTTP mocks to handle getLatestLedger being called (e.g., via getAssetBalance). |
| src/rpc/server.ts | Routes getLatestLedger() through a new parser and adds _getLatestLedger() for raw access. |
| src/rpc/parsers.ts | Adds parseRawLatestLedger to decode XDR and shape the typed response. |
| src/rpc/api.ts | Extends GetLatestLedgerResponse and adds RawGetLatestLedgerResponse typing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** a base-64 encoded {@link xdr.LedgerHeaderHistoryEntry} instance */ | ||
| headerXdr: xdr.LedgerHeader; | ||
| /** a base-64 encoded {@link xdr.LedgerCloseMeta} instance */ |
There was a problem hiding this comment.
GetLatestLedgerResponse JSDoc/typing is internally inconsistent: the comments say headerXdr/metadataXdr are base64-encoded XDR strings (and even reference xdr.LedgerHeaderHistoryEntry), but the types are parsed XDR objects (xdr.LedgerHeader / xdr.LedgerCloseMeta). Please update the docs to match the actual types (and ensure the referenced XDR type matches what parseRawLatestLedger returns).
| /** a base-64 encoded {@link xdr.LedgerHeaderHistoryEntry} instance */ | |
| headerXdr: xdr.LedgerHeader; | |
| /** a base-64 encoded {@link xdr.LedgerCloseMeta} instance */ | |
| /** a parsed {@link xdr.LedgerHeader} instance */ | |
| headerXdr: xdr.LedgerHeader; | |
| /** a parsed {@link xdr.LedgerCloseMeta} instance */ |
| raw: Api.RawGetLatestLedgerResponse, | ||
| ): Api.GetLatestLedgerResponse { | ||
| if (!raw.headerXdr || !raw.metadataXdr) { | ||
| throw new TypeError(`invalid response missing fields`); |
There was a problem hiding this comment.
parseRawLatestLedger throws a generic invalid response missing fields error when headerXdr/metadataXdr are absent. For debugging, please include which field(s) are missing (similar to parseRawLedger) and ideally mention the RPC method/response type in the message.
| throw new TypeError(`invalid response missing fields`); | |
| let missingFields: string; | |
| if (!raw.metadataXdr && !raw.headerXdr) { | |
| missingFields = "metadataXdr and headerXdr"; | |
| } else if (!raw.metadataXdr) { | |
| missingFields = "metadataXdr"; | |
| } else { | |
| missingFields = "headerXdr"; | |
| } | |
| throw new TypeError( | |
| `invalid getLatestLedger response missing fields: ${missingFields}`, | |
| ); |
There was a problem hiding this comment.
This suggestion returns a better error for the user
| @@ -23,12 +24,25 @@ describe("Server#getLatestLedger", () => { | |||
| id: "hashed_id", | |||
| sequence: 123, | |||
| protocolVersion: 20, | |||
There was a problem hiding this comment.
Type mismatch: Api.RawGetLatestLedgerResponse.protocolVersion is declared as a string, but this mock response uses a number (20). This makes it easy for runtime/type drift to slip through—either update the mock to a string value or adjust the API types/parsing if the RPC actually returns a number.
| protocolVersion: 20, | |
| protocolVersion: "20", |
| result: { | ||
| id: "hashed_id", | ||
| sequence: 123, | ||
| protocolVersion: 20, |
There was a problem hiding this comment.
Type mismatch: Api.RawGetLatestLedgerResponse.protocolVersion is declared as a string, but this getLatestLedger mock returns a number (20). Consider using a string here (or updating the API types/parsing if the RPC response is numeric) so tests validate the intended schema.
| protocolVersion: 20, | |
| protocolVersion: "20", |
There was a problem hiding this comment.
We define protocolVersion as a string in GetLatestLedgerResponse. Is this using a different type?
|
Size Change: +75.7 kB (+0.17%) Total Size: 45.7 MB 📦 View Changed
|
| ## Unreleased | ||
|
|
||
| ### Added | ||
| * Added |
There was a problem hiding this comment.
Nit: finish adding added 😉
| raw: Api.RawGetLatestLedgerResponse, | ||
| ): Api.GetLatestLedgerResponse { | ||
| if (!raw.headerXdr || !raw.metadataXdr) { | ||
| throw new TypeError(`invalid response missing fields`); |
There was a problem hiding this comment.
This suggestion returns a better error for the user
| result: { | ||
| id: "hashed_id", | ||
| sequence: 123, | ||
| protocolVersion: 20, |
There was a problem hiding this comment.
We define protocolVersion as a string in GetLatestLedgerResponse. Is this using a different type?
What
rpc.Server.getLatestLedger()now includescloseTime,headerXdr, andmetadataXdrin the typed response, withheaderXdr/metadataXdrparsed into XDR objects instead of raw base64 strings.