From 652d39dcc74191413d3ac14f7a98d1d0843f7411 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 6 Mar 2025 19:26:10 +0800 Subject: [PATCH] itest: fix flakeSkipPendingSweepsCheckDarwin Now that we have the new RPC to assert the HTLC state, this flake should be fixed. --- itest/flakes.go | 34 -------------------- itest/lnd_multi-hop_force_close_test.go | 30 ++++++++++++------ lntest/harness_assertion.go | 41 +++++++++++++++++-------- 3 files changed, 49 insertions(+), 56 deletions(-) diff --git a/itest/flakes.go b/itest/flakes.go index 23666cb6b..68c36a738 100644 --- a/itest/flakes.go +++ b/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 diff --git a/itest/lnd_multi-hop_force_close_test.go b/itest/lnd_multi-hop_force_close_test.go index fda8c054a..9907f6a59 100644 --- a/itest/lnd_multi-hop_force_close_test.go +++ b/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. diff --git a/lntest/harness_assertion.go b/lntest/harness_assertion.go index da823c841..1b195a0a9 100644 --- a/lntest/harness_assertion.go +++ b/lntest/harness_assertion.go @@ -1239,6 +1239,9 @@ 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{} @@ -1246,10 +1249,20 @@ func (h *HarnessTest) AssertNumActiveHtlcs(hn *node.HarnessNode, num int) { 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 }