-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(en): Support Merkle tree recovery with pruning enabled #3172
base: main
Are you sure you want to change the base?
Conversation
this is caused by snapshot storage logs getting pruned; this setup is currently not supported", | ||
snapshot.expected_root_hash | ||
); | ||
if let Some(expected_root_hash) = init_params.expected_root_hash { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be beneficial to record the state root hash of the last hard-pruned L1 batch somewhere (in pruning logs?) so that it can be checked here. If this sounds OK, I'd prefer to implement it in a follow-up PR.
@@ -340,6 +379,21 @@ impl AsyncTreeRecovery { | |||
Ok(output) | |||
} | |||
|
|||
async fn check_pruning_info( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I think this check has low probability of false negatives. Optimally though, we'd want to check the tree root hash so that we know for sure that we don't commit to a broken recovery; see another line comment.
- It would be nice to somehow prevent pruning until the recovery is complete, but I'm not sure how to do that. It could be possible to signal this via Postgres, but there could be issues if the tree recovery is aborted for whatever reason. Alternatively, it may be possible to check whether pruning is enabled at the start / continuation of tree recovery, and error if it is. This may lead to false positives if pruning is configured to never happen in practice, but may be good enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYM by " if pruning is configured to never happen in practice"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, pruning_data_retention_sec
is set to a very large value (like 10 years). Also, another potential issue is that if a node is run as distributed components (IIUC, we don't officially support this, but it's technically possible), then it's possible that a tree component performs this check on the start, and then the core is restarted with Postgres pruning enabled.
What ❔
Generalizes Merkle tree recovery so that it supports recovery after the node has pruned some of its data.
Why ❔
Improves node UX; allows to recover a tree of a full / non-archive node.
Checklist
zkstack dev fmt
andzkstack dev lint
.