Merge pull request #1741 from lightninglabs/passive-asset-split-output-fix

bugfix: fix two long-existing edge case bugs

Author: GeorgeTsagk

docs/release-notes/release-notes-0.7.0.md

@@ -26,6 +26,16 @@
 - [An integration test flake was
   fixed](https://github.com/lightninglabs/taproot-assets/pull/1651).
 
+- Fixed two send related bugs that would lead to either a `invalid transfer 
+  asset witness` or `unable to fund address send: error funding packet: unable 
+  to list eligible coins: unable to query commitments: mismatch of managed utxo
+  and constructed tap commitment root` error when sending assets.
+  The [PR that fixed the two
+  bugs](https://github.com/lightninglabs/taproot-assets/pull/1741) also
+  optimized sending to V2 TAP addresses by removing the need for creating
+  tombstone outputs on a full-value send (by using interactive transfers for V2
+  addresses).
+
 # New Features
 
 ## Functional Enhancements

itest/addrs_v2_test.go

@@ -271,8 +271,7 @@ func testAddressV2WithGroupKey(t *harnessTest) {
 	AssertAssetOutboundTransferWithOutputs(
 		t.t, t.lndHarness.Miner().Client, t.tapd,
 		sendResp.Transfer, [][]byte{firstAssetID, secondAssetID},
-		[]uint64{0, firstAsset.Amount, 0, secondAsset.Amount}, 0,
-		1, 4, true,
+		[]uint64{firstAsset.Amount, secondAsset.Amount}, 0, 1, 2, true,
 	)
 	AssertReceiveEventsCustom(t.t, addrEvents, []taprpc.AddrEventStatus{
 		statusConfirmed,
@@ -284,13 +283,6 @@ func testAddressV2WithGroupKey(t *harnessTest) {
 		WithNumUtxos(2),
 	)
 
-	// The sending node should only have two tombstone outputs, but the
-	// total value should be zero.
-	AssertBalanceByGroup(
-		t.t, t.tapd, hex.EncodeToString(groupKey), 0, WithNumUtxos(2),
-		WithAllScriptKeyTypes(),
-	)
-
 	// We now make sure we can spend those assets again by sending
 	// them back to ourselves, using an address with an amount.
 	groupAddrBob2, _ := NewAddrWithEventStream(
@@ -354,8 +346,8 @@ func testAddressV2WithGroupKey(t *harnessTest) {
 	AssertAssetOutboundTransferWithOutputs(
 		t.t, t.lndHarness.Miner().Client, bobTapd,
 		sendResp3.Transfer, [][]byte{firstAssetID, secondAssetID},
-		[]uint64{0, totalAmount/2 - 50, 50, 0, totalAmount / 2}, 1,
-		2, 5, true,
+		[]uint64{totalAmount/2 - 50, 50, totalAmount / 2}, 1, 2, 3,
+		true,
 	)
 	AssertBalanceByGroup(
 		t.t, t.tapd, hex.EncodeToString(groupKey), totalAmount,
@@ -396,6 +388,264 @@ func testAddressV2WithGroupKey(t *harnessTest) {
 	)
 }
 
+// testAddressV2WithGroupKeyMultipleRoundTrips tests what we can send assets
+// back and forth multiple times using V2 addresses.
+func testAddressV2WithGroupKeyMultipleRoundTrips(t *harnessTest) {
+	// We begin by minting a new asset group with a group key.
+	firstTrancheReq := CopyRequest(issuableAssets[0])
+
+	firstTrancheReq.Asset.Amount = 212e5
+
+	firstTranche := MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		[]*mintrpc.MintAssetRequest{firstTrancheReq},
+	)
+	firstAsset := firstTranche[0]
+	firstAssetID := firstAsset.AssetGenesis.AssetId
+	groupKey := firstAsset.AssetGroup.TweakedGroupKey
+
+	// And then we mint a second tranche of the same asset group.
+	secondTrancheReq := CopyRequest(firstTrancheReq)
+	secondTrancheReq.Asset.Name = "itestbuxx-money-printer-brrr-tranche-2"
+	secondTrancheReq.Asset.GroupedAsset = true
+	secondTrancheReq.Asset.NewGroupedAsset = false
+	secondTrancheReq.Asset.GroupKey = groupKey
+
+	secondTrancheReq.Asset.Amount = 202e5
+
+	secondTranche := MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		[]*mintrpc.MintAssetRequest{secondTrancheReq},
+	)
+	secondAsset := secondTranche[0]
+	secondAssetID := secondAsset.AssetGenesis.AssetId
+
+	// And then we mint a third tranche of the same asset group.
+	thirdTrancheReq := CopyRequest(firstTrancheReq)
+	thirdTrancheReq.Asset.Name = "itestbuxx-money-printer-brrr-tranche-3"
+	thirdTrancheReq.Asset.GroupedAsset = true
+	thirdTrancheReq.Asset.NewGroupedAsset = false
+	thirdTrancheReq.Asset.GroupKey = groupKey
+
+	thirdTrancheReq.Asset.Amount = 182e5
+
+	thirdTranche := MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		[]*mintrpc.MintAssetRequest{thirdTrancheReq},
+	)
+	thirdAsset := thirdTranche[0]
+	thirdAssetID := thirdAsset.AssetGenesis.AssetId
+
+	ctxb := context.Background()
+	ctxt, cancel := context.WithTimeout(ctxb, defaultWaitTimeout)
+	defer cancel()
+
+	totalAmount := firstAsset.Amount + secondAsset.Amount +
+		thirdAsset.Amount
+	t.Logf("Minted %d units for group %x", totalAmount, groupKey)
+
+	// Now we can create an address with the group key.
+	// We'll make a second node now that'll be the receiver of all the
+	// assets made above.
+	bobLnd := t.lndHarness.NewNodeWithCoins("Bob", nil)
+	bobTapd := setupTapdHarness(t.t, t, bobLnd, t.universeServer)
+	defer func() {
+		require.NoError(t.t, bobTapd.stop(!*noDelete))
+	}()
+
+	groupAddrBob, _ := NewAddrWithEventStream(
+		t.t, bobTapd, &taprpc.NewAddrRequest{
+			AddressVersion: taprpc.AddrVersion_ADDR_VERSION_V2,
+			GroupKey:       groupKey,
+		},
+	)
+
+	t.Logf("Got group addr: %v", toJSON(t.t, groupAddrBob))
+
+	currentTransferIdx := -1
+	numTransfers := 0
+
+	bobCurrentTransferIdx := -1
+	bobNumTransfers := 0
+
+	// We send the first tranche from alice to bob.
+	currentTransferIdx += 1
+	numTransfers += 1
+
+	sendResp, err := t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+		AddressesWithAmounts: []*taprpc.AddressWithAmount{
+			{
+				TapAddr: groupAddrBob.Encoded,
+				Amount:  firstAsset.Amount,
+			},
+		},
+	})
+	require.NoError(t.t, err)
+	t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+	AssertAssetOutboundTransferWithOutputs(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		sendResp.Transfer, [][]byte{firstAssetID},
+		[]uint64{firstAsset.Amount}, currentTransferIdx,
+		numTransfers, 1, true,
+	)
+
+	AssertAddrEventByStatus(t.t, bobTapd, statusCompleted, numTransfers)
+
+	// Then we send the second tranche from alice to bob.
+	currentTransferIdx += 1
+	numTransfers += 1
+
+	sendResp, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+		AddressesWithAmounts: []*taprpc.AddressWithAmount{
+			{
+				TapAddr: groupAddrBob.Encoded,
+				Amount:  secondAsset.Amount,
+			},
+		},
+	})
+	require.NoError(t.t, err)
+	t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+	AssertAssetOutboundTransferWithOutputs(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		sendResp.Transfer, [][]byte{secondAssetID},
+		[]uint64{secondAsset.Amount}, currentTransferIdx,
+		numTransfers, 1, true,
+	)
+
+	AssertAddrEventByStatus(t.t, bobTapd, statusCompleted, numTransfers)
+
+	// And now the third tranche from alice to bob.
+	currentTransferIdx += 1
+	numTransfers += 1
+
+	sendResp, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+		AddressesWithAmounts: []*taprpc.AddressWithAmount{
+			{
+				TapAddr: groupAddrBob.Encoded,
+				Amount:  thirdAsset.Amount,
+			},
+		},
+	})
+	require.NoError(t.t, err)
+	t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+	AssertAssetOutboundTransferWithOutputs(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		sendResp.Transfer, [][]byte{thirdAssetID},
+		[]uint64{thirdAsset.Amount}, currentTransferIdx,
+		numTransfers, 1, true,
+	)
+
+	AssertAddrEventByStatus(t.t, bobTapd, statusCompleted, numTransfers)
+
+	// We now make sure we can spend those assets again by sending
+	// them from bob back to Alice, using an address with an amount.
+	// this time, we'll send back all 3 tranches at once.
+	groupAddrAlice, _ := NewAddrWithEventStream(
+		t.t, t.tapd, &taprpc.NewAddrRequest{
+			Amt:            totalAmount,
+			AddressVersion: addrV2,
+			GroupKey:       groupKey,
+		},
+	)
+
+	t.Logf("Got group addr: %v", toJSON(t.t, groupAddrAlice))
+
+	bobCurrentTransferIdx += 1
+	bobNumTransfers += 1
+
+	sendResp, err = bobTapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+		AddressesWithAmounts: []*taprpc.AddressWithAmount{
+			{
+				TapAddr: groupAddrAlice.Encoded,
+			},
+		},
+	})
+	require.NoError(t.t, err)
+
+	t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+
+	MineBlocks(t.t, t.lndHarness.Miner().Client, 1, 1)
+
+	AssertAddrEventByStatus(t.t, t.tapd, statusCompleted, bobNumTransfers)
+
+	// Now we send back from alice to bob again, but this time all at once
+	// instead of individually.
+	for i := 0; i < 4; i++ {
+		currentTransferIdx += 1
+		numTransfers += 1
+
+		sendResp, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+			AddressesWithAmounts: []*taprpc.AddressWithAmount{
+				{
+					TapAddr: groupAddrBob.Encoded,
+					Amount:  totalAmount,
+				},
+			},
+		})
+		require.NoError(t.t, err)
+		t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+		MineBlocks(t.t, t.lndHarness.Miner().Client, 1, 1)
+		AssertAddrEventByStatus(
+			t.t, bobTapd, statusCompleted, numTransfers,
+		)
+
+		// We again return all three tranches back to Alice.
+		bobCurrentTransferIdx += 1
+		bobNumTransfers += 1
+
+		addrReq := []*taprpc.AddressWithAmount{
+			{
+				TapAddr: groupAddrAlice.Encoded,
+			},
+		}
+		sendResp, err = bobTapd.SendAsset(
+			ctxt, &taprpc.SendAssetRequest{
+				AddressesWithAmounts: addrReq,
+			},
+		)
+		require.NoError(t.t, err)
+		t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+		MineBlocks(t.t, t.lndHarness.Miner().Client, 1, 1)
+		AssertAddrEventByStatus(
+			t.t, t.tapd, statusCompleted, bobNumTransfers,
+		)
+	}
+
+	// We now send each tranche back to bob individually, to make sure they
+	// are spendable individually.
+	amounts := []uint64{
+		firstAsset.Amount, secondAsset.Amount, thirdAsset.Amount,
+	}
+	assetIDs := [][]byte{
+		firstAssetID, secondAssetID, thirdAssetID,
+	}
+	for idx, amount := range amounts {
+		currentTransferIdx += 1
+		numTransfers += 1
+
+		sendResp, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+			AddressesWithAmounts: []*taprpc.AddressWithAmount{
+				{
+					TapAddr: groupAddrBob.Encoded,
+					Amount:  amount,
+				},
+			},
+		})
+		require.NoError(t.t, err)
+		t.Logf("Sent asset to group addr: %v", toJSON(t.t, sendResp))
+		AssertAssetOutboundTransferWithOutputs(
+			t.t, t.lndHarness.Miner().Client, t.tapd,
+			sendResp.Transfer, [][]byte{assetIDs[idx]},
+			[]uint64{amount}, currentTransferIdx, numTransfers, 1,
+			true,
+		)
+
+		AssertAddrEventByStatus(
+			t.t, bobTapd, statusCompleted, numTransfers,
+		)
+	}
+}
+
 // testAddressV2WithGroupKeyRestart tests that we can re-try and properly
 // continue the address v2 send process in various scenarios.
 func testAddressV2WithGroupKeyRestart(t *harnessTest) {