feat(backend): improve PSQL balance calculation (#2881)

* chore(backend): add check constraint to ledger transfers * feat(backend): update balance calculation to use psql query
interledger · Aug 26, 2024 · 0248bc2 · 0248bc2
1 parent 90a1415
commit 0248bc2
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 31 deletions.
diff --git a/packages/backend/migrations/20240820161040_add_ledger_transfer_constraint.js b/packages/backend/migrations/20240820161040_add_ledger_transfer_constraint.js
@@ -0,0 +1,23 @@
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.up = function (knex) {
+  return knex.schema.table('ledgerTransfers', function (table) {
+    table.check(
+      `("state" != 'PENDING') OR ("expiresAt" IS NOT NULL)`,
+      null,
+      'check_pending_requires_expires_at'
+    )
+  })
+}
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.down = function (knex) {
+  return knex.schema.table('ledgerTransfers', function (table) {
+    table.dropChecks(['check_pending_requires_expires_at'])
+  })
+}
diff --git a/packages/backend/src/accounting/psql/balance.test.ts b/packages/backend/src/accounting/psql/balance.test.ts
@@ -83,6 +83,27 @@ describe('Balances', (): void => {
       })
     })
 
+    test('ignores expired pending transfers', async (): Promise<void> => {
+      await createLedgerTransfer(
+        {
+          ledger: account.ledger,
+          creditAccountId: account.id,
+          debitAccountId: peerAccount.id,
+          state: LedgerTransferState.PENDING,
+          expiresAt: new Date(Date.now() - 1),
+          amount: 10n
+        },
+        knex
+      )
+
+      await expect(getAccountBalances(serviceDeps, account)).resolves.toEqual({
+        creditsPosted: 0n,
+        creditsPending: 0n,
+        debitsPosted: 0n,
+        debitsPending: 0n
+      })
+    })
+
     describe('calculates balances for single transfers', (): void => {
       const amounts = {
         credit: {

diff --git a/packages/backend/src/accounting/psql/balance.ts b/packages/backend/src/accounting/psql/balance.ts
@@ -1,7 +1,5 @@
 import { LedgerAccount } from './ledger-account/model'
-import { LedgerTransferState } from '../service'
 import { ServiceDependencies } from './service'
-import { getAccountTransfers } from './ledger-transfer'
 import { TransactionOrKnex } from 'objection'
 
 export interface AccountBalance {
@@ -16,38 +14,45 @@ export async function getAccountBalances(
   account: LedgerAccount,
   trx?: TransactionOrKnex
 ): Promise<AccountBalance> {
-  const { credits, debits } = await getAccountTransfers(
-    deps,
-    account.id,
-    undefined, // No limit for balances
-    trx
-  )
+  try {
+    const queryResult = await (trx ?? deps.knex).raw(
+      `
+    SELECT
+      COALESCE(SUM("amount") FILTER(WHERE "creditAccountId" = :accountId AND "state" = 'POSTED'), 0) AS "creditsPosted",
+      COALESCE(SUM("amount") FILTER(WHERE "creditAccountId" = :accountId AND "state" = 'PENDING'), 0) AS "creditsPending",
+      COALESCE(SUM("amount") FILTER(WHERE "debitAccountId" = :accountId AND "state" = 'POSTED'), 0) AS "debitsPosted",
+      COALESCE(SUM("amount") FILTER(WHERE "debitAccountId" = :accountId AND "state" = 'PENDING'), 0) AS "debitsPending"
+    FROM "ledgerTransfers"
+    WHERE ("creditAccountId" = :accountId OR "debitAccountId" = :accountId)
+      AND ("state" = 'POSTED' OR ("state" = 'PENDING' AND "expiresAt" > NOW()));
+    `,
+      { accountId: account.id }
+    )
 
-  let creditsPosted = 0n
-  let creditsPending = 0n
-  let debitsPosted = 0n
-  let debitsPending = 0n
-
-  for (const credit of credits) {
-    if (credit.state === LedgerTransferState.POSTED) {
-      creditsPosted += credit.amount
-    } else if (credit.state === LedgerTransferState.PENDING) {
-      creditsPending += credit.amount
+    if (queryResult?.rows < 1) {
+      throw new Error('No results when fetching balance for account')
     }
-  }
 
-  for (const debit of debits) {
-    if (debit.state === LedgerTransferState.POSTED) {
-      debitsPosted += debit.amount
-    } else if (debit.state === LedgerTransferState.PENDING) {
-      debitsPending += debit.amount
+    const creditsPosted = BigInt(queryResult.rows[0].creditsPosted)
+    const creditsPending = BigInt(queryResult.rows[0].creditsPending)
+    const debitsPosted = BigInt(queryResult.rows[0].debitsPosted)
+    const debitsPending = BigInt(queryResult.rows[0].debitsPending)
+
+    return {
+      creditsPosted,
+      creditsPending,
+      debitsPosted,
+      debitsPending
     }
-  }
+  } catch (err) {
+    deps.logger.error(
+      {
+        err,
+        accountId: account.id
+      },
+      'Could not fetch balances for account'
+    )
 
-  return {
-    creditsPosted,
-    creditsPending,
-    debitsPosted,
-    debitsPending
+    throw err
   }
 }
diff --git a/packages/backend/src/tests/ledgerTransfer.ts b/packages/backend/src/tests/ledgerTransfer.ts
@@ -36,9 +36,13 @@ export const createLedgerTransfer = async (
     creditAccountId: creditAccountId,
     debitAccountId: debitAccountId,
     amount: amount ?? 10n,
-    expiresAt,
     ledger,
     state: state ?? LedgerTransferState.POSTED,
+    expiresAt:
+      expiresAt ??
+      (state === LedgerTransferState.PENDING
+        ? new Date(Date.now() + 86_400_000)
+        : undefined),
     type
   })
 }