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

Automatically decode XDR for getLedgerEntries and friends #154

Merged
merged 14 commits into from
Oct 6, 2023

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Oct 3, 2023

What

We have the following breaking schema change in the top-level response:

-entries?: LedgerEntryResult[];
+entries: LedgerEntryResult[];

and in each entry:

-key: string;
-xdr: string;
+key: xdr.LedgerKey;
+val: xdr.LedgerEntryData;

More specificially,

  • entries is now guaranteed to exist, but it may be empty
  • entries[i].key is an instance of xdr.LedgerKey
  • the entries[i].xdr field is now val, instead
  • entries[i].val is an instance of xdr.LedgerEntryData

If users want to continue to use the raw RPC response, there is still a Server._getLedgerEntries method.

Some helpers from previous PRs have been moved around into a new src/parsers.ts, also.

Why

Better UX, see #128.

@Shaptic Shaptic added this to the Soroban Stable P20 Release milestone Oct 3, 2023
@Shaptic Shaptic requested a review from sreuland October 3, 2023 21:40
@Shaptic Shaptic removed this from the Soroban Stable P20 Release milestone Oct 3, 2023
src/parsers.ts Outdated Show resolved Hide resolved
src/parsers.ts Outdated Show resolved Hide resolved
@Shaptic Shaptic requested a review from sreuland October 6, 2023 19:06
src/index.ts Outdated
export { AxiosClient, version } from "./axios";
export * from "./transaction";

// only needed for testing
Copy link
Contributor

Choose a reason for hiding this comment

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

once it's available in the wild, nothing prevents clients from using in their production side code, should comment just be removed or not exported?

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 can't avoid exporting because it would mean it isn't available for browser testing, unfortunately :( you're right that nothing is stopping people from using it if they want. I added a comment to that effect in 47e445e.

tsconfig.json Show resolved Hide resolved
Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

nice work!

@Shaptic Shaptic merged commit 9335eaf into main Oct 6, 2023
4 checks passed
@Shaptic Shaptic deleted the decode-entries branch October 6, 2023 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants