From dd1d57d82de155871ccd13892ca9a299f213044a Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 7 Aug 2025 16:35:45 +0200 Subject: [PATCH] routing: make sure attempts are always resolved after a timeout We check the context of the payment lifecycle at the beginning of the `resumepayment` loop. This will make sure we have always the latest state of the payment before deciding on the next steps in the function `decideNextStep`. --- routing/payment_lifecycle.go | 29 +++++++++++++++++------------ routing/payment_lifecycle_test.go | 17 ++++------------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 7a6443ab6..a42b21de5 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -226,6 +226,19 @@ func (p *paymentLifecycle) resumePayment(ctx context.Context) ([32]byte, // critical error during path finding. lifecycle: for { + // Before we attempt any new shard, we'll check to see if we've + // gone past the payment attempt timeout or if the context was + // canceled. If the context is done, the payment is marked as + // failed and we reload the latest payment state to reflect + // this. + // + // NOTE: This can be called several times if there are more + // attempts to be resolved after the timeout or context is + // cancelled. + if err := p.checkContext(ctx); err != nil { + return exitWithErr(err) + } + // We update the payment state on every iteration. currentPayment, ps, err := p.reloadPayment() if err != nil { @@ -241,19 +254,11 @@ lifecycle: // We now proceed our lifecycle with the following tasks in // order, - // 1. check context. - // 2. request route. - // 3. create HTLC attempt. - // 4. send HTLC attempt. - // 5. collect HTLC attempt result. + // 1. request route. + // 2. create HTLC attempt. + // 3. send HTLC attempt. + // 4. collect HTLC attempt result. // - // Before we attempt any new shard, we'll check to see if we've - // gone past the payment attempt timeout, or if the context was - // cancelled, or the router is exiting. In any of these cases, - // we'll stop this payment attempt short. - if err := p.checkContext(ctx); err != nil { - return exitWithErr(err) - } // Now decide the next step of the current lifecycle. step, err := p.decideNextStep(payment) diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index 0ee751196..a4f455039 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -868,25 +868,16 @@ func TestResumePaymentFailOnTimeoutErr(t *testing.T) { // Create a test paymentLifecycle with the initial two calls mocked. p, m := setupTestPaymentLifecycle(t) - paymentAmt := lnwire.MilliSatoshi(10000) - - // We now enter the payment lifecycle loop. - // - // 1. calls `FetchPayment` and return the payment. - m.control.On("FetchPayment", p.identifier).Return(m.payment, nil).Once() - - // 2. calls `GetState` and return the state. - ps := &channeldb.MPPaymentState{ - RemainingAmt: paymentAmt, - } - m.payment.On("GetState").Return(ps).Once() + // We now enter the payment lifecycle loop, we will check the router + // quit channel in the beginning and quit immediately without reloading + // the payment. // NOTE: GetStatus is only used to populate the logs which is // not critical so we loosen the checks on how many times it's // been called. m.payment.On("GetStatus").Return(channeldb.StatusInFlight) - // 3. quit the router to return an error. + // Quit the router to return an error. close(p.router.quit) // Send the payment and assert it failed when router is shutting down.