diff --git a/itest/flakes.go b/itest/flakes.go new file mode 100644 index 000000000..465ef4bec --- /dev/null +++ b/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" +} diff --git a/itest/lnd_channel_force_close_test.go b/itest/lnd_channel_force_close_test.go index 24868a019..55bbc0c03 100644 --- a/itest/lnd_channel_force_close_test.go +++ b/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 - // 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) + flakePreimageSettlement(ht) // Carol should have broadcast her sweeping tx. ht.AssertNumTxsInMempool(1) diff --git a/itest/lnd_funding_test.go b/itest/lnd_funding_test.go index c2704c3f4..4a984a06e 100644 --- a/itest/lnd_funding_test.go +++ b/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) diff --git a/itest/lnd_hold_invoice_force_test.go b/itest/lnd_hold_invoice_force_test.go index 5742d3f7c..52dfdaacc 100644 --- a/itest/lnd_hold_invoice_force_test.go +++ b/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)) diff --git a/itest/lnd_misc_test.go b/itest/lnd_misc_test.go index 51175399e..4756ca742 100644 --- a/itest/lnd_misc_test.go +++ b/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 diff --git a/itest/lnd_multi-hop_force_close_test.go b/itest/lnd_multi-hop_force_close_test.go index befaf3447..17fd75758 100644 --- a/itest/lnd_multi-hop_force_close_test.go +++ b/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 - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) - } + flakeSkipPendingSweepsCheckDarwin(ht, 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) - - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) - } + flakeSkipPendingSweepsCheckDarwin(ht, 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 - // 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) - } + flakeTxNotifierNeutrino(ht) 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) - - // 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) - - // 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) + flakeFundExtraUTXO(ht, carol) + flakeFundExtraUTXO(ht, 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 - // 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) - } + flakeTxNotifierNeutrino(ht) // 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 - // 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) + flakePreimageSettlement(ht) // 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) - - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) + flakePreimageSettlement(ht) // 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) - - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) - } + flakeSkipPendingSweepsCheckDarwin(ht, 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 - // 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) - } + flakeTxNotifierNeutrino(ht) // 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 - // 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) - } + flakeTxNotifierNeutrino(ht) - // 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. - // - // TODO(yy): Expose blockbeat to the link layer. - ht.MineEmptyBlocks(1) + flakePreimageSettlement(ht) // 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) - - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) - } + flakeSkipPendingSweepsCheckDarwin(ht, 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 - // 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) + flakePreimageSettlement(ht) // 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) - - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) - } + flakeSkipPendingSweepsCheckDarwin(ht, 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 - // 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) + flakePreimageSettlement(ht) // We expect to see three sweeping txns: // 1. Bob's sweeping tx for all timeout HTLCs. diff --git a/itest/lnd_network_test.go b/itest/lnd_network_test.go index 44a0dcffa..c5df649ed 100644 --- a/itest/lnd_network_test.go +++ b/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) +} diff --git a/itest/lnd_payment_test.go b/itest/lnd_payment_test.go index 0b220cd1b..80f7dcac8 100644 --- a/itest/lnd_payment_test.go +++ b/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 - // 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) + flakePreimageSettlement(ht) // Mine Bob's sweeping tx. ht.MineBlocksAndAssertNumTxes(1, 1) diff --git a/itest/lnd_sweep_test.go b/itest/lnd_sweep_test.go index 377915412..db3e2171e 100644 --- a/itest/lnd_sweep_test.go +++ b/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 - // 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) + flakeFundExtraUTXO(ht, 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 - // 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) + flakeFundExtraUTXO(ht, bob) // Subscribe the invoice. streamCarol := carol.RPC.SubscribeSingleInvoice(payHash[:]) diff --git a/itest/lnd_test.go b/itest/lnd_test.go index 07b8554f2..32c0f1c02 100644 --- a/itest/lnd_test.go +++ b/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"