accounts: add DeleteAccountPayment method to Store

And use it instead of UpdateAccount.

Author: ellemouton

accounts/errors.go

@@ -13,4 +13,10 @@ var (
 	// if the WithErrAlreadySucceeded option is used and the payment has
 	// already succeeded.
 	ErrAlreadySucceeded = errors.New("payment has already succeeded")
+
+	// ErrPaymentNotAssociated indicate that the payment with the given hash
+	// has not yet been associated with the account in question.
+	ErrPaymentNotAssociated = errors.New(
+		"payment not associated with account",
+	)
 )

accounts/interface.go

@@ -246,6 +246,12 @@ type Store interface {
 		status lnrpc.Payment_PaymentStatus,
 		options ...UpsertPaymentOption) (bool, error)
 
+	// DeleteAccountPayment removes a payment entry from the account with
+	// the given ID. It will return the ErrPaymentNotAssociated error if the
+	// payment is not associated with the account.
+	DeleteAccountPayment(_ context.Context, id AccountID,
+		hash lntypes.Hash) error
+
 	// RemoveAccount finds an account by its ID and removes it from the¨
 	// store.
 	RemoveAccount(ctx context.Context, id AccountID) error

accounts/service.go

@@ -467,26 +467,7 @@ func (s *InterceptorService) PaymentErrored(ctx context.Context, id AccountID,
 			"has already started")
 	}
 
-	account, err := s.store.Account(ctx, id)
-	if err != nil {
-		return err
-	}
-
-	// Check that this payment is actually associated with this account.
-	_, ok = account.Payments[hash]
-	if !ok {
-		return fmt.Errorf("payment with hash %s is not associated "+
-			"with this account", hash)
-	}
-
-	// Delete the payment and update the persisted account.
-	delete(account.Payments, hash)
-
-	if err := s.store.UpdateAccount(ctx, account); err != nil {
-		return fmt.Errorf("error updating account: %w", err)
-	}
-
-	return nil
+	return s.store.DeleteAccountPayment(ctx, id, hash)
 }
 
 // AssociatePayment associates a payment (hash) with the given account,

accounts/store_kvdb.go

@@ -317,6 +317,33 @@ func (s *BoltStore) UpsertAccountPayment(_ context.Context, id AccountID,
 	return known, s.updateAccount(id, update)
 }
 
+// DeleteAccountPayment removes a payment entry from the account with the given
+// ID. It will return the ErrPaymentNotAssociated error if the payment is not
+// associated with the account.
+//
+// NOTE: This is part of the Store interface.
+func (s *BoltStore) DeleteAccountPayment(_ context.Context, id AccountID,
+	hash lntypes.Hash) error {
+
+	update := func(account *OffChainBalanceAccount) error {
+		// Check that this payment is actually associated with this
+		// account.
+		_, ok := account.Payments[hash]
+		if !ok {
+			return fmt.Errorf("payment with hash %s is not "+
+				"associated with this account: %w", hash,
+				ErrPaymentNotAssociated)
+		}
+
+		// Delete the payment and update the persisted account.
+		delete(account.Payments, hash)
+
+		return nil
+	}
+
+	return s.updateAccount(id, update)
+}
+
 func (s *BoltStore) updateAccount(id AccountID,
 	updateFn func(*OffChainBalanceAccount) error) error {
 

accounts/store_test.go

@@ -258,7 +258,7 @@ func TestAccountUpdateMethods(t *testing.T) {
 		assertBalance(223)
 	})
 
-	t.Run("UpsertAccountPayment", func(t *testing.T) {
+	t.Run("Upsert and Delete AccountPayment", func(t *testing.T) {
 		store := NewTestDB(t)
 
 		acct, err := store.NewAccount(ctx, 1000, time.Time{}, "foo")
@@ -413,6 +413,23 @@ func TestAccountUpdateMethods(t *testing.T) {
 				FullAmount: 100,
 			},
 		})
+
+		// Delete the first payment and make sure it is removed from the
+		// account.
+		err = store.DeleteAccountPayment(ctx, acct.ID, hash1)
+		require.NoError(t, err)
+
+		assertBalanceAndPayments(400, AccountPayments{
+			hash2: &PaymentEntry{
+				Status:     lnrpc.Payment_SUCCEEDED,
+				FullAmount: 100,
+			},
+		})
+
+		// Test that deleting a payment that does not exist returns an
+		// error.
+		err = store.DeleteAccountPayment(ctx, acct.ID, hash1)
+		require.ErrorIs(t, err, ErrPaymentNotAssociated)
 	})
 }