From 56048133f2f2e7f8d7e061a348b107c9a72c5418 Mon Sep 17 00:00:00 2001
From: Keagan McClelland <keagan.mcclelland@gmail.com>
Date: Thu, 30 May 2024 17:44:30 -0700
Subject: [PATCH] itest+lntest: add itest to reproduce bug #8535

---
 itest/list_on_test.go                         |  4 +
 .../lnd_coop_close_external_delivery_test.go  | 92 +++++++++++++++++++
 lntest/harness.go                             |  5 +
 3 files changed, 101 insertions(+)
 create mode 100644 itest/lnd_coop_close_external_delivery_test.go

diff --git a/itest/list_on_test.go b/itest/list_on_test.go
index 1c563aa20..3619c2282 100644
--- a/itest/list_on_test.go
+++ b/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,
+	},
 }
diff --git a/itest/lnd_coop_close_external_delivery_test.go b/itest/lnd_coop_close_external_delivery_test.go
new file mode 100644
index 000000000..755792303
--- /dev/null
+++ b/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)
+}
diff --git a/lntest/harness.go b/lntest/harness.go
index 4c26e1aa7..92549427f 100644
--- a/lntest/harness.go
+++ b/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,
 	}
 }