lntest: make sure HTLCs are locked in when sending a payment
What the developer wrote
Authored by ziggie
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.
What changed, and why it matters
This commit fixes a timing bug in LND's internal test helper. The helper previously returned too early when sending test payments, before all payment commitments were fully recorded on the channel. This caused occasional flaky test failures because tests captured channel state at the wrong moment. The fix makes the helper wait until every sent payment appears as a pending HTLC. There is no indication this affects real user funds or production code paths.
No security action required. Treat as a normal test reliability improvement. Reviewers may optionally verify that the new PendingHtlcs-based wait condition is robust against HTLCs that settle or fail before the count is checked, though the commit message indicates the intended use is for in-flight HTLCs.
Security signals we found
Race condition in test synchronization helper
No production code path modified
No cryptographic, network, or consensus logic changed
Fixes flaky test revokedCloseRetributionRemoteHodlCase
Evidence from the diff
CompletePaymentRequestsNoWait in lntest/harness.go previously used NumUpdates growth as a proxy for payment commitment completion. Because NumUpdates can increment before every outgoing HTLC is fully locked in, tests that immediately snapshot channel state could observe a later commitment height than expected. The patch replaces the NumUpdates check with a count of outgoing PendingHtlcs, waiting until exactly len(paymentRequests) new outgoing HTLCs are present. This is a test-only synchronization fix; no production payment logic is changed.
Changed components
lntest/harness.goCompletePaymentRequestsNoWait test helperInspect captured patch +35 / −15
diff --git a/lntest/harness.go b/lntest/harness.go
index 21c32a5..4b50257 100644
--- a/lntest/harness.go
+++ b/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,50 @@ func (h *HarnessTest) CompletePaymentRequestsNoWait(hn *node.HarnessNode,
// we return.
oldResp := h.GetChannelByChanPoint(hn, chanPoint)
+ // countOutgoing counts the number of outgoing HTLCs in the given list.
+ countOutgoing := func(htlcs []*lnrpc.HTLC) int {
+ count := 0
+ for _, htlc := range htlcs {
+ if !htlc.Incoming {
+ count++
+ }
+ }
+
+ return count
+ }
+
+ // Count existing outgoing HTLCs before sending.
+ oldOutgoingCount := countOutgoing(oldResp.PendingHtlcs)
+
+ 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 := countOutgoing(newResp.PendingHtlcs)
+
+ 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
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.