From 6c97ba263da9460af9b58acb7ee23297ef7b4c50 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Thu, 3 Feb 2022 14:46:16 -0600 Subject: [PATCH 01/15] feat(backend): manage webhook event lifecycle in webhook service --- .../20211022210203_create_invoices_table.js | 2 - ...26122545_create_outgoing_payments_table.js | 1 - ...20131110501_create_webhook_events_table.js | 21 + packages/backend/src/app.ts | 21 + packages/backend/src/config/app.ts | 2 + .../src/graphql/generated/graphql.schema.json | 58 +++ .../backend/src/graphql/generated/graphql.ts | 16 + .../backend/src/graphql/resolvers/index.ts | 8 +- .../src/graphql/resolvers/liquidity.test.ts | 379 +++++++++++++++++- .../src/graphql/resolvers/liquidity.ts | 96 ++++- packages/backend/src/graphql/schema.graphql | 6 + packages/backend/src/index.ts | 1 + .../src/open_payments/invoice/model.ts | 2 - .../src/open_payments/invoice/service.test.ts | 69 +--- .../src/open_payments/invoice/service.ts | 89 ++-- .../backend/src/outgoing_payment/errors.ts | 18 +- .../backend/src/outgoing_payment/lifecycle.ts | 149 +++---- .../backend/src/outgoing_payment/model.ts | 12 - .../src/outgoing_payment/service.test.ts | 322 ++++++--------- .../backend/src/outgoing_payment/service.ts | 40 ++ .../backend/src/outgoing_payment/worker.ts | 14 +- packages/backend/src/webhook/model.ts | 98 +++++ packages/backend/src/webhook/service.test.ts | 276 ++++++++++--- packages/backend/src/webhook/service.ts | 281 +++++++------ 24 files changed, 1372 insertions(+), 609 deletions(-) create mode 100644 packages/backend/migrations/20220131110501_create_webhook_events_table.js create mode 100644 packages/backend/src/webhook/model.ts diff --git a/packages/backend/migrations/20211022210203_create_invoices_table.js b/packages/backend/migrations/20211022210203_create_invoices_table.js index 6b80caeb2b..013683cffc 100644 --- a/packages/backend/migrations/20211022210203_create_invoices_table.js +++ b/packages/backend/migrations/20211022210203_create_invoices_table.js @@ -12,8 +12,6 @@ exports.up = function (knex) { table.timestamp('processAt').nullable() - table.integer('webhookAttempts').notNullable().defaultTo(0) - table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) diff --git a/packages/backend/migrations/20211026122545_create_outgoing_payments_table.js b/packages/backend/migrations/20211026122545_create_outgoing_payments_table.js index 025a459fff..ec9f5cc2be 100644 --- a/packages/backend/migrations/20211026122545_create_outgoing_payments_table.js +++ b/packages/backend/migrations/20211026122545_create_outgoing_payments_table.js @@ -5,7 +5,6 @@ exports.up = function (knex) { table.string('state').notNullable().index() // PaymentState table.string('error').nullable() table.integer('stateAttempts').notNullable().defaultTo(0) - table.string('webhookId').nullable() table.string('intentPaymentPointer').nullable() table.string('intentInvoiceUrl').nullable() diff --git a/packages/backend/migrations/20220131110501_create_webhook_events_table.js b/packages/backend/migrations/20220131110501_create_webhook_events_table.js new file mode 100644 index 0000000000..e0e50b1157 --- /dev/null +++ b/packages/backend/migrations/20220131110501_create_webhook_events_table.js @@ -0,0 +1,21 @@ +exports.up = function (knex) { + return knex.schema.createTable('webhookEvents', function (table) { + table.uuid('id').notNullable().primary() + + table.string('type').notNullable() + table.jsonb('data').notNullable() + table.integer('attempts').notNullable().defaultTo(0) + table.string('error').nullable() + + table.timestamp('processAt').notNullable() + + table.timestamp('createdAt').defaultTo(knex.fn.now()) + table.timestamp('updatedAt').defaultTo(knex.fn.now()) + + table.index('processAt') + }) +} + +exports.down = function (knex) { + return knex.schema.dropTableIfExists('webhookEvents') +} diff --git a/packages/backend/src/app.ts b/packages/backend/src/app.ts index 272215b813..d7cedf9deb 100644 --- a/packages/backend/src/app.ts +++ b/packages/backend/src/app.ts @@ -143,6 +143,9 @@ export class App { for (let i = 0; i < this.config.invoiceWorkers; i++) { process.nextTick(() => this.processInvoice()) } + for (let i = 0; i < this.config.webhookWorkers; i++) { + process.nextTick(() => this.processWebhook()) + } } } @@ -288,4 +291,22 @@ export class App { ).unref() }) } + + private async processWebhook(): Promise { + const webhookService = await this.container.use('webhookService') + return webhookService + .processNext() + .catch((err) => { + this.logger.warn({ error: err.message }, 'processWebhook error') + return true + }) + .then((hasMoreWork) => { + if (hasMoreWork) process.nextTick(() => this.processWebhook()) + else + setTimeout( + () => this.processWebhook(), + this.config.webhookWorkerIdle + ).unref() + }) + } } diff --git a/packages/backend/src/config/app.ts b/packages/backend/src/config/app.ts index 41da1d1554..6eaac02db0 100644 --- a/packages/backend/src/config/app.ts +++ b/packages/backend/src/config/app.ts @@ -64,6 +64,8 @@ export const Config = { invoiceWorkers: envInt('INVOICE_WORKERS', 1), invoiceWorkerIdle: envInt('INVOICE_WORKER_IDLE', 200), // milliseconds + webhookWorkers: envInt('WEBHOOK_WORKERS', 1), + webhookWorkerIdle: envInt('WEBHOOK_WORKER_IDLE', 200), // milliseconds webhookUrl: envString('WEBHOOK_URL', 'http://127.0.0.1:4001/webhook'), webhookSecret: process.env.WEBHOOK_SECRET, // optional webhookTimeout: envInt('WEBHOOK_TIMEOUT', 2000), // milliseconds diff --git a/packages/backend/src/graphql/generated/graphql.schema.json b/packages/backend/src/graphql/generated/graphql.schema.json index 3951527a5f..35cd1107a1 100644 --- a/packages/backend/src/graphql/generated/graphql.schema.json +++ b/packages/backend/src/graphql/generated/graphql.schema.json @@ -2427,6 +2427,64 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "depositEventLiquidity", + "description": "Deposit webhook event liquidity", + "args": [ + { + "name": "eventId", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ], + "type": { + "kind": "OBJECT", + "name": "LiquidityMutationResponse", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "withdrawEventLiquidity", + "description": "Withdraw webhook event liquidity", + "args": [ + { + "name": "eventId", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ], + "type": { + "kind": "OBJECT", + "name": "LiquidityMutationResponse", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "createApiKey", "description": "Create API Key", diff --git a/packages/backend/src/graphql/generated/graphql.ts b/packages/backend/src/graphql/generated/graphql.ts index e6d7d16456..28af5a91fe 100644 --- a/packages/backend/src/graphql/generated/graphql.ts +++ b/packages/backend/src/graphql/generated/graphql.ts @@ -282,6 +282,10 @@ export type Mutation = { finalizeLiquidityWithdrawal?: Maybe; /** Rollback liquidity withdrawal */ rollbackLiquidityWithdrawal?: Maybe; + /** Deposit webhook event liquidity */ + depositEventLiquidity?: Maybe; + /** Withdraw webhook event liquidity */ + withdrawEventLiquidity?: Maybe; /** Create API Key */ createApiKey?: Maybe; /** Delete all API Keys */ @@ -365,6 +369,16 @@ export type MutationRollbackLiquidityWithdrawalArgs = { }; +export type MutationDepositEventLiquidityArgs = { + eventId: Scalars['String']; +}; + + +export type MutationWithdrawEventLiquidityArgs = { + eventId: Scalars['String']; +}; + + export type MutationCreateApiKeyArgs = { input: CreateApiKeyInput; }; @@ -963,6 +977,8 @@ export type MutationResolvers, ParentType, ContextType, RequireFields>; finalizeLiquidityWithdrawal?: Resolver, ParentType, ContextType, RequireFields>; rollbackLiquidityWithdrawal?: Resolver, ParentType, ContextType, RequireFields>; + depositEventLiquidity?: Resolver, ParentType, ContextType, RequireFields>; + withdrawEventLiquidity?: Resolver, ParentType, ContextType, RequireFields>; createApiKey?: Resolver, ParentType, ContextType, RequireFields>; deleteAllApiKeys?: Resolver, ParentType, ContextType, RequireFields>; redeemApiKey?: Resolver, ParentType, ContextType, RequireFields>; diff --git a/packages/backend/src/graphql/resolvers/index.ts b/packages/backend/src/graphql/resolvers/index.ts index 38f75d8db3..0ac2afe14f 100644 --- a/packages/backend/src/graphql/resolvers/index.ts +++ b/packages/backend/src/graphql/resolvers/index.ts @@ -24,7 +24,9 @@ import { createPeerLiquidityWithdrawal, createAccountWithdrawal, finalizeLiquidityWithdrawal, - rollbackLiquidityWithdrawal + rollbackLiquidityWithdrawal, + depositEventLiquidity, + withdrawEventLiquidity } from './liquidity' import { GraphQLBigInt } from '../scalars' import { refreshSession, revokeSession } from './session' @@ -70,6 +72,8 @@ export const resolvers: Resolvers = { createPeerLiquidityWithdrawal: createPeerLiquidityWithdrawal, createAccountWithdrawal, finalizeLiquidityWithdrawal: finalizeLiquidityWithdrawal, - rollbackLiquidityWithdrawal: rollbackLiquidityWithdrawal + rollbackLiquidityWithdrawal: rollbackLiquidityWithdrawal, + depositEventLiquidity, + withdrawEventLiquidity } } diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index 167b77ea2b..b424a2b0c9 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -1,27 +1,32 @@ import { gql } from 'apollo-server-koa' import Knex from 'knex' import { v4 as uuid } from 'uuid' +import * as Pay from '@interledger/pay' import { createTestApp, TestContainer } from '../../tests/app' import { IocContract } from '@adonisjs/fold' import { AppServices } from '../../app' import { initIocContainer } from '../..' import { Config } from '../../config/app' -import { AccountingService } from '../../accounting/service' +import { AccountingService, LiquidityAccount } from '../../accounting/service' import { Asset } from '../../asset/model' import { AssetService } from '../../asset/service' import { Account } from '../../open_payments/account/model' +import { Invoice } from '../../open_payments/invoice/model' +import { OutgoingPayment, PaymentState } from '../../outgoing_payment/model' import { Peer } from '../../peer/model' import { randomAsset } from '../../tests/asset' import { PeerFactory } from '../../tests/peerFactory' import { truncateTables } from '../../tests/tableManager' +import { DepositEventType, WithdrawEventType } from '../../webhook/model' +import { EventOptions, isPaymentEventType } from '../../webhook/service' import { LiquidityError, LiquidityMutationResponse, AccountWithdrawalMutationResponse } from '../generated/graphql' -describe('Withdrawal Resolvers', (): void => { +describe('Liquidity Resolvers', (): void => { let deps: IocContract let appContainer: TestContainer let accountingService: AccountingService @@ -1561,4 +1566,374 @@ describe('Withdrawal Resolvers', (): void => { }) } ) + + { + let invoice: Invoice + let payment: OutgoingPayment + + beforeEach( + async (): Promise => { + const accountService = await deps.use('accountService') + const { id: accountId } = await accountService.create({ + asset: randomAsset() + }) + const invoiceService = await deps.use('invoiceService') + invoice = await invoiceService.create({ + accountId, + amount: BigInt(56), + expiresAt: new Date(Date.now() + 60 * 1000), + description: 'description!' + }) + const outgoingPaymentService = await deps.use('outgoingPaymentService') + const config = await deps.use('config') + const invoiceUrl = `${config.publicHost}/invoices/${invoice.id}` + // create and then patch quote + payment = await outgoingPaymentService.create({ + accountId, + invoiceUrl, + autoApprove: false + }) + await payment.$query(knex).patch({ + state: PaymentState.Funding, + quote: { + timestamp: new Date(), + activationDeadline: new Date(Date.now() + 1000), + targetType: Pay.PaymentType.FixedSend, + minDeliveryAmount: BigInt(123), + maxSourceAmount: BigInt(456), + maxPacketAmount: BigInt(789), + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + minExchangeRate: Pay.Ratio.from(1.23)!, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + lowExchangeRateEstimate: Pay.Ratio.from(1.2)!, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + highExchangeRateEstimate: Pay.Ratio.from(2.3)!, + amountSent: BigInt(0) + } + }) + } + ) + + describe('depositEventLiquidity', (): void => { + describe.each(Object.values(DepositEventType).map((type) => [type]))( + '%s', + (type): void => { + let eventId: string + + beforeEach( + async (): Promise => { + eventId = uuid() + const webhookService = await deps.use('webhookService') + await webhookService.createEvent({ + id: eventId, + type, + payment, + amountSent: BigInt(0), + balance: BigInt(0) + }) + } + ) + + test('Can deposit account liquidity', async (): Promise => { + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation DepositLiquidity($eventId: String!) { + depositEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.depositEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(true) + expect(response.code).toEqual('200') + expect(response.error).toBeNull() + }) + + test("Can't deposit for non-existent webhook event id", async (): Promise => { + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation DepositLiquidity($eventId: String!) { + depositEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId: uuid() + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.depositEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(false) + expect(response.code).toEqual('400') + expect(response.message).toEqual('Invalid id') + expect(response.error).toEqual(LiquidityError.InvalidId) + }) + + test('Returns an error for existing transfer', async (): Promise => { + await expect( + accountingService.createDeposit({ + id: eventId, + account: invoice, + amount: BigInt(100) + }) + ).resolves.toBeUndefined() + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation DepositLiquidity($eventId: String!) { + depositEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.depositEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(false) + expect(response.code).toEqual('409') + expect(response.message).toEqual('Transfer exists') + expect(response.error).toEqual(LiquidityError.TransferExists) + }) + } + ) + }) + + describe('withdrawEventLiquidity', (): void => { + describe.each(Object.values(WithdrawEventType).map((type) => [type]))( + '%s', + (type): void => { + let eventId: string + + beforeEach( + async (): Promise => { + const webhookService = await deps.use('webhookService') + + eventId = uuid() + const amount = BigInt(10) + let account: LiquidityAccount + let options: EventOptions + if (isPaymentEventType(type)) { + account = payment + options = { + id: eventId, + type, + payment, + amountSent: BigInt(0), + balance: amount + } + } else { + account = invoice + options = { + id: eventId, + type, + invoice, + amountReceived: amount + } + } + await expect( + accountingService.createDeposit({ + id: uuid(), + account, + amount + }) + ).resolves.toBeUndefined() + await expect( + accountingService.createWithdrawal({ + id: eventId, + account, + amount, + timeout + }) + ).resolves.toBeUndefined() + await expect( + accountingService.getBalance(account.id) + ).resolves.toEqual(BigInt(0)) + await webhookService.createEvent(options) + } + ) + + test('Can withdraw account liquidity', async (): Promise => { + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation WithdrawLiquidity($eventId: String!) { + withdrawEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.withdrawEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(true) + expect(response.code).toEqual('200') + expect(response.error).toBeNull() + }) + + test('Returns error for non-existent webhook event id', async (): Promise => { + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation WithdrawLiquidity($eventId: String!) { + withdrawEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId: uuid() + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.withdrawEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(false) + expect(response.code).toEqual('400') + expect(response.message).toEqual('Invalid id') + expect(response.error).toEqual(LiquidityError.InvalidId) + }) + + test('Returns error for finalized withdrawal', async (): Promise => { + await expect( + accountingService.commitWithdrawal(eventId) + ).resolves.toBeUndefined() + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation WithdrawLiquidity($eventId: String!) { + withdrawEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.withdrawEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(false) + expect(response.code).toEqual('409') + expect(response.message).toEqual('Withdrawal already finalized') + expect(response.error).toEqual(LiquidityError.AlreadyCommitted) + }) + + test('Returns error for rolled back withdrawal', async (): Promise => { + await expect( + accountingService.rollbackWithdrawal(eventId) + ).resolves.toBeUndefined() + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation WithdrawLiquidity($eventId: String!) { + withdrawEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.withdrawEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(false) + expect(response.code).toEqual('409') + expect(response.message).toEqual('Withdrawal already rolled back') + expect(response.error).toEqual(LiquidityError.AlreadyRolledBack) + }) + } + ) + }) + } }) diff --git a/packages/backend/src/graphql/resolvers/liquidity.ts b/packages/backend/src/graphql/resolvers/liquidity.ts index 78821b12d3..7a0132a7b7 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.ts @@ -1,3 +1,4 @@ +import assert from 'assert' import { ResolversTypes, MutationResolvers, @@ -5,8 +6,9 @@ import { LiquidityMutationResponse, AccountWithdrawalMutationResponse } from '../generated/graphql' -import { TransferError } from '../../accounting/errors' import { ApolloContext } from '../../app' +import { FundingError, isFundingError } from '../../outgoing_payment/errors' +import { DepositEventType, WithdrawEventType } from '../../webhook/model' export const addPeerLiquidity: MutationResolvers['addPeerLiquidity'] = async ( parent, @@ -278,11 +280,101 @@ export const rollbackLiquidityWithdrawal: MutationResolvers['roll } } +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +const isDepositEventType = (o: any): o is DepositEventType => + Object.values(DepositEventType).includes(o) + +export const depositEventLiquidity: MutationResolvers['depositEventLiquidity'] = async ( + parent, + args, + ctx +): ResolversTypes['LiquidityMutationResponse'] => { + try { + const webhookService = await ctx.container.use('webhookService') + const event = await webhookService.getEvent(args.eventId) + if (!event || !isDepositEventType(event.type)) { + return responses[LiquidityError.InvalidId] + } + assert.ok(event.data.payment?.quote?.maxSourceAmount) + const outgoingPaymentService = await ctx.container.use( + 'outgoingPaymentService' + ) + const paymentOrErr = await outgoingPaymentService.fund({ + id: event.data.payment.id, + amount: BigInt(event.data.payment.quote.maxSourceAmount), + transferId: event.id + }) + if (isFundingError(paymentOrErr)) { + return errorToResponse(paymentOrErr) + } + return { + code: '200', + success: true, + message: 'Deposited liquidity' + } + } catch (error) { + ctx.logger.error( + { + eventId: args.eventId, + error + }, + 'error depositing liquidity' + ) + return { + code: '400', + message: 'Error trying to deposit liquidity', + success: false + } + } +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +const isWithdrawEventType = (o: any): o is WithdrawEventType => + Object.values(WithdrawEventType).includes(o) + +export const withdrawEventLiquidity: MutationResolvers['withdrawEventLiquidity'] = async ( + parent, + args, + ctx +): ResolversTypes['LiquidityMutationResponse'] => { + try { + const webhookService = await ctx.container.use('webhookService') + const event = await webhookService.getEvent(args.eventId) + if (!event || !isWithdrawEventType(event.type)) { + return responses[LiquidityError.InvalidId] + } + const accountingService = await ctx.container.use('accountingService') + const error = await accountingService.commitWithdrawal(args.eventId) + if (error) { + return errorToResponse(error) + } + // TODO: check for and handle leftover invoice or payment balance + return { + code: '200', + success: true, + message: 'Withdrew liquidity' + } + } catch (error) { + ctx.logger.error( + { + eventId: args.eventId, + error + }, + 'error withdrawing liquidity' + ) + return { + code: '400', + message: 'Error trying to withdraw liquidity', + success: false + } + } +} + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types const isLiquidityError = (o: any): o is LiquidityError => Object.values(LiquidityError).includes(o) -const errorToResponse = (error: TransferError): LiquidityMutationResponse => { +const errorToResponse = (error: FundingError): LiquidityMutationResponse => { if (!isLiquidityError(error)) { throw new Error(error) } diff --git a/packages/backend/src/graphql/schema.graphql b/packages/backend/src/graphql/schema.graphql index 6f557fe573..93d2e128a7 100644 --- a/packages/backend/src/graphql/schema.graphql +++ b/packages/backend/src/graphql/schema.graphql @@ -77,6 +77,12 @@ type Mutation { withdrawalId: String! ): LiquidityMutationResponse + "Deposit webhook event liquidity" + depositEventLiquidity(eventId: String!): LiquidityMutationResponse + + "Withdraw webhook event liquidity" + withdrawEventLiquidity(eventId: String!): LiquidityMutationResponse + "Create API Key" createApiKey(input: CreateApiKeyInput!): CreateApiKeyMutationResponse diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index d17ab4be14..d0870bb6db 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -177,6 +177,7 @@ export function initIocContainer( container.singleton('webhookService', async (deps) => { return createWebhookService({ config: await deps.use('config'), + knex: await deps.use('knex'), logger: await deps.use('logger') }) }) diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index 7c31a2b33d..6e0e065a89 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -33,8 +33,6 @@ export class Invoice public processAt!: Date | null - public webhookAttempts!: number - public get asset(): Asset { return this.account.asset } diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index 83a5693dd7..e1d528041e 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -1,8 +1,6 @@ import assert from 'assert' import Knex from 'knex' import { WorkerUtils, makeWorkerUtils } from 'graphile-worker' -import nock from 'nock' -import { URL } from 'url' import { v4 as uuid } from 'uuid' import { InvoiceService } from './service' @@ -17,7 +15,7 @@ import { initIocContainer } from '../../' import { AppServices } from '../../app' import { randomAsset } from '../../tests/asset' import { truncateTables } from '../../tests/tableManager' -import { EventType } from '../../webhook/service' +import { WebhookEvent, EventType } from '../../webhook/model' describe('Invoice Service', (): void => { let deps: IocContract @@ -31,22 +29,6 @@ describe('Invoice Service', (): void => { const mockMessageProducer = { send: jest.fn() } - const webhookUrl = new URL(Config.webhookUrl) - - function mockWebhookServer( - invoiceId: string, - type: EventType, - status = 200 - ): nock.Scope { - return nock(webhookUrl.origin) - .post(webhookUrl.pathname, (body): boolean => { - expect(body.type).toEqual(type) - expect(body.data.invoice.id).toEqual(invoiceId) - expect(body.data.invoice.active).toEqual(false) - return true - }) - .reply(status) - } beforeAll( async (): Promise => { @@ -235,12 +217,12 @@ describe('Invoice Service', (): void => { }) describe.each` - event | expiresAt | amountReceived + type | expiresAt | amountReceived ${EventType.InvoiceExpired} | ${-40_000} | ${BigInt(1)} ${EventType.InvoicePaid} | ${30_000} | ${BigInt(123)} `( - 'handleDeactivated ($event)', - ({ event, expiresAt, amountReceived }): void => { + 'handleDeactivated ($type)', + ({ type, expiresAt, amountReceived }): void => { let invoice: Invoice beforeEach( @@ -258,7 +240,7 @@ describe('Invoice Service', (): void => { amount: amountReceived }) ).resolves.toBeUndefined() - if (event === EventType.InvoiceExpired) { + if (type === EventType.InvoiceExpired) { await expect(invoiceService.processNext()).resolves.toBe( invoice.id ) @@ -277,44 +259,31 @@ describe('Invoice Service', (): void => { } ) - test('Withdraws invoice liquidity', async (): Promise => { - const scope = mockWebhookServer(invoice.id, event) - await expect(invoiceService.processNext()).resolves.toBe(invoice.id) - expect(scope.isDone()).toBe(true) - await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - processAt: null, - webhookAttempts: 0 - }) + test('Creates liquidity withdrawal and webhook event', async (): Promise => { await expect( - accountingService.getBalance(invoice.id) - ).resolves.toEqual(BigInt(0)) - }) - - test("Doesn't withdraw on webhook error", async (): Promise => { - assert.ok(invoice.processAt) - const scope = mockWebhookServer(invoice.id, event, 504) + WebhookEvent.query(knex).where({ type }) + ).resolves.toHaveLength(0) await expect(invoiceService.processNext()).resolves.toBe(invoice.id) - expect(scope.isDone()).toBe(true) + await expect( + WebhookEvent.query(knex) + .whereJsonSupersetOf('data:invoice', { + id: invoice.id + }) + .where({ type }) + ).resolves.toHaveLength(1) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - processAt: new Date(invoice.processAt.getTime() + 10_000), - webhookAttempts: 1 + processAt: null }) await expect( accountingService.getBalance(invoice.id) - ).resolves.toEqual(amountReceived) + ).resolves.toEqual(BigInt(0)) }) - test("Doesn't withdraw on webhook timeout", async (): Promise => { + test.skip("Doesn't withdraw on webhook error", async (): Promise => { assert.ok(invoice.processAt) - const scope = nock(webhookUrl.origin) - .post(webhookUrl.pathname) - .delayConnection(Config.webhookTimeout + 1) - .reply(200) await expect(invoiceService.processNext()).resolves.toBe(invoice.id) - expect(scope.isDone()).toBe(true) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - processAt: new Date(invoice.processAt.getTime() + 10_000), - webhookAttempts: 1 + processAt: invoice.processAt }) await expect( accountingService.getBalance(invoice.id) diff --git a/packages/backend/src/open_payments/invoice/service.ts b/packages/backend/src/open_payments/invoice/service.ts index 5edd26d098..820560eb76 100644 --- a/packages/backend/src/open_payments/invoice/service.ts +++ b/packages/backend/src/open_payments/invoice/service.ts @@ -2,10 +2,12 @@ import { Invoice } from './model' import { AccountingService } from '../../accounting/service' import { BaseService } from '../../shared/baseService' import { Pagination } from '../../shared/pagination' -import { EventType, WebhookService } from '../../webhook/service' +import { EventType } from '../../webhook/model' +import { WebhookService, RETRY_LIMIT_MS } from '../../webhook/service' import assert from 'assert' import { Transaction } from 'knex' import { ForeignKeyViolationError, TransactionOrKnex } from 'objection' +import { v4 as uuid } from 'uuid' export const POSITIVE_SLIPPAGE = BigInt(1) // First retry waits 10 seconds @@ -184,66 +186,47 @@ async function handleDeactivated( invoice: Invoice ): Promise { assert.ok(invoice.processAt) - try { - const amountReceived = await deps.accountingService.getTotalReceived( - invoice.id - ) - if (!amountReceived) { - deps.logger.warn( - { amountReceived }, - 'invoice with processAt and empty balance' - ) - await invoice.$query(deps.knex).patch({ processAt: null }) - return - } - - deps.logger.trace( + const amountReceived = await deps.accountingService.getTotalReceived( + invoice.id + ) + if (!amountReceived) { + deps.logger.warn( { amountReceived }, - 'withdrawing deactivated invoice balance' + 'invoice with processAt and empty balance' ) - const error = await deps.accountingService.createWithdrawal({ - id: invoice.id, - account: invoice, - amount: amountReceived, - timeout: BigInt(deps.webhookService.timeout) * BigInt(1e6) // ms -> ns - }) - if (error) throw error + await invoice.$query(deps.knex).patch({ processAt: null }) + return + } + + deps.logger.trace( + { amountReceived }, + 'withdrawing deactivated invoice balance' + ) - const { status } = await deps.webhookService.send({ - id: invoice.id, + await invoice.$query(deps.knex).patch({ + processAt: null + }) + + const id = uuid() + await deps.webhookService.createEvent( + { + id, + invoice, type: amountReceived < invoice.amount ? EventType.InvoiceExpired : EventType.InvoicePaid, - invoice, amountReceived - }) - if (status === 200 || status === 205) { - const error = await deps.accountingService.commitWithdrawal(invoice.id) - if (error) throw error - if (status === 200) { - await invoice.$query(deps.knex).patch({ - processAt: null - }) - } - } - } catch (error) { - const webhookAttempts = invoice.webhookAttempts + 1 - deps.logger.warn( - { error, webhookAttempts }, - 'webhook attempt failed; retrying' - ) - await deps.accountingService.rollbackWithdrawal(invoice.id) - - const processAt = new Date( - invoice.processAt.getTime() + - Math.min(webhookAttempts, 6) * RETRY_BACKOFF_MS - ) - await invoice.$query(deps.knex).patch({ - processAt, - webhookAttempts - }) - } + }, + deps.knex as Transaction + ) + const error = await deps.accountingService.createWithdrawal({ + id, + account: invoice, + amount: amountReceived, + timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns + }) + if (error) throw new Error(error) } /** TODO: Base64 encode/decode the cursors diff --git a/packages/backend/src/outgoing_payment/errors.ts b/packages/backend/src/outgoing_payment/errors.ts index eb4d1ca7c5..6e719cbb0e 100644 --- a/packages/backend/src/outgoing_payment/errors.ts +++ b/packages/backend/src/outgoing_payment/errors.ts @@ -1,13 +1,26 @@ import * as Pay from '@interledger/pay' +import { TransferError } from '../accounting/errors' + +enum OutgoingPaymentError { + UnknownPayment = 'UnknownPayment', + WrongState = 'WrongState', + InvalidAmount = 'InvalidAmount' +} + +export const FundingError = { ...OutgoingPaymentError, ...TransferError } +export type FundingError = OutgoingPaymentError | TransferError + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +export const isFundingError = (o: any): o is FundingError => + Object.values(FundingError).includes(o) + export type PaymentError = LifecycleError | Pay.PaymentError export enum LifecycleError { QuoteExpired = 'QuoteExpired', // Rate fetch failed. PricesUnavailable = 'PricesUnavailable', - // Payment aborted via outgoing_payment.funding webhook response. - CancelledByWebhook = 'CancelledByWebhook', // Edge error due to retries, partial payment, and database write errors. BadState = 'BadState', @@ -15,7 +28,6 @@ export enum LifecycleError { MissingBalance = 'MissingBalance', MissingQuote = 'MissingQuote', MissingInvoice = 'MissingInvoice', - MissingWebhook = 'MissingWebhook', InvalidRatio = 'InvalidRatio' } diff --git a/packages/backend/src/outgoing_payment/lifecycle.ts b/packages/backend/src/outgoing_payment/lifecycle.ts index 7490927c53..e431314f35 100644 --- a/packages/backend/src/outgoing_payment/lifecycle.ts +++ b/packages/backend/src/outgoing_payment/lifecycle.ts @@ -1,10 +1,14 @@ import * as Pay from '@interledger/pay' +import assert from 'assert' +import { Transaction } from 'knex' +import { v4 as uuid } from 'uuid' import { LifecycleError } from './errors' import { OutgoingPayment, PaymentState } from './model' import { ServiceDependencies } from './service' import { IlpPlugin } from './ilp_plugin' -import { EventType } from '../webhook/service' +import { PaymentEventType } from '../webhook/model' +import { RETRY_LIMIT_MS } from '../webhook/service' const MAX_INT64 = BigInt('9223372036854775807') @@ -60,9 +64,7 @@ export async function handleQuoting( }, 'quote amountToSend bounds error' ) - await payment.$query(deps.knex).patch({ - state: PaymentState.Completed - }) + await handleCompleted(deps, payment) return } } @@ -96,9 +98,7 @@ export async function handleQuoting( // InvoiceAlreadyPaid: the invoice was already paid, either by this payment (which retried due to a failed SENDING→COMPLETED transition commit) or another payment entirely. if (quote === null) { deps.logger.warn('quote invoice already paid') - await payment.$query(deps.knex).patch({ - state: PaymentState.Completed - }) + await handleCompleted(deps, payment) return } @@ -129,6 +129,10 @@ export async function handleQuoting( amountSent } }) + + if (state === PaymentState.Funding) { + await sendWebhookEvent(deps, payment, PaymentEventType.PaymentFunding) + } } // "payment" is locked by the "deps.knex" transaction. @@ -142,44 +146,13 @@ export async function handleFunding( throw LifecycleError.QuoteExpired } - if (!payment.webhookId) throw LifecycleError.MissingWebhook - - const amountSent = await deps.accountingService.getTotalSent(payment.id) - const balance = await deps.accountingService.getBalance(payment.id) - if (amountSent === undefined || balance === undefined) { - throw LifecycleError.MissingBalance - } - - try { - const { status } = await deps.webhookService.send({ - id: payment.webhookId, - type: EventType.PaymentFunding, - payment, - amountSent, - balance - }) - - if (status === 200) { - const error = await deps.accountingService.createDeposit({ - id: payment.webhookId, - account: payment, - amount: payment.quote.maxSourceAmount - }) - if (error) { - throw new Error('Unable to fund payment. error=' + error) - } - await payment.$query(deps.knex).patch({ state: PaymentState.Sending }) - } - } catch (err) { - if (err.isAxiosError && err.response.status === 403) { - await payment.$query(deps.knex).patch({ - state: PaymentState.Cancelled, - error: LifecycleError.CancelledByWebhook - }) - } else { - throw err - } - } + deps.logger.error( + { + activationDeadline: payment.quote.activationDeadline.getTime(), + now: now.getTime() + }, + "handleFunding for payment quote that isn't expired" + ) } // "payment" is locked by the "deps.knex" transaction. @@ -260,9 +233,7 @@ export async function handleSending( }, 'handleSending payment was already paid' ) - await payment.$query(deps.knex).patch({ - state: PaymentState.Completed - }) + await handleCompleted(deps, payment) return } else if ( newMaxSourceAmount <= BigInt(0) || @@ -335,65 +306,63 @@ export async function handleSending( if (receipt.error) throw receipt.error + await handleCompleted(deps, payment) +} + +export async function handleCancelled( + deps: ServiceDependencies, + payment: OutgoingPayment, + error: string +): Promise { await payment.$query(deps.knex).patch({ - state: PaymentState.Completed + state: PaymentState.Cancelled, + error }) + await sendWebhookEvent(deps, payment, PaymentEventType.PaymentCancelled) } -export async function handleCancelledOrCompleted( +const handleCompleted = async ( deps: ServiceDependencies, payment: OutgoingPayment -): Promise { +): Promise => { + await payment.$query(deps.knex).patch({ + state: PaymentState.Completed + }) + await sendWebhookEvent(deps, payment, PaymentEventType.PaymentCompleted) +} + +const sendWebhookEvent = async ( + deps: ServiceDependencies, + payment: OutgoingPayment, + type: PaymentEventType +): Promise => { const amountSent = await deps.accountingService.getTotalSent(payment.id) const balance = await deps.accountingService.getBalance(payment.id) if (amountSent === undefined || balance === undefined) { throw LifecycleError.MissingBalance } - if (!payment.webhookId) throw LifecycleError.MissingWebhook + const id = uuid() + + await deps.webhookService.createEvent( + { + id, + payment, + type, + amountSent, + balance + }, + deps.knex as Transaction + ) if (balance) { + assert.ok(type !== PaymentEventType.PaymentFunding) const error = await deps.accountingService.createWithdrawal({ - id: payment.webhookId, + id, account: payment, amount: balance, - timeout: BigInt(deps.webhookService.timeout) * BigInt(1e6) // ms -> ns - }) - if (error) throw error - } - - try { - const { status } = await deps.webhookService.send({ - id: payment.webhookId, - type: - payment.state === PaymentState.Cancelled - ? EventType.PaymentCancelled - : EventType.PaymentCompleted, - payment, - amountSent, - balance + timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns }) - if (status === 200 || status === 205) { - if (balance) { - const error = await deps.accountingService.commitWithdrawal( - payment.webhookId - ) - if (error) throw error - } - if (status === 200) { - await payment.$query(deps.knex).patch({ - webhookId: null - }) - } else if (payment.state === PaymentState.Cancelled) { - await payment.$query(deps.knex).patch({ - state: PaymentState.Quoting - }) - } - } - } catch (error) { - if (balance) { - await deps.accountingService.rollbackWithdrawal(payment.webhookId) - } - throw error + if (error) throw new Error(error) } } diff --git a/packages/backend/src/outgoing_payment/model.ts b/packages/backend/src/outgoing_payment/model.ts index baefeb825e..8106799d6f 100644 --- a/packages/backend/src/outgoing_payment/model.ts +++ b/packages/backend/src/outgoing_payment/model.ts @@ -1,6 +1,5 @@ import { Pojo, Model, ModelOptions, QueryContext } from 'objection' import * as Pay from '@interledger/pay' -import { v4 as uuid } from 'uuid' import { LiquidityAccount } from '../accounting/service' import { Asset } from '../asset/model' @@ -32,8 +31,6 @@ export class OutgoingPayment // The "| null" is necessary so that `$beforeUpdate` can modify a patch to remove the error. If `$beforeUpdate` set `error = undefined`, the patch would ignore the modification. public error?: string | null public stateAttempts!: number - // The "| null" is necessary so that `$beforeUpdate` can modify a patch to remove the webhookId. If `$beforeUpdate` set `webhookId = undefined`, the patch would ignore the modification. - public webhookId?: string | null public intent!: PaymentIntent @@ -84,15 +81,6 @@ export class OutgoingPayment } if (opts.old['state'] !== this.state) { this.stateAttempts = 0 - switch (this.state) { - case PaymentState.Funding: - case PaymentState.Cancelled: - case PaymentState.Completed: - this.webhookId = uuid() - break - default: - this.webhookId = null - } } } } diff --git a/packages/backend/src/outgoing_payment/service.test.ts b/packages/backend/src/outgoing_payment/service.test.ts index 67423aca58..32326162e8 100644 --- a/packages/backend/src/outgoing_payment/service.test.ts +++ b/packages/backend/src/outgoing_payment/service.test.ts @@ -2,9 +2,9 @@ import assert from 'assert' import nock from 'nock' import Knex from 'knex' import * as Pay from '@interledger/pay' -import { URL } from 'url' import { v4 as uuid } from 'uuid' +import { FundingError } from './errors' import { OutgoingPaymentService } from './service' import { createTestApp, TestContainer } from '../tests/app' import { IAppConfig, Config } from '../config/app' @@ -17,10 +17,11 @@ import { LifecycleError } from './errors' import { RETRY_BACKOFF_SECONDS } from './worker' import { isTransferError } from '../accounting/errors' import { AccountingService, TransferOptions } from '../accounting/service' +import { uuidToBigInt } from '../accounting/utils' import { AssetOptions } from '../asset/service' import { Invoice } from '../open_payments/invoice/model' import { RatesService } from '../rates/service' -import { EventType } from '../webhook/service' +import { WebhookEvent, EventType } from '../webhook/model' describe('OutgoingPaymentService', (): void => { let deps: IocContract @@ -38,39 +39,14 @@ describe('OutgoingPaymentService', (): void => { let amtDelivered: bigint let config: IAppConfig - const webhookUrl = new URL(Config.webhookUrl) - - enum WebhookState { - Funding = PaymentState.Funding, - Cancelled = PaymentState.Cancelled, - Completed = PaymentState.Completed - } - - const isWebhookState = (state: PaymentState): boolean => - Object.values(WebhookState).includes(state) - const webhookTypes: { - [key in WebhookState]: EventType + [key in PaymentState]: EventType | undefined } = { - [WebhookState.Funding]: EventType.PaymentFunding, - [WebhookState.Cancelled]: EventType.PaymentCancelled, - [WebhookState.Completed]: EventType.PaymentCompleted - } - - function mockWebhookServer( - paymentId: string, - state: PaymentState, - status = 200 - ): nock.Scope { - assert.ok(isWebhookState(state)) - return nock(webhookUrl.origin) - .post(webhookUrl.pathname, (body): boolean => { - expect(body.type).toEqual(webhookTypes[state]) - expect(body.data.payment.id).toEqual(paymentId) - expect(body.data.payment.state).toEqual(state) - return true - }) - .reply(status) + [PaymentState.Quoting]: undefined, + [PaymentState.Funding]: EventType.PaymentFunding, + [PaymentState.Sending]: undefined, + [PaymentState.Cancelled]: EventType.PaymentCancelled, + [PaymentState.Completed]: EventType.PaymentCompleted } async function processNext( @@ -83,6 +59,16 @@ describe('OutgoingPaymentService', (): void => { if (!payment) throw 'no payment' if (expectState) expect(payment.state).toBe(expectState) expect(payment.error).toEqual(expectedError || null) + const type = webhookTypes[payment.state] + if (type) { + await expect( + WebhookEvent.query(knex) + .whereJsonSupersetOf('data:payment', { + id: payment.id + }) + .andWhere({ type }) + ).resolves.not.toHaveLength(0) + } return payment } @@ -144,12 +130,14 @@ describe('OutgoingPaymentService', (): void => { amountSent, amountDelivered, accountBalance, - invoiceReceived + invoiceReceived, + withdrawAmount }: { amountSent?: bigint amountDelivered?: bigint accountBalance?: bigint invoiceReceived?: bigint + withdrawAmount?: bigint } ) { if (amountSent !== undefined) { @@ -170,6 +158,16 @@ describe('OutgoingPaymentService', (): void => { accountingService.getTotalReceived(invoice.id) ).resolves.toEqual(invoiceReceived) } + if (withdrawAmount !== undefined) { + const tigerbeetle = await deps.use('tigerbeetle') + await expect( + tigerbeetle.lookupAccounts([uuidToBigInt(payment.id)]) + ).resolves.toMatchObject([ + { + debits_reserved: withdrawAmount + } + ]) + } } beforeAll( @@ -522,7 +520,6 @@ describe('OutgoingPaymentService', (): void => { autoApprove: true }) payment = await processNext(paymentId, PaymentState.Funding) - expect(payment.webhookId).not.toBeNull() } ) @@ -544,42 +541,6 @@ describe('OutgoingPaymentService', (): void => { LifecycleError.QuoteExpired ) }) - - it('CANCELLED (wallet cancelled)', async (): Promise => { - const scope = mockWebhookServer(payment.id, PaymentState.Funding, 403) - await processNext( - payment.id, - PaymentState.Cancelled, - LifecycleError.CancelledByWebhook - ) - expect(scope.isDone()).toBe(true) - }) - - it('SENDING (payment funded)', async (): Promise => { - const scope = mockWebhookServer(payment.id, PaymentState.Funding) - await processNext(payment.id, PaymentState.Sending) - expect(scope.isDone()).toBe(true) - await expectOutcome(payment, { - accountBalance: BigInt(123), - amountSent: BigInt(0), - amountDelivered: BigInt(0) - }) - }) - - it('FUNDING (webhook error)', async (): Promise => { - const scope = mockWebhookServer(payment.id, PaymentState.Funding, 504) - await processNext(payment.id, PaymentState.Funding) - expect(scope.isDone()).toBe(true) - }) - - it('FUNDING (webhook timeout)', async (): Promise => { - const scope = nock(webhookUrl.origin) - .post(webhookUrl.pathname) - .delayConnection(Config.webhookTimeout + 1) - .reply(200) - await processNext(payment.id, PaymentState.Funding) - expect(scope.isDone()).toBe(true) - }) }) describe('SENDING→', (): void => { @@ -607,10 +568,17 @@ describe('OutgoingPaymentService', (): void => { trackAmountDelivered(paymentId) - await processNext(paymentId, PaymentState.Funding) - const scope = mockWebhookServer(paymentId, PaymentState.Funding) - await processNext(paymentId, PaymentState.Sending) - expect(scope.isDone()).toBe(true) + const payment = await processNext(paymentId, PaymentState.Funding) + assert.ok(payment.quote) + await expect( + outgoingPaymentService.fund({ + id: paymentId, + amount: payment.quote.maxSourceAmount, + transferId: uuid() + }) + ).resolves.toMatchObject({ + state: PaymentState.Sending + }) return paymentId } @@ -638,10 +606,11 @@ describe('OutgoingPaymentService', (): void => { if (!payment.quote) throw 'no quote' const amountSent = invoice.amount * BigInt(2) await expectOutcome(payment, { - accountBalance: payment.quote.maxSourceAmount - amountSent, + accountBalance: BigInt(0), amountSent, amountDelivered: invoice.amount, - invoiceReceived: invoice.amount + invoiceReceived: invoice.amount, + withdrawAmount: payment.quote.maxSourceAmount - amountSent }) }) @@ -656,10 +625,11 @@ describe('OutgoingPaymentService', (): void => { if (!payment.quote) throw 'no quote' const amountSent = (invoice.amount - amountAlreadyDelivered) * BigInt(2) await expectOutcome(payment, { - accountBalance: payment.quote.maxSourceAmount - amountSent, + accountBalance: BigInt(0), amountSent, amountDelivered: invoice.amount - amountAlreadyDelivered, - invoiceReceived: invoice.amount + invoiceReceived: invoice.amount, + withdrawAmount: payment.quote.maxSourceAmount - amountSent }) }) @@ -696,9 +666,10 @@ describe('OutgoingPaymentService', (): void => { expect(payment.stateAttempts).toBe(0) // "mockPay" allows a small amount of money to be paid every attempt. await expectOutcome(payment, { - accountBalance: BigInt(123 - 10 * 5), + accountBalance: BigInt(0), amountSent: BigInt(10 * 5), - amountDelivered: BigInt(5 * 5) + amountDelivered: BigInt(5 * 5), + withdrawAmount: BigInt(123 - 10 * 5) }) }) @@ -721,9 +692,10 @@ describe('OutgoingPaymentService', (): void => { Pay.PaymentError.ReceiverProtocolViolation ) await expectOutcome(payment, { - accountBalance: BigInt(123 - 10), + accountBalance: BigInt(0), amountSent: BigInt(10), - amountDelivered: BigInt(5) + amountDelivered: BigInt(5), + withdrawAmount: BigInt(123 - 10) }) }) @@ -790,10 +762,11 @@ describe('OutgoingPaymentService', (): void => { const payment = await processNext(paymentId, PaymentState.Completed) if (!payment.quote) throw 'no quote' await expectOutcome(payment, { - accountBalance: payment.quote.maxSourceAmount, + accountBalance: BigInt(0), amountSent: BigInt(0), amountDelivered: BigInt(0), - invoiceReceived: invoice.amount + invoiceReceived: invoice.amount, + withdrawAmount: payment.quote.maxSourceAmount }) }) @@ -819,128 +792,81 @@ describe('OutgoingPaymentService', (): void => { ) }) }) + }) - describe.each` - state | error - ${PaymentState.Cancelled} | ${Pay.PaymentError.ReceiverProtocolViolation} - ${PaymentState.Completed} | ${undefined} - `('$state→', ({ state, error }): void => { - let paymentId: string - let accountBalance: bigint - - beforeEach( - async (): Promise => { - // Don't send invoice.paid webhook events - const invoiceService = await deps.use('invoiceService') - jest - .spyOn(invoiceService, 'handlePayment') - .mockImplementation(() => Promise.resolve()) - - paymentId = ( - await outgoingPaymentService.create({ - accountId, - invoiceUrl, - autoApprove: true - }) - ).id - - trackAmountDelivered(paymentId) - - await processNext(paymentId, PaymentState.Funding) - const scope = mockWebhookServer(paymentId, PaymentState.Funding) - await processNext(paymentId, PaymentState.Sending) - expect(scope.isDone()).toBe(true) - - if (error) { - jest.spyOn(Pay, 'pay').mockRejectedValueOnce(error) - } - const payment = await processNext(paymentId, state, error) - if (!payment.quote) throw 'no quote' - expect(payment.webhookId).not.toBeNull() - - if (state === PaymentState.Cancelled) { - accountBalance = payment.quote?.maxSourceAmount - await expectOutcome(payment, { - accountBalance, - amountSent: BigInt(0), - amountDelivered: BigInt(0) - }) - } else { - const amountSent = invoice.amount * BigInt(2) - accountBalance = payment.quote?.maxSourceAmount - amountSent - await expectOutcome(payment, { - accountBalance, - amountSent, - amountDelivered: invoice.amount - }) - } - } - ) + describe('fund', (): void => { + let payment: OutgoingPayment + let quoteAmount: bigint - it(`${state} (liquidity withdrawal)`, async (): Promise => { - const scope = mockWebhookServer(paymentId, state) - const payment = await processNext(paymentId, state, error) - expect(scope.isDone()).toBe(true) - expect(payment.webhookId).toBeNull() - await expect(accountingService.getBalance(paymentId)).resolves.toEqual( - BigInt(0) - ) - // Payment is done being processed - await expect( - outgoingPaymentService.processNext() - ).resolves.toBeUndefined() + beforeEach(async (): Promise => { + const { id: paymentId } = await outgoingPaymentService.create({ + accountId, + paymentPointer, + amountToSend: BigInt(123), + autoApprove: false }) + payment = await processNext(paymentId, PaymentState.Funding) + assert.ok(payment.quote) + quoteAmount = payment.quote.maxSourceAmount + await expectOutcome(payment, { accountBalance: BigInt(0) }) + }, 10_000) - it(`${state} (webhook with empty balance)`, async (): Promise => { - jest - .spyOn(accountingService, 'getBalance') - .mockResolvedValueOnce(BigInt(0)) - const withdrawSpy = jest.spyOn(accountingService, 'createWithdrawal') - const scope = mockWebhookServer(paymentId, state) - const payment = await processNext(paymentId, state, error) - expect(scope.isDone()).toBe(true) - expect(withdrawSpy).not.toHaveBeenCalled() - expect(payment.webhookId).toBeNull() - - // Payment is done being processed - await expect( - outgoingPaymentService.processNext() - ).resolves.toBeUndefined() - }) + it('fails when no payment exists', async (): Promise => { + await expect( + outgoingPaymentService.fund({ + id: uuid(), + amount: quoteAmount, + transferId: uuid() + }) + ).resolves.toEqual(FundingError.UnknownPayment) + }) - it(`${state} (webhook error)`, async (): Promise => { - const scope = mockWebhookServer(paymentId, state, 504) - const payment = await processNext(paymentId, state, error) - expect(scope.isDone()).toBe(true) - expect(payment.webhookId).not.toBeNull() - await expect(accountingService.getBalance(paymentId)).resolves.toEqual( - accountBalance - ) + it('transitions a Funding payment to Sending state', async (): Promise => { + await expect( + outgoingPaymentService.fund({ + id: payment.id, + amount: quoteAmount, + transferId: uuid() + }) + ).resolves.toMatchObject({ + id: payment.id, + state: PaymentState.Sending }) - it(`${state} (webhook timeout)`, async (): Promise => { - const scope = nock(webhookUrl.origin) - .post(webhookUrl.pathname) - .delayConnection(Config.webhookTimeout + 1) - .reply(200) - const payment = await processNext(paymentId, state, error) - expect(scope.isDone()).toBe(true) - expect(payment.webhookId).not.toBeNull() - await expect(accountingService.getBalance(paymentId)).resolves.toEqual( - accountBalance - ) - }) + const after = await outgoingPaymentService.get(payment.id) + expect(after?.state).toBe(PaymentState.Sending) + await expectOutcome(payment, { accountBalance: quoteAmount }) + }) - if (state === PaymentState.Cancelled) { - it('QUOTING (withdraw + requote)', async (): Promise => { - const scope = mockWebhookServer(paymentId, state, 205) - await processNext(paymentId, PaymentState.Quoting) - expect(scope.isDone()).toBe(true) - await expect( - accountingService.getBalance(paymentId) - ).resolves.toEqual(BigInt(0)) + it('fails for invalid funding amount', async (): Promise => { + await expect( + outgoingPaymentService.fund({ + id: payment.id, + amount: quoteAmount - BigInt(1), + transferId: uuid() }) - } + ).resolves.toEqual(FundingError.InvalidAmount) + + const after = await outgoingPaymentService.get(payment.id) + expect(after?.state).toBe(PaymentState.Funding) + await expectOutcome(payment, { accountBalance: BigInt(0) }) + }) + + Object.values(PaymentState).forEach((startState) => { + if (startState === PaymentState.Funding) return + it(`does not fund a ${startState} payment`, async (): Promise => { + await payment.$query().patch({ state: startState }) + await expect( + outgoingPaymentService.fund({ + id: payment.id, + amount: quoteAmount, + transferId: uuid() + }) + ).resolves.toEqual(FundingError.WrongState) + + const after = await outgoingPaymentService.get(payment.id) + expect(after?.state).toBe(startState) + }) }) }) diff --git a/packages/backend/src/outgoing_payment/service.ts b/packages/backend/src/outgoing_payment/service.ts index 3a558d5911..9407beb939 100644 --- a/packages/backend/src/outgoing_payment/service.ts +++ b/packages/backend/src/outgoing_payment/service.ts @@ -2,6 +2,7 @@ import { ForeignKeyViolationError, TransactionOrKnex } from 'objection' import * as Pay from '@interledger/pay' import { BaseService } from '../shared/baseService' +import { FundingError, LifecycleError } from './errors' import { OutgoingPayment, PaymentIntent, PaymentState } from './model' import { AccountingService } from '../accounting/service' import { AccountService } from '../open_payments/account/service' @@ -13,6 +14,9 @@ import * as worker from './worker' export interface OutgoingPaymentService { get(id: string): Promise create(options: CreateOutgoingPaymentOptions): Promise + fund( + options: FundOutgoingPaymentOptions + ): Promise processNext(): Promise getAccountPage( accountId: string, @@ -47,6 +51,7 @@ export async function createOutgoingPaymentService( get: (id) => getOutgoingPayment(deps, id), create: (options: CreateOutgoingPaymentOptions) => createOutgoingPayment(deps, options), + fund: (options) => fundPayment(deps, options), processNext: () => worker.processPendingPayment(deps), getAccountPage: (accountId, pagination) => getAccountPage(deps, accountId, pagination) @@ -140,6 +145,41 @@ async function createOutgoingPayment( } } +export interface FundOutgoingPaymentOptions { + id: string + amount: bigint + transferId: string +} + +async function fundPayment( + deps: ServiceDependencies, + { id, amount, transferId }: FundOutgoingPaymentOptions +): Promise { + return deps.knex.transaction(async (trx) => { + const payment = await OutgoingPayment.query(trx) + .findById(id) + .forUpdate() + .withGraphFetched('account.asset') + if (!payment) return FundingError.UnknownPayment + if (payment.state !== PaymentState.Funding) { + return FundingError.WrongState + } + if (!payment.quote) throw LifecycleError.MissingQuote + if (amount !== payment.quote.maxSourceAmount) + return FundingError.InvalidAmount + const error = await deps.accountingService.createDeposit({ + id: transferId, + account: payment, + amount + }) + if (error) { + return error + } + await payment.$query(trx).patch({ state: PaymentState.Sending }) + return payment + }) +} + interface Pagination { after?: string // Forward pagination: cursor. before?: string // Backward pagination: cursor. diff --git a/packages/backend/src/outgoing_payment/worker.ts b/packages/backend/src/outgoing_payment/worker.ts index 9acb953f1a..df17ef1487 100644 --- a/packages/backend/src/outgoing_payment/worker.ts +++ b/packages/backend/src/outgoing_payment/worker.ts @@ -52,11 +52,7 @@ export async function getPendingPayment( .forUpdate() // Don't wait for a payment that is already being processed. .skipLocked() - .where((builder: knex.QueryBuilder) => { - builder - .whereIn('state', [PaymentState.Quoting, PaymentState.Sending]) - .orWhereNotNull('webhookId') - }) + .whereIn('state', [PaymentState.Quoting, PaymentState.Sending]) // Back off between retries. .andWhere((builder: knex.QueryBuilder) => { builder @@ -100,10 +96,7 @@ export async function handlePaymentLifecycle( { state: payment.state, error, stateAttempts }, 'payment lifecycle failed; cancelling' ) - await payment.$query(deps.knex).patch({ - state: PaymentState.Cancelled, - error - }) + await lifecycle.handleCancelled(deps, payment, error) } } @@ -145,9 +138,6 @@ export async function handlePaymentLifecycle( ) }) }) - case PaymentState.Cancelled: - case PaymentState.Completed: - return lifecycle.handleCancelledOrCompleted(deps, payment).catch(onError) default: deps.logger.warn('unexpected payment in lifecycle') break diff --git a/packages/backend/src/webhook/model.ts b/packages/backend/src/webhook/model.ts new file mode 100644 index 0000000000..1a5948dd8e --- /dev/null +++ b/packages/backend/src/webhook/model.ts @@ -0,0 +1,98 @@ +import { PaymentType } from '@interledger/pay' + +import { PaymentState } from '../outgoing_payment/model' +import { BaseModel } from '../shared/baseModel' + +export enum InvoiceEventType { + InvoiceExpired = 'invoice.expired', + InvoicePaid = 'invoice.paid' +} + +enum PaymentDepositType { + PaymentFunding = 'outgoing_payment.funding' +} + +enum PaymentWithdrawType { + PaymentCancelled = 'outgoing_payment.cancelled', + PaymentCompleted = 'outgoing_payment.completed' +} + +export const PaymentEventType = { + ...PaymentDepositType, + ...PaymentWithdrawType +} +export type PaymentEventType = PaymentDepositType | PaymentWithdrawType + +export const EventType = { ...InvoiceEventType, ...PaymentEventType } +export type EventType = InvoiceEventType | PaymentEventType + +export const DepositEventType = PaymentDepositType +export type DepositEventType = PaymentDepositType + +export const WithdrawEventType = { ...InvoiceEventType, ...PaymentWithdrawType } +export type WithdrawEventType = InvoiceEventType | PaymentWithdrawType + +export interface InvoiceData { + invoice: { + id: string + accountId: string + active: boolean + description?: string + createdAt: string + expiresAt: string + amount: string + received: string + } + payment?: never +} + +export interface PaymentData { + invoice?: never + payment: { + id: string + accountId: string + createdAt: string + state: PaymentState + error?: string + stateAttempts: number + intent: { + paymentPointer?: string + invoiceUrl?: string + amountToSend?: string + autoApprove: boolean + } + quote?: { + timestamp: string + activationDeadline: string + targetType: PaymentType + minDeliveryAmount: string + maxSourceAmount: string + maxPacketAmount: string + minExchangeRate: number + lowExchangeRateEstimate: number + highExchangeRateEstimate: number + amountSent: string + } + destinationAccount: { + scale: number + code: string + url?: string + } + outcome: { + amountSent: string + } + balance: string + } +} + +export class WebhookEvent extends BaseModel { + public static get tableName(): string { + return 'webhookEvents' + } + + public type!: EventType + public data!: InvoiceData | PaymentData + public attempts!: number + public error?: string | null + public processAt!: Date +} diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index ecb38e3200..7c6d3cf558 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -1,26 +1,31 @@ import assert from 'assert' +import axios from 'axios' import nock, { Definition } from 'nock' import { URL } from 'url' import Knex from 'knex' import { v4 as uuid } from 'uuid' +import * as Pay from '@interledger/pay' +import { EventType, WebhookEvent } from './model' import { - EventType, isPaymentEventType, WebhookService, generateWebhookSignature, invoiceToData, - paymentToData + paymentToData, + RETENTION_LIMIT_MS, + RETRY_BACKOFF_MS, + RETRY_LIMIT_MS } from './service' import { createTestApp, TestContainer } from '../tests/app' import { randomAsset } from '../tests/asset' -import { truncateTables } from '../tests/tableManager' +import { truncateTable, truncateTables } from '../tests/tableManager' import { Config } from '../config/app' import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../' import { AppServices } from '../app' import { Invoice } from '../open_payments/invoice/model' -import { OutgoingPayment } from '../outgoing_payment/model' +import { OutgoingPayment, PaymentState } from '../outgoing_payment/model' describe('Webhook Service', (): void => { let deps: IocContract @@ -61,6 +66,24 @@ describe('Webhook Service', (): void => { invoiceUrl, autoApprove: false }) + await payment.$query(knex).patch({ + state: PaymentState.Funding, + quote: { + timestamp: new Date(), + activationDeadline: new Date(Date.now() + 1000), + targetType: Pay.PaymentType.FixedSend, + minDeliveryAmount: BigInt(123), + maxSourceAmount: BigInt(456), + maxPacketAmount: BigInt(789), + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + minExchangeRate: Pay.Ratio.from(1.23)!, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + lowExchangeRateEstimate: Pay.Ratio.from(1.2)!, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + highExchangeRateEstimate: Pay.Ratio.from(2.3)!, + amountSent: BigInt(0) + } + }) amountReceived = BigInt(5) amountSent = BigInt(10) balance = BigInt(0) @@ -68,6 +91,13 @@ describe('Webhook Service', (): void => { } ) + afterEach( + async (): Promise => { + jest.useRealTimers() + await truncateTable(knex, WebhookEvent.tableName) + } + ) + afterAll( async (): Promise => { await appContainer.shutdown() @@ -75,67 +105,193 @@ describe('Webhook Service', (): void => { } ) - describe('Send Event', (): void => { - it.each(Object.values(EventType).map((type) => [type]))( - '%s', - async (type): Promise => { - const id = uuid() - nock(webhookUrl.origin) - .post(webhookUrl.pathname, function (this: Definition, body) { - assert.ok(this.headers) - const signature = this.headers['rafiki-signature'] - expect( - generateWebhookSignature( - body, - WEBHOOK_SECRET, - Config.signatureVersion - ) - ).toEqual(signature) - expect(body.id).toEqual(id) - expect(body.type).toEqual(type) - if (isPaymentEventType(type)) { - expect(body.data).toEqual( - paymentToData(payment, amountSent, balance) - ) - } else { - expect(body.data).toEqual(invoiceToData(invoice, amountReceived)) - } - return true - }) - .reply(200) + describe.each(Object.values(EventType).map((type) => [type]))( + '%s', + (type): void => { + describe('Create/Get Webhook Event', (): void => { + test('A webhook event can be created and fetched', async (): Promise => { + const id = uuid() + const options = isPaymentEventType(type) + ? { + id, + type, + payment, + amountSent, + balance + } + : { + id, + type, + invoice, + amountReceived + } + const now = new Date() + jest.useFakeTimers('modern') + jest.setSystemTime(now) - if (isPaymentEventType(type)) { - await webhookService.send({ - id, - type, - payment, - amountSent, - balance - }) - } else { - await webhookService.send({ + const event = await webhookService.createEvent(options) + expect(event).toMatchObject({ id, type, - invoice, - amountReceived + data: isPaymentEventType(type) + ? paymentToData(payment, amountSent, balance) + : invoiceToData(invoice, amountReceived), + error: null, + attempts: 0, + processAt: now }) + await expect(webhookService.getEvent(event.id)).resolves.toEqual( + event + ) + }) + + test('Cannot fetch a bogus webhook event', async (): Promise => { + await expect(webhookService.getEvent(uuid())).resolves.toBeUndefined() + }) + }) + + describe('processNext', (): void => { + let event: WebhookEvent + + beforeEach( + async (): Promise => { + event = await webhookService.createEvent( + isPaymentEventType(type) + ? { + id: uuid(), + type, + payment, + amountSent, + balance + } + : { + id: uuid(), + type, + invoice, + amountReceived + } + ) + } + ) + + function mockWebhookServer(status = 200): nock.Scope { + return nock(webhookUrl.origin) + .post(webhookUrl.pathname, function (this: Definition, body) { + assert.ok(this.headers) + const signature = this.headers['rafiki-signature'] + expect( + generateWebhookSignature( + body, + WEBHOOK_SECRET, + Config.signatureVersion + ) + ).toEqual(signature) + expect(body.id).toEqual(event.id) + expect(body.type).toEqual(type) + if (isPaymentEventType(type)) { + expect(body.data).toEqual( + paymentToData(payment, amountSent, balance) + ) + } else { + expect(body.data).toEqual( + invoiceToData(invoice, amountReceived) + ) + } + return true + }) + .reply(status) } - } - ) - - it('throws for failed request', async (): Promise => { - const scope = nock(webhookUrl.origin).post(webhookUrl.pathname).reply(500) - - await expect( - webhookService.send({ - id: uuid(), - type: EventType.InvoicePaid, - invoice, - amountReceived + + test('Does not process events not scheduled to be sent', async (): Promise => { + await event.$query(knex).patch({ + processAt: new Date(Date.now() + 30_000) + }) + await expect(webhookService.getEvent(event.id)).resolves.toEqual( + event + ) + await expect(webhookService.processNext()).resolves.toBeUndefined() }) - ).rejects.toThrowError('Request failed with status code 500') - expect(scope.isDone()).toBe(true) - }) - }) + test('Sends webhook event', async (): Promise => { + const scope = mockWebhookServer() + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + await expect( + webhookService.getEvent(event.id) + ).resolves.toMatchObject({ + attempts: 0, + error: null, + processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + }) + }) + + test('Schedules retry if request fails', async (): Promise => { + const status = 504 + const scope = mockWebhookServer(status) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + const updatedEvent = await webhookService.getEvent(event.id) + assert.ok(updatedEvent) + expect(updatedEvent).toMatchObject({ + attempts: 1, + error: `Request failed with status code ${status}` + }) + expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( + event.createdAt.getTime() + RETRY_BACKOFF_MS + ) + }) + + test('Schedules retry if request times out', async (): Promise => { + const scope = nock(webhookUrl.origin) + .post(webhookUrl.pathname) + .delayConnection(Config.webhookTimeout + 1) + .reply(200) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + const updatedEvent = await webhookService.getEvent(event.id) + assert.ok(updatedEvent) + expect(updatedEvent).toMatchObject({ + attempts: 1, + error: 'timeout of 2000ms exceeded' + }) + expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( + event.createdAt.getTime() + RETRY_BACKOFF_MS + ) + }) + + test('Stops retrying after limit', async (): Promise => { + jest.useFakeTimers('modern') + jest.setSystemTime( + new Date(event.createdAt.getTime() + RETRY_LIMIT_MS - 1) + ) + + const error = 'last try' + const mockPost = jest + .spyOn(axios, 'post') + .mockRejectedValueOnce(new Error(error)) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(mockPost).toHaveBeenCalledTimes(1) + await expect( + webhookService.getEvent(event.id) + ).resolves.toMatchObject({ + attempts: 1, + error, + processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + }) + }) + + test('Deletes event after retention period', async (): Promise => { + jest.useFakeTimers('modern') + jest.setSystemTime( + new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + ) + + await expect(webhookService.processNext()).resolves.toEqual(event.id) + await expect( + webhookService.getEvent(event.id) + ).resolves.toBeUndefined() + }) + }) + } + ) }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index ca15c50a08..a5935e6b30 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -1,32 +1,31 @@ +import assert from 'assert' +import axios from 'axios' import { createHmac } from 'crypto' -import axios, { AxiosResponse } from 'axios' -import { PaymentType } from '@interledger/pay' -import { Logger } from 'pino' +import { Transaction } from 'objection' +import { + WebhookEvent, + InvoiceData, + PaymentData, + InvoiceEventType, + PaymentEventType +} from './model' import { IAppConfig } from '../config/app' import { Invoice } from '../open_payments/invoice/model' -import { OutgoingPayment, PaymentState } from '../outgoing_payment/model' +import { OutgoingPayment } from '../outgoing_payment/model' +import { BaseService } from '../shared/baseService' -enum InvoiceEventType { - InvoiceExpired = 'invoice.expired', - InvoicePaid = 'invoice.paid' -} - -enum PaymentEventType { - PaymentFunding = 'outgoing_payment.funding', - PaymentCancelled = 'outgoing_payment.cancelled', - PaymentCompleted = 'outgoing_payment.completed' -} - -export const EventType = { ...InvoiceEventType, ...PaymentEventType } -export type EventType = InvoiceEventType | PaymentEventType +// First retry waits 10 seconds, second retry waits 20 (more) seconds, etc. +export const RETRY_BACKOFF_MS = 10_000 +export const RETRY_LIMIT_MS = 60_000 * 60 * 24 // 1 day +export const RETENTION_LIMIT_MS = 60_000 * 60 * 24 * 30 // 30 days // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export const isPaymentEventType = (type: any): type is PaymentEventType => Object.values(PaymentEventType).includes(type) interface InvoiceEvent { - id: string + id?: string type: InvoiceEventType invoice: Invoice payment?: never @@ -36,7 +35,7 @@ interface InvoiceEvent { } interface PaymentEvent { - id: string + id?: string type: PaymentEventType invoice?: never payment: OutgoingPayment @@ -47,77 +46,18 @@ interface PaymentEvent { export type EventOptions = InvoiceEvent | PaymentEvent -interface InvoiceData { - invoice: { - id: string - accountId: string - active: boolean - description?: string - createdAt: string - expiresAt: string - amount: string - received: string - } - payment?: never -} - -interface PaymentData { - invoice?: never - payment: { - id: string - accountId: string - createdAt: string - state: PaymentState - error?: string - stateAttempts: number - intent: { - paymentPointer?: string - invoiceUrl?: string - amountToSend?: string - autoApprove: boolean - } - - quote?: { - timestamp: string - activationDeadline: string - targetType: PaymentType - minDeliveryAmount: string - maxSourceAmount: string - maxPacketAmount: string - minExchangeRate: number - lowExchangeRateEstimate: number - highExchangeRateEstimate: number - } - destinationAccount: { - scale: number - code: string - url?: string - } - outcome: { - amountSent: string - } - balance: string - } -} - -interface WebhookEvent { - id: string - type: EventType - data: InvoiceData | PaymentData -} - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types export const isPaymentEvent = (event: any): event is PaymentEvent => Object.values(PaymentEventType).includes(event.type) export interface WebhookService { - send(options: EventOptions): Promise - readonly timeout: number + createEvent(options: EventOptions, trx?: Transaction): Promise + getEvent(id: string): Promise + processNext(): Promise } -interface ServiceDependencies { +interface ServiceDependencies extends BaseService { config: IAppConfig - logger: Logger } export async function createWebhookService( @@ -128,41 +68,128 @@ export async function createWebhookService( }) const deps = { ...deps_, logger } return { - send: (options) => sendWebhook(deps, options), - timeout: deps.config.webhookTimeout + createEvent: (options) => createWebhookEvent(deps, options), + getEvent: (id) => getWebhookEvent(deps, id), + processNext: () => processNextWebhookEvent(deps) } } -async function sendWebhook( +async function createWebhookEvent( deps: ServiceDependencies, - options: EventOptions -): Promise { - const event = { + options: EventOptions, + trx?: Transaction +): Promise { + return await WebhookEvent.query(trx || deps.knex).insertAndFetch({ id: options.id, type: options.type, - data: isPaymentEvent(options) - ? paymentToData(options.payment, options.amountSent, options.balance) - : invoiceToData(options.invoice, options.amountReceived) - } + data: options.invoice + ? invoiceToData(options.invoice, options.amountReceived) + : paymentToData(options.payment, options.amountSent, options.balance), + processAt: new Date() + }) +} - const requestHeaders = { - 'Content-Type': 'application/json' - } +async function getWebhookEvent( + deps: ServiceDependencies, + id: string +): Promise { + return WebhookEvent.query(deps.knex).findById(id) +} - if (deps.config.webhookSecret) { - requestHeaders['Rafiki-Signature'] = generateWebhookSignature( - event, - deps.config.webhookSecret, - deps.config.signatureVersion - ) - } +// Fetch (and lock) a webhook event for work. +// Returns the id of the processed event (if any). +async function processNextWebhookEvent( + deps_: ServiceDependencies +): Promise { + assert.ok(deps_.knex, 'Knex undefined') + return deps_.knex.transaction(async (trx) => { + const now = Date.now() + const events = await WebhookEvent.query(trx) + .limit(1) + // Ensure the webhook event cannot be processed concurrently by multiple workers. + .forUpdate() + // If a webhook event is locked, don't wait — just come back for it later. + .skipLocked() + .where('processAt', '<=', new Date(now).toISOString()) + + const event = events[0] + if (!event) return + + const deps = { + ...deps_, + knex: trx, + logger: deps_.logger.child({ + event: event.id + }) + } + + if (now >= event.createdAt.getTime() + RETENTION_LIMIT_MS) { + await event.$query(deps.knex).delete() + } else { + await sendWebhookEvent(deps, event) + } - return await axios.post(deps.config.webhookUrl, event, { - timeout: deps.config.webhookTimeout, - headers: requestHeaders + return event.id }) } +async function sendWebhookEvent( + deps: ServiceDependencies, + event: WebhookEvent +): Promise { + try { + const requestHeaders = { + 'Content-Type': 'application/json' + } + + if (deps.config.webhookSecret) { + requestHeaders['Rafiki-Signature'] = generateWebhookSignature( + event, + deps.config.webhookSecret, + deps.config.signatureVersion + ) + } + + const body = { + id: event.id, + type: event.type, + data: event.data + } + + await axios.post(deps.config.webhookUrl, body, { + timeout: deps.config.webhookTimeout, + headers: requestHeaders + }) + + await event.$query(deps.knex).patch({ + error: null, + processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + }) + } catch (err) { + const attempts = event.attempts + 1 + const error = err.message + deps.logger.warn( + { + attempts, + error + }, + 'webhook request failed' + ) + + const retryAt = Date.now() + Math.min(attempts, 6) * RETRY_BACKOFF_MS + const processAt = + retryAt < event.createdAt.getTime() + RETRY_LIMIT_MS + ? new Date(retryAt) + : new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + + await event.$query(deps.knex).patch({ + attempts, + error, + processAt + }) + } +} + export function generateWebhookSignature( event: WebhookEvent, secret: string, @@ -201,27 +228,14 @@ export function paymentToData( amountSent: bigint, balance: bigint ): PaymentData { - return { + const json: PaymentData = { payment: { id: payment.id, accountId: payment.accountId, state: payment.state, - error: payment.error || undefined, stateAttempts: payment.stateAttempts, intent: { - ...payment.intent, - amountToSend: payment.intent.amountToSend?.toString() - }, - quote: payment.quote && { - ...payment.quote, - timestamp: payment.quote.timestamp.toISOString(), - activationDeadline: payment.quote.activationDeadline.toISOString(), - minDeliveryAmount: payment.quote.minDeliveryAmount.toString(), - maxSourceAmount: payment.quote.maxSourceAmount.toString(), - maxPacketAmount: payment.quote.maxPacketAmount.toString(), - minExchangeRate: payment.quote.minExchangeRate.valueOf(), - lowExchangeRateEstimate: payment.quote.lowExchangeRateEstimate.valueOf(), - highExchangeRateEstimate: payment.quote.highExchangeRateEstimate.valueOf() + autoApprove: payment.intent.autoApprove }, destinationAccount: payment.destinationAccount, createdAt: new Date(+payment.createdAt).toISOString(), @@ -231,4 +245,31 @@ export function paymentToData( balance: balance.toString() } } + if (payment.intent.paymentPointer) { + json.payment.intent.paymentPointer = payment.intent.paymentPointer + } + if (payment.intent.invoiceUrl) { + json.payment.intent.invoiceUrl = payment.intent.invoiceUrl + } + if (payment.intent.amountToSend) { + json.payment.intent.amountToSend = payment.intent.amountToSend.toString() + } + if (payment.error) { + json.payment.error = payment.error + } + if (payment.quote) { + json.payment.quote = { + ...payment.quote, + timestamp: payment.quote.timestamp.toISOString(), + activationDeadline: payment.quote.activationDeadline.toISOString(), + minDeliveryAmount: payment.quote.minDeliveryAmount.toString(), + maxSourceAmount: payment.quote.maxSourceAmount.toString(), + maxPacketAmount: payment.quote.maxPacketAmount.toString(), + minExchangeRate: payment.quote.minExchangeRate.valueOf(), + lowExchangeRateEstimate: payment.quote.lowExchangeRateEstimate.valueOf(), + highExchangeRateEstimate: payment.quote.highExchangeRateEstimate.valueOf(), + amountSent: payment.quote.amountSent.toString() + } + } + return json } From aa685732c80de4b5601ff23e441656e40a9ed7d5 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Fri, 4 Feb 2022 21:14:01 -0600 Subject: [PATCH 02/15] chore(backend): add invoice and payment webhook event models --- .../src/graphql/resolvers/liquidity.test.ts | 46 ++++--- .../src/graphql/resolvers/liquidity.ts | 16 ++- .../src/open_payments/invoice/model.ts | 40 ++++++ .../src/open_payments/invoice/service.test.ts | 15 +-- .../src/open_payments/invoice/service.ts | 29 ++-- .../backend/src/outgoing_payment/lifecycle.ts | 29 ++-- .../backend/src/outgoing_payment/model.ts | 121 +++++++++++++++++ .../src/outgoing_payment/service.test.ts | 19 ++- packages/backend/src/webhook/model.ts | 89 +----------- packages/backend/src/webhook/service.test.ts | 87 ++++-------- packages/backend/src/webhook/service.ts | 127 +----------------- 11 files changed, 272 insertions(+), 346 deletions(-) diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index b424a2b0c9..0b20524706 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -3,6 +3,7 @@ import Knex from 'knex' import { v4 as uuid } from 'uuid' import * as Pay from '@interledger/pay' +import { DepositEventType, WithdrawEventType } from './liquidity' import { createTestApp, TestContainer } from '../../tests/app' import { IocContract } from '@adonisjs/fold' import { AppServices } from '../../app' @@ -13,13 +14,17 @@ import { Asset } from '../../asset/model' import { AssetService } from '../../asset/service' import { Account } from '../../open_payments/account/model' import { Invoice } from '../../open_payments/invoice/model' -import { OutgoingPayment, PaymentState } from '../../outgoing_payment/model' +import { + OutgoingPayment, + PaymentState, + PaymentEvent, + isPaymentEventType +} from '../../outgoing_payment/model' import { Peer } from '../../peer/model' import { randomAsset } from '../../tests/asset' import { PeerFactory } from '../../tests/peerFactory' import { truncateTables } from '../../tests/tableManager' -import { DepositEventType, WithdrawEventType } from '../../webhook/model' -import { EventOptions, isPaymentEventType } from '../../webhook/service' +import { WebhookEvent } from '../../webhook/model' import { LiquidityError, LiquidityMutationResponse, @@ -1623,13 +1628,14 @@ describe('Liquidity Resolvers', (): void => { beforeEach( async (): Promise => { eventId = uuid() - const webhookService = await deps.use('webhookService') - await webhookService.createEvent({ + await PaymentEvent.query(knex).insertAndFetch({ id: eventId, type, - payment, - amountSent: BigInt(0), - balance: BigInt(0) + data: payment.toData({ + amountSent: BigInt(0), + balance: BigInt(0) + }), + processAt: new Date() }) } ) @@ -1750,29 +1756,28 @@ describe('Liquidity Resolvers', (): void => { beforeEach( async (): Promise => { - const webhookService = await deps.use('webhookService') - eventId = uuid() const amount = BigInt(10) let account: LiquidityAccount - let options: EventOptions if (isPaymentEventType(type)) { account = payment - options = { + await WebhookEvent.query(knex).insertAndFetch({ id: eventId, type, - payment, - amountSent: BigInt(0), - balance: amount - } + data: payment.toData({ + amountSent: BigInt(0), + balance: amount + }), + processAt: new Date() + }) } else { account = invoice - options = { + await WebhookEvent.query(knex).insertAndFetch({ id: eventId, type, - invoice, - amountReceived: amount - } + data: invoice.toData(amount), + processAt: new Date() + }) } await expect( accountingService.createDeposit({ @@ -1792,7 +1797,6 @@ describe('Liquidity Resolvers', (): void => { await expect( accountingService.getBalance(account.id) ).resolves.toEqual(BigInt(0)) - await webhookService.createEvent(options) } ) diff --git a/packages/backend/src/graphql/resolvers/liquidity.ts b/packages/backend/src/graphql/resolvers/liquidity.ts index 7a0132a7b7..39e3deedf7 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.ts @@ -7,8 +7,13 @@ import { AccountWithdrawalMutationResponse } from '../generated/graphql' import { ApolloContext } from '../../app' +import { InvoiceEventType } from '../../open_payments/invoice/model' import { FundingError, isFundingError } from '../../outgoing_payment/errors' -import { DepositEventType, WithdrawEventType } from '../../webhook/model' +import { + isPaymentEvent, + PaymentDepositType, + PaymentWithdrawType +} from '../../outgoing_payment/model' export const addPeerLiquidity: MutationResolvers['addPeerLiquidity'] = async ( parent, @@ -280,6 +285,9 @@ export const rollbackLiquidityWithdrawal: MutationResolvers['roll } } +export const DepositEventType = PaymentDepositType +export type DepositEventType = PaymentDepositType + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types const isDepositEventType = (o: any): o is DepositEventType => Object.values(DepositEventType).includes(o) @@ -292,7 +300,7 @@ export const depositEventLiquidity: MutationResolvers['depositEve try { const webhookService = await ctx.container.use('webhookService') const event = await webhookService.getEvent(args.eventId) - if (!event || !isDepositEventType(event.type)) { + if (!event || !isPaymentEvent(event) || !isDepositEventType(event.type)) { return responses[LiquidityError.InvalidId] } assert.ok(event.data.payment?.quote?.maxSourceAmount) @@ -328,6 +336,9 @@ export const depositEventLiquidity: MutationResolvers['depositEve } } +export const WithdrawEventType = { ...InvoiceEventType, ...PaymentWithdrawType } +export type WithdrawEventType = InvoiceEventType | PaymentWithdrawType + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types const isWithdrawEventType = (o: any): o is WithdrawEventType => Object.values(WithdrawEventType).includes(o) @@ -339,6 +350,7 @@ export const withdrawEventLiquidity: MutationResolvers['withdrawE ): ResolversTypes['LiquidityMutationResponse'] => { try { const webhookService = await ctx.container.use('webhookService') + // TODO: remove event lookup when commitWithdrawal can verify transfer code const event = await webhookService.getEvent(args.eventId) if (!event || !isWithdrawEventType(event.type)) { return responses[LiquidityError.InvalidId] diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index 6e0e065a89..bdf3bb2eba 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -4,6 +4,7 @@ import { Asset } from '../../asset/model' import { LiquidityAccount } from '../../accounting/service' import { ConnectorAccount } from '../../connector/core/rafiki' import { BaseModel } from '../../shared/baseModel' +import { WebhookEvent } from '../../webhook/model' export class Invoice extends BaseModel @@ -36,4 +37,43 @@ export class Invoice public get asset(): Asset { return this.account.asset } + + public toData(amountReceived: bigint): InvoiceData { + return { + invoice: { + id: this.id, + accountId: this.accountId, + active: this.active, + amount: this.amount.toString(), + description: this.description, + expiresAt: this.expiresAt.toISOString(), + createdAt: new Date(+this.createdAt).toISOString(), + received: amountReceived.toString() + } + } + } +} + +export enum InvoiceEventType { + InvoiceExpired = 'invoice.expired', + InvoicePaid = 'invoice.paid' +} + +export type InvoiceData = { + invoice: { + id: string + accountId: string + active: boolean + description?: string + createdAt: string + expiresAt: string + amount: string + received: string + } + payment?: never +} + +export class InvoiceEvent extends WebhookEvent { + public type!: InvoiceEventType + public data!: InvoiceData } diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index e1d528041e..7d3ce64bc0 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -6,7 +6,7 @@ import { v4 as uuid } from 'uuid' import { InvoiceService } from './service' import { AccountingService } from '../../accounting/service' import { createTestApp, TestContainer } from '../../tests/app' -import { Invoice } from './model' +import { Invoice, InvoiceEvent, InvoiceEventType } from './model' import { resetGraphileDb } from '../../tests/graphileDb' import { GraphileProducer } from '../../messaging/graphileProducer' import { Config } from '../../config/app' @@ -15,7 +15,6 @@ import { initIocContainer } from '../../' import { AppServices } from '../../app' import { randomAsset } from '../../tests/asset' import { truncateTables } from '../../tests/tableManager' -import { WebhookEvent, EventType } from '../../webhook/model' describe('Invoice Service', (): void => { let deps: IocContract @@ -217,9 +216,9 @@ describe('Invoice Service', (): void => { }) describe.each` - type | expiresAt | amountReceived - ${EventType.InvoiceExpired} | ${-40_000} | ${BigInt(1)} - ${EventType.InvoicePaid} | ${30_000} | ${BigInt(123)} + type | expiresAt | amountReceived + ${InvoiceEventType.InvoiceExpired} | ${-40_000} | ${BigInt(1)} + ${InvoiceEventType.InvoicePaid} | ${30_000} | ${BigInt(123)} `( 'handleDeactivated ($type)', ({ type, expiresAt, amountReceived }): void => { @@ -240,7 +239,7 @@ describe('Invoice Service', (): void => { amount: amountReceived }) ).resolves.toBeUndefined() - if (type === EventType.InvoiceExpired) { + if (type === InvoiceEventType.InvoiceExpired) { await expect(invoiceService.processNext()).resolves.toBe( invoice.id ) @@ -261,11 +260,11 @@ describe('Invoice Service', (): void => { test('Creates liquidity withdrawal and webhook event', async (): Promise => { await expect( - WebhookEvent.query(knex).where({ type }) + InvoiceEvent.query(knex).where({ type }) ).resolves.toHaveLength(0) await expect(invoiceService.processNext()).resolves.toBe(invoice.id) await expect( - WebhookEvent.query(knex) + InvoiceEvent.query(knex) .whereJsonSupersetOf('data:invoice', { id: invoice.id }) diff --git a/packages/backend/src/open_payments/invoice/service.ts b/packages/backend/src/open_payments/invoice/service.ts index 820560eb76..b3ef7a4e51 100644 --- a/packages/backend/src/open_payments/invoice/service.ts +++ b/packages/backend/src/open_payments/invoice/service.ts @@ -1,13 +1,11 @@ -import { Invoice } from './model' +import { Invoice, InvoiceEvent, InvoiceEventType } from './model' import { AccountingService } from '../../accounting/service' import { BaseService } from '../../shared/baseService' import { Pagination } from '../../shared/pagination' -import { EventType } from '../../webhook/model' import { WebhookService, RETRY_LIMIT_MS } from '../../webhook/service' import assert from 'assert' import { Transaction } from 'knex' import { ForeignKeyViolationError, TransactionOrKnex } from 'objection' -import { v4 as uuid } from 'uuid' export const POSITIVE_SLIPPAGE = BigInt(1) // First retry waits 10 seconds @@ -207,21 +205,18 @@ async function handleDeactivated( processAt: null }) - const id = uuid() - await deps.webhookService.createEvent( - { - id, - invoice, - type: - amountReceived < invoice.amount - ? EventType.InvoiceExpired - : EventType.InvoicePaid, - amountReceived - }, - deps.knex as Transaction - ) + const event = await InvoiceEvent.query(deps.knex).insertAndFetch({ + type: + amountReceived < invoice.amount + ? InvoiceEventType.InvoiceExpired + : InvoiceEventType.InvoicePaid, + data: invoice.toData(amountReceived), + // TODO: + // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. + processAt: new Date() + }) const error = await deps.accountingService.createWithdrawal({ - id, + id: event.id, account: invoice, amount: amountReceived, timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns diff --git a/packages/backend/src/outgoing_payment/lifecycle.ts b/packages/backend/src/outgoing_payment/lifecycle.ts index e431314f35..fffba8916a 100644 --- a/packages/backend/src/outgoing_payment/lifecycle.ts +++ b/packages/backend/src/outgoing_payment/lifecycle.ts @@ -1,13 +1,15 @@ import * as Pay from '@interledger/pay' import assert from 'assert' -import { Transaction } from 'knex' -import { v4 as uuid } from 'uuid' import { LifecycleError } from './errors' -import { OutgoingPayment, PaymentState } from './model' +import { + OutgoingPayment, + PaymentState, + PaymentEvent, + PaymentEventType +} from './model' import { ServiceDependencies } from './service' import { IlpPlugin } from './ilp_plugin' -import { PaymentEventType } from '../webhook/model' import { RETRY_LIMIT_MS } from '../webhook/service' const MAX_INT64 = BigInt('9223372036854775807') @@ -342,23 +344,16 @@ const sendWebhookEvent = async ( throw LifecycleError.MissingBalance } - const id = uuid() - - await deps.webhookService.createEvent( - { - id, - payment, - type, - amountSent, - balance - }, - deps.knex as Transaction - ) + const event = await PaymentEvent.query(deps.knex).insertAndFetch({ + type, + data: payment.toData({ amountSent, balance }), + processAt: new Date() + }) if (balance) { assert.ok(type !== PaymentEventType.PaymentFunding) const error = await deps.accountingService.createWithdrawal({ - id, + id: event.id, account: payment, amount: balance, timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns diff --git a/packages/backend/src/outgoing_payment/model.ts b/packages/backend/src/outgoing_payment/model.ts index 8106799d6f..9e16fe7092 100644 --- a/packages/backend/src/outgoing_payment/model.ts +++ b/packages/backend/src/outgoing_payment/model.ts @@ -6,6 +6,7 @@ import { Asset } from '../asset/model' import { ConnectorAccount } from '../connector/core/rafiki' import { Account } from '../open_payments/account/model' import { BaseModel } from '../shared/baseModel' +import { WebhookEvent } from '../webhook/model' const fieldPrefixes = ['intent', 'quote', 'destinationAccount', 'outcome'] @@ -132,6 +133,59 @@ export class OutgoingPayment } return json } + + public toData({ + amountSent, + balance + }: { + amountSent: bigint + balance: bigint + }): PaymentData { + const data: PaymentData = { + payment: { + id: this.id, + accountId: this.accountId, + state: this.state, + stateAttempts: this.stateAttempts, + intent: { + autoApprove: this.intent.autoApprove + }, + destinationAccount: this.destinationAccount, + createdAt: new Date(+this.createdAt).toISOString(), + outcome: { + amountSent: amountSent.toString() + }, + balance: balance.toString() + } + } + if (this.intent.paymentPointer) { + data.payment.intent.paymentPointer = this.intent.paymentPointer + } + if (this.intent.invoiceUrl) { + data.payment.intent.invoiceUrl = this.intent.invoiceUrl + } + if (this.intent.amountToSend) { + data.payment.intent.amountToSend = this.intent.amountToSend.toString() + } + if (this.error) { + data.payment.error = this.error + } + if (this.quote) { + data.payment.quote = { + ...this.quote, + timestamp: this.quote.timestamp.toISOString(), + activationDeadline: this.quote.activationDeadline.toISOString(), + minDeliveryAmount: this.quote.minDeliveryAmount.toString(), + maxSourceAmount: this.quote.maxSourceAmount.toString(), + maxPacketAmount: this.quote.maxPacketAmount.toString(), + minExchangeRate: this.quote.minExchangeRate.valueOf(), + lowExchangeRateEstimate: this.quote.lowExchangeRateEstimate.valueOf(), + highExchangeRateEstimate: this.quote.highExchangeRateEstimate.valueOf(), + amountSent: this.quote.amountSent.toString() + } + } + return data + } } export enum PaymentState { @@ -152,3 +206,70 @@ export enum PaymentState { // Successful completion. Completed = 'COMPLETED' } + +export enum PaymentDepositType { + PaymentFunding = 'outgoing_payment.funding' +} + +export enum PaymentWithdrawType { + PaymentCancelled = 'outgoing_payment.cancelled', + PaymentCompleted = 'outgoing_payment.completed' +} + +export const PaymentEventType = { + ...PaymentDepositType, + ...PaymentWithdrawType +} +export type PaymentEventType = PaymentDepositType | PaymentWithdrawType + +export type PaymentData = { + invoice?: never + payment: { + id: string + accountId: string + createdAt: string + state: PaymentState + error?: string + stateAttempts: number + intent: { + paymentPointer?: string + invoiceUrl?: string + amountToSend?: string + autoApprove: boolean + } + quote?: { + timestamp: string + activationDeadline: string + targetType: Pay.PaymentType + minDeliveryAmount: string + maxSourceAmount: string + maxPacketAmount: string + minExchangeRate: number + lowExchangeRateEstimate: number + highExchangeRateEstimate: number + amountSent: string + } + destinationAccount: { + scale: number + code: string + url?: string + } + outcome: { + amountSent: string + } + balance: string + } +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +export const isPaymentEventType = (o: any): o is PaymentEventType => + Object.values(PaymentEventType).includes(o) + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +export const isPaymentEvent = (o: any): o is PaymentEvent => + o instanceof WebhookEvent && isPaymentEventType(o.type) + +export class PaymentEvent extends WebhookEvent { + public type!: PaymentEventType + public data!: PaymentData +} diff --git a/packages/backend/src/outgoing_payment/service.test.ts b/packages/backend/src/outgoing_payment/service.test.ts index 32326162e8..0364843599 100644 --- a/packages/backend/src/outgoing_payment/service.test.ts +++ b/packages/backend/src/outgoing_payment/service.test.ts @@ -12,7 +12,13 @@ import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../' import { AppServices } from '../app' import { truncateTable, truncateTables } from '../tests/tableManager' -import { OutgoingPayment, PaymentIntent, PaymentState } from './model' +import { + OutgoingPayment, + PaymentIntent, + PaymentState, + PaymentEvent, + PaymentEventType +} from './model' import { LifecycleError } from './errors' import { RETRY_BACKOFF_SECONDS } from './worker' import { isTransferError } from '../accounting/errors' @@ -21,7 +27,6 @@ import { uuidToBigInt } from '../accounting/utils' import { AssetOptions } from '../asset/service' import { Invoice } from '../open_payments/invoice/model' import { RatesService } from '../rates/service' -import { WebhookEvent, EventType } from '../webhook/model' describe('OutgoingPaymentService', (): void => { let deps: IocContract @@ -40,13 +45,13 @@ describe('OutgoingPaymentService', (): void => { let config: IAppConfig const webhookTypes: { - [key in PaymentState]: EventType | undefined + [key in PaymentState]: PaymentEventType | undefined } = { [PaymentState.Quoting]: undefined, - [PaymentState.Funding]: EventType.PaymentFunding, + [PaymentState.Funding]: PaymentEventType.PaymentFunding, [PaymentState.Sending]: undefined, - [PaymentState.Cancelled]: EventType.PaymentCancelled, - [PaymentState.Completed]: EventType.PaymentCompleted + [PaymentState.Cancelled]: PaymentEventType.PaymentCancelled, + [PaymentState.Completed]: PaymentEventType.PaymentCompleted } async function processNext( @@ -62,7 +67,7 @@ describe('OutgoingPaymentService', (): void => { const type = webhookTypes[payment.state] if (type) { await expect( - WebhookEvent.query(knex) + PaymentEvent.query(knex) .whereJsonSupersetOf('data:payment', { id: payment.id }) diff --git a/packages/backend/src/webhook/model.ts b/packages/backend/src/webhook/model.ts index 1a5948dd8e..33f1032cac 100644 --- a/packages/backend/src/webhook/model.ts +++ b/packages/backend/src/webhook/model.ts @@ -1,97 +1,12 @@ -import { PaymentType } from '@interledger/pay' - -import { PaymentState } from '../outgoing_payment/model' import { BaseModel } from '../shared/baseModel' -export enum InvoiceEventType { - InvoiceExpired = 'invoice.expired', - InvoicePaid = 'invoice.paid' -} - -enum PaymentDepositType { - PaymentFunding = 'outgoing_payment.funding' -} - -enum PaymentWithdrawType { - PaymentCancelled = 'outgoing_payment.cancelled', - PaymentCompleted = 'outgoing_payment.completed' -} - -export const PaymentEventType = { - ...PaymentDepositType, - ...PaymentWithdrawType -} -export type PaymentEventType = PaymentDepositType | PaymentWithdrawType - -export const EventType = { ...InvoiceEventType, ...PaymentEventType } -export type EventType = InvoiceEventType | PaymentEventType - -export const DepositEventType = PaymentDepositType -export type DepositEventType = PaymentDepositType - -export const WithdrawEventType = { ...InvoiceEventType, ...PaymentWithdrawType } -export type WithdrawEventType = InvoiceEventType | PaymentWithdrawType - -export interface InvoiceData { - invoice: { - id: string - accountId: string - active: boolean - description?: string - createdAt: string - expiresAt: string - amount: string - received: string - } - payment?: never -} - -export interface PaymentData { - invoice?: never - payment: { - id: string - accountId: string - createdAt: string - state: PaymentState - error?: string - stateAttempts: number - intent: { - paymentPointer?: string - invoiceUrl?: string - amountToSend?: string - autoApprove: boolean - } - quote?: { - timestamp: string - activationDeadline: string - targetType: PaymentType - minDeliveryAmount: string - maxSourceAmount: string - maxPacketAmount: string - minExchangeRate: number - lowExchangeRateEstimate: number - highExchangeRateEstimate: number - amountSent: string - } - destinationAccount: { - scale: number - code: string - url?: string - } - outcome: { - amountSent: string - } - balance: string - } -} - export class WebhookEvent extends BaseModel { public static get tableName(): string { return 'webhookEvents' } - public type!: EventType - public data!: InvoiceData | PaymentData + public type!: string + public data!: Record public attempts!: number public error?: string | null public processAt!: Date diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index 7c6d3cf558..360b99f117 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -6,13 +6,10 @@ import Knex from 'knex' import { v4 as uuid } from 'uuid' import * as Pay from '@interledger/pay' -import { EventType, WebhookEvent } from './model' +import { WebhookEvent } from './model' import { - isPaymentEventType, WebhookService, generateWebhookSignature, - invoiceToData, - paymentToData, RETENTION_LIMIT_MS, RETRY_BACKOFF_MS, RETRY_LIMIT_MS @@ -24,8 +21,16 @@ import { Config } from '../config/app' import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../' import { AppServices } from '../app' -import { Invoice } from '../open_payments/invoice/model' -import { OutgoingPayment, PaymentState } from '../outgoing_payment/model' +import { Invoice, InvoiceEventType } from '../open_payments/invoice/model' +import { + OutgoingPayment, + PaymentState, + PaymentEventType, + isPaymentEventType +} from '../outgoing_payment/model' + +export const EventType = { ...InvoiceEventType, ...PaymentEventType } +export type EventType = InvoiceEventType | PaymentEventType describe('Webhook Service', (): void => { let deps: IocContract @@ -108,38 +113,23 @@ describe('Webhook Service', (): void => { describe.each(Object.values(EventType).map((type) => [type]))( '%s', (type): void => { - describe('Create/Get Webhook Event', (): void => { - test('A webhook event can be created and fetched', async (): Promise => { - const id = uuid() - const options = isPaymentEventType(type) - ? { - id, - type, - payment, - amountSent, - balance - } - : { - id, - type, - invoice, - amountReceived - } - const now = new Date() - jest.useFakeTimers('modern') - jest.setSystemTime(now) + let event: WebhookEvent - const event = await webhookService.createEvent(options) - expect(event).toMatchObject({ - id, + beforeEach( + async (): Promise => { + event = await WebhookEvent.query(knex).insertAndFetch({ + id: uuid(), type, data: isPaymentEventType(type) - ? paymentToData(payment, amountSent, balance) - : invoiceToData(invoice, amountReceived), - error: null, - attempts: 0, - processAt: now + ? payment.toData({ amountSent, balance }) + : invoice.toData(amountReceived), + processAt: new Date() }) + } + ) + + describe('Get Webhook Event', (): void => { + test('A webhook event can be fetched', async (): Promise => { await expect(webhookService.getEvent(event.id)).resolves.toEqual( event ) @@ -151,29 +141,6 @@ describe('Webhook Service', (): void => { }) describe('processNext', (): void => { - let event: WebhookEvent - - beforeEach( - async (): Promise => { - event = await webhookService.createEvent( - isPaymentEventType(type) - ? { - id: uuid(), - type, - payment, - amountSent, - balance - } - : { - id: uuid(), - type, - invoice, - amountReceived - } - ) - } - ) - function mockWebhookServer(status = 200): nock.Scope { return nock(webhookUrl.origin) .post(webhookUrl.pathname, function (this: Definition, body) { @@ -190,12 +157,10 @@ describe('Webhook Service', (): void => { expect(body.type).toEqual(type) if (isPaymentEventType(type)) { expect(body.data).toEqual( - paymentToData(payment, amountSent, balance) + payment.toData({ amountSent, balance }) ) } else { - expect(body.data).toEqual( - invoiceToData(invoice, amountReceived) - ) + expect(body.data).toEqual(invoice.toData(amountReceived)) } return true }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index a5935e6b30..25f6b8d0a7 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -1,18 +1,9 @@ import assert from 'assert' import axios from 'axios' import { createHmac } from 'crypto' -import { Transaction } from 'objection' -import { - WebhookEvent, - InvoiceData, - PaymentData, - InvoiceEventType, - PaymentEventType -} from './model' +import { WebhookEvent } from './model' import { IAppConfig } from '../config/app' -import { Invoice } from '../open_payments/invoice/model' -import { OutgoingPayment } from '../outgoing_payment/model' import { BaseService } from '../shared/baseService' // First retry waits 10 seconds, second retry waits 20 (more) seconds, etc. @@ -20,38 +11,7 @@ export const RETRY_BACKOFF_MS = 10_000 export const RETRY_LIMIT_MS = 60_000 * 60 * 24 // 1 day export const RETENTION_LIMIT_MS = 60_000 * 60 * 24 * 30 // 30 days -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -export const isPaymentEventType = (type: any): type is PaymentEventType => - Object.values(PaymentEventType).includes(type) - -interface InvoiceEvent { - id?: string - type: InvoiceEventType - invoice: Invoice - payment?: never - amountReceived: bigint - amountSent?: never - balance?: never -} - -interface PaymentEvent { - id?: string - type: PaymentEventType - invoice?: never - payment: OutgoingPayment - amountReceived?: never - amountSent: bigint - balance: bigint -} - -export type EventOptions = InvoiceEvent | PaymentEvent - -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -export const isPaymentEvent = (event: any): event is PaymentEvent => - Object.values(PaymentEventType).includes(event.type) - export interface WebhookService { - createEvent(options: EventOptions, trx?: Transaction): Promise getEvent(id: string): Promise processNext(): Promise } @@ -68,27 +28,11 @@ export async function createWebhookService( }) const deps = { ...deps_, logger } return { - createEvent: (options) => createWebhookEvent(deps, options), getEvent: (id) => getWebhookEvent(deps, id), processNext: () => processNextWebhookEvent(deps) } } -async function createWebhookEvent( - deps: ServiceDependencies, - options: EventOptions, - trx?: Transaction -): Promise { - return await WebhookEvent.query(trx || deps.knex).insertAndFetch({ - id: options.id, - type: options.type, - data: options.invoice - ? invoiceToData(options.invoice, options.amountReceived) - : paymentToData(options.payment, options.amountSent, options.balance), - processAt: new Date() - }) -} - async function getWebhookEvent( deps: ServiceDependencies, id: string @@ -204,72 +148,3 @@ export function generateWebhookSignature( return `t=${timestamp}, v${version}=${digest}` } - -export function invoiceToData( - invoice: Invoice, - amountReceived: bigint -): InvoiceData { - return { - invoice: { - id: invoice.id, - accountId: invoice.accountId, - active: invoice.active, - amount: invoice.amount.toString(), - description: invoice.description, - expiresAt: invoice.expiresAt.toISOString(), - createdAt: new Date(+invoice.createdAt).toISOString(), - received: amountReceived.toString() - } - } -} - -export function paymentToData( - payment: OutgoingPayment, - amountSent: bigint, - balance: bigint -): PaymentData { - const json: PaymentData = { - payment: { - id: payment.id, - accountId: payment.accountId, - state: payment.state, - stateAttempts: payment.stateAttempts, - intent: { - autoApprove: payment.intent.autoApprove - }, - destinationAccount: payment.destinationAccount, - createdAt: new Date(+payment.createdAt).toISOString(), - outcome: { - amountSent: amountSent.toString() - }, - balance: balance.toString() - } - } - if (payment.intent.paymentPointer) { - json.payment.intent.paymentPointer = payment.intent.paymentPointer - } - if (payment.intent.invoiceUrl) { - json.payment.intent.invoiceUrl = payment.intent.invoiceUrl - } - if (payment.intent.amountToSend) { - json.payment.intent.amountToSend = payment.intent.amountToSend.toString() - } - if (payment.error) { - json.payment.error = payment.error - } - if (payment.quote) { - json.payment.quote = { - ...payment.quote, - timestamp: payment.quote.timestamp.toISOString(), - activationDeadline: payment.quote.activationDeadline.toISOString(), - minDeliveryAmount: payment.quote.minDeliveryAmount.toString(), - maxSourceAmount: payment.quote.maxSourceAmount.toString(), - maxPacketAmount: payment.quote.maxPacketAmount.toString(), - minExchangeRate: payment.quote.minExchangeRate.valueOf(), - lowExchangeRateEstimate: payment.quote.lowExchangeRateEstimate.valueOf(), - highExchangeRateEstimate: payment.quote.highExchangeRateEstimate.valueOf(), - amountSent: payment.quote.amountSent.toString() - } - } - return json -} From e964ca812e437a252e0f493669798e9779ca784e Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Mon, 7 Feb 2022 01:10:11 -0600 Subject: [PATCH 03/15] chore(backend): add onCredit/onDebit to LiquidityAccount --- .../20211022210203_create_invoices_table.js | 15 +- .../backend/src/accounting/service.test.ts | 7 +- packages/backend/src/accounting/service.ts | 116 ++++++++---- .../src/connector/core/factories/account.ts | 6 +- .../src/connector/core/middleware/balance.ts | 2 - packages/backend/src/connector/core/rafiki.ts | 6 +- .../middleware/balance-middleware.test.ts | 6 +- .../src/open_payments/invoice/model.ts | 30 +++- .../src/open_payments/invoice/service.test.ts | 167 ++++++------------ .../src/open_payments/invoice/service.ts | 100 +++-------- .../src/outgoing_payment/service.test.ts | 3 +- packages/backend/src/tests/accountFactory.ts | 19 +- 12 files changed, 222 insertions(+), 255 deletions(-) diff --git a/packages/backend/migrations/20211022210203_create_invoices_table.js b/packages/backend/migrations/20211022210203_create_invoices_table.js index 013683cffc..b8d4c932d8 100644 --- a/packages/backend/migrations/20211022210203_create_invoices_table.js +++ b/packages/backend/migrations/20211022210203_create_invoices_table.js @@ -10,14 +10,23 @@ exports.up = function (knex) { table.timestamp('expiresAt').notNullable() table.bigInteger('amount').notNullable() - table.timestamp('processAt').nullable() - table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) table.index(['accountId', 'createdAt', 'id']) - table.index('processAt') + table.index('expiresAt') + /* + TODO: The latest version of knex supports "partial indexes", which would be more efficient for the deactivateInvoice use case. Unfortunately, the only version of 'objection' that supports this version of knex is still in alpha. + // This is a 'partial index' -- expiresAt is only indexed when active=true. + table.index('expiresAt', 'idx_active_expiresAt', { + // Knex partial indices are a very new feature in Knex and appear to be buggy. + // + // Use a 'raw' condition because "knex.where('active',true)" fails with: + // > migration failed with error: create index "idx_active_expiresAt" on "invoices" ("expiresAt") where "active" = $1 - there is no parameter $1 + predicate: knex.whereRaw('active = TRUE') + }) + */ }) } diff --git a/packages/backend/src/accounting/service.test.ts b/packages/backend/src/accounting/service.test.ts index b4edeaf3bf..9142374f2e 100644 --- a/packages/backend/src/accounting/service.test.ts +++ b/packages/backend/src/accounting/service.test.ts @@ -8,6 +8,7 @@ import { v4 as uuid } from 'uuid' import { AccountingService, LiquidityAccount, + TransferAccount, Deposit, Withdrawal } from './service' @@ -24,7 +25,7 @@ import { startTigerbeetleContainer, TIGERBEETLE_PORT } from '../tests/tigerbeetle' -import { AccountFactory, FactoryAccount } from '../tests/accountFactory' +import { AccountFactory } from '../tests/accountFactory' describe('Accounting Service', (): void => { let deps: IocContract @@ -214,8 +215,8 @@ describe('Accounting Service', (): void => { ${true} | ${'same asset'} ${false} | ${'cross-currency'} `('$description', ({ sameAsset }): void => { - let sourceAccount: LiquidityAccount - let destinationAccount: FactoryAccount + let sourceAccount: TransferAccount + let destinationAccount: TransferAccount const startingSourceBalance = BigInt(10) const startingDestinationLiquidity = BigInt(100) diff --git a/packages/backend/src/accounting/service.ts b/packages/backend/src/accounting/service.ts index b7439dfc86..d9fdc387e6 100644 --- a/packages/backend/src/accounting/service.ts +++ b/packages/backend/src/accounting/service.ts @@ -1,3 +1,4 @@ +import assert from 'assert' import { Client, CreateAccountError as CreateAccountErrorCode @@ -43,6 +44,17 @@ export interface LiquidityAccount { id: string unit: number } + onCredit?: (options: onCreditOptions) => Promise + onDebit?: (options: onDebitOptions) => Promise +} + +export interface onCreditOptions { + balance: bigint + createWithdrawal: AccountingService['createWithdrawal'] +} + +interface onDebitOptions { + balance: bigint } export interface Deposit { @@ -55,9 +67,15 @@ export interface Withdrawal extends Deposit { timeout: bigint } +export interface TransferAccount extends LiquidityAccount { + asset: LiquidityAccount['asset'] & { + asset: LiquidityAccount['asset'] + } +} + export interface TransferOptions { - sourceAccount: LiquidityAccount - destinationAccount: LiquidityAccount + sourceAccount: TransferAccount + destinationAccount: TransferAccount sourceAmount: bigint destinationAmount?: bigint timeout: bigint // nano-seconds @@ -218,38 +236,54 @@ export async function createTransfer( return TransferError.InvalidDestinationAmount } const transfers: Required[] = [] + const sourceAccounts: LiquidityAccount[] = [] + const destinationAccounts: LiquidityAccount[] = [] - // Same asset - if (sourceAccount.asset.unit === destinationAccount.asset.unit) { + const addTransfer = ({ + sourceAccount, + destinationAccount, + amount + }: { + sourceAccount: LiquidityAccount + destinationAccount: LiquidityAccount + amount: bigint + }) => { transfers.push({ id: uuid(), sourceAccountId: sourceAccount.id, destinationAccountId: destinationAccount.id, + amount, + timeout + }) + sourceAccounts.push(sourceAccount) + destinationAccounts.push(destinationAccount) + } + + // Same asset + if (sourceAccount.asset.unit === destinationAccount.asset.unit) { + addTransfer({ + sourceAccount, + destinationAccount, amount: destinationAmount && destinationAmount < sourceAmount ? destinationAmount - : sourceAmount, - timeout + : sourceAmount }) // Same asset, different amounts if (destinationAmount && sourceAmount !== destinationAmount) { // Send excess source amount to liquidity account if (destinationAmount < sourceAmount) { - transfers.push({ - id: uuid(), - sourceAccountId: sourceAccount.id, - destinationAccountId: sourceAccount.asset.id, - amount: sourceAmount - destinationAmount, - timeout + addTransfer({ + sourceAccount, + destinationAccount: sourceAccount.asset, + amount: sourceAmount - destinationAmount }) // Deliver excess destination amount from liquidity account } else { - transfers.push({ - id: uuid(), - sourceAccountId: destinationAccount.asset.id, - destinationAccountId: destinationAccount.id, - amount: destinationAmount - sourceAmount, - timeout + addTransfer({ + sourceAccount: destinationAccount.asset, + destinationAccount, + amount: destinationAmount - sourceAmount }) } } @@ -261,22 +295,16 @@ export async function createTransfer( } // Send to source liquidity account // Deliver from destination liquidity account - transfers.push( - { - id: uuid(), - sourceAccountId: sourceAccount.id, - destinationAccountId: sourceAccount.asset.id, - amount: sourceAmount, - timeout - }, - { - id: uuid(), - sourceAccountId: destinationAccount.asset.id, - destinationAccountId: destinationAccount.id, - amount: destinationAmount, - timeout - } - ) + addTransfer({ + sourceAccount, + destinationAccount: sourceAccount.asset, + amount: sourceAmount + }) + addTransfer({ + sourceAccount: destinationAccount.asset, + destinationAccount, + amount: destinationAmount + }) } const error = await createTransfers(deps, transfers) if (error) { @@ -308,6 +336,26 @@ export async function createTransfer( if (error) { return error.error } + for (const account of sourceAccounts) { + if (account.onDebit) { + const balance = await getAccountBalance(deps, account.id) + assert.ok(balance !== undefined) + await account.onDebit({ + balance + }) + } + } + for (const account of destinationAccounts) { + if (account.onCredit) { + const balance = await getAccountBalance(deps, account.id) + assert.ok(balance !== undefined) + await account.onCredit({ + balance, + createWithdrawal: (withdrawal: Withdrawal) => + createAccountWithdrawal(deps, withdrawal) + }) + } + } }, rollback: async (): Promise => { const error = await rollbackTransfers( diff --git a/packages/backend/src/connector/core/factories/account.ts b/packages/backend/src/connector/core/factories/account.ts index b98d9bfa0b..1211ad8e32 100644 --- a/packages/backend/src/connector/core/factories/account.ts +++ b/packages/backend/src/connector/core/factories/account.ts @@ -14,7 +14,11 @@ const accountAttrs = { id: Faker.datatype.uuid(), code: assetCode, scale: assetScale, - unit: Faker.datatype.number() + unit: Faker.datatype.number(), + asset: { + id: Faker.datatype.uuid(), + unit: Faker.datatype.number() + } }, balance: 0n } diff --git a/packages/backend/src/connector/core/middleware/balance.ts b/packages/backend/src/connector/core/middleware/balance.ts index 0fee5bbda0..f5c7fc1715 100644 --- a/packages/backend/src/connector/core/middleware/balance.ts +++ b/packages/backend/src/connector/core/middleware/balance.ts @@ -66,8 +66,6 @@ export function createBalanceMiddleware(): ILPMiddleware { if (response.fulfill) { await trxOrError.commit() - // TODO: move handlePayment inside accountingServices's trxOrError.commit() - await services.invoices.handlePayment(accounts.outgoing.id) } else { await trxOrError.rollback() } diff --git a/packages/backend/src/connector/core/rafiki.ts b/packages/backend/src/connector/core/rafiki.ts index 5ff08b9dc2..78ddcb33d1 100644 --- a/packages/backend/src/connector/core/rafiki.ts +++ b/packages/backend/src/connector/core/rafiki.ts @@ -14,7 +14,7 @@ import { import { createTokenAuthMiddleware } from './middleware' import { RatesService } from '../../rates/service' import { TransferError } from '../../accounting/errors' -import { LiquidityAccount, Transaction } from '../../accounting/service' +import { TransferAccount, Transaction } from '../../accounting/service' import { AssetOptions } from '../../asset/service' import { AccountService } from '../../open_payments/account/service' import { InvoiceService } from '../../open_payments/invoice/service' @@ -27,8 +27,8 @@ import { PeerService } from '../../peer/service' // ../../open_payments/invoice/model // ../../outgoing_payment/model // ../../peer/model -export interface ConnectorAccount extends LiquidityAccount { - asset: LiquidityAccount['asset'] & AssetOptions +export interface ConnectorAccount extends TransferAccount { + asset: TransferAccount['asset'] & AssetOptions } export interface IncomingAccount extends ConnectorAccount { diff --git a/packages/backend/src/connector/core/test/middleware/balance-middleware.test.ts b/packages/backend/src/connector/core/test/middleware/balance-middleware.test.ts index 06da2dc5a6..96c3f6ad7c 100644 --- a/packages/backend/src/connector/core/test/middleware/balance-middleware.test.ts +++ b/packages/backend/src/connector/core/test/middleware/balance-middleware.test.ts @@ -30,7 +30,7 @@ const ctx = createILPContext({ }, services }) -const { accounting, invoices, rates } = services +const { accounting, rates } = services beforeEach(async () => { ctx.response.fulfill = undefined @@ -53,12 +53,10 @@ describe('Balance Middleware', function () { const next = jest.fn().mockImplementation(() => { ctx.response.fulfill = fulfill }) - const handlePaymentSpy = jest.spyOn(invoices, 'handlePayment') await expect(middleware(ctx, next)).resolves.toBeUndefined() expect(next).toHaveBeenCalledTimes(1) - expect(handlePaymentSpy).toHaveBeenCalledTimes(1) const aliceBalance = await accounting.getBalance(aliceAccount.id) expect(aliceBalance).toEqual(BigInt(0)) @@ -102,7 +100,6 @@ describe('Balance Middleware', function () { }) const createTransferSpy = jest.spyOn(accounting, 'createTransfer') - const handlePaymentSpy = jest.spyOn(invoices, 'handlePayment') if (error) { await expect(middleware(ctx, next)).rejects.toBeInstanceOf(error) @@ -113,7 +110,6 @@ describe('Balance Middleware', function () { } expect(createTransferSpy).toHaveBeenCalledTimes(createTransfer ? 1 : 0) - expect(handlePaymentSpy).not.toHaveBeenCalled() const aliceBalance = await accounting.getBalance(aliceAccount.id) expect(aliceBalance).toEqual(BigInt(100)) diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index bdf3bb2eba..05ace1805e 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -1,10 +1,11 @@ import { Model } from 'objection' import { Account } from '../account/model' import { Asset } from '../../asset/model' -import { LiquidityAccount } from '../../accounting/service' +import { LiquidityAccount, onCreditOptions } from '../../accounting/service' import { ConnectorAccount } from '../../connector/core/rafiki' import { BaseModel } from '../../shared/baseModel' import { WebhookEvent } from '../../webhook/model' +import { RETRY_LIMIT_MS } from '../../webhook/service' export class Invoice extends BaseModel @@ -38,6 +39,33 @@ export class Invoice return this.account.asset } + public async onCredit({ + balance, + createWithdrawal + }: onCreditOptions): Promise { + if (balance >= this.amount) { + return await Invoice.transaction(async (trx) => { + await this.$query(trx).patch({ + active: false + }) + const event = await InvoiceEvent.query(trx).insertAndFetch({ + type: InvoiceEventType.InvoicePaid, + data: this.toData(balance), + // TODO: + // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. + processAt: new Date() + }) + const error = await createWithdrawal({ + id: event.id, + account: this, + amount: balance, + timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns + }) + if (error) throw new Error(error) + }) + } + } + public toData(amountReceived: bigint): InvoiceData { return { invoice: { diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index 7d3ce64bc0..9d1d034a8f 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -1,4 +1,3 @@ -import assert from 'assert' import Knex from 'knex' import { WorkerUtils, makeWorkerUtils } from 'graphile-worker' import { v4 as uuid } from 'uuid' @@ -78,8 +77,7 @@ describe('Invoice Service', (): void => { const accountService = await deps.use('accountService') expect(invoice).toMatchObject({ id: invoice.id, - account: await accountService.get(accountId), - processAt: new Date(invoice.expiresAt.getTime() + 30_000) + account: await accountService.get(accountId) }) const retrievedInvoice = await invoiceService.get(invoice.id) if (!retrievedInvoice) throw new Error('invoice not found') @@ -114,7 +112,7 @@ describe('Invoice Service', (): void => { }) }) - describe('handlePayment', (): void => { + describe('onCredit', (): void => { let invoice: Invoice beforeEach( @@ -129,37 +127,37 @@ describe('Invoice Service', (): void => { ) test('Does not deactivate a partially paid invoice', async (): Promise => { - await expect( - accountingService.createDeposit({ - id: uuid(), - account: invoice, - amount: invoice.amount - BigInt(1) - }) - ).resolves.toBeUndefined() - - await invoiceService.handlePayment(invoice.id) + const createWithdrawal = jest.fn() + await invoice.onCredit({ + balance: invoice.amount - BigInt(1), + createWithdrawal + }) + expect(createWithdrawal).not.toHaveBeenCalled() await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ active: true }) }) - test('Deactivates fully paid invoice', async (): Promise => { - await expect( - accountingService.createDeposit({ - id: uuid(), - account: invoice, - amount: invoice.amount - }) - ).resolves.toBeUndefined() - - const now = new Date() - jest.useFakeTimers('modern') - jest.setSystemTime(now) - await invoiceService.handlePayment(invoice.id) + test('Deactivates fully paid invoice, creates withdrawal & webhook event', async (): Promise => { + const createWithdrawal = jest.fn() + await invoice.onCredit({ + balance: invoice.amount, + createWithdrawal + }) + expect(createWithdrawal).toHaveBeenCalledTimes(1) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - active: false, - processAt: now + active: false }) + + await expect( + InvoiceEvent.query(knex) + .whereJsonSupersetOf('data:invoice', { + id: invoice.id + }) + .where({ + type: InvoiceEventType.InvoicePaid + }) + ).resolves.toHaveLength(1) }) }) @@ -177,8 +175,19 @@ describe('Invoice Service', (): void => { }) }) + test('Does not process inactive, expired invoice', async (): Promise => { + const invoice = await invoiceService.create({ + accountId, + amount: BigInt(123), + description: 'Test invoice', + expiresAt: new Date(Date.now() - 40_000) + }) + await invoice.$query(knex).patch({ active: false }) + await expect(invoiceService.processNext()).resolves.toBeUndefined() + }) + describe('handleExpired', (): void => { - test('Deactivates an expired invoice with received money', async (): Promise => { + test('Deactivates an expired invoice with received money, creates withdrawal & webhook event', async (): Promise => { const invoice = await invoiceService.create({ accountId, amount: BigInt(123), @@ -192,15 +201,29 @@ describe('Invoice Service', (): void => { amount: BigInt(1) }) ).resolves.toBeUndefined() + await expect( + InvoiceEvent.query(knex).where({ + type: InvoiceEventType.InvoiceExpired + }) + ).resolves.toHaveLength(0) - const now = new Date() - jest.useFakeTimers('modern') - jest.setSystemTime(now) await expect(invoiceService.processNext()).resolves.toBe(invoice.id) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - active: false, - processAt: now + active: false }) + + await expect( + InvoiceEvent.query(knex) + .whereJsonSupersetOf('data:invoice', { + id: invoice.id + }) + .where({ + type: InvoiceEventType.InvoiceExpired + }) + ).resolves.toHaveLength(1) + await expect(accountingService.getBalance(invoice.id)).resolves.toEqual( + BigInt(0) + ) }) test('Deletes an expired invoice (and account) with no money', async (): Promise => { @@ -214,82 +237,6 @@ describe('Invoice Service', (): void => { expect(await invoiceService.get(invoice.id)).toBeUndefined() }) }) - - describe.each` - type | expiresAt | amountReceived - ${InvoiceEventType.InvoiceExpired} | ${-40_000} | ${BigInt(1)} - ${InvoiceEventType.InvoicePaid} | ${30_000} | ${BigInt(123)} - `( - 'handleDeactivated ($type)', - ({ type, expiresAt, amountReceived }): void => { - let invoice: Invoice - - beforeEach( - async (): Promise => { - invoice = await invoiceService.create({ - accountId, - amount: BigInt(123), - expiresAt: new Date(Date.now() + expiresAt), - description: 'Test invoice' - }) - await expect( - accountingService.createDeposit({ - id: uuid(), - account: invoice, - amount: amountReceived - }) - ).resolves.toBeUndefined() - if (type === InvoiceEventType.InvoiceExpired) { - await expect(invoiceService.processNext()).resolves.toBe( - invoice.id - ) - } else { - await invoiceService.handlePayment(invoice.id) - } - invoice = (await invoiceService.get(invoice.id)) as Invoice - expect(invoice.active).toBe(false) - expect(invoice.processAt).not.toBeNull() - await expect( - accountingService.getTotalReceived(invoice.id) - ).resolves.toEqual(amountReceived) - await expect( - accountingService.getBalance(invoice.id) - ).resolves.toEqual(amountReceived) - } - ) - - test('Creates liquidity withdrawal and webhook event', async (): Promise => { - await expect( - InvoiceEvent.query(knex).where({ type }) - ).resolves.toHaveLength(0) - await expect(invoiceService.processNext()).resolves.toBe(invoice.id) - await expect( - InvoiceEvent.query(knex) - .whereJsonSupersetOf('data:invoice', { - id: invoice.id - }) - .where({ type }) - ).resolves.toHaveLength(1) - await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - processAt: null - }) - await expect( - accountingService.getBalance(invoice.id) - ).resolves.toEqual(BigInt(0)) - }) - - test.skip("Doesn't withdraw on webhook error", async (): Promise => { - assert.ok(invoice.processAt) - await expect(invoiceService.processNext()).resolves.toBe(invoice.id) - await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - processAt: invoice.processAt - }) - await expect( - accountingService.getBalance(invoice.id) - ).resolves.toEqual(amountReceived) - }) - } - ) }) describe('Invoice pagination', (): void => { diff --git a/packages/backend/src/open_payments/invoice/service.ts b/packages/backend/src/open_payments/invoice/service.ts index b3ef7a4e51..f8527d56d1 100644 --- a/packages/backend/src/open_payments/invoice/service.ts +++ b/packages/backend/src/open_payments/invoice/service.ts @@ -27,7 +27,6 @@ export interface InvoiceService { accountId: string, pagination?: Pagination ): Promise - handlePayment(invoiceId: string): Promise processNext(): Promise } @@ -52,7 +51,6 @@ export async function createInvoiceService( create: (options, trx) => createInvoice(deps, options, trx), getAccountInvoicesPage: (accountId, pagination) => getAccountInvoicesPage(deps, accountId, pagination), - handlePayment: (invoiceId) => handleInvoicePayment(deps, invoiceId), processNext: () => processNextInvoice(deps) } } @@ -78,9 +76,7 @@ async function createInvoice( description, expiresAt, amount, - active: true, - // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. - processAt: new Date(expiresAt.getTime() + 30_000) + active: true }) .withGraphFetched('account.asset') @@ -103,39 +99,22 @@ async function createInvoice( } } -async function handleInvoicePayment( - deps: ServiceDependencies, - invoiceId: string -): Promise { - const amountReceived = await deps.accountingService.getTotalReceived( - invoiceId - ) - if (!amountReceived) { - return - } - await Invoice.query(deps.knex) - .patch({ - active: false, - processAt: new Date() - }) - .where('id', invoiceId) - .andWhere('amount', '<=', amountReceived.toString()) -} - // Fetch (and lock) an invoice for work. // Returns the id of the processed invoice (if any). async function processNextInvoice( deps_: ServiceDependencies ): Promise { return deps_.knex.transaction(async (trx) => { - const now = new Date(Date.now()).toISOString() + // 30 seconds backwards to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. + const now = new Date(Date.now() - 30_000).toISOString() const invoices = await Invoice.query(trx) .limit(1) // Ensure the invoices cannot be processed concurrently by multiple workers. .forUpdate() // If an invoice is locked, don't wait — just come back for it later. .skipLocked() - .where('processAt', '<', now) + .where('active', true) + .andWhere('expiresAt', '<', now) .withGraphFetched('account.asset') const invoice = invoices[0] @@ -148,11 +127,8 @@ async function processNextInvoice( invoice: invoice.id }) } - if (!invoice.active) { - await handleDeactivated(deps, invoice) - } else { - await handleExpired(deps, invoice) - } + await handleExpired(deps, invoice) + return invoice.id }) } @@ -169,61 +145,29 @@ async function handleExpired( if (amountReceived) { deps.logger.trace({ amountReceived }, 'deactivating expired invoice') await invoice.$query(deps.knex).patch({ - active: false, + active: false + }) + + deps.logger.trace({ amountReceived }, 'withdrawing expired invoice balance') + + const event = await InvoiceEvent.query(deps.knex).insertAndFetch({ + type: InvoiceEventType.InvoiceExpired, + data: invoice.toData(amountReceived), processAt: new Date() }) + const error = await deps.accountingService.createWithdrawal({ + id: event.id, + account: invoice, + amount: amountReceived, + timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns + }) + if (error) throw new Error(error) } else { deps.logger.debug({ amountReceived }, 'deleting expired invoice') await invoice.$query(deps.knex).delete() } } -// Withdraw deactivated invoices' liquidity. -async function handleDeactivated( - deps: ServiceDependencies, - invoice: Invoice -): Promise { - assert.ok(invoice.processAt) - const amountReceived = await deps.accountingService.getTotalReceived( - invoice.id - ) - if (!amountReceived) { - deps.logger.warn( - { amountReceived }, - 'invoice with processAt and empty balance' - ) - await invoice.$query(deps.knex).patch({ processAt: null }) - return - } - - deps.logger.trace( - { amountReceived }, - 'withdrawing deactivated invoice balance' - ) - - await invoice.$query(deps.knex).patch({ - processAt: null - }) - - const event = await InvoiceEvent.query(deps.knex).insertAndFetch({ - type: - amountReceived < invoice.amount - ? InvoiceEventType.InvoiceExpired - : InvoiceEventType.InvoicePaid, - data: invoice.toData(amountReceived), - // TODO: - // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. - processAt: new Date() - }) - const error = await deps.accountingService.createWithdrawal({ - id: event.id, - account: invoice, - amount: amountReceived, - timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns - }) - if (error) throw new Error(error) -} - /** TODO: Base64 encode/decode the cursors * Buffer.from("Hello World").toString('base64') * Buffer.from("SGVsbG8gV29ybGQ=", 'base64').toString('ascii') diff --git a/packages/backend/src/outgoing_payment/service.test.ts b/packages/backend/src/outgoing_payment/service.test.ts index 0364843599..aafb06e221 100644 --- a/packages/backend/src/outgoing_payment/service.test.ts +++ b/packages/backend/src/outgoing_payment/service.test.ts @@ -552,9 +552,8 @@ describe('OutgoingPaymentService', (): void => { beforeEach( async (): Promise => { // Don't send invoice.paid webhook events - const invoiceService = await deps.use('invoiceService') jest - .spyOn(invoiceService, 'handlePayment') + .spyOn(invoice, 'onCredit') .mockImplementation(() => Promise.resolve()) } ) diff --git a/packages/backend/src/tests/accountFactory.ts b/packages/backend/src/tests/accountFactory.ts index 0cec0c7f61..237217438a 100644 --- a/packages/backend/src/tests/accountFactory.ts +++ b/packages/backend/src/tests/accountFactory.ts @@ -1,30 +1,23 @@ import { v4 as uuid } from 'uuid' -import { AccountingService, LiquidityAccount } from '../accounting/service' +import { + AccountingService, + LiquidityAccount, + TransferAccount +} from '../accounting/service' import { randomUnit } from './asset' type BuildOptions = Partial & { balance?: bigint } -export type FactoryAccount = Omit & { - asset: { - id: string - unit: number - asset: { - id: string - unit: number - } - } -} - export class AccountFactory { public constructor( private accounts: AccountingService, private unitGenerator: () => number = randomUnit ) {} - public async build(options: BuildOptions = {}): Promise { + public async build(options: BuildOptions = {}): Promise { const assetId = options.asset?.id || uuid() const unit = options.asset?.unit || this.unitGenerator() const asset = { From 3075f086c781a137eb232135894e200082be40de Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Fri, 4 Feb 2022 21:14:48 -0600 Subject: [PATCH 04/15] chore(backend): update transaction api docs --- docs/transaction-api.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/docs/transaction-api.md b/docs/transaction-api.md index 760fb2cd62..b4771da876 100644 --- a/docs/transaction-api.md +++ b/docs/transaction-api.md @@ -87,7 +87,7 @@ An expired invoice that has never received money is deleted. Rafiki sends webhook events to notify the wallet of payment lifecycle states that require liquidity to be added or removed. -Webhook event handlers must be idempotent. +Webhook event handlers must be idempotent and return `200`. ### `EventType` @@ -95,39 +95,37 @@ Webhook event handlers must be idempotent. Invoice has expired. -Credit `invoice.received` to the wallet balance for `invoice.accountId` and return `200`. +Credit `invoice.received` to the wallet balance for `invoice.accountId`, and call `Mutation.withdrawLiquidity` with the event id. #### `invoice.paid` Invoice has received its specified `amount`. -Credit `invoice.received` to the wallet balance for `invoice.accountId` and return `200`. +Credit `invoice.received` to the wallet balance for `invoice.accountId`, and call `Mutation.withdrawLiquidity` with the event id. #### `outgoing_payment.funding` Payment needs liquidity in order to send quoted amount. -To fund the payment, deduct `quote.maxSourceAmount` from the wallet balance for `payment.accountId` and return `200`. - -To cancel the payment, return `403`. +To fund the payment, deduct `quote.maxSourceAmount` from the wallet balance for `payment.accountId` and call `Mutation.depositLiquidity` with the event id. #### `outgoing_payment.cancelled` Payment was cancelled. -Credit `payment.balance` to the wallet balance for `payment.accountId` and return `200` or `205` to retry the payment. +Credit `payment.balance` to the wallet balance for `payment.accountId`, and call `Mutation.withdrawLiquidity` with the event id. #### `outgoing_payment.completed` Payment completed sending the quoted amount. -Credit `payment.balance` to the wallet balance for `payment.accountId` and return `200`. +Credit `payment.balance` to the wallet balance for `payment.accountId`, and call `Mutation.withdrawLiquidity` with the event id. ### Webhook Event | Name | Optional | Type | Description | | :----- | :------- | :------------------------------------------------------------- | :------------------------------------------------ | -| `id` | No | `ID` | Unique ID of the `data` object. | +| `id` | No | `ID` | Unique ID of the webhook event. | | `type` | No | [`EventType`](#eventtype) | Description of the event. | | `data` | No | [`Invoice`](#invoice) or [`OutgoingPayment`](#outgoingpayment) | Object containing data associated with the event. | From cdee2bb4c3218e7e266ca3ee52d982cd82eb0978 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Mon, 7 Feb 2022 12:11:24 -0600 Subject: [PATCH 05/15] chore(backend): update transaction api docs --- docs/transaction-api.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/transaction-api.md b/docs/transaction-api.md index b4771da876..2518702b2f 100644 --- a/docs/transaction-api.md +++ b/docs/transaction-api.md @@ -95,31 +95,31 @@ Webhook event handlers must be idempotent and return `200`. Invoice has expired. -Credit `invoice.received` to the wallet balance for `invoice.accountId`, and call `Mutation.withdrawLiquidity` with the event id. +Credit `invoice.received` to the wallet balance for `invoice.accountId`, and call `Mutation.withdrawEventLiquidity` with the event id. #### `invoice.paid` Invoice has received its specified `amount`. -Credit `invoice.received` to the wallet balance for `invoice.accountId`, and call `Mutation.withdrawLiquidity` with the event id. +Credit `invoice.received` to the wallet balance for `invoice.accountId`, and call `Mutation.withdrawEventLiquidity` with the event id. #### `outgoing_payment.funding` Payment needs liquidity in order to send quoted amount. -To fund the payment, deduct `quote.maxSourceAmount` from the wallet balance for `payment.accountId` and call `Mutation.depositLiquidity` with the event id. +To fund the payment, deduct `quote.maxSourceAmount` from the wallet balance for `payment.accountId` and call `Mutation.depositEventLiquidity` with the event id. #### `outgoing_payment.cancelled` Payment was cancelled. -Credit `payment.balance` to the wallet balance for `payment.accountId`, and call `Mutation.withdrawLiquidity` with the event id. +Credit `payment.balance` to the wallet balance for `payment.accountId`, and call `Mutation.withdrawEventLiquidity` with the event id. #### `outgoing_payment.completed` Payment completed sending the quoted amount. -Credit `payment.balance` to the wallet balance for `payment.accountId`, and call `Mutation.withdrawLiquidity` with the event id. +Credit `payment.balance` to the wallet balance for `payment.accountId`, and call `Mutation.withdrawEventLiquidity` with the event id. ### Webhook Event From 1786e7d075cd9e5466ef8bd4c43ff70c402f2076 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Tue, 8 Feb 2022 16:48:19 -0600 Subject: [PATCH 06/15] chore(backend): create withdrawal in webhook service --- ...20131110501_create_webhook_events_table.js | 5 + packages/backend/src/accounting/service.ts | 23 +- .../src/graphql/resolvers/liquidity.test.ts | 80 +++- .../src/graphql/resolvers/liquidity.ts | 13 +- packages/backend/src/index.ts | 9 +- .../src/open_payments/invoice/model.ts | 18 +- .../src/open_payments/invoice/service.test.ts | 24 +- .../src/open_payments/invoice/service.ts | 3 +- .../backend/src/outgoing_payment/service.ts | 2 - packages/backend/src/webhook/model.ts | 50 +++ packages/backend/src/webhook/service.test.ts | 375 ++++++++---------- packages/backend/src/webhook/service.ts | 26 ++ 12 files changed, 346 insertions(+), 282 deletions(-) diff --git a/packages/backend/migrations/20220131110501_create_webhook_events_table.js b/packages/backend/migrations/20220131110501_create_webhook_events_table.js index e0e50b1157..2b5254baf7 100644 --- a/packages/backend/migrations/20220131110501_create_webhook_events_table.js +++ b/packages/backend/migrations/20220131110501_create_webhook_events_table.js @@ -7,6 +7,11 @@ exports.up = function (knex) { table.integer('attempts').notNullable().defaultTo(0) table.string('error').nullable() + table.uuid('withdrawalAccountId').nullable() + table.uuid('withdrawalAssetId').nullable() + table.foreign('withdrawalAssetId').references('assets.id') + table.bigInteger('withdrawalAmount').nullable() + table.timestamp('processAt').notNullable() table.timestamp('createdAt').defaultTo(knex.fn.now()) diff --git a/packages/backend/src/accounting/service.ts b/packages/backend/src/accounting/service.ts index d9fdc387e6..3a4b10a549 100644 --- a/packages/backend/src/accounting/service.ts +++ b/packages/backend/src/accounting/service.ts @@ -44,17 +44,8 @@ export interface LiquidityAccount { id: string unit: number } - onCredit?: (options: onCreditOptions) => Promise - onDebit?: (options: onDebitOptions) => Promise -} - -export interface onCreditOptions { - balance: bigint - createWithdrawal: AccountingService['createWithdrawal'] -} - -interface onDebitOptions { - balance: bigint + onCredit?: (balance: bigint) => Promise + onDebit?: (balance: bigint) => Promise } export interface Deposit { @@ -340,20 +331,14 @@ export async function createTransfer( if (account.onDebit) { const balance = await getAccountBalance(deps, account.id) assert.ok(balance !== undefined) - await account.onDebit({ - balance - }) + await account.onDebit(balance) } } for (const account of destinationAccounts) { if (account.onCredit) { const balance = await getAccountBalance(deps, account.id) assert.ok(balance !== undefined) - await account.onCredit({ - balance, - createWithdrawal: (withdrawal: Withdrawal) => - createAccountWithdrawal(deps, withdrawal) - }) + await account.onCredit(balance) } } }, diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index 0b20524706..0f9d47d322 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -3,7 +3,7 @@ import Knex from 'knex' import { v4 as uuid } from 'uuid' import * as Pay from '@interledger/pay' -import { DepositEventType, WithdrawEventType } from './liquidity' +import { DepositEventType } from './liquidity' import { createTestApp, TestContainer } from '../../tests/app' import { IocContract } from '@adonisjs/fold' import { AppServices } from '../../app' @@ -13,11 +13,12 @@ import { AccountingService, LiquidityAccount } from '../../accounting/service' import { Asset } from '../../asset/model' import { AssetService } from '../../asset/service' import { Account } from '../../open_payments/account/model' -import { Invoice } from '../../open_payments/invoice/model' +import { Invoice, InvoiceEventType } from '../../open_payments/invoice/model' import { OutgoingPayment, PaymentState, PaymentEvent, + PaymentWithdrawType, isPaymentEventType } from '../../outgoing_payment/model' import { Peer } from '../../peer/model' @@ -1748,6 +1749,9 @@ describe('Liquidity Resolvers', (): void => { ) }) + const WithdrawEventType = { ...InvoiceEventType, ...PaymentWithdrawType } + type WithdrawEventType = InvoiceEventType | PaymentWithdrawType + describe('withdrawEventLiquidity', (): void => { describe.each(Object.values(WithdrawEventType).map((type) => [type]))( '%s', @@ -1759,26 +1763,28 @@ describe('Liquidity Resolvers', (): void => { eventId = uuid() const amount = BigInt(10) let account: LiquidityAccount + let data: Record if (isPaymentEventType(type)) { account = payment - await WebhookEvent.query(knex).insertAndFetch({ - id: eventId, - type, - data: payment.toData({ - amountSent: BigInt(0), - balance: amount - }), - processAt: new Date() + data = payment.toData({ + amountSent: BigInt(0), + balance: amount }) } else { account = invoice - await WebhookEvent.query(knex).insertAndFetch({ - id: eventId, - type, - data: invoice.toData(amount), - processAt: new Date() - }) + data = invoice.toData(amount) } + await WebhookEvent.query(knex).insertAndFetch({ + id: eventId, + type, + data, + processAt: new Date(), + withdrawal: { + accountId: account.id, + assetId: account.asset.id, + amount + } + }) await expect( accountingService.createDeposit({ id: uuid(), @@ -1865,6 +1871,48 @@ describe('Liquidity Resolvers', (): void => { expect(response.error).toEqual(LiquidityError.InvalidId) }) + test('Returns error for non-existent webhook event withdrawal', async (): Promise => { + const webhookService = await deps.use('webhookService') + const { type, data, processAt } = (await webhookService.getEvent( + eventId + )) as WebhookEvent + const event = await WebhookEvent.query(knex).insertAndFetch({ + type, + data, + processAt + }) + const response = await appContainer.apolloClient + .mutate({ + mutation: gql` + mutation WithdrawLiquidity($eventId: String!) { + withdrawEventLiquidity(eventId: $eventId) { + code + success + message + error + } + } + `, + variables: { + eventId: event.id + } + }) + .then( + (query): LiquidityMutationResponse => { + if (query.data) { + return query.data.withdrawEventLiquidity + } else { + throw new Error('Data was empty') + } + } + ) + + expect(response.success).toBe(false) + expect(response.code).toEqual('400') + expect(response.message).toEqual('Invalid id') + expect(response.error).toEqual(LiquidityError.InvalidId) + }) + test('Returns error for finalized withdrawal', async (): Promise => { await expect( accountingService.commitWithdrawal(eventId) diff --git a/packages/backend/src/graphql/resolvers/liquidity.ts b/packages/backend/src/graphql/resolvers/liquidity.ts index 39e3deedf7..a773e9fd0c 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.ts @@ -7,12 +7,10 @@ import { AccountWithdrawalMutationResponse } from '../generated/graphql' import { ApolloContext } from '../../app' -import { InvoiceEventType } from '../../open_payments/invoice/model' import { FundingError, isFundingError } from '../../outgoing_payment/errors' import { isPaymentEvent, - PaymentDepositType, - PaymentWithdrawType + PaymentDepositType } from '../../outgoing_payment/model' export const addPeerLiquidity: MutationResolvers['addPeerLiquidity'] = async ( @@ -336,13 +334,6 @@ export const depositEventLiquidity: MutationResolvers['depositEve } } -export const WithdrawEventType = { ...InvoiceEventType, ...PaymentWithdrawType } -export type WithdrawEventType = InvoiceEventType | PaymentWithdrawType - -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -const isWithdrawEventType = (o: any): o is WithdrawEventType => - Object.values(WithdrawEventType).includes(o) - export const withdrawEventLiquidity: MutationResolvers['withdrawEventLiquidity'] = async ( parent, args, @@ -352,7 +343,7 @@ export const withdrawEventLiquidity: MutationResolvers['withdrawE const webhookService = await ctx.container.use('webhookService') // TODO: remove event lookup when commitWithdrawal can verify transfer code const event = await webhookService.getEvent(args.eventId) - if (!event || !isWithdrawEventType(event.type)) { + if (!event || !event.withdrawal) { return responses[LiquidityError.InvalidId] } const accountingService = await ctx.container.use('accountingService') diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index d0870bb6db..482c1054ae 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -178,15 +178,15 @@ export function initIocContainer( return createWebhookService({ config: await deps.use('config'), knex: await deps.use('knex'), - logger: await deps.use('logger') + logger: await deps.use('logger'), + accountingService: await deps.use('accountingService') }) }) container.singleton('invoiceService', async (deps) => { return await createInvoiceService({ logger: await deps.use('logger'), knex: await deps.use('knex'), - accountingService: await deps.use('accountingService'), - webhookService: await deps.use('webhookService') + accountingService: await deps.use('accountingService') }) }) container.singleton('invoiceRoutes', async (deps) => { @@ -238,8 +238,7 @@ export function initIocContainer( accountingService: await deps.use('accountingService'), makeIlpPlugin: await deps.use('makeIlpPlugin'), accountService: await deps.use('accountService'), - ratesService: await deps.use('ratesService'), - webhookService: await deps.use('webhookService') + ratesService: await deps.use('ratesService') }) }) diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index 05ace1805e..570590e8a5 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -1,11 +1,10 @@ import { Model } from 'objection' import { Account } from '../account/model' import { Asset } from '../../asset/model' -import { LiquidityAccount, onCreditOptions } from '../../accounting/service' +import { LiquidityAccount } from '../../accounting/service' import { ConnectorAccount } from '../../connector/core/rafiki' import { BaseModel } from '../../shared/baseModel' import { WebhookEvent } from '../../webhook/model' -import { RETRY_LIMIT_MS } from '../../webhook/service' export class Invoice extends BaseModel @@ -39,29 +38,20 @@ export class Invoice return this.account.asset } - public async onCredit({ - balance, - createWithdrawal - }: onCreditOptions): Promise { + public async onCredit(balance: bigint): Promise { if (balance >= this.amount) { return await Invoice.transaction(async (trx) => { await this.$query(trx).patch({ active: false }) - const event = await InvoiceEvent.query(trx).insertAndFetch({ + await InvoiceEvent.query(trx).insertAndFetch({ type: InvoiceEventType.InvoicePaid, data: this.toData(balance), // TODO: // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. + // But balance is fixed in the webhook event data. processAt: new Date() }) - const error = await createWithdrawal({ - id: event.id, - account: this, - amount: balance, - timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns - }) - if (error) throw new Error(error) }) } } diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index 9d1d034a8f..1083594a18 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -127,28 +127,26 @@ describe('Invoice Service', (): void => { ) test('Does not deactivate a partially paid invoice', async (): Promise => { - const createWithdrawal = jest.fn() - await invoice.onCredit({ - balance: invoice.amount - BigInt(1), - createWithdrawal - }) - expect(createWithdrawal).not.toHaveBeenCalled() + await invoice.onCredit(invoice.amount - BigInt(1)) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ active: true }) }) - test('Deactivates fully paid invoice, creates withdrawal & webhook event', async (): Promise => { - const createWithdrawal = jest.fn() - await invoice.onCredit({ - balance: invoice.amount, - createWithdrawal - }) - expect(createWithdrawal).toHaveBeenCalledTimes(1) + test('Deactivates fully paid invoice', async (): Promise => { + await invoice.onCredit(invoice.amount) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ active: false }) + }) + test('Creates invoice.paid webhook event', async (): Promise => { + await expect( + InvoiceEvent.query(knex).where({ + type: InvoiceEventType.InvoicePaid + }) + ).resolves.toHaveLength(0) + await invoice.onCredit(invoice.amount) await expect( InvoiceEvent.query(knex) .whereJsonSupersetOf('data:invoice', { diff --git a/packages/backend/src/open_payments/invoice/service.ts b/packages/backend/src/open_payments/invoice/service.ts index f8527d56d1..309139cea7 100644 --- a/packages/backend/src/open_payments/invoice/service.ts +++ b/packages/backend/src/open_payments/invoice/service.ts @@ -2,7 +2,7 @@ import { Invoice, InvoiceEvent, InvoiceEventType } from './model' import { AccountingService } from '../../accounting/service' import { BaseService } from '../../shared/baseService' import { Pagination } from '../../shared/pagination' -import { WebhookService, RETRY_LIMIT_MS } from '../../webhook/service' +import { RETRY_LIMIT_MS } from '../../webhook/service' import assert from 'assert' import { Transaction } from 'knex' import { ForeignKeyViolationError, TransactionOrKnex } from 'objection' @@ -33,7 +33,6 @@ export interface InvoiceService { interface ServiceDependencies extends BaseService { knex: TransactionOrKnex accountingService: AccountingService - webhookService: WebhookService } export async function createInvoiceService( diff --git a/packages/backend/src/outgoing_payment/service.ts b/packages/backend/src/outgoing_payment/service.ts index 9407beb939..912b7bc7e5 100644 --- a/packages/backend/src/outgoing_payment/service.ts +++ b/packages/backend/src/outgoing_payment/service.ts @@ -7,7 +7,6 @@ import { OutgoingPayment, PaymentIntent, PaymentState } from './model' import { AccountingService } from '../accounting/service' import { AccountService } from '../open_payments/account/service' import { RatesService } from '../rates/service' -import { WebhookService } from '../webhook/service' import { IlpPlugin, IlpPluginOptions } from './ilp_plugin' import * as worker from './worker' @@ -36,7 +35,6 @@ export interface ServiceDependencies extends BaseService { accountingService: AccountingService accountService: AccountService ratesService: RatesService - webhookService: WebhookService makeIlpPlugin: (options: IlpPluginOptions) => IlpPlugin } diff --git a/packages/backend/src/webhook/model.ts b/packages/backend/src/webhook/model.ts index 33f1032cac..2e8b2fff9a 100644 --- a/packages/backend/src/webhook/model.ts +++ b/packages/backend/src/webhook/model.ts @@ -1,5 +1,10 @@ +import { Model, Pojo } from 'objection' + +import { Asset } from '../asset/model' import { BaseModel } from '../shared/baseModel' +const fieldPrefixes = ['withdrawal'] + export class WebhookEvent extends BaseModel { public static get tableName(): string { return 'webhookEvents' @@ -10,4 +15,49 @@ export class WebhookEvent extends BaseModel { public attempts!: number public error?: string | null public processAt!: Date + + public withdrawal?: { + accountId: string + assetId: string + amount: bigint + } + + static relationMappings = { + withdrawalAsset: { + relation: Model.HasOneRelation, + modelClass: Asset, + join: { + from: 'webhookEvents.withdrawalAssetId', + to: 'assets.id' + } + } + } + + $formatDatabaseJson(json: Pojo): Pojo { + for (const prefix of fieldPrefixes) { + if (!json[prefix]) continue + for (const key in json[prefix]) { + json[prefix + key.charAt(0).toUpperCase() + key.slice(1)] = + json[prefix][key] + } + delete json[prefix] + } + return super.$formatDatabaseJson(json) + } + + $parseDatabaseJson(json: Pojo): Pojo { + json = super.$parseDatabaseJson(json) + for (const key in json) { + const prefix = fieldPrefixes.find((prefix) => key.startsWith(prefix)) + if (!prefix) continue + if (json[key] !== null) { + if (!json[prefix]) json[prefix] = {} + json[prefix][ + key.charAt(prefix.length).toLowerCase() + key.slice(prefix.length + 1) + ] = json[key] + } + delete json[key] + } + return json + } } diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index 360b99f117..d740363752 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -4,7 +4,6 @@ import nock, { Definition } from 'nock' import { URL } from 'url' import Knex from 'knex' import { v4 as uuid } from 'uuid' -import * as Pay from '@interledger/pay' import { WebhookEvent } from './model' import { @@ -14,37 +13,44 @@ import { RETRY_BACKOFF_MS, RETRY_LIMIT_MS } from './service' +import { AccountingService } from '../accounting/service' import { createTestApp, TestContainer } from '../tests/app' +import { AccountFactory } from '../tests/accountFactory' import { randomAsset } from '../tests/asset' -import { truncateTable, truncateTables } from '../tests/tableManager' +import { truncateTables } from '../tests/tableManager' import { Config } from '../config/app' import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../' import { AppServices } from '../app' -import { Invoice, InvoiceEventType } from '../open_payments/invoice/model' -import { - OutgoingPayment, - PaymentState, - PaymentEventType, - isPaymentEventType -} from '../outgoing_payment/model' - -export const EventType = { ...InvoiceEventType, ...PaymentEventType } -export type EventType = InvoiceEventType | PaymentEventType describe('Webhook Service', (): void => { let deps: IocContract let appContainer: TestContainer let webhookService: WebhookService + let accountingService: AccountingService let knex: Knex - let invoice: Invoice - let payment: OutgoingPayment - let amountReceived: bigint - let amountSent: bigint - let balance: bigint let webhookUrl: URL + let event: WebhookEvent const WEBHOOK_SECRET = 'test secret' + async function makeWithdrawalEvent(event: WebhookEvent): Promise { + const assetService = await deps.use('assetService') + const accountFactory = new AccountFactory(accountingService) + const asset = await assetService.getOrCreate(randomAsset()) + const amount = BigInt(10) + const account = await accountFactory.build({ + asset, + balance: amount + }) + await event.$query(knex).patch({ + withdrawal: { + accountId: account.id, + assetId: account.asset.id, + amount + } + }) + } + beforeAll( async (): Promise => { Config.webhookSecret = WEBHOOK_SECRET @@ -52,211 +58,180 @@ describe('Webhook Service', (): void => { appContainer = await createTestApp(deps) knex = await deps.use('knex') webhookService = await deps.use('webhookService') - const accountService = await deps.use('accountService') - const { id: accountId } = await accountService.create({ - asset: randomAsset() - }) - const invoiceService = await deps.use('invoiceService') - invoice = await invoiceService.create({ - accountId, - amount: BigInt(56), - expiresAt: new Date(Date.now() + 60 * 1000), - description: 'description!' - }) - const outgoingPaymentService = await deps.use('outgoingPaymentService') - const config = await deps.use('config') - const invoiceUrl = `${config.publicHost}/invoices/${invoice.id}` - payment = await outgoingPaymentService.create({ - accountId, - invoiceUrl, - autoApprove: false - }) - await payment.$query(knex).patch({ - state: PaymentState.Funding, - quote: { - timestamp: new Date(), - activationDeadline: new Date(Date.now() + 1000), - targetType: Pay.PaymentType.FixedSend, - minDeliveryAmount: BigInt(123), - maxSourceAmount: BigInt(456), - maxPacketAmount: BigInt(789), - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - minExchangeRate: Pay.Ratio.from(1.23)!, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - lowExchangeRateEstimate: Pay.Ratio.from(1.2)!, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - highExchangeRateEstimate: Pay.Ratio.from(2.3)!, - amountSent: BigInt(0) - } + accountingService = await deps.use('accountingService') + webhookUrl = new URL(Config.webhookUrl) + } + ) + + beforeEach( + async (): Promise => { + event = await WebhookEvent.query(knex).insertAndFetch({ + id: uuid(), + type: 'account.test_event', + data: { + account: { + id: uuid() + } + }, + processAt: new Date() }) - amountReceived = BigInt(5) - amountSent = BigInt(10) - balance = BigInt(0) - webhookUrl = new URL(config.webhookUrl) } ) afterEach( async (): Promise => { jest.useRealTimers() - await truncateTable(knex, WebhookEvent.tableName) + await truncateTables(knex) } ) afterAll( async (): Promise => { await appContainer.shutdown() - await truncateTables(knex) } ) - describe.each(Object.values(EventType).map((type) => [type]))( - '%s', - (type): void => { - let event: WebhookEvent - - beforeEach( - async (): Promise => { - event = await WebhookEvent.query(knex).insertAndFetch({ - id: uuid(), - type, - data: isPaymentEventType(type) - ? payment.toData({ amountSent, balance }) - : invoice.toData(amountReceived), - processAt: new Date() + describe('Get Webhook Event', (): void => { + test('A webhook event can be fetched', async (): Promise => { + await expect(webhookService.getEvent(event.id)).resolves.toEqual(event) + }) + + test('A withdrawal webhook event can be fetched', async (): Promise => { + await makeWithdrawalEvent(event) + assert.ok(event.withdrawal) + await expect(webhookService.getEvent(event.id)).resolves.toEqual(event) + }) + + test('Cannot fetch a bogus webhook event', async (): Promise => { + await expect(webhookService.getEvent(uuid())).resolves.toBeUndefined() + }) + }) + + describe('processNext', (): void => { + function mockWebhookServer(status = 200): nock.Scope { + return nock(webhookUrl.origin) + .post(webhookUrl.pathname, function (this: Definition, body) { + assert.ok(this.headers) + const signature = this.headers['rafiki-signature'] + expect( + generateWebhookSignature( + body, + WEBHOOK_SECRET, + Config.signatureVersion + ) + ).toEqual(signature) + expect(body).toMatchObject({ + id: event.id, + type: event.type, + data: event.data }) - } - ) - - describe('Get Webhook Event', (): void => { - test('A webhook event can be fetched', async (): Promise => { - await expect(webhookService.getEvent(event.id)).resolves.toEqual( - event - ) + return true }) + .reply(status) + } - test('Cannot fetch a bogus webhook event', async (): Promise => { - await expect(webhookService.getEvent(uuid())).resolves.toBeUndefined() - }) + test('Does not process events not scheduled to be sent', async (): Promise => { + await event.$query(knex).patch({ + processAt: new Date(Date.now() + 30_000) }) + await expect(webhookService.getEvent(event.id)).resolves.toEqual(event) + await expect(webhookService.processNext()).resolves.toBeUndefined() + }) + + test('Creates webhook event withdrawal', async (): Promise => { + await makeWithdrawalEvent(event) + assert.ok(event.withdrawal) + const asset = await WebhookEvent.relatedQuery('withdrawalAsset', knex) + .for(event.id) + .first() + assert.ok(asset) + const withdrawalSpy = jest.spyOn(accountingService, 'createWithdrawal') + const scope = mockWebhookServer() + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + expect(withdrawalSpy).toHaveBeenCalledWith({ + id: event.id, + account: { + id: event.withdrawal.accountId, + asset + }, + amount: event.withdrawal.amount, + timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) + }) + }) + + test('Sends webhook event', async (): Promise => { + const scope = mockWebhookServer() + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ + attempts: 0, + error: null, + processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + }) + }) + + test('Schedules retry if request fails', async (): Promise => { + const status = 504 + const scope = mockWebhookServer(status) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + const updatedEvent = await webhookService.getEvent(event.id) + assert.ok(updatedEvent) + expect(updatedEvent).toMatchObject({ + attempts: 1, + error: `Request failed with status code ${status}` + }) + expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( + event.createdAt.getTime() + RETRY_BACKOFF_MS + ) + }) + + test('Schedules retry if request times out', async (): Promise => { + const scope = nock(webhookUrl.origin) + .post(webhookUrl.pathname) + .delayConnection(Config.webhookTimeout + 1) + .reply(200) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + const updatedEvent = await webhookService.getEvent(event.id) + assert.ok(updatedEvent) + expect(updatedEvent).toMatchObject({ + attempts: 1, + error: 'timeout of 2000ms exceeded' + }) + expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( + event.createdAt.getTime() + RETRY_BACKOFF_MS + ) + }) - describe('processNext', (): void => { - function mockWebhookServer(status = 200): nock.Scope { - return nock(webhookUrl.origin) - .post(webhookUrl.pathname, function (this: Definition, body) { - assert.ok(this.headers) - const signature = this.headers['rafiki-signature'] - expect( - generateWebhookSignature( - body, - WEBHOOK_SECRET, - Config.signatureVersion - ) - ).toEqual(signature) - expect(body.id).toEqual(event.id) - expect(body.type).toEqual(type) - if (isPaymentEventType(type)) { - expect(body.data).toEqual( - payment.toData({ amountSent, balance }) - ) - } else { - expect(body.data).toEqual(invoice.toData(amountReceived)) - } - return true - }) - .reply(status) - } - - test('Does not process events not scheduled to be sent', async (): Promise => { - await event.$query(knex).patch({ - processAt: new Date(Date.now() + 30_000) - }) - await expect(webhookService.getEvent(event.id)).resolves.toEqual( - event - ) - await expect(webhookService.processNext()).resolves.toBeUndefined() - }) - - test('Sends webhook event', async (): Promise => { - const scope = mockWebhookServer() - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(scope.isDone()).toBe(true) - await expect( - webhookService.getEvent(event.id) - ).resolves.toMatchObject({ - attempts: 0, - error: null, - processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) - }) - }) - - test('Schedules retry if request fails', async (): Promise => { - const status = 504 - const scope = mockWebhookServer(status) - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(scope.isDone()).toBe(true) - const updatedEvent = await webhookService.getEvent(event.id) - assert.ok(updatedEvent) - expect(updatedEvent).toMatchObject({ - attempts: 1, - error: `Request failed with status code ${status}` - }) - expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( - event.createdAt.getTime() + RETRY_BACKOFF_MS - ) - }) - - test('Schedules retry if request times out', async (): Promise => { - const scope = nock(webhookUrl.origin) - .post(webhookUrl.pathname) - .delayConnection(Config.webhookTimeout + 1) - .reply(200) - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(scope.isDone()).toBe(true) - const updatedEvent = await webhookService.getEvent(event.id) - assert.ok(updatedEvent) - expect(updatedEvent).toMatchObject({ - attempts: 1, - error: 'timeout of 2000ms exceeded' - }) - expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( - event.createdAt.getTime() + RETRY_BACKOFF_MS - ) - }) - - test('Stops retrying after limit', async (): Promise => { - jest.useFakeTimers('modern') - jest.setSystemTime( - new Date(event.createdAt.getTime() + RETRY_LIMIT_MS - 1) - ) + test('Stops retrying after limit', async (): Promise => { + jest.useFakeTimers('modern') + jest.setSystemTime( + new Date(event.createdAt.getTime() + RETRY_LIMIT_MS - 1) + ) - const error = 'last try' - const mockPost = jest - .spyOn(axios, 'post') - .mockRejectedValueOnce(new Error(error)) - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(mockPost).toHaveBeenCalledTimes(1) - await expect( - webhookService.getEvent(event.id) - ).resolves.toMatchObject({ - attempts: 1, - error, - processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) - }) - }) + const error = 'last try' + const mockPost = jest + .spyOn(axios, 'post') + .mockRejectedValueOnce(new Error(error)) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(mockPost).toHaveBeenCalledTimes(1) + await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ + attempts: 1, + error, + processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + }) + }) - test('Deletes event after retention period', async (): Promise => { - jest.useFakeTimers('modern') - jest.setSystemTime( - new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) - ) + test('Deletes event after retention period', async (): Promise => { + jest.useFakeTimers('modern') + jest.setSystemTime( + new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + ) - await expect(webhookService.processNext()).resolves.toEqual(event.id) - await expect( - webhookService.getEvent(event.id) - ).resolves.toBeUndefined() - }) - }) - } - ) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + await expect(webhookService.getEvent(event.id)).resolves.toBeUndefined() + }) + }) }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index 25f6b8d0a7..805f4d8d0b 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -3,6 +3,9 @@ import axios from 'axios' import { createHmac } from 'crypto' import { WebhookEvent } from './model' +import { TransferError } from '../accounting/errors' +import { AccountingService } from '../accounting/service' +import { Asset } from '../asset/model' import { IAppConfig } from '../config/app' import { BaseService } from '../shared/baseService' @@ -18,6 +21,7 @@ export interface WebhookService { interface ServiceDependencies extends BaseService { config: IAppConfig + accountingService: AccountingService } export async function createWebhookService( @@ -82,6 +86,28 @@ async function sendWebhookEvent( event: WebhookEvent ): Promise { try { + if (event.withdrawal) { + const asset = (await WebhookEvent.relatedQuery( + 'withdrawalAsset', + deps.knex + ) + .for(event.id) + .first()) as Asset + assert.ok(asset) + const error = await deps.accountingService.createWithdrawal({ + id: event.id, + account: { + id: event.withdrawal.accountId, + asset + }, + amount: event.withdrawal.amount, + timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns + }) + if (error && error !== TransferError.TransferExists) { + throw new Error(error) + } + } + const requestHeaders = { 'Content-Type': 'application/json' } From e47695c9611f756a70aa630b847d83030dd34462 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Wed, 9 Feb 2022 14:29:20 -0600 Subject: [PATCH 07/15] chore(backend): specify webhook event withdrawals If invoice event withdrawal hasn't been created, update the amount. Otherwise, create new invoice event for subsequent received amounts. --- ...20131110501_create_webhook_events_table.js | 4 +- ...> 20220209103122_create_invoices_table.js} | 2 + packages/backend/src/accounting/service.ts | 4 +- .../src/graphql/resolvers/liquidity.test.ts | 9 +- .../src/open_payments/invoice/model.ts | 106 ++++++++++++------ .../src/open_payments/invoice/service.test.ts | 84 +++++++++++--- .../backend/src/outgoing_payment/lifecycle.ts | 24 ++-- .../src/outgoing_payment/service.test.ts | 40 +++---- packages/backend/src/webhook/service.test.ts | 5 +- packages/backend/src/webhook/service.ts | 1 + 10 files changed, 170 insertions(+), 109 deletions(-) rename packages/backend/migrations/{20211022210203_create_invoices_table.js => 20220209103122_create_invoices_table.js} (93%) diff --git a/packages/backend/migrations/20220131110501_create_webhook_events_table.js b/packages/backend/migrations/20220131110501_create_webhook_events_table.js index 2b5254baf7..7fda4c60fe 100644 --- a/packages/backend/migrations/20220131110501_create_webhook_events_table.js +++ b/packages/backend/migrations/20220131110501_create_webhook_events_table.js @@ -3,7 +3,7 @@ exports.up = function (knex) { table.uuid('id').notNullable().primary() table.string('type').notNullable() - table.jsonb('data').notNullable() + table.json('data').notNullable() table.integer('attempts').notNullable().defaultTo(0) table.string('error').nullable() @@ -12,7 +12,7 @@ exports.up = function (knex) { table.foreign('withdrawalAssetId').references('assets.id') table.bigInteger('withdrawalAmount').nullable() - table.timestamp('processAt').notNullable() + table.timestamp('processAt').notNullable().defaultTo(knex.fn.now()) table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) diff --git a/packages/backend/migrations/20211022210203_create_invoices_table.js b/packages/backend/migrations/20220209103122_create_invoices_table.js similarity index 93% rename from packages/backend/migrations/20211022210203_create_invoices_table.js rename to packages/backend/migrations/20220209103122_create_invoices_table.js index b8d4c932d8..b8a8454d5a 100644 --- a/packages/backend/migrations/20211022210203_create_invoices_table.js +++ b/packages/backend/migrations/20220209103122_create_invoices_table.js @@ -9,6 +9,8 @@ exports.up = function (knex) { table.string('description').nullable() table.timestamp('expiresAt').notNullable() table.bigInteger('amount').notNullable() + table.uuid('eventId').nullable() + table.foreign('eventId').references('webhookEvents.id') table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) diff --git a/packages/backend/src/accounting/service.ts b/packages/backend/src/accounting/service.ts index 3a4b10a549..5424fdfc59 100644 --- a/packages/backend/src/accounting/service.ts +++ b/packages/backend/src/accounting/service.ts @@ -44,8 +44,8 @@ export interface LiquidityAccount { id: string unit: number } - onCredit?: (balance: bigint) => Promise - onDebit?: (balance: bigint) => Promise + onCredit?: (balance: bigint) => Promise + onDebit?: (balance: bigint) => Promise } export interface Deposit { diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index 0f9d47d322..260db18fa4 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -1635,8 +1635,7 @@ describe('Liquidity Resolvers', (): void => { data: payment.toData({ amountSent: BigInt(0), balance: BigInt(0) - }), - processAt: new Date() + }) }) } ) @@ -1778,7 +1777,6 @@ describe('Liquidity Resolvers', (): void => { id: eventId, type, data, - processAt: new Date(), withdrawal: { accountId: account.id, assetId: account.asset.id, @@ -1873,13 +1871,12 @@ describe('Liquidity Resolvers', (): void => { test('Returns error for non-existent webhook event withdrawal', async (): Promise => { const webhookService = await deps.use('webhookService') - const { type, data, processAt } = (await webhookService.getEvent( + const { type, data } = (await webhookService.getEvent( eventId )) as WebhookEvent const event = await WebhookEvent.query(knex).insertAndFetch({ type, - data, - processAt + data }) const response = await appContainer.apolloClient .mutate({ diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index 570590e8a5..d9295a0ceb 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -6,6 +6,29 @@ import { ConnectorAccount } from '../../connector/core/rafiki' import { BaseModel } from '../../shared/baseModel' import { WebhookEvent } from '../../webhook/model' +export enum InvoiceEventType { + InvoiceExpired = 'invoice.expired', + InvoicePaid = 'invoice.paid' +} + +export type InvoiceData = { + invoice: { + id: string + accountId: string + description?: string + createdAt: string + expiresAt: string + amount: string + received: string + } + payment?: never +} + +export class InvoiceEvent extends WebhookEvent { + public type!: InvoiceEventType + public data!: InvoiceData +} + export class Invoice extends BaseModel implements ConnectorAccount, LiquidityAccount { @@ -21,6 +44,14 @@ export class Invoice from: 'invoices.accountId', to: 'accounts.id' } + }, + event: { + relation: Model.HasOneRelation, + modelClass: InvoiceEvent, + join: { + from: 'invoices.eventId', + to: 'webhookEvents.id' + } } } @@ -31,6 +62,8 @@ export class Invoice public description?: string public expiresAt!: Date public readonly amount!: bigint + public eventId?: string + public event?: InvoiceEvent public processAt!: Date | null @@ -38,22 +71,46 @@ export class Invoice return this.account.asset } - public async onCredit(balance: bigint): Promise { - if (balance >= this.amount) { - return await Invoice.transaction(async (trx) => { + public async onCredit(balance: bigint): Promise { + if (this.active && balance < this.amount) { + return this + } + return await Invoice.transaction(async (trx) => { + this.event = await this.$relatedQuery('event', trx) + // Ensure the event cannot be processed concurrently. + .forUpdate() + .first() + if (!this.event || this.event.attempts) { + this.event = await InvoiceEvent.query(trx).insertAndFetch({ + type: InvoiceEventType.InvoicePaid, + data: this.toData(balance), + // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before creating withdrawal. + processAt: new Date(Date.now() + 30_000), + withdrawal: { + accountId: this.id, + assetId: this.account.assetId, + amount: balance + } + }) await this.$query(trx).patch({ - active: false + active: false, + eventId: this.event.id }) - await InvoiceEvent.query(trx).insertAndFetch({ - type: InvoiceEventType.InvoicePaid, + } else { + // Update the event withdrawal amount if the withdrawal hasn't been created (event.attempts === 0). + await this.event.$query(trx).patchAndFetch({ data: this.toData(balance), - // TODO: - // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. - // But balance is fixed in the webhook event data. - processAt: new Date() + // Add 30 seconds to allow additional prepared packets to finish before creating withdrawal. + processAt: new Date(Date.now() + 30_000), + withdrawal: { + accountId: this.id, + assetId: this.account.assetId, + amount: balance + } }) - }) - } + } + return this + }) } public toData(amountReceived: bigint): InvoiceData { @@ -61,7 +118,6 @@ export class Invoice invoice: { id: this.id, accountId: this.accountId, - active: this.active, amount: this.amount.toString(), description: this.description, expiresAt: this.expiresAt.toISOString(), @@ -71,27 +127,3 @@ export class Invoice } } } - -export enum InvoiceEventType { - InvoiceExpired = 'invoice.expired', - InvoicePaid = 'invoice.paid' -} - -export type InvoiceData = { - invoice: { - id: string - accountId: string - active: boolean - description?: string - createdAt: string - expiresAt: string - amount: string - received: string - } - payment?: never -} - -export class InvoiceEvent extends WebhookEvent { - public type!: InvoiceEventType - public data!: InvoiceData -} diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index 1083594a18..6e89370b17 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -1,3 +1,4 @@ +import assert from 'assert' import Knex from 'knex' import { WorkerUtils, makeWorkerUtils } from 'graphile-worker' import { v4 as uuid } from 'uuid' @@ -127,35 +128,84 @@ describe('Invoice Service', (): void => { ) test('Does not deactivate a partially paid invoice', async (): Promise => { - await invoice.onCredit(invoice.amount - BigInt(1)) + await expect( + invoice.onCredit(invoice.amount - BigInt(1)) + ).resolves.toEqual(invoice) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ active: true }) }) test('Deactivates fully paid invoice', async (): Promise => { - await invoice.onCredit(invoice.amount) + await expect(invoice.onCredit(invoice.amount)).resolves.toMatchObject({ + id: invoice.id, + active: false + }) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ active: false }) }) test('Creates invoice.paid webhook event', async (): Promise => { - await expect( - InvoiceEvent.query(knex).where({ - type: InvoiceEventType.InvoicePaid - }) - ).resolves.toHaveLength(0) - await invoice.onCredit(invoice.amount) - await expect( - InvoiceEvent.query(knex) - .whereJsonSupersetOf('data:invoice', { - id: invoice.id - }) - .where({ - type: InvoiceEventType.InvoicePaid - }) - ).resolves.toHaveLength(1) + jest.useFakeTimers('modern') + const now = Date.now() + jest.setSystemTime(new Date(now)) + expect(invoice.eventId).toBeNull() + await expect(invoice.onCredit(invoice.amount)).resolves.toMatchObject({ + event: { + type: InvoiceEventType.InvoicePaid, + data: invoice.toData(invoice.amount), + processAt: new Date(now + 30_000), + withdrawal: { + accountId: invoice.id, + assetId: invoice.account.assetId, + amount: invoice.amount + } + } + }) + }) + + test('Updates invoice.paid webhook event withdrawal amount', async (): Promise => { + const { eventId } = await invoice.onCredit(invoice.amount) + const amount = invoice.amount + BigInt(2) + jest.useFakeTimers('modern') + const now = Date.now() + jest.setSystemTime(new Date(now)) + await expect(invoice.onCredit(amount)).resolves.toMatchObject({ + event: { + id: eventId, + type: InvoiceEventType.InvoicePaid, + data: invoice.toData(amount), + processAt: new Date(now + 30_000), + withdrawal: { + accountId: invoice.id, + assetId: invoice.account.assetId, + amount + } + } + }) + }) + + test('Creates subsequent invoice.paid webhook event for leftover amount', async (): Promise => { + invoice = await invoice.onCredit(invoice.amount) + assert.ok(invoice.event) + await invoice.event.$query(knex).patch({ attempts: 1 }) + const amount = BigInt(1) + jest.useFakeTimers('modern') + const now = Date.now() + jest.setSystemTime(new Date(now)) + await expect(invoice.onCredit(amount)).resolves.toMatchObject({ + event: { + type: InvoiceEventType.InvoicePaid, + data: invoice.toData(amount), + processAt: new Date(now + 30_000), + withdrawal: { + accountId: invoice.id, + assetId: invoice.account.assetId, + amount + } + } + }) }) }) diff --git a/packages/backend/src/outgoing_payment/lifecycle.ts b/packages/backend/src/outgoing_payment/lifecycle.ts index fffba8916a..16ae77411a 100644 --- a/packages/backend/src/outgoing_payment/lifecycle.ts +++ b/packages/backend/src/outgoing_payment/lifecycle.ts @@ -1,5 +1,4 @@ import * as Pay from '@interledger/pay' -import assert from 'assert' import { LifecycleError } from './errors' import { @@ -10,7 +9,6 @@ import { } from './model' import { ServiceDependencies } from './service' import { IlpPlugin } from './ilp_plugin' -import { RETRY_LIMIT_MS } from '../webhook/service' const MAX_INT64 = BigInt('9223372036854775807') @@ -344,20 +342,16 @@ const sendWebhookEvent = async ( throw LifecycleError.MissingBalance } - const event = await PaymentEvent.query(deps.knex).insertAndFetch({ + const withdrawal = balance + ? { + accountId: payment.id, + assetId: payment.account.assetId, + amount: balance + } + : undefined + await PaymentEvent.query(deps.knex).insertAndFetch({ type, data: payment.toData({ amountSent, balance }), - processAt: new Date() + withdrawal }) - - if (balance) { - assert.ok(type !== PaymentEventType.PaymentFunding) - const error = await deps.accountingService.createWithdrawal({ - id: event.id, - account: payment, - amount: balance, - timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns - }) - if (error) throw new Error(error) - } } diff --git a/packages/backend/src/outgoing_payment/service.test.ts b/packages/backend/src/outgoing_payment/service.test.ts index aafb06e221..1e465fb6af 100644 --- a/packages/backend/src/outgoing_payment/service.test.ts +++ b/packages/backend/src/outgoing_payment/service.test.ts @@ -23,7 +23,6 @@ import { LifecycleError } from './errors' import { RETRY_BACKOFF_SECONDS } from './worker' import { isTransferError } from '../accounting/errors' import { AccountingService, TransferOptions } from '../accounting/service' -import { uuidToBigInt } from '../accounting/utils' import { AssetOptions } from '../asset/service' import { Invoice } from '../open_payments/invoice/model' import { RatesService } from '../rates/service' @@ -67,11 +66,9 @@ describe('OutgoingPaymentService', (): void => { const type = webhookTypes[payment.state] if (type) { await expect( - PaymentEvent.query(knex) - .whereJsonSupersetOf('data:payment', { - id: payment.id - }) - .andWhere({ type }) + PaymentEvent.query(knex).where({ + type + }) ).resolves.not.toHaveLength(0) } return payment @@ -164,14 +161,12 @@ describe('OutgoingPaymentService', (): void => { ).resolves.toEqual(invoiceReceived) } if (withdrawAmount !== undefined) { - const tigerbeetle = await deps.use('tigerbeetle') await expect( - tigerbeetle.lookupAccounts([uuidToBigInt(payment.id)]) - ).resolves.toMatchObject([ - { - debits_reserved: withdrawAmount - } - ]) + PaymentEvent.query(knex).where({ + withdrawalAccountId: payment.id, + withdrawalAmount: withdrawAmount + }) + ).resolves.toHaveLength(1) } } @@ -549,15 +544,6 @@ describe('OutgoingPaymentService', (): void => { }) describe('SENDING→', (): void => { - beforeEach( - async (): Promise => { - // Don't send invoice.paid webhook events - jest - .spyOn(invoice, 'onCredit') - .mockImplementation(() => Promise.resolve()) - } - ) - async function setup( opts: Pick< PaymentIntent, @@ -610,7 +596,7 @@ describe('OutgoingPaymentService', (): void => { if (!payment.quote) throw 'no quote' const amountSent = invoice.amount * BigInt(2) await expectOutcome(payment, { - accountBalance: BigInt(0), + accountBalance: payment.quote.maxSourceAmount - amountSent, amountSent, amountDelivered: invoice.amount, invoiceReceived: invoice.amount, @@ -629,7 +615,7 @@ describe('OutgoingPaymentService', (): void => { if (!payment.quote) throw 'no quote' const amountSent = (invoice.amount - amountAlreadyDelivered) * BigInt(2) await expectOutcome(payment, { - accountBalance: BigInt(0), + accountBalance: payment.quote.maxSourceAmount - amountSent, amountSent, amountDelivered: invoice.amount - amountAlreadyDelivered, invoiceReceived: invoice.amount, @@ -670,7 +656,7 @@ describe('OutgoingPaymentService', (): void => { expect(payment.stateAttempts).toBe(0) // "mockPay" allows a small amount of money to be paid every attempt. await expectOutcome(payment, { - accountBalance: BigInt(0), + accountBalance: BigInt(123 - 10 * 5), amountSent: BigInt(10 * 5), amountDelivered: BigInt(5 * 5), withdrawAmount: BigInt(123 - 10 * 5) @@ -696,7 +682,7 @@ describe('OutgoingPaymentService', (): void => { Pay.PaymentError.ReceiverProtocolViolation ) await expectOutcome(payment, { - accountBalance: BigInt(0), + accountBalance: BigInt(123 - 10), amountSent: BigInt(10), amountDelivered: BigInt(5), withdrawAmount: BigInt(123 - 10) @@ -766,7 +752,7 @@ describe('OutgoingPaymentService', (): void => { const payment = await processNext(paymentId, PaymentState.Completed) if (!payment.quote) throw 'no quote' await expectOutcome(payment, { - accountBalance: BigInt(0), + accountBalance: payment.quote.maxSourceAmount, amountSent: BigInt(0), amountDelivered: BigInt(0), invoiceReceived: invoice.amount, diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index d740363752..5010c7f8af 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -72,8 +72,7 @@ describe('Webhook Service', (): void => { account: { id: uuid() } - }, - processAt: new Date() + } }) } ) @@ -165,7 +164,7 @@ describe('Webhook Service', (): void => { await expect(webhookService.processNext()).resolves.toEqual(event.id) expect(scope.isDone()).toBe(true) await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ - attempts: 0, + attempts: 1, error: null, processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index 805f4d8d0b..10b28c5381 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -132,6 +132,7 @@ async function sendWebhookEvent( }) await event.$query(deps.knex).patch({ + attempts: event.attempts + 1, error: null, processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) }) From 07661cf5c6b1b0df867b84a98c34b11ad153b022 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Wed, 9 Feb 2022 17:13:46 -0600 Subject: [PATCH 08/15] chore(backend): only call onCredit for destinationAccount Remove onDebit. Remove TransferAccount. Update invoices index. Update transaction-api.md docs. --- docs/transaction-api.md | 21 +++--- .../20220209103122_create_invoices_table.js | 2 +- .../backend/src/accounting/service.test.ts | 7 +- packages/backend/src/accounting/service.ts | 64 +++++++------------ packages/backend/src/connector/core/rafiki.ts | 6 +- packages/backend/src/tests/accountFactory.ts | 19 ++++-- 6 files changed, 52 insertions(+), 67 deletions(-) diff --git a/docs/transaction-api.md b/docs/transaction-api.md index 2518702b2f..9cb5db93fd 100644 --- a/docs/transaction-api.md +++ b/docs/transaction-api.md @@ -87,7 +87,7 @@ An expired invoice that has never received money is deleted. Rafiki sends webhook events to notify the wallet of payment lifecycle states that require liquidity to be added or removed. -Webhook event handlers must be idempotent and return `200`. +Webhook event handlers must be idempotent and return `200` on success. Rafiki will retry unsuccessful webhook requests for up to one day. ### `EventType` @@ -185,13 +185,12 @@ The intent must include `invoiceUrl` xor (`paymentPointer` and `amountToSend`). ### `Invoice` -| Name | Optional | Type | Description | -| :------------ | :------- | :-------- | :----------------------------------------------------------------------------------------------------------------------------- | -| `id` | No | `ID` | Unique ID for this invoice, randomly generated by Rafiki. | -| `accountId` | No | `String` | Id of the recipient's Open Payments account. | -| `amount` | No | `UInt64` | The amount that must be paid at the time the invoice is created, in base units of the account asset. | -| `received` | No | `UInt64` | The total amount received, in base units of the account asset. | -| `active` | No | `Boolean` | If `true`, the invoice may receive funds. If `false`, the invoice is either expired or has already received `amount` of funds. | -| `description` | Yes | `String` | Human readable description of the invoice. | -| `createdAt` | No | `String` | | -| `expiresAt` | No | `String` | | +| Name | Optional | Type | Description | +| :------------ | :------- | :------- | :--------------------------------------------------------------------------------------------------- | +| `id` | No | `ID` | Unique ID for this invoice, randomly generated by Rafiki. | +| `accountId` | No | `String` | Id of the recipient's Open Payments account. | +| `amount` | No | `UInt64` | The amount that must be paid at the time the invoice is created, in base units of the account asset. | +| `received` | No | `UInt64` | The total amount received, in base units of the account asset. | +| `description` | Yes | `String` | Human readable description of the invoice. | +| `createdAt` | No | `String` | | +| `expiresAt` | No | `String` | | diff --git a/packages/backend/migrations/20220209103122_create_invoices_table.js b/packages/backend/migrations/20220209103122_create_invoices_table.js index b8a8454d5a..eb2412541e 100644 --- a/packages/backend/migrations/20220209103122_create_invoices_table.js +++ b/packages/backend/migrations/20220209103122_create_invoices_table.js @@ -17,7 +17,7 @@ exports.up = function (knex) { table.index(['accountId', 'createdAt', 'id']) - table.index('expiresAt') + table.index(['active', 'expiresAt']) /* TODO: The latest version of knex supports "partial indexes", which would be more efficient for the deactivateInvoice use case. Unfortunately, the only version of 'objection' that supports this version of knex is still in alpha. // This is a 'partial index' -- expiresAt is only indexed when active=true. diff --git a/packages/backend/src/accounting/service.test.ts b/packages/backend/src/accounting/service.test.ts index 9142374f2e..b4edeaf3bf 100644 --- a/packages/backend/src/accounting/service.test.ts +++ b/packages/backend/src/accounting/service.test.ts @@ -8,7 +8,6 @@ import { v4 as uuid } from 'uuid' import { AccountingService, LiquidityAccount, - TransferAccount, Deposit, Withdrawal } from './service' @@ -25,7 +24,7 @@ import { startTigerbeetleContainer, TIGERBEETLE_PORT } from '../tests/tigerbeetle' -import { AccountFactory } from '../tests/accountFactory' +import { AccountFactory, FactoryAccount } from '../tests/accountFactory' describe('Accounting Service', (): void => { let deps: IocContract @@ -215,8 +214,8 @@ describe('Accounting Service', (): void => { ${true} | ${'same asset'} ${false} | ${'cross-currency'} `('$description', ({ sameAsset }): void => { - let sourceAccount: TransferAccount - let destinationAccount: TransferAccount + let sourceAccount: LiquidityAccount + let destinationAccount: FactoryAccount const startingSourceBalance = BigInt(10) const startingDestinationLiquidity = BigInt(100) diff --git a/packages/backend/src/accounting/service.ts b/packages/backend/src/accounting/service.ts index 5424fdfc59..f887ae3691 100644 --- a/packages/backend/src/accounting/service.ts +++ b/packages/backend/src/accounting/service.ts @@ -45,7 +45,6 @@ export interface LiquidityAccount { unit: number } onCredit?: (balance: bigint) => Promise - onDebit?: (balance: bigint) => Promise } export interface Deposit { @@ -58,15 +57,9 @@ export interface Withdrawal extends Deposit { timeout: bigint } -export interface TransferAccount extends LiquidityAccount { - asset: LiquidityAccount['asset'] & { - asset: LiquidityAccount['asset'] - } -} - export interface TransferOptions { - sourceAccount: TransferAccount - destinationAccount: TransferAccount + sourceAccount: LiquidityAccount + destinationAccount: LiquidityAccount sourceAmount: bigint destinationAmount?: bigint timeout: bigint // nano-seconds @@ -227,34 +220,30 @@ export async function createTransfer( return TransferError.InvalidDestinationAmount } const transfers: Required[] = [] - const sourceAccounts: LiquidityAccount[] = [] - const destinationAccounts: LiquidityAccount[] = [] const addTransfer = ({ - sourceAccount, - destinationAccount, + sourceAccountId, + destinationAccountId, amount }: { - sourceAccount: LiquidityAccount - destinationAccount: LiquidityAccount + sourceAccountId: string + destinationAccountId: string amount: bigint }) => { transfers.push({ id: uuid(), - sourceAccountId: sourceAccount.id, - destinationAccountId: destinationAccount.id, + sourceAccountId, + destinationAccountId, amount, timeout }) - sourceAccounts.push(sourceAccount) - destinationAccounts.push(destinationAccount) } // Same asset if (sourceAccount.asset.unit === destinationAccount.asset.unit) { addTransfer({ - sourceAccount, - destinationAccount, + sourceAccountId: sourceAccount.id, + destinationAccountId: destinationAccount.id, amount: destinationAmount && destinationAmount < sourceAmount ? destinationAmount @@ -265,15 +254,15 @@ export async function createTransfer( // Send excess source amount to liquidity account if (destinationAmount < sourceAmount) { addTransfer({ - sourceAccount, - destinationAccount: sourceAccount.asset, + sourceAccountId: sourceAccount.id, + destinationAccountId: sourceAccount.asset.id, amount: sourceAmount - destinationAmount }) // Deliver excess destination amount from liquidity account } else { addTransfer({ - sourceAccount: destinationAccount.asset, - destinationAccount, + sourceAccountId: destinationAccount.asset.id, + destinationAccountId: destinationAccount.id, amount: destinationAmount - sourceAmount }) } @@ -287,13 +276,13 @@ export async function createTransfer( // Send to source liquidity account // Deliver from destination liquidity account addTransfer({ - sourceAccount, - destinationAccount: sourceAccount.asset, + sourceAccountId: sourceAccount.id, + destinationAccountId: sourceAccount.asset.id, amount: sourceAmount }) addTransfer({ - sourceAccount: destinationAccount.asset, - destinationAccount, + sourceAccountId: destinationAccount.asset.id, + destinationAccountId: destinationAccount.id, amount: destinationAmount }) } @@ -327,19 +316,10 @@ export async function createTransfer( if (error) { return error.error } - for (const account of sourceAccounts) { - if (account.onDebit) { - const balance = await getAccountBalance(deps, account.id) - assert.ok(balance !== undefined) - await account.onDebit(balance) - } - } - for (const account of destinationAccounts) { - if (account.onCredit) { - const balance = await getAccountBalance(deps, account.id) - assert.ok(balance !== undefined) - await account.onCredit(balance) - } + if (destinationAccount.onCredit) { + const balance = await getAccountBalance(deps, destinationAccount.id) + assert.ok(balance !== undefined) + await destinationAccount.onCredit(balance) } }, rollback: async (): Promise => { diff --git a/packages/backend/src/connector/core/rafiki.ts b/packages/backend/src/connector/core/rafiki.ts index 78ddcb33d1..5ff08b9dc2 100644 --- a/packages/backend/src/connector/core/rafiki.ts +++ b/packages/backend/src/connector/core/rafiki.ts @@ -14,7 +14,7 @@ import { import { createTokenAuthMiddleware } from './middleware' import { RatesService } from '../../rates/service' import { TransferError } from '../../accounting/errors' -import { TransferAccount, Transaction } from '../../accounting/service' +import { LiquidityAccount, Transaction } from '../../accounting/service' import { AssetOptions } from '../../asset/service' import { AccountService } from '../../open_payments/account/service' import { InvoiceService } from '../../open_payments/invoice/service' @@ -27,8 +27,8 @@ import { PeerService } from '../../peer/service' // ../../open_payments/invoice/model // ../../outgoing_payment/model // ../../peer/model -export interface ConnectorAccount extends TransferAccount { - asset: TransferAccount['asset'] & AssetOptions +export interface ConnectorAccount extends LiquidityAccount { + asset: LiquidityAccount['asset'] & AssetOptions } export interface IncomingAccount extends ConnectorAccount { diff --git a/packages/backend/src/tests/accountFactory.ts b/packages/backend/src/tests/accountFactory.ts index 237217438a..0cec0c7f61 100644 --- a/packages/backend/src/tests/accountFactory.ts +++ b/packages/backend/src/tests/accountFactory.ts @@ -1,23 +1,30 @@ import { v4 as uuid } from 'uuid' -import { - AccountingService, - LiquidityAccount, - TransferAccount -} from '../accounting/service' +import { AccountingService, LiquidityAccount } from '../accounting/service' import { randomUnit } from './asset' type BuildOptions = Partial & { balance?: bigint } +export type FactoryAccount = Omit & { + asset: { + id: string + unit: number + asset: { + id: string + unit: number + } + } +} + export class AccountFactory { public constructor( private accounts: AccountingService, private unitGenerator: () => number = randomUnit ) {} - public async build(options: BuildOptions = {}): Promise { + public async build(options: BuildOptions = {}): Promise { const assetId = options.asset?.id || uuid() const unit = options.asset?.unit || this.unitGenerator() const asset = { From 5f1fbbd27651deca1caed102330df95cc6b4e457 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Thu, 10 Feb 2022 13:16:28 -0600 Subject: [PATCH 09/15] chore(backend): require 200 webhook response Replace error with statusCode in webhookEvents table. --- ...20131110501_create_webhook_events_table.js | 1 + packages/backend/src/webhook/model.ts | 2 +- packages/backend/src/webhook/service.test.ts | 39 ++++++++++--------- packages/backend/src/webhook/service.ts | 7 ++-- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/packages/backend/migrations/20220131110501_create_webhook_events_table.js b/packages/backend/migrations/20220131110501_create_webhook_events_table.js index 7fda4c60fe..9ee49d6d47 100644 --- a/packages/backend/migrations/20220131110501_create_webhook_events_table.js +++ b/packages/backend/migrations/20220131110501_create_webhook_events_table.js @@ -5,6 +5,7 @@ exports.up = function (knex) { table.string('type').notNullable() table.json('data').notNullable() table.integer('attempts').notNullable().defaultTo(0) + table.integer('statusCode').nullable() table.string('error').nullable() table.uuid('withdrawalAccountId').nullable() diff --git a/packages/backend/src/webhook/model.ts b/packages/backend/src/webhook/model.ts index 2e8b2fff9a..ada852ef21 100644 --- a/packages/backend/src/webhook/model.ts +++ b/packages/backend/src/webhook/model.ts @@ -13,7 +13,7 @@ export class WebhookEvent extends BaseModel { public type!: string public data!: Record public attempts!: number - public error?: string | null + public statusCode?: number public processAt!: Date public withdrawal?: { diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index 5010c7f8af..dc5cb3a2a4 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -170,21 +170,23 @@ describe('Webhook Service', (): void => { }) }) - test('Schedules retry if request fails', async (): Promise => { - const status = 504 - const scope = mockWebhookServer(status) - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(scope.isDone()).toBe(true) - const updatedEvent = await webhookService.getEvent(event.id) - assert.ok(updatedEvent) - expect(updatedEvent).toMatchObject({ - attempts: 1, - error: `Request failed with status code ${status}` - }) - expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( - event.createdAt.getTime() + RETRY_BACKOFF_MS - ) - }) + test.each([[201], [400], [504]])( + 'Schedules retry if request fails (%i)', + async (status): Promise => { + const scope = mockWebhookServer(status) + await expect(webhookService.processNext()).resolves.toEqual(event.id) + expect(scope.isDone()).toBe(true) + const updatedEvent = await webhookService.getEvent(event.id) + assert.ok(updatedEvent) + expect(updatedEvent).toMatchObject({ + attempts: 1, + statusCode: status + }) + expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( + event.createdAt.getTime() + RETRY_BACKOFF_MS + ) + } + ) test('Schedules retry if request times out', async (): Promise => { const scope = nock(webhookUrl.origin) @@ -197,7 +199,7 @@ describe('Webhook Service', (): void => { assert.ok(updatedEvent) expect(updatedEvent).toMatchObject({ attempts: 1, - error: 'timeout of 2000ms exceeded' + statusCode: null }) expect(updatedEvent.processAt.getTime()).toBeGreaterThanOrEqual( event.createdAt.getTime() + RETRY_BACKOFF_MS @@ -210,15 +212,14 @@ describe('Webhook Service', (): void => { new Date(event.createdAt.getTime() + RETRY_LIMIT_MS - 1) ) - const error = 'last try' const mockPost = jest .spyOn(axios, 'post') - .mockRejectedValueOnce(new Error(error)) + .mockRejectedValueOnce(new Error('last try')) await expect(webhookService.processNext()).resolves.toEqual(event.id) expect(mockPost).toHaveBeenCalledTimes(1) await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ attempts: 1, - error, + statusCode: null, processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) }) }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index 10b28c5381..d47c171809 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -128,12 +128,13 @@ async function sendWebhookEvent( await axios.post(deps.config.webhookUrl, body, { timeout: deps.config.webhookTimeout, - headers: requestHeaders + headers: requestHeaders, + validateStatus: (status) => status === 200 }) await event.$query(deps.knex).patch({ attempts: event.attempts + 1, - error: null, + statusCode: 200, processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) }) } catch (err) { @@ -155,7 +156,7 @@ async function sendWebhookEvent( await event.$query(deps.knex).patch({ attempts, - error, + statusCode: err.isAxiosError && err.response?.status, processAt }) } From 42d82eaa6cb1c64defbb7ac82f5a8c0a08a869fe Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Thu, 10 Feb 2022 16:48:58 -0600 Subject: [PATCH 10/15] fix(backend): don't update event amount after processAt --- .../src/open_payments/invoice/model.ts | 6 ++- .../src/open_payments/invoice/service.test.ts | 50 +++++++++++-------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index d9295a0ceb..60a155aac9 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -80,7 +80,11 @@ export class Invoice // Ensure the event cannot be processed concurrently. .forUpdate() .first() - if (!this.event || this.event.attempts) { + if ( + !this.event || + this.event.attempts || + this.event.processAt.getTime() <= Date.now() + ) { this.event = await InvoiceEvent.query(trx).insertAndFetch({ type: InvoiceEventType.InvoicePaid, data: this.toData(balance), diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index 6e89370b17..0bb4859019 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -186,27 +186,37 @@ describe('Invoice Service', (): void => { }) }) - test('Creates subsequent invoice.paid webhook event for leftover amount', async (): Promise => { - invoice = await invoice.onCredit(invoice.amount) - assert.ok(invoice.event) - await invoice.event.$query(knex).patch({ attempts: 1 }) - const amount = BigInt(1) - jest.useFakeTimers('modern') - const now = Date.now() - jest.setSystemTime(new Date(now)) - await expect(invoice.onCredit(amount)).resolves.toMatchObject({ - event: { - type: InvoiceEventType.InvoicePaid, - data: invoice.toData(amount), - processAt: new Date(now + 30_000), - withdrawal: { - accountId: invoice.id, - assetId: invoice.account.assetId, - amount + test.each` + attempts | processAt + ${1} | ${undefined} + ${0} | ${new Date()} + `( + 'Creates subsequent invoice.paid webhook event for leftover amount', + async ({ attempts, processAt }): Promise => { + invoice = await invoice.onCredit(invoice.amount) + assert.ok(invoice.event) + await invoice.event.$query(knex).patch({ + attempts, + processAt + }) + const amount = BigInt(1) + jest.useFakeTimers('modern') + const now = Date.now() + jest.setSystemTime(new Date(now)) + await expect(invoice.onCredit(amount)).resolves.toMatchObject({ + event: { + type: InvoiceEventType.InvoicePaid, + data: invoice.toData(amount), + processAt: new Date(now + 30_000), + withdrawal: { + accountId: invoice.id, + assetId: invoice.account.assetId, + amount + } } - } - }) - }) + }) + } + ) }) describe('processNext', (): void => { From c13231a330ccb57c7966eac1ac52d95f742d2caf Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Tue, 15 Feb 2022 09:47:18 -0600 Subject: [PATCH 11/15] chore(backend): do single-phase withdrawal in withdrawEventLiquidity --- ...20131110501_create_webhook_events_table.js | 1 - .../backend/src/accounting/service.test.ts | 14 ++- packages/backend/src/accounting/service.ts | 2 +- .../src/graphql/resolvers/liquidity.test.ts | 109 +++--------------- .../src/graphql/resolvers/liquidity.ts | 13 ++- packages/backend/src/index.ts | 3 +- packages/backend/src/webhook/service.test.ts | 24 +--- packages/backend/src/webhook/service.ts | 26 ----- 8 files changed, 43 insertions(+), 149 deletions(-) diff --git a/packages/backend/migrations/20220131110501_create_webhook_events_table.js b/packages/backend/migrations/20220131110501_create_webhook_events_table.js index 9ee49d6d47..8da10ab13c 100644 --- a/packages/backend/migrations/20220131110501_create_webhook_events_table.js +++ b/packages/backend/migrations/20220131110501_create_webhook_events_table.js @@ -6,7 +6,6 @@ exports.up = function (knex) { table.json('data').notNullable() table.integer('attempts').notNullable().defaultTo(0) table.integer('statusCode').nullable() - table.string('error').nullable() table.uuid('withdrawalAccountId').nullable() table.uuid('withdrawalAssetId').nullable() diff --git a/packages/backend/src/accounting/service.test.ts b/packages/backend/src/accounting/service.test.ts index b4edeaf3bf..2200520d90 100644 --- a/packages/backend/src/accounting/service.test.ts +++ b/packages/backend/src/accounting/service.test.ts @@ -530,7 +530,15 @@ describe('Accounting Service', (): void => { } ) - describe('Create', (): void => { + describe.each` + timeout | description + ${undefined} | ${'single-phase'} + ${timeout} | ${'two-phase'} + `('Create ($description)', ({ timeout }): void => { + beforeEach((): void => { + withdrawal.timeout = timeout + }) + test('A withdrawal can be created', async (): Promise => { await expect( accountingService.createWithdrawal(withdrawal) @@ -540,7 +548,9 @@ describe('Accounting Service', (): void => { ).resolves.toEqual(startingBalance - withdrawal.amount) await expect( accountingService.getSettlementBalance(withdrawal.account.asset.unit) - ).resolves.toEqual(startingBalance) + ).resolves.toEqual( + timeout ? startingBalance : startingBalance - withdrawal.amount + ) }) test('Cannot create withdrawal with invalid id', async (): Promise => { diff --git a/packages/backend/src/accounting/service.ts b/packages/backend/src/accounting/service.ts index f887ae3691..60290f25ea 100644 --- a/packages/backend/src/accounting/service.ts +++ b/packages/backend/src/accounting/service.ts @@ -54,7 +54,7 @@ export interface Deposit { } export interface Withdrawal extends Deposit { - timeout: bigint + timeout?: bigint } export interface TransferOptions { diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index 260db18fa4..2382f381ca 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -9,7 +9,11 @@ import { IocContract } from '@adonisjs/fold' import { AppServices } from '../../app' import { initIocContainer } from '../..' import { Config } from '../../config/app' -import { AccountingService, LiquidityAccount } from '../../accounting/service' +import { + AccountingService, + LiquidityAccount, + Withdrawal +} from '../../accounting/service' import { Asset } from '../../asset/model' import { AssetService } from '../../asset/service' import { Account } from '../../open_payments/account/model' @@ -1756,6 +1760,7 @@ describe('Liquidity Resolvers', (): void => { '%s', (type): void => { let eventId: string + let withdrawal: Withdrawal beforeEach( async (): Promise => { @@ -1790,17 +1795,14 @@ describe('Liquidity Resolvers', (): void => { amount }) ).resolves.toBeUndefined() - await expect( - accountingService.createWithdrawal({ - id: eventId, - account, - amount, - timeout - }) - ).resolves.toBeUndefined() await expect( accountingService.getBalance(account.id) - ).resolves.toEqual(BigInt(0)) + ).resolves.toEqual(amount) + withdrawal = { + id: eventId, + account, + amount + } } ) @@ -1869,86 +1871,9 @@ describe('Liquidity Resolvers', (): void => { expect(response.error).toEqual(LiquidityError.InvalidId) }) - test('Returns error for non-existent webhook event withdrawal', async (): Promise => { - const webhookService = await deps.use('webhookService') - const { type, data } = (await webhookService.getEvent( - eventId - )) as WebhookEvent - const event = await WebhookEvent.query(knex).insertAndFetch({ - type, - data - }) - const response = await appContainer.apolloClient - .mutate({ - mutation: gql` - mutation WithdrawLiquidity($eventId: String!) { - withdrawEventLiquidity(eventId: $eventId) { - code - success - message - error - } - } - `, - variables: { - eventId: event.id - } - }) - .then( - (query): LiquidityMutationResponse => { - if (query.data) { - return query.data.withdrawEventLiquidity - } else { - throw new Error('Data was empty') - } - } - ) - - expect(response.success).toBe(false) - expect(response.code).toEqual('400') - expect(response.message).toEqual('Invalid id') - expect(response.error).toEqual(LiquidityError.InvalidId) - }) - - test('Returns error for finalized withdrawal', async (): Promise => { - await expect( - accountingService.commitWithdrawal(eventId) - ).resolves.toBeUndefined() - const response = await appContainer.apolloClient - .mutate({ - mutation: gql` - mutation WithdrawLiquidity($eventId: String!) { - withdrawEventLiquidity(eventId: $eventId) { - code - success - message - error - } - } - `, - variables: { - eventId - } - }) - .then( - (query): LiquidityMutationResponse => { - if (query.data) { - return query.data.withdrawEventLiquidity - } else { - throw new Error('Data was empty') - } - } - ) - - expect(response.success).toBe(false) - expect(response.code).toEqual('409') - expect(response.message).toEqual('Withdrawal already finalized') - expect(response.error).toEqual(LiquidityError.AlreadyCommitted) - }) - - test('Returns error for rolled back withdrawal', async (): Promise => { + test('Returns error for already completed withdrawal', async (): Promise => { await expect( - accountingService.rollbackWithdrawal(eventId) + accountingService.createWithdrawal(withdrawal) ).resolves.toBeUndefined() const response = await appContainer.apolloClient .mutate({ @@ -1977,9 +1902,9 @@ describe('Liquidity Resolvers', (): void => { ) expect(response.success).toBe(false) - expect(response.code).toEqual('409') - expect(response.message).toEqual('Withdrawal already rolled back') - expect(response.error).toEqual(LiquidityError.AlreadyRolledBack) + expect(response.code).toEqual('403') + expect(response.message).toEqual('Insufficient balance') + expect(response.error).toEqual(LiquidityError.InsufficientBalance) }) } ) diff --git a/packages/backend/src/graphql/resolvers/liquidity.ts b/packages/backend/src/graphql/resolvers/liquidity.ts index a773e9fd0c..ec0f888845 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.ts @@ -341,13 +341,22 @@ export const withdrawEventLiquidity: MutationResolvers['withdrawE ): ResolversTypes['LiquidityMutationResponse'] => { try { const webhookService = await ctx.container.use('webhookService') - // TODO: remove event lookup when commitWithdrawal can verify transfer code const event = await webhookService.getEvent(args.eventId) if (!event || !event.withdrawal) { return responses[LiquidityError.InvalidId] } + const assetService = await ctx.container.use('assetService') + const asset = await assetService.getById(event.withdrawal.assetId) + assert.ok(asset) const accountingService = await ctx.container.use('accountingService') - const error = await accountingService.commitWithdrawal(args.eventId) + const error = await accountingService.createWithdrawal({ + id: event.id, + account: { + id: event.withdrawal.accountId, + asset + }, + amount: event.withdrawal.amount + }) if (error) { return errorToResponse(error) } diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 482c1054ae..7b3813b4ff 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -178,8 +178,7 @@ export function initIocContainer( return createWebhookService({ config: await deps.use('config'), knex: await deps.use('knex'), - logger: await deps.use('logger'), - accountingService: await deps.use('accountingService') + logger: await deps.use('logger') }) }) container.singleton('invoiceService', async (deps) => { diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index dc5cb3a2a4..0409aa318e 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -137,35 +137,13 @@ describe('Webhook Service', (): void => { await expect(webhookService.processNext()).resolves.toBeUndefined() }) - test('Creates webhook event withdrawal', async (): Promise => { - await makeWithdrawalEvent(event) - assert.ok(event.withdrawal) - const asset = await WebhookEvent.relatedQuery('withdrawalAsset', knex) - .for(event.id) - .first() - assert.ok(asset) - const withdrawalSpy = jest.spyOn(accountingService, 'createWithdrawal') - const scope = mockWebhookServer() - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(scope.isDone()).toBe(true) - expect(withdrawalSpy).toHaveBeenCalledWith({ - id: event.id, - account: { - id: event.withdrawal.accountId, - asset - }, - amount: event.withdrawal.amount, - timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) - }) - }) - test('Sends webhook event', async (): Promise => { const scope = mockWebhookServer() await expect(webhookService.processNext()).resolves.toEqual(event.id) expect(scope.isDone()).toBe(true) await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ attempts: 1, - error: null, + statusCode: 200, processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) }) }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index d47c171809..c555ca032c 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -3,9 +3,6 @@ import axios from 'axios' import { createHmac } from 'crypto' import { WebhookEvent } from './model' -import { TransferError } from '../accounting/errors' -import { AccountingService } from '../accounting/service' -import { Asset } from '../asset/model' import { IAppConfig } from '../config/app' import { BaseService } from '../shared/baseService' @@ -21,7 +18,6 @@ export interface WebhookService { interface ServiceDependencies extends BaseService { config: IAppConfig - accountingService: AccountingService } export async function createWebhookService( @@ -86,28 +82,6 @@ async function sendWebhookEvent( event: WebhookEvent ): Promise { try { - if (event.withdrawal) { - const asset = (await WebhookEvent.relatedQuery( - 'withdrawalAsset', - deps.knex - ) - .for(event.id) - .first()) as Asset - assert.ok(asset) - const error = await deps.accountingService.createWithdrawal({ - id: event.id, - account: { - id: event.withdrawal.accountId, - asset - }, - amount: event.withdrawal.amount, - timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns - }) - if (error && error !== TransferError.TransferExists) { - throw new Error(error) - } - } - const requestHeaders = { 'Content-Type': 'application/json' } From 9ac5b150c1aebec29b9c9c6a7868dd407291f1da Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Tue, 15 Feb 2022 12:59:35 -0600 Subject: [PATCH 12/15] chore(backend): create one webhook event per invoice --- .../20211022210203_create_invoices_table.js | 26 +++ .../20220209103122_create_invoices_table.js | 37 ---- .../src/open_payments/invoice/model.ts | 61 ++---- .../src/open_payments/invoice/service.test.ts | 191 ++++++++---------- .../src/open_payments/invoice/service.ts | 75 +++++-- 5 files changed, 175 insertions(+), 215 deletions(-) create mode 100644 packages/backend/migrations/20211022210203_create_invoices_table.js delete mode 100644 packages/backend/migrations/20220209103122_create_invoices_table.js diff --git a/packages/backend/migrations/20211022210203_create_invoices_table.js b/packages/backend/migrations/20211022210203_create_invoices_table.js new file mode 100644 index 0000000000..013683cffc --- /dev/null +++ b/packages/backend/migrations/20211022210203_create_invoices_table.js @@ -0,0 +1,26 @@ +exports.up = function (knex) { + return knex.schema.createTable('invoices', function (table) { + table.uuid('id').notNullable().primary() + + // Open payments account id + table.uuid('accountId').notNullable() + table.foreign('accountId').references('accounts.id') + table.boolean('active').notNullable() + table.string('description').nullable() + table.timestamp('expiresAt').notNullable() + table.bigInteger('amount').notNullable() + + table.timestamp('processAt').nullable() + + table.timestamp('createdAt').defaultTo(knex.fn.now()) + table.timestamp('updatedAt').defaultTo(knex.fn.now()) + + table.index(['accountId', 'createdAt', 'id']) + + table.index('processAt') + }) +} + +exports.down = function (knex) { + return knex.schema.dropTableIfExists('invoices') +} diff --git a/packages/backend/migrations/20220209103122_create_invoices_table.js b/packages/backend/migrations/20220209103122_create_invoices_table.js deleted file mode 100644 index eb2412541e..0000000000 --- a/packages/backend/migrations/20220209103122_create_invoices_table.js +++ /dev/null @@ -1,37 +0,0 @@ -exports.up = function (knex) { - return knex.schema.createTable('invoices', function (table) { - table.uuid('id').notNullable().primary() - - // Open payments account id - table.uuid('accountId').notNullable() - table.foreign('accountId').references('accounts.id') - table.boolean('active').notNullable() - table.string('description').nullable() - table.timestamp('expiresAt').notNullable() - table.bigInteger('amount').notNullable() - table.uuid('eventId').nullable() - table.foreign('eventId').references('webhookEvents.id') - - table.timestamp('createdAt').defaultTo(knex.fn.now()) - table.timestamp('updatedAt').defaultTo(knex.fn.now()) - - table.index(['accountId', 'createdAt', 'id']) - - table.index(['active', 'expiresAt']) - /* - TODO: The latest version of knex supports "partial indexes", which would be more efficient for the deactivateInvoice use case. Unfortunately, the only version of 'objection' that supports this version of knex is still in alpha. - // This is a 'partial index' -- expiresAt is only indexed when active=true. - table.index('expiresAt', 'idx_active_expiresAt', { - // Knex partial indices are a very new feature in Knex and appear to be buggy. - // - // Use a 'raw' condition because "knex.where('active',true)" fails with: - // > migration failed with error: create index "idx_active_expiresAt" on "invoices" ("expiresAt") where "active" = $1 - there is no parameter $1 - predicate: knex.whereRaw('active = TRUE') - }) - */ - }) -} - -exports.down = function (knex) { - return knex.schema.dropTableIfExists('invoices') -} diff --git a/packages/backend/src/open_payments/invoice/model.ts b/packages/backend/src/open_payments/invoice/model.ts index 60a155aac9..5225bdc7d2 100644 --- a/packages/backend/src/open_payments/invoice/model.ts +++ b/packages/backend/src/open_payments/invoice/model.ts @@ -44,14 +44,6 @@ export class Invoice from: 'invoices.accountId', to: 'accounts.id' } - }, - event: { - relation: Model.HasOneRelation, - modelClass: InvoiceEvent, - join: { - from: 'invoices.eventId', - to: 'webhookEvents.id' - } } } @@ -62,8 +54,6 @@ export class Invoice public description?: string public expiresAt!: Date public readonly amount!: bigint - public eventId?: string - public event?: InvoiceEvent public processAt!: Date | null @@ -72,49 +62,22 @@ export class Invoice } public async onCredit(balance: bigint): Promise { - if (this.active && balance < this.amount) { - return this - } - return await Invoice.transaction(async (trx) => { - this.event = await this.$relatedQuery('event', trx) - // Ensure the event cannot be processed concurrently. - .forUpdate() - .first() - if ( - !this.event || - this.event.attempts || - this.event.processAt.getTime() <= Date.now() - ) { - this.event = await InvoiceEvent.query(trx).insertAndFetch({ - type: InvoiceEventType.InvoicePaid, - data: this.toData(balance), - // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before creating withdrawal. - processAt: new Date(Date.now() + 30_000), - withdrawal: { - accountId: this.id, - assetId: this.account.assetId, - amount: balance - } - }) - await this.$query(trx).patch({ + if (this.amount <= balance) { + const invoice = await this.$query() + .patchAndFetch({ active: false, - eventId: this.event.id + // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before sending webhook event. + processAt: new Date(Date.now() + 30_000) }) - } else { - // Update the event withdrawal amount if the withdrawal hasn't been created (event.attempts === 0). - await this.event.$query(trx).patchAndFetch({ - data: this.toData(balance), - // Add 30 seconds to allow additional prepared packets to finish before creating withdrawal. - processAt: new Date(Date.now() + 30_000), - withdrawal: { - accountId: this.id, - assetId: this.account.assetId, - amount: balance - } + .where({ + id: this.id, + active: true }) + if (invoice) { + return invoice } - return this - }) + } + return this } public toData(amountReceived: bigint): InvoiceData { diff --git a/packages/backend/src/open_payments/invoice/service.test.ts b/packages/backend/src/open_payments/invoice/service.test.ts index 0bb4859019..b57b3dffc4 100644 --- a/packages/backend/src/open_payments/invoice/service.test.ts +++ b/packages/backend/src/open_payments/invoice/service.test.ts @@ -78,7 +78,8 @@ describe('Invoice Service', (): void => { const accountService = await deps.use('accountService') expect(invoice).toMatchObject({ id: invoice.id, - account: await accountService.get(accountId) + account: await accountService.get(accountId), + processAt: new Date(invoice.expiresAt.getTime()) }) const retrievedInvoice = await invoiceService.get(invoice.id) if (!retrievedInvoice) throw new Error('invoice not found') @@ -132,91 +133,25 @@ describe('Invoice Service', (): void => { invoice.onCredit(invoice.amount - BigInt(1)) ).resolves.toEqual(invoice) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - active: true + active: true, + processAt: new Date(invoice.expiresAt.getTime()) }) }) test('Deactivates fully paid invoice', async (): Promise => { + const now = new Date() + jest.useFakeTimers('modern') + jest.setSystemTime(now) await expect(invoice.onCredit(invoice.amount)).resolves.toMatchObject({ id: invoice.id, - active: false + active: false, + processAt: new Date(now.getTime() + 30_000) }) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - active: false - }) - }) - - test('Creates invoice.paid webhook event', async (): Promise => { - jest.useFakeTimers('modern') - const now = Date.now() - jest.setSystemTime(new Date(now)) - expect(invoice.eventId).toBeNull() - await expect(invoice.onCredit(invoice.amount)).resolves.toMatchObject({ - event: { - type: InvoiceEventType.InvoicePaid, - data: invoice.toData(invoice.amount), - processAt: new Date(now + 30_000), - withdrawal: { - accountId: invoice.id, - assetId: invoice.account.assetId, - amount: invoice.amount - } - } - }) - }) - - test('Updates invoice.paid webhook event withdrawal amount', async (): Promise => { - const { eventId } = await invoice.onCredit(invoice.amount) - const amount = invoice.amount + BigInt(2) - jest.useFakeTimers('modern') - const now = Date.now() - jest.setSystemTime(new Date(now)) - await expect(invoice.onCredit(amount)).resolves.toMatchObject({ - event: { - id: eventId, - type: InvoiceEventType.InvoicePaid, - data: invoice.toData(amount), - processAt: new Date(now + 30_000), - withdrawal: { - accountId: invoice.id, - assetId: invoice.account.assetId, - amount - } - } + active: false, + processAt: new Date(now.getTime() + 30_000) }) }) - - test.each` - attempts | processAt - ${1} | ${undefined} - ${0} | ${new Date()} - `( - 'Creates subsequent invoice.paid webhook event for leftover amount', - async ({ attempts, processAt }): Promise => { - invoice = await invoice.onCredit(invoice.amount) - assert.ok(invoice.event) - await invoice.event.$query(knex).patch({ - attempts, - processAt - }) - const amount = BigInt(1) - jest.useFakeTimers('modern') - const now = Date.now() - jest.setSystemTime(new Date(now)) - await expect(invoice.onCredit(amount)).resolves.toMatchObject({ - event: { - type: InvoiceEventType.InvoicePaid, - data: invoice.toData(amount), - processAt: new Date(now + 30_000), - withdrawal: { - accountId: invoice.id, - assetId: invoice.account.assetId, - amount - } - } - }) - } - ) }) describe('processNext', (): void => { @@ -233,19 +168,8 @@ describe('Invoice Service', (): void => { }) }) - test('Does not process inactive, expired invoice', async (): Promise => { - const invoice = await invoiceService.create({ - accountId, - amount: BigInt(123), - description: 'Test invoice', - expiresAt: new Date(Date.now() - 40_000) - }) - await invoice.$query(knex).patch({ active: false }) - await expect(invoiceService.processNext()).resolves.toBeUndefined() - }) - describe('handleExpired', (): void => { - test('Deactivates an expired invoice with received money, creates withdrawal & webhook event', async (): Promise => { + test('Deactivates an expired invoice with received money', async (): Promise => { const invoice = await invoiceService.create({ accountId, amount: BigInt(123), @@ -259,29 +183,15 @@ describe('Invoice Service', (): void => { amount: BigInt(1) }) ).resolves.toBeUndefined() - await expect( - InvoiceEvent.query(knex).where({ - type: InvoiceEventType.InvoiceExpired - }) - ).resolves.toHaveLength(0) + const now = new Date() + jest.useFakeTimers('modern') + jest.setSystemTime(now) await expect(invoiceService.processNext()).resolves.toBe(invoice.id) await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ - active: false + active: false, + processAt: new Date(now.getTime() + 30_000) }) - - await expect( - InvoiceEvent.query(knex) - .whereJsonSupersetOf('data:invoice', { - id: invoice.id - }) - .where({ - type: InvoiceEventType.InvoiceExpired - }) - ).resolves.toHaveLength(1) - await expect(accountingService.getBalance(invoice.id)).resolves.toEqual( - BigInt(0) - ) }) test('Deletes an expired invoice (and account) with no money', async (): Promise => { @@ -295,6 +205,73 @@ describe('Invoice Service', (): void => { expect(await invoiceService.get(invoice.id)).toBeUndefined() }) }) + + describe.each` + eventType | expiresAt | amountReceived + ${InvoiceEventType.InvoiceExpired} | ${-40_000} | ${BigInt(1)} + ${InvoiceEventType.InvoicePaid} | ${30_000} | ${BigInt(123)} + `( + 'handleDeactivated ($eventType)', + ({ eventType, expiresAt, amountReceived }): void => { + let invoice: Invoice + + beforeEach( + async (): Promise => { + invoice = await invoiceService.create({ + accountId, + amount: BigInt(123), + expiresAt: new Date(Date.now() + expiresAt), + description: 'Test invoice' + }) + await expect( + accountingService.createDeposit({ + id: uuid(), + account: invoice, + amount: amountReceived + }) + ).resolves.toBeUndefined() + if (eventType === InvoiceEventType.InvoiceExpired) { + await expect(invoiceService.processNext()).resolves.toBe( + invoice.id + ) + } else { + await invoice.onCredit(invoice.amount) + } + invoice = (await invoiceService.get(invoice.id)) as Invoice + expect(invoice.active).toBe(false) + expect(invoice.processAt).not.toBeNull() + await expect( + accountingService.getTotalReceived(invoice.id) + ).resolves.toEqual(amountReceived) + await expect( + accountingService.getBalance(invoice.id) + ).resolves.toEqual(amountReceived) + } + ) + + test('Creates webhook event', async (): Promise => { + await expect( + InvoiceEvent.query(knex).where({ + type: eventType + }) + ).resolves.toHaveLength(0) + assert.ok(invoice.processAt) + jest.useFakeTimers('modern') + jest.setSystemTime(invoice.processAt) + await expect(invoiceService.processNext()).resolves.toBe(invoice.id) + await expect( + InvoiceEvent.query(knex).where({ + type: eventType, + withdrawalAccountId: invoice.id, + withdrawalAmount: amountReceived + }) + ).resolves.toHaveLength(1) + await expect(invoiceService.get(invoice.id)).resolves.toMatchObject({ + processAt: null + }) + }) + } + ) }) describe('Invoice pagination', (): void => { diff --git a/packages/backend/src/open_payments/invoice/service.ts b/packages/backend/src/open_payments/invoice/service.ts index 309139cea7..2b2c82f186 100644 --- a/packages/backend/src/open_payments/invoice/service.ts +++ b/packages/backend/src/open_payments/invoice/service.ts @@ -2,7 +2,6 @@ import { Invoice, InvoiceEvent, InvoiceEventType } from './model' import { AccountingService } from '../../accounting/service' import { BaseService } from '../../shared/baseService' import { Pagination } from '../../shared/pagination' -import { RETRY_LIMIT_MS } from '../../webhook/service' import assert from 'assert' import { Transaction } from 'knex' import { ForeignKeyViolationError, TransactionOrKnex } from 'objection' @@ -75,7 +74,8 @@ async function createInvoice( description, expiresAt, amount, - active: true + active: true, + processAt: new Date(expiresAt.getTime()) }) .withGraphFetched('account.asset') @@ -104,16 +104,14 @@ async function processNextInvoice( deps_: ServiceDependencies ): Promise { return deps_.knex.transaction(async (trx) => { - // 30 seconds backwards to allow a prepared (but not yet fulfilled/rejected) packet to finish before being deactivated. - const now = new Date(Date.now() - 30_000).toISOString() + const now = new Date(Date.now()).toISOString() const invoices = await Invoice.query(trx) .limit(1) // Ensure the invoices cannot be processed concurrently by multiple workers. .forUpdate() // If an invoice is locked, don't wait — just come back for it later. .skipLocked() - .where('active', true) - .andWhere('expiresAt', '<', now) + .where('processAt', '<=', now) .withGraphFetched('account.asset') const invoice = invoices[0] @@ -126,8 +124,11 @@ async function processNextInvoice( invoice: invoice.id }) } - await handleExpired(deps, invoice) - + if (!invoice.active) { + await handleDeactivated(deps, invoice) + } else { + await handleExpired(deps, invoice) + } return invoice.id }) } @@ -144,26 +145,56 @@ async function handleExpired( if (amountReceived) { deps.logger.trace({ amountReceived }, 'deactivating expired invoice') await invoice.$query(deps.knex).patch({ - active: false + active: false, + // Add 30 seconds to allow a prepared (but not yet fulfilled/rejected) packet to finish before sending webhook event. + processAt: new Date(Date.now() + 30_000) }) + } else { + deps.logger.debug({ amountReceived }, 'deleting expired invoice') + await invoice.$query(deps.knex).delete() + } +} - deps.logger.trace({ amountReceived }, 'withdrawing expired invoice balance') +// Create webhook event to withdraw deactivated invoices' liquidity. +async function handleDeactivated( + deps: ServiceDependencies, + invoice: Invoice +): Promise { + assert.ok(invoice.processAt) + try { + const amountReceived = await deps.accountingService.getTotalReceived( + invoice.id + ) + if (!amountReceived) { + deps.logger.warn( + { amountReceived }, + 'deactivated invoice and empty balance' + ) + await invoice.$query(deps.knex).patch({ processAt: null }) + return + } - const event = await InvoiceEvent.query(deps.knex).insertAndFetch({ - type: InvoiceEventType.InvoiceExpired, + const type = + amountReceived < invoice.amount + ? InvoiceEventType.InvoiceExpired + : InvoiceEventType.InvoicePaid + deps.logger.trace({ type }, 'creating invoice webhook event') + + await InvoiceEvent.query(deps.knex).insertAndFetch({ + type, data: invoice.toData(amountReceived), - processAt: new Date() + withdrawal: { + accountId: invoice.id, + assetId: invoice.account.assetId, + amount: amountReceived + } }) - const error = await deps.accountingService.createWithdrawal({ - id: event.id, - account: invoice, - amount: amountReceived, - timeout: BigInt(RETRY_LIMIT_MS) * BigInt(1e6) // ms -> ns + + await invoice.$query(deps.knex).patch({ + processAt: null }) - if (error) throw new Error(error) - } else { - deps.logger.debug({ amountReceived }, 'deleting expired invoice') - await invoice.$query(deps.knex).delete() + } catch (error) { + deps.logger.warn({ error }, 'webhook event creation failed; retrying') } } From f5a2ae9491a25ccb0fb1e1f97755b0bb6d1ada89 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Tue, 15 Feb 2022 13:49:48 -0600 Subject: [PATCH 13/15] chore(backend): remove retry and retention event limits --- ...20131110501_create_webhook_events_table.js | 2 +- packages/backend/src/webhook/model.ts | 2 +- packages/backend/src/webhook/service.test.ts | 39 ++----------------- packages/backend/src/webhook/service.ts | 22 +++-------- 4 files changed, 12 insertions(+), 53 deletions(-) diff --git a/packages/backend/migrations/20220131110501_create_webhook_events_table.js b/packages/backend/migrations/20220131110501_create_webhook_events_table.js index 8da10ab13c..54eb5b6486 100644 --- a/packages/backend/migrations/20220131110501_create_webhook_events_table.js +++ b/packages/backend/migrations/20220131110501_create_webhook_events_table.js @@ -12,7 +12,7 @@ exports.up = function (knex) { table.foreign('withdrawalAssetId').references('assets.id') table.bigInteger('withdrawalAmount').nullable() - table.timestamp('processAt').notNullable().defaultTo(knex.fn.now()) + table.timestamp('processAt').nullable().defaultTo(knex.fn.now()) table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) diff --git a/packages/backend/src/webhook/model.ts b/packages/backend/src/webhook/model.ts index ada852ef21..51b18e23e9 100644 --- a/packages/backend/src/webhook/model.ts +++ b/packages/backend/src/webhook/model.ts @@ -14,7 +14,7 @@ export class WebhookEvent extends BaseModel { public data!: Record public attempts!: number public statusCode?: number - public processAt!: Date + public processAt!: Date | null public withdrawal?: { accountId: string diff --git a/packages/backend/src/webhook/service.test.ts b/packages/backend/src/webhook/service.test.ts index 0409aa318e..ce5d6da35d 100644 --- a/packages/backend/src/webhook/service.test.ts +++ b/packages/backend/src/webhook/service.test.ts @@ -1,5 +1,4 @@ import assert from 'assert' -import axios from 'axios' import nock, { Definition } from 'nock' import { URL } from 'url' import Knex from 'knex' @@ -9,9 +8,7 @@ import { WebhookEvent } from './model' import { WebhookService, generateWebhookSignature, - RETENTION_LIMIT_MS, - RETRY_BACKOFF_MS, - RETRY_LIMIT_MS + RETRY_BACKOFF_MS } from './service' import { AccountingService } from '../accounting/service' import { createTestApp, TestContainer } from '../tests/app' @@ -144,7 +141,7 @@ describe('Webhook Service', (): void => { await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ attempts: 1, statusCode: 200, - processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + processAt: null }) }) @@ -155,7 +152,7 @@ describe('Webhook Service', (): void => { await expect(webhookService.processNext()).resolves.toEqual(event.id) expect(scope.isDone()).toBe(true) const updatedEvent = await webhookService.getEvent(event.id) - assert.ok(updatedEvent) + assert.ok(updatedEvent?.processAt) expect(updatedEvent).toMatchObject({ attempts: 1, statusCode: status @@ -174,7 +171,7 @@ describe('Webhook Service', (): void => { await expect(webhookService.processNext()).resolves.toEqual(event.id) expect(scope.isDone()).toBe(true) const updatedEvent = await webhookService.getEvent(event.id) - assert.ok(updatedEvent) + assert.ok(updatedEvent?.processAt) expect(updatedEvent).toMatchObject({ attempts: 1, statusCode: null @@ -183,33 +180,5 @@ describe('Webhook Service', (): void => { event.createdAt.getTime() + RETRY_BACKOFF_MS ) }) - - test('Stops retrying after limit', async (): Promise => { - jest.useFakeTimers('modern') - jest.setSystemTime( - new Date(event.createdAt.getTime() + RETRY_LIMIT_MS - 1) - ) - - const mockPost = jest - .spyOn(axios, 'post') - .mockRejectedValueOnce(new Error('last try')) - await expect(webhookService.processNext()).resolves.toEqual(event.id) - expect(mockPost).toHaveBeenCalledTimes(1) - await expect(webhookService.getEvent(event.id)).resolves.toMatchObject({ - attempts: 1, - statusCode: null, - processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) - }) - }) - - test('Deletes event after retention period', async (): Promise => { - jest.useFakeTimers('modern') - jest.setSystemTime( - new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) - ) - - await expect(webhookService.processNext()).resolves.toEqual(event.id) - await expect(webhookService.getEvent(event.id)).resolves.toBeUndefined() - }) }) }) diff --git a/packages/backend/src/webhook/service.ts b/packages/backend/src/webhook/service.ts index c555ca032c..cef93c1251 100644 --- a/packages/backend/src/webhook/service.ts +++ b/packages/backend/src/webhook/service.ts @@ -6,10 +6,10 @@ import { WebhookEvent } from './model' import { IAppConfig } from '../config/app' import { BaseService } from '../shared/baseService' -// First retry waits 10 seconds, second retry waits 20 (more) seconds, etc. +// First retry waits 10 seconds +// Second retry waits 20 (more) seconds +// Third retry waits 30 (more) seconds, etc. up to 60 seconds export const RETRY_BACKOFF_MS = 10_000 -export const RETRY_LIMIT_MS = 60_000 * 60 * 24 // 1 day -export const RETENTION_LIMIT_MS = 60_000 * 60 * 24 * 30 // 30 days export interface WebhookService { getEvent(id: string): Promise @@ -67,11 +67,7 @@ async function processNextWebhookEvent( }) } - if (now >= event.createdAt.getTime() + RETENTION_LIMIT_MS) { - await event.$query(deps.knex).delete() - } else { - await sendWebhookEvent(deps, event) - } + await sendWebhookEvent(deps, event) return event.id }) @@ -109,7 +105,7 @@ async function sendWebhookEvent( await event.$query(deps.knex).patch({ attempts: event.attempts + 1, statusCode: 200, - processAt: new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) + processAt: null }) } catch (err) { const attempts = event.attempts + 1 @@ -122,16 +118,10 @@ async function sendWebhookEvent( 'webhook request failed' ) - const retryAt = Date.now() + Math.min(attempts, 6) * RETRY_BACKOFF_MS - const processAt = - retryAt < event.createdAt.getTime() + RETRY_LIMIT_MS - ? new Date(retryAt) - : new Date(event.createdAt.getTime() + RETENTION_LIMIT_MS) - await event.$query(deps.knex).patch({ attempts, statusCode: err.isAxiosError && err.response?.status, - processAt + processAt: new Date(Date.now() + Math.min(attempts, 6) * RETRY_BACKOFF_MS) }) } } From 1cc89ceb85f2a44039d2e0bd68868e6152a7edca Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Wed, 16 Feb 2022 16:21:11 -0600 Subject: [PATCH 14/15] chore(backend): verify event deposit is created --- .../src/graphql/resolvers/liquidity.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index 2382f381ca..467407d2e6 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -1,3 +1,4 @@ +import assert from 'assert' import { gql } from 'apollo-server-koa' import Knex from 'knex' import { v4 as uuid } from 'uuid' @@ -1621,6 +1622,9 @@ describe('Liquidity Resolvers', (): void => { amountSent: BigInt(0) } }) + await expect(accountingService.getBalance(payment.id)).resolves.toEqual( + BigInt(0) + ) } ) @@ -1645,6 +1649,7 @@ describe('Liquidity Resolvers', (): void => { ) test('Can deposit account liquidity', async (): Promise => { + const depositSpy = jest.spyOn(accountingService, 'createDeposit') const response = await appContainer.apolloClient .mutate({ mutation: gql` @@ -1674,6 +1679,15 @@ describe('Liquidity Resolvers', (): void => { expect(response.success).toBe(true) expect(response.code).toEqual('200') expect(response.error).toBeNull() + assert.ok(payment.quote) + await expect(depositSpy).toHaveBeenCalledWith({ + id: eventId, + account: expect.any(OutgoingPayment), + amount: payment.quote.maxSourceAmount + }) + await expect( + accountingService.getBalance(payment.id) + ).resolves.toEqual(payment.quote.maxSourceAmount) }) test("Can't deposit for non-existent webhook event id", async (): Promise => { From 5757cbbdb2a00bad7e1356c3afcc9efb921fba58 Mon Sep 17 00:00:00 2001 From: Brandon Wilson Date: Wed, 16 Feb 2022 16:32:39 -0600 Subject: [PATCH 15/15] chore(backend): make timeouts in milliseconds --- packages/backend/src/accounting/service.test.ts | 6 +++--- packages/backend/src/accounting/service.ts | 2 +- packages/backend/src/accounting/transfers.ts | 4 ++-- packages/backend/src/connector/core/rafiki.ts | 2 +- packages/backend/src/graphql/resolvers/liquidity.test.ts | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/accounting/service.test.ts b/packages/backend/src/accounting/service.test.ts index 2200520d90..0a58c75bf8 100644 --- a/packages/backend/src/accounting/service.test.ts +++ b/packages/backend/src/accounting/service.test.ts @@ -34,7 +34,7 @@ describe('Accounting Service', (): void => { let accountingService: AccountingService let accountFactory: AccountFactory let tigerbeetleContainer: StartedTestContainer - const timeout = BigInt(10e9) // 10 seconds + const timeout = BigInt(10_000) // 10 seconds const messageProducer = new GraphileProducer() const mockMessageProducer = { send: jest.fn() @@ -659,7 +659,7 @@ describe('Accounting Service', (): void => { const expiringWithdrawal = { ...withdrawal, id: uuid(), - timeout: BigInt(1) // nano-second + timeout: BigInt(1) } await expect( accountingService.createWithdrawal(expiringWithdrawal) @@ -726,7 +726,7 @@ describe('Accounting Service', (): void => { const expiringWithdrawal = { ...withdrawal, id: uuid(), - timeout: BigInt(1) // nano-second + timeout: BigInt(1) } await expect( accountingService.createWithdrawal(expiringWithdrawal) diff --git a/packages/backend/src/accounting/service.ts b/packages/backend/src/accounting/service.ts index 60290f25ea..aabf77b97e 100644 --- a/packages/backend/src/accounting/service.ts +++ b/packages/backend/src/accounting/service.ts @@ -62,7 +62,7 @@ export interface TransferOptions { destinationAccount: LiquidityAccount sourceAmount: bigint destinationAmount?: bigint - timeout: bigint // nano-seconds + timeout: bigint } export interface Transaction { diff --git a/packages/backend/src/accounting/transfers.ts b/packages/backend/src/accounting/transfers.ts index 006b6e23e0..0d50381cff 100644 --- a/packages/backend/src/accounting/transfers.ts +++ b/packages/backend/src/accounting/transfers.ts @@ -28,7 +28,7 @@ export interface CreateTransferOptions { sourceAccountId: AccountId destinationAccountId: AccountId amount: bigint - timeout?: bigint // nano-seconds + timeout?: bigint } export async function createTransfers( @@ -57,7 +57,7 @@ export async function createTransfers( reserved: TRANSFER_RESERVED, code: 0, flags, - timeout: transfer.timeout || BigInt(0), + timeout: transfer.timeout ? transfer.timeout * BigInt(10e6) : BigInt(0), // ms -> ns timestamp: BigInt(0) }) } diff --git a/packages/backend/src/connector/core/rafiki.ts b/packages/backend/src/connector/core/rafiki.ts index 5ff08b9dc2..718ebab7bc 100644 --- a/packages/backend/src/connector/core/rafiki.ts +++ b/packages/backend/src/connector/core/rafiki.ts @@ -50,7 +50,7 @@ export interface TransferOptions { destinationAccount: OutgoingAccount sourceAmount: bigint destinationAmount?: bigint - timeout: bigint // nano-seconds + timeout: bigint } export interface AccountingService { diff --git a/packages/backend/src/graphql/resolvers/liquidity.test.ts b/packages/backend/src/graphql/resolvers/liquidity.test.ts index 467407d2e6..e8e5052de3 100644 --- a/packages/backend/src/graphql/resolvers/liquidity.test.ts +++ b/packages/backend/src/graphql/resolvers/liquidity.test.ts @@ -44,7 +44,7 @@ describe('Liquidity Resolvers', (): void => { let assetService: AssetService let peerFactory: PeerFactory let knex: Knex - const timeout = BigInt(10e9) // 10 seconds + const timeout = BigInt(10_000) // 10 seconds beforeAll( async (): Promise => {