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

fix(snapshot): set entry expiration info in set_ledger_info #1111

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

ElliotFriend
Copy link
Contributor

I've noticed some inconsistencies with snapshot creation and use where the various *_entry_expiration values are not set properly when create an Env from a snapshot that did have those values set.

This could possibly be related to #1101, but I'm not sure, since Leigh seemed more focused on prng.

Refs: #1101

What

This commit updates the ledger-snapshot crate to also set the various ledger entry expiration values, if they are set on the snapshot being set in the environment.

Why

When trying to load a ledger snapshot that I've created from an Env with valid entry expiration values, the new Env uses the defaults (which turn out to be 0) for those expiration values.

I noticed trouble trying to run env.register_contract() or env.register_stellar_asset_contract() in a test file after loading a previously valid snapshot, and receive the error: trying to bump past max expiration ledger. (That expiration ledger, being 0 ledgers from now.)

Known limitations

It's possible this is deliberate, and I'm misunderstanding how to use snapshots. If that's the case, we should probably bolster documentation around ledger snapshots to communicate that.

I've noticed some inconsistencies with snapshot creation and use
where the various `*_entry_expiration` values are not set properly
when create an `Env` from a snapshot that did have those values
set.

This could possibly be related to stellar#1101, but I'm not sure, since
Leigh seemed more focused on prng.

Refs: stellar#1101
@leighmcculloch leighmcculloch added this pull request to the merge queue Oct 9, 2023
@leighmcculloch
Copy link
Member

@ElliotFriend heads up there's another related issue to restoring from snapshots that I'm fixing:

Merged via the queue into stellar:main with commit 086a44c Oct 9, 2023
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.

2 participants