Skip to content

Fix/accounts transaction integrity#248

Open
Kenshiin13 wants to merge 8 commits into
overextended:mainfrom
Kenshiin13:fix/accounts-transaction-integrity
Open

Fix/accounts transaction integrity#248
Kenshiin13 wants to merge 8 commits into
overextended:mainfrom
Kenshiin13:fix/accounts-transaction-integrity

Conversation

@Kenshiin13

Copy link
Copy Markdown

Description

The account money functions had several ways to lose or duplicate money when a query failed partway through, all rooted in how transactions were opened, committed, and cleaned up. This fixes them across server/accounts/db.ts, server/groups/db.ts, and server/db/index.ts.

What was wrong

Balance changes and their ledger rows weren't atomic. UpdateBalance updated the balance and inserted the ledger row as two separate autocommitted statements. If the ledger insert failed, the balance had already changed and committed while the function returned failure.

Invoice payments moved money in separate steps. UpdateInvoice debited one account and credited the other independently, then marked the invoice paid in a third statement. A failure between any of them lost money or left a paid invoice unflagged.

Transactions committed and rolled back without awaiting. PerformTransaction, DepositMoney, and WithdrawMoney leaned on connection disposal to commit, and disposal fired the commit without awaiting it before releasing the connection back to the pool. The rollbacks had the same race.

Disposal committed whatever was still open. If a function threw mid-transaction, the connection wrapper committed the half-finished transaction on disposal instead of rolling it back.

What changed

  • UpdateBalance wraps its balance update and ledger insert in one transaction.
  • DepositMoney, WithdrawMoney, PerformTransaction, and InsertGroup commit explicitly instead of relying on disposal.
  • Every rollback is awaited.
  • Connection disposal now rolls back any still-open transaction and awaits both the rollback and the release. Functions that open a transaction use await using so this runs as async cleanup (server/db/index.ts).
  • UpdateInvoice and PerformTransaction share one ApplyTransfer helper, so the invoice's money move and its paid flag commit together in a single transaction:
await conn.beginTransaction();

try {
  const transferred = await ApplyTransfer(conn, invoice.toAccount, invoice.fromAccount, invoice.amount, false, fromBalance, toBalance, locales('invoice_payment'), undefined, charId);
  if (!transferred) { await conn.rollback(); return { success: false, message: 'no_balance' }; }

  const invoiceUpdated = await conn.update('UPDATE `accounts_invoices` SET `payerId` = ?, `paidAt` = ? WHERE `id` = ?', [player.charId, new Date(), invoiceId]);
  if (!invoiceUpdated) { await conn.rollback(); return { success: false, message: 'invoice_not_updated' }; }

  await conn.commit();
} catch (e) {
  await conn.rollback();
  return { success: false, message: 'something_went_wrong' };
}

The invoice ledger is now a single transfer row and one ox:transferredMoney event, same as a normal transfer. ox:invoicePaid still fires.

@Kenshiin13 Kenshiin13 requested a review from thelindat June 8, 2026 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant