fix: decouple proof generation from persistence flags - #18
Conversation
6e1a617 to
a96bba5
Compare
|
Force-pushed: rustfmt applied, folded into the existing commit so this stays a single commit.
Unrelated: |
|
Note for whenever this lands — no action needed here. #22 ( Flagging it here because that's where the trigger is — #22's description already records the dependency from its own side. Tracking it; nothing required of reviewers. |
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.
a96bba5 to
c007a4b
Compare
1 participant
generate_proof()calledtree.reset(), clearing flags that a downstreamstorage.update()needs forcollect_changed_nodes. A proof-first caller(required for correct Lookup proofs) found only the root — internal nodes from
rebalancing were never persisted.
Decouple proof generation from persistence:
modified_nodesvec, populated byon_node_visit, queried bypack_treevia pointer-identity lookup.
pack_treeno longer touchesvisited.perform_one_operation()at the cycleboundary, not by
generate_proof().storage.update()keeps its reset.check_tree_helper's post-proof assertion.All 22 tests pass.
test_successful_modifications20/20 consecutive.Found via the ergo Rust full-node persistence backend.