itest: accept NO_ROUTE or TIMEOUT in testLocalClaimOutgoingHTLC
What changed, and why it matters
This commit only changes an integration test to accept either of two valid failure reasons for a payment, fixing a flaky test. It does not change production code or fix a security vulnerability.
No security action needed; this is a test-only reliability fix.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit modifies itest/lnd_multi-hop_force_close_test.go and lntest/harness_assertion.go to add AssertPaymentFailureReasonAny, which allows a test to accept either FAILURE_REASON_NO_ROUTE or FAILURE_REASON_TIMEOUT depending on a race condition between channel closure propagation and payment timeout. The production failure-handling logic is unchanged.
Changed components
itest/lnd_multi-hop_force_close_test.golntest/harness_assertion.goInspect captured patch +50 / −10
diff --git a/itest/lnd_multi-hop_force_close_test.go b/itest/lnd_multi-hop_force_close_test.go
index 034a564..02da2e5 100644
--- a/itest/lnd_multi-hop_force_close_test.go
+++ b/itest/lnd_multi-hop_force_close_test.go
@@ -517,19 +517,25 @@ func runLocalClaimOutgoingHTLC(ht *lntest.HarnessTest,
// Now that Bob has claimed his HTLCs, Alice should mark the two
// payments as failed.
//
- // Alice will mark this payment as failed with no route as the only
- // route she has is Alice->Bob->Carol. This won't be the case if she
- // has a second route, as another attempt will be tried.
- //
- // TODO(yy): we should instead mark this payment as timed out if she has
- // a second route to try this payment, which is the timeout set by Alice
- // when sending the payment.
- expectedReason := lnrpc.PaymentFailureReason_FAILURE_REASON_NO_ROUTE
- p := ht.AssertPaymentFailureReason(alice, preimage, expectedReason)
+ // Alice's payment can fail with either NO_ROUTE or TIMEOUT depending
+ // on timing. There's a race between:
+ // 1. The channel closure propagating to Alice's graph (-> NO_ROUTE)
+ // 2. The payment attempt timeout firing (-> TIMEOUT)
+ // Both failure reasons are correct.
+ p := ht.AssertPaymentFailureReasonAny(alice, preimage,
+ lnrpc.PaymentFailureReason_FAILURE_REASON_NO_ROUTE,
+ lnrpc.PaymentFailureReason_FAILURE_REASON_TIMEOUT,
+ )
+
+ // The HTLC-level failure code should be PERMANENT_CHANNEL_FAILURE
+ // regardless of which payment-level failure reason we got.
require.Equal(ht, lnrpc.Failure_PERMANENT_CHANNEL_FAILURE,
p.Htlcs[0].Failure.Code)
- p = ht.AssertPaymentFailureReason(alice, preimageDust, expectedReason)
+ p = ht.AssertPaymentFailureReasonAny(alice, preimageDust,
+ lnrpc.PaymentFailureReason_FAILURE_REASON_NO_ROUTE,
+ lnrpc.PaymentFailureReason_FAILURE_REASON_TIMEOUT,
+ )
require.Equal(ht, lnrpc.Failure_PERMANENT_CHANNEL_FAILURE,
p.Htlcs[0].Failure.Code)
}
diff --git a/lntest/harness_assertion.go b/lntest/harness_assertion.go
index 544576b..9ae4251 100644
--- a/lntest/harness_assertion.go
+++ b/lntest/harness_assertion.go
@@ -1648,6 +1648,40 @@ func (h *HarnessTest) AssertPaymentFailureReason(
return payment
}
+// AssertPaymentFailureReasonAny asserts that the given node lists a payment
+// with the given preimage which has one of the expected failure reasons.
+func (h *HarnessTest) AssertPaymentFailureReasonAny(
+ hn *node.HarnessNode, preimage lntypes.Preimage,
+ reasons ...lnrpc.PaymentFailureReason) *lnrpc.Payment {
+
+ var payment *lnrpc.Payment
+
+ payHash := preimage.Hash()
+ err := wait.NoError(func() error {
+ p, err := h.findPayment(hn, payHash.String())
+ if err != nil {
+ return err
+ }
+
+ payment = p
+
+ // Check if the payment failure reason matches any of the
+ // expected reasons.
+ for _, reason := range reasons {
+ if reason == p.FailureReason {
+ return nil
+ }
+ }
+
+ return fmt.Errorf("payment: %v failure reason not match, "+
+ "want one of %v, got %s(%d)", payHash, reasons,
+ p.FailureReason, p.FailureReason)
+ }, DefaultTimeout)
+ require.NoError(h, err, "timeout checking payment failure reason")
+
+ return payment
+}
+
// AssertActiveNodesSynced asserts all active nodes have synced to the chain.
func (h *HarnessTest) AssertActiveNodesSynced() {
for _, node := range h.manager.activeNodes {
Why this scored 15/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.