assignReward decrements MaintenancePool.balance before poolWithdraw() and never restores it when the withdrawal fails

Open
  • Is something wrong?
#459 5 comments 0 reactions 1 assignee View on GitHub

Maintainers usually reply within 1 day

@Fabulouz34 is already working on this.

Since Sep 29, 2026.

  • #471 by @Proxima84-code — open
  • #475 by @Fabulouz34 — open

Assessment

This issue has not been assessed yet.

Description

architecture bug Stellar Wave very hard

Problem

In MaintenancePoolService.assignReward(), the #274 atomic check-and-decrement runs before the on-chain payout:

const updateResult = await this.poolRepo
  .createQueryBuilder()
  .update(MaintenancePool)
  .set({ balance: () => `balance - :amount` })
  .where('id = :id AND balance >= :amount', { id, amount: Number(amount) })
  ...
  .execute();
if (updateResult.affected === 0) throw new BadRequestException(...);

const payment = await this.escrowService.poolWithdraw(pool.escrowId, amount, recipientAddress, recipientId);
return payment;

poolWithdraw() has several ways to throw after the balance has already been reduced:

  • assertLocked(escrow)
  • assertValidAmount(amount)
  • assertRecipientsMatchUsers(...): a recipientAddress that doesn't match recipientId's wallet
  • a Soroban simulate, send or poll failure inside invokeOnLockedEscrow
  • the final paymentRepo.save

None of these paths gives the reserved amount back, so every failed reward permanently shrinks the pool's DB balance, even though no USDC left the contract. Over time MaintenancePool.balance drifts below the real on-chain balance. Rewards then start failing with "exceeds pool balance" while funds are still in escrow, and there's no admin path to reconcile.

Suggested fix

  • Wrap the payout in try/catch. On failure, restore the reservation with increment({ id }, 'balance', Number(amount)) and rethrow.
  • Only do that when the on-chain call is known not to have gone through. invokeOnLockedEscrow's state distinguishes "never submitted" from "submitted, outcome unknown".
  • Move the cheap validation (recipient/user match, amount) before the decrement so bad input never touches the balance.
  • Add tests with poolWithdraw rejecting that assert the pool balance is unchanged.

Related: #51, #274 (introduced the reserve-first ordering), #163.

Dominant language
TypeScript
Stars
2
Forks
90
Avg merge
31m
Merged PRs (30d)
20

Getting set up

  • Ships a Dockerfile or Docker Compose file
  • No pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MergeFi/backend

All issues in MergeFi/backend

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.