From 2ca3eea3f7f175e9557d03908271febc013d397a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 29 Oct 2021 09:50:27 +0200 Subject: [PATCH 1/4] itest: fix REST WebSocket ping/pong itest This commit fixes a flake in the REST itest that was caused by an invoice being created before the actual subscription client was registered. --- lntest/itest/lnd_rest_api_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lntest/itest/lnd_rest_api_test.go b/lntest/itest/lnd_rest_api_test.go index 290b98171..c02e7da4c 100644 --- a/lntest/itest/lnd_rest_api_test.go +++ b/lntest/itest/lnd_rest_api_test.go @@ -638,6 +638,16 @@ func wsTestPingPongTimeout(ht *harnessTest, net *lntest.NetworkHarness) { } }() + // The SubscribeInvoices call returns immediately after the gRPC/REST + // connection is established. But it can happen that the goroutine in + // lnd that actually registers the subscriber in the invoice backend + // didn't get any CPU time just yet. So we can run into the situation + // where we add our first invoice _before_ the subscription client is + // registered. If that happens, we'll never get notified about the + // invoice in question. So all we really can do is wait a bit here to + // make sure the subscription is registered correctly. + time.Sleep(500 * time.Millisecond) + // Let's create five invoices and wait for them to arrive. We'll wait // for at least one ping/pong cycle between each invoice. ctxb := context.Background() From 2a6f128a43fa67eb3ffdf6ea965f34fd9f0106df Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 29 Oct 2021 11:18:37 +0200 Subject: [PATCH 2/4] itest: wait for channel to be properly closed It looks like in some cases a channel is still being closed while we already try to create a sweep output. In that case the pending closed channel is still counted as anchor channel and a reserve output is created. To make sure that doesn't happen, we make sure there are no pending or open channels of any kind around before we create the sweep transaction. --- lntest/itest/lnd_onchain_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lntest/itest/lnd_onchain_test.go b/lntest/itest/lnd_onchain_test.go index da4af2fa0..cfae6921d 100644 --- a/lntest/itest/lnd_onchain_test.go +++ b/lntest/itest/lnd_onchain_test.go @@ -209,8 +209,7 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) { // Alice opens a smaller channel. This works since it will have a // change output. aliceChanPoint1 := openChannelAndAssert( - t, net, alice, bob, - lntest.OpenChannelParams{ + t, net, alice, bob, lntest.OpenChannelParams{ Amt: chanAmt / 4, }, ) @@ -326,7 +325,7 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) { block = mineBlocks(t, net, 1, 1)[0] // The sweep transaction should have exactly one inputs as we only had - // the the single output from above in the wallet. + // the single output from above in the wallet. sweepTx = block.Transactions[1] if len(sweepTx.TxIn) != 1 { t.Fatalf("expected 1 inputs instead have %v", len(sweepTx.TxIn)) @@ -352,9 +351,12 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("Alice's balance did not increase after channel close") } + // Assert there are no open or pending channels anymore. + assertNumPendingChannels(t, alice, 0, 0) + assertNodeNumChannels(t, alice, 0) + // We'll wait for the balance to reflect that the channel has been // closed and the funds are in the wallet. - sweepReq = &lnrpc.SendCoinsRequest{ Addr: minerAddr.String(), SendAll: true, From 066a83315ea48f139557c431dad6b0a9b05f8fac Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 29 Oct 2021 11:24:45 +0200 Subject: [PATCH 3/4] docs: update release notes --- docs/release-notes/release-notes-0.14.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index a4fa77e4c..df7992ced 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -462,6 +462,9 @@ messages directly. There is no routing/path finding involved. * [Run channeldb tests on postgres](https://github.com/lightningnetwork/lnd/pull/5550) +* [Fixed two flakes in itests that were caused by things progressing too + fast](https://github.com/lightningnetwork/lnd/pull/5905). + ## Database * [Ensure single writer for legacy From eb2be7c7c0a6fba536a615202807219d51f95fa5 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 29 Oct 2021 14:18:08 +0200 Subject: [PATCH 4/4] lntest: count unconfirmed balance as well It looks like in some cases (apparently mostly on Windows) a previous sub test sometimes leaves some unconfirmed balance in the wallet. That balance is confirmed in the next test when new coins are sent to the wallet. --- lntest/harness.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lntest/harness.go b/lntest/harness.go index 3cba2a013..5d96efac8 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -1615,7 +1615,9 @@ func (n *NetworkHarness) sendCoins(amt btcutil.Amount, target *HarnessNode, return err } - expectedBalance := btcutil.Amount(initialBalance.ConfirmedBalance) + amt + fullInitialBalance := initialBalance.ConfirmedBalance + + initialBalance.UnconfirmedBalance + expectedBalance := btcutil.Amount(fullInitialBalance) + amt return target.WaitForBalance(expectedBalance, true) }