Skip to content

fix(maintenance-pool): wrap deposit() in dataSource.transaction() to resolve PessimisticLockTransactionRequiredError (#457) - #478

Open
PINYOPATTANAWASANPORN wants to merge 1 commit into
MergeFi:mainfrom
PINYOPATTANAWASANPORN:fix/maintenance-pool-deposit-transaction-457
Open

PINYOPATTANAWASANPORN wants to merge 1 commit into
MergeFi:mainfrom
PINYOPATTANAWASANPORN:fix/maintenance-pool-deposit-transaction-457

Conversation

@PINYOPATTANAWASANPORN

Copy link
Copy Markdown

Summary of Changes

Wraps the entire MaintenancePoolService.deposit() in dataSource.transaction() so the SELECT … FOR UPDATE pessimistic lock executes inside an active transaction.

Root Cause (#457)

The #275 fix added setLock('pessimistic_write') to the pool query inside deposit(), but nothing wraps the method in a transaction. TypeORM's SelectQueryBuilder.executeEntitiesAndRawResults guards:

if (this.expressionMap.lockMode === "pessimistic_write" && !queryRunner.isTransactionActive)
  throw new PessimisticLockTransactionRequiredError();

Every POST /maintenance-pool/:id/deposit therefore 500s before reaching any business logic, making the endpoint completely non-functional.

Fix

- async deposit(id, amount, funderAddress): Promise<MaintenancePool> {
-   const pool = await this.poolRepo
-     .createQueryBuilder('pool')
-     .setLock('pessimistic_write')
-     .where('pool.id = :id', { id })
-     .getOne();
+ async deposit(id, amount, funderAddress): Promise<MaintenancePool> {
+   return this.dataSource.transaction(async (manager) => {
+     const pool = await manager
+       .createQueryBuilder(MaintenancePool, 'pool')
+       .setLock('pessimistic_write')
+       .where('pool.id = :id', { id })
+       .getOne();

All subsequent poolRepo.update / poolRepo.increment / findOne calls inside the method are replaced with manager.* equivalents so all operations share the same connection and transaction boundary.

Verification & Testing

5 unit tests added to maintenance-pool.service.spec.ts:

  1. wraps deposit() in dataSource.transaction() — dataSource.transaction called once
  2. issues SELECT … FOR UPDATE (pessimistic_write) on the manager QueryBuilder — setLock('pessimistic_write') asserted
  3. throws NotFoundException when pool is not found inside the transaction
  4. throws BadRequestException for a non-ACTIVE pool
  5. calls escrowService.fund on first deposit (escrowId is null)

Impact & Compatibility

  • Breaking changes: None — deposit() public signature is unchanged.
  • DataSource is injected via @InjectDataSource(), already available in the NestJS DI container.

Closes #457

…rgeFi#457)

SELECT FOR UPDATE (pessimistic_write) on a plain injected Repository throws
PessimisticLockTransactionRequiredError because TypeORM refuses pessimistic
locks without an active transaction. Wrapping deposit() in
dataSource.transaction() provides an active QueryRunner before the locked
query executes.

All operations inside the transaction now use the EntityManager API
(manager.createQueryBuilder, manager.update, manager.increment,
manager.findOneByOrFail) to share the same connection and transaction.

Tests added: 5 unit tests verifying transaction wrapping, SELECT FOR UPDATE
lock, NotFoundException, BadRequestException, and escrow creation paths.
@vercel

vercel Bot commented Oct 1, 2026

Copy link
Copy Markdown

@PINYOPATTANAWASANPORN is attempting to deploy a commit to the chonilius' projects Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
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.

MaintenancePoolService.deposit() always throws PessimisticLockTransactionRequiredError — the #275 SELECT ... FOR UPDATE runs outside any transaction

1 participant