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

clients/stellarcore: Add support for Core's new /getledgerentry HTTP endpoint #5542

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

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Dec 3, 2024

What

Add support for Core's /getledgerentry as implemented in CAP-66.

Why

See #5426

@Shaptic Shaptic changed the title [Draft] clients/stellarcore: Add support for new Protocol 23 HTTP endpoints clients/stellarcore: Add support for Core's new /getledgerentry HTTP endpoint Jan 22, 2025
@Shaptic Shaptic requested review from 2opremio, sreuland, a team and tamirms January 22, 2025 18:27
@Shaptic Shaptic marked this pull request as ready for review January 22, 2025 18:28
@Shaptic Shaptic requested a review from SirTyson January 22, 2025 18:29
@Shaptic Shaptic force-pushed the p23-core-endpoints branch from a126a16 to 4db05df Compare January 22, 2025 18:37
@2opremio
Copy link
Contributor

Looks good!

@@ -250,6 +250,11 @@ func (c *Client) GetLedgerEntryRaw(ctx context.Context, ledgerSeq uint32, keys .
return resp, c.makeLedgerKeyRequest(ctx, &resp, "getledgerentryraw", ledgerSeq, keys...)
}

func (c *Client) GetLedgerEntries(ctx context.Context, ledgerSeq uint32, keys ...xdr.LedgerKey) (*proto.GetLedgerEntryResponse, error) {
var resp *proto.GetLedgerEntryResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var resp *proto.GetLedgerEntryResponse
var resp proto.GetLedgerEntryResponse

I think this should not be a pointer

require.EqualValues(t, "live", resp.Entries[0].State)
require.EqualValues(t, "archived", resp.Entries[1].State)

key.Type = xdr.LedgerEntryTypeTtl
Copy link
Contributor

Choose a reason for hiding this comment

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

nice coverage on the not so happy path!

Copy link
Contributor

@SirTyson SirTyson left a comment

Choose a reason for hiding this comment

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

LGTM! Just curious, for the new test case did you upgrade captice-core git hash in CI?

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

Successfully merging this pull request may close these issues.

5 participants