lntest: make sure HTLCs are locked in when sending a payment

Before this change, CompletePaymentRequestsNoWait would return as
soon as the channel's NumUpdates increased by at least one. When
sending multiple payments, this meant the function could return
while some HTLCs were still in-flight and not yet committed to the
channel state.

The problem occurred when tests captured the channel state
immediately after calling this function. Even though we read the
current NumUpdates from the channel, HTLCs could still be in the
process of being committed. This led to a race where the channel
would progress to a new state after we thought we had correctly
captured it, causing tests to see unexpected commitment heights.

Fix this by waiting for all outgoing HTLCs to appear in
PendingHtlcs before returning. We count outgoing HTLCs before
sending, then wait until exactly len(paymentRequests) new HTLCs
are present. This guarantees all payments have fully completed
their commitment exchange and are locked in on both sides.

Fixes the flaky revokedCloseRetributionRemoteHodlCase test where
backups would capture state at height N+1 instead of the expected
  height N.

Author: ziggie1984

lntest/harness.go

@@ -1619,8 +1619,9 @@ func (h *HarnessTest) CompletePaymentRequests(hn *node.HarnessNode,
 }
 
 // CompletePaymentRequestsNoWait sends payments from a node to complete all
-// payment requests without waiting for the results. Instead, it checks the
-// number of updates in the specified channel has increased.
+// payment requests without waiting for the results. Instead, it waits for
+// all HTLCs to be locked in on the sender's channel by checking the number
+// of pending HTLCs.
 func (h *HarnessTest) CompletePaymentRequestsNoWait(hn *node.HarnessNode,
 	paymentRequests []string, chanPoint *lnrpc.ChannelPoint) {
 
@@ -1629,31 +1630,48 @@ func (h *HarnessTest) CompletePaymentRequestsNoWait(hn *node.HarnessNode,
 	// we return.
 	oldResp := h.GetChannelByChanPoint(hn, chanPoint)
 
+	// Count existing outgoing HTLCs before sending.
+	oldOutgoingCount := 0
+	for _, htlc := range oldResp.PendingHtlcs {
+		if !htlc.Incoming {
+			oldOutgoingCount++
+		}
+	}
+
+	numPayments := len(paymentRequests)
+
 	// Send payments and assert they are in-flight.
 	h.completePaymentRequestsAssertStatus(
 		hn, paymentRequests, lnrpc.Payment_IN_FLIGHT,
 	)
 
-	// We are not waiting for feedback in the form of a response, but we
-	// should still wait long enough for the server to receive and handle
-	// the send before cancelling the request. We wait for the number of
-	// updates to one of our channels has increased before we return.
+	// Wait for all HTLCs to be locked in. We check that the number of
+	// outgoing pending HTLCs has increased by exactly the number of
+	// payments sent. This ensures all HTLCs are committed on the sender's
+	// side.
 	err := wait.NoError(func() error {
 		newResp := h.GetChannelByChanPoint(hn, chanPoint)
 
-		// If this channel has an increased number of updates, we
-		// assume the payments are committed, and we can return.
-		if newResp.NumUpdates > oldResp.NumUpdates {
+		// Count current outgoing HTLCs.
+		newOutgoingCount := 0
+		for _, htlc := range newResp.PendingHtlcs {
+			if !htlc.Incoming {
+				newOutgoingCount++
+			}
+		}
+
+		htlcsAdded := newOutgoingCount - oldOutgoingCount
+
+		// Verify all HTLCs are locked in.
+		if htlcsAdded == numPayments {
 			return nil
 		}
 
-		// Otherwise return an error as the NumUpdates are not
-		// increased.
-		return fmt.Errorf("%s: channel:%v not updated after sending "+
-			"payments, old updates: %v, new updates: %v", hn.Name(),
-			chanPoint, oldResp.NumUpdates, newResp.NumUpdates)
+		return fmt.Errorf("%s: channel:%v waiting for HTLCs, "+
+			"added: %d/%d", hn.Name(), chanPoint,
+			htlcsAdded, numPayments)
 	}, DefaultTimeout)
-	require.NoError(h, err, "timeout while checking for channel updates")
+	require.NoError(h, err, "timeout while waiting for HTLCs to lock in")
 }
 
 // OpenChannelPsbt attempts to open a channel between srcNode and destNode with