From 5f4d0b5a6fb76c41b5a43338d3195ee48a7c2233 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Mon, 26 Aug 2024 16:14:10 +0300 Subject: [PATCH 1/7] refactor(backend): update rate caching (#2891) --- packages/backend/src/rates/service.test.ts | 29 +++++----- packages/backend/src/rates/service.ts | 61 +++++++++++----------- 2 files changed, 48 insertions(+), 42 deletions(-) diff --git a/packages/backend/src/rates/service.test.ts b/packages/backend/src/rates/service.test.ts index 5778a54fc6..8145a3cd56 100644 --- a/packages/backend/src/rates/service.test.ts +++ b/packages/backend/src/rates/service.test.ts @@ -6,6 +6,7 @@ import { initIocContainer } from '../' import { AppServices } from '../app' import { CacheDataStore } from '../middleware/cache/data-stores' import { mockRatesApi } from '../tests/rates' +import { AxiosInstance } from 'axios' const nock = (global as unknown as { nock: typeof import('nock') }).nock @@ -214,23 +215,27 @@ describe('Rates service', function () { expect(apiRequestCount).toBe(2) }) - it('prefetches when the cached request is old', async () => { + it('returns new rates after cache expires', async () => { await expect(service.rates('USD')).resolves.toEqual(usdRates) - jest.advanceTimersByTime(exchangeRatesLifetime * 0.5 + 1) - // ... cache isn't expired yet, but it will be soon - await expect(service.rates('USD')).resolves.toEqual(usdRates) - expect(apiRequestCount).toBe(1) - - // Invalidate the cache. - jest.advanceTimersByTime(exchangeRatesLifetime * 0.5 + 1) + jest.advanceTimersByTime(exchangeRatesLifetime + 1) await expect(service.rates('USD')).resolves.toEqual(usdRates) - // The prefetch response is promoted to the cache. expect(apiRequestCount).toBe(2) }) - it('cannot use an expired cache', async () => { - await expect(service.rates('USD')).resolves.toEqual(usdRates) - jest.advanceTimersByTime(exchangeRatesLifetime + 1) + it('returns rates on second request (first one was error)', async () => { + jest + .spyOn( + (service as RatesService & { axios: AxiosInstance }).axios, + 'get' + ) + .mockImplementationOnce(() => { + apiRequestCount++ + throw new Error() + }) + + await expect(service.rates('USD')).rejects.toThrow( + 'Could not fetch rates' + ) await expect(service.rates('USD')).resolves.toEqual(usdRates) expect(apiRequestCount).toBe(2) }) diff --git a/packages/backend/src/rates/service.ts b/packages/backend/src/rates/service.ts index 5a6ed7322e..f4cae1f819 100644 --- a/packages/backend/src/rates/service.ts +++ b/packages/backend/src/rates/service.ts @@ -1,5 +1,5 @@ import { BaseService } from '../shared/baseService' -import Axios, { AxiosInstance } from 'axios' +import Axios, { AxiosInstance, isAxiosError } from 'axios' import { convert, ConvertOptions } from './util' import { createInMemoryDataStore } from '../middleware/cache/data-stores/in-memory' import { CacheDataStore } from '../middleware/cache/data-stores' @@ -74,29 +74,13 @@ class RatesServiceImpl implements RatesService { } private async getRates(baseAssetCode: string): Promise { - const [cachedRate, cachedExpiry] = await Promise.all([ - this.cachedRates.get(baseAssetCode), - this.cachedRates.getKeyExpiry(baseAssetCode) - ]) - - if (cachedRate && cachedExpiry) { - const isCloseToExpiry = - cachedExpiry.getTime() < - Date.now() + 0.5 * this.deps.exchangeRatesLifetime - - if (isCloseToExpiry) { - this.fetchNewRatesAndCache(baseAssetCode) // don't await, just get new rates for later - } + const cachedRate = await this.cachedRates.get(baseAssetCode) + if (cachedRate) { return JSON.parse(cachedRate) } - try { - return await this.fetchNewRatesAndCache(baseAssetCode) - } catch (err) { - this.cachedRates.delete(baseAssetCode) - throw err - } + return await this.fetchNewRatesAndCache(baseAssetCode) } private async fetchNewRatesAndCache(baseAssetCode: string): Promise { @@ -106,12 +90,32 @@ class RatesServiceImpl implements RatesService { this.inProgressRequests[baseAssetCode] = this.fetchNewRates(baseAssetCode) } - const rates = await this.inProgressRequests[baseAssetCode] + try { + const rates = await this.inProgressRequests[baseAssetCode] - delete this.inProgressRequests[baseAssetCode] + await this.cachedRates.set(baseAssetCode, JSON.stringify(rates)) + return rates + } catch (err) { + const errorMessage = 'Could not fetch rates' + + this.deps.logger.error( + { + ...(isAxiosError(err) + ? { + errorMessage: err.message, + errorCode: err.code, + errorStatus: err.status + } + : { err }), + url: this.deps.exchangeRatesUrl + }, + errorMessage + ) - await this.cachedRates.set(baseAssetCode, JSON.stringify(rates)) - return rates + throw new Error(errorMessage) + } finally { + delete this.inProgressRequests[baseAssetCode] + } } private async fetchNewRates(baseAssetCode: string): Promise { @@ -120,12 +124,9 @@ class RatesServiceImpl implements RatesService { return { base: baseAssetCode, rates: {} } } - const res = await this.axios - .get(url, { params: { base: baseAssetCode } }) - .catch((err) => { - this.deps.logger.warn({ err: err.message }, 'price request error') - throw err - }) + const res = await this.axios.get(url, { + params: { base: baseAssetCode } + }) const { base, rates } = res.data this.checkBaseAsset(base) From 3bc368a8e7e670f89028731596b147da8a5b2d20 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Tue, 27 Aug 2024 09:19:18 +0300 Subject: [PATCH 2/7] chore(localenv): fix startup migration (#2897) --- localenv/cloud-nine-wallet/dbinit.sql | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/localenv/cloud-nine-wallet/dbinit.sql b/localenv/cloud-nine-wallet/dbinit.sql index cb8895204f..0fe8c3e88b 100644 --- a/localenv/cloud-nine-wallet/dbinit.sql +++ b/localenv/cloud-nine-wallet/dbinit.sql @@ -9,3 +9,7 @@ ALTER DATABASE cloud_nine_wallet_auth OWNER TO cloud_nine_wallet_auth; CREATE USER happy_life_bank_backend WITH PASSWORD 'happy_life_bank_backend'; CREATE DATABASE happy_life_bank_backend; ALTER DATABASE happy_life_bank_backend OWNER TO happy_life_bank_backend; + +CREATE USER happy_life_bank_auth WITH PASSWORD 'happy_life_bank_auth'; +CREATE DATABASE happy_life_bank_auth; +ALTER DATABASE happy_life_bank_auth OWNER TO happy_life_bank_auth; From b9be601ac22cf36f0c1211a6caeeff1a75aae580 Mon Sep 17 00:00:00 2001 From: Nathan Lie Date: Tue, 27 Aug 2024 11:09:27 +0200 Subject: [PATCH 3/7] feat(backend): add trace to outgoing payment lifecycle (#2884) * feat(backend): add trace to outgoing payment lifecycle * feat(backend): encapsulate query lookup in trace * chore(localenv): remove traces from default localenv --------- Co-authored-by: Max Kurapov --- .../open_payments/payment/outgoing/worker.ts | 45 ++++++++++++------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/packages/backend/src/open_payments/payment/outgoing/worker.ts b/packages/backend/src/open_payments/payment/outgoing/worker.ts index 4c9e35cf60..0df698f960 100644 --- a/packages/backend/src/open_payments/payment/outgoing/worker.ts +++ b/packages/backend/src/open_payments/payment/outgoing/worker.ts @@ -5,6 +5,7 @@ import { OutgoingPayment, OutgoingPaymentState } from './model' import { LifecycleError, PaymentError } from './errors' import * as lifecycle from './lifecycle' import { PaymentMethodHandlerError } from '../../../payment-method/handler/errors' +import { trace, Span } from '@opentelemetry/api' // First retry waits 10 seconds, second retry waits 20 (more) seconds, etc. export const RETRY_BACKOFF_SECONDS = 10 @@ -15,23 +16,33 @@ const MAX_STATE_ATTEMPTS = 5 export async function processPendingPayment( deps_: ServiceDependencies ): Promise { - return deps_.knex.transaction(async (trx) => { - const payment = await getPendingPayment(trx) - if (!payment) return - - await handlePaymentLifecycle( - { - ...deps_, - knex: trx, - logger: deps_.logger.child({ - payment: payment.id, - from_state: payment.state - }) - }, - payment - ) - return payment.id - }) + const tracer = trace.getTracer('outgoing_payment_worker') + + return tracer.startActiveSpan( + 'outgoingPaymentLifecycle', + async (span: Span) => { + const paymentId = await deps_.knex.transaction(async (trx) => { + const payment = await getPendingPayment(trx) + if (!payment) return + + await handlePaymentLifecycle( + { + ...deps_, + knex: trx, + logger: deps_.logger.child({ + payment: payment.id, + from_state: payment.state + }) + }, + payment + ) + return payment.id + }) + + span.end() + return paymentId + } + ) } // Fetch (and lock) a payment for work. From 5dfa6c4f6d2efca764b8d0575410dbcd9abe0e15 Mon Sep 17 00:00:00 2001 From: bsanduc Date: Tue, 27 Aug 2024 16:27:37 +0300 Subject: [PATCH 4/7] Add migration files --- ...20240827131401_create_tenants_tables.js.js | 21 ++++++ ...27131417_update_grants_with_tenantId.js.js | 25 +++++++ ...20240827115808_create_tenants_tables.js.js | 21 ++++++ ...15832_update_resources_with_tenantId.js.js | 75 +++++++++++++++++++ 4 files changed, 142 insertions(+) create mode 100644 packages/auth/migrations/20240827131401_create_tenants_tables.js.js create mode 100644 packages/auth/migrations/20240827131417_update_grants_with_tenantId.js.js create mode 100644 packages/backend/migrations/20240827115808_create_tenants_tables.js.js create mode 100644 packages/backend/migrations/20240827115832_update_resources_with_tenantId.js.js diff --git a/packages/auth/migrations/20240827131401_create_tenants_tables.js.js b/packages/auth/migrations/20240827131401_create_tenants_tables.js.js new file mode 100644 index 0000000000..aef995843a --- /dev/null +++ b/packages/auth/migrations/20240827131401_create_tenants_tables.js.js @@ -0,0 +1,21 @@ +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.up = function (knex) { + return knex.schema.createTable('tenants', function (table) { + table.uuid('id').primary() + table.string('kratosUrl').notNullable() + table.timestamp('createdAt').defaultTo(knex.fn.now()) + table.timestamp('updatedAt').defaultTo(knex.fn.now()) + table.timestamp('deletedAt').nullable().defaultTo(null) + }) +} + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = function (knex) { + return knex.schema.dropTable('tenants') +} diff --git a/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js.js b/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js.js new file mode 100644 index 0000000000..758d187419 --- /dev/null +++ b/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js.js @@ -0,0 +1,25 @@ +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.up = function (knex) { + return knex.schema.table('grants', function (table) { + table.uuid('tenantId').notNullable() + table + .foreign('tenantId') + .references('id') + .inTable('tenants') + .onDelete('CASCADE') + }) +} + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = function (knex) { + return knex.schema.table('grants', function (table) { + table.dropForeign(['tenantId']) + table.dropColumn('tenantId') + }) +} diff --git a/packages/backend/migrations/20240827115808_create_tenants_tables.js.js b/packages/backend/migrations/20240827115808_create_tenants_tables.js.js new file mode 100644 index 0000000000..aef995843a --- /dev/null +++ b/packages/backend/migrations/20240827115808_create_tenants_tables.js.js @@ -0,0 +1,21 @@ +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.up = function (knex) { + return knex.schema.createTable('tenants', function (table) { + table.uuid('id').primary() + table.string('kratosUrl').notNullable() + table.timestamp('createdAt').defaultTo(knex.fn.now()) + table.timestamp('updatedAt').defaultTo(knex.fn.now()) + table.timestamp('deletedAt').nullable().defaultTo(null) + }) +} + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = function (knex) { + return knex.schema.dropTable('tenants') +} diff --git a/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js.js b/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js.js new file mode 100644 index 0000000000..aa1adde645 --- /dev/null +++ b/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js.js @@ -0,0 +1,75 @@ +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.up = function (knex) { + return knex.schema + .table('quotes', function (table) { + table.uuid('tenantId').notNullable() + table + .foreign('tenantId') + .references('id') + .inTable('tenants') + .onDelete('CASCADE') + }) + .table('incomingPayments', function (table) { + table.uuid('tenantId').notNullable() + table + .foreign('tenantId') + .references('id') + .inTable('tenants') + .onDelete('CASCADE') + }) + .table('outgoingPayments', function (table) { + table.uuid('tenantId').notNullable() + table + .foreign('tenantId') + .references('id') + .inTable('tenants') + .onDelete('CASCADE') + }) + .table('walletAddresses', function (table) { + table.uuid('tenantId').notNullable() + table + .foreign('tenantId') + .references('id') + .inTable('tenants') + .onDelete('CASCADE') + }) + .table('grants', function (table) { + table.uuid('tenantId').notNullable() + table + .foreign('tenantId') + .references('id') + .inTable('tenants') + .onDelete('CASCADE') + }) +} + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = function (knex) { + return knex.schema + .table('quotes', function (table) { + table.dropForeign(['tenantId']) + table.dropColumn('tenantId') + }) + .table('incomingPayments', function (table) { + table.dropForeign(['tenantId']) + table.dropColumn('tenantId') + }) + .table('outgoingPayments', function (table) { + table.dropForeign(['tenantId']) + table.dropColumn('tenantId') + }) + .table('walletAddresses', function (table) { + table.dropForeign(['tenantId']) + table.dropColumn('tenantId') + }) + .table('grants', function (table) { + table.dropForeign(['tenantId']) + table.dropColumn('tenantId') + }) +} From 413c9044ffad6232227cb34dd04ca7fb13552517 Mon Sep 17 00:00:00 2001 From: bsanduc Date: Tue, 27 Aug 2024 16:56:56 +0300 Subject: [PATCH 5/7] Update migration files --- .../auth/migrations/20240827131401_create_tenants_tables.js.js | 3 ++- .../migrations/20240827115808_create_tenants_tables.js.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/auth/migrations/20240827131401_create_tenants_tables.js.js b/packages/auth/migrations/20240827131401_create_tenants_tables.js.js index aef995843a..4aa68c3ff4 100644 --- a/packages/auth/migrations/20240827131401_create_tenants_tables.js.js +++ b/packages/auth/migrations/20240827131401_create_tenants_tables.js.js @@ -5,7 +5,8 @@ exports.up = function (knex) { return knex.schema.createTable('tenants', function (table) { table.uuid('id').primary() - table.string('kratosUrl').notNullable() + table.string('idpConsentEndpoint').notNullable() + table.string('idpSecret').notNullable() table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) table.timestamp('deletedAt').nullable().defaultTo(null) diff --git a/packages/backend/migrations/20240827115808_create_tenants_tables.js.js b/packages/backend/migrations/20240827115808_create_tenants_tables.js.js index aef995843a..cfa2582e1d 100644 --- a/packages/backend/migrations/20240827115808_create_tenants_tables.js.js +++ b/packages/backend/migrations/20240827115808_create_tenants_tables.js.js @@ -5,7 +5,7 @@ exports.up = function (knex) { return knex.schema.createTable('tenants', function (table) { table.uuid('id').primary() - table.string('kratosUrl').notNullable() + table.string('kratosIdentityId').notNullable() table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) table.timestamp('deletedAt').nullable().defaultTo(null) From 11720c886cd5189aa74ee7b4e90776cbdb32345d Mon Sep 17 00:00:00 2001 From: bsanduc Date: Tue, 27 Aug 2024 17:02:20 +0300 Subject: [PATCH 6/7] Fix file name for migrations --- ...nants_tables.js.js => 20240827131401_create_tenants_tables.js} | 0 ...nantId.js.js => 20240827131417_update_grants_with_tenantId.js} | 0 ...nants_tables.js.js => 20240827115808_create_tenants_tables.js} | 0 ...tId.js.js => 20240827115832_update_resources_with_tenantId.js} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename packages/auth/migrations/{20240827131401_create_tenants_tables.js.js => 20240827131401_create_tenants_tables.js} (100%) rename packages/auth/migrations/{20240827131417_update_grants_with_tenantId.js.js => 20240827131417_update_grants_with_tenantId.js} (100%) rename packages/backend/migrations/{20240827115808_create_tenants_tables.js.js => 20240827115808_create_tenants_tables.js} (100%) rename packages/backend/migrations/{20240827115832_update_resources_with_tenantId.js.js => 20240827115832_update_resources_with_tenantId.js} (100%) diff --git a/packages/auth/migrations/20240827131401_create_tenants_tables.js.js b/packages/auth/migrations/20240827131401_create_tenants_tables.js similarity index 100% rename from packages/auth/migrations/20240827131401_create_tenants_tables.js.js rename to packages/auth/migrations/20240827131401_create_tenants_tables.js diff --git a/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js.js b/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js similarity index 100% rename from packages/auth/migrations/20240827131417_update_grants_with_tenantId.js.js rename to packages/auth/migrations/20240827131417_update_grants_with_tenantId.js diff --git a/packages/backend/migrations/20240827115808_create_tenants_tables.js.js b/packages/backend/migrations/20240827115808_create_tenants_tables.js similarity index 100% rename from packages/backend/migrations/20240827115808_create_tenants_tables.js.js rename to packages/backend/migrations/20240827115808_create_tenants_tables.js diff --git a/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js.js b/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js similarity index 100% rename from packages/backend/migrations/20240827115832_update_resources_with_tenantId.js.js rename to packages/backend/migrations/20240827115832_update_resources_with_tenantId.js From b3096c477ace9f114c1c2dc721a9966b8e012e17 Mon Sep 17 00:00:00 2001 From: bsanduc Date: Tue, 27 Aug 2024 17:12:41 +0300 Subject: [PATCH 7/7] Remove cascade deletion --- ...40827131417_update_grants_with_tenantId.js | 6 +--- ...27115832_update_resources_with_tenantId.js | 30 ++++--------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js b/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js index 758d187419..98ca85fbe7 100644 --- a/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js +++ b/packages/auth/migrations/20240827131417_update_grants_with_tenantId.js @@ -5,11 +5,7 @@ exports.up = function (knex) { return knex.schema.table('grants', function (table) { table.uuid('tenantId').notNullable() - table - .foreign('tenantId') - .references('id') - .inTable('tenants') - .onDelete('CASCADE') + table.foreign('tenantId').references('id').inTable('tenants') }) } diff --git a/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js b/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js index aa1adde645..6d9f201a57 100644 --- a/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js +++ b/packages/backend/migrations/20240827115832_update_resources_with_tenantId.js @@ -6,43 +6,23 @@ exports.up = function (knex) { return knex.schema .table('quotes', function (table) { table.uuid('tenantId').notNullable() - table - .foreign('tenantId') - .references('id') - .inTable('tenants') - .onDelete('CASCADE') + table.foreign('tenantId').references('id').inTable('tenants') }) .table('incomingPayments', function (table) { table.uuid('tenantId').notNullable() - table - .foreign('tenantId') - .references('id') - .inTable('tenants') - .onDelete('CASCADE') + table.foreign('tenantId').references('id').inTable('tenants') }) .table('outgoingPayments', function (table) { table.uuid('tenantId').notNullable() - table - .foreign('tenantId') - .references('id') - .inTable('tenants') - .onDelete('CASCADE') + table.foreign('tenantId').references('id').inTable('tenants') }) .table('walletAddresses', function (table) { table.uuid('tenantId').notNullable() - table - .foreign('tenantId') - .references('id') - .inTable('tenants') - .onDelete('CASCADE') + table.foreign('tenantId').references('id').inTable('tenants') }) .table('grants', function (table) { table.uuid('tenantId').notNullable() - table - .foreign('tenantId') - .references('id') - .inTable('tenants') - .onDelete('CASCADE') + table.foreign('tenantId').references('id').inTable('tenants') }) }