Merge pull request #797 from lightninglabs/initiator-zero-balance-coop-close

custom channels: add more itest coverage, fix pending channel bug

Author: guggero

config.go

@@ -400,19 +400,6 @@ func loadAndValidateConfig(interceptor signal.Interceptor) (*Config, error) {
 		cfg.Lnd.RPCMiddleware.Enable = true
 	}
 
-	// For the integration of tapd with lnd, we need to allow tapd to send
-	// custom error messages to peers through the SendCustomMessage RPC in
-	// lnd. Since the error messages aren't in the custom range, we
-	// explicitly need to allow them. This isn't currently needed in remote
-	// mode, because custom channels are only available if both lnd and tapd
-	// are running in integrated mode.
-	if !cfg.lndRemote {
-		cfg.Lnd.ProtocolOptions.CustomMessage = append(
-			cfg.Lnd.ProtocolOptions.CustomMessage,
-			lnwire.MsgError,
-		)
-	}
-
 	// Validate the lightning-terminal config options.
 	litDir := lnd.CleanAndExpandPath(preCfg.LitDir)
 	cfg.LetsEncryptDir = lncfg.CleanAndExpandPath(cfg.LetsEncryptDir)
@@ -620,6 +607,19 @@ func loadConfigFile(preCfg *Config, interceptor signal.Interceptor) (*Config,
 	// configuration is fully valid. This also sets up the main logger that
 	// logs to a sub-directory in the .lnd folder.
 	case ModeIntegrated:
+		// For the integration of tapd with lnd, we need to allow tapd
+		// to send custom error messages to peers through the
+		// SendCustomMessage RPC in lnd. Since the error messages aren't
+		// in the custom range, we explicitly need to allow them. This
+		// isn't currently needed in remote mode, because custom
+		// channels are only available if both lnd and tapd are running
+		// in integrated mode. We need to set this value before we call
+		// lnd.ValidateConfig() below, because that's what's going to
+		// inject these values into the lnwire package.
+		cfg.Lnd.ProtocolOptions.CustomMessage = append(
+			cfg.Lnd.ProtocolOptions.CustomMessage, lnwire.MsgError,
+		)
+
 		var err error
 		cfg.Lnd, err = lnd.ValidateConfig(
 			*cfg.Lnd, interceptor, fileParser, flagParser,

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/lightninglabs/loop/swapserverrpc v1.0.8
 	github.com/lightninglabs/pool v0.6.5-beta.0.20240604070222-e121aadb3289
 	github.com/lightninglabs/pool/auctioneerrpc v1.1.2
-	github.com/lightninglabs/taproot-assets v0.4.0-rc4.0.20240716183814-7647ba8f48d5
+	github.com/lightninglabs/taproot-assets v0.4.0-rc4.0.20240719071936-7035dedc860a
 	github.com/lightningnetwork/lnd v0.18.0-beta.rc4.0.20240712025014-90f997c908d0
 	github.com/lightningnetwork/lnd/cert v1.2.2
 	github.com/lightningnetwork/lnd/fn v1.1.0

go.sum

@@ -1175,8 +1175,8 @@ github.com/lightninglabs/pool/auctioneerrpc v1.1.2 h1:Dbg+9Z9jXnhimR27EN37foc4aB
 github.com/lightninglabs/pool/auctioneerrpc v1.1.2/go.mod h1:1wKDzN2zEP8srOi0B9iySlEsPdoPhw6oo3Vbm1v4Mhw=
 github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display h1:pRdza2wleRN1L2fJXd6ZoQ9ZegVFTAb2bOQfruJPKcY=
 github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
-github.com/lightninglabs/taproot-assets v0.4.0-rc4.0.20240716183814-7647ba8f48d5 h1:+d5HMOuBUWMsUVx6tAzy7+g8m4A76tec5/FOb1w5iVs=
-github.com/lightninglabs/taproot-assets v0.4.0-rc4.0.20240716183814-7647ba8f48d5/go.mod h1:oAiEnRj2sCbPHAURot+tmKbyDhIoxnvkmag0JqlF1bs=
+github.com/lightninglabs/taproot-assets v0.4.0-rc4.0.20240719071936-7035dedc860a h1:iWKqqgFcrOCJc4tviMguRNncdwtR/jATWkulQD2FBIQ=
+github.com/lightninglabs/taproot-assets v0.4.0-rc4.0.20240719071936-7035dedc860a/go.mod h1:oAiEnRj2sCbPHAURot+tmKbyDhIoxnvkmag0JqlF1bs=
 github.com/lightningnetwork/lightning-onion v1.2.1-0.20230823005744-06182b1d7d2f h1:Pua7+5TcFEJXIIZ1I2YAUapmbcttmLj4TTi786bIi3s=
 github.com/lightningnetwork/lightning-onion v1.2.1-0.20230823005744-06182b1d7d2f/go.mod h1:c0kvRShutpj3l6B9WtTsNTBUtjSmjZXbJd9ZBRQOSKI=
 github.com/lightningnetwork/lnd v0.18.0-beta.rc4.0.20240712025014-90f997c908d0 h1:V+PoltFSxN5oijkErYe+QbnVz5WJjBsAzaMNRrhmz3Q=

itest/assets_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/btcsuite/btcd/btcec/v2/schnorr"
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
+	"github.com/btcsuite/btcd/wire"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/lightninglabs/taproot-assets/itest"
 	"github.com/lightninglabs/taproot-assets/proof"
@@ -916,10 +917,19 @@ func waitForSendEvent(t *testing.T,
 	}
 }
 
+// coOpCloseBalanceCheck is a function type that can be passed into
+// closeAssetChannelAndAsset to asset the final balance of the closing
+// transaction.
+type coOpCloseBalanceCheck func(t *testing.T, local, remote *HarnessNode,
+	closeTx *wire.MsgTx, closeUpdate *lnrpc.ChannelCloseUpdate,
+	assetID, groupKey []byte, universeTap *tapClient)
+
+// closeAssetChannelAndAssert closes the channel between the local and remote
+// node and asserts the final balances of the closing transaction.
 func closeAssetChannelAndAssert(t *harnessTest, net *NetworkHarness,
 	local, remote *HarnessNode, chanPoint *lnrpc.ChannelPoint,
-	assetID, groupKey []byte, universeTap *tapClient, remoteBtcBalance,
-	remoteAssetBalance bool) {
+	assetID, groupKey []byte, universeTap *tapClient,
+	balanceCheck coOpCloseBalanceCheck) {
 
 	t.t.Helper()
 
@@ -953,6 +963,41 @@ func closeAssetChannelAndAssert(t *harnessTest, net *NetworkHarness,
 
 	waitForSendEvent(t.t, sendEvents, tapfreighter.SendStateComplete)
 
+	// Check the final balance of the closing transaction.
+	balanceCheck(
+		t.t, local, remote, closeTx, closeUpdate, assetID, groupKey,
+		universeTap,
+	)
+}
+
+// assertDefaultCoOpCloseBalance returns a default implementation of the co-op
+// close balance check that can be used in tests. It assumes the initiator has
+// both an asset and BTC balance left, while the responder's balance can be
+// specified with the boolean variables.
+func assertDefaultCoOpCloseBalance(remoteBtcBalance,
+	remoteAssetBalance bool) coOpCloseBalanceCheck {
+
+	return func(t *testing.T, local, remote *HarnessNode,
+		closeTx *wire.MsgTx, closeUpdate *lnrpc.ChannelCloseUpdate,
+		assetID, groupKey []byte, universeTap *tapClient) {
+
+		defaultCoOpCloseBalanceCheck(
+			t, local, remote, closeTx, closeUpdate, assetID,
+			groupKey, universeTap, remoteBtcBalance,
+			remoteAssetBalance,
+		)
+	}
+}
+
+// defaultCoOpCloseBalanceCheck is a default implementation of the co-op close
+// balance check that can be used in tests. It assumes the initiator has both
+// an asset and BTC balance left, while the responder's balance can be specified
+// with the boolean variables.
+func defaultCoOpCloseBalanceCheck(t *testing.T, local, remote *HarnessNode,
+	closeTx *wire.MsgTx, closeUpdate *lnrpc.ChannelCloseUpdate,
+	assetID, groupKey []byte, universeTap *tapClient, remoteBtcBalance,
+	remoteAssetBalance bool) {
+
 	// With the channel closed, we'll now assert that the co-op close
 	// transaction was inserted into the local universe.
 	//
@@ -972,49 +1017,50 @@ func closeAssetChannelAndAssert(t *harnessTest, net *NetworkHarness,
 		additionalOutputs++
 	}
 
-	require.Len(t.t, closeTx.TxOut, numOutputs)
+	closeTxid := closeTx.TxHash()
+	require.Len(t, closeTx.TxOut, numOutputs)
 
 	outIdx := 0
 	dummyAmt := int64(1000)
-	require.LessOrEqual(t.t, closeTx.TxOut[outIdx].Value, dummyAmt)
+	require.LessOrEqual(t, closeTx.TxOut[outIdx].Value, dummyAmt)
 
 	if remoteAssetBalance {
 		outIdx++
-		require.LessOrEqual(t.t, closeTx.TxOut[outIdx].Value, dummyAmt)
+		require.LessOrEqual(t, closeTx.TxOut[outIdx].Value, dummyAmt)
 	}
 
 	// We also require there to be at most two additional outputs, one for
 	// each of the asset outputs with balance.
-	require.Len(t.t, closeUpdate.AdditionalOutputs, additionalOutputs)
+	require.Len(t, closeUpdate.AdditionalOutputs, additionalOutputs)
 
 	var remoteCloseOut *lnrpc.CloseOutput
 	if remoteBtcBalance {
 		// The remote node has received a couple of HTLCs with an above
 		// dust value, so it should also have accumulated a non-dust
 		// balance, even after subtracting 1k sats for the asset output.
 		remoteCloseOut = closeUpdate.RemoteCloseOutput
-		require.NotNil(t.t, remoteCloseOut)
+		require.NotNil(t, remoteCloseOut)
 
 		outIdx++
 		require.EqualValues(
-			t.t, remoteCloseOut.AmountSat-dummyAmt,
+			t, remoteCloseOut.AmountSat-dummyAmt,
 			closeTx.TxOut[outIdx].Value,
 		)
 	} else if remoteAssetBalance {
 		// The remote node has received a couple of HTLCs but not enough
 		// to go above dust. So it should still have an asset balance
 		// that we can verify.
 		remoteCloseOut = closeUpdate.RemoteCloseOutput
-		require.NotNil(t.t, remoteCloseOut)
+		require.NotNil(t, remoteCloseOut)
 	}
 
 	// The local node should have received the local BTC balance minus the
 	// TX fees and 1k sats for the asset output.
 	localCloseOut := closeUpdate.LocalCloseOutput
-	require.NotNil(t.t, localCloseOut)
+	require.NotNil(t, localCloseOut)
 	outIdx++
 	require.Greater(
-		t.t, closeTx.TxOut[outIdx].Value,
+		t, closeTx.TxOut[outIdx].Value,
 		localCloseOut.AmountSat-dummyAmt,
 	)
 
@@ -1039,39 +1085,41 @@ func closeAssetChannelAndAssert(t *harnessTest, net *NetworkHarness,
 
 	if remoteAuxOut != nil {
 		require.Equal(
-			t.t, remoteAuxOut.PkScript,
+			t, remoteAuxOut.PkScript,
 			closeTx.TxOut[remoteAssetIndex].PkScript,
 		)
 	}
 
 	require.Equal(
-		t.t, localAuxOut.PkScript,
+		t, localAuxOut.PkScript,
 		closeTx.TxOut[localAssetIndex].PkScript,
 	)
 
 	// We now verify the arrival of the local balance asset proof at the
 	// universe server.
 	var localAssetCloseOut rfqmsg.JsonCloseOutput
-	err = json.Unmarshal(
+	err := json.Unmarshal(
 		localCloseOut.CustomChannelData, &localAssetCloseOut,
 	)
-	require.NoError(t.t, err)
+	require.NoError(t, err)
 
 	for assetIDStr, scriptKeyStr := range localAssetCloseOut.ScriptKeys {
 		scriptKeyBytes, err := hex.DecodeString(scriptKeyStr)
-		require.NoError(t.t, err)
+		require.NoError(t, err)
 
-		require.Equal(t.t, hex.EncodeToString(assetID), assetIDStr)
+		require.Equal(t, hex.EncodeToString(assetID), assetIDStr)
 
 		a := assertUniverseProofExists(
-			t.t, universeTap, assetID, groupKey, scriptKeyBytes,
+			t, universeTap, assetID, groupKey, scriptKeyBytes,
 			fmt.Sprintf("%v:%v", closeTxid, localAssetIndex),
 		)
 
+		localTapd := newTapClient(t, local)
+
 		scriptKey, err := btcec.ParsePubKey(scriptKeyBytes)
-		require.NoError(t.t, err)
+		require.NoError(t, err)
 		assertAssetExists(
-			t.t, localTapd, assetID, a.Amount, scriptKey, true,
+			t, localTapd, assetID, a.Amount, scriptKey, true,
 			true, false,
 		)
 	}
@@ -1083,38 +1131,145 @@ func closeAssetChannelAndAssert(t *harnessTest, net *NetworkHarness,
 
 	// At this point the remote close output should be defined, otherwise
 	// something went wrong.
-	require.NotNil(t.t, remoteCloseOut)
+	require.NotNil(t, remoteCloseOut)
 
 	// And then we verify the arrival of the remote balance asset proof at
 	// the universe server as well.
 	var remoteAssetCloseOut rfqmsg.JsonCloseOutput
 	err = json.Unmarshal(
 		remoteCloseOut.CustomChannelData, &remoteAssetCloseOut,
 	)
-	require.NoError(t.t, err)
+	require.NoError(t, err)
 
 	for assetIDStr, scriptKeyStr := range remoteAssetCloseOut.ScriptKeys {
 		scriptKeyBytes, err := hex.DecodeString(scriptKeyStr)
-		require.NoError(t.t, err)
+		require.NoError(t, err)
 
-		require.Equal(t.t, hex.EncodeToString(assetID), assetIDStr)
+		require.Equal(t, hex.EncodeToString(assetID), assetIDStr)
 
 		a := assertUniverseProofExists(
-			t.t, universeTap, assetID, groupKey, scriptKeyBytes,
+			t, universeTap, assetID, groupKey, scriptKeyBytes,
 			fmt.Sprintf("%v:%v", closeTxid, remoteAssetIndex),
 		)
 
-		remoteTapd := newTapClient(t.t, remote)
+		remoteTapd := newTapClient(t, remote)
+
+		scriptKey, err := btcec.ParsePubKey(scriptKeyBytes)
+		require.NoError(t, err)
+		assertAssetExists(
+			t, remoteTapd, assetID, a.Amount, scriptKey, true,
+			true, false,
+		)
+	}
+}
+
+// initiatorZeroAssetBalanceCoOpBalanceCheck is a co-op close balance check
+// function that can be used when the initiator has a zero asset balance.
+func initiatorZeroAssetBalanceCoOpBalanceCheck(t *testing.T, _,
+	remote *HarnessNode, closeTx *wire.MsgTx,
+	closeUpdate *lnrpc.ChannelCloseUpdate, assetID, groupKey []byte,
+	universeTap *tapClient) {
+
+	// With the channel closed, we'll now assert that the co-op close
+	// transaction was inserted into the local universe.
+	//
+	// Since the initiator has a zero asset balance, we expect that at most
+	// three outputs exist: one for the remote asset output, one for the
+	// remote BTC channel balance and one for the initiator's BTC channel
+	// balance (which cannot be zero or below dust due to the mandatory
+	// channel reserve).
+	numOutputs := 3
+
+	closeTxid := closeTx.TxHash()
+	require.Len(t, closeTx.TxOut, numOutputs)
+
+	// We assume that the local node has a non-zero BTC balance left.
+	localOut, _ := closeTxOut(t, closeTx, closeUpdate, true)
+	require.Greater(t, localOut.Value, int64(1000))
+
+	// We also require there to be exactly one additional output, which is
+	// the remote asset output.
+	require.Len(t, closeUpdate.AdditionalOutputs, 1)
+	assetTxOut, assetOutputIndex := findTxOut(
+		t, closeTx, closeUpdate.AdditionalOutputs[0].PkScript,
+	)
+	require.LessOrEqual(t, assetTxOut.Value, int64(1000))
+
+	// The remote node has received a couple of HTLCs with an above
+	// dust value, so it should also have accumulated a non-dust
+	// balance, even after subtracting 1k sats for the asset output.
+	remoteCloseOut := closeUpdate.RemoteCloseOutput
+	require.NotNil(t, remoteCloseOut)
+
+	// Find out which of the additional outputs is the local one and which
+	// is the remote.
+	remoteAuxOut := closeUpdate.AdditionalOutputs[0]
+	require.False(t, remoteAuxOut.IsLocal)
+
+	// And then we verify the arrival of the remote balance asset proof at
+	// the universe server as well.
+	var remoteAssetCloseOut rfqmsg.JsonCloseOutput
+	err := json.Unmarshal(
+		remoteCloseOut.CustomChannelData, &remoteAssetCloseOut,
+	)
+	require.NoError(t, err)
+
+	for assetIDStr, scriptKeyStr := range remoteAssetCloseOut.ScriptKeys {
+		scriptKeyBytes, err := hex.DecodeString(scriptKeyStr)
+		require.NoError(t, err)
+
+		require.Equal(t, hex.EncodeToString(assetID), assetIDStr)
+
+		a := assertUniverseProofExists(
+			t, universeTap, assetID, groupKey, scriptKeyBytes,
+			fmt.Sprintf("%v:%v", closeTxid, assetOutputIndex),
+		)
+
+		remoteTapd := newTapClient(t, remote)
 
 		scriptKey, err := btcec.ParsePubKey(scriptKeyBytes)
-		require.NoError(t.t, err)
+		require.NoError(t, err)
 		assertAssetExists(
-			t.t, remoteTapd, assetID, a.Amount, scriptKey, true,
+			t, remoteTapd, assetID, a.Amount, scriptKey, true,
 			true, false,
 		)
 	}
 }
 
+// closeTxOut returns either the local or remote output from the close
+// transaction, based on the information given in the close update.
+func closeTxOut(t *testing.T, closeTx *wire.MsgTx,
+	closeUpdate *lnrpc.ChannelCloseUpdate, local bool) (*wire.TxOut, int) {
+
+	var targetPkScript []byte
+	if local {
+		require.NotNil(t, closeUpdate.LocalCloseOutput)
+		targetPkScript = closeUpdate.LocalCloseOutput.PkScript
+	} else {
+		require.NotNil(t, closeUpdate.RemoteCloseOutput)
+		targetPkScript = closeUpdate.RemoteCloseOutput.PkScript
+	}
+
+	return findTxOut(t, closeTx, targetPkScript)
+}
+
+// findTxOut returns the transaction output with the target pk script from the
+// given transaction.
+func findTxOut(t *testing.T, tx *wire.MsgTx, targetPkScript []byte) (
+	*wire.TxOut, int) {
+
+	for i, txOut := range tx.TxOut {
+		if bytes.Equal(txOut.PkScript, targetPkScript) {
+			return txOut, i
+		}
+	}
+
+	t.Fatalf("close output (targetPkScript=%x) not found in close "+
+		"transaction", targetPkScript)
+
+	return &wire.TxOut{}, 0
+}
+
 type tapClient struct {
 	node *HarnessNode
 	lnd  *rpc.HarnessRPC