13923358635

Committed 18 Mar 2025 12:36PM UTC coverage: 68.632%. First build

Build # 13923358635

Build Type

Pull #9609

github

Committed by

web-flow

Commit Message

Merge d3888f2b2 into bcc80e7f9

Pull Request Pull Request #9609: Fix inaccurate `listunspent` result

Run Details

30 of 40 new or added lines in 8 files covered. (75.0%)

130352 of 189928 relevant lines covered (68.63%)

23560.07 hits per line

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

0.0

/itest/flakes.go

package itest

import (
        "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)
}

// 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)
        }
}

// 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)
}

// flakeRaceInBitcoinClientNotifications documents a bug found that the
// `ListUnspent` gives inaccurate results. In specific,
//   - an output is confirmed in block X, which is under the process of being
//     credited to our wallet.
//   - `ListUnspent` is called inbetween the above process, returning an
//     inaccurate result, causing the sweeper to think there's no wallet utxo.
//   - the sweeping will fail at block X due to not enough inputs.
//
// Under the hood, the RPC client created for handling wallet txns and handling
// block notifications are independent. For the block notification, which is
// registered via `RegisterBlockEpochNtfn`, is managed by the `chainntnfs`,
// which is hooked to a bitcoind client created at startup. For the wallet, it
// uses another bitcoind client to receive online events. Although they share
// the same bitcoind RPC conn, these two clients are act independently.
// With this setup, it means there's no coordination between the two system -
// `lnwallet` and `chainntnfs` can disagree on the latest onchain state for a
// short period, causing an inconsistent state which leads to the failed
// sweeping attempt.
//
// TODO(yy): We need to adhere to the SSOT principle, and make the effort to
// ensure the whole system only uses one bitcoind client.
func flakeRaceInBitcoinClientNotifications(ht *lntest.HarnessTest,
        hn *node.HarnessNode) {

        ht.FundCoins(btcutil.SatoshiPerBitcoin, hn)
}

1	package itest
2
3	import (
4	"time"
5
6	"github.com/btcsuite/btcd/btcutil"
7	"github.com/lightningnetwork/lnd/lntest"
8	"github.com/lightningnetwork/lnd/lntest/node"
9	)
10
11	// flakePreimageSettlement documents a flake found when testing the preimage
12	// extraction logic in a force close. The scenario is,
13	// - Alice and Bob have a channel.
14	// - Alice sends an HTLC to Bob, and Bob won't settle it.
15	// - Alice goes offline.
16	// - Bob force closes the channel and claims the HTLC using the preimage using
17	// a sweeping tx.
18	//
19	// TODO(yy): Expose blockbeat to the link layer so the preimage extraction
20	// happens in the same block where it's spent.
21	func flakePreimageSettlement(ht *lntest.HarnessTest) {	×
22	// Mine a block to trigger the sweep. This is needed because the	×
23	// preimage extraction logic from the link is not managed by the	×
24	// blockbeat, which means the preimage may be sent to the contest	×
25	// resolver after it's launched, which means Bob would miss the block to	×
26	// launch the resolver.	×
27	ht.MineEmptyBlocks(1)	×
28		×
29	// Sleep for 2 seconds, which is needed to make sure the mempool has the	×
30	// correct tx. The above mined block can cause an RBF, if the preimage	×
31	// extraction has already been finished before the block is mined. This	×
32	// means Bob would have created the sweeping tx - mining another block	×
33	// would cause the sweeper to RBF it.	×
34	time.Sleep(2 * time.Second)	×
35	}	×
36
37	// flakeTxNotifierNeutrino documents a flake found when running force close
38	// tests using neutrino backend, which is a race between two notifications - one
39	// for the spending notification, the other for the block which contains the
40	// spending tx.
41	//
42	// TODO(yy): remove it once the issue is resolved.
43	func flakeTxNotifierNeutrino(ht *lntest.HarnessTest) {	×
44	// Mine an empty block the for neutrino backend. We need this step to	×
45	// trigger Bob's chain watcher to detect the force close tx. Deep down,	×
46	// this happens because the notification system for neutrino is very	×
47	// different from others. Specifically, when a block contains the force	×
48	// close tx is notified, these two calls,	×
49	// - RegisterBlockEpochNtfn, will notify the block first.	×
50	// - RegisterSpendNtfn, will wait for the neutrino notifier to sync to	×
51	// the block, then perform a GetUtxo, which, by the time the spend	×
52	// details are sent, the blockbeat is considered processed in Bob's	×
53	// chain watcher.	×
54	//	×
55	// TODO(yy): refactor txNotifier to fix the above issue.	×
56	if ht.IsNeutrinoBackend() {	×
57	ht.MineEmptyBlocks(1)	×
58	}	×
59	}
60
61	// flakeInconsistentHTLCView documents a flake found that the `ListChannels` RPC
62	// can give inaccurate HTLC states, which is found when we call
63	// `AssertHTLCNotActive` after a commitment dance is finished. Suppose Carol is
64	// settling an invoice with Bob, from Bob's PoV, a typical healthy settlement
65	// flow goes like this:
66	//
67	// [DBG] PEER brontide.go:2412: Peer([Carol]): Received UpdateFulfillHTLC
68	// [DBG] HSWC switch.go:1315: Closed completed SETTLE circuit for ...
69	// [INF] HSWC switch.go:3044: Forwarded HTLC...
70	// [DBG] PEER brontide.go:2412: Peer([Carol]): Received CommitSig
71	// [DBG] PEER brontide.go:2412: Peer([Carol]): Sending RevokeAndAck
72	// [DBG] PEER brontide.go:2412: Peer([Carol]): Sending CommitSig
73	// [DBG] PEER brontide.go:2412: Peer([Carol]): Received RevokeAndAck
74	// [DBG] HSWC link.go:3617: ChannelLink([ChanPoint: Bob=>Carol]): settle-fail-filter: count=1, filter=[0]
75	// [DBG] HSWC switch.go:3001: Circuit is closing for packet...
76	//
77	// Bob receives the preimage, closes the circuit, and exchanges commit sig and
78	// revoke msgs with Carol. Once Bob receives the `CommitSig` from Carol, the
79	// HTLC should be removed from his `LocalCommitment` via
80	// `RevokeCurrentCommitment`.
81	//
82	// However, in the test where `AssertHTLCNotActive` is called, although the
83	// above process is finished, the `ListChannels“ still finds the HTLC. Also note
84	// that the RPC makes direct call to the channeldb without any locks, which
85	// should be fine as the struct `OpenChannel.LocalCommitment` is passed by
86	// value, although we need to double check.
87	//
88	// TODO(yy): In order to fix it, we should make the RPC share the same view of
89	// our channel state machine. Instead of making DB queries, it should instead
90	// use `lnwallet.LightningChannel` instead to stay consistent.
91	//
92	//nolint:ll
93	func flakeInconsistentHTLCView() {	×
94	// Perform a sleep so the commiment dance can be finished before we call	×
95	// the ListChannels.	×
96	time.Sleep(2 * time.Second)	×
97	}	×
98
99	// flakePaymentStreamReturnEarly documents a flake found in the test which
100	// relies on a given payment to be settled before testing other state changes.
101	// The issue comes from the payment stream created from the RPC `SendPaymentV2`
102	// gives premature settled event for a given payment, which is found in,
103	// - if we force close the channel immediately, we may get an error because
104	// the commitment dance is not finished.
105	// - if we subscribe HTLC events immediately, we may get extra events, which
106	// is also related to the above unfinished commitment dance.
107	//
108	// TODO(yy): Make sure we only mark the payment being settled once the
109	// commitment dance is finished. In addition, we should also fix the exit hop
110	// logic in the invoice settlement flow to make sure the invoice is only marked
111	// as settled after the commitment dance is finished.
112	func flakePaymentStreamReturnEarly() {	×
113	// Sleep 2 seconds so the pending HTLCs will be removed from the	×
114	// commitment.	×
115	time.Sleep(2 * time.Second)	×
116	}	×
117
118	// flakeRaceInBitcoinClientNotifications documents a bug found that the
119	// `ListUnspent` gives inaccurate results. In specific,
120	// - an output is confirmed in block X, which is under the process of being
121	// credited to our wallet.
122	// - `ListUnspent` is called inbetween the above process, returning an
123	// inaccurate result, causing the sweeper to think there's no wallet utxo.
124	// - the sweeping will fail at block X due to not enough inputs.
125	//
126	// Under the hood, the RPC client created for handling wallet txns and handling
127	// block notifications are independent. For the block notification, which is
128	// registered via `RegisterBlockEpochNtfn`, is managed by the `chainntnfs`,
129	// which is hooked to a bitcoind client created at startup. For the wallet, it
130	// uses another bitcoind client to receive online events. Although they share
131	// the same bitcoind RPC conn, these two clients are act independently.
132	// With this setup, it means there's no coordination between the two system -
133	// `lnwallet` and `chainntnfs` can disagree on the latest onchain state for a
134	// short period, causing an inconsistent state which leads to the failed
135	// sweeping attempt.
136	//
137	// TODO(yy): We need to adhere to the SSOT principle, and make the effort to
138	// ensure the whole system only uses one bitcoind client.
139	func flakeRaceInBitcoinClientNotifications(ht *lntest.HarnessTest,
NEW 140	hn *node.HarnessNode) {	×
NEW 141		×
NEW 142	ht.FundCoins(btcutil.SatoshiPerBitcoin, hn)	×
NEW 143	}	×

lightningnetwork / lnd / 13923358635

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