Merge pull request #8800 from ProofOfKeags/bugfix/8535

contractcourt: consider delivery addresses when evaluating toSelfAmount

Author: yyforyongyu

contractcourt/chain_watcher.go

@@ -3,6 +3,7 @@ package contractcourt
 import (
 	"bytes"
 	"fmt"
+	"slices"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -16,8 +17,10 @@ import (
 	"github.com/davecgh/go-spew/spew"
 	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/fn"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnwallet"
+	"github.com/lightningnetwork/lnd/lnwire"
 )
 
 const (
@@ -970,28 +973,67 @@ func (c *chainWatcher) handleUnknownRemoteState(
 }
 
 // toSelfAmount takes a transaction and returns the sum of all outputs that pay
-// to a script that the wallet controls. If no outputs pay to us, then we
+// to a script that the wallet controls or the channel defines as its delivery
+// script . If no outputs pay to us (determined by these criteria), then we
 // return zero. This is possible as our output may have been trimmed due to
 // being dust.
 func (c *chainWatcher) toSelfAmount(tx *wire.MsgTx) btcutil.Amount {
-	var selfAmt btcutil.Amount
-	for _, txOut := range tx.TxOut {
+	// There are two main cases we have to handle here. First, in the coop
+	// close case we will always have saved the delivery address we used
+	// whether it was from the upfront shutdown, from the delivery address
+	// requested at close time, or even an automatically generated one. All
+	// coop-close cases can be identified in the following manner:
+	shutdown, _ := c.cfg.chanState.ShutdownInfo()
+	oDeliveryAddr := fn.MapOption(
+		func(i channeldb.ShutdownInfo) lnwire.DeliveryAddress {
+			return i.DeliveryScript.Val
+		})(shutdown)
+
+	// Here we define a function capable of identifying whether an output
+	// corresponds with our local delivery script from a ShutdownInfo if we
+	// have a ShutdownInfo for this chainWatcher's underlying channel.
+	//
+	// isDeliveryOutput :: *TxOut -> bool
+	isDeliveryOutput := func(o *wire.TxOut) bool {
+		return fn.ElimOption(
+			oDeliveryAddr,
+			// If we don't have a delivery addr, then the output
+			// can't match it.
+			func() bool { return false },
+			// Otherwise if the PkScript of the TxOut matches our
+			// delivery script then this is a delivery output.
+			func(a lnwire.DeliveryAddress) bool {
+				return slices.Equal(a, o.PkScript)
+			},
+		)
+	}
+
+	// Here we define a function capable of identifying whether an output
+	// belongs to the LND wallet. We use this as a heuristic in the case
+	// where we might be looking for spendable force closure outputs.
+	//
+	// isWalletOutput :: *TxOut -> bool
+	isWalletOutput := func(out *wire.TxOut) bool {
 		_, addrs, _, err := txscript.ExtractPkScriptAddrs(
 			// Doesn't matter what net we actually pass in.
-			txOut.PkScript, &chaincfg.TestNet3Params,
+			out.PkScript, &chaincfg.TestNet3Params,
 		)
 		if err != nil {
-			continue
+			return false
 		}
 
-		for _, addr := range addrs {
-			if c.cfg.isOurAddr(addr) {
-				selfAmt += btcutil.Amount(txOut.Value)
-			}
-		}
+		return fn.Any(c.cfg.isOurAddr, addrs)
 	}
 
-	return selfAmt
+	// Grab all of the outputs that correspond with our delivery address
+	// or our wallet is aware of.
+	outs := fn.Filter(fn.PredOr(isDeliveryOutput, isWalletOutput), tx.TxOut)
+
+	// Grab the values for those outputs.
+	vals := fn.Map(func(o *wire.TxOut) int64 { return o.Value }, outs)
+
+	// Return the sum.
+	return btcutil.Amount(fn.Sum(vals))
 }
 
 // dispatchCooperativeClose processed a detect cooperative channel closure.

docs/release-notes/release-notes-0.18.1.md

@@ -19,6 +19,10 @@
 
 # Bug Fixes
 
+* `closedchannels` now [successfully reports](https://github.com/lightningnetwork/lnd/pull/8800)
+  settled balances even if the delivery address is set to an address that
+  LND does not control.
+
 # New Features
 ## Functional Enhancements
 ## RPC Additions

go.mod

@@ -35,7 +35,7 @@ require (
 	github.com/lightningnetwork/lightning-onion v1.2.1-0.20230823005744-06182b1d7d2f
 	github.com/lightningnetwork/lnd/cert v1.2.2
 	github.com/lightningnetwork/lnd/clock v1.1.1
-	github.com/lightningnetwork/lnd/fn v1.0.5
+	github.com/lightningnetwork/lnd/fn v1.0.9
 	github.com/lightningnetwork/lnd/healthcheck v1.2.4
 	github.com/lightningnetwork/lnd/kvdb v1.4.8
 	github.com/lightningnetwork/lnd/queue v1.1.1

go.sum

@@ -448,8 +448,8 @@ github.com/lightningnetwork/lnd/cert v1.2.2 h1:71YK6hogeJtxSxw2teq3eGeuy4rHGKcFf
 github.com/lightningnetwork/lnd/cert v1.2.2/go.mod h1:jQmFn/Ez4zhDgq2hnYSw8r35bqGVxViXhX6Cd7HXM6U=
 github.com/lightningnetwork/lnd/clock v1.1.1 h1:OfR3/zcJd2RhH0RU+zX/77c0ZiOnIMsDIBjgjWdZgA0=
 github.com/lightningnetwork/lnd/clock v1.1.1/go.mod h1:mGnAhPyjYZQJmebS7aevElXKTFDuO+uNFFfMXK1W8xQ=
-github.com/lightningnetwork/lnd/fn v1.0.5 h1:ffDgMSn83avw6rNzxhbt6w5/2oIrwQKTPGfyaLupZtE=
-github.com/lightningnetwork/lnd/fn v1.0.5/go.mod h1:P027+0CyELd92H9gnReUkGGAqbFA1HwjHWdfaDFD51U=
+github.com/lightningnetwork/lnd/fn v1.0.9 h1:VPljrzHGh0Wfs2NZe/ugUfH0hl6/L2eXW0LLXMUEy3s=
+github.com/lightningnetwork/lnd/fn v1.0.9/go.mod h1:P027+0CyELd92H9gnReUkGGAqbFA1HwjHWdfaDFD51U=
 github.com/lightningnetwork/lnd/healthcheck v1.2.4 h1:lLPLac+p/TllByxGSlkCwkJlkddqMP5UCoawCj3mgFQ=
 github.com/lightningnetwork/lnd/healthcheck v1.2.4/go.mod h1:G7Tst2tVvWo7cx6mSBEToQC5L1XOGxzZTPB29g9Rv2I=
 github.com/lightningnetwork/lnd/kvdb v1.4.8 h1:xH0a5Vi1yrcZ5BEeF2ba3vlKBRxrL9uYXlWTjOjbNTY=

itest/list_on_test.go

@@ -626,4 +626,8 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "sweep commit output and anchor",
 		TestFunc: testSweepCommitOutputAndAnchor,
 	},
+	{
+		Name:     "coop close with external delivery",
+		TestFunc: testCoopCloseWithExternalDelivery,
+	},
 }

itest/lnd_coop_close_external_delivery_test.go

@@ -0,0 +1,92 @@
+package itest
+
+import (
+	"testing"
+
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/lightningnetwork/lnd/lnrpc"
+	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/stretchr/testify/require"
+)
+
+func testCoopCloseWithExternalDelivery(ht *lntest.HarnessTest) {
+	ht.Run("set delivery address at open", func(t *testing.T) {
+		tt := ht.Subtest(t)
+		testCoopCloseWithExternalDeliveryImpl(tt, true)
+	})
+	ht.Run("set delivery address at close", func(t *testing.T) {
+		tt := ht.Subtest(t)
+		testCoopCloseWithExternalDeliveryImpl(tt, false)
+	})
+}
+
+// testCoopCloseWithExternalDeliveryImpl ensures that we have a valid settled
+// balance irrespective of whether the delivery address is in LND's wallet or
+// not. Some users set this value to be an address in a different wallet and
+// this should not affect our ability to accurately report the settled balance.
+func testCoopCloseWithExternalDeliveryImpl(ht *lntest.HarnessTest,
+	upfrontShutdown bool) {
+
+	alice, bob := ht.Alice, ht.Bob
+	ht.ConnectNodes(alice, bob)
+
+	// Here we generate a final delivery address in bob's wallet but set by
+	// alice. We do this to ensure that the address is not in alice's LND
+	// wallet. We already correctly track settled balances when the address
+	// is in the LND wallet.
+	addr := bob.RPC.NewAddress(&lnrpc.NewAddressRequest{
+		Type: lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH,
+	})
+
+	// Prepare for channel open.
+	openParams := lntest.OpenChannelParams{
+		Amt: btcutil.Amount(1000000),
+	}
+
+	// If we are testing the case where we set it on open then we'll set the
+	// upfront shutdown script in the channel open parameters.
+	if upfrontShutdown {
+		openParams.CloseAddress = addr.Address
+	}
+
+	// Open the channel!
+	chanPoint := ht.OpenChannel(alice, bob, openParams)
+
+	// Prepare for channel close.
+	closeParams := lnrpc.CloseChannelRequest{
+		ChannelPoint: chanPoint,
+		TargetConf:   6,
+	}
+
+	// If we are testing the case where we set the delivery address on
+	// channel close then we will set it in the channel close parameters.
+	if !upfrontShutdown {
+		closeParams.DeliveryAddress = addr.Address
+	}
+
+	// Close the channel!
+	closeClient := alice.RPC.CloseChannel(&closeParams)
+
+	// Assert that we got a channel update when we get a closing txid.
+	_, err := closeClient.Recv()
+	require.NoError(ht, err)
+
+	// Mine the closing transaction.
+	ht.MineClosingTx(chanPoint)
+
+	// Assert that we got a channel update when the closing tx was mined.
+	_, err = closeClient.Recv()
+	require.NoError(ht, err)
+
+	// Here we query our closed channels to conduct the final test
+	// assertion. We want to ensure that even though alice's delivery
+	// address is set to an address in bob's wallet, we should still show
+	// the balance as settled.
+	closed := alice.RPC.ClosedChannels(&lnrpc.ClosedChannelsRequest{
+		Cooperative: true,
+	})
+
+	// The settled balance should never be zero at this point.
+	require.NotZero(ht, len(closed.Channels))
+	require.NotZero(ht, closed.Channels[0].SettledBalance)
+}

lntest/harness.go

@@ -1007,6 +1007,10 @@ type OpenChannelParams struct {
 	// FundMax flag is specified the entirety of selected funds is
 	// allocated towards channel funding.
 	Outpoints []*lnrpc.OutPoint
+
+	// CloseAddress sets the upfront_shutdown_script parameter during
+	// channel open. It is expected to be encoded as a bitcoin address.
+	CloseAddress string
 }
 
 // prepareOpenChannel waits for both nodes to be synced to chain and returns an
@@ -1059,6 +1063,7 @@ func (h *HarnessTest) prepareOpenChannel(srcNode, destNode *node.HarnessNode,
 		FundMax:            p.FundMax,
 		Memo:               p.Memo,
 		Outpoints:          p.Outpoints,
+		CloseAddress:       p.CloseAddress,
 	}
 }