itest: fix flakeSkipPendingSweepsCheckDarwin

Now that we have the new RPC to assert the HTLC state, this flake should
be fixed.

Author: yyforyongyu

itest/flakes.go

@@ -1,7 +1,6 @@
 package itest
 
 import (
-	"runtime"
 	"time"
 
 	"github.com/btcsuite/btcd/btcutil"
@@ -72,39 +71,6 @@ func flakeTxNotifierNeutrino(ht *lntest.HarnessTest) {
 	}
 }
 
-// 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

itest/lnd_multi-hop_force_close_test.go

@@ -422,8 +422,7 @@ func runLocalClaimOutgoingHTLC(ht *lntest.HarnessTest,
 	if ht.IsNeutrinoBackend() {
 		numSweeps = 2
 	}
-
-	flakeSkipPendingSweepsCheckDarwin(ht, bob, numSweeps)
+	ht.AssertNumPendingSweeps(bob, numSweeps)
 
 	// We expect to see tow txns in the mempool,
 	// 1. Bob's force close tx.
@@ -753,6 +752,9 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,
 	// channel arbitrator won't go to chain.
 	carol.RPC.SettleInvoice(preimage[:])
 
+	// Carol should still have one incoming HTLC on channel Bob -> Carol.
+	ht.AssertNumActiveHtlcs(carol, 1)
+
 	// We now advance the block height to the point where Carol will force
 	// close her channel with Bob, broadcast the closing tx but keep it
 	// unconfirmed.
@@ -780,8 +782,7 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,
 	if ht.IsNeutrinoBackend() {
 		numSweeps = 2
 	}
-
-	flakeSkipPendingSweepsCheckDarwin(ht, carol, numSweeps)
+	ht.AssertNumPendingSweeps(carol, numSweeps)
 
 	// We expect to see tow txns in the mempool,
 	// 1. Carol's force close tx.
@@ -1700,6 +1701,9 @@ func runLocalClaimIncomingHTLC(ht *lntest.HarnessTest,
 	// channel arbitrator won't go to chain.
 	carol.RPC.SettleInvoice(preimage[:])
 
+	// Carol should still have one incoming HTLC on channel Bob -> Carol.
+	ht.AssertNumActiveHtlcs(carol, 1)
+
 	// We now advance the block height to the point where Carol will force
 	// close her channel with Bob, broadcast the closing tx but keep it
 	// unconfirmed.
@@ -1978,6 +1982,9 @@ func runLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest,
 	// channel arbitrator won't go to chain.
 	carol.RPC.SettleInvoice(preimage[:])
 
+	// Carol should still have one incoming HTLC on channel Bob -> Carol.
+	ht.AssertNumActiveHtlcs(carol, 1)
+
 	// We now advance the block height to the point where Carol will force
 	// close her channel with Bob, broadcast the closing tx but keep it
 	// unconfirmed.
@@ -2319,6 +2326,9 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
 	// channel arbitrator won't go to chain.
 	carol.RPC.SettleInvoice(preimage[:])
 
+	// Carol should still have one incoming HTLC on channel Bob -> Carol.
+	ht.AssertNumActiveHtlcs(carol, 1)
+
 	ht.Logf("Invoice expire height: %d, current: %d", invoiceExpiry,
 		ht.CurrentHeight())
 
@@ -2338,8 +2348,7 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
 	if ht.IsNeutrinoBackend() {
 		numSweeps = 2
 	}
-
-	flakeSkipPendingSweepsCheckDarwin(ht, carol, numSweeps)
+	ht.AssertNumPendingSweeps(carol, numSweeps)
 
 	// We should see two txns in the mempool, we now a block to confirm,
 	// - Carol's force close tx.
@@ -2560,6 +2569,9 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
 	// channel arbitrator won't go to chain.
 	carol.RPC.SettleInvoice(preimage[:])
 
+	// Carol should still have one incoming HTLC on channel Bob -> Carol.
+	ht.AssertNumActiveHtlcs(carol, 1)
+
 	ht.Logf("Invoice expire height: %d, current: %d", invoiceExpiry,
 		ht.CurrentHeight())
 
@@ -2579,8 +2591,7 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
 	if ht.IsNeutrinoBackend() {
 		numSweeps = 2
 	}
-
-	flakeSkipPendingSweepsCheckDarwin(ht, carol, numSweeps)
+	ht.AssertNumPendingSweeps(carol, numSweeps)
 
 	// We should see two txns in the mempool, we now a block to confirm,
 	// - Carol's force close tx.
@@ -2988,8 +2999,7 @@ func runHtlcAggregation(ht *lntest.HarnessTest,
 	if ht.IsNeutrinoBackend() {
 		numSweeps = 2
 	}
-
-	flakeSkipPendingSweepsCheckDarwin(ht, bob, numSweeps)
+	ht.AssertNumPendingSweeps(bob, numSweeps)
 
 	// Bob's force close tx and anchor sweeping tx should now be found in
 	// the mempool.

lntest/harness_assertion.go

@@ -1239,17 +1239,30 @@ func (h *HarnessTest) AssertNumActiveHtlcs(hn *node.HarnessNode, num int) {
 	old := hn.State.HTLC
 
 	err := wait.NoError(func() error {
+		// pendingHTLCs is used to print unacked HTLCs, if found.
+		var pendingHTLCs []string
+
 		// We require the RPC call to be succeeded and won't wait for
 		// it as it's an unexpected behavior.
 		req := &lnrpc.ListChannelsRequest{}
 		nodeChans := hn.RPC.ListChannels(req)
 
 		total := 0
 		for _, channel := range nodeChans.Channels {
-			total += len(channel.PendingHtlcs)
+			for _, htlc := range channel.PendingHtlcs {
+				if htlc.LockedIn {
+					total++
+				}
+
+				rHash := fmt.Sprintf("%x", htlc.HashLock)
+				pendingHTLCs = append(pendingHTLCs, rHash)
+			}
 		}
 		if total-old != num {
-			return errNumNotMatched(hn.Name(), "active HTLCs",
+			desc := fmt.Sprintf("active HTLCs: unacked HTLCs: %v",
+				pendingHTLCs)
+
+			return errNumNotMatched(hn.Name(), desc,
 				num, total-old, total, old)
 		}
 
@@ -1291,17 +1304,22 @@ func (h *HarnessTest) assertHTLCActive(hn *node.HarnessNode,
 
 		// Check all payment hashes active for this channel.
 		for _, htlc := range ch.PendingHtlcs {
-			h := hex.EncodeToString(htlc.HashLock)
-			if h != target {
+			rHash := hex.EncodeToString(htlc.HashLock)
+			if rHash != target {
 				continue
 			}
 
 			// If the payment hash is found, check the incoming
 			// field.
 			if htlc.Incoming == incoming {
-				// Found it and return.
-				result = htlc
-				return nil
+				// Return the result if it's locked in.
+				if htlc.LockedIn {
+					result = htlc
+					return nil
+				}
+
+				return fmt.Errorf("htlc(%x) not locked in",
+					payHash)
 			}
 
 			// Otherwise we do have the HTLC but its direction is
@@ -1311,14 +1329,13 @@ func (h *HarnessTest) assertHTLCActive(hn *node.HarnessNode,
 				have, want = "incoming", "outgoing"
 			}
 
-			return fmt.Errorf("node[%s] have htlc(%v), want: %s, "+
-				"have: %s", hn.Name(), payHash, want, have)
+			return fmt.Errorf("htlc(%x) has wrong direction - "+
+				"want: %s, have: %s", payHash, want, have)
 		}
 
-		return fmt.Errorf("node [%s:%x] didn't have: the payHash %x",
-			hn.Name(), hn.PubKey[:], payHash)
+		return fmt.Errorf("htlc not found using payHash %x", payHash)
 	}, DefaultTimeout)
-	require.NoError(h, err, "timeout checking pending HTLC")
+	require.NoError(h, err, "%s: timeout checking pending HTLC", hn.Name())
 
 	return result
 }