Skip to content

oor: reject incomplete ancestry index on incoming OOR receive (#374) - #442

Closed
ellemouton wants to merge 1 commit into
mainfrom
fix/374-ancestry-index-validation
Closed

oor: reject incomplete ancestry index on incoming OOR receive (#374)#442
ellemouton wants to merge 1 commit into
mainfrom
fix/374-ancestry-index-validation

Conversation

@ellemouton

Copy link
Copy Markdown
Member

Closes #374.

Summary

oor.validateIncomingAncestry checked that each fragment's InputIndices was non-empty and within range, but it did not:

  • reject duplicate indices, and
  • require the union of all fragments' InputIndices to cover every Ark tx input.

A malicious indexer could therefore return ancestry for only a subset of a multi-input OOR Ark tx; the descriptor would be accepted and persisted with incomplete recovery lineage, leaving the VTXO stranded if the operator later refuses cooperation (no rooted-path material for fraud.BuildWatchPlan or unilateral-exit assembly on the uncovered inputs).

This is on the same IncomingVTXOMetadata.Ancestry artifact, distinct from #366 (PackageArtifact PSBT graph) and from #371 (resolver read path).

Fix

oor/incoming_vtxo.go::validateIncomingAncestry:

  • tracks covered []bool sized to the Ark tx input count
  • rejects duplicate indices at the offending fragment
  • after the per-fragment loop, asserts every input is covered

Failures continue to surface as *ErrInvalidAncestry so the receive FSM routes to session-failure ack.

Test plan

  • new TestValidateIncomingAncestryInputCoverage — partitions: single/two-fragment partition success, missing-coverage truncation, mid-range gap, intra/cross-fragment duplicates
  • TestBuildIncomingVTXODescriptorNormalizesPrimaryAncestry updated to use a new two-input materialization helper (the old setup was a single-input PSBT with two fake fragments — itself a coverage violation under the corrected rule)
  • make lint-native — 0 issues
  • go test ./oor ./darepod ./fraud ./vtxo ./db -count=1 — pass

Related: distinct from #366 (write path of persisted package graph) and from #371 (resolver / read path). No bundle.

validateIncomingAncestry checked that each fragment's InputIndices was
non-empty and within range, but did not reject duplicate indices or
require the union of all fragments to cover every Ark tx input. An
indexer/operator could return ancestry for only a subset of a
multi-input OOR Ark tx — for example a two-input Ark tx with a single
fragment naming only input 0 — and the descriptor would be accepted
and persisted with incomplete recovery lineage. If the operator later
disappears, the uncovered input has no rooted-path material for
fraud-watch or unilateral-exit assembly, stranding the received VTXO.

Track which input indices each fragment claims, reject duplicates at
the malformed fragment, and require coverage of every Ark tx input
before returning. Add direct table-driven coverage for the partition
checks and a two-input materialization helper so the existing
primary-ancestry normalization test exercises a genuine cross-round
multi-input shape rather than a single-input PSBT with two fake
fragments.

Closes #374.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the validation of incoming OOR VTXOs by ensuring that all Ark transaction inputs are uniquely covered by ancestry fragments. It introduces checks for duplicate input indices and missing coverage within the validateIncomingAncestry function to prevent potential fund loss. Corresponding tests and test helpers were added to verify these partition checks for multi-input transactions. Feedback suggests a potential optimization for the coverage check using a counter, though the current loop-based approach allows for more specific error messages regarding missing indices.

Comment thread oor/incoming_vtxo.go
Comment on lines +383 to +394
for idx, ok := range covered {
if !ok {
return &ErrInvalidAncestry{
Reason: fmt.Sprintf(
"ark tx input %d is not covered by "+
"any ancestry fragment "+
"(incoming ancestry must "+
"cover every input)", idx,
),
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The coverage check iterates through the entire covered slice to find missing indices. While correct, this could be optimized by maintaining a counter of unique indices seen during the fragment loop. If the counter equals arkTxInputCount at the end, full coverage is guaranteed (since duplicates are already rejected). The current loop is only necessary if you want to identify the specific missing index for the error message.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the explicit loop intentionally — the second pass lets the error message identify the specific missing input index, which is useful for debugging malformed indexer responses. Happy to revisit if the cost shows up in profiling.

@ellemouton
ellemouton marked this pull request as ready for review May 15, 2026 03:13
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@ellemouton

Copy link
Copy Markdown
Member Author

Superseded by consolidated PR #459. Closing to reduce CI load.

@ellemouton ellemouton closed this May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security][high] Incomplete ancestry index validation can strand OOR funds

1 participant