Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,29 @@ Unit tests cover critical domains including:
- `src/github/webhook-signature.util.spec.ts` — GitHub webhook HMAC-SHA256 signature verification.
- `src/github/github-webhooks.service.spec.ts` — webhook-to-escrow release logic.
- `src/bounties/bounties.service.spec.ts` — bounty core management.
- `src/sponsors/sponsors.service.spec.ts` — sponsor dashboard aggregate queries (budgetLocked/totalSpend read the Escrow/Payment ledger directly).
- `src/database/escrow-fk-integrity.integration.spec.ts` — **integration** test against a real Postgres (requires `DATABASE_URL`, not mocked): the exactly-one-parent CHECK constraint on `escrows`, and that sponsor dashboard figures survive a parent bounty/milestone being deleted.

`DATABASE_SYNCHRONIZE=true` (set in development) will auto-create tables from entities for fast local iteration. A real migration workflow (`typeorm migration:generate`) is recommended before deploying to production (see Roadmap).
`DATABASE_SYNCHRONIZE=true` (set in development) will auto-create tables from entities for fast local iteration. Real deployments should run migrations instead — see `src/database/migrations/` and the `migration:*` npm scripts below.

### Migrations

Schema changes are tracked as TypeORM migrations under `src/database/migrations/`, driven by the `DataSource` in `src/database/data-source.ts`:

```bash
# Generate a migration from entity changes (requires DATABASE_URL pointed at a real DB to diff against)
npm run migration:generate -- src/database/migrations/SomeChange

# Run all pending migrations
npm run migration:run

# Revert the most recently applied migration
npm run migration:revert
```

## Roadmap

- [ ] Wire up TypeORM migrations (currently relies on `synchronize` for local dev only).
- [x] ~~Wire up TypeORM migrations (currently relies on `synchronize` for local dev only).~~ See `src/database/migrations/` and the Migrations section above.
- [ ] Move GitHub sync from a static PAT to a GitHub App installation-token flow for multi-org, least-privilege access.
- [ ] Deploy the real escrow contract from `mergefi-contracts` and drop the Soroban dry-run fallback.
- [ ] Replace the single `TREASURY_SECRET` signer with a proper signing service (KMS / multi-sig) before handling real funds.
Expand Down
7 changes: 6 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
"test:watch": "jest --watch",
"test:cov": "jest --coverage",
"test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
"test:e2e": "jest --config ./test/jest-e2e.json"
"test:e2e": "jest --config ./test/jest-e2e.json",
"typeorm": "typeorm-ts-node-commonjs -d src/database/data-source.ts",
"migration:generate": "npm run typeorm -- migration:generate",
"migration:create": "typeorm-ts-node-commonjs migration:create",
"migration:run": "npm run typeorm -- migration:run",
"migration:revert": "npm run typeorm -- migration:revert"
},
"dependencies": {
"@nestjs/common": "^11.0.1",
Expand Down
7 changes: 6 additions & 1 deletion src/bounties/bounties.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,17 @@ describe('BountiesService', () => {
status: BountyStatus.OPEN,
amount: '100',
asset: AssetType.USDC,
sponsorId: 'sponsor-1',
});

const bounty = await service.fund('b1', 'GFUNDER');

expect(escrowService.fund).toHaveBeenCalledWith(
expect.objectContaining({ bountyId: 'b1', funderAddress: 'GFUNDER' }),
expect.objectContaining({
bountyId: 'b1',
funderAddress: 'GFUNDER',
sponsorId: 'sponsor-1',
}),
);
expect(bounty.status).toBe(BountyStatus.FUNDED);
});
Expand Down
1 change: 1 addition & 0 deletions src/bounties/bounties.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class BountiesService {
asset: bounty.asset,
funderAddress,
bountyId: bounty.id,
sponsorId: bounty.sponsorId,
});

bounty.escrow = escrow;
Expand Down
5 changes: 4 additions & 1 deletion src/common/entities/bounty.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,12 @@ export class Bounty {
@Column({ type: 'timestamptz', nullable: true })
deadline: Date | null;

// cascade excludes 'remove'/'soft-remove': removing a Bounty entity via the
// ORM must not also remove its Escrow — the escrow row (and the funds it
// represents) must outlive the bounty record. See #27.
@OneToOne(() => Escrow, (escrow) => escrow.bounty, {
nullable: true,
cascade: true,
cascade: ['insert', 'update'],
})
escrow: Escrow | null;

Expand Down
42 changes: 39 additions & 3 deletions src/common/entities/escrow.entity.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
Check,
Column,
CreateDateColumn,
Entity,
Expand All @@ -20,15 +21,39 @@ import { AssetType, EscrowStatus } from '../enums';
* The actual funds custody lives in the deployed escrow contract on Stellar;
* this table mirrors state so the API can serve fast reads and so we have an
* audit trail independent of Horizon/RPC availability.
*
* The parent link (bounty/milestone/maintenancePool) is `onDelete: 'SET
* NULL'`, not CASCADE: deleting a bounty/milestone must never delete the
* escrow row (and, transitively, its payments) out from under real,
* possibly still-LOCKED, funds. See #27 — an escrow whose parent was
* deleted stays a first-class, still-queryable ledger row, orphaned but
* intact, attributable to its sponsor via the denormalized `sponsorId`
* below (which survives independently of the parent row).
*
* The CHECK constraint below only enforces *at most one* parent, not
* *exactly* one: `ON DELETE SET NULL` nulls out an escrow's only parent
* column, which is precisely the state an orphaned-by-deletion escrow ends
* up in (0 of 3 set) — a CHECK requiring exactly one would make that very
* SET NULL fail with a constraint violation the moment it fires. "Exactly
* one at creation" is enforced instead where it belongs: application-side,
* in EscrowService.fund (see assertExactlyOneParent).
*/
@Entity('escrows')
@Check(
'CHK_escrow_at_most_one_parent',
`(
(CASE WHEN "bountyId" IS NOT NULL THEN 1 ELSE 0 END) +
(CASE WHEN "milestoneId" IS NOT NULL THEN 1 ELSE 0 END) +
(CASE WHEN "maintenancePoolId" IS NOT NULL THEN 1 ELSE 0 END)
) <= 1`,
)
export class Escrow {
@PrimaryGeneratedColumn('uuid')
id: string;

@OneToOne(() => Bounty, (bounty) => bounty.escrow, {
nullable: true,
onDelete: 'CASCADE',
onDelete: 'SET NULL',
})
@JoinColumn()
bounty: Bounty | null;
Expand All @@ -38,7 +63,7 @@ export class Escrow {

@OneToOne(() => Milestone, (milestone) => milestone.escrow, {
nullable: true,
onDelete: 'CASCADE',
onDelete: 'SET NULL',
})
@JoinColumn()
milestone: Milestone | null;
Expand All @@ -48,14 +73,25 @@ export class Escrow {

@OneToOne(() => MaintenancePool, (pool) => pool.escrow, {
nullable: true,
onDelete: 'CASCADE',
onDelete: 'SET NULL',
})
@JoinColumn()
maintenancePool: MaintenancePool | null;

@Column({ type: 'varchar', nullable: true })
maintenancePoolId: string | null;

/**
* Denormalized sponsor identity, captured from the parent bounty/milestone
* at fund time. Sponsor-dashboard aggregates (src/sponsors/sponsors.service.ts)
* read this column directly rather than joining through bounty/milestone,
* so a locked or spent escrow is still correctly attributed to its sponsor
* even after the parent record is deleted (#27). Null for
* maintenance-pool escrows, which aren't sponsor-attributed the same way.
*/
@Column({ type: 'varchar', nullable: true })
sponsorId: string | null;

/** Deployed Soroban contract ID this escrow instance is held by. */
@Column({ type: 'varchar', nullable: true })
contractId: string | null;
Expand Down
7 changes: 6 additions & 1 deletion src/common/entities/payment.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ export class Payment {
@PrimaryGeneratedColumn('uuid')
id: string;

@ManyToOne(() => Escrow, (escrow) => escrow.payments, { onDelete: 'CASCADE' })
// RESTRICT, not CASCADE: a Payment is a record of money that actually
// moved. Deleting its parent Escrow must never silently delete that
// payout record too — the database refuses the delete instead. See #27.
@ManyToOne(() => Escrow, (escrow) => escrow.payments, {
onDelete: 'RESTRICT',
})
@JoinColumn()
escrow: Escrow;

Expand Down
26 changes: 26 additions & 0 deletions src/database/data-source.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import 'reflect-metadata';
import { DataSource } from 'typeorm';
import { entities } from '../common/entities/typeorm-entities';

/**
* TypeORM CLI DataSource — used only for `migration:generate`/`migration:run`/
* `migration:revert` (see package.json scripts). The running application
* connects via `TypeOrmModule.forRootAsync` in src/app.module.ts instead;
* the two are kept in sync by importing the same `entities` list.
*
* Before this, the app relied entirely on `synchronize` and had no migration
* history at all (#27) — every schema change (including the exactly-one-
* parent CHECK constraint this DataSource ships the first migration for)
* now goes through a reviewable, revertible migration file instead.
*/
export const AppDataSource = new DataSource({
type: 'postgres',
url:
process.env.DATABASE_URL ??
'postgresql://postgres:postgres@localhost:5432/mergefi',
entities,
migrations: [__dirname + '/migrations/*{.ts,.js}'],
migrationsTableName: 'migrations',
synchronize: false,
logging: process.env.DATABASE_LOGGING === 'true',
});
Loading
Loading