Orphan UTXO filter and limit

docs/release-notes/release-notes-0.8.0.md

@@ -39,6 +39,15 @@
   `AnchorVirtualPsbts`. A new configuration is available to control the sweeping 
   via the flag `wallet.sweep-orphan-utxos`.
 
+- [Improve orphan UTXO sweeping](https://github.com/lightninglabs/taproot-assets/pull/1905):
+  Fixed two issues with fetching orphan UTXOs for sweeping during transaction
+  building:
+  - Added filtering to exclude orphan UTXOs with missing signing information
+    (KeyFamily=0 and KeyIndex=0). These UTXOs were created in prior versions
+    that didn't store this information, causing LND to fail when signing.
+  - Added a limit (`MaxOrphanUTXOs = 20`) to prevent transactions from becoming
+    too large when sweeping many orphan UTXOs at once.
+
 ## RPC Updates
 
 - [PR#1841](https://github.com/lightninglabs/taproot-assets/pull/1841): Remove
@@ -61,6 +70,11 @@
   `max-proof-cache-size` sets the proof cache limit in bytes and accepts
   human-readable values such as `64MB`.
 
+- [Enable orphan UTXO sweeping by default](https://github.com/lightninglabs/taproot-assets/pull/1905):
+  The `wallet.sweep-orphan-utxos` configuration option is now enabled by
+  default. This automatically sweeps tombstone and burn outputs when executing
+  on-chain transactions. Set to `false` to disable.
+
 ## Code Health
 
 - [PR#1897](https://github.com/lightninglabs/taproot-assets/pull/1897)

itest/tapd_harness.go

@@ -261,7 +261,9 @@ func newTapdHarness(t *testing.T, ht *harnessTest, cfg tapdConfig,
 	tapCfg.Universe.DisableSupplyVerifierChainWatch = opts.disableSupplyVerifierChainWatch
 
 	// Pass through the sweep orphan UTXOs flag. If the option was not set,
-	// this will be false, which is the default.
+	// this will be false. Note: The production default is true, but we
+	// disable it by default in tests to avoid interference with test
+	// assertions unless explicitly enabled via WithSweepOrphanUtxos().
 	tapCfg.Wallet.SweepOrphanUtxos = opts.sweepOrphanUtxos
 
 	switch {

tapcfg/config.go

@@ -297,7 +297,7 @@ type WalletConfig struct {
 
 	// SweepOrphanUtxos, when true, sweeps orphaned UTXOs into anchor
 	// transactions created during sends and burns.
-	SweepOrphanUtxos bool `long:"sweep-orphan-utxos" description:"Sweep orphaned UTXOs into anchor transactions created during sends and burns. Disabled by default."`
+	SweepOrphanUtxos bool `long:"sweep-orphan-utxos" description:"Sweep orphaned UTXOs into anchor transactions created during sends and burns. Enabled by default."`
 }
 
 // UniverseConfig is the config that houses any Universe related config
@@ -498,7 +498,7 @@ func DefaultConfig() Config {
 		},
 		Wallet: &WalletConfig{
 			PsbtMaxFeeRatio:  DefaultPsbtMaxFeeRatio,
-			SweepOrphanUtxos: false,
+			SweepOrphanUtxos: true,
 		},
 		AddrBook: &AddrBookConfig{
 			DisableSyncer: false,

tapdb/assets_store.go

@@ -188,6 +188,13 @@ type (
 	QueryBurnsFilters = sqlc.QueryBurnsParams
 )
 
+const (
+	// MaxOrphanUTXOs is the maximum number of orphan UTXOs that can be
+	// fetched at once. This limit prevents transactions from becoming too
+	// large when sweeping orphan UTXOs.
+	MaxOrphanUTXOs = 20
+)
+
 // ActiveAssetsStore is a sub-set of the main sqlc.Querier interface that
 // contains methods related to querying the set of confirmed assets.
 type ActiveAssetsStore interface {
@@ -1340,6 +1347,12 @@ func (a *AssetStore) FetchOrphanUTXOs(ctx context.Context) (
 		}
 
 		for _, u := range utxos {
+			if len(results) >= MaxOrphanUTXOs {
+				log.DebugS(ctx, "reached max orphan UTXOs "+
+					"limit", "limit", MaxOrphanUTXOs)
+				break
+			}
+
 			if len(u.LeaseOwner) > 0 &&
 				u.LeaseExpiry.Valid &&
 				u.LeaseExpiry.Time.UTC().After(now) {
@@ -1351,6 +1364,17 @@ func (a *AssetStore) FetchOrphanUTXOs(ctx context.Context) (
 				continue
 			}
 
+			// Skip UTXOs with missing signing information. In prior
+			// versions, we didn't store KeyFamily and KeyIndex for
+			// orphan UTXOs. If both are 0, LND will fail to sign
+			// the transaction.
+			if u.KeyFamily == 0 && u.KeyIndex == 0 {
+				log.DebugS(ctx, "skipping orphan utxo with "+
+					"missing signing info",
+					"raw_key", fmt.Sprintf("%x", u.RawKey))
+				continue
+			}
+
 			var anchorPoint wire.OutPoint
 			err := readOutPoint(
 				bytes.NewReader(u.Outpoint), 0, 0, &anchorPoint,

tapdb/assets_store_test.go

@@ -3383,3 +3383,262 @@ func TestUpsertAssetsWithSplitCommitments(t *testing.T) {
 		})
 	}
 }
+
+// TestFetchOrphanUTXOs tests that FetchOrphanUTXOs:
+// 1. Filters out UTXOs with missing signing info (KeyFamily=0 AND KeyIndex=0)
+// 2. Respects the MaxOrphanUTXOs limit.
+func TestFetchOrphanUTXOs(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name          string
+		utxosToInsert [][2]int32
+		expectedCount int
+		checkFunc     func(t *testing.T,
+			orphans []*tapfreighter.ZeroValueInput)
+	}{
+		{
+			name: "filters missing signing info",
+			utxosToInsert: [][2]int32{
+				{212, 5}, // Valid signing info.
+				{0, 0},   // Missing signing info.
+			},
+			expectedCount: 1,
+			checkFunc: func(t *testing.T,
+				orphans []*tapfreighter.ZeroValueInput) {
+
+				require.Equal(
+					t, keychain.KeyFamily(212),
+					orphans[0].InternalKey.Family,
+				)
+				require.Equal(
+					t, uint32(5),
+					orphans[0].InternalKey.Index,
+				)
+			},
+		},
+		{
+			name: "respects max limit",
+			// Insert MaxOrphanUTXOs + 10 UTXOs with valid signing
+			// info.
+			utxosToInsert: func() [][2]int32 {
+				utxos := make([][2]int32, MaxOrphanUTXOs+10)
+				for i := range utxos {
+					utxos[i] = [2]int32{
+						int32(i + 1), int32(i + 1),
+					}
+				}
+				return utxos
+			}(),
+			expectedCount: MaxOrphanUTXOs,
+			checkFunc: func(t *testing.T,
+				orphans []*tapfreighter.ZeroValueInput) {
+
+				// Verify all returned UTXOs have valid signing
+				// info.
+				for _, orphan := range orphans {
+					require.NotZero(
+						t, orphan.InternalKey.Family,
+					)
+					require.NotZero(
+						t, orphan.InternalKey.Index,
+					)
+				}
+			},
+		},
+		{
+			name: "ignores invalid even if total exceeds limit",
+			utxosToInsert: func() [][2]int32 {
+				utxos := make([][2]int32, 0, MaxOrphanUTXOs+1)
+
+				// Add MaxOrphanUTXOs entries with missing
+				// signing info.
+				for range MaxOrphanUTXOs {
+					utxos = append(utxos, [2]int32{0, 0})
+				}
+
+				// Add a single valid entry that should still
+				// be returned.
+				utxos = append(utxos, [2]int32{99, 42})
+
+				return utxos
+			}(),
+			expectedCount: 1,
+			checkFunc: func(t *testing.T,
+				orphans []*tapfreighter.ZeroValueInput) {
+
+				require.Equal(
+					t, keychain.KeyFamily(99),
+					orphans[0].InternalKey.Family,
+				)
+				require.Equal(
+					t, uint32(42),
+					orphans[0].InternalKey.Index,
+				)
+			},
+		},
+		{
+			name: "filters invalid before enforcing max limit",
+			utxosToInsert: func() [][2]int32 {
+				utxos := make([][2]int32, 0, MaxOrphanUTXOs*2+1)
+
+				// Add MaxOrphanUTXOs invalid entries that
+				// should be filtered out.
+				for range MaxOrphanUTXOs {
+					utxos = append(utxos, [2]int32{0, 0})
+				}
+
+				// Add MaxOrphanUTXOs+1 valid entries; only
+				// MaxOrphanUTXOs should be returned.
+				for i := range MaxOrphanUTXOs + 1 {
+					val := int32(i) + 1
+					utxos = append(
+						utxos, [2]int32{val, val},
+					)
+				}
+
+				return utxos
+			}(),
+			expectedCount: MaxOrphanUTXOs,
+			checkFunc: func(t *testing.T,
+				orphans []*tapfreighter.ZeroValueInput) {
+
+				for _, orphan := range orphans {
+					require.NotZero(
+						t, orphan.InternalKey.Family,
+					)
+					require.NotZero(
+						t, orphan.InternalKey.Index,
+					)
+				}
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, assetsStore, db := newAssetStore(t)
+			ctx := context.Background()
+
+			// Insert all UTXOs for this test case.
+			for _, keyInfo := range tc.utxosToInsert {
+				insertOrphanUTXO(
+					t, ctx, db, assetsStore,
+					keyInfo[0], keyInfo[1],
+				)
+			}
+
+			// Fetch orphan UTXOs.
+			orphans, err := assetsStore.FetchOrphanUTXOs(ctx)
+			require.NoError(t, err)
+
+			require.Len(t, orphans, tc.expectedCount)
+
+			if tc.checkFunc != nil {
+				tc.checkFunc(t, orphans)
+			}
+		})
+	}
+}
+
+// insertOrphanUTXO inserts a managed UTXO with a tombstone asset and the
+// specified KeyFamily and KeyIndex values for the internal key.
+func insertOrphanUTXO(t *testing.T, ctx context.Context, db sqlc.Querier,
+	assetsStore *AssetStore, keyFamily, keyIndex int32) wire.OutPoint {
+
+	internalKey := test.RandPubKey(t)
+
+	// Insert the internal key with specific KeyFamily and KeyIndex.
+	_, err := db.UpsertInternalKey(ctx, sqlc.UpsertInternalKeyParams{
+		RawKey:    internalKey.SerializeCompressed(),
+		KeyFamily: keyFamily,
+		KeyIndex:  keyIndex,
+	})
+	require.NoError(t, err)
+
+	// Create a chain transaction.
+	anchorTx := wire.NewMsgTx(2)
+	anchorTx.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: test.RandOp(t),
+	})
+	anchorTx.AddTxOut(&wire.TxOut{
+		PkScript: bytes.Repeat([]byte{0x01}, 34),
+		Value:    1000,
+	})
+
+	txBytes, err := fn.Serialize(anchorTx)
+	require.NoError(t, err)
+
+	txHash := anchorTx.TxHash()
+	chainTxID, err := db.UpsertChainTx(ctx, sqlc.UpsertChainTxParams{
+		Txid:        txHash[:],
+		RawTx:       txBytes,
+		BlockHeight: sqlInt32(100),
+	})
+	require.NoError(t, err)
+
+	outpoint := wire.OutPoint{
+		Hash:  txHash,
+		Index: 0,
+	}
+	outpointBytes, err := encodeOutpoint(outpoint)
+	require.NoError(t, err)
+
+	// Insert the managed UTXO.
+	_, err = db.UpsertManagedUTXO(ctx, sqlc.UpsertManagedUTXOParams{
+		RawKey:           internalKey.SerializeCompressed(),
+		Outpoint:         outpointBytes,
+		AmtSats:          1000,
+		TaprootAssetRoot: test.RandBytes(32),
+		MerkleRoot:       test.RandBytes(32),
+		TxnID:            chainTxID,
+	})
+	require.NoError(t, err)
+
+	// Create and insert a tombstone asset for this UTXO.
+	gen := asset.RandGenesis(t, asset.Normal)
+	gen.FirstPrevOut = outpoint
+	tombstone := asset.NewAssetNoErr(
+		t, gen, 0, 0, 0, asset.NUMSScriptKey, nil,
+	)
+
+	assetCommitment, err := commitment.NewAssetCommitment(tombstone)
+	require.NoError(t, err)
+
+	tapCommitment, err := commitment.NewTapCommitment(nil, assetCommitment)
+	require.NoError(t, err)
+
+	txMerkleProof, err := proof.NewTxMerkleProof([]*wire.MsgTx{anchorTx}, 0)
+	require.NoError(t, err)
+
+	assetProof := proof.Proof{
+		AnchorTx:      *anchorTx,
+		BlockHeight:   100,
+		TxMerkleProof: *txMerkleProof,
+		Asset:         *tombstone,
+		InclusionProof: proof.TaprootProof{
+			OutputIndex: 0,
+			InternalKey: internalKey,
+		},
+	}
+
+	proofBlob, err := proof.EncodeAsProofFile(&assetProof)
+	require.NoError(t, err)
+
+	err = assetsStore.ImportProofs(
+		ctx, proof.MockVerifierCtx, false,
+		&proof.AnnotatedProof{
+			AssetSnapshot: &proof.AssetSnapshot{
+				AnchorTx:          anchorTx,
+				InternalKey:       internalKey,
+				Asset:             tombstone,
+				ScriptRoot:        tapCommitment,
+				AnchorBlockHeight: 100,
+			},
+			Blob: proofBlob,
+		},
+	)
+	require.NoError(t, err)
+
+	return outpoint
+}