[Regression on closed #102 / #367] markMergedAndRelease marks a team bounty PAID when the team has no splits or no longer exists, skipping the payout entirely
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 初心者へのやさしさ
- 72/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- node.js, typescript
調査の方向性
Start with BountiesService.markMergedAndRelease in src/bounties/bounties.service.ts and the existing cases in src/bounties/bounties.service.spec.ts. Check how skipped payouts are handled by the RELEASE_FAILED transition in src/bounties/bounty-state-machine.ts. Add coverage for empty team splits, a missing team, and a bounty with no recipient; done when each case leaves the bounty retryable and does not mark it PAID.
索引モデルが issue の本文から書いたものです。
説明
Location
src/bounties/bounties.service.ts:191-238 — BountiesService.markMergedAndRelease
try {
if (bounty.teamId) {
const team = await this.teamRepo.findOne({
where: { id: bounty.teamId },
relations: { splits: true },
});
if (team && team.splits.length > 0) {
...
await this.escrowService.splitRelease(bounty.escrowId, recipients);
}
} else if (bounty.claimedById) {
...
await this.escrowService.release(bounty.escrowId, contributor?.stellarAddress ?? '', bounty.claimedById);
}
} catch (err) { ... }
assertTransition(bounty.status, BountyStatus.PAID);
bounty.status = BountyStatus.PAID;
bounty.paidAt = new Date();
Problem
The if (team && team.splits.length > 0) guard has no else. When it is false, the whole payout is skipped, no exception is thrown, control falls through to line 233, and the bounty is written as PAID with paidAt set — while the escrow is still LOCKED and no Payment row exists.
Three distinct inputs reach that branch:
team.splitsis empty — a team created or edited into a zero-split state.teamisnull—bounty.teamIdpoints at a row that no longer exists.Bounty.teamIdis a nullable FK with no cascade back to the bounty, so a deleted team leaves the id dangling.- The
else if (bounty.claimedById)arm is likewise silently skipped when a bounty has neitherteamIdnorclaimedById— reachable becauseIN_REVIEWis driven by webhook from PR linkage, not by a claim (see #385).
This is the defect closed as #102 and re-filed and closed as #367. The fix is not present on main.
Why it matters
PAID is terminal in the state machine — BOUNTY_TRANSITIONS[BountyStatus.PAID] = [] (src/bounties/bounty-state-machine.ts:38). So the bounty is unrecoverable through the API:
- it cannot be retried (
PAID -> MERGEDis not a legal transition, andmarkMergedAndReleasewould reject at its ownassertTransition(PAID, MERGED)on line 179), - it cannot be refunded (
PAID -> REFUNDEDis not legal either), - the escrow stays
LOCKEDindefinitely, so the sponsor's funds are stranded on-chain with no route that can move them.
Meanwhile every downstream consumer reads the bounty as settled: PAID + paidAt is what the sponsor dashboard's spend aggregate, the analytics payout figures, and reputation all key off. So the platform reports the money as paid out to the team while the contributors received nothing and the sponsor cannot get it back. The contributor has done the work, the PR is merged on GitHub, and there is no state left from which to pay them.
The RELEASE_FAILED escape hatch that exists precisely for this situation (line 227-230, and RELEASE_FAILED -> MERGED retry on bounty-state-machine.ts:37) is never reached, because nothing threw — the payout was skipped rather than attempted.
There is no test coverage for the branch: src/bounties/bounties.service.spec.ts exercises teamId: null (line 185) and teamId: 'team-1' with populated splits (line 208), but nothing with an empty splits array or a missing team.
Reproduction / failure path
Empty splits
POST /teamscreating teamT, then remove its splits (or create it in a state wheresplitsis empty —TeamsService.updateSplits' delete-then-insert window also exposes this, cf. #375/#536).POST /bounties→B;POST /bounties/B/fund→ escrowELOCKED; assignTtoBsoB.teamId = T.- Claim, open a PR, merge it. The webhook drives
markMergedAndRelease(B). team.splits.length === 0→splitReleaseis never called.- Result:
B.status = 'paid',B.paidAtset,E.status = 'LOCKED', zeroPaymentrows forE.GET /bounties/Breports a paid bounty.
Missing team
- Same setup, then delete team
Tfrom theteamstable (directly, or via any future delete endpoint). - Merge the PR →
teamRepo.findOnereturnsnull→ same outcome.
Suggested fix
Make the unpayable case an explicit failure rather than a fall-through, so the existing RELEASE_FAILED retry path handles it:
if (bounty.teamId) {
const team = await this.teamRepo.findOne({
where: { id: bounty.teamId },
relations: { splits: true },
});
if (!team) {
throw new BadRequestException(
`Bounty ${bounty.id} references team ${bounty.teamId}, which no longer exists`,
);
}
if (team.splits.length === 0) {
throw new BadRequestException(
`Team ${team.id} has no configured splits — cannot release bounty ${bounty.id}`,
);
}
...
await this.escrowService.splitRelease(bounty.escrowId, recipients);
} else if (bounty.claimedById) {
...
} else {
throw new BadRequestException(
`Bounty ${bounty.id} has neither a team nor a claimant — no payout recipient`,
);
}
Because these throws happen inside the existing try, the bounty lands in RELEASE_FAILED (not PAID), stays refundable and retryable, and the escrow is still releasable once the team's splits are repaired.
Additionally, guard at the source so the bad state is harder to create: reject assigning a team with zero splits to a bounty in TeamsService.assignToBounty, and require splits.length > 0 whenever a team's splits are rewritten.
Tests to add in src/bounties/bounties.service.spec.ts: merge a team bounty whose splits is [], merge one whose teamRepo.findOne resolves null, and merge one with neither teamId nor claimedById — each asserting the bounty ends in RELEASE_FAILED, that escrowService.splitRelease/release was not called, and that PAID was never written.
- 主要言語
- TypeScript
- スター
- 2
- フォーク
- 90
- 平均マージ
- 31分
- マージ済み PR(30日)
- 20
環境構築
- Dockerfile または Docker Compose ファイルあり
- プルリクエストのテンプレートなし
- コントリビューションガイドなし
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
MergeFi/backend のほかの issue
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
メンテナーはふだん 1 日以内に返信
-
bug security
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
メンテナーはふだん 1 日以内に返信
-
bug testing
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
メンテナーはふだん 1 日以内に返信
-
bug security
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
メンテナーはふだん 1 日以内に返信
MergeFi/backend の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
zapcooking/frontend#946 ·
メンテナーはふだん 1 日以内に返信
-
enhancement good first issue
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
WiliamRosa/cosmos-semantic-platform#40 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 77/100
liberation-data/drivine#733 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 64/100
makecindy/cindy#10062 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
メンテナーはふだん 1 日以内に返信