unroll: drop zero-tree-depth gate that can block legitimate proofs (#372) - #455
Closed
ellemouton wants to merge 1 commit into
Closed
unroll: drop zero-tree-depth gate that can block legitimate proofs (#372)#455ellemouton wants to merge 1 commit into
ellemouton wants to merge 1 commit into
Conversation
The per-fragment validateProofDescriptorShape rejected any Ancestry fragment whose TreeDepth scalar was zero. TreeDepth is expiry-timing metadata (see vtxo.Descriptor.MaxTreeDepth and vtxo/expiry.go); the proof assembler walks TreePath.Root directly and never reads the scalar. Because incoming OOR ancestry is built from indexer RPC data by copying p.GetTreeDepth() verbatim, a malicious or version-skewed indexer that supplies a non-empty TreePath with TreeDepth omitted or forged to zero can persist a VTXO that accepts and validates fine on receive but is permanently rejected at unroll time. That turns an indexer-controlled scalar into a fund-stranding lever on the cooperative-operator-unavailable path, which is the exact threat model unilateral exit exists to defend against. Drop the gate so a zero TreeDepth no longer blocks proof assembly, and document the receive-side ingest boundary as the proper place to validate the scalar against TreePath.Depth() (issue #370). Closes #372.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the proof validation logic to stop requiring a non-zero TreeDepth in ancestry fragments. This change prevents a malicious or faulty indexer from blocking unilateral exits by providing a zeroed TreeDepth scalar, as the proof assembler relies on the TreePath instead. The PR includes updated documentation and a regression test to ensure zero tree depth is accepted. I have no feedback to provide.
ellemouton
marked this pull request as ready for review
May 15, 2026 12:31
Member
Author
|
Superseded by consolidated PR #459. Closing to reduce CI load. |
ellemouton
pushed a commit
that referenced
this pull request
May 22, 2026
client: update dependency for WalletDK
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.
Closes #372.
Summary
unroll/proof_assembler.go::validateProofDescriptorShaperejected anyAncestryfragment withTreeDepth == 0. ButTreeDepthis expiry-timing metadata — only consumed byvtxo.Descriptor.MaxTreeDepth()invtxo/expiry.go. The proof assembler walksTreePath.Rootdirectly viaaddTreePathNodesand never reads the scalar. With a malicious-indexer-poisonedTreeDepth = 0already persisted (or legacy data from before stricter ingest validation), the unroll path would refuse to assemble a perfectly recoverable proof — stranding the VTXO whenever the operator is unavailable.Fix
Removed the
TreeDepth == 0arm invalidateProofDescriptorShape. The unroll boundary stays liberal about the scalar because the proof material is theTreePath, not the depth claim. Updated explanatory comments inproof_assembler.goanddescriptor_resolver.goto document where the proper ingest-time check belongs (the sibling #370 fix atarkrpc.ValidateAncestryPathDepth).Test plan
TestValidateProofDescriptorAcceptsZeroTreeDepth— two-call form locks in "no sticky state" retry semanticsmake lint-native— 0 issuesgo test ./unroll/... ./oor/... ./vtxo/...— passRelationship to #370
Complementary, not conflicting. #370 (
indexer-supplied zero tree depth can strand OOR VTXOs) installs the ingest-time validator atarkrpc.ValidateAncestryPathDepthso zero/under-reported values never reach persistence. #372 (this PR) ensures the unroll path stays liberal so legacy/pre-fix data still recovers. The new comment cross-references the #370 ingest path so a future tuner doesn't re-add the gate.