fix(backend): local payments - allow multiple outgoing payments pay into single incoming (#3428)

* fix(backend): allow paying multiple times into incoming payment

* chore(backend): add log for outgoing payment worker

* feat(integration): add CreateOutgoingPaymentFromIncomingPayment admin action

* test(integration): add & update integration tests for multiple outgoing payments into single incoming

* chore: lint

Author: mkurapov

packages/backend/src/open_payments/payment/outgoing/worker.ts

@@ -119,21 +119,25 @@ async function onLifecycleError(
   const error = typeof err === 'string' ? err : err.message
   const stateAttempts = payment.stateAttempts + 1
 
+  const errorDescription =
+    err instanceof PaymentMethodHandlerError ? err.description : undefined
+
+  const errLog = {
+    state: payment.state,
+    error,
+    stateAttempts,
+    errorDescription
+  }
+
   if (
     stateAttempts < deps.config.maxOutgoingPaymentRetryAttempts &&
     isRetryableError(err)
   ) {
-    deps.logger.warn(
-      { state: payment.state, error, stateAttempts },
-      'payment lifecycle failed; retrying'
-    )
+    deps.logger.warn(errLog, 'payment lifecycle failed; retrying')
     await payment.$query(deps.knex).patch({ stateAttempts })
   } else {
     // Too many attempts or non-retryable error; fail payment.
-    deps.logger.warn(
-      { state: payment.state, error, stateAttempts },
-      'payment lifecycle failed'
-    )
+    deps.logger.warn(errLog, 'payment lifecycle failed')
     await lifecycle.handleFailed(deps, payment, error)
   }
 }

packages/backend/src/payment-method/local/service.test.ts

@@ -466,8 +466,8 @@ describe('LocalPaymentService', (): void => {
       }
     })
 
-    test('throws error if incoming payment state is not pending', async (): Promise<void> => {
-      const { receiver, outgoingPayment } =
+    test('throws error if incoming payment is completed', async (): Promise<void> => {
+      const { receiver, outgoingPayment, incomingPayment } =
         await createOutgoingPaymentWithReceiver(deps, {
           sendingWalletAddress: walletAddressMap['USD'],
           receivingWalletAddress: walletAddressMap['USD'],
@@ -481,9 +481,51 @@ describe('LocalPaymentService', (): void => {
           }
         })
 
-      jest.spyOn(incomingPaymentService, 'get').mockResolvedValueOnce({
-        state: IncomingPaymentState.Processing
-      } as IncomingPayment)
+      incomingPayment.state = IncomingPaymentState.Completed
+
+      jest
+        .spyOn(incomingPaymentService, 'get')
+        .mockResolvedValueOnce(incomingPayment)
+
+      expect.assertions(4)
+      try {
+        await localPaymentService.pay({
+          receiver,
+          outgoingPayment,
+          finalDebitAmount: 100n,
+          finalReceiveAmount: 100n
+        })
+      } catch (err) {
+        expect(err).toBeInstanceOf(PaymentMethodHandlerError)
+        expect((err as PaymentMethodHandlerError).message).toBe(
+          'Bad Incoming Payment State'
+        )
+        expect((err as PaymentMethodHandlerError).description).toBe(
+          'Incoming Payment cannot be expired or completed'
+        )
+        expect((err as PaymentMethodHandlerError).retryable).toBe(false)
+      }
+    })
+
+    test('throws error if incoming payment is expired', async (): Promise<void> => {
+      const { receiver, outgoingPayment, incomingPayment } =
+        await createOutgoingPaymentWithReceiver(deps, {
+          sendingWalletAddress: walletAddressMap['USD'],
+          receivingWalletAddress: walletAddressMap['USD'],
+          method: 'ilp',
+          quoteOptions: {
+            debitAmount: {
+              value: 100n,
+              assetScale: walletAddressMap['USD'].asset.scale,
+              assetCode: walletAddressMap['USD'].asset.code
+            }
+          }
+        })
+
+      incomingPayment.state = IncomingPaymentState.Expired
+      jest
+        .spyOn(incomingPaymentService, 'get')
+        .mockResolvedValueOnce(incomingPayment)
 
       expect.assertions(4)
       try {
@@ -499,7 +541,7 @@ describe('LocalPaymentService', (): void => {
           'Bad Incoming Payment State'
         )
         expect((err as PaymentMethodHandlerError).description).toBe(
-          `Incoming Payment state should be ${IncomingPaymentState.Pending}`
+          'Incoming Payment cannot be expired or completed'
         )
         expect((err as PaymentMethodHandlerError).retryable).toBe(false)
       }

packages/backend/src/payment-method/local/service.ts

@@ -30,7 +30,6 @@ import {
   TransferError
 } from '../../accounting/errors'
 import { IncomingPaymentService } from '../../open_payments/payment/incoming/service'
-import { IncomingPaymentState } from '../../open_payments/payment/incoming/model'
 import { ConvertResults } from '../../rates/util'
 
 export interface LocalPaymentService extends PaymentMethodService {}
@@ -226,9 +225,9 @@ async function pay(
       retryable: false
     })
   }
-  if (incomingPayment.state !== IncomingPaymentState.Pending) {
+  if (incomingPayment.isExpiredOrComplete()) {
     throw new PaymentMethodHandlerError('Bad Incoming Payment State', {
-      description: `Incoming Payment state should be ${IncomingPaymentState.Pending}`,
+      description: `Incoming Payment cannot be expired or completed`,
       retryable: false
     })
   }

test/integration/integration.test.ts

@@ -1,6 +1,6 @@
 import assert from 'assert'
 import { MockASE, C9_CONFIG, HLB_CONFIG } from 'test-lib'
-import { Fee, WebhookEventType } from 'mock-account-service-lib'
+import { WebhookEventType } from 'mock-account-service-lib'
 import { poll } from './lib/utils'
 import { TestActions, createTestActions } from './lib/test-actions'
 import { IncomingPaymentState } from 'test-lib/dist/generated/graphql'
@@ -126,7 +126,6 @@ describe('Integration tests', (): void => {
         const outgoingPaymentGrant = await grantRequestOutgoingPayment(
           senderWalletAddress,
           {
-            debitAmount: quote.debitAmount,
             receiveAmount: quote.receiveAmount
           }
         )
@@ -202,10 +201,7 @@ describe('Integration tests', (): void => {
         )
         const outgoingPaymentGrant = await grantRequestOutgoingPayment(
           senderWalletAddress,
-          {
-            debitAmount: quote.debitAmount,
-            receiveAmount: quote.receiveAmount
-          },
+          { receiveAmount: quote.receiveAmount },
           {
             method: 'redirect',
             uri: 'https://example.com',
@@ -238,14 +234,15 @@ describe('Integration tests', (): void => {
 
         await getPublicIncomingPayment(incomingPayment.id, amountValueToSend)
       })
-      test('Open Payments without Quote', async (): Promise<void> => {
+      test('Open Payments Multiple Outgoing Payments into Incoming Payment', async (): Promise<void> => {
         const {
           grantRequestIncomingPayment,
           createIncomingPayment,
           grantRequestOutgoingPayment,
           pollGrantContinue,
           createOutgoingPayment,
-          getOutgoingPayment
+          getOutgoingPayment,
+          getPublicIncomingPayment
         } = testActions.openPayments
         const { consentInteraction } = testActions
 
@@ -264,11 +261,7 @@ describe('Integration tests', (): void => {
         })
         expect(senderWalletAddress.id).toBe(senderWalletAddressUrl)
 
-        const debitAmount = {
-          assetCode: senderWalletAddress.assetCode,
-          assetScale: senderWalletAddress.assetScale,
-          value: '500'
-        }
+        const grantValue = 100n
 
         const incomingPaymentGrant = await grantRequestIncomingPayment(
           receiverWalletAddress
@@ -281,13 +274,23 @@ describe('Integration tests', (): void => {
         const outgoingPaymentGrant = await grantRequestOutgoingPayment(
           senderWalletAddress,
           {
-            debitAmount,
-            receiveAmount: debitAmount
+            debitAmount: {
+              assetCode: senderWalletAddress.assetCode,
+              assetScale: senderWalletAddress.assetScale,
+              value: String(grantValue)
+            }
           }
         )
         await consentInteraction(outgoingPaymentGrant, senderWalletAddress)
         const grantContinue = await pollGrantContinue(outgoingPaymentGrant)
-        const outgoingPayment = await createOutgoingPayment(
+
+        const debitAmount = {
+          assetCode: senderWalletAddress.assetCode,
+          assetScale: senderWalletAddress.assetScale,
+          value: '50'
+        }
+
+        const outgoingPayment1 = await createOutgoingPayment(
           senderWalletAddress,
           grantContinue,
           {
@@ -296,12 +299,32 @@ describe('Integration tests', (): void => {
           }
         )
 
-        const outgoingPayment_ = await getOutgoingPayment(
-          outgoingPayment.id,
-          grantContinue
+        await poll(
+          async () => getOutgoingPayment(outgoingPayment1.id, grantContinue),
+          (responseData) => BigInt(responseData.sentAmount.value) > 0n,
+          5,
+          0.5
+        )
+
+        expect(outgoingPayment1.debitAmount).toMatchObject(debitAmount)
+
+        const outgoingPayment2 = await createOutgoingPayment(
+          senderWalletAddress,
+          grantContinue,
+          {
+            incomingPayment: incomingPayment.id,
+            debitAmount
+          }
+        )
+
+        await poll(
+          async () => getOutgoingPayment(outgoingPayment2.id, grantContinue),
+          (responseData) => BigInt(responseData.sentAmount.value) > 0n,
+          5,
+          0.5
         )
 
-        expect(outgoingPayment_.debitAmount).toMatchObject(debitAmount)
+        await getPublicIncomingPayment(incomingPayment.id, '98') // adjusted for ILP slippage
       })
       test('Peer to Peer', async (): Promise<void> => {
         const {
@@ -399,7 +422,6 @@ describe('Integration tests', (): void => {
         const receiverAssetCode = receiver.incomingAmount.assetCode
         const exchangeRate =
           hlb.config.seed.rates[senderAssetCode][receiverAssetCode]
-        const fee = c9.config.seed.fees.find((fee: Fee) => fee.asset === 'USD')
 
         // Expected amounts depend on the configuration of asset codes, scale, exchange rate, and fees.
         assert(receiverAssetCode === 'EUR')
@@ -409,11 +431,6 @@ describe('Integration tests', (): void => {
         )
         assert(senderWalletAddress.assetScale === 2)
         assert(exchangeRate === 0.91)
-        assert(fee)
-        assert(fee.fixed === 100)
-        assert(fee.basisPoints === 200)
-        assert(fee.asset === 'USD')
-        assert(fee.scale === 2)
         expect(completedOutgoingPayment.receiveAmount).toMatchObject({
           assetCode: 'EUR',
           assetScale: 2,
@@ -422,7 +439,7 @@ describe('Integration tests', (): void => {
         expect(completedOutgoingPayment.debitAmount).toMatchObject({
           assetCode: 'USD',
           assetScale: 2,
-          value: 668n
+          value: 556n // with ILP slippage
         })
         expect(completedOutgoingPayment.sentAmount).toMatchObject({
           assetCode: 'USD',
@@ -604,7 +621,6 @@ describe('Integration tests', (): void => {
         const receiverAssetCode = receiver.incomingAmount.assetCode
         const exchangeRate =
           c9.config.seed.rates[senderAssetCode][receiverAssetCode]
-        const fee = c9.config.seed.fees.find((fee: Fee) => fee.asset === 'USD')
 
         // Expected amounts depend on the configuration of asset codes, scale, exchange rate, and fees.
         assert(receiverAssetCode === 'EUR')
@@ -614,11 +630,6 @@ describe('Integration tests', (): void => {
         )
         assert(senderWalletAddress.assetScale === 2)
         assert(exchangeRate === 0.91)
-        assert(fee)
-        assert(fee.fixed === 100)
-        assert(fee.basisPoints === 200)
-        assert(fee.asset === 'USD')
-        assert(fee.scale === 2)
         expect(completedOutgoingPayment.receiveAmount).toMatchObject({
           assetCode: 'EUR',
           assetScale: 2,
@@ -627,7 +638,7 @@ describe('Integration tests', (): void => {
         expect(completedOutgoingPayment.debitAmount).toMatchObject({
           assetCode: 'USD',
           assetScale: 2,
-          value: 661n
+          value: 550n
         })
         expect(completedOutgoingPayment.sentAmount).toMatchObject({
           assetCode: 'USD',
@@ -644,6 +655,55 @@ describe('Integration tests', (): void => {
         })
         expect(incomingPayment.state).toBe(IncomingPaymentState.Completed)
       })
+
+      test('Peer to Peer - Multiple Outgoing Payments into Incoming Payment', async (): Promise<void> => {
+        const {
+          createReceiver,
+          createOutgoingPaymentFromIncomingPayment,
+          getIncomingPayment
+        } = testActions.admin
+
+        const senderWalletAddress = await c9.accounts.getByWalletAddressUrl(
+          'https://cloud-nine-wallet-test-backend:3100/accounts/gfranklin'
+        )
+        assert(senderWalletAddress?.walletAddressID)
+
+        const senderWalletAddressId = senderWalletAddress.walletAddressID
+        const createReceiverInput = {
+          metadata: {
+            description: 'For lunch!'
+          },
+          walletAddressUrl:
+            'https://cloud-nine-wallet-test-backend:3100/accounts/bhamchest'
+        }
+
+        const value = '500'
+        const receiver = await createReceiver(createReceiverInput)
+
+        await createOutgoingPaymentFromIncomingPayment({
+          incomingPayment: receiver.id,
+          walletAddressId: senderWalletAddressId,
+          debitAmount: {
+            assetCode: 'USD',
+            assetScale: 2,
+            value: value as unknown as bigint
+          }
+        })
+        await createOutgoingPaymentFromIncomingPayment({
+          incomingPayment: receiver.id,
+          walletAddressId: senderWalletAddressId,
+          debitAmount: {
+            assetCode: 'USD',
+            assetScale: 2,
+            value: value as unknown as bigint
+          }
+        })
+
+        const incomingPaymentId = receiver.id.split('/').slice(-1)[0]
+        const incomingPayment = await getIncomingPayment(incomingPaymentId)
+        expect(incomingPayment.receivedAmount.value).toBe(BigInt(value) * 2n)
+        expect(incomingPayment.state).toBe(IncomingPaymentState.Processing)
+      })
     })
   })
 })