Implement PrevId[] UTXO selection for FundVirtualPSBT RPC method; valid for single PrevId at first

[habibitcoin]

Related to #1164

This PR simply implements the plumbing required to utilize the PrevIds payload for FundVirtualPSBT. Similar to how we can only handle 1 address for a raw input, this PR also only implements support for a single PrevId in the PrevIds payload.

At a high level, we perform the following:

• Update FundAddressSend() to take in an optional array of PrevID pointers
• If we see the PrevID pointers array is provided, and the length is 1, we add these values to the CommitmentConstraints object. CommitmentConstraints is also updated as a struct to hold an array of PrevIDs
• We update constraintsToDbFilter to utilize the prevID if the length is 1:

if len(query.PrevIDs) == 1 {
			prevID := query.PrevIDs[0]
			encodedOutpoint, err := encodeOutpoint(prevID.OutPoint)
			if err == nil {
				assetFilter.AnchorPoint = encodedOutpoint
				assetFilter.TweakedScriptKey = prevID.ScriptKey.CopyBytes()
			}
			// If there's an error, we simply skip setting the fields
			// TODO, maybe log error or return it?
		}

• Return an error to the RPC if an attempt is made to send more than 1 input

To-dos:

• For some reason, my outpoint is not being encoded properly, but during testing if I simply comment out assetFilter.AnchorPoint = encodedOutpoint, everything else works as expected and is serving our needs in our demo environment
• itest

[coveralls]

Pull Request Test Coverage Report for Build 13447455949

Details

• 73 of 73 (100.0%) changed or added relevant lines in 5 files are covered.
• 49 unchanged lines in 11 files lost coverage.
• Overall coverage increased (+0.07%) to 54.49%

---

Files with Coverage Reduction	New Missed Lines	%
commitment/tap.go	1	85.0%
fn/context_guard.go	1	88.71%
asset/asset.go	2	80.04%
itest/assertions.go	3	90.52%
proof/courier.go	3	79.37%
tapchannel/aux_leaf_signer.go	3	43.43%
tapgarden/caretaker.go	4	77.52%
tapdb/multiverse.go	6	81.65%
tapfreighter/chain_porter.go	6	80.44%
asset/mock.go	9	71.87%

Totals
Change from base Build 13435465457:	0.07%
Covered Lines:	48712
Relevant Lines:	89397

---

💛 - Coveralls

[guggero]

Thanks for the PR! This is definitely something we want as well and just didn't get to yet.
See inline comment for alternative and slightly more generic approach.

[habibitcoin]

@guggero addressed! Any thoughts on modifying or adding a new itest? I was thinking maybe to modify

// testPsbtExternalCommit tests the ability to fully customize the BTC level of
// an asset transfer using a PSBT. This exercises the CommitVirtualPsbts and
// PublishAndLogTransfer RPCs. The test case moves some assets into an output
// that has a hash lock tapscript.
func testPsbtExternalCommit(t *harnessTest) {

Or copy + update the scenario to include an array of 2 inputs perhaps

[guggero]

Thanks a lot for the updates!
Mostly nits remaining. Would be nice to perhaps add an integration test? Do you have time to do that?

[habibitcoin]

@guggero I decided to modify the testAssetBalances itest, as it seemed to be a suitable candidate to test this functionality.

I am having trouble however testing both the FundVirtualPSBT RPC with a valid PrevId, release the input lease, and then trying again without any PrevId specified, confirming we succeed

I was trying to use tapgarden.NewMockWalletAnchor(), but that doesn't seem to work. I was also seeing if maybe I could create a taprootassets.NewLndRpcWalletAnchor(), but wasn't sure what to use as the input. Curious if you had any recommendations there? I think once that is fixed, all the tests will pass and this should be ready to ship!

	// 4. Succeed if no Inputs are provided.
	// Unlock the input so that it can be spent again.
	walletAnchor := tapgarden.NewMockWalletAnchor()
	err = walletAnchor.UnlockInput(ctxb, *out)
	require.NoError(t.t, err)

	_, err = aliceTapd.FundVirtualPsbt(
		ctxt, &wrpc.FundVirtualPsbtRequest{
			Template: &wrpc.FundVirtualPsbtRequest_Raw{
				Raw: &wrpc.TxTemplate{
					Recipients: recipients,
				},
			},
		},
	)
	require.NoError(t.t, err)
	
	```

[guggero]

Yeah, that sounds good as an integration test.

You can't use a mock in itests, as that will literally not do anything. What you'll want to call into instead is assetwalletrpc.RemoveUTXOLease to release the asset-level input lease.
It's possible that any BTC on-chain funds added to pay for the fees then also remain leased. Those you need to release on the lnd RPC with walletrpc.ReleaseOutput.
Or you just make sure the lnd that's backing Alice has enough on-chain UTXOs so it doesn't become an issue in the first place.

[guggero]

Thanks a lot for the update and test!
Looks very close, just a couple of suggestions and fixes.

[guggero]

Thanks a lot, LGTM 🎉

[habibitcoin]

@dstadulis just wanted to bump this before it gets stale

[gijswijs]

Great work! I made some comments and a clarifying question and a single change request. Very nice to see this addition.

This also needs a rebase, before we can merge.

[lightninglabs-deploy]

@habibitcoin, remember to re-request review from reviewers when ready

[habibitcoin]

@gijswijs thank you for the review and appreciate your patience - this should be in good shape now!

[gijswijs]

LGTM 🎉