diff --git a/payments/db/interface.go b/payments/db/interface.go index 2d2c47b3b..6edaa7f45 100644 --- a/payments/db/interface.go +++ b/payments/db/interface.go @@ -104,7 +104,7 @@ type PaymentControl interface { // DeleteFailedAttempts removes all failed HTLCs from the db. It should // be called for a given payment whenever all inflight htlcs are // completed, and the payment has reached a final terminal state. - DeleteFailedAttempts(lntypes.Hash) error + DeleteFailedAttempts(context.Context, lntypes.Hash) error } // DBMPPayment is an interface that represents the payment state during a diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go index 285074b51..0ce0601e4 100644 --- a/payments/db/kv_store.go +++ b/payments/db/kv_store.go @@ -290,12 +290,14 @@ func (p *KVStore) InitPayment(_ context.Context, paymentHash lntypes.Hash, // DeleteFailedAttempts deletes all failed htlcs for a payment if configured // by the KVStore db. -func (p *KVStore) DeleteFailedAttempts(hash lntypes.Hash) error { +func (p *KVStore) DeleteFailedAttempts(ctx context.Context, + hash lntypes.Hash) error { + // TODO(ziggie): Refactor to not mix application logic with database // logic. This decision should be made in the application layer. if !p.keepFailedPaymentAttempts { const failedHtlcsOnly = true - err := p.DeletePayment(context.TODO(), hash, failedHtlcsOnly) + err := p.DeletePayment(ctx, hash, failedHtlcsOnly) if err != nil { return err } diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index 5f1ab69c1..25aafbb54 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -503,7 +503,9 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { // Calling DeleteFailedAttempts on a failed payment should delete all // HTLCs. - require.NoError(t, paymentDB.DeleteFailedAttempts(payments[0].id)) + require.NoError(t, paymentDB.DeleteFailedAttempts( + t.Context(), payments[0].id, + )) // Expect all HTLCs to be deleted if the config is set to delete them. if !keepFailedPaymentAttempts { @@ -518,11 +520,15 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { // operation are performed in general therefore we do NOT expect an // error in this case. if keepFailedPaymentAttempts { - require.NoError(t, paymentDB.DeleteFailedAttempts( - payments[1].id), + err := paymentDB.DeleteFailedAttempts( + t.Context(), payments[1].id, ) + require.NoError(t, err) } else { - require.Error(t, paymentDB.DeleteFailedAttempts(payments[1].id)) + err := paymentDB.DeleteFailedAttempts( + t.Context(), payments[1].id, + ) + require.Error(t, err) } // Since DeleteFailedAttempts returned an error, we should expect the @@ -530,7 +536,9 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { assertDBPayments(t, paymentDB, payments) // Cleaning up a successful payment should remove failed htlcs. - require.NoError(t, paymentDB.DeleteFailedAttempts(payments[2].id)) + require.NoError(t, paymentDB.DeleteFailedAttempts( + t.Context(), payments[2].id, + )) // Expect all HTLCs except for the settled one to be deleted if the // config is set to delete them. @@ -547,13 +555,17 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { // payments, if the control tower is configured to keep failed // HTLCs. require.NoError( - t, paymentDB.DeleteFailedAttempts(lntypes.ZeroHash), + t, paymentDB.DeleteFailedAttempts( + t.Context(), lntypes.ZeroHash, + ), ) } else { // Attempting to cleanup a non-existent payment returns an // error. require.Error( - t, paymentDB.DeleteFailedAttempts(lntypes.ZeroHash), + t, paymentDB.DeleteFailedAttempts( + t.Context(), lntypes.ZeroHash, + ), ) } } diff --git a/payments/db/sql_store.go b/payments/db/sql_store.go index a9863b53e..d23e80895 100644 --- a/payments/db/sql_store.go +++ b/payments/db/sql_store.go @@ -1106,8 +1106,8 @@ func (s *SQLStore) FetchInFlightPayments(ctx context.Context) ([]*MPPayment, // the final step (step 5) in the payment lifecycle control flow and should be // called after a payment reaches a terminal state (succeeded or permanently // failed) to clean up historical failed attempts. -func (s *SQLStore) DeleteFailedAttempts(paymentHash lntypes.Hash) error { - ctx := context.TODO() +func (s *SQLStore) DeleteFailedAttempts(ctx context.Context, + paymentHash lntypes.Hash) error { // In case we are configured to keep failed payment attempts, we exit // early. diff --git a/routing/control_tower.go b/routing/control_tower.go index b39a378bd..1c246f17d 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -26,7 +26,7 @@ type ControlTower interface { // DeleteFailedAttempts removes all failed HTLCs from the db. It should // be called for a given payment whenever all inflight htlcs are // completed, and the payment has reached a final settled state. - DeleteFailedAttempts(lntypes.Hash) error + DeleteFailedAttempts(context.Context, lntypes.Hash) error // RegisterAttempt atomically records the provided HTLCAttemptInfo. // @@ -192,8 +192,10 @@ func (p *controlTower) InitPayment(ctx context.Context, // DeleteFailedAttempts deletes all failed htlcs if the payment was // successfully settled. -func (p *controlTower) DeleteFailedAttempts(paymentHash lntypes.Hash) error { - return p.db.DeleteFailedAttempts(paymentHash) +func (p *controlTower) DeleteFailedAttempts(ctx context.Context, + paymentHash lntypes.Hash) error { + + return p.db.DeleteFailedAttempts(ctx, paymentHash) } // RegisterAttempt atomically records the provided HTLCAttemptInfo to the diff --git a/routing/mock_test.go b/routing/mock_test.go index e72b39249..472f12616 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -328,7 +328,9 @@ func (m *mockControlTowerOld) InitPayment(_ context.Context, return nil } -func (m *mockControlTowerOld) DeleteFailedAttempts(phash lntypes.Hash) error { +func (m *mockControlTowerOld) DeleteFailedAttempts(_ context.Context, + phash lntypes.Hash) error { + p, ok := m.payments[phash] if !ok { return paymentsdb.ErrPaymentNotInitiated @@ -742,7 +744,9 @@ func (m *mockControlTower) InitPayment(_ context.Context, phash lntypes.Hash, return args.Error(0) } -func (m *mockControlTower) DeleteFailedAttempts(phash lntypes.Hash) error { +func (m *mockControlTower) DeleteFailedAttempts(_ context.Context, + phash lntypes.Hash) error { + args := m.Called(phash) return args.Error(0) } diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 37dbd1c8a..6405e8506 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -190,6 +190,10 @@ func (p *paymentLifecycle) decideNextStep( func (p *paymentLifecycle) resumePayment(ctx context.Context) ([32]byte, *route.Route, error) { + // We need to make sure we can still do db operations after the context + // is cancelled. + cleanupCtx := context.WithoutCancel(ctx) + // When the payment lifecycle loop exits, we make sure to signal any // sub goroutine of the HTLC attempt to exit, then wait for them to // return. @@ -328,7 +332,9 @@ lifecycle: // Optionally delete the failed attempts from the database. Depends on // the database options deleting attempts is not allowed so this will // just be a no-op. - err = p.router.cfg.Control.DeleteFailedAttempts(p.identifier) + err = p.router.cfg.Control.DeleteFailedAttempts( + cleanupCtx, p.identifier, + ) if err != nil { log.Errorf("Error deleting failed htlc attempts for payment "+ "%v: %v", p.identifier, err)