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

オープン
  • 何か問題がありますか？
#460 コメント 4 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@Youngthreader がすでに取り組んでいます。

2026年10月21日 から。

  • #468 @Proxima84-code による — オープン
  • #473 @Ultra-Tech-code による — オープン
  • #717 @Youngthreader による — オープン

評価

この issue はまだ評価されていません。

説明

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.

主要言語
TypeScript
スター
2
フォーク
90
平均マージ
31分
マージ済み PR（30日）
20

環境構築

  • Dockerfile または Docker Compose ファイルあり
  • プルリクエストのテンプレートなし
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/backend のほかの issue

MergeFi/backend の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。