multi: thread context through DeleteFailedAttempts
What changed, and why it matters
This change threads a context parameter through the DeleteFailedAttempts function so that database cleanup operations can continue even if the original request context is cancelled. It is a robustness improvement, not a fix for an actively exploitable security vulnerability. The commit message and diff do not describe any security issue.
No immediate security action required. Treat as normal code-quality/robustness improvement. Reviewers may verify that context.WithoutCancel is used appropriately and that cleanup does not run indefinitely.
Security signals we found
No security relevance claimed by vendor in commit message or title
Change improves graceful cleanup under cancellation
No input validation, access control, or cryptographic changes
No references to CVEs, advisories, or security reports
Evidence from the diff
The commit modifies DeleteFailedAttempts signatures across the payment database interfaces (KVStore, SQLStore), control tower, and tests to accept a context.Context. In payment_lifecycle.go, it creates a context.WithoutCancel(ctx) named cleanupCtx and passes it to DeleteFailedAttempts. This ensures that final cleanup of failed HTLC attempts is not aborted if the caller’s context is cancelled. The previous SQL implementation used context.TODO(), and the KV implementation used context.TODO() internally; both now use the supplied context. This is a correctness/robustness change rather than a vulnerability patch.
Changed components
payments/db/interface.gopayments/db/kv_store.gopayments/db/sql_store.gopayments/db/payment_test.gorouting/control_tower.gorouting/payment_lifecycle.gorouting/mock_test.goInspect captured patch +44 / −18
diff --git a/payments/db/interface.go b/payments/db/interface.go
index 2d2c47b..6edaa7f 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 285074b..0ce0601 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 5f1ab69..25aafbb 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 a9863b5..d23e808 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 b39a378..1c246f1 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 e72b392..472f126 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 37dbd1c..6405e85 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)
Why this scored 21/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.