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

Abierto
  • ¿Algo no está bien?
#459 5 comentarios 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@Fabulouz34 ya está trabajando en esto.

Desde el 29/9/2026.

  • #471 de @Proxima84-code — abierto
  • #475 de @Fabulouz34 — abierto

Evaluación

Este issue todavía no se ha evaluado.

Descripción

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.

Lenguaje dominante
TypeScript
Estrellas
2
Forks
90
Merge medio
31 min
PR fusionados (30 d)
20

Preparar el entorno

  • Incluye un Dockerfile o un archivo de Docker Compose
  • Sin plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/backend

Todos los issues de MergeFi/backend

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.