Repository: lightninglabs/taproot-assets

Issue:
[bug/feature]: Implement PrevId[] UTXO selection for FundVirtualPSBT RPC method 
### Background

When operating a pocket universe, an operator may want to assist users in funding a PSBT to fulfill a Taproot Asset address transfer. In the case of Tajfi, we do this by taking the user's public key and then searching for UTXOs we control that have a script key matching the user, and then including an array of user-controlled-UTXOs that match, or exceed, the decoded amount of the Taproot Asset address invoice to fulfill.

Currently, we cannot provide the PrevId because the RPC method is not yet implemented, but it does seem that future support was already planned: https://github.com/lightninglabs/taproot-assets/blob/main/rpcserver.go#L2065
```
case req.GetRaw() != nil:
		raw := req.GetRaw()
		if len(raw.Inputs) > 0 {
			return nil, fmt.Errorf("template inputs not yet " +
				"supported")
		}
```

So I'd say this is kinda inbetween a bug and feature request. I think this would probably be a great beginner user task, and I'll probably take a stab at it soon.

### Your environment

Using tapd v0.4.1. Although probably not relevant since as mentioned above, this behavior/error path seems to be expected regardless of path.

### Steps to reproduce
This should be reproducible by sending a valid RPC payload for FundVirtualPSBT. An example of this payload for the HTTP request is provided below:

```
{
  "raw": {
    "recipients": {
      "tapabcxyz": 0 
    },
    "inputs": [
      {
        "outpoint": {
          "txid": "da742200b442ab7d6b8d5e39c2b1203f74b1ff3fcb5237d0418771b960e5243f",
          "output_index": 1
        },
        "id": "0a17de7503cc844f3a4fbf964af818266cd5f8369b2927971e8bafd62d897fdf",
        "script_key": "02d4d18c8f36edad6f3fba2407ac4a8b5d9416071febac4016adeeff8dd862e83a"
      }
    ]
  }
}
```

Removing the inputs object will allow the call to continue, but can result in a problem where the UTXOs selected might not have the desired script_key the pocket universe user intends on using to fulfill their transfer.

### Expected behavior

The daemon should attempt to only fund the virtual PSBT using the inputs provided in the raw payload.


Discussion:
@habibitcoin: Gave a stab at this in https://github.com/lightninglabs/taproot-assets/pull/1172 and have something that has at least unblocked our internal testing.@guggero: Fixed by #1172.

Solution patch:
diff --git a/rpcserver.go b/rpcserver.go
index 0327d562..fb896e29 100644
--- a/rpcserver.go
+++ b/rpcserver.go
@@ -2146,10 +2146,47 @@ func (r *rpcServer) FundVirtualPsbt(ctx context.Context,
 		}
 
 	case req.GetRaw() != nil:
-		raw := req.GetRaw()
-		if len(raw.Inputs) > 0 {
-			return nil, fmt.Errorf("template inputs not yet " +
-				"supported")
+		var (
+			raw     = req.GetRaw()
+			prevIDs []asset.PrevID
+		)
+		for i, input := range raw.Inputs {
+			if input.Outpoint == nil {
+				return nil, fmt.Errorf("input at index %d has "+
+					"a nil Outpoint", i)
+			}
+
+			hash, err := chainhash.NewHash(input.Outpoint.Txid)
+			if err != nil {
+				return nil, fmt.Errorf("input at index %d has "+
+					"invalid Txid: %w", i, err)
+			}
+
+			scriptKey, err := parseUserKey(input.ScriptKey)
+			if err != nil {
+				return nil, fmt.Errorf("input at index %d has "+
+					"invalid script key: %w", i, err)
+			}
+
+			if len(input.Id) != 32 {
+				return nil, fmt.Errorf("input at index %d has "+
+					"invalid asset ID of %d bytes, must "+
+					"be 32 bytes", i, len(input.Id))
+			}
+
+			// Decode the input into an asset.PrevID.
+			outpoint := wire.OutPoint{
+				Hash:  *hash,
+				Index: input.Outpoint.OutputIndex,
+			}
+			prevID := asset.PrevID{
+				OutPoint: outpoint,
+				ID:       asset.ID(input.Id),
+				ScriptKey: asset.ToSerialized(
+					scriptKey,
+				),
+			}
+			prevIDs = append(prevIDs, prevID)
 		}
 		if len(raw.Recipients) > 1 {
 			return nil, fmt.Errorf("only one recipient supported")
@@ -2172,7 +2209,7 @@ func (r *rpcServer) FundVirtualPsbt(ctx context.Context,
 		}
 
 		fundedVPkt, err = r.cfg.AssetWallet.FundAddressSend(
-			ctx, coinSelectType, addr,
+			ctx, coinSelectType, prevIDs, addr,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("error funding address send: "+
diff --git a/tapdb/assets_store.go b/tapdb/assets_store.go
index 3067ece7..75c839d0 100644
--- a/tapdb/assets_store.go
+++ b/tapdb/assets_store.go
@@ -2001,7 +2001,48 @@ func (a *AssetStore) ListEligibleCoins(ctx context.Context,
 	// have a block height of 0, so we set the minimum block height to 1.
 	assetFilter.MinAnchorHeight = sqlInt32(1)
 
-	return a.queryCommitments(ctx, assetFilter)
+	selectedCommitments, err := a.queryCommitments(ctx, assetFilter)
+	if err != nil {
+		return nil, fmt.Errorf("unable to query commitments: %w", err)
+	}
+
+	// If we want to restrict on specific inputs, we do the filtering now.
+	if len(constraints.PrevIDs) > 0 {
+		selectedCommitments = filterCommitmentsByPrevIDs(
+			selectedCommitments, constraints.PrevIDs,
+		)
+
+		// If this results in an empty list, we return the same error we
+		// would if there were no coins found without the filter.
+		if len(selectedCommitments) == 0 {
+			return nil, tapfreighter.ErrMatchingAssetsNotFound
+		}
+	}
+
+	return selectedCommitments, nil
+}
+
+// filterCommitmentsByPrevIDs filters the given commitments by the previous IDs
+// given.
+func filterCommitmentsByPrevIDs(commitments []*tapfreighter.AnchoredCommitment,
+	prevIDs []asset.PrevID) []*tapfreighter.AnchoredCommitment {
+
+	prevIDMatches := func(p asset.PrevID,
+		c *tapfreighter.AnchoredCommitment) bool {
+
+		return p.OutPoint == c.AnchorPoint && p.ID == c.Asset.ID() &&
+			p.ScriptKey == asset.ToSerialized(
+				c.Asset.ScriptKey.PubKey,
+			)
+	}
+
+	commitmentInList := func(c *tapfreighter.AnchoredCommitment) bool {
+		return fn.Any(prevIDs, func(p asset.PrevID) bool {
+			return prevIDMatches(p, c)
+		})
+	}
+
+	return fn.Filter(commitments, commitmentInList)
 }
 
 // LeaseCoins leases/locks/reserves coins for the given lease owner until the
diff --git a/tapfreighter/chain_porter.go b/tapfreighter/chain_porter.go
index e58bfe67..a32c3145 100644
--- a/tapfreighter/chain_porter.go
+++ b/tapfreighter/chain_porter.go
@@ -1084,7 +1084,7 @@ func (p *ChainPorter) stateStep(currentPkg sendPackage) (*sendPackage, error) {
 				"address parcel")
 		}
 		fundSendRes, err := p.cfg.AssetWallet.FundAddressSend(
-			ctx, tapsend.Bip86Only, addrParcel.destAddrs...,
+			ctx, tapsend.Bip86Only, nil, addrParcel.destAddrs...,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("unable to fund address send: "+
diff --git a/tapfreighter/coin_select.go b/tapfreighter/coin_select.go
index 7ef11171..b3b49092 100644
--- a/tapfreighter/coin_select.go
+++ b/tapfreighter/coin_select.go
@@ -55,6 +55,7 @@ func (s *CoinSelect) SelectCoins(ctx context.Context,
 		AssetSpecifier: constraints.AssetSpecifier,
 		MinAmt:         1,
 		CoinSelectType: constraints.CoinSelectType,
+		PrevIDs:        constraints.PrevIDs,
 	}
 	eligibleCommitments, err := s.coinLister.ListEligibleCoins(
 		ctx, listConstraints,
diff --git a/tapfreighter/interface.go b/tapfreighter/interface.go
index 3649b419..b16131ab 100644
--- a/tapfreighter/interface.go
+++ b/tapfreighter/interface.go
@@ -35,6 +35,9 @@ type CommitmentConstraints struct {
 	// to satisfy the constraints.
 	MinAmt uint64
 
+	// PrevIDs are the set of inputs allowed to be used.
+	PrevIDs []asset.PrevID
+
 	// CoinSelectType is the type of coins that should be selected.
 	CoinSelectType tapsend.CoinSelectType
 }
diff --git a/tapfreighter/wallet.go b/tapfreighter/wallet.go
index baff3a3b..7c8a754d 100644
--- a/tapfreighter/wallet.go
+++ b/tapfreighter/wallet.go
@@ -69,7 +69,7 @@ type Wallet interface {
 	// asset re-anchors and the Taproot Asset level commitment of the
 	// selected assets.
 	FundAddressSend(ctx context.Context,
-		coinSelectType tapsend.CoinSelectType,
+		coinSelectType tapsend.CoinSelectType, prevIDs []asset.PrevID,
 		receiverAddrs ...*address.Tap) (*FundedVPacket, error)
 
 	// FundPacket funds a virtual transaction, selecting assets to spend
@@ -241,6 +241,7 @@ type FundedVPacket struct {
 // NOTE: This is part of the Wallet interface.
 func (f *AssetWallet) FundAddressSend(ctx context.Context,
 	coinSelectType tapsend.CoinSelectType,
+	prevIDs []asset.PrevID,
 	receiverAddrs ...*address.Tap) (*FundedVPacket, error) {
 
 	// We start by creating a new virtual transaction that will be used to
@@ -258,6 +259,11 @@ func (f *AssetWallet) FundAddressSend(ctx context.Context,
 		return nil, fmt.Errorf("unable to describe recipients: %w", err)
 	}
 
+	// We need to constrain the prevIDs if they are provided.
+	if len(prevIDs) > 0 {
+		fundDesc.PrevIDs = prevIDs
+	}
+
 	fundDesc.CoinSelectType = coinSelectType
 	fundedVPkt, err := f.FundPacket(ctx, fundDesc, vPkt)
 	if err != nil {
@@ -398,6 +404,7 @@ func (f *AssetWallet) FundPacket(ctx context.Context,
 		AssetSpecifier: fundDesc.AssetSpecifier,
 		MinAmt:         fundDesc.Amount,
 		CoinSelectType: fundDesc.CoinSelectType,
+		PrevIDs:        fundDesc.PrevIDs,
 	}
 
 	anchorVersion, err := tappsbt.CommitmentVersion(vPkt.Version)
diff --git a/tapsend/send.go b/tapsend/send.go
index 5b47b670..a0a42083 100644
--- a/tapsend/send.go
+++ b/tapsend/send.go
@@ -203,6 +203,9 @@ type FundingDescriptor struct {
 	// Amount is the amount of the asset to transfer.
 	Amount uint64
 
+	// PrevIDs is the set of inputs that can be used to fund the transfer.
+	PrevIDs []asset.PrevID
+
 	// CoinSelectType specifies the type of coins that should be selected.
 	CoinSelectType CoinSelectType
 }



Test patch:
diff --git a/itest/assets_test.go b/itest/assets_test.go
index 36682ce0..83b1249d 100644
--- a/itest/assets_test.go
+++ b/itest/assets_test.go
@@ -627,16 +627,27 @@ func testMintBatchAndTransfer(t *harnessTest) {
 	require.True(t.t, proto.Equal(originalBatch, afterBatch))
 }
 
-// testAssetBalances tests the balance retrieval functionality for issued
-// assets. The function mints two batches of assets and asserts if the tapcli
-// `assets balance` returns the correct balances. It then funds a vPSBT, putting
-// a lease on one of the two batches. It then asserts whether the endpoint still
-// returns the correct balances, taking into account the `include_leased` flag.
+// testAssetBalances validates the balance retrieval and virtual PSBT funding
+// functionality for issued assets. The test performs the following steps:
+//  1. Mints two batches of assets and verifies that the tapcli
+//     `assets balance` returns the correct balances.
+//  2. Tests funding a vPSBT, putting a lease on one of the two batches, with
+//     the `FundVirtualPsbt` RPC for various scenarios:
+//     - Fails if the Inputs field contains a nil Outpoint.
+//     - Fails if the Inputs field contains an invalid Outpoint.
+//     - Fails if the Inputs field contains an invalid short AssetId.
+//     - Fails if the Inputs field contains an invalid short ScriptKey.
+//     - Succeeds if a valid Outpoint is provided in the Inputs field.
+//     - Succeeds if the Inputs field is not provided at all, using asset
+//     coin selection instead.
+//  3. Ensures that leased assets are reflected correctly in the balance
+//     retrieval, with and without the `include_leased` flag.
 func testAssetBalances(t *harnessTest) {
 	ctxb := context.Background()
 	ctxt, cancel := context.WithTimeout(ctxb, defaultWaitTimeout)
 	defer cancel()
 
+	// Mint assets for testing.
 	rpcSimpleAssets := MintAssetsConfirmBatch(
 		t.t, t.lndHarness.Miner().Client, t.tapd, simpleAssets,
 	)
@@ -652,8 +663,18 @@ func testAssetBalances(t *harnessTest) {
 		t.t, t.tapd, rpcSimpleAssets, rpcIssuableAssets, false,
 	)
 
+	// Check chain anchor for the target asset.
+	out, err := wire.NewOutPointFromString(
+		targetAsset.ChainAnchor.AnchorOutpoint,
+	)
+	require.NoError(t.t, err)
+
 	var (
 		targetAssetGenesis = targetAsset.AssetGenesis
+		assetId            = targetAssetGenesis.AssetId
+		scriptKey          = targetAsset.ScriptKey
+		anchorTxid         = out.Hash.CloneBytes()
+		anchorVout         = out.Index
 		aliceTapd          = t.tapd
 		bobLnd             = t.lndHarness.Bob
 	)
@@ -672,31 +693,154 @@ func testAssetBalances(t *harnessTest) {
 	})
 	require.NoError(t.t, err)
 
-	// Now we can create our virtual transaction and ask Alice's tapd to
-	// fund it.
 	recipients := map[string]uint64{
 		bobAddr.Encoded: bobAddr.Amount,
 	}
-	_, err = aliceTapd.FundVirtualPsbt(
-		ctxt, &wrpc.FundVirtualPsbtRequest{
-			Template: &wrpc.FundVirtualPsbtRequest_Raw{
-				Raw: &wrpc.TxTemplate{
-					Recipients: recipients,
+
+	subTests := []struct {
+		name           string
+		inputs         []*wrpc.PrevId
+		expectError    bool
+		expectedErrMsg string
+	}{
+		{
+			name: "Fail if Inputs are provided but Outpoint is nil",
+			inputs: []*wrpc.PrevId{
+				{
+					Outpoint:  nil,
+					Id:        assetId,
+					ScriptKey: scriptKey,
 				},
 			},
+			expectError:    true,
+			expectedErrMsg: "index 0 has a nil Outpoint",
 		},
-	)
-	require.NoError(t.t, err)
+		{
+			name: "Fail if Inputs contain an invalid Outpoint",
+			inputs: []*wrpc.PrevId{
+				{
+					Outpoint: &taprpc.OutPoint{
+						Txid: []byte(
+							"invalid_txid",
+						),
+						OutputIndex: anchorVout,
+					},
+					Id:        assetId,
+					ScriptKey: scriptKey,
+				},
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid Txid",
+		},
+		{
+			name: "Fail if AssetId is too short",
+			inputs: []*wrpc.PrevId{
+				{
+					Outpoint: &taprpc.OutPoint{
+						Txid:        anchorTxid,
+						OutputIndex: anchorVout,
+					},
+					Id:        []byte{1, 2},
+					ScriptKey: scriptKey,
+				},
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid asset ID",
+		},
+		{
+			name: "Fail if ScriptKey is too short",
+			inputs: []*wrpc.PrevId{
+				{
+					Outpoint: &taprpc.OutPoint{
+						Txid:        anchorTxid,
+						OutputIndex: anchorVout,
+					},
+					Id:        assetId,
+					ScriptKey: []byte{1, 2},
+				},
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid script key",
+		},
+		{
+			name: "Fail if valid, unfindable Outpoint is provided",
+			inputs: []*wrpc.PrevId{
+				{
+					Outpoint: &taprpc.OutPoint{
+						Txid:        anchorTxid,
+						OutputIndex: anchorVout + 1,
+					},
+					Id:        assetId,
+					ScriptKey: scriptKey,
+				},
+			},
+			expectError:    true,
+			expectedErrMsg: "failed to find coin(s)",
+		},
+		{
+			name: "Succeed if a valid Outpoint is provided",
+			inputs: []*wrpc.PrevId{
+				{
+					Outpoint: &taprpc.OutPoint{
+						Txid:        anchorTxid,
+						OutputIndex: anchorVout,
+					},
+					Id:        assetId,
+					ScriptKey: scriptKey,
+				},
+			},
+			expectError: false,
+		},
+		{
+			name:        "Succeed if no Inputs are provided",
+			inputs:      nil,
+			expectError: false,
+		},
+	}
 
-	// With a transaction funding should have led to a lease on the simple
-	// assets, we'll use the balance calls to ensure that we're able to
-	// retrieve the proper balances.
-	rpcEmptyAssets := []*taprpc.Asset{}
-	AssertAssetBalances(
-		t.t, t.tapd, rpcEmptyAssets, rpcIssuableAssets, false,
-	)
+	for _, tt := range subTests {
+		// Now we can create our virtual transaction and ask Alice's
+		// tapd to fund it.
+		_, err := aliceTapd.FundVirtualPsbt(
+			ctxt, &wrpc.FundVirtualPsbtRequest{
+				Template: &wrpc.FundVirtualPsbtRequest_Raw{
+					Raw: &wrpc.TxTemplate{
+						Recipients: recipients,
+						Inputs:     tt.inputs,
+					},
+				},
+			},
+		)
 
-	AssertAssetBalances(
-		t.t, t.tapd, rpcSimpleAssets, rpcIssuableAssets, true,
-	)
+		if tt.expectError {
+			require.ErrorContains(t.t, err, tt.expectedErrMsg)
+		} else {
+			require.NoError(t.t, err)
+
+			// With a transaction funding should have led to a
+			// lease on the simple assets, we'll use the balance
+			// calls to ensure that we're able to retrieve the
+			// proper balances.
+			rpcEmptyAssets := []*taprpc.Asset{}
+			AssertAssetBalances(
+				t.t, t.tapd, rpcEmptyAssets, rpcIssuableAssets,
+				false,
+			)
+			AssertAssetBalances(
+				t.t, t.tapd, rpcSimpleAssets, rpcIssuableAssets,
+				true,
+			)
+
+			// Unlock the input if provided so it can be reused.
+			for _, input := range tt.inputs {
+				_, err = aliceTapd.RemoveUTXOLease(
+					ctxb,
+					&wrpc.RemoveUTXOLeaseRequest{
+						Outpoint: input.Outpoint,
+					},
+				)
+				require.NoError(t.t, err)
+			}
+		}
+	}
 }