Skip to content

Remove support for storing ledger entries in local db #417

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

Draft
wants to merge 5 commits into
base: protocol-23
Choose a base branch
from

Conversation

urvisavla
Copy link
Contributor

What

[TODO: Short statement about what is changing.]

Why

[TODO: Why this change is being made. Include any context required to understand the why.]

Known limitations

[TODO or N/A]

@urvisavla urvisavla force-pushed the drop-ledgerentries branch from a03a40e to 3fdd4d8 Compare April 23, 2025 22:59
@urvisavla urvisavla force-pushed the drop-ledgerentries branch from 3fdd4d8 to 39ef7c4 Compare April 23, 2025 23:08
Comment on lines 7 to 11
type LedgerKeyAndEntry struct {
Key xdr.LedgerKey
Entry xdr.LedgerEntry
LiveUntilLedgerSeq *uint32 // optional live-until ledger seq, when applicable.
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be more appropriate to have this in the ledgerentries package

@@ -211,11 +209,6 @@ func (s *Service) fillEntriesFromCheckpoint(ctx context.Context, archive history
ctx, cancel = context.WithTimeout(ctx, s.timeout)
defer cancel()

reader, err := ingest.NewCheckpointChangeReader(ctx, archive, checkpointLedger)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're no longer reading changes, can we remove fillEntriesFromCheckpoint and simplify maybeFillEntriesFromCheckpoint?

@Shaptic
Copy link
Contributor

Shaptic commented Apr 25, 2025

I was discussing stellar/js-stellar-sdk#1165 recently, and this feature would require processing ledger entries 🙈 I don't think we should keep it, but I just wanted to note it for visibility - we can always revert this change in a simpler form down the line if this feature request comes to fruition.

@urvisavla urvisavla force-pushed the drop-ledgerentries branch from eedf4ee to c05b84d Compare April 25, 2025 18:16
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.

3 participants