feat: persistent-prover support — proof cycle, rewind path, storage flush - #27
Open
mwaddip wants to merge 4 commits into
Open
feat: persistent-prover support — proof cycle, rewind path, storage flush#27mwaddip wants to merge 4 commits into
mwaddip wants to merge 4 commits into
Conversation
Adds a default-no-op method on the storage trait so callers can force a durable commit (fsync) of outstanding writes. Implementations using deferred writes (e.g. redb Durability::None) should override this to fsync the backing store. In-memory or always-durable storages can keep the default no-op. Motivation: PersistentBatchAVLProver::generate_proof_and_update_storage uses non-durable writes for throughput. Without a caller-triggered flush, graceful SIGTERM leaves uncommitted state pending; on reopen, redb sees no valid commit and treats the storage as empty. Periodic flush calls bound crash data loss. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (cherry picked from commit 9a67695)
Proof generation (pack_tree) and persistence (collect_changed_nodes) both relied on the tree's visited/is_new flags, creating a conflict: generate_proof() called tree.reset(), clearing flags that a subsequent storage.update() needed to find changed nodes. This caused internal nodes created during AVL rebalancing to never be persisted. Decouple them: 1. Add modified_nodes tracking (AuthenticatedTreeOpsBase) — populated unconditionally in on_node_visit, cleared by generate_proof(). was_modified() keys on Rc::as_ptr, independent of tree flags. 2. Switch pack_tree from tree.visited() to was_modified() — proof generation no longer touches visited/is_new. 3. Replace tree.reset() in generate_proof() with modified_nodes.clear() + a needs_cycle_reset flag. The next perform_one_operation() resets visited/is_new (persistence flags) at the cycle boundary, after update() had a chance to collect them. 4. Relax check_tree_helper's post_proof assertion — flags are now cleared by update() or perform_one_operation(), not generate_proof(). 5. Reset the cloned tree in generate_proof_for_operations() so the clone's on_node_visit starts with fresh flags. modified_nodes is a BTreeMap keyed on the node's address rather than a list, because pack_tree asks was_modified() once per node it walks: a linear scan there makes proof generation quadratic in the number of visits, which on a remove-heavy benchmark costs minutes rather than seconds. The map's NodeId values keep each node alive, so an address cannot be recycled while it is still a key, and the set stays per-prover — a tree clone shares its nodes, so per-node state would leak proof bookkeeping between the original and the clone. (cherry picked from commit a42be22)
Add a public method that installs a persisted root and rebases the proof cycle atomically. Replaces the ad-hoc field assignments currently spread across four restore sites in the node. After restoring a storage-loaded root, callers must: - Clear is_new/visited flags on the fresh tree - Drop stale changed-node buffers from the previous cycle - Rebase old_top_node to the new root - Clear accumulated directions from any prior (failed) cycle restore_root() does all of this in one place, owning the proof-cycle invariant. (cherry picked from commit 87da2da)
restore_root dropped the changed-node buffers but not modified_nodes, the map pack_tree gates on. Only generate_proof() clears that map, and a cycle that is rewound never reaches it -- so a block that is applied and then rejected leaves its entire visited set marked. The next pack_tree() then expands nodes it should have labelled. Where the rewind reinstalls a root that is still address-live -- which is what a storage layer holding node handles does -- that is a different proof for identical tree state, not merely retained memory: 740 vs 735 bytes in the round-trip test below. PersistentBatchAVLProver::rollback had drifted the same way, by re-implementing the rewind by hand instead of calling it; it had already missed the old_top_node rebase once for the same reason. It now delegates to restore_root, so there is a single rewind implementation that cannot drift from itself. Each clause is pinned: with only the modified_nodes clear, the two rollback tests still fail.
This was referenced Aug 12, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Supersedes #11, #18, #19, #22 — the same change split four ways — rebased onto current
main, plus a defect that appears only once they are combined.VersionedAVLStorage::flush(): default no-op hook, non-breaking.pack_treegates onmodified_nodesrather than the persistence flags, sotree.reset()no longer perturbs a proof cycle.restore_rootinstalls a storage-loaded root and rebases the proof baseline atomically.modified_nodes, which onlygenerate_proof()clears — and an abandoned cycle never reaches it. If the restored root is still address-live, the nextpack_treeexpands nodes it should have labelled: a different proof for identical state (740 vs 735 bytes).rollbacknow delegates torestore_root, so there is one rewind path.Each commit is individually green; the three new tests fail on the parent commit.