From 635a67c94a18a76139705394d0c0f066142d6f01 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 21 Oct 2025 10:11:32 +0200 Subject: [PATCH] multi: thread context through Fail payment functions --- payments/db/interface.go | 2 +- payments/db/kv_store.go | 2 +- payments/db/kv_store_test.go | 2 +- payments/db/payment_test.go | 13 +++++++------ payments/db/sql_store.go | 4 +--- routing/control_tower.go | 9 +++++---- routing/control_tower_test.go | 3 ++- routing/mock_test.go | 4 ++-- routing/payment_lifecycle.go | 21 ++++++++++++++++++--- routing/router.go | 4 +++- 10 files changed, 41 insertions(+), 23 deletions(-) diff --git a/payments/db/interface.go b/payments/db/interface.go index 45d0e9a8a..2d2c47b3b 100644 --- a/payments/db/interface.go +++ b/payments/db/interface.go @@ -99,7 +99,7 @@ type PaymentControl interface { // invoking this method, InitPayment should return nil on its next call // for this payment hash, allowing the user to make a subsequent // payment. - Fail(lntypes.Hash, FailureReason) (*MPPayment, error) + Fail(context.Context, lntypes.Hash, FailureReason) (*MPPayment, error) // DeleteFailedAttempts removes all failed HTLCs from the db. It should // be called for a given payment whenever all inflight htlcs are diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index 59fe24f36..285074b51 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -528,7 +528,7 @@ func (p *KVStore) updateHtlcKey(paymentHash lntypes.Hash, // payment failed. After invoking this method, InitPayment should return nil on // its next call for this payment hash, allowing the switch to make a // subsequent payment. -func (p *KVStore) Fail(paymentHash lntypes.Hash, +func (p *KVStore) Fail(_ context.Context, paymentHash lntypes.Hash, reason FailureReason) (*MPPayment, error) { var ( diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index ee8412a6f..de3fc4ad2 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -112,7 +112,7 @@ func TestKVStoreDeleteNonInFlight(t *testing.T) { // Fail the payment, which should moved it to Failed. failReason := FailureReasonNoRoute _, err = paymentDB.Fail( - info.PaymentIdentifier, failReason, + ctx, info.PaymentIdentifier, failReason, ) if err != nil { t.Fatalf("unable to fail payment hash: %v", err) diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index 879dfb124..5f1ab69c1 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -191,8 +191,9 @@ func createTestPayments(t *testing.T, p DB, payments []*payment) { require.NoError(t, err, "unable to fail htlc") failReason := FailureReasonNoRoute - _, err = p.Fail(info.PaymentIdentifier, - failReason) + _, err = p.Fail( + ctx, info.PaymentIdentifier, failReason, + ) require.NoError(t, err, "unable to fail payment hash") // Settle the attempt @@ -1667,7 +1668,7 @@ func TestFailsWithoutInFlight(t *testing.T) { // Calling Fail should return an error. _, err = paymentDB.Fail( - info.PaymentIdentifier, FailureReasonNoRoute, + t.Context(), info.PaymentIdentifier, FailureReasonNoRoute, ) require.ErrorIs(t, err, ErrPaymentNotInitiated) } @@ -1843,7 +1844,7 @@ func TestSwitchFail(t *testing.T) { // Fail the payment, which should moved it to Failed. failReason := FailureReasonNoRoute - _, err = paymentDB.Fail(info.PaymentIdentifier, failReason) + _, err = paymentDB.Fail(ctx, info.PaymentIdentifier, failReason) require.NoError(t, err, "unable to fail payment hash") // Verify the status is indeed Failed. @@ -2139,7 +2140,7 @@ func TestMultiShard(t *testing.T) { // a terminal state. failReason := FailureReasonNoRoute _, err = paymentDB.Fail( - info.PaymentIdentifier, failReason, + ctx, info.PaymentIdentifier, failReason, ) if err != nil { t.Fatalf("unable to fail payment hash: %v", err) @@ -2232,7 +2233,7 @@ func TestMultiShard(t *testing.T) { // syncing. failReason := FailureReasonPaymentDetails _, err = paymentDB.Fail( - info.PaymentIdentifier, failReason, + ctx, info.PaymentIdentifier, failReason, ) require.NoError(t, err, "unable to fail") } diff --git a/payments/db/sql_store.go b/payments/db/sql_store.go index a921a1233..a9863b53e 100644 --- a/payments/db/sql_store.go +++ b/payments/db/sql_store.go @@ -1782,11 +1782,9 @@ func (s *SQLStore) FailAttempt(ctx context.Context, paymentHash lntypes.Hash, // This method is part of the PaymentControl interface, which is embedded in // the PaymentWriter interface and ultimately the DB interface. It represents // step 4 in the payment lifecycle control flow. -func (s *SQLStore) Fail(paymentHash lntypes.Hash, +func (s *SQLStore) Fail(ctx context.Context, paymentHash lntypes.Hash, reason FailureReason) (*MPPayment, error) { - ctx := context.TODO() - var mpPayment *MPPayment err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error { diff --git a/routing/control_tower.go b/routing/control_tower.go index cbb79d4c7..b39a378bd 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -66,7 +66,8 @@ type ControlTower interface { // payment. // // NOTE: Subscribers should be notified by the new state of the payment. - FailPayment(lntypes.Hash, paymentsdb.FailureReason) error + FailPayment(context.Context, lntypes.Hash, + paymentsdb.FailureReason) error // FetchInFlightPayments returns all payments with status InFlight. FetchInFlightPayments(ctx context.Context) ([]*paymentsdb.MPPayment, @@ -272,13 +273,13 @@ func (p *controlTower) FetchPayment(ctx context.Context, // // NOTE: This method will overwrite the failure reason if the payment is already // failed. -func (p *controlTower) FailPayment(paymentHash lntypes.Hash, - reason paymentsdb.FailureReason) error { +func (p *controlTower) FailPayment(ctx context.Context, + paymentHash lntypes.Hash, reason paymentsdb.FailureReason) error { p.paymentsMtx.Lock(paymentHash) defer p.paymentsMtx.Unlock(paymentHash) - payment, err := p.db.Fail(paymentHash, reason) + payment, err := p.db.Fail(ctx, paymentHash, reason) if err != nil { return err } diff --git a/routing/control_tower_test.go b/routing/control_tower_test.go index 5241d814d..c9e8f4857 100644 --- a/routing/control_tower_test.go +++ b/routing/control_tower_test.go @@ -516,7 +516,8 @@ func testKVStoreSubscribeFail(t *testing.T, registerAttempt, // Mark the payment as failed. err = pControl.FailPayment( - info.PaymentIdentifier, paymentsdb.FailureReasonTimeout, + t.Context(), info.PaymentIdentifier, + paymentsdb.FailureReasonTimeout, ) if err != nil { t.Fatal(err) diff --git a/routing/mock_test.go b/routing/mock_test.go index f10c38ad0..e72b39249 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -491,7 +491,7 @@ func (m *mockControlTowerOld) FailAttempt(_ context.Context, phash lntypes.Hash, return nil, fmt.Errorf("pid not found") } -func (m *mockControlTowerOld) FailPayment(phash lntypes.Hash, +func (m *mockControlTowerOld) FailPayment(_ context.Context, phash lntypes.Hash, reason paymentsdb.FailureReason) error { m.Lock() @@ -782,7 +782,7 @@ func (m *mockControlTower) FailAttempt(_ context.Context, phash lntypes.Hash, return attempt.(*paymentsdb.HTLCAttempt), args.Error(1) } -func (m *mockControlTower) FailPayment(phash lntypes.Hash, +func (m *mockControlTower) FailPayment(_ context.Context, phash lntypes.Hash, reason paymentsdb.FailureReason) error { args := m.Called(phash, reason) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 904d399cd..37dbd1c8a 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -364,11 +364,18 @@ func (p *paymentLifecycle) checkContext(ctx context.Context) error { p.identifier.String()) } + // The context is already cancelled at this point, so we create + // a new context so the payment can successfully be marked as + // failed. + cleanupCtx := context.WithoutCancel(ctx) + // By marking the payment failed, depending on whether it has // inflight HTLCs or not, its status will now either be // `StatusInflight` or `StatusFailed`. In either case, no more // HTLCs will be attempted. - err := p.router.cfg.Control.FailPayment(p.identifier, reason) + err := p.router.cfg.Control.FailPayment( + cleanupCtx, p.identifier, reason, + ) if err != nil { return fmt.Errorf("FailPayment got %w", err) } @@ -389,6 +396,8 @@ func (p *paymentLifecycle) checkContext(ctx context.Context) error { func (p *paymentLifecycle) requestRoute( ps *paymentsdb.MPPaymentState) (*route.Route, error) { + ctx := context.TODO() + remainingFees := p.calcFeeBudget(ps.FeesPaid) // Query our payment session to construct a route. @@ -430,7 +439,9 @@ func (p *paymentLifecycle) requestRoute( log.Warnf("Marking payment %v permanently failed with no route: %v", p.identifier, failureCode) - err = p.router.cfg.Control.FailPayment(p.identifier, failureCode) + err = p.router.cfg.Control.FailPayment( + ctx, p.identifier, failureCode, + ) if err != nil { return nil, fmt.Errorf("FailPayment got: %w", err) } @@ -800,6 +811,8 @@ func (p *paymentLifecycle) failPaymentAndAttempt( attemptID uint64, reason *paymentsdb.FailureReason, sendErr error) (*attemptResult, error) { + ctx := context.TODO() + log.Errorf("Payment %v failed: final_outcome=%v, raw_err=%v", p.identifier, *reason, sendErr) @@ -808,7 +821,9 @@ func (p *paymentLifecycle) failPaymentAndAttempt( // NOTE: we must fail the payment first before failing the attempt. // Otherwise, once the attempt is marked as failed, another goroutine // might make another attempt while we are failing the payment. - err := p.router.cfg.Control.FailPayment(p.identifier, *reason) + err := p.router.cfg.Control.FailPayment( + ctx, p.identifier, *reason, + ) if err != nil { log.Errorf("Unable to fail payment: %v", err) return nil, err diff --git a/routing/router.go b/routing/router.go index 93192609b..fe8d06797 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1088,7 +1088,9 @@ func (r *ChannelRouter) sendToRoute(htlcHash lntypes.Hash, rt *route.Route, return nil } - return r.cfg.Control.FailPayment(paymentIdentifier, reason) + return r.cfg.Control.FailPayment( + ctx, paymentIdentifier, reason, + ) } log.Debugf("SendToRoute for payment %v with skipTempErr=%v",