diff --git a/accounts/errors.go b/accounts/errors.go index 3e7674e5..520b2768 100644 --- a/accounts/errors.go +++ b/accounts/errors.go @@ -15,7 +15,9 @@ var ( 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. + // has not yet been associated with the account in question. It is also + // returned when the WithErrIfUnknown option is used with + // UpsertAccountPayment if the payment is not yet known. ErrPaymentNotAssociated = errors.New( "payment not associated with account", ) diff --git a/accounts/interface.go b/accounts/interface.go index 9dfb12e7..da396089 100644 --- a/accounts/interface.go +++ b/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 ErrPaymentNotAssociated +// error should be returned if the payment is not associated with the account. +func WithErrIfUnknown() UpsertPaymentOption { + return func(o *upsertAcctPaymentOption) { + o.errIfUnknown = true + } +} diff --git a/accounts/service.go b/accounts/service.go index e912ee9a..7f146d96 100644 --- a/accounts/service.go +++ b/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 + _, 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 + // exits 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, ErrPaymentNotAssociated) { + return fmt.Errorf("error updating account: %w", err) } pendingPayment.cancel() delete(s.pendingPayments, hash) - // Have we associated the payment with the account already? - _, ok = account.Payments[hash] - if !ok { - return nil - } - - // 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. diff --git a/accounts/store_kvdb.go b/accounts/store_kvdb.go index 74ce5d03..54c25beb 100644 --- a/accounts/store_kvdb.go +++ b/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 ErrPaymentNotAssociated } account.Payments[paymentHash] = &PaymentEntry{ diff --git a/accounts/store_test.go b/accounts/store_test.go index 259e4c99..a5bcbeb3 100644 --- a/accounts/store_test.go +++ b/accounts/store_test.go @@ -430,6 +430,74 @@ func TestAccountUpdateMethods(t *testing.T) { // error. err = store.DeleteAccountPayment(ctx, acct.ID, hash1) require.ErrorIs(t, err, ErrPaymentNotAssociated) + + // Try once more to insert a payment that is currently unknown + // but this time add the WithErrIfUnknown option. This should + // return the ErrPaymentNotAssociated error. + _, err = store.UpsertAccountPayment( + ctx, acct.ID, hash1, 600, lnrpc.Payment_SUCCEEDED, + WithErrIfUnknown(), + ) + require.ErrorIs(t, err, ErrPaymentNotAssociated) + + // Show that using the two options WithErrIfUnknown and + // WithPendingAmount together will return the + // ErrPaymentNotAssociated and will not successfully update + // the status. We call this for hash1 since it is no longer + // known. We do this to simulate the behaviour of + // removePayment. + _, err = store.UpsertAccountPayment( + ctx, acct.ID, hash1, 0, lnrpc.Payment_SUCCEEDED, + WithErrIfUnknown(), + WithPendingAmount(), + ) + require.ErrorIs(t, err, ErrPaymentNotAssociated) + + assertBalanceAndPayments(400, AccountPayments{ + hash2: &PaymentEntry{ + Status: lnrpc.Payment_SUCCEEDED, + FullAmount: 100, + }, + }) + + // Now insert hash 1 again. + _, err = store.UpsertAccountPayment( + ctx, acct.ID, hash1, 600, lnrpc.Payment_IN_FLIGHT, + ) + require.NoError(t, err) + + assertBalanceAndPayments(400, AccountPayments{ + hash1: &PaymentEntry{ + Status: lnrpc.Payment_IN_FLIGHT, + FullAmount: 600, + }, + hash2: &PaymentEntry{ + Status: lnrpc.Payment_SUCCEEDED, + FullAmount: 100, + }, + }) + + // Once again call UpsertAccountPayment with both the + // WithErrIfUnknown and WithPendingAmount options. This time + // it should succeed since the payment is now known and so the + // status should be updated. + _, err = store.UpsertAccountPayment( + ctx, acct.ID, hash1, 0, lnrpc.Payment_SUCCEEDED, + WithErrIfUnknown(), + WithPendingAmount(), + ) + require.NoError(t, err) + + assertBalanceAndPayments(400, AccountPayments{ + hash1: &PaymentEntry{ + Status: lnrpc.Payment_SUCCEEDED, + FullAmount: 600, + }, + hash2: &PaymentEntry{ + Status: lnrpc.Payment_SUCCEEDED, + FullAmount: 100, + }, + }) }) }