accounts: use UpsertAccountPayment in removePayment

Instead of UpdateAccount.

Author: ellemouton

accounts/errors.go

@@ -13,4 +13,8 @@ var (
 	// if the WithErrAlreadySucceeded option is used and the payment has
 	// already succeeded.
 	ErrAlreadySucceeded = errors.New("payment has already succeeded")
+
+	// ErrPaymentUnknown is returned by the UpsertAccountPayment method if
+	// the WithErrIfUnknown option is used and the payment is not yet known.
+	ErrPaymentUnknown = errors.New("payment unknown")
 )

accounts/interface.go

@@ -345,6 +345,7 @@ type upsertAcctPaymentOption struct {
 	errIfAlreadyPending   bool
 	usePendingAmount      bool
 	errIfAlreadySucceeded bool
+	errIfUnknown          bool
 }
 
 // newUpsertPaymentOption creates a new upsertAcctPaymentOption with default
@@ -355,6 +356,7 @@ func newUpsertPaymentOption() *upsertAcctPaymentOption {
 		errIfAlreadyPending:   false,
 		usePendingAmount:      false,
 		errIfAlreadySucceeded: false,
+		errIfUnknown:          false,
 	}
 }
 
@@ -394,3 +396,12 @@ func WithPendingAmount() UpsertPaymentOption {
 		o.usePendingAmount = true
 	}
 }
+
+// WithErrIfUnknown is a functional option that can be passed to the
+// UpsertAccountPayment method to indicate that the ErrPaymentUnknown error
+// should be returned if the payment is not associated with the account.
+func WithErrIfUnknown() UpsertPaymentOption {
+	return func(o *upsertAcctPaymentOption) {
+		o.errIfUnknown = true
+	}
+}

accounts/service.go

@@ -840,23 +840,24 @@ func (s *InterceptorService) removePayment(ctx context.Context,
 		return nil
 	}
 
-	account, err := s.store.Account(ctx, pendingPayment.accountID)
-	if err != nil {
-		return err
-	}
-
 	pendingPayment.cancel()
 	delete(s.pendingPayments, hash)
 
-	// Have we associated the payment with the account already?
-	_, ok = account.Payments[hash]
-	if !ok {
-		return nil
+	_, err := s.store.UpsertAccountPayment(
+		ctx, pendingPayment.accountID, hash, 0, status,
+		// We don't want the payment to be inserted if it isn't already
+		// known. So we pass in this option to ensure that the call
+		// exists early if the payment is unknown.
+		WithErrIfUnknown(),
+		// Otherwise, we just want to update the status of the payment
+		// and use the existing pending amount.
+		WithPendingAmount(),
+	)
+	if err != nil && !errors.Is(err, ErrPaymentUnknown) {
+		return fmt.Errorf("error updating account: %w", err)
 	}
 
-	// If we did, let's set the status correctly in the DB now.
-	account.Payments[hash].Status = status
-	return s.store.UpdateAccount(ctx, account)
+	return nil
 }
 
 // successState returns true if a payment was completed successfully.

accounts/store_kvdb.go

@@ -300,6 +300,8 @@ func (s *BoltStore) UpsertAccountPayment(_ context.Context, id AccountID,
 			if opts.usePendingAmount {
 				fullAmount = entry.FullAmount
 			}
+		} else if opts.errIfUnknown {
+			return ErrPaymentUnknown
 		}
 
 		account.Payments[paymentHash] = &PaymentEntry{

accounts/store_test.go

@@ -430,6 +430,15 @@ func TestAccountUpdateMethods(t *testing.T) {
 		// error.
 		err = store.DeleteAccountPayment(ctx, acct.ID, hash1)
 		require.ErrorContains(t, err, "is not associated")
+
+		// Try once more to insert a payment that is currently unknown
+		// but this time add the WithErrIfUnknown option. This should
+		// return the ErrPaymentUnknown error.
+		_, err = store.UpsertAccountPayment(
+			ctx, acct.ID, hash1, 600, lnrpc.Payment_SUCCEEDED,
+			WithErrIfUnknown(),
+		)
+		require.ErrorIs(t, err, ErrPaymentUnknown)
 	})
 }