paymentsdb: implement DeleteFailedAttempts for sql backend
What changed, and why it matters
This commit adds a missing database cleanup feature for the SQL backend of LND's payment database. It lets the node delete failed payment attempts after a payment finishes, matching behavior that already existed for the older key-value database backend. There is no obvious security vulnerability in the change; it is a feature-completion patch with a defensive guard that refuses cleanup while a payment still has active/in-flight HTLCs.
No immediate security action required. Review the TODO about moving keepFailedPaymentAttempts logic to the application layer in a follow-up, and verify that paymentStatus.removable() correctly rejects StatusInFlight and accepts terminal states. Consider adding tests for idempotency and concurrent in-flight status handling.
Security signals we found
New deletion path guarded by payment status check (removable())
Explicit refusal to delete attempts for in-flight payments (ErrPaymentInFlight)
Transaction-wrapped deletion to maintain consistency
Configuration flag (keepFailedPaymentAttempts) can skip deletion entirely
TODO comments flag mixing of application logic with database logic
Evidence from the diff
The patch implements DeleteFailedAttempts for the SQL-backed payments store. It wraps deletion in a write transaction, fetches the payment, computes its status from minimal HTLC resolution data, and only proceeds if paymentStatus.removable() permits. StatusInFlight returns ErrPaymentInFlight, preventing deletion of attempts for payments with active HTLCs. If keepFailedPaymentAttempts is set, it exits early. The SQL query deletes payment_htlc_attempts rows whose resolution_type is 2 (failed). A helper computePaymentStatusFromDB is introduced to avoid loading full route data. A TODO notes that the keep-failed-attempts decision should move to the application layer.
Changed components
payments/db/sql_store.gopayments/db/kv_store.gosqldb/sqlc/payments.sql.gosqldb/sqlc/querier.goInspect captured patch +126 / −0
diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go
index 62f0b83..8494684 100644
--- a/payments/db/kv_store.go
+++ b/payments/db/kv_store.go
@@ -291,6 +291,8 @@ func (p *KVStore) InitPayment(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 {
+ // 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(hash, failedHtlcsOnly)
diff --git a/payments/db/sql_store.go b/payments/db/sql_store.go
index d2500d6..5f22f47 100644
--- a/payments/db/sql_store.go
+++ b/payments/db/sql_store.go
@@ -712,3 +712,123 @@ func (s *SQLStore) FetchPayment(paymentHash lntypes.Hash) (*MPPayment, error) {
return mpPayment, nil
}
+
+// DeleteFailedAttempts removes all failed HTLC attempts from the database for
+// the specified payment, while preserving the payment record itself and any
+// successful or in-flight attempts.
+//
+// The method performs the following validations before deletion:
+// - StatusInitiated: Can delete failed attempts
+// - StatusInFlight: Cannot delete, returns ErrPaymentInFlight (active HTLCs
+// still on the network)
+// - StatusSucceeded: Can delete failed attempts (payment completed)
+// - StatusFailed: Can delete failed attempts (payment permanently failed)
+//
+// If the keepFailedPaymentAttempts configuration flag is enabled, this method
+// returns immediately without deleting anything, allowing failed attempts to
+// be retained for debugging or auditing purposes.
+//
+// This method is idempotent - calling it multiple times on the same payment
+// has no adverse effects.
+//
+// This method is part of the PaymentControl interface, which is embedded in
+// the PaymentWriter interface and ultimately the DB interface. It represents
+// 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()
+
+ // In case we are configured to keep failed payment attempts, we exit
+ // early.
+ //
+ // TODO(ziggie): Refactor to not mix application logic with database
+ // logic. This decision should be made in the application layer.
+ if s.keepFailedPaymentAttempts {
+ return nil
+ }
+
+ err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error {
+ dbPayment, err := db.FetchPayment(ctx, paymentHash[:])
+ if err != nil {
+ return fmt.Errorf("failed to fetch payment: %w", err)
+ }
+
+ paymentStatus, err := computePaymentStatusFromDB(
+ ctx, db, dbPayment,
+ )
+ if err != nil {
+ return fmt.Errorf("failed to compute payment "+
+ "status: %w", err)
+ }
+
+ if err := paymentStatus.removable(); err != nil {
+ return fmt.Errorf("cannot delete failed "+
+ "attempts for payment %v: %w", paymentHash, err)
+ }
+
+ // Then we delete the failed attempts for this payment.
+ return db.DeleteFailedAttempts(ctx, dbPayment.Payment.ID)
+ }, sqldb.NoOpReset)
+ if err != nil {
+ return fmt.Errorf("failed to delete failed attempts for "+
+ "payment %v: %w", paymentHash, err)
+ }
+
+ return nil
+}
+
+// computePaymentStatusFromDB computes the payment status by fetching minimal
+// data from the database. This is a lightweight query optimized for SQL that
+// doesn't load route data, making it significantly more efficient than
+// FetchPayment when only the status is needed.
+func computePaymentStatusFromDB(ctx context.Context, db SQLQueries,
+ dbPayment sqlc.PaymentAndIntent) (PaymentStatus, error) {
+
+ payment := dbPayment.GetPayment()
+
+ resolutionTypes, err := db.FetchHtlcAttemptResolutionsForPayment(
+ ctx, payment.ID,
+ )
+ if err != nil {
+ return 0, fmt.Errorf("failed to fetch htlc resolutions: %w",
+ err)
+ }
+
+ // Build minimal HTLCAttempt slice with only resolution info.
+ htlcs := make([]HTLCAttempt, len(resolutionTypes))
+ for i, resType := range resolutionTypes {
+ if !resType.Valid {
+ // NULL resolution_type means in-flight (no Settle, no
+ // Failure).
+ continue
+ }
+
+ switch HTLCAttemptResolutionType(resType.Int32) {
+ case HTLCAttemptResolutionSettled:
+ // Mark as settled (preimage details not needed for
+ // status).
+ htlcs[i].Settle = &HTLCSettleInfo{}
+
+ case HTLCAttemptResolutionFailed:
+ // Mark as failed (failure details not needed for
+ // status).
+ htlcs[i].Failure = &HTLCFailInfo{}
+ }
+ }
+
+ // Convert fail reason to FailureReason pointer.
+ var failureReason *FailureReason
+ if payment.FailReason.Valid {
+ reason := FailureReason(payment.FailReason.Int32)
+ failureReason = &reason
+ }
+
+ // Use the existing status decision logic.
+ status, err := decidePaymentStatus(htlcs, failureReason)
+ if err != nil {
+ return 0, fmt.Errorf("failed to decide payment status: %w", err)
+ }
+
+ return status, nil
+}
diff --git a/sqldb/sqlc/payments.sql.go b/sqldb/sqlc/payments.sql.go
index dd135e3..0883023 100644
--- a/sqldb/sqlc/payments.sql.go
+++ b/sqldb/sqlc/payments.sql.go
@@ -29,6 +29,8 @@ DELETE FROM payment_htlc_attempts WHERE payment_id = $1 AND attempt_index IN (
)
`
+// Delete all failed HTLC attempts for the given payment. Resolution type 2
+// indicates a failed attempt.
func (q *Queries) DeleteFailedAttempts(ctx context.Context, paymentID int64) error {
_, err := q.db.ExecContext(ctx, deleteFailedAttempts, paymentID)
return err
diff --git a/sqldb/sqlc/querier.go b/sqldb/sqlc/querier.go
index 008624a..9ba6f66 100644
--- a/sqldb/sqlc/querier.go
+++ b/sqldb/sqlc/querier.go
@@ -22,6 +22,8 @@ type Querier interface {
DeleteChannelPolicyExtraTypes(ctx context.Context, channelPolicyID int64) error
DeleteChannels(ctx context.Context, ids []int64) error
DeleteExtraNodeType(ctx context.Context, arg DeleteExtraNodeTypeParams) error
+ // Delete all failed HTLC attempts for the given payment. Resolution type 2
+ // indicates a failed attempt.
DeleteFailedAttempts(ctx context.Context, paymentID int64) error
DeleteInvoice(ctx context.Context, arg DeleteInvoiceParams) (sql.Result, error)
DeleteNode(ctx context.Context, id int64) error
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.