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

First-pass ledger hashes in replayer checkpoint files #14390

Merged
merged 7 commits into from
Nov 1, 2023

Conversation

psteckler
Copy link
Member

@psteckler psteckler commented Oct 18, 2023

The replayer checks that every new snarked ledger hash it sees is some first-pass ledger hash. If we run the replayer from a checkpoint file, a new snarked ledger hash might be equal to a first-pass ledger hash that was produced before the checkpoint.

Solution: save the pending first-pass ledger hashes to the checkpoint file, and populate the set of hashes from the saved hashes when the replayer starts. Also save the last snarked ledger hash, which is used to know whether to search the first-pass hashes.

The set of ledger hashes are indexed by order of occurrence. That indexing allows flushing older hashes when we find the first-pass hash that matches a snarked ledger hash. The ledger hashes are saved as a list whose order is given by the indexes. When starting the replayer, the set of hashes uses new indexes created when iterating over that list.

The first_pass_ledger_hashes and last_snarked_ledger_hash fields are optional in the input file, so compatibility with old checkpoint files is maintained. Confirmed that fact by running the replayer on an old checkpoint file.

Closes #13732.

@psteckler
Copy link
Member Author

!ci-build-me

@psteckler psteckler marked this pull request as draft October 19, 2023 05:07
@psteckler psteckler marked this pull request as ready for review October 19, 2023 17:04
@psteckler
Copy link
Member Author

!ci-build-me

psteckler pushed a commit that referenced this pull request Oct 19, 2023
@psteckler
Copy link
Member Author

!ci-build-me

@dkijania dkijania self-requested a review October 30, 2023 21:24
incr count

let find ledger_hash =
Base.Hash_set.find hash_set ~f:(fun (hash, _n) ->
Copy link
Member

Choose a reason for hiding this comment

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

This is an O(n) operation, right?

If yes, I wonder why the hash set is used instead of simple list?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Probably when I started on it, I thought we could do lookups on the hash alone.

The hash set is a mutable data structure, with the list we'd need to use a mutable reference. Not sure which is better.

@deepthiskumar
Copy link
Member

!ci-build-me

@deepthiskumar
Copy link
Member

!approved-for-mainnet

@psteckler psteckler merged commit 3ab1740 into berkeley Nov 1, 2023
1 check passed
@psteckler psteckler deleted the feature/ledger-hashes-in-checkpoint branch November 1, 2023 22:17
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.

4 participants