From fa8527af09fccff0d202662056bbb3494b4af1f8 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Mon, 24 Feb 2025 18:47:44 +0800
Subject: [PATCH 01/10] Makefile+scripts: add unit test flake hunter

This commit adds a script to hunt flakes for a specific unit test with
trace logs. Also rename the make commands to make them more clear on
whether it's a unit test, itest, or paralleled itest.
---
 Makefile                          | 16 ++++++++--------
 scripts/unit-test-flake-hunter.sh | 23 +++++++++++++++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)
 create mode 100755 scripts/unit-test-flake-hunter.sh

diff --git a/Makefile b/Makefile
index ebd2b69ec..fc7a04118 100644
--- a/Makefile
+++ b/Makefile
@@ -276,18 +276,18 @@ unit-bench: $(BTCD_BIN)
 # FLAKE HUNTING
 # =============
 
-#? flakehunter: Run the integration tests continuously until one fails
-flakehunter: build-itest
+#? flakehunter-itest: Run the integration tests continuously until one fails
+flakehunter-itest: build-itest
 	@$(call print, "Flake hunting ${backend} integration tests.")
 	while [ $$? -eq 0 ]; do make itest-only icase='${icase}' backend='${backend}'; done
 
-#? flake-unit: Run the unit tests continuously until one fails
-flake-unit:
-	@$(call print, "Flake hunting unit tests.")
-	while [ $$? -eq 0 ]; do GOTRACEBACK=all $(UNIT) -count=1; done
+#? flakehunter-unit: Run the unit tests continuously until one fails
+flakehunter-unit:
+	@$(call print, "Flake hunting unit test.")
+	scripts/unit-test-flake-hunter.sh ${pkg} ${case}
 
-#? flakehunter-parallel: Run the integration tests continuously until one fails, running up to ITEST_PARALLELISM test tranches in parallel (default 4)
-flakehunter-parallel:
+#? flakehunter-itest-parallel: Run the integration tests continuously until one fails, running up to ITEST_PARALLELISM test tranches in parallel (default 4)
+flakehunter-itest-parallel:
 	@$(call print, "Flake hunting ${backend} integration tests in parallel.")
 	while [ $$? -eq 0 ]; do make itest-parallel tranches=1 parallel=${ITEST_PARALLELISM} icase='${icase}' backend='${backend}'; done
 
diff --git a/scripts/unit-test-flake-hunter.sh b/scripts/unit-test-flake-hunter.sh
new file mode 100755
index 000000000..89406e01a
--- /dev/null
+++ b/scripts/unit-test-flake-hunter.sh
@@ -0,0 +1,23 @@
+#!/bin/bash
+
+# Check if pkg and case variables are provided.
+if [ $# -lt 2 ] || [ $# -gt 3 ]; then
+	echo "Usage: $0 <pkg> <case> [timeout]"
+	exit 1
+fi
+
+pkg=$1
+case=$2
+timeout=${3:-30s} # Default to 30s if not provided.
+
+counter=0
+
+# Run the command in a loop until it fails.
+while output=$(go clean -testcache && make unit-debug log="stdlog trace" pkg=$pkg case=$case timeout=$timeout 2>&1); do
+	((counter++))
+	echo "Test $case passed, count: $counter"
+done
+
+# Only log the output when it fails.
+echo "Test $case failed. Output:"
+echo "$output"

From 7666d62a43e1e9b3ddff7c0b26494207fbee18da Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 16:38:06 +0800
Subject: [PATCH 02/10] lntest/unittest: update config for miner and bitcoind

The config used for the miner is updated to skip banning and debug. For
bitcoind, the config is updated to use a unique port for P2P conn.
---
 lntest/unittest/backend.go | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go
index ac700044d..d14f88241 100644
--- a/lntest/unittest/backend.go
+++ b/lntest/unittest/backend.go
@@ -37,9 +37,15 @@ func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string,
 
 	t.Helper()
 
-	// Add the trickle interval argument to the extra args.
-	trickle := fmt.Sprintf("--trickleinterval=%v", TrickleInterval)
-	extraArgs = append(extraArgs, trickle)
+	args := []string{
+		"--nobanning",
+		"--debuglevel=debug",
+		fmt.Sprintf("--trickleinterval=%v", TrickleInterval),
+
+		// Don't disconnect if a reply takes too long.
+		"--nostalldetect",
+	}
+	extraArgs = append(extraArgs, args...)
 
 	node, err := rpctest.New(netParams, nil, extraArgs, "")
 	require.NoError(t, err, "unable to create backend node")
@@ -88,6 +94,11 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	zmqBlockHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqBlockPort)
 	zmqTxHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqTxPort)
 
+	// TODO(yy): Make this configurable via `chain.BitcoindConfig` and
+	// replace the default P2P port when set.
+	p2pPort := port.NextAvailablePort()
+	netParams.DefaultPort = fmt.Sprintf("%d", p2pPort)
+
 	args := []string{
 		"-connect=" + minerAddr,
 		"-datadir=" + tempBitcoindDir,
@@ -96,6 +107,7 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 			"284294edb5773b05544b220110063096c221be9933c82d38e1",
 		fmt.Sprintf("-rpcport=%d", rpcPort),
 		fmt.Sprintf("-bind=127.0.0.1:%d=onion", torBindPort),
+		fmt.Sprintf("-port=%d", p2pPort),
 		"-disablewallet",
 		"-zmqpubrawblock=" + zmqBlockHost,
 		"-zmqpubrawtx=" + zmqTxHost,

From 245ea85894c23329b023bfa0892e18aa74be44f1 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 16:44:18 +0800
Subject: [PATCH 03/10] lntest/unittest: assert `bitcoind` is shut down

Make sure the shutdown of `bitcoind` is finished without any errors.
---
 lntest/unittest/backend.go | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go
index d14f88241..39c4d9d29 100644
--- a/lntest/unittest/backend.go
+++ b/lntest/unittest/backend.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -117,12 +118,20 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	}
 
 	bitcoind := exec.Command("bitcoind", args...)
-	if err := bitcoind.Start(); err != nil {
-		t.Fatalf("unable to start bitcoind: %v", err)
-	}
+	err := bitcoind.Start()
+	require.NoError(t, err, "unable to start bitcoind")
+
 	t.Cleanup(func() {
-		_ = bitcoind.Process.Kill()
-		_ = bitcoind.Wait()
+		// Kill `bitcoind` and assert there's no error.
+		err = bitcoind.Process.Kill()
+		require.NoError(t, err)
+
+		err = bitcoind.Wait()
+		if strings.Contains(err.Error(), "signal: killed") {
+			return
+		}
+
+		require.NoError(t, err)
 	})
 
 	// Wait for the bitcoind instance to start up.
@@ -154,7 +163,7 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	}
 
 	var conn *chain.BitcoindConn
-	err := wait.NoError(func() error {
+	err = wait.NoError(func() error {
 		var err error
 		conn, err = chain.NewBitcoindConn(cfg)
 		if err != nil {

From 175301628f042439c204ca6eeb0ac86c393e7635 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 16:45:36 +0800
Subject: [PATCH 04/10] lntest/unittest: make sure miner is connected to
 `bitcoind`

We change how the `bitcoind` node connects to the miner by creating a
temp RPC client and use it to connect to the miner. In addition we also
assert the connection is made.
---
 lntest/unittest/backend.go | 44 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go
index 39c4d9d29..f759a3e1a 100644
--- a/lntest/unittest/backend.go
+++ b/lntest/unittest/backend.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/integration/rpctest"
+	"github.com/btcsuite/btcd/rpcclient"
 	"github.com/btcsuite/btcwallet/chain"
 	"github.com/btcsuite/btcwallet/walletdb"
 	"github.com/lightninglabs/neutrino"
@@ -101,7 +102,6 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	netParams.DefaultPort = fmt.Sprintf("%d", p2pPort)
 
 	args := []string{
-		"-connect=" + minerAddr,
 		"-datadir=" + tempBitcoindDir,
 		"-regtest",
 		"-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6fd$507c670e800a95" +
@@ -178,6 +178,48 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	}
 	t.Cleanup(conn.Stop)
 
+	// Assert that the connection with the miner is made.
+	//
+	// Create a new RPC client.
+	rpcCfg := rpcclient.ConnConfig{
+		Host:                 cfg.Host,
+		User:                 cfg.User,
+		Pass:                 cfg.Pass,
+		DisableConnectOnNew:  true,
+		DisableAutoReconnect: false,
+		DisableTLS:           true,
+		HTTPPostMode:         true,
+	}
+
+	rpcClient, err := rpcclient.New(&rpcCfg, nil)
+	require.NoError(t, err, "failed to create RPC client")
+
+	// Connect to the miner node.
+	err = rpcClient.AddNode(minerAddr, rpcclient.ANAdd)
+	require.NoError(t, err, "failed to connect to miner")
+
+	// Get the network info and assert the num of outbound connections is 1.
+	err = wait.NoError(func() error {
+		result, err := rpcClient.GetNetworkInfo()
+		require.NoError(t, err)
+
+		if int(result.Connections) != 1 {
+			return fmt.Errorf("want 1 conn, got %d",
+				result.Connections)
+		}
+
+		if int(result.ConnectionsOut) != 1 {
+			return fmt.Errorf("want 1 outbound conn, got %d",
+				result.Connections)
+		}
+
+		return nil
+	}, wait.DefaultTimeout)
+	require.NoError(t, err, "timeout connecting to the miner")
+
+	// Tear down the rpc client.
+	rpcClient.Shutdown()
+
 	return conn
 }
 

From 1618d2c789726b6a391fc111f9752292269f2876 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 16:48:25 +0800
Subject: [PATCH 05/10] chainntnfs+lntest: fix sync to miner block flake

In this commit we document an unexpected behavior found when connecting
a bitcoind node to a btcd node. We mitigate this in our test by
reconnecting the nodes when the connection is broken. We also limit the
connection made from `bitcoind` to be v1 only.
---
 chainntnfs/bitcoindnotify/bitcoind_test.go | 53 +++++++++++++++++++++-
 lntest/unittest/backend.go                 |  9 ++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go
index cf96a7cce..696fe06c2 100644
--- a/chainntnfs/bitcoindnotify/bitcoind_test.go
+++ b/chainntnfs/bitcoindnotify/bitcoind_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/integration/rpctest"
+	"github.com/btcsuite/btcd/rpcclient"
 	"github.com/btcsuite/btcwallet/chain"
 	"github.com/lightningnetwork/lnd/blockcache"
 	"github.com/lightningnetwork/lnd/chainntnfs"
@@ -103,6 +104,54 @@ func syncNotifierWithMiner(t *testing.T, notifier *BitcoindNotifier,
 				"err=%v, minerHeight=%v, bitcoindHeight=%v",
 				err, minerHeight, bitcoindHeight)
 		}
+
+		// Get the num of connections the miner has. We expect it to
+		// have at least one connection with the chain backend.
+		count, err := miner.Client.GetConnectionCount()
+		require.NoError(t, err)
+		if count != 0 {
+			continue
+		}
+
+		// Reconnect the miner and the chain backend.
+		//
+		// NOTE: The connection should have been made before we perform
+		// the `syncNotifierWithMiner`. However, due to an unknown
+		// reason, the miner may refuse to process the inbound
+		// connection made by the bitcoind node, causing the connection
+		// to fail. It's possible there's a bug in the handshake between
+		// the two nodes.
+		//
+		// A normal flow is, bitcoind starts a v2 handshake flow, which
+		// btcd will fail and disconnect. Upon seeing this
+		// disconnection, bitcoind will try a v1 handshake and succeeds.
+		// The failed flow is, upon seeing the v2 handshake, btcd
+		// doesn't seem to perform the disconnect. Instead an EOF
+		// websocket error is found.
+		//
+		// TODO(yy): Fix the above bug in `btcd`. This can be reproduced
+		// using `make flakehunter-unit pkg=$pkg case=$case`, with,
+		// `case=TestHistoricalConfDetailsNoTxIndex/rpc_polling_enabled`
+		// `pkg=chainntnfs/bitcoindnotify`.
+		// Also need to modify the temp dir logic so we can save the
+		// debug logs.
+		// This bug is likely to be fixed when we implement the
+		// encrypted p2p conn, or when we properly fix the shutdown
+		// issues in all our RPC conns.
+		t.Log("Expected to the chain backend to have one conn with " +
+			"the miner, instead it's disconnected!")
+
+		// We now ask the miner to add the chain backend back.
+		host := fmt.Sprintf(
+			"127.0.0.1:%s", notifier.chainParams.DefaultPort,
+		)
+
+		// NOTE:AddNode must take a host that has the format
+		// `host:port`, otherwise the default port will be used. Check
+		// `normalizeAddress` in btcd for details.
+		err = miner.Client.AddNode(host, rpcclient.ANAdd)
+		require.NoError(t, err, "Failed to connect miner to the chain "+
+			"backend")
 	}
 }
 
@@ -236,7 +285,9 @@ func testHistoricalConfDetailsNoTxIndex(t *testing.T, rpcpolling bool) {
 	hintCache := initHintCache(t)
 	blockCache := blockcache.NewBlockCache(10000)
 
-	notifier := setUpNotifier(t, bitcoindConn, hintCache, hintCache, blockCache)
+	notifier := setUpNotifier(
+		t, bitcoindConn, hintCache, hintCache, blockCache,
+	)
 
 	// Since the node has its txindex disabled, we fall back to scanning the
 	// chain manually. A transaction unknown to the network should not be
diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go
index f759a3e1a..390284e8a 100644
--- a/lntest/unittest/backend.go
+++ b/lntest/unittest/backend.go
@@ -112,6 +112,15 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 		"-disablewallet",
 		"-zmqpubrawblock=" + zmqBlockHost,
 		"-zmqpubrawtx=" + zmqTxHost,
+
+		// whitelist localhost to speed up relay.
+		"-whitelist=127.0.0.1",
+
+		// Disable v2 transport as btcd doesn't support it yet.
+		//
+		// TODO(yy): Remove this line once v2 conn is supported in
+		// `btcd`.
+		"-v2transport=0",
 	}
 	if txindex {
 		args = append(args, "-txindex")

From c725ba9f25540e4159ccbb415ce78ed2b76a7141 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 16:54:56 +0800
Subject: [PATCH 06/10] docs: update release notes re flake fix

---
 docs/release-notes/release-notes-0.19.0.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md
index 3c8e53e76..1e3da2f32 100644
--- a/docs/release-notes/release-notes-0.19.0.md
+++ b/docs/release-notes/release-notes-0.19.0.md
@@ -317,6 +317,9 @@ The underlying functionality between those two options remain the same.
   now documented and
   [fixed](https://github.com/lightningnetwork/lnd/pull/9368).
 
+* [Fixed](https://github.com/lightningnetwork/lnd/pull/9549) a long standing
+  unit test flake found in the `chainntnfs/bitcoindnotify` package.
+
 ## Database
 
 * [Migrate the mission control 

From 99d49dec6a76a9c2d64899b0fa8207711f89bb9f Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 21:33:16 +0800
Subject: [PATCH 07/10] multi: change `NewBitcoindBackend` to take a miner as
 its param

Prepares for the following commit where we assert the chain backend is
synced to the miner.
---
 chainntnfs/bitcoindnotify/bitcoind_test.go |  4 +--
 chainntnfs/test/test_interface.go          |  4 +--
 lntest/unittest/backend.go                 |  4 +--
 lnwallet/test/test_interface.go            |  6 ++---
 routing/chainview/interface_test.go        | 29 +++++++++++-----------
 5 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go
index 696fe06c2..62acffba7 100644
--- a/chainntnfs/bitcoindnotify/bitcoind_test.go
+++ b/chainntnfs/bitcoindnotify/bitcoind_test.go
@@ -179,7 +179,7 @@ func testHistoricalConfDetailsTxIndex(t *testing.T, rpcPolling bool) {
 	)
 
 	bitcoindConn := unittest.NewBitcoindBackend(
-		t, unittest.NetParams, miner.P2PAddress(), true, rpcPolling,
+		t, unittest.NetParams, miner, true, rpcPolling,
 	)
 
 	hintCache := initHintCache(t)
@@ -279,7 +279,7 @@ func testHistoricalConfDetailsNoTxIndex(t *testing.T, rpcpolling bool) {
 	miner := unittest.NewMiner(t, unittest.NetParams, nil, true, 25)
 
 	bitcoindConn := unittest.NewBitcoindBackend(
-		t, unittest.NetParams, miner.P2PAddress(), false, rpcpolling,
+		t, unittest.NetParams, miner, false, rpcpolling,
 	)
 
 	hintCache := initHintCache(t)
diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go
index fd42b10de..7536d24c5 100644
--- a/chainntnfs/test/test_interface.go
+++ b/chainntnfs/test/test_interface.go
@@ -1932,7 +1932,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) {
 		case "bitcoind":
 			var bitcoindConn *chain.BitcoindConn
 			bitcoindConn = unittest.NewBitcoindBackend(
-				t, unittest.NetParams, p2pAddr, true, false,
+				t, unittest.NetParams, miner, true, false,
 			)
 			newNotifier = func() (chainntnfs.TestChainNotifier, error) {
 				return bitcoindnotify.New(
@@ -1944,7 +1944,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) {
 		case "bitcoind-rpc-polling":
 			var bitcoindConn *chain.BitcoindConn
 			bitcoindConn = unittest.NewBitcoindBackend(
-				t, unittest.NetParams, p2pAddr, true, true,
+				t, unittest.NetParams, miner, true, true,
 			)
 			newNotifier = func() (chainntnfs.TestChainNotifier, error) {
 				return bitcoindnotify.New(
diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go
index 390284e8a..30c1a00a4 100644
--- a/lntest/unittest/backend.go
+++ b/lntest/unittest/backend.go
@@ -83,7 +83,7 @@ func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string,
 // used for block and tx notifications or if its ZMQ interface should be used.
 // A connection to the newly spawned bitcoind node is returned.
 func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
-	minerAddr string, txindex, rpcpolling bool) *chain.BitcoindConn {
+	miner *rpctest.Harness, txindex, rpcpolling bool) *chain.BitcoindConn {
 
 	t.Helper()
 
@@ -204,7 +204,7 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	require.NoError(t, err, "failed to create RPC client")
 
 	// Connect to the miner node.
-	err = rpcClient.AddNode(minerAddr, rpcclient.ANAdd)
+	err = rpcClient.AddNode(miner.P2PAddress(), rpcclient.ANAdd)
 	require.NoError(t, err, "failed to connect to miner")
 
 	// Get the network info and assert the num of outbound connections is 1.
diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go
index 27de51708..52ad1dded 100644
--- a/lnwallet/test/test_interface.go
+++ b/lnwallet/test/test_interface.go
@@ -3352,8 +3352,7 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver,
 		case "bitcoind":
 			// Start a bitcoind instance.
 			chainConn := unittest.NewBitcoindBackend(
-				t, unittest.NetParams, miningNode.P2PAddress(),
-				true, false,
+				t, unittest.NetParams, miningNode, true, false,
 			)
 
 			// Create a btcwallet bitcoind client for both Alice and
@@ -3364,8 +3363,7 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver,
 		case "bitcoind-rpc-polling":
 			// Start a bitcoind instance.
 			chainConn := unittest.NewBitcoindBackend(
-				t, unittest.NetParams, miningNode.P2PAddress(),
-				true, true,
+				t, unittest.NetParams, miningNode, true, true,
 			)
 
 			// Create a btcwallet bitcoind client for both Alice and
diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go
index b953389af..c0f336a68 100644
--- a/routing/chainview/interface_test.go
+++ b/routing/chainview/interface_test.go
@@ -460,7 +460,7 @@ func testFilterBlockDisconnected(node *rpctest.Harness,
 
 	// Init a chain view that has this node as its block source.
 	reorgView, err := chainViewInit(
-		t, reorgNode.RPCConfig(), reorgNode.P2PAddress(), bestHeight,
+		t, reorgNode.RPCConfig(), reorgNode, bestHeight,
 	)
 	require.NoError(t, err, "unable to create chain view")
 
@@ -632,7 +632,7 @@ func testFilterBlockDisconnected(node *rpctest.Harness,
 }
 
 type chainViewInitFunc func(t *testing.T, rpcInfo rpcclient.ConnConfig,
-	p2pAddr string, bestHeight int32) (FilteredChainView, error)
+	miner *rpctest.Harness, bestHeight int32) (FilteredChainView, error)
 
 type testCase struct {
 	name string
@@ -666,12 +666,12 @@ var interfaceImpls = []struct {
 	{
 		name: "bitcoind_zmq",
 		chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig,
-			p2pAddr string, bestHeight int32) (FilteredChainView,
-			error) {
+			miner *rpctest.Harness, bestHeight int32) (
+			FilteredChainView, error) {
 
 			// Start a bitcoind instance.
 			chainConn := unittest.NewBitcoindBackend(
-				t, unittest.NetParams, p2pAddr, true,
+				t, unittest.NetParams, miner, true,
 				false,
 			)
 			blockCache := blockcache.NewBlockCache(10000)
@@ -686,12 +686,12 @@ var interfaceImpls = []struct {
 	{
 		name: "bitcoind_polling",
 		chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig,
-			p2pAddr string, bestHeight int32) (FilteredChainView,
-			error) {
+			miner *rpctest.Harness, bestHeight int32) (
+			FilteredChainView, error) {
 
 			// Wait for the bitcoind instance to start up.
 			chainConn := unittest.NewBitcoindBackend(
-				t, unittest.NetParams, p2pAddr, true,
+				t, unittest.NetParams, miner, true,
 				true,
 			)
 			blockCache := blockcache.NewBlockCache(10000)
@@ -706,8 +706,8 @@ var interfaceImpls = []struct {
 	{
 		name: "p2p_neutrino",
 		chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig,
-			p2pAddr string, bestHeight int32) (FilteredChainView,
-			error) {
+			miner *rpctest.Harness, bestHeight int32) (
+			FilteredChainView, error) {
 
 			spvDir := t.TempDir()
 
@@ -723,7 +723,7 @@ var interfaceImpls = []struct {
 				DataDir:      spvDir,
 				Database:     spvDatabase,
 				ChainParams:  *netParams,
-				ConnectPeers: []string{p2pAddr},
+				ConnectPeers: []string{miner.P2PAddress()},
 			}
 
 			spvNode, err := neutrino.NewChainService(spvConfig)
@@ -776,8 +776,8 @@ var interfaceImpls = []struct {
 	{
 		name: "btcd_websockets",
 		chainViewInit: func(_ *testing.T, config rpcclient.ConnConfig,
-			p2pAddr string, bestHeight int32) (FilteredChainView,
-			error) {
+			_ *rpctest.Harness, _ int32) (
+			FilteredChainView, error) {
 
 			blockCache := blockcache.NewBlockCache(10000)
 			chainView, err := NewBtcdFilteredChainView(
@@ -802,7 +802,6 @@ func TestFilteredChainView(t *testing.T) {
 	)
 
 	rpcConfig := miner.RPCConfig()
-	p2pAddr := miner.P2PAddress()
 
 	for _, chainViewImpl := range interfaceImpls {
 		t.Logf("Testing '%v' implementation of FilteredChainView",
@@ -814,7 +813,7 @@ func TestFilteredChainView(t *testing.T) {
 		}
 
 		chainView, err := chainViewImpl.chainViewInit(
-			t, rpcConfig, p2pAddr, bestHeight,
+			t, rpcConfig, miner, bestHeight,
 		)
 		if err != nil {
 			t.Fatalf("unable to make chain view: %v", err)

From 4bfcfea2ee6fc91b39566e8e29655e5d1fe88851 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 21:48:13 +0800
Subject: [PATCH 08/10] lntest+chainntnfs: make sure bitcoind node is synced

This commit makes sure the bitcoind node is synced to the miner when
initialized and returned from `NewBitcoindBackend`.
---
 chainntnfs/bitcoindnotify/bitcoind_test.go |  8 ++-
 lntest/unittest/backend.go                 | 83 +++++++++++++++++++++-
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go
index 62acffba7..0a2d14fbc 100644
--- a/chainntnfs/bitcoindnotify/bitcoind_test.go
+++ b/chainntnfs/bitcoindnotify/bitcoind_test.go
@@ -189,8 +189,6 @@ func testHistoricalConfDetailsTxIndex(t *testing.T, rpcPolling bool) {
 		t, bitcoindConn, hintCache, hintCache, blockCache,
 	)
 
-	syncNotifierWithMiner(t, notifier, miner)
-
 	// A transaction unknown to the node should not be found within the
 	// txindex even if it is enabled, so we should not proceed with any
 	// fallback methods.
@@ -296,7 +294,11 @@ func testHistoricalConfDetailsNoTxIndex(t *testing.T, rpcpolling bool) {
 	copy(unknownHash[:], bytes.Repeat([]byte{0x10}, 32))
 	unknownConfReq, err := chainntnfs.NewConfRequest(&unknownHash, testScript)
 	require.NoError(t, err, "unable to create conf request")
-	broadcastHeight := syncNotifierWithMiner(t, notifier, miner)
+
+	// Get the current best height.
+	_, broadcastHeight, err := miner.Client.GetBestBlock()
+	require.NoError(t, err, "unable to retrieve miner's current height")
+
 	_, txStatus, err := notifier.historicalConfDetails(
 		unknownConfReq, uint32(broadcastHeight), uint32(broadcastHeight),
 	)
diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go
index 30c1a00a4..2bca3f96e 100644
--- a/lntest/unittest/backend.go
+++ b/lntest/unittest/backend.go
@@ -81,7 +81,8 @@ func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string,
 // backend node should maintain a transaction index. The rpcpolling boolean
 // can be set to determine whether bitcoind's RPC polling interface should be
 // used for block and tx notifications or if its ZMQ interface should be used.
-// A connection to the newly spawned bitcoind node is returned.
+// A connection to the newly spawned bitcoind node is returned once the bitcoind
+// is synced to the miner's best height.
 func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	miner *rpctest.Harness, txindex, rpcpolling bool) *chain.BitcoindConn {
 
@@ -226,12 +227,92 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params,
 	}, wait.DefaultTimeout)
 	require.NoError(t, err, "timeout connecting to the miner")
 
+	// Assert the chain backend is synced to the miner.
+	syncBitcoindWithMiner(t, rpcClient, miner, p2pPort)
+
 	// Tear down the rpc client.
 	rpcClient.Shutdown()
 
 	return conn
 }
 
+// syncBitcoindWithMiner waits until the bitcoind node is synced with the miner.
+func syncBitcoindWithMiner(t *testing.T, notifier *rpcclient.Client,
+	miner *rpctest.Harness, p2pPort int) uint32 {
+
+	_, minerHeight, err := miner.Client.GetBestBlock()
+	require.NoError(t, err, "unable to retrieve miner's current height")
+
+	timeout := time.After(10 * time.Second)
+	for {
+		info, err := notifier.GetBlockChainInfo()
+		require.NoError(t, err)
+
+		bitcoindHeight := info.Blocks
+
+		t.Logf("miner height=%v, bitcoind height=%v", minerHeight,
+			bitcoindHeight)
+
+		if bitcoindHeight == minerHeight {
+			return uint32(bitcoindHeight)
+		}
+
+		select {
+		case <-time.After(100 * time.Millisecond):
+		case <-timeout:
+			t.Fatalf("timed out in syncNotifierWithMiner, got "+
+				"err=%v, minerHeight=%v, bitcoindHeight=%v",
+				err, minerHeight, bitcoindHeight)
+		}
+
+		// Get the num of connections the miner has. We expect it to
+		// have at least one connection with the chain backend.
+		count, err := miner.Client.GetConnectionCount()
+		require.NoError(t, err)
+		if count != 0 {
+			continue
+		}
+
+		// Reconnect the miner and the chain backend.
+		//
+		// NOTE: The connection should have been made before we perform
+		// the `syncNotifierWithMiner`. However, due unknown reason, the
+		// miner may refuse to process the inbound connection made by
+		// the bitcoind node, causing the connection to fail. It's
+		// possible there's a bug in the handshake between the two
+		// nodes.
+		//
+		// A normal flow is, bitcoind starts a v2 handshake flow, which
+		// btcd will fail and disconnect. Upon seeing this
+		// disconnection, bitcoind will try a v1 handshake and succeeds.
+		// The failed flow is, upon seeing the v2 handshake, btcd
+		// doesn't seem to perform the disconnect. Instead an EOF
+		// websocket error is found.
+		//
+		// TODO(yy): Fix the above bug in `btcd`. This can be reproduced
+		// using `make flakehunter-unit pkg=$pkg case=$case`, with,
+		// `case=TestHistoricalConfDetailsNoTxIndex/rpc_polling_enabled`
+		// `pkg=chainntnfs/bitcoindnotify`.
+		// Also need to modify the temp dir logic so we can save the
+		// debug logs.
+		// This bug is likely to be fixed when we implement the
+		// encrypted p2p conn, or when we properly fix the shutdown
+		// issues in all our RPC conns.
+		t.Log("Expected to the chain backend to have one conn with " +
+			"the miner, instead it's disconnected!")
+
+		// We now ask the miner to add the chain backend back.
+		host := fmt.Sprintf("127.0.0.1:%d", p2pPort)
+
+		// NOTE:AddNode must take a host that has the format
+		// `host:port`, otherwise the default port will be used. Check
+		// `normalizeAddress` in btcd for details.
+		err = miner.Client.AddNode(host, rpcclient.ANAdd)
+		require.NoError(t, err, "Failed to connect miner to the chain "+
+			"backend")
+	}
+}
+
 // NewNeutrinoBackend spawns a new neutrino node that connects to a miner at
 // the specified address.
 func NewNeutrinoBackend(t *testing.T, netParams *chaincfg.Params,

From 56fa3aae34eea5b4f467cc99afdf96f9276e79da Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Tue, 25 Feb 2025 22:12:25 +0800
Subject: [PATCH 09/10] routing/chainview: refactor `TestFilteredChainView`

So each test has its own miner and chainView.
---
 routing/chainview/interface_test.go | 87 +++++++++++++++--------------
 1 file changed, 45 insertions(+), 42 deletions(-)

diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go
index c0f336a68..f4daf22b5 100644
--- a/routing/chainview/interface_test.go
+++ b/routing/chainview/interface_test.go
@@ -160,9 +160,40 @@ func assertFilteredBlock(t *testing.T, fb *FilteredBlock, expectedHeight int32,
 
 }
 
-func testFilterBlockNotifications(node *rpctest.Harness,
-	chainView FilteredChainView, chainViewInit chainViewInitFunc,
-	t *testing.T) {
+// setupMinerAndChainView creates a miner node and initialize a chain view using
+// this miner node. It returns the miner and the chain view.
+func setupMinerAndChainView(t *testing.T, chainViewInit chainViewInitFunc) (
+	*rpctest.Harness, FilteredChainView) {
+
+	// Initialize the harness around a btcd node which will serve as our
+	// dedicated miner to generate blocks, cause re-orgs, etc. We'll set up
+	// this node with a chain length of 125, so we have plenty of BTC to
+	// play around with.
+	miner := unittest.NewMiner(
+		t, netParams, []string{"--txindex"}, true, 25,
+	)
+	rpcConfig := miner.RPCConfig()
+
+	_, bestHeight, err := miner.Client.GetBestBlock()
+	require.NoError(t, err)
+
+	chainView, err := chainViewInit(t, rpcConfig, miner, bestHeight)
+	require.NoError(t, err)
+
+	err = chainView.Start()
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		require.NoError(t, chainView.Stop())
+	})
+
+	return miner, chainView
+}
+
+func testFilterBlockNotifications(t *testing.T,
+	chainViewInit chainViewInitFunc) {
+
+	node, chainView := setupMinerAndChainView(t, chainViewInit)
 
 	// To start the test, we'll create to fresh outputs paying to the
 	// private key that we generated above.
@@ -270,9 +301,8 @@ func testFilterBlockNotifications(node *rpctest.Harness,
 	}
 }
 
-func testUpdateFilterBackTrack(node *rpctest.Harness,
-	chainView FilteredChainView, chainViewInit chainViewInitFunc,
-	t *testing.T) {
+func testUpdateFilterBackTrack(t *testing.T, chainViewInit chainViewInitFunc) {
+	node, chainView := setupMinerAndChainView(t, chainViewInit)
 
 	// To start, we'll create a fresh output paying to the height generated
 	// above.
@@ -345,8 +375,8 @@ func testUpdateFilterBackTrack(node *rpctest.Harness,
 	}
 }
 
-func testFilterSingleBlock(node *rpctest.Harness, chainView FilteredChainView,
-	chainViewInit chainViewInitFunc, t *testing.T) {
+func testFilterSingleBlock(t *testing.T, chainViewInit chainViewInitFunc) {
+	node, chainView := setupMinerAndChainView(t, chainViewInit)
 
 	// In this test, we'll test the manual filtration of blocks, which can
 	// be used by clients to manually rescan their sub-set of the UTXO set.
@@ -445,9 +475,10 @@ func testFilterSingleBlock(node *rpctest.Harness, chainView FilteredChainView,
 // testFilterBlockDisconnected triggers a reorg all the way back to genesis,
 // and a small 5 block reorg, ensuring the chainView notifies about
 // disconnected and connected blocks in the order we expect.
-func testFilterBlockDisconnected(node *rpctest.Harness,
-	chainView FilteredChainView, chainViewInit chainViewInitFunc,
-	t *testing.T) {
+func testFilterBlockDisconnected(t *testing.T,
+	chainViewInit chainViewInitFunc) {
+
+	node, _ := setupMinerAndChainView(t, chainViewInit)
 
 	// Create a node that has a shorter chain than the main chain, so we
 	// can trigger a reorg.
@@ -636,8 +667,7 @@ type chainViewInitFunc func(t *testing.T, rpcInfo rpcclient.ConnConfig,
 
 type testCase struct {
 	name string
-	test func(*rpctest.Harness, FilteredChainView, chainViewInitFunc,
-		*testing.T)
+	test func(*testing.T, chainViewInitFunc)
 }
 
 var chainViewTests = []testCase{
@@ -793,41 +823,17 @@ var interfaceImpls = []struct {
 }
 
 func TestFilteredChainView(t *testing.T) {
-	// Initialize the harness around a btcd node which will serve as our
-	// dedicated miner to generate blocks, cause re-orgs, etc. We'll set up
-	// this node with a chain length of 125, so we have plenty of BTC to
-	// play around with.
-	miner := unittest.NewMiner(
-		t, netParams, []string{"--txindex"}, true, 25,
-	)
-
-	rpcConfig := miner.RPCConfig()
-
 	for _, chainViewImpl := range interfaceImpls {
 		t.Logf("Testing '%v' implementation of FilteredChainView",
 			chainViewImpl.name)
 
-		_, bestHeight, err := miner.Client.GetBestBlock()
-		if err != nil {
-			t.Fatalf("error getting best block: %v", err)
-		}
-
-		chainView, err := chainViewImpl.chainViewInit(
-			t, rpcConfig, miner, bestHeight,
-		)
-		if err != nil {
-			t.Fatalf("unable to make chain view: %v", err)
-		}
-		if err := chainView.Start(); err != nil {
-			t.Fatalf("unable to start chain view: %v", err)
-		}
 		for _, chainViewTest := range chainViewTests {
 			testName := fmt.Sprintf("%v: %v", chainViewImpl.name,
 				chainViewTest.name)
+
 			success := t.Run(testName, func(t *testing.T) {
 				chainViewTest.test(
-					miner, chainView,
-					chainViewImpl.chainViewInit, t,
+					t, chainViewImpl.chainViewInit,
 				)
 			})
 
@@ -836,8 +842,5 @@ func TestFilteredChainView(t *testing.T) {
 			}
 		}
 
-		if err := chainView.Stop(); err != nil {
-			t.Fatalf("unable to stop chain view: %v", err)
-		}
 	}
 }

From cfa4341740910ace6efa1013f411b8428eb7404d Mon Sep 17 00:00:00 2001
From: yyforyongyu <yong2452@gmail.com>
Date: Wed, 26 Feb 2025 03:39:05 +0800
Subject: [PATCH 10/10] routing: fix flake in
 `TestFilteredChainView/bitcoind_polling`

---
 routing/chainview/interface_test.go | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go
index f4daf22b5..b8726a875 100644
--- a/routing/chainview/interface_test.go
+++ b/routing/chainview/interface_test.go
@@ -730,6 +730,20 @@ var interfaceImpls = []struct {
 				chainConn, blockCache,
 			)
 
+			// When running in rpc polling mode, the `reorg` method
+			// in `BitcoindClient`'s `ntfnHandler` may be invoked to
+			// handle the last block received from the miner during
+			// bitcoind's startup. This behavior will cause a block
+			// disconnected and a block connected notifications to
+			// be sent to the channels.
+			//
+			// TODO(yy): unify the chain backend logic and put
+			// everything in `btcwallet/chain` instead. The only
+			// place we use this chain view is in `graph/builder`,
+			// in which we subscribe to `FilteredBlocks` and
+			// `DisconnectedBlocks`.
+			time.Sleep(1 * time.Second)
+
 			return chainView, nil
 		},
 	},