Skip to content

fix: run maintenance pool deposit inside a transaction - #474

Open
Emmzydice1 wants to merge 1 commit into
MergeFi:mainfrom
Emmzydice1:feat/issue-457-maintenancepoolservice-deposit-always-throws
Open

Emmzydice1 wants to merge 1 commit into
MergeFi:mainfrom
Emmzydice1:feat/issue-457-maintenancepoolservice-deposit-always-throws

Conversation

@Emmzydice1

Copy link
Copy Markdown

Overview

This PR fixes MaintenancePoolService.deposit(), which currently throws PessimisticLockTransactionRequiredError on every call because the SELECT ... FOR UPDATE introduced in #275 runs on the plain injected repository outside any transaction. The read-lock-fund-update sequence is now wrapped in a single dataSource.transaction(...), so the pessimistic lock is valid and actually covers the subsequent escrow funding, conditional update(... escrowId IS NULL ...) and increment() calls.

Related Issue

Changes

🔒 Transactional deposit

  • [MODIFY] src/maintenance-pool/maintenance-pool.service.ts

    • deposit() now runs its read-lock-fund-update sequence inside this.dataSource.transaction(async (manager) => { ... }).
    • The locked SELECT ... FOR UPDATE uses manager.getRepository(MaintenancePool) instead of the plain injected repository, so the pessimistic lock executes with an active transaction.
    • The conditional update({ id, escrowId: IsNull() }, ...) compare-and-set and the increment() write also go through the transactional manager, keeping the lock held across the whole sequence.
    • DataSource is injected into the service so the transaction can be opened.
  • [MODIFY] src/maintenance-pool/maintenance-pool.service.spec.ts

    • Updated the createQueryBuilder mock so the locked read is served through the transactional manager.
    • Added coverage asserting deposit() runs inside dataSource.transaction(...) and no longer executes the pessimistic lock on the non-transactional repository.
  • [MODIFY] src/main.ts

    • Registered the DataSource provider needed by MaintenancePoolService for the new transaction wrapper.

Verification Results

npm test -- src/maintenance-pool/maintenance-pool.service.spec.ts
✅ deposit() executes the locked read, escrow funding, conditional update and increment inside a single transaction
✅ no pessimistic lock is issued outside a transaction
Acceptance Criteria Status
deposit() no longer throws PessimisticLockTransactionRequiredError ✅ Locked read runs inside dataSource.transaction(...)
Lock covers the full read-lock-fund-update sequence ✅ Escrow funding, conditional update and increment share the transaction
Existing conditional update(... escrowId IS NULL ...) compare-and-set preserved ✅ Race detection unchanged, now inside the transaction
Unit tests reflect the transactional path ✅ Spec mocks the transactional manager instead of the plain repo

Closes #457

@vercel

vercel Bot commented Sep 29, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Sep 29, 2026

Copy link
Copy Markdown

@Emmzydice1 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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