BountiesService.expireOverdue() overwrites concurrently-advanced bounties to EXPIRED — read-then-save with no status re-check and no assertTransition

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

Los mantenedores suelen responder en 1 día

@Youngthreader ya está trabajando en esto.

Desde el 21/10/2026.

  • #468 de @Proxima84-code — abierto
  • #473 de @Ultra-Tech-code — abierto
  • #717 de @Youngthreader — abierto

Evaluación

Este issue todavía no se ha evaluado.

Descripción

architecture bug Stellar Wave very hard

Problem

BountiesService.expireOverdue(), run hourly by BountyExpiryScheduler, loads every overdue OPEN, FUNDED or CLAIMED bounty and then saves them one at a time:

const overdue = await this.bountyRepo.createQueryBuilder('bounty')
  .where('bounty.deadline IS NOT NULL AND bounty.deadline < :now', { now: new Date() })
  .andWhere('bounty.status IN (:...statuses)', { statuses: [OPEN, FUNDED, CLAIMED] })
  .getMany();

for (const bounty of overdue) {
  bounty.status = BountyStatus.EXPIRED;
  await this.bountyRepo.save(bounty);
}

The status filter only applies at read time. Between the SELECT and each save(), and across a loop of per-row awaits that can take a while on a large backlog, a webhook can move the same bounty forward: CLAIMED → IN_REVIEW when a PR opens (#168), or on to MERGED and PAID. The save() then issues UPDATE ... SET status='expired' WHERE id=... from the stale in-memory copy and clobbers the newer status.

  • The loop also skips assertTransition(), unlike every other status change in BountiesService, so it doesn't consult the state machine (IN_REVIEW → EXPIRED isn't a valid transition).
  • A bounty that has just been merged and paid can be flipped to EXPIRED. Since EXPIRED → REFUNDED is allowed, the sponsor can then also refund a bounty that was already paid out.

Suggested fix

Replace the loop with one conditional update, so the status check and the write are atomic:

const result = await this.bountyRepo.createQueryBuilder()
  .update(Bounty)
  .set({ status: BountyStatus.EXPIRED })
  .where('deadline IS NOT NULL AND deadline < :now', { now })
  .andWhere('status IN (:...statuses)', { statuses: [OPEN, FUNDED, CLAIMED] })
  .execute();
return result.affected ?? 0;

Or, per row: update({ id, status: In([...]) }, { status: EXPIRED }) and count affected. Add a test where the bounty's status changes between the read and the write, and it isn't expired.

Related: #152, #266, #337, #347.

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.