multi: thread context through DeleteFailedAttempts

Author: ziggie1984

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

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
 		}

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,19 +520,25 @@ 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
 	// payment to be unchanged.
 	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,
+			),
 		)
 	}
 }

payments/db/sql_store.go

@@ -1012,7 +1012,9 @@ 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 {
+func (s *SQLStore) DeleteFailedAttempts(ctx context.Context,
+	paymentHash lntypes.Hash) error {
+
 	// In case we are configured to keep failed payment attempts, we exit
 	// early.
 	//
@@ -1021,7 +1023,6 @@ func (s *SQLStore) DeleteFailedAttempts(paymentHash lntypes.Hash) error {
 	if s.keepFailedPaymentAttempts {
 		return nil
 	}
-	ctx := context.TODO()
 
 	err := s.db.ExecTx(ctx, sqldb.WriteTxOpt(), func(db SQLQueries) error {
 		dbPayment, err := fetchPaymentByHash(ctx, db, paymentHash)

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

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)
 }

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)