Skip to content

fix(insurance): atomic payouts and transactional multi-tier coverage - #20

Merged
ameeribro4-sudo merged 2 commits into
OpenPeerX:mainfrom
Xhristin3:fix/issue-13-insurance-fund-atomic-payouts
Aug 20, 2026
Merged

fix(insurance): atomic payouts and transactional multi-tier coverage#20
ameeribro4-sudo merged 2 commits into
OpenPeerX:mainfrom
Xhristin3:fix/issue-13-insurance-fund-atomic-payouts

Conversation

@Xhristin3

Copy link
Copy Markdown
Contributor

Summary

Closes #13

Insurance fund payouts can no longer double-spend a balance. recordTransaction() applies every payout as a single conditional UPDATE insurance_funds SET balance = balance - :amount WHERE id = :id AND balance >= :amount and rejects the call when zero rows are affected, and coverShortfall() debits tiers in fixed priority order inside one DataSource transaction, so concurrent liquidations serialize on fund row locks instead of racing a read-modify-write. The single most important design decision: the issue offered two acceptable primitives (pessimistic_write locks vs an atomic conditional UPDATE) — this PR uses the atomic UPDATE, because it is driver-agnostic (SQLite — the repo's test harness — rejects pessimistic_write), never holds a row lock across the Stellar network call, and is exactly the primitive the two concurrent-payout acceptance criteria demand.

Why

The old path did getFund() → check balance >= amount in application memory → fund.balance = balanceAfter; save(fund) — a full-row overwrite with no lock and no transaction. Two concurrent liquidations (the engine polls every 5s) could both read the same balance, both pass the check, and both overwrite it, paying out the same funds twice. coverShortfall() made it worse by wrapping each payout in try { ... } catch { continue; }, silently turning Insufficient fund balance for payout into a partially-covered liquidation with no signal. This fix closes the race at the database (the guard is evaluated under the row lock) and converts partial coverage into an explicit outcome (event + status field), and it deliberately reuses the existing margin-module discipline (dataSource.transaction(...)) rather than introducing a second mechanism.

What was built

src/protection/services/:

File What it contains
insurance-fund.service.ts Atomic conditional debit/credit helpers (balance -/+ :amount with an affected-row check), a manager-aware recordTransaction(fundId, type, amount, options, manager?) that joins an outer transaction when one is passed and otherwise runs the debit + audit row + health refresh in a transaction of its own, plus getFund(fundId, manager?) for race retries. Matching tests: insurance-fund.service.spec.ts.
liquidation-protection.service.ts coverShortfall() rewritten: advisory fund reads before the transaction, per-tier guarded debits with one bounded retry after a re-read of the committed balance, single-transaction persistence of the LiquidationEvent, post-commit health recalculation and events, and a narrow NotFoundException skip for uninitialized tiers (everything else propagates). Matching tests: liquidation-protection.service.spec.ts.

New tests:

File What it contains
insurance-fund-concurrency.spec.ts Two real SQLite connections on one file database (WAL mode — SQLite's MVCC, modeling Postgres row locks). Proves two concurrent 1000-payouts against a 1500 balance yield exactly one success + one BadRequestException + balance 500 + exactly one payout record, and that two concurrent coverShortfall(5000) calls against 10000 total capacity both succeed with no deadlock and no negative balance.

Integration changes outside services/

  • src/protection/insurance-fund.integration.spec.ts — rewritten from hand-rolled in-memory mocks to a real SQLite DataSource (TypeOrmModule), preserving the lifecycle and health-alert tests and adding a real-DB partial-coverage test. This was unavoidable: the old mock harness could not exercise real transactions or the atomic UPDATE path.
  • README.md — new "Insurance fund concurrency model" section documenting where locks live and why (per the issue's documentation criterion).
  • src/margin/services/liquidation-engine.service.ts — listed in the issue's "files in scope" but intentionally untouched: it already logs coverage failures, and none of the acceptance criteria require engine changes.

Acceptance criteria coverage

Service

  • Two concurrent payouts that would exceed the fund balance result in exactly one success and one rejection, with no negative balance. (insurance-fund-concurrency.spec.ts — double-spend test; the same test fails against the base implementation, where both payouts succeed)
  • coverShortfall debits multiple tiers atomically and does not deadlock under concurrent liquidations. (single dataSource.transaction + fixed TIER_PRIORITY lock order; insurance-fund-concurrency.spec.ts — concurrent coverShortfall test with both calls covered, balances ending at 0, none negative)
  • Partial coverage is reported explicitly rather than silently swallowed. (LiquidationEvent status: 'PARTIAL' + liquidation.shortfall event with cascadePrevented: false; the blanket catch { continue; } is replaced by a narrow NotFoundException skip — every other failure propagates)

Tests

  • A test simulates concurrent recordTransaction payouts against the same fund and asserts the fund is never double-spent. (insurance-fund-concurrency.spec.ts — balance 1500, two concurrent 1000 payouts, asserts exactly one success, one rejection, final balance 500, one payout record)
  • A test covers the partial-coverage path when the fund balance is insufficient. (insurance-fund.integration.spec.ts real-DB test: LOW 3000 + MEDIUM 1000, 10000 shortfall → covered 4000, remaining 6000, PARTIAL, events emitted; plus the mock-suite insufficient-payout rejection test)

Documentation

  • The concurrency guarantees of the insurance fund are documented (where locks live and why), so future payouts follow the same pattern. (README.md — "Insurance fund concurrency model"; also doc-comments on recordTransaction and coverShortfall)

Deliberately deferred

  • Liquidation/coverage boundary atomicity (issue's hard problem deps: bump @types/node from 22.19.21 to 26.1.1 #3): a crash between liquidatePosition() committing and coverShortfall() running leaves a liquidated position without coverage and no retry path. Closing that requires either moving coverage inside the margin position transaction or a durable reconciliation job — a stateful change to the margin module that none of this issue's acceptance criteria require. The atomic, transactional coverage built here is the prerequisite. Happy to align on the approach (same-transaction vs reconciliation) before a follow-up PR.

Test plan

  • npm run build — succeeds
  • npm test — 454/506 passing, 52 failing; the 52 failing are byte-identical to the base branch (pre-existing ts-jest resolution of absolute src/... imports — verified by diffing the failing-suite lists with the changes stashed). 4 net new tests added and passing.
  • npx eslint on changed files — strictly fewer errors than base: insurance-fund.service.spec.ts 13→11, liquidation-protection.service.spec.ts 2→2, insurance-fund.integration.spec.ts 32→6, new concurrency spec 0, both service files 0
  • npx madge --circular — same 3 pre-existing cycles as base, none involving protection
  • npx jest src/protection — 20/20 passing (incl. concurrency suite run 5× for timing stability)
  • Manual: none — concurrency verified automatically against two real SQLite connections

Env vars / Notes

No new env vars or config keys. The concurrency spec writes a throwaway SQLite file under os.tmpdir() and deletes it in afterAll. One sqlite-specific finding worth recording: a read inside an open transaction before any write makes SQLite return SQLITE_BUSY immediately when another writer is active (both rollback-journal and WAL); coverShortfall therefore performs its advisory reads before the transaction, which also matches Postgres' MVCC behavior. Postgres row locks make the same interleaving safe in production.

…sactional

Payouts previously used a lock-free read-modify-write, so concurrent
liquidations could pass the balance check twice and double-spend a fund.
recordTransaction now applies every balance change as a single conditional
UPDATE (balance = balance - :amount WHERE balance >= :amount) with an
affected-row check, and coverShortfall debits tiers in fixed order inside
one transaction so concurrent liquidations serialize on row locks instead
of deadlocking. Partial coverage is reported explicitly (status PARTIAL +
cascadePrevented: false event) instead of being swallowed by catch-all
error handling; health and events run only after commit.

Co-Authored-By: Xhristin3 <208627422+Xhristin3@users.noreply.github.com>

@ameeribro4-sudo ameeribro4-sudo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ameeribro4-sudo
ameeribro4-sudo merged commit 733cd3c into OpenPeerX:main Aug 20, 2026
4 checks passed
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.

Insurance fund payouts are non-atomic: concurrent liquidations can double-spend the fund

2 participants