[Regression on closed #273] assignReward's double-payout guard is keyed on (pool, recipient) instead of (pool, issue) — the same issue can still be paid twice

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

Maintainers usually reply within 1 day

@MrOmale is already working on this.

Since Oct 21, 2026.

  • #472 by @Proxima84-code — open
  • #473 by @Ultra-Tech-code — open
  • #479 by @PINYOPATTANAWASANPORN — open
  • #715 by @MrOmale — open

Assessment

This issue has not been assessed yet.

Description

bug security Stellar Wave very hard

Problem

#273 ("no guard against being called twice for the same issueId") was closed with this check in MaintenancePoolService.assignReward():

// Guard against double/triple payout for the same issue (#273).
const existingPayment = await this.paymentRepo.findOne({
  where: { recipientId: recipientId ?? null },
});
const existingPoolPayment = await this.paymentRepo
  .createQueryBuilder('payment')
  .innerJoin('payment.escrow', 'escrow')
  .where('escrow.maintenancePoolId = :poolId', { poolId: id })
  .andWhere('payment.recipientId = :recipientId', { recipientId: recipientId ?? null })
  .getOne();
if (existingPoolPayment) {
  throw new ConflictException(`Issue ${issueId} has already received a reward from pool ${id}`);
}

issueId doesn't appear in either query, and Payment has no issue column at all. The guard checks who was paid from this pool, not which issue was rewarded:

  1. The same issue can still be paid twice. Call assignReward(pool, issue#1, …, recipientA) and then assignReward(pool, issue#1, …, recipientB), and both succeed. This is the double payout #273 asked to prevent.
  2. Anonymous payouts are never guarded. When recipientId is omitted, the query becomes payment.recipientId = NULL, which is never true in SQL. So repeated rewards for the same issue with no recipientId always pass.
  3. Legitimate rewards are rejected. A maintainer who fixes two different maintenance issues gets a 409 on the second one, with a misleading message saying that issue was already rewarded.
  4. existingPayment is dead code. It's an unscoped query across every payment in the system, whose result is never read, and it costs an extra DB round trip per call.

Suggested fix

  • Record the rewarded issue on the payment, for example a nullable Payment.maintenanceIssueId (FK to issues), plus a unique index on (escrowId, maintenanceIssueId).
  • Guard on (pool escrow, issueId), and let the unique index close the concurrent-call race.
  • Remove the unused existingPayment query.
  • Add tests: the same issue with two recipients gets a 409, and two issues for the same recipient both succeed.

Related: #273, #274, #276.

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.