perf: hold a grant to one parent resolve and one read per moved node - #1845
Conversation
…ed node The convergence pass resolved the parent scope root a second time, after converge_grant_subtree had already resolved it for the resume probe. The pass now takes the proved pair from its caller, so sweep_pass keeps the resolve and converge_subtree does not repeat it. The resumed interior walk read every already-moved node twice: resolve_child paid a fanout resolve, a head fetch and a decode, then dropped them and moved_interior_node repeated all three. One seam, resolve_moving_child, now reads the record once and classifies it against both scope ids, each arm opening under the derivation of the scope the record claims. The promoted root's override seed is recovered once for the whole move instead of once per node. MovedScopeSeed holds it behind an Rc keyed on the root's own identity, so the memo is the terminal owner and the bytes zeroize when the slot is replaced.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughChangesMoving scope convergence
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant sweep_pass
participant converge_subtree
participant GrantResumeResolver
participant OwnerRotationNet
sweep_pass->>converge_subtree: pass resolved scope_ref and SweptScope
converge_subtree->>GrantResumeResolver: resolve_moving_child(source, root, node)
GrantResumeResolver->>OwnerRotationNet: read and classify child record
OwnerRotationNet-->>GrantResumeResolver: MovingChild
GrantResumeResolver-->>converge_subtree: continue, reseal, or stop traversal
Merge Risk: ⚪ Minimal · up to The change has no identified merge-blocking risk in the reviewed scope. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation Issue Full details: Docstring CoverageExplanation Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 6 files. (2 skipped: 2 too large.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
What this changes
Two repeated reads on the grant-creation path, both found while reviewing #1624 and merged into one issue.
One resolve of the parent scope root per grant.
converge_grant_subtreeresolves the parent so the resume probe can read the scope source that resolve parks. It then calledconverge_subtree, which resolved the same name again insidewalk_and_converge. The pass now takes the proved(ChildScopeRef, SweptScope)pair from its caller;sweep_passkeeps its own resolve, so nothing else changes.One read of every already-moved node. A resumed interior walk met each node the first attempt had moved with
resolve_child, which paid a fanout resolve, a head-block fetch and an envelope decode, refused the record and dropped the decoded bytes. The fall-through then ranmoved_interior_node, which repeated all three on the same name. One seam replaces the pair:resolve_moving_childreads the record once and classifies it against both scope ids, and each arm opens it under the derivation of the scope the record claims.One override-seed recovery per move.
moved_interior_nodeandreseal_interior_nodeeach recovered the promoted root's override seed from its owner blob on every node, which is an X25519 operation plus an HPKE open per node.MovedScopeSeedholds the recovered seed behind anRc, keyed on the root's own scope id, name and read epoch. A record naming any other root recovers its own seed rather than aliasing onto the held one.Behaviour
The verdicts do not move. A grant-section record still runs the descendant gate, a record claiming neither scope still reports
InteriorResolvewithRejected, and a version skew still reports as skew. The new seam additionally demands the parked source scope, which the publish arm already demanded.Tests
a_grant_resolves_the_parent_scope_root_once— the whole command costs one scope-root resolve.a_resumed_walk_reads_each_already_moved_node_once— a re-drive reads each moved node once and resolves the parent name once.one_override_seed_recovery_serves_a_whole_interior_move— the memo answers the same root from one recovery and re-recovers for another root.All three run in the Test gate.
Body checks / follow-ups filed
Closes #1714.
Summary by CodeRabbit
Bug Fixes
Tests
Note
Reduce grant moves to one parent resolve and one read per moved node
GrantResumeResolver::resolve_moving_child, which reads each node once against the source and destination scopes and returns aMovingChild(Pending,Moved, orScopeRoot). Moved nodes are traversed without re-sealing; pending source nodes are re-sealed; scope roots are rejected.MovedScopeSeedtoOwnerRotationNet— a one-slot cache keyed by destination scope id, IPNS name, and read epoch — so the destination override seed is recovered once per root and reused across interior reseals and promotion.walk_and_convergeandconverge_subtreeto accept a caller-suppliedSweptScope, lettingconverge_grant_subtreereuse the resume probe's parent-scope proof instead of resolving the parent name a second time.GrantResumeResolverrenames its moved-interior callback toresolve_moving_childwith a newMovingChildreturn type; all in-tree implementors (OwnerRotationNet,FakeNetincreate.rs,invite_mint.rs,contract.rs) are updated, but out-of-tree implementations of the oldmoved_interior_nodetrait method will break.Macroscope summarized 7f4f13a.