Merge pull request #9582 from yyforyongyu/flake-doc

itest: properly document the known flakes

itest/flakes.go

@@ -0,0 +1,106 @@
+package itest
+
+import (
+	"runtime"
+	"time"
+
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/lightningnetwork/lnd/lntest/node"
+)
+
+// flakePreimageSettlement documents a flake found when testing the preimage
+// extraction logic in a force close. The scenario is,
+//   - Alice and Bob have a channel.
+//   - Alice sends an HTLC to Bob, and Bob won't settle it.
+//   - Alice goes offline.
+//   - Bob force closes the channel and claims the HTLC using the preimage using
+//     a sweeping tx.
+//
+// TODO(yy): Expose blockbeat to the link layer so the preimage extraction
+// happens in the same block where it's spent.
+func flakePreimageSettlement(ht *lntest.HarnessTest) {
+	// Mine a block to trigger the sweep. This is needed because the
+	// preimage extraction logic from the link is not managed by the
+	// blockbeat, which means the preimage may be sent to the contest
+	// resolver after it's launched, which means Bob would miss the block to
+	// launch the resolver.
+	ht.MineEmptyBlocks(1)
+
+	// Sleep for 2 seconds, which is needed to make sure the mempool has the
+	// correct tx. The above mined block can cause an RBF, if the preimage
+	// extraction has already been finished before the block is mined. This
+	// means Bob would have created the sweeping tx - mining another block
+	// would cause the sweeper to RBF it.
+	time.Sleep(2 * time.Second)
+}
+
+// flakeFundExtraUTXO documents a flake found when testing the sweeping behavior
+// of the given node, which would fail due to no wallet UTXO available, while
+// there should be enough.
+//
+// TODO(yy): remove it once the issue is resolved.
+func flakeFundExtraUTXO(ht *lntest.HarnessTest, node *node.HarnessNode) {
+	// The node should have enough wallet UTXOs here to sweep the HTLC in
+	// the end of this test. However, due to a known issue, the node's
+	// wallet may report there's no UTXO available. For details,
+	// - https://github.com/lightningnetwork/lnd/issues/8786
+	ht.FundCoins(btcutil.SatoshiPerBitcoin, node)
+}
+
+// flakeTxNotifierNeutrino documents a flake found when running force close
+// tests using neutrino backend, which is a race between two notifications - one
+// for the spending notification, the other for the block which contains the
+// spending tx.
+//
+// TODO(yy): remove it once the issue is resolved.
+func flakeTxNotifierNeutrino(ht *lntest.HarnessTest) {
+	// Mine an empty block the for neutrino backend. We need this step to
+	// trigger Bob's chain watcher to detect the force close tx. Deep down,
+	// this happens because the notification system for neutrino is very
+	// different from others. Specifically, when a block contains the force
+	// close tx is notified, these two calls,
+	// - RegisterBlockEpochNtfn, will notify the block first.
+	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to
+	//   the block, then perform a GetUtxo, which, by the time the spend
+	//   details are sent, the blockbeat is considered processed in Bob's
+	//   chain watcher.
+	//
+	// TODO(yy): refactor txNotifier to fix the above issue.
+	if ht.IsNeutrinoBackend() {
+		ht.MineEmptyBlocks(1)
+	}
+}
+
+// flakeSkipPendingSweepsCheckDarwin documents a flake found only in macOS
+// build. When running in macOS, we might see three anchor sweeps - one from the
+// local, one from the remote, and one from the pending remote:
+//   - the local one will be swept.
+//   - the remote one will be marked as failed due to `testmempoolaccept` check.
+//   - the pending remote one will not be attempted due to it being uneconomical
+//     since it was not used for CPFP.
+//
+// The anchor from the pending remote may or may not appear, which is a bug
+// found only in macOS - when updating the commitments, the channel state
+// machine somehow thinks we still have a pending remote commitment, causing it
+// to sweep the anchor from that version.
+//
+// TODO(yy): fix the above bug in the channel state machine.
+func flakeSkipPendingSweepsCheckDarwin(ht *lntest.HarnessTest,
+	node *node.HarnessNode, num int) {
+
+	// Skip the assertion below if it's on macOS.
+	if isDarwin() {
+		ht.Logf("Skipped AssertNumPendingSweeps for node %s",
+			node.Name())
+
+		return
+	}
+
+	ht.AssertNumPendingSweeps(node, num)
+}
+
+// isDarwin returns true if the test is running on a macOS.
+func isDarwin() bool {
+	return runtime.GOOS == "darwin"
+}

itest/lnd_channel_force_close_test.go

@@ -1371,13 +1371,7 @@ func testFailingChannel(ht *lntest.HarnessTest) {
 	// Carol will use the correct preimage to resolve the HTLC on-chain.
 	ht.AssertNumPendingSweeps(carol, 1)
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// Carol should have broadcast her sweeping tx.
 	ht.AssertNumTxsInMempool(1)

itest/lnd_funding_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"testing"
-	"time"
 
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -559,17 +558,6 @@ func runChannelFundingInputTypes(ht *lntest.HarnessTest, alice,
 		checkChannelBalance(carol, carolLocalBalance, 0, 0, 0)
 		checkChannelBalance(alice, 0, carolLocalBalance, 0, 0)
 
-		// TODO(yy): remove the sleep once the following bug is fixed.
-		//
-		// We may get the error `unable to gracefully close channel
-		// while peer is offline (try force closing it instead):
-		// channel link not found`. This happens because the channel
-		// link hasn't been added yet but we now proceed to closing the
-		// channel. We may need to revisit how the channel open event
-		// is created and make sure the event is only sent after all
-		// relevant states have been updated.
-		time.Sleep(2 * time.Second)
-
 		// Now that we're done with the test, the channel can be closed.
 		ht.CloseChannel(carol, chanPoint)
 
@@ -685,14 +673,6 @@ func runExternalFundingScriptEnforced(ht *lntest.HarnessTest) {
 	// HTLCs.
 	ht.AssertInvoiceSettled(dave, resp.PaymentAddr)
 
-	// TODO(yy): remove the sleep once the following bug is fixed.
-	// When the invoice is reported settled, the commitment dance is not
-	// yet finished, which can cause an error when closing the channel,
-	// saying there's active HTLCs. We need to investigate this issue and
-	// reverse the order to, first finish the commitment dance, then report
-	// the invoice as settled.
-	time.Sleep(2 * time.Second)
-
 	// Next we'll try but this time with Dave (the responder) as the
 	// initiator. This time the channel should be closed as normal.
 	ht.CloseChannel(dave, chanPoint2)
@@ -845,14 +825,6 @@ func runExternalFundingTaproot(ht *lntest.HarnessTest) {
 	// HTLCs.
 	ht.AssertInvoiceSettled(dave, resp.PaymentAddr)
 
-	// TODO(yy): remove the sleep once the following bug is fixed.
-	// When the invoice is reported settled, the commitment dance is not
-	// yet finished, which can cause an error when closing the channel,
-	// saying there's active HTLCs. We need to investigate this issue and
-	// reverse the order to, first finish the commitment dance, then report
-	// the invoice as settled.
-	time.Sleep(2 * time.Second)
-
 	// Next we'll try but this time with Dave (the responder) as the
 	// initiator. This time the channel should be closed as normal.
 	ht.CloseChannel(dave, chanPoint2)

itest/lnd_hold_invoice_force_test.go

@@ -89,13 +89,6 @@ func testHoldInvoiceForceClose(ht *lntest.HarnessTest) {
 	blocksTillCancel := blocksTillExpiry -
 		lncfg.DefaultHoldInvoiceExpiryDelta
 
-	// When using ht.MineBlocks, for bitcoind backend, the block height
-	// synced differ significantly among subsystems. From observation, the
-	// LNWL syncs much faster than other subsystems, with more than 10
-	// blocks ahead. For this test case, CRTR may be lagging behind for
-	// more than 20 blocks. Thus we use slow mining instead.
-	// TODO(yy): fix block height asymmetry among all the subsystems.
-	//
 	// We first mine enough blocks to trigger an invoice cancelation.
 	ht.MineBlocks(int(blocksTillCancel))
 

itest/lnd_misc_test.go

@@ -24,96 +24,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// testDisconnectingTargetPeer performs a test which disconnects Alice-peer
-// from Bob-peer and then re-connects them again. We expect Alice to be able to
-// disconnect at any point.
-//
-// TODO(yy): move to lnd_network_test.
-func testDisconnectingTargetPeer(ht *lntest.HarnessTest) {
-	// We'll start both nodes with a high backoff so that they don't
-	// reconnect automatically during our test.
-	args := []string{
-		"--minbackoff=1m",
-		"--maxbackoff=1m",
-	}
-
-	alice := ht.NewNodeWithCoins("Alice", args)
-	bob := ht.NewNodeWithCoins("Bob", args)
-
-	// Start by connecting Alice and Bob with no channels.
-	ht.EnsureConnected(alice, bob)
-
-	chanAmt := funding.MaxBtcFundingAmount
-	pushAmt := btcutil.Amount(0)
-
-	// Create a new channel that requires 1 confs before it's considered
-	// open, then broadcast the funding transaction
-	const numConfs = 1
-	p := lntest.OpenChannelParams{
-		Amt:     chanAmt,
-		PushAmt: pushAmt,
-	}
-	stream := ht.OpenChannelAssertStream(alice, bob, p)
-
-	// At this point, the channel's funding transaction will have been
-	// broadcast, but not confirmed. Alice and Bob's nodes should reflect
-	// this when queried via RPC.
-	ht.AssertNumPendingOpenChannels(alice, 1)
-	ht.AssertNumPendingOpenChannels(bob, 1)
-
-	// Disconnect Alice-peer from Bob-peer should have no error.
-	ht.DisconnectNodes(alice, bob)
-
-	// Assert that the connection was torn down.
-	ht.AssertNotConnected(alice, bob)
-
-	// Mine a block, then wait for Alice's node to notify us that the
-	// channel has been opened.
-	ht.MineBlocksAndAssertNumTxes(numConfs, 1)
-
-	// At this point, the channel should be fully opened and there should
-	// be no pending channels remaining for either node.
-	ht.AssertNumPendingOpenChannels(alice, 0)
-	ht.AssertNumPendingOpenChannels(bob, 0)
-
-	// Reconnect the nodes so that the channel can become active.
-	ht.ConnectNodes(alice, bob)
-
-	// The channel should be listed in the peer information returned by
-	// both peers.
-	chanPoint := ht.WaitForChannelOpenEvent(stream)
-
-	// Check both nodes to ensure that the channel is ready for operation.
-	ht.AssertChannelExists(alice, chanPoint)
-	ht.AssertChannelExists(bob, chanPoint)
-
-	// Disconnect Alice-peer from Bob-peer should have no error.
-	ht.DisconnectNodes(alice, bob)
-
-	// Check existing connection.
-	ht.AssertNotConnected(alice, bob)
-
-	// Reconnect both nodes before force closing the channel.
-	ht.ConnectNodes(alice, bob)
-
-	// Finally, immediately close the channel. This function will also
-	// block until the channel is closed and will additionally assert the
-	// relevant channel closing post conditions.
-	ht.ForceCloseChannel(alice, chanPoint)
-
-	// Disconnect Alice-peer from Bob-peer should have no error.
-	ht.DisconnectNodes(alice, bob)
-
-	// Check that the nodes not connected.
-	ht.AssertNotConnected(alice, bob)
-
-	// Finally, re-connect both nodes.
-	ht.ConnectNodes(alice, bob)
-
-	// Check existing connection.
-	ht.AssertConnected(alice, bob)
-}
-
 // testSphinxReplayPersistence verifies that replayed onion packets are
 // rejected by a remote peer after a restart. We use a combination of unsafe
 // configuration arguments to force Carol to replay the same sphinx packet

itest/lnd_multi-hop_force_close_test.go

@@ -341,13 +341,7 @@ func runLocalClaimOutgoingHTLC(ht *lntest.HarnessTest,
 		ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 	}
 
-	// Bob should have enough wallet UTXOs here to sweep the HTLC in the
+	flakeFundExtraUTXO(ht, bob)
-	// end of this test. However, due to a known issue, Bob's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 
 	// Now that our channels are set up, we'll send two HTLC's from Alice
 	// to Carol. The first HTLC will be universally considered "dust",
@@ -429,22 +423,7 @@ func runLocalClaimOutgoingHTLC(ht *lntest.HarnessTest,
 		numSweeps = 2
 	}
 
-	// When running in macOS, we might see three anchor sweeps - one from
+	flakeSkipPendingSweepsCheckDarwin(ht, bob, numSweeps)
-	// the local, one from the remote, and one from the pending remote:
-	// - the local one will be swept.
-	// - the remote one will be marked as failed due to `testmempoolaccept`
-	//   check.
-	// - the pending remote one will not be attempted due to it being
-	//   uneconomical since it was not used for CPFP.
-	// The anchor from the pending remote may or may not appear, which is a
-	// bug found only in macOS - when updating the commitments, the channel
-	// state machine somehow thinks we still have a pending remote
-	// commitment, causing it to sweep the anchor from that version.
-	//
-	// TODO(yy): fix the above bug in the channel state machine.
-	if !isDarwin() {
-		ht.AssertNumPendingSweeps(bob, numSweeps)
-	}
 
 	// We expect to see tow txns in the mempool,
 	// 1. Bob's force close tx.
@@ -707,14 +686,7 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,
 
 	// Fund Carol one UTXO so she can sweep outputs.
 	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
-
+	flakeFundExtraUTXO(ht, carol)
-	// Carol should have enough wallet UTXOs here to sweep the HTLC in the
-	// end of this test. However, due to a known issue, Carol's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
 
 	// If this is a taproot channel, then we'll need to make some manual
 	// route hints so Alice can actually find a route.
@@ -808,22 +780,7 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,
 		numSweeps = 2
 	}
 
-	// When running in macOS, we might see three anchor sweeps - one from
+	flakeSkipPendingSweepsCheckDarwin(ht, carol, numSweeps)
-	// the local, one from the remote, and one from the pending remote:
-	// - the local one will be swept.
-	// - the remote one will be marked as failed due to `testmempoolaccept`
-	//   check.
-	// - the pending remote one will not be attempted due to it being
-	//   uneconomical since it was not used for CPFP.
-	// The anchor from the pending remote may or may not appear, which is a
-	// bug found only in macOS - when updating the commitments, the channel
-	// state machine somehow thinks we still have a pending remote
-	// commitment, causing it to sweep the anchor from that version.
-	//
-	// TODO(yy): fix the above bug in the channel state machine.
-	if !isDarwin() {
-		ht.AssertNumPendingSweeps(carol, numSweeps)
-	}
 
 	// We expect to see tow txns in the mempool,
 	// 1. Carol's force close tx.
@@ -859,21 +816,7 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,
 		ht.AssertNumPendingSweeps(bob, 2)
 	}
 
-	// Mine an empty block the for neutrino backend. We need this step to
+	flakeTxNotifierNeutrino(ht)
-	// trigger Bob's chain watcher to detect the force close tx. Deep down,
-	// this happens because the notification system for neutrino is very
-	// different from others. Specifically, when a block contains the force
-	// close tx is notified, these two calls,
-	// - RegisterBlockEpochNtfn, will notify the block first.
-	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to
-	//   the block, then perform a GetUtxo, which, by the time the spend
-	//   details are sent, the blockbeat is considered processed in Bob's
-	//   chain watcher.
-	//
-	// TODO(yy): refactor txNotifier to fix the above issue.
-	if ht.IsNeutrinoBackend() {
-		ht.MineEmptyBlocks(1)
-	}
 
 	if params.CommitmentType == leasedType {
 		// We expect to see 1 txns in the mempool,
@@ -1656,22 +1599,8 @@ func runLocalClaimIncomingHTLC(ht *lntest.HarnessTest,
 
 	// Fund Carol one UTXO so she can sweep outputs.
 	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
-
+	flakeFundExtraUTXO(ht, carol)
-	// Carol should have enough wallet UTXOs here to sweep the HTLC in the
+	flakeFundExtraUTXO(ht, bob)
-	// end of this test. However, due to a known issue, Carol's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
-
-	// Bob should have enough wallet UTXOs here to sweep the HTLC in the
-	// end of this test. However, due to a known issue, Bob's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 
 	// If this is a taproot channel, then we'll need to make some manual
 	// route hints so Alice can actually find a route.
@@ -1802,21 +1731,7 @@ func runLocalClaimIncomingHTLC(ht *lntest.HarnessTest,
 	// - the anchor output from channel Bob=>Carol.
 	ht.AssertNumPendingSweeps(bob, 3)
 
-	// Mine an empty block the for neutrino backend. We need this step to
+	flakeTxNotifierNeutrino(ht)
-	// trigger Bob's chain watcher to detect the force close tx. Deep down,
-	// this happens because the notification system for neutrino is very
-	// different from others. Specifically, when a block contains the force
-	// close tx is notified, these two calls,
-	// - RegisterBlockEpochNtfn, will notify the block first.
-	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to
-	//   the block, then perform a GetUtxo, which, by the time the spend
-	//   details are sent, the blockbeat is considered processed in Bob's
-	//   chain watcher.
-	//
-	// TODO(yy): refactor txNotifier to fix the above issue.
-	if ht.IsNeutrinoBackend() {
-		ht.MineEmptyBlocks(1)
-	}
 
 	// Assert txns can be found in the mempool.
 	//
@@ -1842,13 +1757,7 @@ func runLocalClaimIncomingHTLC(ht *lntest.HarnessTest,
 	// - the anchor output from channel Bob=>Carol.
 	ht.AssertNumPendingSweeps(bob, 3)
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// At this point, Bob should have broadcast his second layer success
 	// tx, and should have sent it to his sweeper.
@@ -1965,8 +1874,6 @@ func testLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest) {
 // we force close a channel with an incoming HTLC, and later find out the
 // preimage via the witness beacon, we properly settle the HTLC on-chain using
 // the HTLC success transaction in order to ensure we don't lose any funds.
-//
-// TODO(yy): simplify or remove this test as it's too complicated.
 func runLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest,
 	cfgs [][]string, params lntest.OpenChannelParams) {
 
@@ -1984,14 +1891,7 @@ func runLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest,
 
 	// Fund Carol one UTXO so she can sweep outputs.
 	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
-
+	flakeFundExtraUTXO(ht, carol)
-	// Carol should have enough wallet UTXOs here to sweep the HTLC in the
-	// end of this test. However, due to a known issue, Carol's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
 
 	// With the network active, we'll now add a new hodl invoice at Carol's
 	// end. Make sure the cltv expiry delta is large enough, otherwise Bob
@@ -2126,13 +2026,7 @@ func runLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest,
 	// - the anchor output from channel Bob=>Carol.
 	ht.AssertNumPendingSweeps(bob, 3)
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// At this point, Bob should have broadcast his second layer success
 	// tx, and should have sent it to his sweeper.
@@ -2326,14 +2220,7 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
 
 	// Fund Carol one UTXO so she can sweep outputs.
 	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
-
+	flakeFundExtraUTXO(ht, carol)
-	// Carol should have enough wallet UTXOs here to sweep the HTLC in the
-	// end of this test. However, due to a known issue, Carol's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
 
 	// If this is a taproot channel, then we'll need to make some manual
 	// route hints so Alice can actually find a route.
@@ -2448,22 +2335,7 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
 		numSweeps = 2
 	}
 
-	// When running in macOS, we might see three anchor sweeps - one from
+	flakeSkipPendingSweepsCheckDarwin(ht, carol, numSweeps)
-	// the local, one from the remote, and one from the pending remote:
-	// - the local one will be swept.
-	// - the remote one will be marked as failed due to `testmempoolaccept`
-	//   check.
-	// - the pending remote one will not be attempted due to it being
-	//   uneconomical since it was not used for CPFP.
-	// The anchor from the pending remote may or may not appear, which is a
-	// bug found only in macOS - when updating the commitments, the channel
-	// state machine somehow thinks we still have a pending remote
-	// commitment, causing it to sweep the anchor from that version.
-	//
-	// TODO(yy): fix the above bug in the channel state machine.
-	if !isDarwin() {
-		ht.AssertNumPendingSweeps(carol, numSweeps)
-	}
 
 	// We should see two txns in the mempool, we now a block to confirm,
 	// - Carol's force close tx.
@@ -2483,21 +2355,7 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
 	// - the commit output sweep from the channel with Carol, no timelock.
 	ht.AssertNumPendingSweeps(bob, 3)
 
-	// Mine an empty block the for neutrino backend. We need this step to
+	flakeTxNotifierNeutrino(ht)
-	// trigger Bob's chain watcher to detect the force close tx. Deep down,
-	// this happens because the notification system for neutrino is very
-	// different from others. Specifically, when a block contains the force
-	// close tx is notified, these two calls,
-	// - RegisterBlockEpochNtfn, will notify the block first.
-	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to
-	//   the block, then perform a GetUtxo, which, by the time the spend
-	//   details are sent, the blockbeat is considered processed in Bob's
-	//   chain watcher.
-	//
-	// TODO(yy): refactor txNotifier to fix the above issue.
-	if ht.IsNeutrinoBackend() {
-		ht.MineEmptyBlocks(1)
-	}
 
 	// We mine one block to confirm,
 	// - Carol's sweeping tx of the incoming HTLC.
@@ -2511,29 +2369,9 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
 	// - the htlc sweeping tx.
 	ht.AssertNumPendingSweeps(bob, 3)
 
-	// Mine an empty block the for neutrino backend. We need this step to
+	flakeTxNotifierNeutrino(ht)
-	// trigger Bob's chain watcher to detect the force close tx. Deep down,
-	// this happens because the notification system for neutrino is very
-	// different from others. Specifically, when a block contains the force
-	// close tx is notified, these two calls,
-	// - RegisterBlockEpochNtfn, will notify the block first.
-	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to
-	//   the block, then perform a GetUtxo, which, by the time the spend
-	//   details are sent, the blockbeat is considered processed in Bob's
-	//   chain watcher.
-	//
-	// TODO(yy): refactor txNotifier to fix the above issue.
-	if ht.IsNeutrinoBackend() {
-		ht.MineEmptyBlocks(1)
-	}
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// Bob should broadcast the sweeping of the direct preimage spent now.
 	bobHtlcSweep := ht.GetNumTxsFromMempool(1)[0]
@@ -2639,14 +2477,7 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
 
 	// Fund Carol one UTXO so she can sweep outputs.
 	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
-
+	flakeFundExtraUTXO(ht, carol)
-	// Carol should have enough wallet UTXOs here to sweep the HTLC in the
-	// end of this test. However, due to a known issue, Carol's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
 
 	// With the network active, we'll now add a new hodl invoice at Carol's
 	// end. Make sure the cltv expiry delta is large enough, otherwise Bob
@@ -2744,22 +2575,7 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
 		numSweeps = 2
 	}
 
-	// When running in macOS, we might see three anchor sweeps - one from
+	flakeSkipPendingSweepsCheckDarwin(ht, carol, numSweeps)
-	// the local, one from the remote, and one from the pending remote:
-	// - the local one will be swept.
-	// - the remote one will be marked as failed due to `testmempoolaccept`
-	//   check.
-	// - the pending remote one will not be attempted due to it being
-	//   uneconomical since it was not used for CPFP.
-	// The anchor from the pending remote may or may not appear, which is a
-	// bug found only in macOS - when updating the commitments, the channel
-	// state machine somehow thinks we still have a pending remote
-	// commitment, causing it to sweep the anchor from that version.
-	//
-	// TODO(yy): fix the above bug in the channel state machine.
-	if !isDarwin() {
-		ht.AssertNumPendingSweeps(carol, numSweeps)
-	}
 
 	// We should see two txns in the mempool, we now a block to confirm,
 	// - Carol's force close tx.
@@ -2791,13 +2607,7 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
 	// - the htlc sweeping tx.
 	ht.AssertNumPendingSweeps(bob, 3)
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// Bob should broadcast the sweeping of the direct preimage spent now.
 	bobHtlcSweep := ht.GetNumTxsFromMempool(1)[0]
@@ -3016,14 +2826,7 @@ func runHtlcAggregation(ht *lntest.HarnessTest,
 	// We need one additional UTXO to create the sweeping tx for the
 	// second-level success txes.
 	ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
-
+	flakeFundExtraUTXO(ht, bob)
-	// Bob should have enough wallet UTXOs here to sweep the HTLC in the
-	// end of this test. However, due to a known issue, Bob's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 
 	// If this is a taproot channel, then we'll need to make some manual
 	// route hints so Alice+Carol can actually find a route.
@@ -3180,22 +2983,7 @@ func runHtlcAggregation(ht *lntest.HarnessTest,
 		numSweeps = 2
 	}
 
-	// When running in macOS, we might see three anchor sweeps - one from
+	flakeSkipPendingSweepsCheckDarwin(ht, bob, numSweeps)
-	// the local, one from the remote, and one from the pending remote:
-	// - the local one will be swept.
-	// - the remote one will be marked as failed due to `testmempoolaccept`
-	//   check.
-	// - the pending remote one will not be attempted due to it being
-	//   uneconomical since it was not used for CPFP.
-	// The anchor from the pending remote may or may not appear, which is a
-	// bug found only in macOS - when updating the commitments, the channel
-	// state machine somehow thinks we still have a pending remote
-	// commitment, causing it to sweep the anchor from that version.
-	//
-	// TODO(yy): fix the above bug in the channel state machine.
-	if !isDarwin() {
-		ht.AssertNumPendingSweeps(bob, numSweeps)
-	}
 
 	// Bob's force close tx and anchor sweeping tx should now be found in
 	// the mempool.
@@ -3223,13 +3011,7 @@ func runHtlcAggregation(ht *lntest.HarnessTest,
 	// txns.
 	ht.AssertNumPendingSweeps(bob, numInvoices*2)
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// We expect to see three sweeping txns:
 	// 1. Bob's sweeping tx for all timeout HTLCs.

itest/lnd_network_test.go

@@ -5,6 +5,8 @@ import (
 	"net"
 	"time"
 
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/lightningnetwork/lnd/funding"
 	"github.com/lightningnetwork/lnd/lncfg"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lntest"
@@ -243,3 +245,91 @@ func testAddPeerConfig(ht *lntest.HarnessTest) {
 
 	require.Equal(ht, parsedKeyStr, listPeersResp.Peers[0].PubKey)
 }
+
+// testDisconnectingTargetPeer performs a test which disconnects Alice-peer
+// from Bob-peer and then re-connects them again. We expect Alice to be able to
+// disconnect at any point.
+func testDisconnectingTargetPeer(ht *lntest.HarnessTest) {
+	// We'll start both nodes with a high backoff so that they don't
+	// reconnect automatically during our test.
+	args := []string{
+		"--minbackoff=1m",
+		"--maxbackoff=1m",
+	}
+
+	alice := ht.NewNodeWithCoins("Alice", args)
+	bob := ht.NewNodeWithCoins("Bob", args)
+
+	// Start by connecting Alice and Bob with no channels.
+	ht.EnsureConnected(alice, bob)
+
+	chanAmt := funding.MaxBtcFundingAmount
+	pushAmt := btcutil.Amount(0)
+
+	// Create a new channel that requires 1 confs before it's considered
+	// open, then broadcast the funding transaction
+	const numConfs = 1
+	p := lntest.OpenChannelParams{
+		Amt:     chanAmt,
+		PushAmt: pushAmt,
+	}
+	stream := ht.OpenChannelAssertStream(alice, bob, p)
+
+	// At this point, the channel's funding transaction will have been
+	// broadcast, but not confirmed. Alice and Bob's nodes should reflect
+	// this when queried via RPC.
+	ht.AssertNumPendingOpenChannels(alice, 1)
+	ht.AssertNumPendingOpenChannels(bob, 1)
+
+	// Disconnect Alice-peer from Bob-peer should have no error.
+	ht.DisconnectNodes(alice, bob)
+
+	// Assert that the connection was torn down.
+	ht.AssertNotConnected(alice, bob)
+
+	// Mine a block, then wait for Alice's node to notify us that the
+	// channel has been opened.
+	ht.MineBlocksAndAssertNumTxes(numConfs, 1)
+
+	// At this point, the channel should be fully opened and there should
+	// be no pending channels remaining for either node.
+	ht.AssertNumPendingOpenChannels(alice, 0)
+	ht.AssertNumPendingOpenChannels(bob, 0)
+
+	// Reconnect the nodes so that the channel can become active.
+	ht.ConnectNodes(alice, bob)
+
+	// The channel should be listed in the peer information returned by
+	// both peers.
+	chanPoint := ht.WaitForChannelOpenEvent(stream)
+
+	// Check both nodes to ensure that the channel is ready for operation.
+	ht.AssertChannelExists(alice, chanPoint)
+	ht.AssertChannelExists(bob, chanPoint)
+
+	// Disconnect Alice-peer from Bob-peer should have no error.
+	ht.DisconnectNodes(alice, bob)
+
+	// Check existing connection.
+	ht.AssertNotConnected(alice, bob)
+
+	// Reconnect both nodes before force closing the channel.
+	ht.ConnectNodes(alice, bob)
+
+	// Finally, immediately close the channel. This function will also
+	// block until the channel is closed and will additionally assert the
+	// relevant channel closing post conditions.
+	ht.ForceCloseChannel(alice, chanPoint)
+
+	// Disconnect Alice-peer from Bob-peer should have no error.
+	ht.DisconnectNodes(alice, bob)
+
+	// Check that the nodes not connected.
+	ht.AssertNotConnected(alice, bob)
+
+	// Finally, re-connect both nodes.
+	ht.ConnectNodes(alice, bob)
+
+	// Check existing connection.
+	ht.AssertConnected(alice, bob)
+}

itest/lnd_payment_test.go

@@ -152,13 +152,7 @@ func testPaymentSucceededHTLCRemoteSwept(ht *lntest.HarnessTest) {
 	// Alice should have a pending force close channel.
 	ht.AssertNumPendingForceClose(alice, 1)
 
-	// Mine a block to trigger the sweep. This is needed because the
+	flakePreimageSettlement(ht)
-	// preimage extraction logic from the link is not managed by the
-	// blockbeat, which means the preimage may be sent to the contest
-	// resolver after it's launched.
-	//
-	// TODO(yy): Expose blockbeat to the link layer.
-	ht.MineEmptyBlocks(1)
 
 	// Mine Bob's sweeping tx.
 	ht.MineBlocksAndAssertNumTxes(1, 1)

itest/lnd_sweep_test.go

@@ -114,13 +114,7 @@ func testSweepCPFPAnchorOutgoingTimeout(ht *lntest.HarnessTest) {
 		ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 	}
 
-	// Bob should have enough wallet UTXOs here to sweep the HTLC in the
+	flakeFundExtraUTXO(ht, bob)
-	// end of this test. However, due to a known issue, Bob's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 
 	// Subscribe the invoice.
 	streamCarol := carol.RPC.SubscribeSingleInvoice(payHash[:])
@@ -440,13 +434,7 @@ func testSweepCPFPAnchorIncomingTimeout(ht *lntest.HarnessTest) {
 		ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 	}
 
-	// Bob should have enough wallet UTXOs here to sweep the HTLC in the
+	flakeFundExtraUTXO(ht, bob)
-	// end of this test. However, due to a known issue, Bob's wallet may
-	// report there's no UTXO available. For details,
-	// - https://github.com/lightningnetwork/lnd/issues/8786
-	//
-	// TODO(yy): remove this step once the issue is resolved.
-	ht.FundCoins(btcutil.SatoshiPerBitcoin, bob)
 
 	// Subscribe the invoice.
 	streamCarol := carol.RPC.SubscribeSingleInvoice(payHash[:])

itest/lnd_test.go

@@ -254,11 +254,6 @@ func getLndBinary(t *testing.T) string {
 	return binary
 }
 
-// isDarwin returns true if the test is running on a macOS.
-func isDarwin() bool {
-	return runtime.GOOS == "darwin"
-}
-
 // isWindowsOS returns true if the test is running on a Windows OS.
 func isWindowsOS() bool {
 	return runtime.GOOS == "windows"