From 622e230b7cf87d43ca9219d1f3fce9f628a1d8a0 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 14 May 2026 19:26:19 -0700 Subject: [PATCH] oor: Reject incomplete ancestry index on incoming OOR receive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- oor/incoming_vtxo.go | 61 ++++++++ oor/incoming_vtxo_test.go | 200 ++++++++++++++++++++++++-- oor/local_persistence_handler_test.go | 114 +++++++++++++++ 3 files changed, 365 insertions(+), 10 deletions(-) diff --git a/oor/incoming_vtxo.go b/oor/incoming_vtxo.go index 38b069ed4..67369417c 100644 --- a/oor/incoming_vtxo.go +++ b/oor/incoming_vtxo.go @@ -242,6 +242,19 @@ func BuildIncomingVTXODescriptor(ark *psbt.Packet, // Ancestry contract for incoming OOR VTXOs (those are always // produced by an OOR Ark tx); an out-of-range index points at a // non-existent input so the unroll proof would never resolve. +// +// - No input index repeats within a fragment or across fragments, +// and the union of all fragments' InputIndices covers every Ark +// tx input exactly once. A duplicate or missing index means at +// least one Ark tx input has no rooted-path material attached: +// fraud-watch plans (BuildWatchPlan) would lack a watch for the +// uncovered input's lineage and unilateral-exit proof assembly +// would have no fragment to broadcast for that input. The +// descriptor would persist cleanly and the gap would only surface +// if the operator later refuses cooperation, at which point the +// user is racing a CSV with no way to recover. We reject at the +// receive boundary so the bad indexer response fails before any +// funds are credited. func validateIncomingAncestry(meta IncomingVTXOMetadata, arkTxInputCount uint32) error { @@ -251,6 +264,11 @@ func validateIncomingAncestry(meta IncomingVTXOMetadata, } } + // Track which Ark tx input indices each fragment has claimed so we + // can verify partition coverage at the end. Sized to the Ark tx's + // declared input count; we only set entries after the per-fragment + // range check has passed, so out-of-range writes are impossible. + covered := make([]bool, arkTxInputCount) seen := make(map[chainhash.Hash]struct{}, len(meta.Ancestry)) hasPrimary := false for i, frag := range meta.Ancestry { @@ -319,6 +337,30 @@ func validateIncomingAncestry(meta IncomingVTXOMetadata, ), } } + + // Reject duplicate input indices (either repeated + // within this fragment or already claimed by an + // earlier fragment). A duplicate means another Ark + // tx input is silently uncovered, which the + // post-loop coverage check below would also detect + // — we flag the duplicate here so the failure + // reason points at the malformed fragment rather + // than at a missing input that looks like a + // truncation. + if covered[idx] { + return &ErrInvalidAncestry{ + Reason: fmt.Sprintf( + "fragment %d input index "+ + "[%d]=%d duplicates "+ + "an index already "+ + "claimed by another "+ + "fragment (or earlier "+ + "in this fragment)", + i, j, idx, + ), + } + } + covered[idx] = true } } @@ -332,6 +374,25 @@ func validateIncomingAncestry(meta IncomingVTXOMetadata, } } + // Every Ark tx input must be covered by exactly one fragment. + // Duplicates were rejected above, so a missing-coverage failure + // here means the indexer truncated the ancestry. Accepting it + // would leave the uncovered input with no rooted-path material + // for unilateral exit, stranding the received VTXO if the + // operator later refuses cooperation. + 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, + ), + } + } + } + return nil } diff --git a/oor/incoming_vtxo_test.go b/oor/incoming_vtxo_test.go index 62b914dd5..a584481c2 100644 --- a/oor/incoming_vtxo_test.go +++ b/oor/incoming_vtxo_test.go @@ -4,6 +4,9 @@ import ( "testing" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + lib_tree "github.com/lightninglabs/darepo-client/lib/tree" + "github.com/lightninglabs/darepo-client/vtxo" "github.com/lightningnetwork/lnd/keychain" "github.com/stretchr/testify/require" ) @@ -81,17 +84,64 @@ func TestBuildIncomingVTXODescriptorZeroChainDepth(t *testing.T) { // cross-round multi-input metadata may carry the descriptor's commitment // fragment after another valid fragment, and descriptor construction still // preserves legacy Ancestry[0] primary semantics. +// +// The test exercises the genuine cross-round multi-input shape: a +// two-input Ark tx with one fragment per input. The secondary fragment +// is supplied first in the metadata so descriptor construction must +// reorder it before persistence. func TestBuildIncomingVTXODescriptorNormalizesPrimaryAncestry(t *testing.T) { t.Parallel() - arkPSBT, _, recipients, commitHash, recipientKey, - operatorKey := buildTestIncomingMaterialization(t) + arkPSBT, _, recipients, commits, recipientKey, + operatorKey := buildTestIncomingMaterializationMultiInput(t) - otherHash := chainhash.Hash{0xee} - ancestry := validTestIncomingAncestry(otherHash) - ancestry = append( - ancestry, validTestIncomingAncestry(commitHash)[0], - ) + // BuildArkPSBT applies BIP69 input ordering, so locate which + // of the two commitment hashes ends up at Ark input index 0 + // vs 1 in the canonical PSBT. Each fragment must name the + // input it actually serves. + indexOf := func(h chainhash.Hash) uint32 { + for i, in := range arkPSBT.UnsignedTx.TxIn { + if in.PreviousOutPoint.Hash == h { + return uint32(i) + } + } + t.Fatalf("commit %s not found in ark inputs", h) + + return 0 + } + primaryCommit := commits[0] + secondaryCommit := commits[1] + + ancestry := []vtxo.Ancestry{ + // Secondary fragment first — descriptor construction must + // re-order so Ancestry[0] is the primary commitment. + { + TreePath: &lib_tree.Tree{ + Root: &lib_tree.Node{}, + BatchOutpoint: wire.OutPoint{ + Hash: secondaryCommit, + }, + }, + CommitmentTxID: secondaryCommit, + InputIndices: []uint32{ + indexOf(secondaryCommit), + }, + TreeDepth: 1, + }, + { + TreePath: &lib_tree.Tree{ + Root: &lib_tree.Node{}, + BatchOutpoint: wire.OutPoint{ + Hash: primaryCommit, + }, + }, + CommitmentTxID: primaryCommit, + InputIndices: []uint32{ + indexOf(primaryCommit), + }, + TreeDepth: 1, + }, + } desc, err := BuildIncomingVTXODescriptor(arkPSBT, IncomingVTXOConfig{ @@ -103,7 +153,7 @@ func TestBuildIncomingVTXODescriptorNormalizesPrimaryAncestry(t *testing.T) { ExitDelay: 10, Metadata: IncomingVTXOMetadata{ RoundID: "test-round", - CommitmentTxID: commitHash, + CommitmentTxID: primaryCommit, BatchExpiry: 1000, ChainDepth: 1, CreatedHeight: 500, @@ -113,8 +163,8 @@ func TestBuildIncomingVTXODescriptorNormalizesPrimaryAncestry(t *testing.T) { ) require.NoError(t, err) require.Len(t, desc.Ancestry, 2) - require.Equal(t, commitHash, desc.Ancestry[0].CommitmentTxID) - require.Equal(t, otherHash, desc.Ancestry[1].CommitmentTxID) + require.Equal(t, primaryCommit, desc.Ancestry[0].CommitmentTxID) + require.Equal(t, secondaryCommit, desc.Ancestry[1].CommitmentTxID) } // TestBuildIncomingVTXODescriptorRejectsNilArk verifies that a nil Ark @@ -248,3 +298,133 @@ func TestBuildIncomingVTXODescriptorRejectsInvalidAncestry(t *testing.T) { }) } } + +// TestValidateIncomingAncestryInputCoverage exercises the InputIndices +// partition checks for multi-input Ark transactions. The other rejection +// branches are covered via BuildIncomingVTXODescriptor in +// TestBuildIncomingVTXODescriptorRejectsInvalidAncestry; here we drive +// validateIncomingAncestry directly so we can vary arkTxInputCount +// without rebuilding a real PSBT. +// +// The scenarios assert two properties that the receive boundary must +// enforce so that a malicious or truncated indexer response cannot +// strand received OOR funds: +// +// - The union of all fragments' InputIndices covers every Ark tx +// input (0..arkTxInputCount-1). +// - No input index appears in more than one fragment (or twice +// within a single fragment), since a duplicate hides a missing +// fragment behind apparently-full coverage. +func TestValidateIncomingAncestryInputCoverage(t *testing.T) { + t.Parallel() + + primary := chainhash.Hash{0x01} + secondary := chainhash.Hash{0x02} + + fragment := func(commit chainhash.Hash, + indices ...uint32) vtxo.Ancestry { + + return vtxo.Ancestry{ + TreePath: &lib_tree.Tree{ + Root: &lib_tree.Node{}, + BatchOutpoint: wire.OutPoint{ + Hash: commit, + }, + }, + CommitmentTxID: commit, + InputIndices: append( + []uint32(nil), indices..., + ), + TreeDepth: 1, + } + } + + cases := []struct { + name string + arkTxInputCount uint32 + ancestry []vtxo.Ancestry + wantReason string + }{ + { + name: "single fragment covers single input", + arkTxInputCount: 1, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0), + }, + }, + { + name: "two fragments partition two inputs", + arkTxInputCount: 2, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0), + fragment(secondary, 1), + }, + }, + { + name: "single fragment covers both inputs", + arkTxInputCount: 2, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0, 1), + }, + }, + { + name: "missing coverage truncated fragment", + arkTxInputCount: 2, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0), + }, + wantReason: "ark tx input 1 is not covered", + }, + { + name: "missing coverage gap mid range", + arkTxInputCount: 3, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0, 2), + }, + wantReason: "ark tx input 1 is not covered", + }, + { + name: "duplicate within fragment", + arkTxInputCount: 2, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0, 0), + }, + wantReason: "duplicates an index already claimed", + }, + { + name: "duplicate across fragments", + arkTxInputCount: 2, + ancestry: []vtxo.Ancestry{ + fragment(primary, 0), + fragment(secondary, 0), + }, + wantReason: "duplicates an index already claimed", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + meta := IncomingVTXOMetadata{ + RoundID: "test-round", + CommitmentTxID: primary, + BatchExpiry: 1000, + ChainDepth: 1, + CreatedHeight: 500, + Ancestry: tc.ancestry, + } + + err := validateIncomingAncestry( + meta, tc.arkTxInputCount, + ) + + if tc.wantReason == "" { + require.NoError(t, err) + + return + } + require.Error(t, err) + require.ErrorIs(t, err, &ErrInvalidAncestry{}) + require.Contains(t, err.Error(), tc.wantReason) + }) + } +} diff --git a/oor/local_persistence_handler_test.go b/oor/local_persistence_handler_test.go index 67f9c0e89..099731902 100644 --- a/oor/local_persistence_handler_test.go +++ b/oor/local_persistence_handler_test.go @@ -1105,6 +1105,120 @@ func buildTestIncomingMaterialization(t *testing.T) (*psbt.Packet, operatorKey.PubKey() } +// buildTestIncomingMaterializationMultiInput is the two-checkpoint +// variant of buildTestIncomingMaterialization. It returns an Ark PSBT +// spending two distinct checkpoint inputs (so len(arkPSBT.UnsignedTx.TxIn) +// == 2). Cross-round multi-input OOR receive coverage exercises +// validateIncomingAncestry's partition checks, which require the union +// of all fragments' InputIndices to cover every Ark input — a property +// that cannot be exercised against the single-input helper. +// +// The two commitment txids returned correspond to inputs[0] and +// inputs[1] respectively; callers stitch them into two-fragment +// IncomingVTXOMetadata.Ancestry slices. +func buildTestIncomingMaterializationMultiInput(t *testing.T) (*psbt.Packet, + []*psbt.Packet, []ArkRecipientOutput, [2]chainhash.Hash, + *btcec.PrivateKey, *btcec.PublicKey) { + + t.Helper() + + operatorKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + policy := arkscript.CheckpointPolicy{ + OperatorKey: operatorKey.PubKey(), + CSVDelay: 10, + } + + recipientKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + // Two independent checkpoint inputs anchored to distinct + // upstream Ark txids so the produced Ark tx has two inputs, + // each contributable by a different ancestry fragment. + inputAmt := btcutil.Amount(5_000) + makeInput := func(seed byte) oortx.CheckpointInput { + return oortx.CheckpointInput{ + SpentVTXO: oortx.SpentVTXORef{ + Outpoint: wire.OutPoint{ + Hash: [32]byte{ + seed, + }, + Index: 0, + }, + Output: &wire.TxOut{ + Value: int64(inputAmt), + PkScript: newTestTaprootPkScript( + t, operatorKey.PubKey(), + ), + }, + }, + OwnerLeafScript: []byte{ + 0x51, + }, + } + } + inputs := []oortx.CheckpointInput{ + makeInput(0x11), makeInput(0x22), + } + + cp0, err := oortx.BuildCheckpointPSBT(policy, inputs[0]) + require.NoError(t, err) + + cp1, err := oortx.BuildCheckpointPSBT(policy, inputs[1]) + require.NoError(t, err) + + vtxoTapKey, err := arkscript.VTXOTapKey( + recipientKey.PubKey(), policy.OperatorKey, 10, + ) + require.NoError(t, err) + + recipientPkScript, err := txscript.PayToTaprootScript(vtxoTapKey) + require.NoError(t, err) + + outputs := []oortx.RecipientOutput{ + { + PkScript: recipientPkScript, + Value: inputAmt * 2, + }, + } + + arkPSBT, err := oortx.BuildArkPSBT( + []oortx.CheckpointOutput{ + { + Txid: cp0.PSBT.UnsignedTx.TxHash(), + Output: cp0.PSBT.UnsignedTx.TxOut[0], + TapTreeEncoded: cp0.TapTreeEncoded, + }, + { + Txid: cp1.PSBT.UnsignedTx.TxHash(), + Output: cp1.PSBT.UnsignedTx.TxOut[0], + TapTreeEncoded: cp1.TapTreeEncoded, + }, + }, + outputs, + ) + require.NoError(t, err) + + recipients, err := ExtractArkRecipients(arkPSBT) + require.NoError(t, err) + + // Use the checkpoint tx ids as the per-fragment "commitment" + // txids so that callers can name a real Ark-tx input prevout + // for each fragment. (Ark inputs reference checkpoint tx ids, + // not the upstream SpentVTXO outpoint hashes.) The validator + // only requires that BatchOutpoint.Hash matches CommitmentTxID + // across the per-fragment cross-check; it does not interpret + // the commitment txid itself. + commits := [2]chainhash.Hash{ + cp0.PSBT.UnsignedTx.TxHash(), + cp1.PSBT.UnsignedTx.TxHash(), + } + + return arkPSBT, []*psbt.Packet{cp0.PSBT, cp1.PSBT}, recipients, + commits, recipientKey, operatorKey.PubKey() +} + // validTestIncomingAncestry returns a minimal Ancestry slice that passes // BuildIncomingVTXODescriptor's structural cross-check, anchored at the // supplied commitment txid. The test ark PSBT built by