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