diff --git a/src/analytics/analytics.service.ts b/src/analytics/analytics.service.ts index 427c778..f98409b 100644 --- a/src/analytics/analytics.service.ts +++ b/src/analytics/analytics.service.ts @@ -34,7 +34,7 @@ export class AnalyticsService { }); const paid = claimed.filter((b) => b.status === BountyStatus.PAID); const merged = claimed.filter((b) => - [BountyStatus.MERGED, BountyStatus.PAID].includes(b.status), + [BountyStatus.MERGED, BountyStatus.RELEASE_PENDING, BountyStatus.PAID].includes(b.status), ); const lifetimeEarnings = paid.reduce((sum, b) => sum + Number(b.amount), 0); diff --git a/src/bounties/bounties.service.spec.ts b/src/bounties/bounties.service.spec.ts index fd5a497..f73865a 100644 --- a/src/bounties/bounties.service.spec.ts +++ b/src/bounties/bounties.service.spec.ts @@ -13,6 +13,7 @@ describe('BountiesService', () => { fund: jest.Mock; release: jest.Mock; splitRelease: jest.Mock; + refund: jest.Mock; }; beforeEach(async () => { @@ -29,6 +30,7 @@ describe('BountiesService', () => { fund: jest.fn().mockResolvedValue({ id: 'escrow-1', status: 'locked' }), release: jest.fn().mockResolvedValue(undefined), splitRelease: jest.fn().mockResolvedValue([]), + refund: jest.fn().mockResolvedValue(undefined), }; const module: TestingModule = await Test.createTestingModule({ @@ -143,4 +145,91 @@ describe('BountiesService', () => { ); expect(bounty.status).toBe(BountyStatus.PAID); }); + + it('markMergedAndRelease moves to RELEASE_PENDING when escrow fails, allowing retry', async () => { + bountyRepo.findOne.mockResolvedValue({ + id: 'b1', + status: BountyStatus.IN_REVIEW, + escrowId: 'escrow-1', + claimedById: 'contributor-1', + teamId: null, + }); + + escrowService.release.mockRejectedValue(new Error('Soroban RPC timeout')); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + BountiesService, + { provide: getRepositoryToken(Bounty), useValue: bountyRepo }, + { + provide: getRepositoryToken(User), + useValue: { + findOne: jest.fn().mockResolvedValue({ + id: 'contributor-1', + stellarAddress: 'GCONTRIB', + }), + }, + }, + { provide: getRepositoryToken(Team), useValue: { findOne: jest.fn() } }, + { provide: EscrowService, useValue: escrowService }, + ], + }).compile(); + service = module.get(BountiesService); + + const bounty = await service.markMergedAndRelease('b1'); + + expect(bounty.status).toBe(BountyStatus.RELEASE_PENDING); + expect(escrowService.release).toHaveBeenCalled(); + }); + + it('allows retrying escrow release from RELEASE_PENDING state', async () => { + bountyRepo.findOne.mockResolvedValue({ + id: 'b1', + status: BountyStatus.RELEASE_PENDING, + escrowId: 'escrow-1', + claimedById: 'contributor-1', + teamId: null, + }); + + escrowService.release.mockResolvedValue(undefined); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + BountiesService, + { provide: getRepositoryToken(Bounty), useValue: bountyRepo }, + { + provide: getRepositoryToken(User), + useValue: { + findOne: jest.fn().mockResolvedValue({ + id: 'contributor-1', + stellarAddress: 'GCONTRIB', + }), + }, + }, + { provide: getRepositoryToken(Team), useValue: { findOne: jest.fn() } }, + { provide: EscrowService, useValue: escrowService }, + }, + }).compile(); + service = module.get(BountiesService); + + const bounty = await service.markMergedAndRelease('b1'); + + expect(bounty.status).toBe(BountyStatus.PAID); + expect(escrowService.release).toHaveBeenCalled(); + }); + + it('moves unfunded bounty directly to PAID when merged', async () => { + bountyRepo.findOne.mockResolvedValue({ + id: 'b1', + status: BountyStatus.IN_REVIEW, + escrowId: null, + claimedById: null, + teamId: null, + }); + + const bounty = await service.markMergedAndRelease('b1'); + + expect(bounty.status).toBe(BountyStatus.PAID); + expect(escrowService.release).not.toHaveBeenCalled(); + }); }); diff --git a/src/bounties/bounties.service.ts b/src/bounties/bounties.service.ts index 7bf75f8..35d596c 100644 --- a/src/bounties/bounties.service.ts +++ b/src/bounties/bounties.service.ts @@ -1,4 +1,4 @@ -import { Injectable, NotFoundException } from '@nestjs/common'; +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { Bounty, Team, User } from '../common/entities'; @@ -9,6 +9,8 @@ import { CreateBountyDto } from './dto/create-bounty.dto'; @Injectable() export class BountiesService { + private readonly logger = new Logger(BountiesService.name); + constructor( @InjectRepository(Bounty) private readonly bountyRepo: Repository, @InjectRepository(User) private readonly userRepo: Repository, @@ -81,12 +83,21 @@ export class BountiesService { } /** - * The linked PR was merged on GitHub. Transitions to MERGED and immediately - * triggers the escrow release (single recipient or team split), moving to - * PAID once the on-chain release call succeeds. + * The linked PR was merged on GitHub. Transitions to MERGED, then immediately + * attempts the escrow release (single recipient or team split). + * + * If the escrow release succeeds, moves to PAID. + * If the escrow release fails, moves to RELEASE_PENDING so that a retry + * can be attempted later — the bounty is no longer permanently stuck. */ async markMergedAndRelease(id: string): Promise { const bounty = await this.findOne(id); + + // Allow retry from RELEASE_PENDING (escrow release previously failed) + if (bounty.status === BountyStatus.RELEASE_PENDING) { + return this.attemptEscrowRelease(bounty); + } + assertTransition(bounty.status, BountyStatus.MERGED); bounty.status = BountyStatus.MERGED; @@ -95,44 +106,68 @@ export class BountiesService { if (!bounty.escrowId) { // No escrow was ever funded (e.g. informally tracked bounty) — nothing to release. - return bounty; + assertTransition(bounty.status, BountyStatus.PAID); + bounty.status = BountyStatus.PAID; + bounty.paidAt = new Date(); + return this.bountyRepo.save(bounty); } - if (bounty.teamId) { - const team = await this.teamRepo.findOne({ - where: { id: bounty.teamId }, - relations: { splits: true }, - }); - if (team && team.splits.length > 0) { - const recipients = await Promise.all( - team.splits.map(async (split) => { - const user = await this.userRepo.findOne({ - where: { id: split.userId }, - }); - return { - recipientId: split.userId, - recipientAddress: user?.stellarAddress ?? '', - percentage: Number(split.percentage), - }; - }), + return this.attemptEscrowRelease(bounty); + } + + /** + * Attempts to release escrow funds. On success, moves to PAID. + * On failure, moves to RELEASE_PENDING so the release can be retried. + */ + private async attemptEscrowRelease(bounty: Bounty): Promise { + assertTransition(bounty.status, BountyStatus.RELEASE_PENDING); + bounty.status = BountyStatus.RELEASE_PENDING; + await this.bountyRepo.save(bounty); + + try { + if (bounty.teamId) { + const team = await this.teamRepo.findOne({ + where: { id: bounty.teamId }, + relations: { splits: true }, + }); + if (team && team.splits.length > 0) { + const recipients = await Promise.all( + team.splits.map(async (split) => { + const user = await this.userRepo.findOne({ + where: { id: split.userId }, + }); + return { + recipientId: split.userId, + recipientAddress: user?.stellarAddress ?? '', + percentage: Number(split.percentage), + }; + }), + ); + await this.escrowService.splitRelease(bounty.escrowId!, recipients); + } + } else if (bounty.claimedById) { + const contributor = await this.userRepo.findOne({ + where: { id: bounty.claimedById }, + }); + await this.escrowService.release( + bounty.escrowId!, + contributor?.stellarAddress ?? '', + bounty.claimedById, ); - await this.escrowService.splitRelease(bounty.escrowId, recipients); } - } else if (bounty.claimedById) { - const contributor = await this.userRepo.findOne({ - where: { id: bounty.claimedById }, - }); - await this.escrowService.release( - bounty.escrowId, - contributor?.stellarAddress ?? '', - bounty.claimedById, + + assertTransition(bounty.status, BountyStatus.PAID); + bounty.status = BountyStatus.PAID; + bounty.paidAt = new Date(); + return this.bountyRepo.save(bounty); + } catch (err) { + this.logger.error( + `Escrow release failed for bounty ${bounty.id}: ${(err as Error).message}. ` + + `Bounty is now in RELEASE_PENDING state and can be retried.`, ); + // Bounty stays in RELEASE_PENDING — caller can retry later + return bounty; } - - assertTransition(bounty.status, BountyStatus.PAID); - bounty.status = BountyStatus.PAID; - bounty.paidAt = new Date(); - return this.bountyRepo.save(bounty); } /** Sponsor (or admin/expiry job) reclaims escrowed funds. */ diff --git a/src/bounties/bounty-state-machine.spec.ts b/src/bounties/bounty-state-machine.spec.ts index d084baf..df4e268 100644 --- a/src/bounties/bounty-state-machine.spec.ts +++ b/src/bounties/bounty-state-machine.spec.ts @@ -15,7 +15,12 @@ describe('bounty state machine', () => { expect(canTransition(BountyStatus.IN_REVIEW, BountyStatus.MERGED)).toBe( true, ); - expect(canTransition(BountyStatus.MERGED, BountyStatus.PAID)).toBe(true); + expect(canTransition(BountyStatus.MERGED, BountyStatus.RELEASE_PENDING)).toBe( + true, + ); + expect(canTransition(BountyStatus.RELEASE_PENDING, BountyStatus.PAID)).toBe( + true, + ); }); it('allows moving back from in_review to claimed (PR closed without merge)', () => { @@ -24,13 +29,20 @@ describe('bounty state machine', () => { ); }); - it('allows refund from open, funded, claimed, in_review, merged, and expired', () => { + it('allows retry from release_pending to release_pending', () => { + expect(canTransition(BountyStatus.RELEASE_PENDING, BountyStatus.RELEASE_PENDING)).toBe( + true, + ); + }); + + it('allows refund from open, funded, claimed, in_review, merged, release_pending, and expired', () => { for (const status of [ BountyStatus.OPEN, BountyStatus.FUNDED, BountyStatus.CLAIMED, BountyStatus.IN_REVIEW, BountyStatus.MERGED, + BountyStatus.RELEASE_PENDING, BountyStatus.EXPIRED, ]) { expect(canTransition(status, BountyStatus.REFUNDED)).toBe(true); @@ -41,6 +53,11 @@ describe('bounty state machine', () => { expect(canTransition(BountyStatus.OPEN, BountyStatus.MERGED)).toBe(false); }); + it('disallows direct transition from merged to paid', () => { + // Must go through release_pending first + expect(canTransition(BountyStatus.MERGED, BountyStatus.PAID)).toBe(false); + }); + it('disallows any transition out of a terminal PAID state', () => { expect(canTransition(BountyStatus.PAID, BountyStatus.REFUNDED)).toBe(false); expect(canTransition(BountyStatus.PAID, BountyStatus.OPEN)).toBe(false); diff --git a/src/bounties/bounty-state-machine.ts b/src/bounties/bounty-state-machine.ts index b89fb7c..1fa12ea 100644 --- a/src/bounties/bounty-state-machine.ts +++ b/src/bounties/bounty-state-machine.ts @@ -3,10 +3,11 @@ import { BountyStatus } from '../common/enums'; /** * Valid forward transitions for a bounty's lifecycle: * - * open -> funded -> claimed -> in_review -> merged -> paid + * open -> funded -> claimed -> in_review -> merged -> release_pending -> paid * \-> refunded * (open|funded|claimed) -> expired * (open|funded) -> refunded + * release_pending -> release_pending (retry on escrow failure) */ export const BOUNTY_TRANSITIONS: Record = { [BountyStatus.OPEN]: [ @@ -29,7 +30,12 @@ export const BOUNTY_TRANSITIONS: Record = { BountyStatus.CLAIMED, BountyStatus.REFUNDED, ], - [BountyStatus.MERGED]: [BountyStatus.PAID, BountyStatus.REFUNDED], + [BountyStatus.MERGED]: [BountyStatus.RELEASE_PENDING], + [BountyStatus.RELEASE_PENDING]: [ + BountyStatus.PAID, + BountyStatus.RELEASE_PENDING, // Allow retry on escrow failure + BountyStatus.REFUNDED, + ], [BountyStatus.PAID]: [], [BountyStatus.REFUNDED]: [], [BountyStatus.EXPIRED]: [BountyStatus.REFUNDED], diff --git a/src/common/enums/index.ts b/src/common/enums/index.ts index a2a6be7..6621c9b 100644 --- a/src/common/enums/index.ts +++ b/src/common/enums/index.ts @@ -22,6 +22,7 @@ export enum BountyStatus { CLAIMED = 'claimed', IN_REVIEW = 'in_review', MERGED = 'merged', + RELEASE_PENDING = 'release_pending', PAID = 'paid', REFUNDED = 'refunded', EXPIRED = 'expired', diff --git a/src/reputation/reputation.service.ts b/src/reputation/reputation.service.ts index 0404858..e2bf38b 100644 --- a/src/reputation/reputation.service.ts +++ b/src/reputation/reputation.service.ts @@ -22,7 +22,7 @@ export class ReputationService { where: { claimedById: userId }, }); const merged = claimedBounties.filter((b) => - [BountyStatus.MERGED, BountyStatus.PAID].includes(b.status), + [BountyStatus.MERGED, BountyStatus.RELEASE_PENDING, BountyStatus.PAID].includes(b.status), ); const paid = claimedBounties.filter((b) => b.status === BountyStatus.PAID);