From c493f7d9dd510595895c194f42eda9a0f5c09553 Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 19 Dec 2025 00:27:20 +0100 Subject: [PATCH] itest: accept NO_ROUTE or TIMEOUT in testLocalClaimOutgoingHTLC Fix flaky test due to race between channel closure propagation and payment timeout. Both failure reasons are correct depending on timing. --- itest/lnd_multi-hop_force_close_test.go | 26 +++++++++++-------- lntest/harness_assertion.go | 34 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/itest/lnd_multi-hop_force_close_test.go b/itest/lnd_multi-hop_force_close_test.go index 034a5641b..02da2e5fd 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 544576bef..9ae425144 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 {