Problem
The #275 fix made MaintenancePoolService.deposit() (src/maintenance-pool/maintenance-pool.service.ts) load the pool with a pessimistic lock:
const pool = await this.poolRepo
.createQueryBuilder('pool')
.setLock('pessimistic_write')
.where('pool.id = :id', { id })
.getOne();poolRepo is the plain injected repository. Nothing wraps deposit() in a transaction: there's no dataSource.transaction(...), no QueryRunner and no transactional decorator anywhere in src/. TypeORM refuses to run a pessimistic lock outside a transaction. In the locked version (package-lock.json → typeorm@1.0.0), SelectQueryBuilder.executeEntitiesAndRawResults, which getOne() goes through, does:
if ((this.expressionMap.lockMode === "pessimistic_read" ||
this.expressionMap.lockMode === "pessimistic_write" || ...) &&
!queryRunner.isTransactionActive)
throw new PessimisticLockTransactionRequiredError();Impact
- Every
POST /maintenance-pool/:id/deposit fails with a 500 before reaching any business logic, including the first deposit into a brand-new pool.
- Because nothing can be deposited, no pool can ever get an escrow or a balance, so
assignReward() is unreachable too. In practice the whole maintenance-pool feature is non-functional.
maintenance-pool.service.spec.ts mocks createQueryBuilder ("Default mock for deposit's createQueryBuilder (SELECT ... FOR UPDATE)"), so the unit tests can't see this.
Even if the lock worked, it would be released immediately in autocommit mode. It could never cover the later escrowService.fund(), update(... escrowId IS NULL ...) and increment() calls, which is what #275 needs.
Suggested fix
- Run the read-lock-fund-update sequence inside
this.dataSource.transaction(async (manager) => { ... }), using manager.getRepository(MaintenancePool) for the locked read and the writes.
- Or drop the pessimistic lock entirely and rely on the existing conditional
update({ id, escrowId: IsNull() }, ...) compare-and-set, which already detects the race on its own.
- Add an integration test against a real Postgres (like
escrow-fk-integrity.integration.spec.ts) that calls deposit() twice and asserts both succeed.
Related: #275 (introduced the lock), #48, #339.
Problem
The #275 fix made
MaintenancePoolService.deposit()(src/maintenance-pool/maintenance-pool.service.ts) load the pool with a pessimistic lock:poolRepois the plain injected repository. Nothing wrapsdeposit()in a transaction: there's nodataSource.transaction(...), noQueryRunnerand no transactional decorator anywhere insrc/. TypeORM refuses to run a pessimistic lock outside a transaction. In the locked version (package-lock.json→typeorm@1.0.0),SelectQueryBuilder.executeEntitiesAndRawResults, whichgetOne()goes through, does:Impact
POST /maintenance-pool/:id/depositfails with a 500 before reaching any business logic, including the first deposit into a brand-new pool.assignReward()is unreachable too. In practice the whole maintenance-pool feature is non-functional.maintenance-pool.service.spec.tsmockscreateQueryBuilder("Default mock for deposit's createQueryBuilder (SELECT ... FOR UPDATE)"), so the unit tests can't see this.Even if the lock worked, it would be released immediately in autocommit mode. It could never cover the later
escrowService.fund(),update(... escrowId IS NULL ...)andincrement()calls, which is what #275 needs.Suggested fix
this.dataSource.transaction(async (manager) => { ... }), usingmanager.getRepository(MaintenancePool)for the locked read and the writes.update({ id, escrowId: IsNull() }, ...)compare-and-set, which already detects the race on its own.escrow-fk-integrity.integration.spec.ts) that callsdeposit()twice and asserts both succeed.Related: #275 (introduced the lock), #48, #339.