What changed, and why it matters
This change threads a request context through the DeletePayment database operation. In practical terms, it lets the database operation respect cancellation or timeout signals from the original RPC request, rather than running with a blank 'do nothing' context. The SQL backend now uses the real request context, while the older key-value backend still ignores it for now. This is a code-quality and robustness improvement, not a fix for an active security vulnerability.
Treat as a routine reliability/maintenance improvement. If the project is moving toward context-aware database operations, follow up by threading context through the KV backend as well. No urgent security action is required.
Security signals we found
Context propagation added to database write path
SQL store now uses caller-provided context instead of context.TODO()
KV store still ignores context (partial patch)
No changes to authorization, input validation, or cryptographic checks
Evidence from the diff
The commit modifies the PaymentWriter.DeletePayment signature to accept a context.Context and propagates that context from rpcserver.DeletePayment down into the SQL store. The KV store accepts the parameter but uses context.TODO() internally, so only the SQL backend benefits immediately. This enables request-scoped cancellation/timeouts for the SQL transaction and queries (fetchPaymentByHash, ExecTx). It reduces the risk of orphaned or unbounded database work when a client disconnects or times out, but does not change authorization, validation, or access-control logic.
Changed components
payments/db/interface.gopayments/db/sql_store.gopayments/db/kv_store.gorpcserver.gopayments/db/payment_test.goInspect captured patch +32 / −17
diff --git a/payments/db/interface.go b/payments/db/interface.go
index 7fefad0..caf222b 100644
--- a/payments/db/interface.go
+++ b/payments/db/interface.go
@@ -31,7 +31,8 @@ type PaymentReader interface {
// database.
type PaymentWriter interface {
// DeletePayment deletes a payment from the DB given its payment hash.
- DeletePayment(paymentHash lntypes.Hash, failedAttemptsOnly bool) error
+ DeletePayment(ctx context.Context, paymentHash lntypes.Hash,
+ failedAttemptsOnly bool) error
// DeletePayments deletes all payments from the DB given the specified
// flags.
diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go
index 8494684..138edb6 100644
--- a/payments/db/kv_store.go
+++ b/payments/db/kv_store.go
@@ -295,7 +295,7 @@ func (p *KVStore) DeleteFailedAttempts(hash lntypes.Hash) error {
// logic. This decision should be made in the application layer.
if !p.keepFailedPaymentAttempts {
const failedHtlcsOnly = true
- err := p.DeletePayment(hash, failedHtlcsOnly)
+ err := p.DeletePayment(context.TODO(), hash, failedHtlcsOnly)
if err != nil {
return err
}
@@ -1275,7 +1275,7 @@ func fetchPaymentWithSequenceNumber(tx kvdb.RTx, paymentHash lntypes.Hash,
// DeletePayment deletes a payment from the DB given its payment hash. If
// failedHtlcsOnly is set, only failed HTLC attempts of the payment will be
// deleted.
-func (p *KVStore) DeletePayment(paymentHash lntypes.Hash,
+func (p *KVStore) DeletePayment(_ context.Context, paymentHash lntypes.Hash,
failedHtlcsOnly bool) error {
return kvdb.Update(p.db, func(tx kvdb.RwTx) error {
diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go
index ddba0e0..1180cf9 100644
--- a/payments/db/payment_test.go
+++ b/payments/db/payment_test.go
@@ -511,8 +511,8 @@ 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),
+ require.NoError(t, paymentDB.DeleteFailedAttempts(
+ payments[1].id),
)
} else {
require.Error(t, paymentDB.DeleteFailedAttempts(payments[1].id))
@@ -656,6 +656,8 @@ func TestMPPRecordValidation(t *testing.T) {
func TestDeleteSinglePayment(t *testing.T) {
t.Parallel()
+ ctx := t.Context()
+
paymentDB, _ := NewTestDB(t)
// Register four payments:
@@ -687,7 +689,9 @@ func TestDeleteSinglePayment(t *testing.T) {
assertDBPayments(t, paymentDB, payments)
// Delete HTLC attempts for first payment only.
- require.NoError(t, paymentDB.DeletePayment(payments[0].id, true))
+ require.NoError(t, paymentDB.DeletePayment(
+ ctx, payments[0].id, true,
+ ))
// The first payment is the only altered one as its failed HTLC should
// have been removed but is still present as payment.
@@ -695,19 +699,25 @@ func TestDeleteSinglePayment(t *testing.T) {
assertDBPayments(t, paymentDB, payments)
// Delete the first payment completely.
- require.NoError(t, paymentDB.DeletePayment(payments[0].id, false))
+ require.NoError(t, paymentDB.DeletePayment(
+ ctx, payments[0].id, false,
+ ))
// The first payment should have been deleted.
assertDBPayments(t, paymentDB, payments[1:])
// Now delete the second payment completely.
- require.NoError(t, paymentDB.DeletePayment(payments[1].id, false))
+ require.NoError(t, paymentDB.DeletePayment(
+ ctx, payments[1].id, false,
+ ))
// The Second payment should have been deleted.
assertDBPayments(t, paymentDB, payments[2:])
// Delete failed HTLC attempts for the third payment.
- require.NoError(t, paymentDB.DeletePayment(payments[2].id, true))
+ require.NoError(t, paymentDB.DeletePayment(
+ ctx, payments[2].id, true,
+ ))
// Only the successful HTLC attempt should be left for the third
// payment.
@@ -715,21 +725,27 @@ func TestDeleteSinglePayment(t *testing.T) {
assertDBPayments(t, paymentDB, payments[2:])
// Now delete the third payment completely.
- require.NoError(t, paymentDB.DeletePayment(payments[2].id, false))
+ require.NoError(t, paymentDB.DeletePayment(
+ ctx, payments[2].id, false,
+ ))
// Only the last payment should be left.
assertDBPayments(t, paymentDB, payments[3:])
// Deleting HTLC attempts from InFlight payments should not work and an
// error returned.
- require.Error(t, paymentDB.DeletePayment(payments[3].id, true))
+ require.Error(t, paymentDB.DeletePayment(
+ ctx, payments[3].id, true,
+ ))
// The payment is InFlight and therefore should not have been altered.
assertDBPayments(t, paymentDB, payments[3:])
// Finally deleting the InFlight payment should also not work and an
// error returned.
- require.Error(t, paymentDB.DeletePayment(payments[3].id, false))
+ require.Error(t, paymentDB.DeletePayment(
+ ctx, payments[3].id, false,
+ ))
// The payment is InFlight and therefore should not have been altered.
assertDBPayments(t, paymentDB, payments[3:])
@@ -2597,7 +2613,7 @@ func TestQueryPayments(t *testing.T) {
// We delete the whole payment.
err = paymentDB.DeletePayment(
- paymentInfos[1].PaymentIdentifier, false,
+ ctx, paymentInfos[1].PaymentIdentifier, false,
)
require.NoError(t, err)
diff --git a/payments/db/sql_store.go b/payments/db/sql_store.go
index 0109ca1..f6f5d0d 100644
--- a/payments/db/sql_store.go
+++ b/payments/db/sql_store.go
@@ -1206,11 +1206,9 @@ func computePaymentStatusFromDB(ctx context.Context, cfg *sqldb.QueryConfig,
//
// This method is part of the PaymentWriter interface, which is embedded in
// the DB interface.
-func (s *SQLStore) DeletePayment(paymentHash lntypes.Hash,
+func (s *SQLStore) DeletePayment(ctx context.Context, paymentHash lntypes.Hash,
failedHtlcsOnly bool) error {
- ctx := context.TODO()
-
err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error {
dbPayment, err := fetchPaymentByHash(ctx, db, paymentHash)
if err != nil {
diff --git a/rpcserver.go b/rpcserver.go
index 8cbeb74..df27c41 100644
--- a/rpcserver.go
+++ b/rpcserver.go
@@ -7741,7 +7741,7 @@ func (r *rpcServer) DeletePayment(ctx context.Context,
rpcsLog.Infof("[DeletePayment] payment_identifier=%v, "+
"failed_htlcs_only=%v", hash, req.FailedHtlcsOnly)
- err = r.server.paymentsDB.DeletePayment(hash, req.FailedHtlcsOnly)
+ err = r.server.paymentsDB.DeletePayment(ctx, hash, req.FailedHtlcsOnly)
if err != nil {
return nil, err
}
Why this scored 18/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.