From 3bbee637cfdaf713d3a5ebb7214c54bd1fbc5ef1 Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 05:24:45 -0400 Subject: [PATCH 1/3] fix(escrow): splitRelease strips the nested escrow.metadata like the others MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit splitRelease returned raw Payment[] — a loaded payment.escrow relation carried Escrow.metadata (raw Soroban diagnostics, the class #19 keeps server-side) straight to the client, while fund/findOne/release/refund all wrap in toPublicEscrow. Added toPublicPayment — strips the relation's metadata — and applied it. Resolves #91 --- src/escrow/escrow-response.mapper.ts | 17 ++++++++++++++++- src/escrow/escrow.controller.ts | 12 ++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/escrow/escrow-response.mapper.ts b/src/escrow/escrow-response.mapper.ts index 9a8b78fd..928047e7 100644 --- a/src/escrow/escrow-response.mapper.ts +++ b/src/escrow/escrow-response.mapper.ts @@ -1,7 +1,12 @@ -import { Escrow } from '../common/entities'; +import { Escrow, Payment } from '../common/entities'; export type PublicEscrow = Omit; +/** A Payment with its `escrow` relation redacted to the public shape. */ +export type PublicPayment = Omit & { + escrow?: PublicEscrow; +}; + /** * Strips `metadata` before an Escrow reaches an HTTP client. `metadata` * intentionally stores raw diagnostic detail from Soroban RPC calls — @@ -20,3 +25,13 @@ export function toPublicEscrow(escrow: Escrow): PublicEscrow { delete publicEscrow.metadata; return publicEscrow as PublicEscrow; } + +/** + * #91 — `splitRelease` returned raw `Payment[]`, so a loaded `payment.escrow` + * relation carried `Escrow.metadata` (raw Soroban diagnostics) to the client. + * Strip it the same way `toPublicEscrow` does for the escrow routes. + */ +export function toPublicPayment(payment: Payment): PublicPayment { + const { escrow, ...rest } = payment; + return escrow ? { ...rest, escrow: toPublicEscrow(escrow) } : { ...rest }; +} diff --git a/src/escrow/escrow.controller.ts b/src/escrow/escrow.controller.ts index a3c3c2ef..916aab6f 100644 --- a/src/escrow/escrow.controller.ts +++ b/src/escrow/escrow.controller.ts @@ -17,7 +17,7 @@ import { EscrowService } from './escrow.service'; import { FundEscrowDto } from './dto/fund-escrow.dto'; import { ReleaseEscrowDto } from './dto/release-escrow.dto'; import { SplitReleaseDto } from './dto/split-release.dto'; -import { toPublicEscrow } from './escrow-response.mapper'; +import { toPublicEscrow, toPublicPayment } from './escrow-response.mapper'; import { Idempotent } from '../common/idempotency/idempotent.decorator'; import { ApiInternalErrorResponse, @@ -47,7 +47,8 @@ export class EscrowController { @ApiOperation({ summary: 'Release an escrow to its recipient', - description: 'Rate-limited to 1 request/second to protect against replay/DoS.', + description: + 'Rate-limited to 1 request/second to protect against replay/DoS.', }) @ApiBearerAuth() @ApiStandardErrorResponses() @@ -82,12 +83,15 @@ export class EscrowController { @Param('id', new ParseUUIDPipe()) id: string, @Body() dto: SplitReleaseDto, ) { - return this.escrowService.splitRelease(id, dto.recipients); + return this.escrowService + .splitRelease(id, dto.recipients) + .then((payments) => payments.map(toPublicPayment)); } @ApiOperation({ summary: 'Refund an escrow to its original funder', - description: 'Rate-limited to 1 request/second to protect against replay/DoS.', + description: + 'Rate-limited to 1 request/second to protect against replay/DoS.', }) @ApiBearerAuth() @ApiStandardErrorResponses() From 1da40f887faa23d5d75cb12cc0b1fcd9db447aaa Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 22:23:54 -0400 Subject: [PATCH 2/3] fix: repair three mangled merges + sync drifted specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stacked-merge base left three files unable to compile: - escrow.service.ts: overlapping invokeOnLockedEscrow returns (kept u64) - sponsors.controller.ts: interleaved validatePage/parsePagination + duplicate returns (kept validatePage — ParseIntPipe already gives numbers) - github-webhooks.service.spec.ts: missing it()/describe() closers Spec drift synced: fund() callerUserId arg, releasePartial's manager arg, lowercase role enum values, dto-shaped release()/splitRelease() calls, checksum-valid Stellar/UUID fixtures, and a StrKey stub in the jest mock. Real bug fixed along the way: FundEscrowDto.deadline had IsDateString + a @Type(() => Date) transform — the Date it produces can never pass IsDateString, so every request with a deadline 400'd. Now IsDate, which still rejects Invalid-Date results. --- src/escrow/dto/fund-escrow.dto.ts | 7 +++++-- src/escrow/escrow.controller.spec.ts | 22 +++++++++++----------- src/escrow/escrow.service.ts | 10 ++-------- src/github/github-webhooks.service.spec.ts | 3 +++ src/milestones/milestones.service.spec.ts | 8 +++++--- src/sponsors/sponsors.controller.ts | 21 --------------------- test/mocks/stellar-sdk.mock.js | 9 +++++++++ 7 files changed, 35 insertions(+), 45 deletions(-) diff --git a/src/escrow/dto/fund-escrow.dto.ts b/src/escrow/dto/fund-escrow.dto.ts index 0bc4508c..c8885001 100644 --- a/src/escrow/dto/fund-escrow.dto.ts +++ b/src/escrow/dto/fund-escrow.dto.ts @@ -1,7 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { Type } from 'class-transformer'; import { - IsDateString, + IsDate, IsOptional, IsString, IsUUID, @@ -89,7 +89,10 @@ export class FundEscrowDto { 'ISO 8601 refund-deadline passed to escrow::fund (defaults to the configured window)', }) @IsOptional() - @IsDateString() + // @Type(() => Date) transforms before validation, so IsDateString (which + // requires a string) would reject every real deadline. IsDate accepts the + // transformed value and still rejects NaN "Invalid Date" results. + @IsDate() @Type(() => Date) deadline?: Date; } diff --git a/src/escrow/escrow.controller.spec.ts b/src/escrow/escrow.controller.spec.ts index 5d9e4a4b..c1fab578 100644 --- a/src/escrow/escrow.controller.spec.ts +++ b/src/escrow/escrow.controller.spec.ts @@ -115,7 +115,7 @@ describe('EscrowController', () => { }); it('release() returns the service escrow without metadata', async () => { - const result = await controller.release('escrow-1', 'GRECIPIENT', 'user-1'); + const result = await controller.release('escrow-1', { recipientAddress: 'GRECIPIENT', recipientId: 'user-1' }); expect(mockEscrowService.release).toHaveBeenCalledWith( 'escrow-1', @@ -142,7 +142,7 @@ describe('EscrowController', () => { { recipientAddress: 'GB', percentage: 50 }, ]; - const result = await controller.splitRelease('escrow-1', recipients); + const result = await controller.splitRelease('escrow-1', { recipients }); expect(mockEscrowService.splitRelease).toHaveBeenCalledWith( 'escrow-1', @@ -210,11 +210,11 @@ describe('EscrowController', () => { it('release() is restricted to MAINTAINER and refund() to MAINTAINER/SPONSOR', () => { expect(Reflect.getMetadata(ROLES_KEY, handlerFor('release'))).toEqual([ - 'MAINTAINER', + 'maintainer', ]); expect(Reflect.getMetadata(ROLES_KEY, handlerFor('refund'))).toEqual([ - 'MAINTAINER', - 'SPONSOR', + 'maintainer', + 'sponsor', ]); }); @@ -235,15 +235,15 @@ describe('EscrowController', () => { const base = { amount: '100.0000000', asset: AssetType.USDC, - funderAddress: 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567abcdefghijklmn', - bountyId: '00000000-0000-0000-0000-000000000001', + funderAddress: 'GAHK7EEG2WWHVKDNT4CEQFZGKF2LGDSW2IVM4S5DP42RBW3K6BTODB4A', + bountyId: 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', }; it('accepts onChainIssueId, sponsorId and deadline', async () => { const dto = plainToInstance(FundEscrowDto, { ...base, onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: 'b1eebc99-9c0b-4ef8-bb6d-6bb9bd380a22', deadline: '2026-12-31T00:00:00.000Z', }); const errors = await validate(dto); @@ -289,7 +289,7 @@ describe('EscrowController', () => { const dto = plainToInstance(FundEscrowDto, { amount: 'not-money', asset: AssetType.USDC, - funderAddress: 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567abcdefghijklmn', + funderAddress: 'GAHK7EEG2WWHVKDNT4CEQFZGKF2LGDSW2IVM4S5DP42RBW3K6BTODB4A', }); const errors = await validate(dto); expect(errors.length).toBeGreaterThan(0); @@ -299,7 +299,7 @@ describe('EscrowController', () => { const dto = plainToInstance(FundEscrowDto, { ...base, onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: 'b1eebc99-9c0b-4ef8-bb6d-6bb9bd380a22', deadline: '2026-12-31T00:00:00.000Z', }); mockEscrowService.fund.mockResolvedValueOnce({ id: 'escrow-1' }); @@ -307,7 +307,7 @@ describe('EscrowController', () => { expect(mockEscrowService.fund).toHaveBeenCalledWith( expect.objectContaining({ onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: 'b1eebc99-9c0b-4ef8-bb6d-6bb9bd380a22', deadline: new Date('2026-12-31T00:00:00.000Z'), }), ); diff --git a/src/escrow/escrow.service.ts b/src/escrow/escrow.service.ts index 6d2dd289..c252993f 100644 --- a/src/escrow/escrow.service.ts +++ b/src/escrow/escrow.service.ts @@ -584,20 +584,14 @@ export class EscrowService { recipients: Array<[string, number]>, manager?: EntityManager, ): Promise { - return this.invokeOnLockedEscrow(escrow, operation, () => - this.soroban.invoke( - 'release', - // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301). - [u64(this.onChainKeyFor(escrow)), recipients], - this.contractOpts(escrow), - ), return this.invokeOnLockedEscrow( escrow, operation, () => this.soroban.invoke( 'release', - [this.onChainKeyFor(escrow), recipients], + // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301). + [u64(this.onChainKeyFor(escrow)), recipients], this.contractOpts(escrow), ), manager, diff --git a/src/github/github-webhooks.service.spec.ts b/src/github/github-webhooks.service.spec.ts index 45bb6fe8..a35ac3be 100644 --- a/src/github/github-webhooks.service.spec.ts +++ b/src/github/github-webhooks.service.spec.ts @@ -869,6 +869,9 @@ describe('GithubWebhooksService', () => { expect(bountiesService.markInReview).not.toHaveBeenCalled(); expect(bountiesService.markMergedAndRelease).not.toHaveBeenCalled(); + }); + }); + // #308: a redelivered X-GitHub-Delivery used to hit the unique constraint // on webhook_events.deliveryId and escape handleEvent as a 500, so every // redelivery of that event failed forever. diff --git a/src/milestones/milestones.service.spec.ts b/src/milestones/milestones.service.spec.ts index 612bde8d..4cd10a5e 100644 --- a/src/milestones/milestones.service.spec.ts +++ b/src/milestones/milestones.service.spec.ts @@ -75,7 +75,7 @@ describe('MilestonesService', () => { sponsorId: 'sponsor-1', }); - const milestone = await service.fund('m1', 'GFUNDER'); + const milestone = await service.fund('m1', 'GFUNDER', 'sponsor-1'); expect(milestone.status).toBe(MilestoneStatus.FUNDED); expect(milestone.escrowId).toBe('escrow-1'); @@ -90,7 +90,7 @@ describe('MilestonesService', () => { sponsorId: 'sponsor-1', }); - await service.fund('m1', 'GFUNDER'); + await service.fund('m1', 'GFUNDER', 'sponsor-1'); expect(escrowService.fund).toHaveBeenCalledWith( expect.objectContaining({ @@ -263,6 +263,7 @@ describe('MilestonesService', () => { '250.0000000', 'RECIPIENT_ADDR', 'user-1', + expect.anything(), ); expect(payment.id).toBe('payment-1'); expect(dataSource.transaction).toHaveBeenCalled(); @@ -341,7 +342,7 @@ describe('MilestonesService', () => { await expect( service.resolveIssue('m1', 'i1', 'RECIPIENT_ADDR'), - ).rejects.toThrow('Milestone m1 is not accepting distributions'); + ).rejects.toThrow('Cannot transition milestone'); }); it('rejects when the issue does not belong to the milestone (#114)', async () => { @@ -406,6 +407,7 @@ describe('MilestonesService', () => { '333.3333333', 'RECIPIENT_ADDR', undefined, + expect.anything(), ); }); }); diff --git a/src/sponsors/sponsors.controller.ts b/src/sponsors/sponsors.controller.ts index b7a48cca..c097bf1e 100644 --- a/src/sponsors/sponsors.controller.ts +++ b/src/sponsors/sponsors.controller.ts @@ -26,25 +26,6 @@ function validatePage(limit?: number, offset?: number) { throw new BadRequestException('offset must be >= 0'); } return { limit, offset }; -/** - * Parse optional limit/offset query params, rejecting NaN, negatives, - * non-integers, and limits above MAX_PAGE_LIMIT with a 400 (#280). - */ -function parsePagination(limit?: string, offset?: string) { - const parse = (name: string, raw: string | undefined, min: number, max: number) => { - if (raw === undefined || raw === '') return undefined; - const n = Number(raw); - if (!Number.isInteger(n) || n < min || n > max) { - throw new BadRequestException( - `${name} must be an integer between ${min} and ${max}`, - ); - } - return n; - }; - return { - limit: parse('limit', limit, 1, MAX_PAGE_LIMIT), - offset: parse('offset', offset, 0, Number.MAX_SAFE_INTEGER), - }; } @ApiTags('sponsors') @@ -73,7 +54,6 @@ export class SponsorsController { ) { this.assertOwnsSponsor(user, id); return this.sponsorsService.dashboard(id, validatePage(limit, offset)); - return this.sponsorsService.dashboard(id, parsePagination(limit, offset)); } @ApiBearerAuth() @@ -91,6 +71,5 @@ export class SponsorsController { id, validatePage(limit, offset), ); - return this.sponsorsService.milestoneProgress(id, parsePagination(limit, offset)); } } diff --git a/test/mocks/stellar-sdk.mock.js b/test/mocks/stellar-sdk.mock.js index a1a32f55..6fe33f51 100644 --- a/test/mocks/stellar-sdk.mock.js +++ b/test/mocks/stellar-sdk.mock.js @@ -40,7 +40,16 @@ class TransactionBuilder { } } + +class StrKey { + // Tests only need accept/reject — real CRC16 stays in the SDK's prod path. + static isValidEd25519PublicKey(value) { + return typeof value === 'string' && /^G[A-Z2-7]{55}$/.test(value); + } +} + module.exports = { + StrKey, Contract, Address, Keypair, From 112d6fa5a358a295ab61818e1baa490349b24ba1 Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 22:33:26 -0400 Subject: [PATCH 3/3] fix: port the webhooks-spec mock-signature sync from the sibling branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findIssueByRepoAndNumber is positional (repoId, number) — the { where } object mocks silently returned undefined, so the #47/#309/#310 tests never reached the bounty lookups. 36/36 pass. --- src/github/github-webhooks.service.spec.ts | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/github/github-webhooks.service.spec.ts b/src/github/github-webhooks.service.spec.ts index a35ac3be..9e38e3c0 100644 --- a/src/github/github-webhooks.service.spec.ts +++ b/src/github/github-webhooks.service.spec.ts @@ -10,7 +10,6 @@ import * as sigUtil from './webhook-signature.util'; describe('GithubWebhooksService', () => { let service: GithubWebhooksService; - let webhookEventRepo: { create: jest.Mock; save: jest.Mock }; let webhookEventRepo: { create: jest.Mock; save: jest.Mock; @@ -40,6 +39,7 @@ describe('GithubWebhooksService', () => { findOne: jest.fn().mockResolvedValue(null), }; bountyRepo = { findOne: jest.fn() }; + issueRepo = { findOne: jest.fn().mockResolvedValue(null) }; bountiesService = { markInReview: jest.fn().mockResolvedValue(undefined), markMergedAndRelease: jest.fn().mockResolvedValue(undefined), @@ -264,11 +264,11 @@ describe('GithubWebhooksService', () => { byNumber: Record, ) { syncService.findIssueByRepoAndNumber.mockImplementation( - ({ where }: { where: { number: number } }) => { - const entry = byNumber[where.number]; + (_repoId: string, number: number) => { + const entry = byNumber[number]; return Promise.resolve( entry - ? { id: `issue-${where.number}`, bounty: { id: entry.bountyId } } + ? { id: `issue-${number}`, bounty: { id: entry.bountyId } } : null, ); }, @@ -637,9 +637,8 @@ describe('GithubWebhooksService', () => { describe('comma-separated closing references (#309)', () => { /** Collects every issue number the merged-PR path ends up releasing. */ function releasedNumbers(): number[] { - return issueRepo.findOne.mock.calls.map( - (call: unknown[]) => - (call[0] as { where: { number: number } }).where.number, + return syncService.findIssueByRepoAndNumber.mock.calls.map( + (call: unknown[]) => call[1] as number, ); } @@ -732,11 +731,11 @@ describe('GithubWebhooksService', () => { }); it('marks a comma-separated bounty in review when the PR is opened', async () => { - issueRepo.findOne.mockImplementation( - ({ where }: { where: { number: number } }) => + syncService.findIssueByRepoAndNumber.mockImplementation( + (_repoId: string, number: number) => Promise.resolve({ - id: `issue-${where.number}`, - bounty: { id: `bounty-${where.number}` }, + id: `issue-${number}`, + bounty: { id: `bounty-${number}` }, }), ); bountyRepo.findOne.mockImplementation( @@ -782,10 +781,10 @@ describe('GithubWebhooksService', () => { describe('pull_request edited events (#310)', () => { it('moves a bounty to in_review when a closing keyword is added after opening', async () => { - issueRepo.findOne.mockImplementation( - ({ where }: { where: { number: number } }) => + syncService.findIssueByRepoAndNumber.mockImplementation( + (_repoId: string, number: number) => Promise.resolve({ - id: `issue-${where.number}`, + id: `issue-${number}`, bounty: { id: 'bounty-42' }, }), );