From f526c771beff5d8342a4d936163515a96a39d746 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 10 Mar 2025 01:42:58 +0800 Subject: [PATCH] itest+lntest: add `flakePaymentStreamReturnEarly` Fix the flake found in the `testRelayingBlindedError` and documents the flake found in other tests in one place. --- itest/flakes.go | 19 +++++++++++++++++++ itest/lnd_channel_backup_test.go | 8 +------- itest/lnd_route_blinding_test.go | 4 ++++ itest/lnd_wipe_fwdpkgs_test.go | 10 +--------- lntest/harness_assertion.go | 3 ++- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/itest/flakes.go b/itest/flakes.go index ff5db12a1..23666cb6b 100644 --- a/itest/flakes.go +++ b/itest/flakes.go @@ -142,3 +142,22 @@ func flakeInconsistentHTLCView() { // 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) +} diff --git a/itest/lnd_channel_backup_test.go b/itest/lnd_channel_backup_test.go index 8b12f4fdc..f84b65ce3 100644 --- a/itest/lnd_channel_backup_test.go +++ b/itest/lnd_channel_backup_test.go @@ -1224,13 +1224,7 @@ func testDataLossProtection(ht *lntest.HarnessTest) { // payment hashes. ht.CompletePaymentRequests(carol, payReqs[numInvoices/2:]) - // TODO(yy): remove the sleep once the following bug is fixed. - // - // While the payment is reported as settled, the commitment - // dance may not be finished, which leaves several HTLCs in the - // commitment. Later on, when Carol force closes this channel, - // she would have HTLCs there and the test won't pass. - time.Sleep(2 * time.Second) + flakePaymentStreamReturnEarly() // Now we shutdown Dave, copying over the its temporary // database state which has the *prior* channel state over his diff --git a/itest/lnd_route_blinding_test.go b/itest/lnd_route_blinding_test.go index 9792b0ca2..d46a84c16 100644 --- a/itest/lnd_route_blinding_test.go +++ b/itest/lnd_route_blinding_test.go @@ -667,6 +667,10 @@ func testRelayingBlindedError(ht *lntest.HarnessTest) { // so that she can't forward the payment to Dave. testCase.drainCarolLiquidity(false) + // NOTE: The above draining requires Carol to send a payment, which may + // create extra events, causing the `AssertHtlcEvents` to fail below. + flakePaymentStreamReturnEarly() + // Subscribe to Carol's HTLC events so that we can observe the payment // coming in. carolEvents := testCase.carol.RPC.SubscribeHtlcEvents() diff --git a/itest/lnd_wipe_fwdpkgs_test.go b/itest/lnd_wipe_fwdpkgs_test.go index 9aafd960a..337331fd5 100644 --- a/itest/lnd_wipe_fwdpkgs_test.go +++ b/itest/lnd_wipe_fwdpkgs_test.go @@ -1,8 +1,6 @@ package itest import ( - "time" - "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" @@ -49,13 +47,7 @@ func testWipeForwardingPackages(ht *lntest.HarnessTest) { ht.CompletePaymentRequests(alice, []string{resp.PaymentRequest}) } - // 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) + flakePaymentStreamReturnEarly() // Firstly, Bob force closes the channel. ht.CloseChannelAssertPending(bob, chanPointAB, true) diff --git a/lntest/harness_assertion.go b/lntest/harness_assertion.go index 0523562b6..ddbc4a325 100644 --- a/lntest/harness_assertion.go +++ b/lntest/harness_assertion.go @@ -2329,7 +2329,8 @@ func (h *HarnessTest) AssertHtlcEvents(client rpc.HtlcEventsClient, event := h.ReceiveHtlcEvent(client) require.Containsf(h, eventTypes, event.EventType, - "wrong event type, got %v", userType, event.EventType) + "wrong event type, want %v, got %v", userType, + event.EventType) events = append(events, event)