Use historical hashing for ExecutionStateView
#4977
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
afck
approved these changes
Nov 17, 2025
afck
reviewed
Nov 17, 2025
MathieuDutSik
approved these changes
Nov 17, 2025
Contributor
MathieuDutSik
left a comment
MathieuDutSik
left a comment
There was a problem hiding this comment.
This is definitely better than having the hash set to zero.
That is why I approve it.
Otherwise, the problem of historical hash is that if we have
10000 batches being inserted, then we have to process every
one of them in order to get to a state that we are sure is the right
one by the batches. That is why it may be advantageous to have
a checkpointing where the block creator can choose at a given
step that the block being created is the classic hash of the state,
not of the batch. Maybe an issue can be created on this point.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 18, 2025
## Motivation The derived implementation of `View::context` currently returns just the context of the first sub-view of the view. This means that if a view A has a sub-view B, and view B has a sub-view C, when A calls `context()` on B and assumes that its own base key is eg. a byte shorter than what it gets as a result, it will in fact get a context with C's base key, which may lead to wrong results if B also appends some bytes to the base key before passing the context on to C (which happens in hashable wrappers, for example!). ## Proposal Make the implementation of `context()` in all views correctly trim the base key contained in the context before returning the value. This unfortunately introduces the necessity of cloning the context and returning it by value instead of by reference. An alternative (but somewhat more complicated) solution would be to require _all_ views to store their inner contexts, and make the `View` derive macro require a `context` field in all structs for which `View` is derived. ## Test Plan CI will catch regressions. This was also manually confirmed to fix a bug that surfaced in #4977. ## Release Plan - Nothing to do / These changes follow the usual release cycle. - This bug may be causing some issues on the testnet, but the fix cannot be safely backported without a complex migration. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
bart-linera
force-pushed
the
switch-to-historical-hash
branch
from
ed8d0c1 to
27b629d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Hashing of all the data in
ExecutionStateViews became a bottleneck in large chain states.Proposal
Replace the current hashing mechanism of
ExecutionStateViews with historical hashing.Test Plan
CI will catch regressions
Release Plan
Links