itest+lntest: add flakePaymentStreamReturnEarly

Fix the flake found in the `testRelayingBlindedError` and documents the
flake found in other tests in one place.

Author: yyforyongyu

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

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

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()

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)

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)