diff --git a/src/admin/admin.controller.ts b/src/admin/admin.controller.ts index 2a27932..354abe5 100644 --- a/src/admin/admin.controller.ts +++ b/src/admin/admin.controller.ts @@ -7,8 +7,9 @@ import { Request, ParseUUIDPipe, } from '@nestjs/common'; -import { AdminService } from './admin.service'; +import { AdminService, ReconcileOutcome } from './admin.service'; import { SuspendCampaignDto } from './dtos/suspend-campaign.dto'; +import { ReconcileBalanceDto } from './dtos/reconcile-balance.dto'; import { Roles } from '../common/decorators/roles.decorator'; import { RolesGuard } from '../common/guards/roles.guard'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; @@ -33,4 +34,30 @@ export class AdminController { req.user.email, ); } + + /** + * POST /admin/campaigns/:id/reconcile-balance + * + * Reads the on-chain Stellar account and the APPROVED/RELEASED fund + * releases for this campaign, then either: + * * reports the figures (DRY_RUN) if `body.force !== true`, or + * * writes the canonical `Campaign.raisedAmount = netAvailableByAssetTotal` + * and records an AuditLog (mode: WRITE) when `body.force === true`. + * + * Body: { force: boolean, reason?: string }. The endpoint is the only + * safe path to correct a discrepancy and cannot be triggered silently. + */ + @Post('campaigns/:id/reconcile-balance') + async reconcileCampaignBalance( + @Param('id', ParseUUIDPipe) id: string, + @Body() dto: ReconcileBalanceDto, + @Request() req: any, + ): Promise { + return this.adminService.reconcileCampaignBalance( + id, + dto, + req.user.sub, + req.user.email, + ); + } } diff --git a/src/admin/admin.module.ts b/src/admin/admin.module.ts index fd9ba87..016ba7a 100644 --- a/src/admin/admin.module.ts +++ b/src/admin/admin.module.ts @@ -6,10 +6,11 @@ import { AdminController } from './admin.controller'; import { NotificationsModule } from '../notifications/notifications.module'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { RolesGuard } from '../common/guards/roles.guard'; +import { CampaignsModule } from '../campaigns/campaigns.module'; /** Module providing admin campaign suspension, user moderation, and audit logging */ @Module({ - imports: [PrismaModule, AuthModule, NotificationsModule], + imports: [PrismaModule, AuthModule, NotificationsModule, CampaignsModule], controllers: [AdminController], providers: [AdminService, JwtAuthGuard, RolesGuard], }) diff --git a/src/admin/admin.service.ts b/src/admin/admin.service.ts index 05e8284..866d976 100644 --- a/src/admin/admin.service.ts +++ b/src/admin/admin.service.ts @@ -5,15 +5,115 @@ import { } from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; import { NotificationsService } from '../notifications/notifications.service'; +import { CampaignsService } from '../campaigns/campaigns.service'; import { SuspendCampaignDto } from './dtos/suspend-campaign.dto'; +import { ReconcileBalanceDto } from './dtos/reconcile-balance.dto'; + +export interface ReconcileOutcome { + campaignId: string; + storedRaisedAmount: string; + netAvailableByAssetTotal: string; + onChainTotal: string; + netReleasedAmount: string; + discrepancyDetected: boolean; + auditLogId: string; + applied: boolean; + reason?: string; +} @Injectable() export class AdminService { constructor( private readonly prisma: PrismaService, private readonly notificationsService: NotificationsService, + private readonly campaignsService: CampaignsService, ) {} + /** + * Reconcile a campaign's stored `raisedAmount` against the on-chain Stellar + * account. This is the ONLY path that may write a corrected `raisedAmount` + * after accounting for approved/released `FundRelease` outflows. + * + * `dto.force` MUST be true to perform the write. Without it, the endpoint + * runs in dry-run mode: it returns the projected figures and an AuditLog + * row marked DRY_RUN so admins can inspect but nothing is mutated. + * + * Every invocation writes an `AuditLog` row with `action = ADMIN_ACTION` + * and `resourceType = 'campaign_balance_reconciliation'` so the trace is + * permanent and searchable. + */ + async reconcileCampaignBalance( + campaignId: string, + dto: ReconcileBalanceDto, + adminId: string, + adminEmail: string, + ): Promise { + const campaign = await this.prisma.campaign.findUnique({ + where: { id: campaignId }, + select: { id: true, contractId: true }, + }); + if (!campaign) { + throw new NotFoundException(`Campaign ${campaignId} not found`); + } + if (!campaign.contractId) { + throw new BadRequestException('Campaign has no contractId set'); + } + + // CampaignsService.getContractBalance is the single source of truth for + // the canonical per-asset net figure. It never writes to the database. + const report = await this.campaignsService.getContractBalance(campaignId); + + const applied = dto.force === true && report.discrepancyDetected; + + let auditLogId: string; + let writtenAmount: string | null = null; + + if (applied) { + const updated = await this.prisma.campaign.update({ + where: { id: campaignId }, + data: { raisedAmount: report.netAvailableByAssetTotal }, + select: { raisedAmount: true }, + }); + writtenAmount = updated.raisedAmount.toString(); + } + + const audit = await this.prisma.auditLog.create({ + data: { + userId: adminId, + action: 'ADMIN_ACTION', + resourceType: 'campaign_balance_reconciliation', + resourceId: campaignId, + details: JSON.stringify({ + kind: 'BALANCE_RECONCILED', + mode: applied ? 'WRITE' : 'DRY_RUN', + force: dto.force === true, + reason: dto.reason ?? null, + adminEmail, + contractId: campaign.contractId, + storedRaisedAmount: report.storedRaisedAmount, + netAvailableByAssetTotal: report.netAvailableByAssetTotal, + onChainTotal: report.onChainTotal, + netReleasedAmount: report.netReleasedAmount, + discrepancyDetected: report.discrepancyDetected, + writtenAmount, + }), + }, + }); + auditLogId = audit.id; + + return { + campaignId, + storedRaisedAmount: report.storedRaisedAmount, + netAvailableByAssetTotal: report.netAvailableByAssetTotal, + onChainTotal: report.onChainTotal, + netReleasedAmount: report.netReleasedAmount, + discrepancyDetected: report.discrepancyDetected, + auditLogId, + applied, + reason: dto.reason, + }; + } + /** Suspend a campaign with an audit log entry and creator notification */ async suspendCampaign( campaignId: string, diff --git a/src/admin/dtos/reconcile-balance.dto.ts b/src/admin/dtos/reconcile-balance.dto.ts new file mode 100644 index 0000000..1959e7b --- /dev/null +++ b/src/admin/dtos/reconcile-balance.dto.ts @@ -0,0 +1,21 @@ +import { IsBoolean, IsOptional, IsString, MaxLength } from 'class-validator'; + +/** + * Request body for POST /admin/campaigns/:id/reconcile-balance. + * + * `force` is REQUIRED. The endpoint refuses to write a corrected + * `Campaign.raisedAmount` unless the admin explicitly acknowledges that + * a non-readonly mutation is being performed. This is the audit gate that + * the issue (#1) asked us to add in place of the silent write. + * + * `reason` is optional human-readable text captured in the AuditLog row. + */ +export class ReconcileBalanceDto { + @IsBoolean() + force!: boolean; + + @IsOptional() + @IsString() + @MaxLength(500) + reason?: string; +} diff --git a/src/campaigns/campaigns.service.spec.ts b/src/campaigns/campaigns.service.spec.ts index 73184e8..802e5a6 100644 --- a/src/campaigns/campaigns.service.spec.ts +++ b/src/campaigns/campaigns.service.spec.ts @@ -71,3 +71,202 @@ describe('CampaignsService milestone target validation', () => { ); }); }); + +import { NotFoundException } from '@nestjs/common'; + +type Balance = { + assetCode: string; + assetIssuer?: string; + balance: string; + isNative: boolean; +}; + +const createService = ({ + campaign, + balances, + releases, +}: { + campaign: any; + balances: Balance[]; + releases: Array<{ amount: string | number }>; +}) => { + const prisma: any = { + campaign: { + findUnique: jest.fn().mockResolvedValue(campaign), + update: jest.fn(), + }, + fundRelease: { + findMany: jest.fn().mockResolvedValue(releases), + }, + }; + + const stellarTransactions: any = { + getContractBalances: jest.fn().mockResolvedValue(balances), + }; + + const service = new CampaignsService(prisma, stellarTransactions); + return { service, prisma, stellarTransactions }; +}; + +describe('CampaignsService.getContractBalance safety fixes (issue #1)', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('throws NotFoundException when the campaign does not exist', async () => { + const { service } = createService({ + campaign: null, + balances: [], + releases: [], + }); + await expect(service.getContractBalance('missing')).rejects.toBeInstanceOf( + NotFoundException, + ); + }); + + it('throws BadRequestException when the campaign has no contractId', async () => { + const { service } = createService({ + campaign: { id: 'c1', contractId: null, raisedAmount: 0 }, + balances: [], + releases: [], + }); + await expect(service.getContractBalance('c1')).rejects.toBeInstanceOf( + BadRequestException, + ); + }); + + it('XLM-only case: a single native balance matching the stored amount reports NO discrepancy', async () => { + const { service, prisma } = createService({ + campaign: { id: 'c1', contractId: 'CONTRACT', raisedAmount: 100 }, + balances: [ + { assetCode: 'XLM', balance: '100', isNative: true }, + ], + releases: [], + }); + + const report = await service.getContractBalance('c1'); + + expect(report.discrepancyDetected).toBe(false); + expect(report.netAvailableByAssetTotal).toBe('100'); + expect(report.netReleasedAmount).toBe('0'); + expect(report.onChainTotal).toBe('100'); + expect(report.perAsset).toHaveLength(1); + expect(report.perAsset[0]).toMatchObject({ + assetCode: 'XLM', + isNative: true, + grossOnChain: '100', + released: '0', + netAvailable: '100', + }); + // The fix to issue #1: never silently write raisedAmount. + expect(prisma.campaign.update).not.toHaveBeenCalled(); + }); + + it('multi-asset case: an issued-asset balance is NOT mixed into the XLM counter', async () => { + // 50 XLM on-chain + 1_000 of an issued credit asset. If the buggy + // implementation summed numerically, netAvailableByAssetTotal would + // equal 1050 and the discrepancy flag would be wrong. The fix keeps + // XLM as the canonical XLM-denominated figure (Campaign.raisedAmount + // is XLM-denominated), so USDC appears in `perAsset` but does NOT + // fold into the canonical total. + const { service, prisma } = createService({ + campaign: { id: 'c2', contractId: 'CONTRACT', raisedAmount: 80 }, + balances: [ + { assetCode: 'XLM', balance: '50', isNative: true }, + { + assetCode: 'USDC', + assetIssuer: 'G-ISSUER', + balance: '1000', + isNative: false, + }, + ], + releases: [], + }); + + const report = await service.getContractBalance('c2'); + + // XLM-only net figure is 50 vs stored 80 -> discrepancy, but the + // service MUST NOT silently write. The audit-gated write lives in + // AdminService.reconcileCampaignBalance, not here. + expect(report.discrepancyDetected).toBe(true); + expect(report.netAvailableByAssetTotal).toBe('50'); + expect(report.onChainTotal).toBe('50'); + expect(report.perAsset).toHaveLength(2); + + const xlm = report.perAsset.find((p) => p.isNative); + const usdc = report.perAsset.find((p) => !p.isNative); + expect(xlm).toMatchObject({ + assetCode: 'XLM', + grossOnChain: '50', + released: '0', + netAvailable: '50', + }); + expect(usdc).toMatchObject({ + assetCode: 'USDC', + assetIssuer: 'G-ISSUER', + grossOnChain: '1000', + released: '0', + netAvailable: '1000', + }); + + expect(prisma.campaign.update).not.toHaveBeenCalled(); + }); + + it('post-release netted case: stored raisedAmount equals on-chain + APPROVED/RELEASED outflows', async () => { + // Campaign has 15 XLM of `raisedAmount` recorded in the DB. The contract + // account currently holds 10 XLM (5 XLM drained via an APPROVED release). + // naive on-chain comparison would flag this as a discrepancy; the fix + // nets against approved/released fund releases so 10 + 5 == 15. + const { service, prisma } = createService({ + campaign: { id: 'c3', contractId: 'CONTRACT', raisedAmount: 15 }, + balances: [ + { assetCode: 'XLM', balance: '10', isNative: true }, + ], + releases: [{ amount: 5 }], + }); + + const report = await service.getContractBalance('c3'); + + expect(report.discrepancyDetected).toBe(false); + expect(report.onChainTotal).toBe('10'); + expect(report.netReleasedAmount).toBe('5'); + expect(report.netAvailableByAssetTotal).toBe('15'); + expect(prisma.campaign.update).not.toHaveBeenCalled(); + }); + + it('releases that exceed on-chain holdings surface a discrepancy even after netting', async () => { + // Without any on-chain balance but with a prior APPROVED release of 5 + // XLM, the canonical net is 0 + 5 = 5 while stored is 10. The flag + // surfaces the case for an admin override. The service still does not + // write to the DB. + const { service, prisma } = createService({ + campaign: { id: 'c4', contractId: 'CONTRACT', raisedAmount: 10 }, + balances: [{ assetCode: 'XLM', balance: '0', isNative: true }], + releases: [{ amount: 5 }], + }); + + const report = await service.getContractBalance('c4'); + + expect(report.netAvailableByAssetTotal).toBe('5'); + expect(report.netReleasedAmount).toBe('5'); + expect(report.discrepancyDetected).toBe(true); + expect(report.storedRaisedAmount).toBe('10'); + expect(prisma.campaign.update).not.toHaveBeenCalled(); + }); + + it('refuses to silently write raisedAmount when the figures diverge', async () => { + const { service, prisma } = createService({ + campaign: { id: 'c5', contractId: 'CONTRACT', raisedAmount: 9999 }, + balances: [{ assetCode: 'XLM', balance: '5', isNative: true }], + releases: [], + }); + + const report = await service.getContractBalance('c5'); + + expect(report.discrepancyDetected).toBe(true); + expect(report.netAvailableByAssetTotal).toBe('5'); + expect(report.storedRaisedAmount).toBe('9999'); + // The critical fix: this method must NEVER write to the DB. + expect(prisma.campaign.update).not.toHaveBeenCalled(); + }); +}); diff --git a/src/campaigns/campaigns.service.ts b/src/campaigns/campaigns.service.ts index 04b14a0..47e21e7 100644 --- a/src/campaigns/campaigns.service.ts +++ b/src/campaigns/campaigns.service.ts @@ -14,9 +14,24 @@ import { import { CreateCampaignDto } from './dto/create-campaign.dto'; import { UpdateCampaignDto } from './dto/update-campaign.dto'; import type { CreateUpdateDto } from './dto/create-update.dto'; -import { ContractBalanceResponseDto } from './dto/contract-balance.dto'; +import { + ContractBalanceResponseDto, + PerAssetBalanceDto, +} from './dto/contract-balance.dto'; const MIN_MILESTONE_TARGET_AMOUNT = 0.0000001; +const DISCREPANCY_TOLERANCE = new Prisma.Decimal('0.0001'); + +function parseDecimalOrZero(raw: string | number | undefined | null): string { + const s = String(raw ?? '0').trim(); + if (s === '') return '0'; + const n = Number(s); + if (!Number.isFinite(n)) return '0'; + // Horizon can return negative balances for accounts with buy-liabilities; + // floor them at zero rather than introducing a signed arithmetic that + // downstream `raisedAmount` storage cannot accept. + return n >= 0 ? s : '0'; +} @Injectable() export class CampaignsService { @@ -199,10 +214,31 @@ export class CampaignsService { } /** - * Fetch on-chain contract balance from Stellar and compare with stored raisedAmount. - * Auto-corrects discrepancies. + * Fetch on-chain contract balance from Stellar and compute a deterministic + * comparison against the stored `Campaign.raisedAmount`. + * + * SAFETY: This method is READ-ONLY with respect to `Campaign.raisedAmount`. + * It computes two side-by-side figures: + * + * * The on-chain account's per-asset balances (raw, in each asset's native + * decimals), summed PER ASSET and never as a mixed-denomination aggregate. + * * The total sum of APPROVED/RELEASED `FundRelease.amount` values, summed + * PER ASSET (XLM releases attach to the `native` slot, issued-asset + * releases attach to matching `code:issuer` slots). + * + * The on-chain amount that "should" still belong to the campaign is + * `grossOnChain + released` per asset, because released funds have already + * been moved off the contract account. Each asset's contribution to the + * canonical `Campaign.raisedAmount` is independent of the others. + * + * Discrepancies are REPORTED via `discrepancyDetected` only. The stored + * `Campaign.raisedAmount` is NEVER mutated by this method. Correcting a + * discrepancy requires an explicit admin invocation through + * `AdminService.reconcileCampaignBalance`, which writes an `AuditLog`. */ - async getContractBalance(campaignId: string) { + async getContractBalance( + campaignId: string, + ): Promise { const campaign = await this.prisma.campaign.findUnique({ where: { id: campaignId }, }); @@ -219,35 +255,100 @@ export class CampaignsService { campaign.contractId, ); - // Calculate total on-chain balance - let onChainTotal = 0; - for (const b of balances) { - onChainTotal += parseFloat(b.balance); - } - - const storedRaisedAmount = parseFloat(campaign.raisedAmount.toString()); - const discrepancyDetected = - Math.abs(onChainTotal - storedRaisedAmount) > 0.0001; + // The Prisma `FundRelease` schema does not record `assetCode` / `assetIssuer`, + // so per-asset release netting is unimplementable today without a schema + // migration. We treat the released sum as a single XLM-denominated bucket + // since `Campaign.raisedAmount` (and `Milestone.targetAmount`) are stored + // in XLM-equivalent units. The aggregate is reported on the XLM row of + // `perAsset` only; non-XLM asset rows show their on-chain balance without + // any released offset. + const totalReleased = await this.sumApprovedReleasedAmount(campaignId); + const releasedApplied = totalReleased.gt(0); + + const perAsset: PerAssetBalanceDto[] = balances.map((b) => { + const gross = new Prisma.Decimal(parseDecimalOrZero(b.balance)); + const isNative = b.isNative; + const released = isNative && releasedApplied ? totalReleased : new Prisma.Decimal(0); + const net = isNative ? gross.plus(released) : gross; + return { + assetCode: b.assetCode, + assetIssuer: b.assetIssuer, + isNative, + grossOnChain: gross.toString(), + released: released.toString(), + netAvailable: net.toString(), + }; + }); - // If discrepancy detected, update the stored raisedAmount - if (discrepancyDetected) { - await this.prisma.campaign.update({ - where: { id: campaignId }, - data: { - raisedAmount: onChainTotal, - }, - }); - } + // `netAvailableByAssetTotal` is the canonical XLM-denominated balance + // (the only denomination `Campaign.raisedAmount` is stored in). We + // deliberately sum ONLY native (XLM) per-asset nets so this figure is + // safe to compare against the stored value. Non-XLM assets remain + // visible in the `perAsset` array but are NEVER folded into the + // canonical total — folding them would re-create the mixed- + // denomination bug that prompted this fix. + const netAvailableByAssetTotal = perAsset + .filter((p) => p.isNative) + .reduce((sum, p) => sum.plus(p.netAvailable), new Prisma.Decimal(0)); + + const netReleasedAmount = totalReleased; + + // `onChainTotal` is a backwards-compatible diagnostic exposed to + // clients: SUM of on-chain `balances[].balance` strings for native + // (XLM) assets only. Mirrors the behaviour callers would expect from + // the pre-fix endpoint when the campaign accepted only XLM. + const onChainTotal = perAsset + .filter((p) => p.isNative) + .reduce((sum, p) => sum.plus(p.grossOnChain), new Prisma.Decimal(0)); + + const storedRaisedAmount = new Prisma.Decimal( + campaign.raisedAmount.toString(), + ); + const discrepancyDetected = netAvailableByAssetTotal + .minus(storedRaisedAmount) + .abs() + .gt(DISCREPANCY_TOLERANCE); return { contractId: campaign.contractId, balances, - storedRaisedAmount: campaign.raisedAmount.toString(), + perAsset, + netAvailableByAssetTotal: netAvailableByAssetTotal.toString(), + netReleasedAmount: netReleasedAmount.toString(), onChainTotal: onChainTotal.toString(), + storedRaisedAmount: storedRaisedAmount.toString(), discrepancyDetected, }; } + /** + * Sum the `amount` of every `FundRelease` row for this campaign whose + * status is APPROVED or RELEASED. Releases are conceptually XLM-denominated + * for accounting purposes because `Campaign.raisedAmount` and + * `Milestone.targetAmount` are stored in XLM-equivalent units. + * + * NOTE: A future schema migration that adds `assetCode`/`assetIssuer` to + * `FundRelease` would let us net per-asset instead. Today the field is not + * tracked, so we use this conservative single-bucket sum. + */ + private async sumApprovedReleasedAmount( + campaignId: string, + ): Promise { + const releases = await this.prisma.fundRelease.findMany({ + where: { + campaignId, + status: { in: ['APPROVED', 'RELEASED'] }, + }, + select: { amount: true }, + }); + + return releases.reduce( + (sum, r) => + sum.plus(new Prisma.Decimal(r.amount.toString())), + new Prisma.Decimal(0), + ); + } + /** Recalculate a campaign's raisedAmount from confirmed donations */ async recalculateCampaignStats(campaignId: string) { const agg = await this.prisma.donation.aggregate({ diff --git a/src/campaigns/dto/contract-balance.dto.ts b/src/campaigns/dto/contract-balance.dto.ts index 2646908..95af70a 100644 --- a/src/campaigns/dto/contract-balance.dto.ts +++ b/src/campaigns/dto/contract-balance.dto.ts @@ -6,11 +6,49 @@ export class AssetBalanceDto { isNative: boolean; } +/** + * Per-asset projection used for a deterministic reconciliation figure. + * `grossOnChain` is the raw on-chain balance for one asset. `released` is + * the sum of APPROVED/RELEASED FundRelease amounts denominated in the same + * asset (XLM releases map to native; non-native asset releases require an + * issuer+code match). `netAvailable` is `grossOnChain + released` since + * released funds have already been moved off the contract account. + */ +export class PerAssetBalanceDto { + assetCode: string; + assetIssuer?: string; + isNative: boolean; + grossOnChain: string; + released: string; + netAvailable: string; +} + export class ContractBalanceResponseDto { contractId: string; balances: AssetBalanceDto[]; - totalValueInXlm?: string; + perAsset: PerAssetBalanceDto[]; + /** + * Canonical campaign balance in XLM (the only denomination + * `Campaign.raisedAmount` is stored in). Equals the sum of `perAsset[*].netAvailable` + * for native (XLM) assets only. Non-XLM asset contributions are reported in + * `perAsset` but NEVER folded into this figure. + */ + netAvailableByAssetTotal: string; + /** + * Native (XLM) on-chain balance, summed across the contract's XLM balances. + * Provided for backwards-compatibility with the original endpoint shape. + */ + onChainTotal: string; + /** + * Sum of APPROVED/RELEASED FundRelease amounts for native (XLM) releases + * only. Per-asset non-XLM release amounts are reported in `perAsset`. + */ + netReleasedAmount: string; + /** + * True iff the difference between `netAvailableByAssetTotal` and the + * stored `Campaign.raisedAmount` exceeds 0.0001. The handler MUST NOT + * silently persist a correction — see `/admin/campaigns/:id/reconcile-balance`. + */ discrepancyDetected: boolean; storedRaisedAmount: string; - onChainTotal: string; }