feat: Caches appearance refactoring #326
Merged
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
0xVolosnikov
force-pushed
the
vv-caches-refactoring-from-v2
branch
3 times, most recently
from
087d7c4 to
bc7e45d
Compare
0xVolosnikov
force-pushed
the
vv-caches-refactoring-from-v2
branch
from
6b2b564 to
62598e5
Compare
0xVolosnikov
changed the title
Contributor
Author
|
It would be nice to keep history_map simple (huh) and avoid adding any new functionality to it. However, in this case, this seems like the simplest and most effective solution, so we can live with itfor now (until we decide to refactor history_map and the caches in general again). |
Contributor
Benchmark report
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
What ❔
Simplified and refined change of caches appearance mechanics from "V2".
From what I see in the rr/ethproofs branch, we do not use the "updated" or "deleted" options in any meaningful way. Since these values can be encapsulated, I decided to remove them. Additionally, the global "updated" / "deleted" options may create sneaky issues related to rollbacks. For example, if an element was marked as "updated" and the actual change is later rolled back, the label becomes misleading. This means we would always have to keep these nuances in mind. In practice, we already need to check diffs of storage slots or accounts to determine whether any changes occurred, so these labels do not add much useful information anyway.
The "deleted" option may become useful in the future if we decide to implement some optimized storage model, but its use would again require careful handling.
I use enums instead of bool values to keep it extendable if we decide to add new options (like "deleted"). Also it's somewhat more explicit.
Is this a breaking change?
Checklist