13786526560

Committed 11 Mar 2025 11:06AM UTC coverage: 68.603% (-0.009%) from 68.612%

Build # 13786526560

Build Type

Pull #9600

github

Committed by

web-flow

Commit Message

Merge 39f57cbe0 into a673826de

Pull Request Pull Request #9600: lntest+itest: document and fix more flakes

Run Details

14 of 29 new or added lines in 4 files covered. (48.28%)

67 existing lines in 16 files now uncovered.

129988 of 189479 relevant lines covered (68.6%)

23728.64 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

0.0

/itest/flakes.go

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"
}

// flakeInconsistentHTLCView documents a flake found that the `ListChannels` RPC
// can give inaccurate HTLC states, which is found when we call
// `AssertHTLCNotActive` after a commitment dance is finished. Suppose Carol is
// settling an invoice with Bob, from Bob's PoV, a typical healthy settlement
// flow goes like this:
//
//        [DBG] PEER brontide.go:2412: Peer([Carol]): Received UpdateFulfillHTLC
//        [DBG] HSWC switch.go:1315: Closed completed SETTLE circuit for ...
//        [INF] HSWC switch.go:3044: Forwarded HTLC...
//        [DBG] PEER brontide.go:2412: Peer([Carol]): Received CommitSig
//        [DBG] PEER brontide.go:2412: Peer([Carol]): Sending RevokeAndAck
//        [DBG] PEER brontide.go:2412: Peer([Carol]): Sending CommitSig
//        [DBG] PEER brontide.go:2412: Peer([Carol]): Received RevokeAndAck
//        [DBG] HSWC link.go:3617: ChannelLink([ChanPoint: Bob=>Carol]): settle-fail-filter: count=1, filter=[0]
//        [DBG] HSWC switch.go:3001: Circuit is closing for packet...
//
// Bob receives the preimage, closes the circuit, and exchanges commit sig and
// revoke msgs with Carol. Once Bob receives the `CommitSig` from Carol, the
// HTLC should be removed from his `LocalCommitment` via
// `RevokeCurrentCommitment`.
//
// However, in the test where `AssertHTLCNotActive` is called, although the
// above process is finished, the `ListChannels“ still finds the HTLC. Also note
// that the RPC makes direct call to the channeldb without any locks, which
// should be fine as the struct `OpenChannel.LocalCommitment` is passed by
// value, although we need to double check.
//
// TODO(yy): In order to fix it, we should make the RPC share the same view of
// our channel state machine. Instead of making DB queries, it should instead
// use `lnwallet.LightningChannel` instead to stay consistent.
//
//nolint:ll
func flakeInconsistentHTLCView() {
        // Perform a sleep so the commiment dance can be finished before we call
        // the ListChannels.
        time.Sleep(2 * time.Second)
}

// flakePaymentStreamReturnEarly documents a flake found in the test which
// relies on a given payment to be settled before testing other state changes.
// The issue comes from the payment stream created from the RPC `SendPaymentV2`
// gives premature settled event for a given payment, which is found in,
//   - if we force close the channel immediately, we may get an error because
//     the commitment dance is not finished.
//   - if we subscribe HTLC events immediately, we may get extra events, which
//     is also related to the above unfinished commitment dance.
//
// TODO(yy): Make sure we only mark the payment being settled once the
// commitment dance is finished. In addition, we should also fix the exit hop
// logic in the invoice settlement flow to make sure the invoice is only marked
// as settled after the commitment dance is finished.
func flakePaymentStreamReturnEarly() {
        // Sleep 2 seconds so the pending HTLCs will be removed from the
        // commitment.
        time.Sleep(2 * time.Second)
}

1	package itest
2
3	import (
4	"runtime"
5	"time"
6
7	"github.com/btcsuite/btcd/btcutil"
8	"github.com/lightningnetwork/lnd/lntest"
9	"github.com/lightningnetwork/lnd/lntest/node"
10	)
11
12	// flakePreimageSettlement documents a flake found when testing the preimage
13	// extraction logic in a force close. The scenario is,
14	// - Alice and Bob have a channel.
15	// - Alice sends an HTLC to Bob, and Bob won't settle it.
16	// - Alice goes offline.
17	// - Bob force closes the channel and claims the HTLC using the preimage using
18	// a sweeping tx.
19	//
20	// TODO(yy): Expose blockbeat to the link layer so the preimage extraction
21	// happens in the same block where it's spent.
22	func flakePreimageSettlement(ht *lntest.HarnessTest) {	×
23	// Mine a block to trigger the sweep. This is needed because the	×
24	// preimage extraction logic from the link is not managed by the	×
25	// blockbeat, which means the preimage may be sent to the contest	×
26	// resolver after it's launched, which means Bob would miss the block to	×
27	// launch the resolver.	×
28	ht.MineEmptyBlocks(1)	×
29		×
30	// Sleep for 2 seconds, which is needed to make sure the mempool has the	×
31	// correct tx. The above mined block can cause an RBF, if the preimage	×
32	// extraction has already been finished before the block is mined. This	×
33	// means Bob would have created the sweeping tx - mining another block	×
34	// would cause the sweeper to RBF it.	×
35	time.Sleep(2 * time.Second)	×
36	}	×
37
38	// flakeFundExtraUTXO documents a flake found when testing the sweeping behavior
39	// of the given node, which would fail due to no wallet UTXO available, while
40	// there should be enough.
41	//
42	// TODO(yy): remove it once the issue is resolved.
43	func flakeFundExtraUTXO(ht lntest.HarnessTest, node node.HarnessNode) {	×
44	// The node should have enough wallet UTXOs here to sweep the HTLC in	×
45	// the end of this test. However, due to a known issue, the node's	×
46	// wallet may report there's no UTXO available. For details,	×
47	// - https://github.com/lightningnetwork/lnd/issues/8786	×
48	ht.FundCoins(btcutil.SatoshiPerBitcoin, node)	×
49	}	×
50
51	// flakeTxNotifierNeutrino documents a flake found when running force close
52	// tests using neutrino backend, which is a race between two notifications - one
53	// for the spending notification, the other for the block which contains the
54	// spending tx.
55	//
56	// TODO(yy): remove it once the issue is resolved.
57	func flakeTxNotifierNeutrino(ht *lntest.HarnessTest) {	×
58	// Mine an empty block the for neutrino backend. We need this step to	×
59	// trigger Bob's chain watcher to detect the force close tx. Deep down,	×
60	// this happens because the notification system for neutrino is very	×
61	// different from others. Specifically, when a block contains the force	×
62	// close tx is notified, these two calls,	×
63	// - RegisterBlockEpochNtfn, will notify the block first.	×
64	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to	×
65	// the block, then perform a GetUtxo, which, by the time the spend	×
66	// details are sent, the blockbeat is considered processed in Bob's	×
67	// chain watcher.	×
68	//	×
69	// TODO(yy): refactor txNotifier to fix the above issue.	×
70	if ht.IsNeutrinoBackend() {	×
71	ht.MineEmptyBlocks(1)	×
72	}	×
73	}
74
75	// flakeSkipPendingSweepsCheckDarwin documents a flake found only in macOS
76	// build. When running in macOS, we might see three anchor sweeps - one from the
77	// local, one from the remote, and one from the pending remote:
78	// - the local one will be swept.
79	// - the remote one will be marked as failed due to `testmempoolaccept` check.
80	// - the pending remote one will not be attempted due to it being uneconomical
81	// since it was not used for CPFP.
82	//
83	// The anchor from the pending remote may or may not appear, which is a bug
84	// found only in macOS - when updating the commitments, the channel state
85	// machine somehow thinks we still have a pending remote commitment, causing it
86	// to sweep the anchor from that version.
87	//
88	// TODO(yy): fix the above bug in the channel state machine.
89	func flakeSkipPendingSweepsCheckDarwin(ht *lntest.HarnessTest,
90	node *node.HarnessNode, num int) {	×
91		×
92	// Skip the assertion below if it's on macOS.	×
93	if isDarwin() {	×
94	ht.Logf("Skipped AssertNumPendingSweeps for node %s",	×
95	node.Name())	×
96		×
97	return	×
98	}	×
99
100	ht.AssertNumPendingSweeps(node, num)	×
101	}
102
103	// isDarwin returns true if the test is running on a macOS.
104	func isDarwin() bool {	×
105	return runtime.GOOS == "darwin"	×
106	}	×
107
108	// flakeInconsistentHTLCView documents a flake found that the `ListChannels` RPC
109	// can give inaccurate HTLC states, which is found when we call
110	// `AssertHTLCNotActive` after a commitment dance is finished. Suppose Carol is
111	// settling an invoice with Bob, from Bob's PoV, a typical healthy settlement
112	// flow goes like this:
113	//
114	// [DBG] PEER brontide.go:2412: Peer([Carol]): Received UpdateFulfillHTLC
115	// [DBG] HSWC switch.go:1315: Closed completed SETTLE circuit for ...
116	// [INF] HSWC switch.go:3044: Forwarded HTLC...
117	// [DBG] PEER brontide.go:2412: Peer([Carol]): Received CommitSig
118	// [DBG] PEER brontide.go:2412: Peer([Carol]): Sending RevokeAndAck
119	// [DBG] PEER brontide.go:2412: Peer([Carol]): Sending CommitSig
120	// [DBG] PEER brontide.go:2412: Peer([Carol]): Received RevokeAndAck
121	// [DBG] HSWC link.go:3617: ChannelLink([ChanPoint: Bob=>Carol]): settle-fail-filter: count=1, filter=[0]
122	// [DBG] HSWC switch.go:3001: Circuit is closing for packet...
123	//
124	// Bob receives the preimage, closes the circuit, and exchanges commit sig and
125	// revoke msgs with Carol. Once Bob receives the `CommitSig` from Carol, the
126	// HTLC should be removed from his `LocalCommitment` via
127	// `RevokeCurrentCommitment`.
128	//
129	// However, in the test where `AssertHTLCNotActive` is called, although the
130	// above process is finished, the `ListChannels“ still finds the HTLC. Also note
131	// that the RPC makes direct call to the channeldb without any locks, which
132	// should be fine as the struct `OpenChannel.LocalCommitment` is passed by
133	// value, although we need to double check.
134	//
135	// TODO(yy): In order to fix it, we should make the RPC share the same view of
136	// our channel state machine. Instead of making DB queries, it should instead
137	// use `lnwallet.LightningChannel` instead to stay consistent.
138	//
139	//nolint:ll
NEW 140	func flakeInconsistentHTLCView() {	×
NEW 141	// Perform a sleep so the commiment dance can be finished before we call	×
NEW 142	// the ListChannels.	×
NEW 143	time.Sleep(2 * time.Second)	×
NEW 144	}	×
145
146	// flakePaymentStreamReturnEarly documents a flake found in the test which
147	// relies on a given payment to be settled before testing other state changes.
148	// The issue comes from the payment stream created from the RPC `SendPaymentV2`
149	// gives premature settled event for a given payment, which is found in,
150	// - if we force close the channel immediately, we may get an error because
151	// the commitment dance is not finished.
152	// - if we subscribe HTLC events immediately, we may get extra events, which
153	// is also related to the above unfinished commitment dance.
154	//
155	// TODO(yy): Make sure we only mark the payment being settled once the
156	// commitment dance is finished. In addition, we should also fix the exit hop
157	// logic in the invoice settlement flow to make sure the invoice is only marked
158	// as settled after the commitment dance is finished.
NEW 159	func flakePaymentStreamReturnEarly() {	×
NEW 160	// Sleep 2 seconds so the pending HTLCs will be removed from the	×
NEW 161	// commitment.	×
NEW 162	time.Sleep(2 * time.Second)	×
NEW 163	}	×

lightningnetwork / lnd / 13786526560

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous