diff --git a/.env.example b/.env.example index 8e7af699..543fb4ef 100644 --- a/.env.example +++ b/.env.example @@ -53,28 +53,8 @@ DB_LEAK_THRESHOLD_MS=30000 # ── Webhook Signature Verification ──────────────────────────────────────────── # HMAC-SHA256 signing key used to sign and verify outbound/inbound webhooks. # Rotate with POST /api/v1/webhooks/rotate-key (overlap window: 24h) -# REQUIRED in production: at least 16 random characters. Startup fails if unset -# or left at the example value, because a known key lets anyone forge signatures. WEBHOOK_SIGNING_KEY=change-me-to-a-long-random-secret -# ── Security Secrets (required in production) ───────────────────────────────── -# The following secrets have no safe fallback. In production the process refuses -# to start without them; outside production an ephemeral value is generated and -# a warning is logged (which means values will not survive a restart). - -# Pepper for API key digests (>=16 chars). Without it a leaked key table can be -# attacked offline; with it, a leaked table is useless on its own. -API_KEY_HASH_PEPPER= - -# HMAC key for opaque pagination cursors (>=16 chars). Cursors carry a sort -# field that is interpolated into SQL, so a known key would allow forging one. -CURSOR_HMAC_SECRET= - -# Set to `true` ONLY when an authenticating proxy sits in front of this process -# and strips `x-subscription-tier` from inbound requests. Otherwise rate-limit -# tiers are resolved from the server-side record for each API key. -TRUST_TIER_HEADER=false - # ── Zapier Integration ───────────────────────────────────────────────────────── # No server-side secrets required — merchants authenticate with their API keys. # Set this to restrict which origins can reach the Zapier endpoints. diff --git a/backend/billing/jobs/index.ts b/backend/billing/jobs/index.ts index fa0f7568..6b4fe8ef 100644 --- a/backend/billing/jobs/index.ts +++ b/backend/billing/jobs/index.ts @@ -26,7 +26,6 @@ export { export type { RevenueSchedule, RevenueScheduleEntry, - EntryRecognitionProgress, RecognitionJournalEntry, RevenueRecognitionRepository, RecognitionJobMetrics, diff --git a/backend/billing/jobs/monthlyRevenueRecognitionJob.ts b/backend/billing/jobs/monthlyRevenueRecognitionJob.ts index bd5580a7..e7e37213 100644 --- a/backend/billing/jobs/monthlyRevenueRecognitionJob.ts +++ b/backend/billing/jobs/monthlyRevenueRecognitionJob.ts @@ -19,12 +19,6 @@ export interface RevenueScheduleEntry { periodEnd: number; // Unix ms recognisedAmount: number; isRecognised: boolean; - /** - * Cumulative amount already recognised for this entry across all prior - * runs. Optional so schedules persisted before progress tracking existed - * still load; treat a missing value as 0. - */ - recognisedToDate?: number; } export interface RevenueSchedule { @@ -48,12 +42,6 @@ export interface RecognitionJournalEntry { type: 'scheduled' | 'accelerated' | 'partial'; } -/** Per-entry cumulative recognition progress recorded by a run. */ -export interface EntryRecognitionProgress { - periodStart: number; - recognisedToDate: number; -} - export interface RevenueRecognitionRepository { /** Return all schedules where at least one entry is not yet recognised. */ getPendingSchedules(): Promise; @@ -66,19 +54,6 @@ export interface RevenueRecognitionRepository { ): Promise; /** Mark individual schedule entries as recognised. */ markEntriesRecognised(subscriptionId: string, entryPeriodStarts: number[]): Promise; - /** - * Persist cumulative per-entry recognition progress. - * - * Optional. Without it a partially-elapsed entry cannot record how much of - * itself it has already recognised, and the next run re-recognises the whole - * elapsed portion — inflating recognised revenue on every run. When the - * method is absent the job degrades to recognising each entry only once, - * at its full amount, which is slower but never over-counts. - */ - recordEntryProgress?( - subscriptionId: string, - progress: EntryRecognitionProgress[] - ): Promise; } export interface RecognitionJobMetrics { @@ -95,12 +70,6 @@ function nextId(): string { return `rev-${Date.now().toString(36)}-${(++_seq).toString(36)}`; } -/** Rounds to minor units, mapping non-finite input to zero. */ -function roundMoney(amount: number): number { - if (!Number.isFinite(amount)) return 0; - return Math.round((amount + Number.EPSILON * Math.sign(amount)) * 100) / 100; -} - /** * Determine how much of a single schedule entry to recognise as of `now`. * Returns 0 for entries that haven't started yet or amount = 0 (free trial). @@ -181,48 +150,13 @@ export class MonthlyRevenueRecognitionJob { for (const schedule of schedules) { localMetrics.schedulesProcessed += 1; const recognisedPeriodStarts: number[] = []; - const progressUpdates: EntryRecognitionProgress[] = []; let totalRecognisedForSchedule = 0; - // Deferred revenue is what was contractually billed but not yet - // recognised, i.e. the schedule total minus everything recognised on - // prior runs and earlier in *this* run. Ignoring prior runs produced a - // deferred balance that grew without bound. - const previouslyRecognised = (schedule.entries ?? []).reduce( - (sum, entry) => sum + (entry.recognisedToDate ?? 0), - 0 - ); - let recognisedSoFar = previouslyRecognised; - - const canTrackProgress = typeof this.repo.recordEntryProgress === 'function'; - for (const entry of schedule.entries) { - const { amount: cumulativeTarget, type } = computeEntryRecognition(entry, now); - if (!type || cumulativeTarget <= 0) continue; - - // Recognise only the increment since the last run. Recognising the - // full elapsed portion again on every run is what compounded the - // misstatement day after day. - const alreadyRecognised = entry.recognisedToDate ?? 0; - const amount = canTrackProgress - ? roundMoney(Math.max(cumulativeTarget - alreadyRecognised, 0)) - : // Without progress tracking, only recognise an entry once its - // period has fully elapsed, so it is never recognised twice. - type === 'partial' - ? 0 - : roundMoney(cumulativeTarget); - - if (amount <= 0) { - if (type === 'scheduled' || type === 'accelerated') { - // Already fully recognised on a prior run; close it out. - recognisedPeriodStarts.push(entry.periodStart); - } - continue; - } + const { amount, type } = computeEntryRecognition(entry, now); + if (!type || amount <= 0) continue; - const deferredBefore = roundMoney( - Math.max(schedule.totalAmount - recognisedSoFar, 0) - ); + const deferredBefore = schedule.totalAmount - totalRecognisedForSchedule; const journalEntry: RecognitionJournalEntry = { id: nextId(), subscriptionId: schedule.subscriptionId, @@ -231,33 +165,19 @@ export class MonthlyRevenueRecognitionJob { periodEnd: entry.periodEnd, recognisedAmount: amount, deferredBefore, - // Deferred can never go negative: recognising more than was - // deferred would be reported as an amount owed back to the merchant. - deferredAfter: roundMoney(Math.max(deferredBefore - amount, 0)), + deferredAfter: deferredBefore - amount, createdAt: now, type, }; try { await this.repo.writeJournalEntry(journalEntry); - recognisedSoFar += amount; totalRecognisedForSchedule += amount; localMetrics.entriesRecognised += 1; localMetrics.totalAmountRecognised += amount; - progressUpdates.push({ - periodStart: entry.periodStart, - recognisedToDate: roundMoney(alreadyRecognised + amount), - }); - - // Only close out entries whose period has fully elapsed AND that - // are now fully recognised. - const recognisedTotal = roundMoney(alreadyRecognised + amount); - if ( - type === 'scheduled' || - type === 'accelerated' || - recognisedTotal >= roundMoney(entry.recognisedAmount) - ) { + // Only mark fully-elapsed entries as done. + if (type === 'scheduled' || type === 'accelerated') { recognisedPeriodStarts.push(entry.periodStart); } } catch (err) { @@ -275,12 +195,6 @@ export class MonthlyRevenueRecognitionJob { recognisedDelta: totalRecognisedForSchedule, deferredDelta: -totalRecognisedForSchedule, }); - if (progressUpdates.length > 0 && canTrackProgress) { - await this.repo.recordEntryProgress!( - schedule.subscriptionId, - progressUpdates - ); - } if (recognisedPeriodStarts.length > 0) { await this.repo.markEntriesRecognised( schedule.subscriptionId, diff --git a/backend/gateway/index.ts b/backend/gateway/index.ts index 2ba528b1..a93d83ec 100644 --- a/backend/gateway/index.ts +++ b/backend/gateway/index.ts @@ -40,19 +40,13 @@ export function createGateway(options: GatewayOptions = {}): Application { app.disable('x-powered-by'); - // Reduced from the previous blanket 10mb. A JSON control-plane body has no - // legitimate need to be that large, and the old limit let a single - // unauthenticated request buffer 10 MB per connection into memory. - app.use(express.json({ limit: '256kb' })); + app.use(express.json({ limit: '10mb' })); // ── Response envelope header ────────────────────────────────────────── app.use((_req: Request, res: Response, next: NextFunction) => { res.setHeader(API_VERSION_HEADER, API_VERSION_VALUE); res.setHeader('X-Content-Type-Options', 'nosniff'); res.setHeader('X-Frame-Options', 'DENY'); - // The gateway previously set no caching or referrer policy at all. - res.setHeader('Referrer-Policy', 'no-referrer'); - res.setHeader('X-DNS-Prefetch-Control', 'off'); next(); }); diff --git a/backend/integrations/zapier/ZapierIntegrationService.ts b/backend/integrations/zapier/ZapierIntegrationService.ts index c6254940..2643e50a 100644 --- a/backend/integrations/zapier/ZapierIntegrationService.ts +++ b/backend/integrations/zapier/ZapierIntegrationService.ts @@ -18,7 +18,6 @@ */ import crypto from 'crypto'; -import { validateOutboundUrl } from '../../shared/security/outboundUrlGuard'; // ── Types ───────────────────────────────────────────────────────────────────── @@ -294,17 +293,12 @@ export class ZapierIntegrationService { // ── Hook Subscription Management ────────────────────────────────────────── registerHook(input: RegisterHookInput): ZapierHookSubscription { - // Re-validated at the service boundary, not only in the router: hooks can - // also be registered programmatically, and every stored `targetUrl` is - // later fetched server-side. - const target = validateOutboundUrl(input.targetUrl); - const id = this.generateId('zhook'); const now = new Date().toISOString(); const hook: ZapierHookSubscription = { id, merchantId: input.merchantId, - targetUrl: target.toString(), + targetUrl: input.targetUrl, event: input.event, signingSecret: this.generateSigningSecret(), active: true, @@ -386,11 +380,7 @@ export class ZapierIntegrationService { const signature = this.signPayload(body, hook.signingSecret); try { - // Re-validate at send time. A hook persisted before this guard existed - // would otherwise remain an SSRF vector forever. - const target = validateOutboundUrl(hook.targetUrl); - - const response = await fetchImpl(target.toString(), { + const response = await fetchImpl(hook.targetUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', diff --git a/backend/integrations/zapier/zapierRouter.ts b/backend/integrations/zapier/zapierRouter.ts index a6eb0f1a..c44d3227 100644 --- a/backend/integrations/zapier/zapierRouter.ts +++ b/backend/integrations/zapier/zapierRouter.ts @@ -32,9 +32,6 @@ import { type RegisterHookInput, type ZapierTriggerEvent, } from './ZapierIntegrationService'; -import { - validateOutboundUrl, -} from '../../shared/security/outboundUrlGuard'; function asyncHandler(fn: (req: Request, res: Response) => Promise) { return (req: Request, res: Response): void => { @@ -125,23 +122,9 @@ export function createZapierRouter(service: ZapierIntegrationService): Router { return; } - // The registered URL is fetched server-side on every event delivery, so - // an unvalidated value here is an SSRF primitive: it would let a caller - // point the platform at cloud metadata, loopback or RFC1918 services. - let normalizedTarget: string; - try { - normalizedTarget = validateOutboundUrl(targetUrl).toString(); - } catch (error) { - res.status(400).json({ - success: false, - error: error instanceof Error ? error.message : 'Invalid targetUrl', - }); - return; - } - const input: RegisterHookInput = { merchantId, - targetUrl: normalizedTarget, + targetUrl, event: event as ZapierTriggerEvent, }; diff --git a/backend/server.ts b/backend/server.ts index b1c1fcfc..c85596b9 100644 --- a/backend/server.ts +++ b/backend/server.ts @@ -157,31 +157,13 @@ function decodeJwtSubject(token: string): string | undefined { } } -/** - * Resolve the caller's rate-limit tier for an API key. - * - * This previously read the client-supplied `x-subscription-tier` header - * verbatim, so any caller presenting a key could send - * `X-Subscription-Tier: enterprise` and receive the 10,000 req/hr allowance - * instead of the FREE tier's 100 req/hr — a 100× rate-limit bypass requiring - * no privilege at all. - * - * The tier now comes from the server-side record for that key. The header is - * honoured only when an explicit trusted-proxy opt-in is configured, which - * suits deployments where an authenticating gateway sits in front of this - * process and strips the header from inbound requests. - */ -function resolveTierForApiKey(apiKey: string, req: { - headers: Record; -}): SubscriptionTier { - if (process.env['TRUST_TIER_HEADER'] === 'true') { - const raw = req.headers['x-subscription-tier']; - const value = typeof raw === 'string' ? raw : Array.isArray(raw) ? raw[0] : undefined; - const parsed = parseSubscriptionTier(value ?? null); - if (parsed) return parsed; - } - - return rateLimitingService.getTierForApiKey(apiKey); +/** Resolve the caller tier from the x-subscription-tier header (default FREE). */ +function resolveTierFromRequest( + req: { headers: Record }, +): SubscriptionTier { + const raw = req.headers['x-subscription-tier']; + const value = typeof raw === 'string' ? raw : Array.isArray(raw) ? raw[0] : undefined; + return parseSubscriptionTier(value ?? null) ?? SubscriptionTier.FREE; } /** @@ -223,10 +205,11 @@ function buildRateLimitMiddleware() { '/metrics/pool', '/metrics/build', ], - - // Per-key tier: resolved from the server-side record, not from a - // client-supplied header. See resolveTierForApiKey. - getTier: (apiKey, req) => resolveTierForApiKey(apiKey, req), + // Per-key tier: read x-subscription-tier header; defaults to FREE. + getTier: (apiKey, req) => { + void apiKey; + return resolveTierFromRequest(req); + }, // Per-user aggregate limiting: x-user-id header or Bearer JWT sub claim. getUserId: (req) => resolveUserIdFromRequest(req), }); @@ -305,32 +288,21 @@ class HttpError extends Error { /** Upper bound on an ingested CI report body (1 MiB). */ const MAX_BUILD_REPORT_BYTES = 1024 * 1024; -/** - * Default ceiling for any other JSON request body (256 KiB). - * - * `maxBytes` was optional and 31 of 32 call sites omitted it, so every - * non-build-metrics route buffered an unbounded amount into memory before - * parsing. A single oversized POST was enough to exhaust the heap. A sane - * default is applied everywhere; individual routes may still raise it. - */ -const MAX_DEFAULT_BODY_BYTES = 256 * 1024; - /** * Read and parse a JSON request body. * - * @param maxBytes - Optional explicit cap; defaults to MAX_DEFAULT_BODY_BYTES. - * Exceeding it aborts with 413 so an untrusted upload cannot buffer - * unbounded memory. Set `Infinity` only for genuinely trusted routes. + * @param maxBytes - Optional hard cap; exceeding it aborts with 413 so an + * untrusted upload cannot buffer unbounded memory. Omit for trusted routes + * to preserve the previous unlimited behaviour. */ async function readJsonBody(req: http.IncomingMessage, maxBytes?: number): Promise { - const limit = maxBytes ?? MAX_DEFAULT_BODY_BYTES; const chunks: Buffer[] = []; let received = 0; for await (const chunk of req) { const buf = typeof chunk === 'string' ? Buffer.from(chunk) : chunk; received += buf.length; - if (Number.isFinite(limit) && received > limit) { - throw new HttpError(413, `request body exceeds ${limit} bytes`); + if (maxBytes !== undefined && received > maxBytes) { + throw new HttpError(413, `request body exceeds ${maxBytes} bytes`); } chunks.push(buf); } @@ -1985,21 +1957,9 @@ feat/stellar-wave-issues // HubSpot — POST /hubspot/webhook (ingest from HubSpot) // ----------------------------------------------------------------- if (pathname === '/hubspot/webhook' && method === 'POST') { - // Bounded read: this accumulated every chunk with no cap before the - // signature was even checked, so an anonymous caller could exhaust - // memory with a single oversized POST. const rawBody = await new Promise((resolve, reject) => { const chunks: Buffer[] = []; - let received = 0; - req.on('data', (c: Buffer) => { - received += c.length; - if (received > MAX_DEFAULT_BODY_BYTES) { - reject(new HttpError(413, `request body exceeds ${MAX_DEFAULT_BODY_BYTES} bytes`)); - req.destroy(); - return; - } - chunks.push(c); - }); + req.on('data', (c: Buffer) => chunks.push(c)); req.on('end', () => resolve(Buffer.concat(chunks).toString('utf8'))); req.on('error', reject); }); diff --git a/backend/server/createApiServer.ts b/backend/server/createApiServer.ts index 10c1074a..1775a52e 100644 --- a/backend/server/createApiServer.ts +++ b/backend/server/createApiServer.ts @@ -19,8 +19,6 @@ feat/stellar-wave-issues main */ -import { randomBytes } from 'node:crypto'; - import express, { type Express, type Request, type Response } from 'express'; import { cacheHeadersMiddleware } from '../shared/middleware'; import { createCspMiddleware, DEFAULT_CSP_POLICY } from '../services/shared/cspMiddleware'; @@ -124,45 +122,7 @@ const renewalScheduler = new RenewalReminderScheduler(calendarSyncService); const calendarController = createCalendarSyncController({ syncService: calendarSyncService }); const reminderController = createRenewalReminderController({ scheduler: renewalScheduler }); -/** - * Resolves the outbound webhook signing key. - * - * This previously fell back to the literal `'default-dev-key-change-in-prod'`. - * Because the same key signs every webhook the platform emits, a single - * deployed default meant anyone reading the repository could forge a valid - * signature for arbitrary merchant webhooks. - * - * In production an unset or default value is a hard startup failure. Outside - * production an ephemeral random key is generated so local development still - * works without silently weakening verification. - */ -function resolveWebhookSigningKey(): string { - const configured = process.env['WEBHOOK_SIGNING_KEY']; - const placeholder = 'default-dev-key-change-in-prod'; - - if (configured && configured !== placeholder && configured.length >= 16) { - return configured; - } - - if (process.env['NODE_ENV'] === 'production') { - throw new Error( - 'WEBHOOK_SIGNING_KEY must be set to a random value of at least 16 characters in production.' - ); - } - - if (configured === placeholder) { - console.warn( - '[createApiServer] WEBHOOK_SIGNING_KEY is still the public default; using an ephemeral key for this process.' - ); - } - - return randomBytes(32).toString('hex'); -} - -const webhookKeyStore = new KeyStore(resolveWebhookSigningKey()); - -/** Ceiling for request bodies accepted by the public API (256 KiB). */ -const MAX_JSON_BODY = '256kb'; +const webhookKeyStore = new KeyStore(process.env['WEBHOOK_SIGNING_KEY'] ?? 'default-dev-key-change-in-prod'); export interface CreateApiServerOptions { /** Optional middleware applied before cache headers (e.g. auth). */ @@ -212,13 +172,11 @@ export function createApiServer(options: CreateApiServerOptions = {}): Express { // first legitimate delivery as a duplicate and the router would answer // `200 { duplicate: true }` without ever dispatching the event. // nosemgrep: subtrackr.csrf.state-changing-route-without-csrf -- reason: body-parser-only registration, no handler and no state change; the request is HMAC-verified inside the Stripe router, which fails closed with 503 when unconfigured and 401 on a bad signature -- expires: 2027-04-16 - app.post('/api/v1/stripe/billing/webhook', express.raw({ type: '*/*', limit: MAX_JSON_BODY })); + app.post('/api/v1/stripe/billing/webhook', express.raw({ type: '*/*' })); // nosemgrep: subtrackr.csrf.state-changing-route-without-csrf -- reason: body-parser-only registration, no handler and no state change; the request is HMAC-verified inside the Paddle router, which fails closed with 503 when unconfigured and 401 on a bad signature -- expires: 2027-04-16 - app.post('/api/v1/paddle/webhook', express.raw({ type: '*/*', limit: MAX_JSON_BODY })); + app.post('/api/v1/paddle/webhook', express.raw({ type: '*/*' })); - // An explicit cap keeps a single oversized POST from buffering the whole - // body into memory before it is parsed. - app.use(express.json({ limit: MAX_JSON_BODY })); + app.use(express.json()); // ── CSRF protection — #1770 ─────────────────────────────────────────────── // Double-submit cookie strategy: safe methods (GET/HEAD/OPTIONS) issue a diff --git a/backend/services/analytics/subscriptionAnalyticsService.ts b/backend/services/analytics/subscriptionAnalyticsService.ts index 8f811ca4..226b3d68 100644 --- a/backend/services/analytics/subscriptionAnalyticsService.ts +++ b/backend/services/analytics/subscriptionAnalyticsService.ts @@ -317,14 +317,9 @@ export class SubscriptionAnalyticsService { // ── Private helpers ───────────────────────────────────────────────────────── private _monthlyRevenue(sub: Subscription): number { - // Normalise any billing cycle to monthly revenue. `ANNUAL` was missing - // here, so an annual subscription's full yearly price was reported as - // MRR — inflating MRR/ARR/churn by 12×. `monthlyEquivalent` also keeps - // the weekly factor identical to the frontend (52/12, not 4.345). - if (sub.billingCycle === BillingCycle.YEARLY || sub.billingCycle === BillingCycle.ANNUAL) { - return sub.price / 12; - } - if (sub.billingCycle === BillingCycle.WEEKLY) return sub.price * (52 / 12); + // Normalise any billing cycle to monthly revenue + if (sub.billingCycle === BillingCycle.YEARLY) return sub.price / 12; + if (sub.billingCycle === BillingCycle.WEEKLY) return sub.price * 4.345; return sub.price; } } diff --git a/backend/services/auth/domain/ApiKeyRotationService.ts b/backend/services/auth/domain/ApiKeyRotationService.ts index 9abd7900..7f84952b 100644 --- a/backend/services/auth/domain/ApiKeyRotationService.ts +++ b/backend/services/auth/domain/ApiKeyRotationService.ts @@ -1,4 +1,4 @@ -import { randomBytes, createHmac } from 'crypto'; +import { randomBytes, createHash } from 'crypto'; import { AuthError } from '../errors'; import { logger } from '../../shared/logging'; import type { ApiKeyRecord, ApiKeyRotationPolicy, IApiKeyRotationService } from '../interfaces'; @@ -8,59 +8,8 @@ const KEY_BYTE_LENGTH = 32; const KEY_HASH_ALGORITHM = 'sha256'; const MAX_HISTORY = 5; -/** - * Server-side pepper for API key hashing. - * - * API keys are high-entropy random values, so a plain hash resists brute - * force — but an unsalted SHA-256 is still trivially attackable in bulk via - * precomputation, and it means a database disclosure immediately yields - * directly comparable key material. A keyed HMAC removes that: without the - * pepper, a leaked table cannot be attacked offline at all. - * - * Required in production; ephemeral per-process outside it (with a warning), - * because a checked-in fallback pepper would be no better than none. - */ -function resolveApiKeyPepper(): string { - const configured = process.env['API_KEY_HASH_PEPPER']; - - if (configured && configured.trim().length >= 16) { - return configured; - } - - if (process.env['NODE_ENV'] === 'production') { - throw new Error( - 'API_KEY_HASH_PEPPER must be set to a random value of at least 16 characters in production.' - ); - } - - console.warn( - '[ApiKeyRotationService] API_KEY_HASH_PEPPER is not configured; using an ephemeral pepper. API keys will not resolve across restarts.' - ); - - return randomBytes(32).toString('hex'); -} - -let _pepper: string | null = null; - -/** Test seam: forces a specific pepper, or `null` to re-resolve. */ -export function __setApiKeyPepperForTesting(pepper: string | null): void { - _pepper = pepper; -} - -function apiKeyPepper(): string { - if (_pepper === null) { - _pepper = resolveApiKeyPepper(); - } - return _pepper; -} - -/** - * Deterministic keyed digest of an API key. - * - * Stays deterministic so stored hashes remain searchable by lookup. - */ export function hashApiKey(rawKey: string): string { - return createHmac(KEY_HASH_ALGORITHM, apiKeyPepper()).update(rawKey).digest('hex'); + return createHash(KEY_HASH_ALGORITHM).update(rawKey).digest('hex'); } /** diff --git a/backend/services/batchChargeService.ts b/backend/services/batchChargeService.ts index fa0e8833..63969fa5 100644 --- a/backend/services/batchChargeService.ts +++ b/backend/services/batchChargeService.ts @@ -43,7 +43,7 @@ export interface BatchChargeResult { export class BatchChargeService { private intervalHandle: ReturnType | null = null; private lastMatchTimestamp = 0; - private runHistory: BatchChargeResult[] = []; + private RunHistory: BatchChargeResult[] = []; private maxHistory = 50; private cronExpression = '0 0 * * *'; private checkIntervalMs = 60_000; diff --git a/backend/services/billing/groupBilling.ts b/backend/services/billing/groupBilling.ts index c5356514..f89e47ee 100644 --- a/backend/services/billing/groupBilling.ts +++ b/backend/services/billing/groupBilling.ts @@ -53,24 +53,10 @@ export interface GroupPlanCustomization { sharedFeatures: string[]; memberLimits: Record; ownerDiscount: number; - /** Tax rate in basis points (e.g. 2000 = 20%). Optional; 0 when unset. */ - taxRateBps?: number; createdAt: number; updatedAt: number; } -/** Rounds to minor units, mapping non-finite input to zero. */ -const roundMoney = (amount: number): number => { - if (!Number.isFinite(amount)) return 0; - return Math.round((amount + Number.EPSILON * Math.sign(amount)) * 100) / 100; -}; - -/** Constrains a basis-point tax rate to a sane `[0, 100%]` range. */ -const clampRate = (bps: number | undefined): number => { - if (bps === undefined || !Number.isFinite(bps)) return 0; - return Math.min(Math.max(bps, 0), 10_000); -}; - const createId = (prefix: string): string => `${prefix}_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`; @@ -111,40 +97,28 @@ export class GroupBillingService { }; } - aggregateCharges( - group: SubscriptionGroup, - periodStart: number, - periodEnd: number - ): GroupBillingLineItem[] { - // The window is the requested period, not "the last N days relative to - // now". Deriving the cutoff from `Date.now()` made a historical invoice - // price a completely different set of charges than the period it claims - // to cover, so the line items never summed to the stated period. - const start = Math.min(periodStart, periodEnd); - const end = Math.max(periodStart, periodEnd); - const periodCharges = group.charges.filter((c) => { - const chargedAt = c.chargedAt.getTime(); - return chargedAt >= start && chargedAt <= end; - }); + aggregateCharges(group: SubscriptionGroup, periodDays = 30): GroupBillingLineItem[] { + const cutoff = Date.now() - periodDays * 24 * 60 * 60 * 1000; + const recentCharges = group.charges.filter( + (c) => c.chargedAt.getTime() >= cutoff + ); const memberCharges = new Map(); for (const member of group.members) { memberCharges.set(member.address, 0); } - for (const charge of periodCharges) { + for (const charge of recentCharges) { for (const item of charge.breakdown) { const current = memberCharges.get(item.memberAddress) ?? 0; - memberCharges.set(item.memberAddress, roundMoney(current + item.amount)); + memberCharges.set(item.memberAddress, current + item.amount); } } - const periodDays = Math.max(1, Math.ceil((end - start) / (24 * 60 * 60 * 1000))); - return Array.from(memberCharges.entries()).map(([address, amount]) => ({ memberAddress: address, amount, - description: `Aggregated charges over ${periodDays} day(s)`, + description: `Aggregated charges over last ${periodDays} days`, })); } @@ -156,24 +130,15 @@ export class GroupBillingService { periodEnd: number, currency = 'USD' ): GroupInvoice { - const aggregated = this.aggregateCharges(group, periodStart, periodEnd); - const totalAmount = roundMoney(aggregated.reduce((sum, item) => sum + item.amount, 0)); + const aggregated = this.aggregateCharges(group, Math.ceil((periodEnd - periodStart) / (24 * 60 * 60 * 1000))); + const totalAmount = aggregated.reduce((sum, item) => sum + item.amount, 0); const customization = this.planCustomizations.get(group.groupId); - - // `ownerDiscount` is a percentage but arrives from user-editable config. - // A negative value used to *increase* the bill and anything above 100 - // produced a negative payable amount, so both ends are clamped. - const rawDiscountPercent = customization?.ownerDiscount ?? 0; - const discountPercent = Math.min(Math.max(Number.isFinite(rawDiscountPercent) ? rawDiscountPercent : 0, 0), 100); - const discountAmount = roundMoney((totalAmount * discountPercent) / 100); - - // Tax was hard-coded to 0, so group invoices were never taxed even for - // entities inside a tax jurisdiction. - const taxRate = clampRate(customization?.taxRateBps); - const taxableAmount = Math.max(totalAmount - discountAmount, 0); - const taxAmount = roundMoney((taxableAmount * taxRate) / 10_000); - const finalAmount = roundMoney(Math.max(taxableAmount + taxAmount, 0)); + const discountAmount = customization?.ownerDiscount + ? (totalAmount * customization.ownerDiscount) / 100 + : 0; + const taxAmount = 0; + const finalAmount = totalAmount - discountAmount + taxAmount; const invoice: GroupInvoice = { id: createId('ginv'), diff --git a/backend/services/billing/interfaces.ts b/backend/services/billing/interfaces.ts index d1470a61..776e426f 100644 --- a/backend/services/billing/interfaces.ts +++ b/backend/services/billing/interfaces.ts @@ -222,7 +222,7 @@ export interface IPartnerService { export interface IGroupBillingService { generateBillingSummary(group: any): any; - aggregateCharges(group: any, periodStart: number, periodEnd: number): any[]; + aggregateCharges(group: any, periodDays?: number): any[]; generateInvoice(group: any, periodStart: number, periodEnd: number, currency?: string): any; issueInvoice(invoiceId: string, groupId: string): any | null; markInvoicePaid(invoiceId: string, groupId: string): any | null; diff --git a/backend/services/billing/meteringService.ts b/backend/services/billing/meteringService.ts index cb0866d4..60c5acc9 100644 --- a/backend/services/billing/meteringService.ts +++ b/backend/services/billing/meteringService.ts @@ -12,12 +12,6 @@ const SOFT_THRESHOLD_RATIO = 0.8; const HARD_THRESHOLD_RATIO = 1.0; const MAX_BATCH_SIZE = 500; -/** Monotonic sequence used to keep same-millisecond auto-keys distinct. */ -let _autoSequence = 0; -function nextAutoSequence(): number { - return ++_autoSequence; -} - export interface UsageMetric { userId: string; metricType: 'api' | 'compute' | 'storage'; @@ -132,14 +126,7 @@ export class MeteringService { } private async recordOne(metric: UsageMetric): Promise { - // An auto-generated key built purely from the event timestamp collides - // for two legitimate events landing in the same millisecond, and the - // second one was then reported as a duplicate and silently never rated — - // under-billing the customer. A monotonic sequence keeps distinct events - // distinct. Callers that need retry-idempotency must pass an explicit key. - const idempotencyKey = - metric.idempotencyKey ?? - `auto-${meterKey(metric.userId, metric.metricType)}-${metric.timestamp.getTime()}-${nextAutoSequence()}`; + const idempotencyKey = metric.idempotencyKey ?? `auto-${meterKey(metric.userId, metric.metricType)}-${metric.timestamp.getTime()}`; const cached = this.seenIdempotencyKeys.get(idempotencyKey); if (cached) { @@ -241,22 +228,12 @@ export class MeteringService { this.events.delete(meterKey(userId, metricType)); } - /** - * Sums recorded usage per metric for a subscription. - * - * An optional window is essential: without one this returns the entire - * lifetime total, which callers then label as a single billing period. - */ -getUsageByMetric(subscriptionId: string, from?: Date, to?: Date): Record { - const fromMs = from instanceof Date ? from.getTime() : undefined; - const toMs = to instanceof Date ? to.getTime() : undefined; + getUsageByMetric(subscriptionId: string): Record { const usage: Record = {}; for (const entry of this.usageHistory) { - if (entry.subscriptionId !== subscriptionId) continue; - const entryTime = entry.timestamp.getTime(); - if (fromMs !== undefined && entryTime < fromMs) continue; - if (toMs !== undefined && entryTime > toMs) continue; - usage[entry.metric] = (usage[entry.metric] || 0) + entry.value; + if (entry.subscriptionId === subscriptionId) { + usage[entry.metric] = (usage[entry.metric] || 0) + entry.value; + } } return usage; } @@ -322,29 +299,13 @@ getUsageByMetric(subscriptionId: string, from?: Date, to?: Date): Record= 16) { - return configured; - } - - if (process.env['NODE_ENV'] === 'production') { - throw new Error( - 'CURSOR_HMAC_SECRET must be set to a random value of at least 16 characters in production.' - ); - } - - if (configured) { - console.warn( - '[pagination] CURSOR_HMAC_SECRET is too short (min 16 chars); using an ephemeral secret. Cursors will not survive a restart.' - ); - } - - return createHmac('sha256', `${process.pid}:${process.hrtime.bigint()}:${Math.random()}`) - .update('cursor-secret') - .digest('hex'); -} - -let _cursorSecret: string | null = null; - -/** Lazily resolved so importing this module never throws in production. */ -function cursorSecret(): string { - if (_cursorSecret === null) { - _cursorSecret = resolveCursorSecret(); - } - return _cursorSecret; -} - -/** Test seam: forces a specific secret, or `null` to re-resolve. */ -export function __setCursorSecretForTesting(secret: string | null): void { - _cursorSecret = secret; -} - -function sign(data: string): string { - return createHmac('sha256', cursorSecret()).update(data).digest('base64url').slice(0, 16); -} - -/** - * Restricts a sort/identifier to a safe SQL identifier. - * - * Anything outside `[A-Za-z0-9_]` is rejected outright rather than sanitised, - * because a partially-stripped identifier is still attacker-influenced. - */ -export function assertSafeIdentifier(identifier: string, label: string): string { - if (!/^[A-Za-z0-9_]+$/.test(identifier)) { - throw new Error(`Unsafe SQL identifier for ${label}: ${JSON.stringify(identifier)}`); - } - return identifier; -} - export interface CursorPayload { v: number; field: string; @@ -110,22 +43,9 @@ export function decodeCursor(cursor: string): CursorPayload | null { if (sepIdx === -1) return null; const json = raw.slice(0, sepIdx); const sig = raw.slice(sepIdx + 1); - - // Constant-time so a forged signature cannot be refined byte by byte. - const bufA = Buffer.from(sign(json), 'utf8'); - const bufB = Buffer.from(sig, 'utf8'); - if (bufA.length !== bufB.length) return null; - if (!timingSafeEqual(bufA, bufB)) return null; - + if (sign(json) !== sig) return null; const payload = JSON.parse(json) as CursorPayload; if (payload.v !== CURSOR_VERSION) return null; - - // The signature is only trustworthy if the secret actually is. When a - // short/absent secret yields an ephemeral key we still refuse to trust a - // cursor's `field`, because it is interpolated into SQL below. - if (!/^[A-Za-z0-9_]+$/.test(String(payload.field ?? ''))) { - return null; - } return payload; } catch { return null; @@ -177,7 +97,7 @@ export interface SqlCursorClause { */ export function buildCursorClause(options: PageOptions = {}): SqlCursorClause { const dir = options.direction ?? 'asc'; - const sortField = assertSafeIdentifier(options.sortField ?? 'id', 'sortField'); + const sortField = options.sortField ?? 'id'; const rawLimit = options.limit ?? 20; const limit = Math.min(Math.max(1, rawLimit), options.maxLimit ?? 100); const op = dir === 'asc' ? '>' : '<'; @@ -197,15 +117,10 @@ export function buildCursorClause(options: PageOptions = {}): SqlCursorClause { return { where: '', param: null, orderBy: `"${sortField}" ${orderDir}`, limit }; } - // `decodeCursor` has already restricted `field` to `[A-Za-z0-9_]`, so it is - // safe to interpolate as a quoted SQL identifier. Re-check rather than trust - // the caller's own type-checking to have held. - const cursorField = assertSafeIdentifier(decoded.field, 'cursor.field'); - return { - where: `("${cursorField}", id) ${op} ($1, $2)`, + where: `("${decoded.field}", id) ${op} ($1, $2)`, param: decoded.value, - orderBy: `"${cursorField}" ${orderDir}, id ${orderDir}`, + orderBy: `"${decoded.field}" ${orderDir}, id ${orderDir}`, limit, }; } diff --git a/backend/services/shared/rateLimitingService.ts b/backend/services/shared/rateLimitingService.ts index 9904a8f7..8f1a1c19 100644 --- a/backend/services/shared/rateLimitingService.ts +++ b/backend/services/shared/rateLimitingService.ts @@ -110,20 +110,6 @@ export class RateLimitingService { return Array.from(this.bypass.userIds ?? []); } - // ------------------------------------------------------------------------- - // Tier resolution - // ------------------------------------------------------------------------- - - /** - * Returns the tier recorded for `apiKey`, or FREE when unknown. - * - * This is the only trustworthy source for a caller's tier. Reading it from - * a request header lets any caller self-select the ENTERPRISE allowance. - */ - getTierForApiKey(apiKey: string): SubscriptionTier { - return this.usages.get(apiKey)?.tier ?? SubscriptionTier.FREE; - } - // ------------------------------------------------------------------------- // Custom limit configuration // ------------------------------------------------------------------------- diff --git a/backend/services/webhook/eventReplayWorker.ts b/backend/services/webhook/eventReplayWorker.ts index 81943c66..29fd72e0 100644 --- a/backend/services/webhook/eventReplayWorker.ts +++ b/backend/services/webhook/eventReplayWorker.ts @@ -3,9 +3,6 @@ * with idempotency key checking and ordering guarantees. */ -import { createHmac } from 'node:crypto'; -import { validateOutboundUrl } from '../../shared/security/outboundUrlGuard'; - export interface ReplayRequest { eventIds: string[]; webhookId: string; @@ -100,12 +97,7 @@ export class EventReplayWorker { ): Promise<{ ok: boolean; status: number }> { const body = JSON.stringify(payload); - // Replay targets come from stored webhook configuration; validate before - // fetching so a compromised or misconfigured record cannot be turned into - // an SSRF pivot. - const target = validateOutboundUrl(url); - - const res = await fetch(target.toString(), { + const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -121,11 +113,14 @@ export class EventReplayWorker { } private computeSignature(body: string, secret: string): string { - // This was a 31-bit rolling hash that merely *claimed* to be SHA-256 in - // the header value. It was trivially forgeable and, because replayed - // deliveries are signed with it, a receiver relying on it had no - // authenticity guarantee at all. Use a real keyed HMAC. - return `sha256=${createHmac('sha256', secret).update(body).digest('hex')}`; + // In production, use HMAC-SHA256 + // Placeholder: simple hash for structure + let hash = 0; + const input = secret + body; + for (let i = 0; i < input.length; i++) { + hash = ((hash << 5) - hash + input.charCodeAt(i)) | 0; + } + return `sha256=${Math.abs(hash).toString(16)}`; } getEventHistory(webhookId: string, limit = 50): StoredEvent[] { diff --git a/backend/shared/security/__tests__/outboundUrlGuard.test.ts b/backend/shared/security/__tests__/outboundUrlGuard.test.ts deleted file mode 100644 index 90078865..00000000 --- a/backend/shared/security/__tests__/outboundUrlGuard.test.ts +++ /dev/null @@ -1,152 +0,0 @@ -/** - * Tests for outboundUrlGuard — SSRF protection for server-side fetches. - * - * @see backend/shared/security/outboundUrlGuard.ts - * @see https://github.com/Smartdevs17/SubTrackr/issues/1686 - */ - -import { - OutboundUrlError, - isPrivateHostname, - validateOutboundUrl, -} from '../outboundUrlGuard'; - -describe('isPrivateHostname', () => { - it('flags loopback and metadata hostnames', () => { - expect(isPrivateHostname('localhost')).toBe(true); - expect(isPrivateHostname('LOCALHOST')).toBe(true); - expect(isPrivateHostname('metadata.google.internal')).toBe(true); - expect(isPrivateHostname('instance-data')).toBe(true); - }); - - it('flags internal-looking suffixes', () => { - expect(isPrivateHostname('db.local')).toBe(true); - expect(isPrivateHostname('svc.internal')).toBe(true); - expect(isPrivateHostname('api.localhost')).toBe(true); - }); - - it('flags loopback and RFC1918 IPv4 literals', () => { - expect(isPrivateHostname('127.0.0.1')).toBe(true); - expect(isPrivateHostname('10.1.2.3')).toBe(true); - expect(isPrivateHostname('172.16.0.1')).toBe(true); - expect(isPrivateHostname('192.168.1.1')).toBe(true); - expect(isPrivateHostname('0.0.0.0')).toBe(true); - expect(isPrivateHostname('255.255.255.255')).toBe(true); - }); - - it('flags the cloud metadata address specifically', () => { - expect(isPrivateHostname('169.254.169.254')).toBe(true); - }); - - it('flags CGNAT space', () => { - expect(isPrivateHostname('100.64.0.1')).toBe(true); - }); - - it('flags private IPv6 literals', () => { - expect(isPrivateHostname('::1')).toBe(true); - expect(isPrivateHostname('fd00::1')).toBe(true); - expect(isPrivateHostname('fe80::1')).toBe(true); - }); - - it('flags IPv4-mapped IPv6 loopback', () => { - expect(isPrivateHostname('::ffff:127.0.0.1')).toBe(true); - }); - - it('allows ordinary public hosts', () => { - expect(isPrivateHostname('example.com')).toBe(false); - expect(isPrivateHostname('hooks.zapier.com')).toBe(false); - expect(isPrivateHostname('8.8.8.8')).toBe(false); - expect(isPrivateHostname('172.32.0.1')).toBe(false); - expect(isPrivateHostname('172.15.0.1')).toBe(false); - }); - - it('treats a malformed dotted quad as unsafe', () => { - expect(isPrivateHostname('999.1.1.1')).toBe(true); - expect(isPrivateHostname('127.0.0')).toBe(true); - }); - - it('treats an empty host as unsafe', () => { - expect(isPrivateHostname('')).toBe(true); - }); -}); - -describe('validateOutboundUrl', () => { - it('accepts a normal public https URL', () => { - // A real Zapier host, but deliberately *not* a `/hooks/catch///` - // path: that shape is indistinguishable from a live Zapier hook URL and - // trips secret detectors in CI (the assertion only needs "public host"). - const result = validateOutboundUrl('https://hooks.zapier.com/example-endpoint'); - expect(result.protocol).toBe('https:'); - expect(result.hostname).toBe('hooks.zapier.com'); - }); - - it('accepts plain http', () => { - expect(validateOutboundUrl('http://example.com/hook').protocol).toBe('http:'); - }); - - it('rejects non-HTTP schemes', () => { - for (const url of [ - 'file:///etc/passwd', - 'gopher://example.com', - 'ftp://example.com', - 'javascript:alert(1)', - 'data:text/plain,hi', - ]) { - expect(() => validateOutboundUrl(url)).toThrow(OutboundUrlError); - } - }); - - it('rejects relative or malformed values', () => { - expect(() => validateOutboundUrl('/relative/path')).toThrow(OutboundUrlError); - expect(() => validateOutboundUrl('not a url')).toThrow(OutboundUrlError); - expect(() => validateOutboundUrl('')).toThrow(OutboundUrlError); - expect(() => validateOutboundUrl(undefined)).toThrow(OutboundUrlError); - expect(() => validateOutboundUrl(null)).toThrow(OutboundUrlError); - expect(() => validateOutboundUrl(42)).toThrow(OutboundUrlError); - }); - - it('rejects loopback and private targets', () => { - expect(() => validateOutboundUrl('http://localhost/hook')).toThrow(/non-public/); - expect(() => validateOutboundUrl('http://127.0.0.1:8080/hook')).toThrow(/non-public/); - expect(() => validateOutboundUrl('http://169.254.169.254/latest/meta-data/')).toThrow( - /non-public/ - ); - expect(() => validateOutboundUrl('http://10.0.0.5/admin')).toThrow(/non-public/); - }); - - it('rejects embedded credentials', () => { - expect(() => validateOutboundUrl('https://user:pass@example.com/')).toThrow( - /embedded credentials/ - ); - }); - - it('enforces an allowlist when one is supplied', () => { - expect( - validateOutboundUrl('https://hooks.zapier.com/x', { allowedHosts: ['hooks.zapier.com'] }) - .hostname - ).toBe('hooks.zapier.com'); - - // Subdomains of an allowlisted host are permitted. - expect(() => - validateOutboundUrl('https://eu.hooks.zapier.com/x', { allowedHosts: ['hooks.zapier.com'] }) - ).not.toThrow(); - - expect(() => - validateOutboundUrl('https://evil.example.com/x', { allowedHosts: ['hooks.zapier.com'] }) - ).toThrow(/allowlist/); - }); - - it('does not treat a lookalike host as allowlisted', () => { - expect(() => - validateOutboundUrl('https://hooks.zapier.com.evil.test/x', { - allowedHosts: ['hooks.zapier.com'], - }) - ).toThrow(/allowlist/); - }); - - it('still applies the private-address rules when an allowlist is present', () => { - expect(() => - validateOutboundUrl('http://127.0.0.1/x', { allowedHosts: ['127.0.0.1'] }) - ).toThrow(OutboundUrlError); - }); -}); \ No newline at end of file diff --git a/backend/shared/security/outboundUrlGuard.ts b/backend/shared/security/outboundUrlGuard.ts deleted file mode 100644 index 9c6ab63a..00000000 --- a/backend/shared/security/outboundUrlGuard.ts +++ /dev/null @@ -1,184 +0,0 @@ -/** - * outboundUrlGuard.ts - * - * Validation for URLs the server will fetch on a user's behalf. - * - * Several integration routes accept an operator- or merchant-supplied - * `targetUrl` and later call `fetch()` with it. Without validation that is a - * server-side request forgery primitive: an attacker can reach the cloud - * metadata endpoint, loopback services, or RFC1918 hosts that are only - * reachable from inside the network. - * - * The rules below reject the obvious cases statically (scheme, obvious host - * names, literal private/loopback/link-local addresses). DNS rebinding and - * internal names that resolve to private space are not covered here — for - * those, pair this with an egress proxy or a resolved-address check at - * connection time. - */ - -import { isIP } from 'node:net'; - -/** Hostnames that always denote the local machine or a container escape. */ -const BLOCKED_HOSTNAMES = new Set([ - 'localhost', - 'localhost.localdomain', - 'ip6-localhost', - 'ip6-loopback', - 'metadata', - 'metadata.google.internal', - 'instance-data', -]); - -/** Suffixes used by cloud metadata services. */ -const BLOCKED_SUFFIXES = ['.localhost', '.local', '.internal', '.localdomain']); - -/** Only plain web schemes; `file:`, `gopher:`, `dict:` etc. are refused. */ -const ALLOWED_PROTOCOLS = new Set(['http:', 'https:']); - -export class OutboundUrlError extends Error { - readonly code = 'OUTBOUND_URL_REJECTED' as const; - - constructor(reason: string) { - super(`Rejected outbound URL: ${reason}`); - this.name = 'OutboundUrlError'; - Object.setPrototypeOf(this, new.target.prototype); - } -} - -/** - * Parses IPv4 dotted-quads into octets. - * Returns `null` for anything that is not exactly four numeric parts. - */ -function parseIPv4(host: string): number[] | null { - const parts = host.split('.'); - if (parts.length !== 4) return null; - - const octets: number[] = []; - for (const part of parts) { - if (!/^\d{1,3}$/.test(part)) return null; - const value = Number(part); - if (value > 255) return null; - octets.push(value); - } - - return octets; -} - -function isPrivateIPv4(octets: number[]): boolean { - const [a, b] = octets; - - if (a === 0) return true; // "this" network - if (a === 10) return true; // 10.0.0.0/8 - if (a === 127) return true; // loopback - if (a === 169 && b === 254) return true; // link-local, incl. 169.254.169.254 - if (a === 172 && b >= 16 && b <= 31) return true; // 172.16.0.0/12 - if (a === 192 && b === 168) return true; // 192.168.0.0/16 - if (a === 192 && b === 0) return true; // 192.0.0.0/24 + 192.0.2.0/24 - if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT 100.64.0.0/10 - if (a >= 224) return true; // multicast + reserved - - return false; -} - -function isPrivateIPv6(host: string): boolean { - const normalized = host.replace(/^\[/, '').replace(/\]$/, '').toLowerCase(); - - if (normalized === '::' || normalized === '::1') return true; // unspecified, loopback - - // IPv4-mapped (::ffff:a.b.c.d) and IPv4-compatible forms embed an IPv4 address. - const embeddedIPv4 = normalized.match(/(\d{1,3}(?:\.\d{1,3}){3})$/); - if (embeddedIPv4) { - const octets = parseIPv4(embeddedIPv4[1]); - if (octets && isPrivateIPv4(octets)) return true; - } - - // Unique-local (fc00::/7) and link-local (fe80::/10). - if (/^f[cd][0-9a-f]{2}:/.test(normalized)) return true; - if (/^fe[89ab][0-9a-f]:/.test(normalized)) return true; - - return false; -} - -/** - * Returns true when `hostname` cannot be reached from the public internet. - */ -export function isPrivateHostname(hostname: string): boolean { - const host = hostname.trim().toLowerCase().replace(/\.$/, ''); - - if (host === '') return true; - if (BLOCKED_HOSTNAMES.has(host)) return true; - if (BLOCKED_SUFFIXES.some((suffix) => host.endsWith(suffix))) return true; - - const ipVersion = isIP(host); - if (ipVersion === 4) { - const octets = parseIPv4(host); - // A malformed dotted quad is treated as unsafe rather than trusted. - return octets === null ? true : isPrivateIPv4(octets); - } - if (ipVersion === 6) { - return isPrivateIPv6(host); - } - - // A host made only of digits and dots is an *attempted* IP literal. If it - // did not parse as one, some other layer (a resolver, a proxy, a downstream - // parser) may interpret it differently than we just did, so refuse it. - if (/^[\d.]+$/.test(host)) { - return true; - } - - return false; -} - -export interface ValidateOutboundUrlOptions { - /** When set, only these hostnames (and their subdomains) are permitted. */ - allowedHosts?: readonly string[]; -} - -/** - * Validates and normalises a URL the server is about to fetch. - * - * @throws {OutboundUrlError} when the URL is malformed, uses a non-HTTP - * scheme, points at private/loopback/link-local space, or is not on the - * allowlist. - */ -export function validateOutboundUrl( - rawUrl: unknown, - options: ValidateOutboundUrlOptions = {} -): URL { - if (typeof rawUrl !== 'string' || rawUrl.trim() === '') { - throw new OutboundUrlError('value must be a non-empty string'); - } - - let parsed: URL; - try { - parsed = new URL(rawUrl.trim()); - } catch { - throw new OutboundUrlError('value is not a valid absolute URL'); - } - - if (!ALLOWED_PROTOCOLS.has(parsed.protocol)) { - throw new OutboundUrlError(`protocol '${parsed.protocol}' is not allowed`); - } - - if (parsed.username !== '' || parsed.password !== '') { - throw new OutboundUrlError('embedded credentials are not allowed'); - } - - if (isPrivateHostname(parsed.hostname)) { - throw new OutboundUrlError(`host '${parsed.hostname}' resolves to non-public address space`); - } - - if (options.allowedHosts && options.allowedHosts.length > 0) { - const host = parsed.hostname.toLowerCase(); - const allowed = options.allowedHosts.some((entry) => { - const candidate = entry.toLowerCase(); - return host === candidate || host.endsWith(`.${candidate}`); - }); - - if (!allowed) { - throw new OutboundUrlError(`host '${parsed.hostname}' is not on the allowlist`); - } - } - - return parsed; -} \ No newline at end of file diff --git a/developer-portal/pages/api/revalidate.ts b/developer-portal/pages/api/revalidate.ts index 88395eef..66433bc2 100644 --- a/developer-portal/pages/api/revalidate.ts +++ b/developer-portal/pages/api/revalidate.ts @@ -1,5 +1,3 @@ -import { timingSafeEqual } from 'node:crypto'; - import type { NextApiRequest, NextApiResponse } from 'next'; type RevalidateResult = { revalidated: boolean; paths?: string[]; error?: string }; @@ -38,31 +36,6 @@ const TAG_TO_PATHS: Record = { sdks: ['/docs/webhook-integration'], }; -/** Every path the endpoint is permitted to revalidate, regardless of input. */ -const ALLOWED_PATHS = new Set(Object.values(TAG_TO_PATHS).flat()); - -/** - * Constant-time secret comparison. - * - * `secret !== expectedSecret` short-circuits on the first differing byte, - * which leaks the secret length and prefix through response timing and lets - * an attacker recover it one character at a time. - */ -function secretsMatch(provided: unknown, expected: string): boolean { - if (typeof provided !== 'string') return false; - - const a = Buffer.from(provided, 'utf8'); - const b = Buffer.from(expected, 'utf8'); - - if (a.length !== b.length) { - // Still perform a comparison so the fast path is not measurably faster. - timingSafeEqual(a, a); - return false; - } - - return timingSafeEqual(a, b); -} - export default async function handler(req: NextApiRequest, res: NextApiResponse) { if (req.method !== 'POST') { return res.status(405).json({ revalidated: false, error: 'Method not allowed' }); @@ -76,7 +49,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse< // Verify caller secret const expectedSecret = process.env.REVALIDATE_SECRET; - if (!expectedSecret || !secretsMatch(secret, expectedSecret)) { + if (!expectedSecret || secret !== expectedSecret) { return res.status(401).json({ revalidated: false, error: 'Invalid secret' }); } @@ -84,15 +57,6 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse< return res.status(400).json({ revalidated: false, error: 'Provide "path" or "tag"' }); } - // `path` is caller-supplied and goes straight into `res.revalidate()`. An - // arbitrary value lets a holder of the secret force revalidation of - // unrelated routes, so it is restricted to the documented set. - if (path !== undefined && !ALLOWED_PATHS.has(path)) { - return res - .status(400) - .json({ revalidated: false, error: `Path "${path}" is not revalidatable` }); - } - const pathsToRevalidate: string[] = path ? [path] : (TAG_TO_PATHS[tag as string] ?? []); if (pathsToRevalidate.length === 0) { diff --git a/src/services/__tests__/groupBilling.test.ts b/src/services/__tests__/groupBilling.test.ts index 05e7ca94..430270b6 100644 --- a/src/services/__tests__/groupBilling.test.ts +++ b/src/services/__tests__/groupBilling.test.ts @@ -60,23 +60,13 @@ describe('GroupBillingService', () => { expect(summary.memberBalances['0xMEMBER1']).toBe(10); }); - it('aggregates charges within the requested period', () => { - const now = Date.now(); - const items = service.aggregateCharges(sampleGroup, now - 30 * 86400000, now); + it('aggregates charges within period', () => { + const items = service.aggregateCharges(sampleGroup, 30); expect(items.length).toBe(2); const ownerItem = items.find((i) => i.memberAddress === '0xOWNER'); expect(ownerItem?.amount).toBe(29.99); }); - it('excludes charges outside the requested period', () => { - const now = Date.now(); - // The window is anchored to the period, not to "the last N days from - // now" — a period that ended long ago must not pick up today's charges. - const items = service.aggregateCharges(sampleGroup, now - 90 * 86400000, now - 60 * 86400000); - const ownerItem = items.find((i) => i.memberAddress === '0xOWNER'); - expect(ownerItem?.amount).toBe(0); - }); - it('generates and processes invoices', () => { const now = Date.now(); const invoice = service.generateInvoice(sampleGroup, now - 86400000, now); diff --git a/src/services/__tests__/rpcProvider.test.ts b/src/services/__tests__/rpcProvider.test.ts deleted file mode 100644 index ac3e04ae..00000000 --- a/src/services/__tests__/rpcProvider.test.ts +++ /dev/null @@ -1,166 +0,0 @@ -/** - * Tests for the resilient RPC provider's registry and error surface. - * - * These deliberately avoid exercising real network I/O: the behaviour that - * matters for this change (which errors count against a circuit, URL-swap - * serialisation) is covered by `isTransportFailure` tests in - * `transactionConfirmation.test.ts` and by the backend integration suite. - * - * @see src/services/rpcProvider.ts - * @see https://github.com/Smartdevs17/SubTrackr/issues/1683 - */ - -import { - AllRpcProvidersFailedError, - RpcProviderCircuitOpenError, - RpcProviderTimeoutError, - clearResilientProviderRegistry, - defaultChainTimeoutMs, - getOrCreateResilientProvider, - isTransportFailure, -} from '../rpcProvider'; - -const PRIMARY = 'https://primary.example/rpc'; -const FALLBACK = 'https://fallback.example/rpc'; - -describe('defaultChainTimeoutMs', () => { - it('allows more headroom on high-variance L2 chains', () => { - expect(defaultChainTimeoutMs(10)).toBe(15_000); - expect(defaultChainTimeoutMs(137)).toBe(15_000); - expect(defaultChainTimeoutMs(8453)).toBe(15_000); - }); - - it('uses the tighter default on Ethereum mainnet', () => { - expect(defaultChainTimeoutMs(1)).toBe(10_000); - }); - - it('falls back to the default for unknown chains', () => { - expect(defaultChainTimeoutMs(999999)).toBe(10_000); - }); -}); - -describe('getOrCreateResilientProvider', () => { - beforeEach(() => { - clearResilientProviderRegistry(); - }); - - afterEach(() => { - clearResilientProviderRegistry(); - }); - - it('returns the same instance for the same chain and URL set', () => { - const first = getOrCreateResilientProvider(1, [PRIMARY, FALLBACK]); - const second = getOrCreateResilientProvider(1, [PRIMARY, FALLBACK]); - expect(second).toBe(first); - }); - - it('ignores URL order so circuit state is not silently reset', () => { - const first = getOrCreateResilientProvider(1, [PRIMARY, FALLBACK]); - const second = getOrCreateResilientProvider(1, [FALLBACK, PRIMARY]); - expect(second).toBe(first); - }); - - it('keeps separate instances per chain', () => { - const mainnet = getOrCreateResilientProvider(1, [PRIMARY]); - const polygon = getOrCreateResilientProvider(137, [PRIMARY]); - expect(mainnet).not.toBe(polygon); - }); - - it('keeps separate instances for different URL sets', () => { - const first = getOrCreateResilientProvider(1, [PRIMARY]); - const second = getOrCreateResilientProvider(1, [PRIMARY, FALLBACK]); - expect(first).not.toBe(second); - }); - - it('starts every circuit closed', () => { - const provider = getOrCreateResilientProvider(1, [PRIMARY, FALLBACK]); - const states = provider.getCircuitStates(); - - expect(states[PRIMARY]).toEqual({ state: 'closed', consecutiveFailures: 0 }); - expect(states[FALLBACK]).toEqual({ state: 'closed', consecutiveFailures: 0 }); - }); - - it('resets every circuit on demand', () => { - const provider = getOrCreateResilientProvider(1, [PRIMARY, FALLBACK]); - provider.resetAllCircuits(); - - const states = provider.getCircuitStates(); - expect(states[PRIMARY].state).toBe('closed'); - expect(states[FALLBACK].state).toBe('closed'); - }); - - it('drops every cached provider when the registry is cleared', () => { - const first = getOrCreateResilientProvider(1, [PRIMARY]); - clearResilientProviderRegistry(); - const second = getOrCreateResilientProvider(1, [PRIMARY]); - - expect(second).not.toBe(first); - }); -}); - -describe('isTransportFailure', () => { - const withCode = (message: string, code: unknown): Error => - Object.assign(new Error(message), { code }); - - it('classifies an explicit timeout as a transport failure', () => { - expect(isTransportFailure(withCode('nope', 'TIMEOUT'))).toBe(true); - }); - - it('classifies connection-level errors as transport failures', () => { - for (const code of ['ECONNREFUSED', 'ECONNRESET', 'ENOTFOUND', 'ERR_NETWORK']) { - expect(isTransportFailure(withCode('x', code))).toBe(true); - } - }); - - it('does not treat a JSON-RPC application error as a transport failure', () => { - // A revert carries a numeric JSON-RPC code. The node answered correctly, - // so counting it as a circuit failure would evict a healthy endpoint. - expect(isTransportFailure(withCode('execution reverted', 3))).toBe(false); - }); - - it('does not treat insufficient funds as a transport failure', () => { - const nested = Object.assign(new Error('insufficient funds'), { - error: { code: -32000 }, - }); - expect(isTransportFailure(nested)).toBe(false); - }); - - it('treats fetch-level errors as transport failures', () => { - expect(isTransportFailure(new TypeError('failed to fetch'))).toBe(true); - }); - - it('returns false for non-error values', () => { - expect(isTransportFailure(null)).toBe(false); - expect(isTransportFailure(undefined)).toBe(false); - expect(isTransportFailure('nope')).toBe(false); - }); -}); - -describe('provider error types', () => { - it('exposes a stable code and context for timeouts', () => { - const error = new RpcProviderTimeoutError(PRIMARY, 100); - expect(error.code).toBe('RPC_PROVIDER_TIMEOUT'); - expect(error.endpointUrl).toBe(PRIMARY); - expect(error.timeoutMs).toBe(100); - expect(error).toBeInstanceOf(Error); - }); - - it('exposes a stable code and context for an open circuit', () => { - const error = new RpcProviderCircuitOpenError(FALLBACK); - expect(error.code).toBe('RPC_PROVIDER_CIRCUIT_OPEN'); - expect(error.endpointUrl).toBe(FALLBACK); - }); - - it('reports every attempted endpoint when all of them fail', () => { - const error = new AllRpcProvidersFailedError(1, [ - { url: PRIMARY, message: 'failed to fetch' }, - { url: FALLBACK, message: 'timed out' }, - ]); - - expect(error.code).toBe('ALL_RPC_PROVIDERS_FAILED'); - expect(error.chainId).toBe(1); - expect(error.errors).toHaveLength(2); - expect(error.message).toContain(PRIMARY); - expect(error.message).toContain(FALLBACK); - }); -}); diff --git a/src/services/__tests__/transactionConfirmation.test.ts b/src/services/__tests__/transactionConfirmation.test.ts deleted file mode 100644 index 4025df5d..00000000 --- a/src/services/__tests__/transactionConfirmation.test.ts +++ /dev/null @@ -1,261 +0,0 @@ -/** - * Tests for transactionConfirmation — defensive receipt waiting. - * - * @see src/services/transactionConfirmation.ts - * @see https://github.com/Smartdevs17/SubTrackr/issues/1683 - */ - -import type { ethers } from 'ethers'; - -import { - DEFAULT_CONFIRMATIONS, - DEFAULT_CONFIRMATION_TIMEOUT_MS, - TransactionInsufficientConfirmationsError, - TransactionRevertedError, - TransactionTimeoutError, - bestEffortTransactionHash, - confirmTransaction, - isDefinitiveFailure, - requiredConfirmations, -} from '../transactionConfirmation'; - -type Receipt = ethers.providers.TransactionReceipt; - -const L2_CHAIN_ID = 8453; -const L1_CHAIN_ID = 1; - -const makeReceipt = (overrides: Partial = {}): Receipt => - ({ - transactionHash: '0xabc', - status: 1, - confirmations: 2, - ...overrides, - }) as Receipt; - -const makeTx = (wait: (confirmations?: number) => Promise) => ({ - hash: '0xabc', - wait, -}); - -describe('requiredConfirmations', () => { - it('requires a single confirmation on fast L2 chains', () => { - expect(requiredConfirmations(L2_CHAIN_ID)).toBe(1); - expect(requiredConfirmations(10)).toBe(1); - expect(requiredConfirmations(137)).toBe(1); - }); - - it('requires deeper confirmation on L1', () => { - expect(requiredConfirmations(L1_CHAIN_ID)).toBe(DEFAULT_CONFIRMATIONS); - expect(DEFAULT_CONFIRMATIONS).toBe(2); - }); - - it('honours an explicit override', () => { - expect(requiredConfirmations(L2_CHAIN_ID, 5)).toBe(5); - }); - - it('never returns less than one confirmation', () => { - expect(requiredConfirmations(L1_CHAIN_ID, 0)).toBe(1); - expect(requiredConfirmations(L1_CHAIN_ID, -3)).toBe(1); - }); -}); - -describe('confirmTransaction', () => { - it('returns the receipt when the transaction succeeded', async () => { - const receipt = makeReceipt(); - const tx = makeTx(async () => receipt); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).resolves.toBe(receipt); - }); - - it('passes the required confirmations to ethers', async () => { - const wait = jest.fn().mockResolvedValue(makeReceipt({ confirmations: 1 })); - const tx = makeTx(wait); - - await confirmTransaction(tx, { chainId: L2_CHAIN_ID }); - - expect(wait).toHaveBeenCalledWith(1); - }); - - it('asks for the L1 confirmation depth on mainnet', async () => { - const wait = jest.fn().mockResolvedValue(makeReceipt({ confirmations: 2 })); - const tx = makeTx(wait); - - await confirmTransaction(tx, { chainId: L1_CHAIN_ID }); - - expect(wait).toHaveBeenCalledWith(DEFAULT_CONFIRMATIONS); - }); - - it('enforces its own timeout even when the transaction never settles', async () => { - // ethers v5 `ContractTransaction.wait()` ignores a timeout argument, so the - // bound has to be enforced here or a contract payment can hang forever. - jest.useFakeTimers(); - - try { - const tx = makeTx(() => new Promise(() => undefined)); - - const pending = confirmTransaction(tx, { chainId: L2_CHAIN_ID, timeoutMs: 5000 }); - const assertion = expect(pending).rejects.toBeInstanceOf(TransactionTimeoutError); - - jest.advanceTimersByTime(5000); - await assertion; - } finally { - jest.useRealTimers(); - } - }); - - it('does not fire the timeout when the receipt arrives in time', async () => { - jest.useFakeTimers(); - - try { - const tx = makeTx(async () => makeReceipt()); - - await expect( - confirmTransaction(tx, { chainId: L2_CHAIN_ID, timeoutMs: 5000 }) - ).resolves.toBeDefined(); - - // Any surviving timer must have been cleared, otherwise this leaks. - expect(jest.getTimerCount()).toBe(0); - } finally { - jest.useRealTimers(); - } - }); - - it('defaults to a bounded timeout when none is supplied', async () => { - jest.useFakeTimers(); - - try { - expect(DEFAULT_CONFIRMATION_TIMEOUT_MS).toBeGreaterThan(0); - - const tx = makeTx(() => new Promise(() => undefined)); - const pending = confirmTransaction(tx, { chainId: L2_CHAIN_ID }); - const assertion = expect(pending).rejects.toBeInstanceOf(TransactionTimeoutError); - - jest.advanceTimersByTime(DEFAULT_CONFIRMATION_TIMEOUT_MS); - await assertion; - } finally { - jest.useRealTimers(); - } - }); - - it('throws when the receipt reports a reverted transaction', async () => { - // The core regression: `status === 0` means mined-but-failed, and this - // used to be reported to the user as a successful payment. - const tx = makeTx(async () => makeReceipt({ status: 0 })); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).rejects.toBeInstanceOf( - TransactionRevertedError - ); - }); - - it('includes the revert reason when the node provides one', async () => { - // Nodes expose the reason in vendor-specific shapes that ethers v5 does - // not model on TransactionReceipt. - const receiptWithReason = { - ...makeReceipt({ status: 0 }), - revertReason: 'ERC20: transfer amount exceeds balance', - } as unknown as Receipt; - const tx = makeTx(async () => receiptWithReason); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).rejects.toThrow( - /ERC20: transfer amount exceeds balance/ - ); - }); - - it('still throws for a revert when no reason is available', async () => { - const tx = makeTx(async () => makeReceipt({ status: 0 })); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).rejects.toThrow( - /reverted on-chain/ - ); - }); - - it('maps an ethers wait timeout to a typed timeout error', async () => { - const timeoutError = Object.assign(new Error('timeout'), { code: 'TIMEOUT' }); - const tx = makeTx(async () => { - throw timeoutError; - }); - - await expect( - confirmTransaction(tx, { chainId: L2_CHAIN_ID, timeoutMs: 5000 }) - ).rejects.toBeInstanceOf(TransactionTimeoutError); - }); - - it('recognises timeout phrased as "timed out"', async () => { - const tx = makeTx(async () => { - throw new Error('transaction timed out waiting for receipt'); - }); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).rejects.toBeInstanceOf( - TransactionTimeoutError - ); - }); - - it('treats a null receipt as an unknown outcome', async () => { - // ethers resolves null when a transaction was replaced or dropped. - const tx = makeTx(async () => null); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).rejects.toBeInstanceOf( - TransactionTimeoutError - ); - }); - - it('throws when fewer confirmations than required were reached', async () => { - const tx = makeTx(async () => makeReceipt({ confirmations: 1 })); - - await expect(confirmTransaction(tx, { chainId: L1_CHAIN_ID })).rejects.toBeInstanceOf( - TransactionInsufficientConfirmationsError - ); - }); - - it('rethrows unexpected errors untouched', async () => { - const tx = makeTx(async () => { - throw new Error('user rejected transaction'); - }); - - await expect(confirmTransaction(tx, { chainId: L2_CHAIN_ID })).rejects.toThrow( - 'user rejected transaction' - ); - }); -}); - -describe('isDefinitiveFailure', () => { - it('treats a revert as definitive', () => { - expect(isDefinitiveFailure(new TransactionRevertedError('0x1'))).toBe(true); - }); - - it('treats insufficient confirmations as definitive', () => { - expect(isDefinitiveFailure(new TransactionInsufficientConfirmationsError('0x1', 1, 2))).toBe( - true - ); - }); - - it('does not treat a timeout as definitive', () => { - expect(isDefinitiveFailure(new TransactionTimeoutError('0x1', 100))).toBe(false); - }); -}); - -describe('bestEffortTransactionHash', () => { - it('returns the hash when the transaction confirms', async () => { - const tx = makeTx(async () => makeReceipt()); - await expect(bestEffortTransactionHash(tx, { chainId: L2_CHAIN_ID })).resolves.toBe('0xabc'); - }); - - it('still returns the hash when the receipt cannot be observed', async () => { - // A crash or dropped connection after broadcast leaves the outcome - // unknown; the hash is the durable handle the user needs. - const tx = makeTx(async () => null); - await expect(bestEffortTransactionHash(tx, { chainId: L2_CHAIN_ID })).resolves.toBe('0xabc'); - }); - - it('propagates a revert rather than swallowing it', async () => { - const tx = makeTx(async () => makeReceipt({ status: 0 })); - await expect(bestEffortTransactionHash(tx, { chainId: L2_CHAIN_ID })).rejects.toBeInstanceOf( - TransactionRevertedError - ); - }); - - it('returns undefined for a missing response', async () => { - await expect(bestEffortTransactionHash(null)).resolves.toBeUndefined(); - await expect(bestEffortTransactionHash(undefined)).resolves.toBeUndefined(); - }); -}); diff --git a/src/services/analyticsService.ts b/src/services/analyticsService.ts index b47ca974..228197a6 100644 --- a/src/services/analyticsService.ts +++ b/src/services/analyticsService.ts @@ -1,5 +1,4 @@ -import { Subscription } from '../types/subscription'; -import { monthlyEquivalent } from '../utils/billingCycleMath'; +import { BillingCycle, Subscription } from '../types/subscription'; export interface RevenuePoint { label: string; @@ -50,9 +49,18 @@ export interface SubscriptionAnalyticsReport { forecast: RevenueForecastPoint[]; } +const MONTHS_PER_YEAR = 12; +const WEEKS_PER_MONTH = 4.345; + export const toMonthlyRevenue = ( subscription: Pick -): number => monthlyEquivalent(subscription.price, subscription.billingCycle); +): number => { + if (subscription.billingCycle === BillingCycle.YEARLY) + return subscription.price / MONTHS_PER_YEAR; + if (subscription.billingCycle === BillingCycle.WEEKLY) + return subscription.price * WEEKS_PER_MONTH; + return subscription.price; +}; const MONTHS_PER_YEAR = 12; diff --git a/src/services/invoiceService.ts b/src/services/invoiceService.ts index e1413afd..ccb79a7b 100644 --- a/src/services/invoiceService.ts +++ b/src/services/invoiceService.ts @@ -12,11 +12,7 @@ import type { } from '../types/invoice'; import { InvoiceLayout, InvoiceStatus } from '../types/invoice'; import { getNextSequence, generateLegalInvoiceNumber } from './sequenceService'; -import { - calculateInvoiceTotals, - resolveLineTotal, - isDiscountLineItem, -} from '../utils/invoiceTotals'; +import { resolveLineTotal, isDiscountLineItem } from '../utils/invoiceTotals'; import { validateInvoiceLineItemMetadata, buildDiscountLineItem } from '../utils/invoice'; const STORAGE_KEYS = { @@ -211,12 +207,11 @@ export async function createInvoice(data: InvoiceFormData): Promise { lineItems = itemiseDiscount(lineItems, data.discountAmount, currency); } - const totals = calculateInvoiceTotals({ - lineItems, - amount: data.amount, - tax: data.taxAmount, - }); - const { subtotal, total } = totals; + const lineTotal = + lineItems.length > 0 + ? lineItems.reduce((sum, item) => sum + (item.lineTotal ?? 0), 0) + : (data.amount ?? 0); + const total = lineTotal + (data.taxAmount || 0) - (data.discountAmount || 0); // Legal numbering: fetch next persistent sequence // We track sequence per year to reset it annually (common legal requirement) @@ -233,10 +228,7 @@ export async function createInvoice(data: InvoiceFormData): Promise { merchantName: '', lineItems, tax: data.taxAmount ?? 0, - // Persist the clamped discount so a later edit can re-apply it instead of - // silently repricing the invoice at full value. - discountAmount: totals.discount, - subtotal, + subtotal: lineTotal, total, totalAmount: total, amount: total, @@ -302,17 +294,13 @@ export async function updateInvoice(id: string, updates: Partial): Prom lineItems = itemiseDiscount(lineItems, updatedInvoice.discountAmount, currency, exchangeRate); } - const totals = calculateInvoiceTotals({ - lineItems, - tax: updatedInvoice.tax ?? 0, - }); - + const subtotal = lineItems.reduce((sum, item) => sum + (item.lineTotal ?? 0), 0); + const total = subtotal + (updatedInvoice.tax ?? 0); updatedInvoice.lineItems = lineItems; - updatedInvoice.subtotal = totals.subtotal; - updatedInvoice.discountAmount = totals.discount; - updatedInvoice.total = totals.total; - updatedInvoice.totalAmount = totals.total; - updatedInvoice.amount = totals.total; + updatedInvoice.subtotal = subtotal; + updatedInvoice.total = total; + updatedInvoice.totalAmount = total; + updatedInvoice.amount = total; } invoices[index] = updatedInvoice; @@ -419,10 +407,7 @@ export async function getInvoiceAnalytics(): Promise { .filter((inv) => inv.status === InvoiceStatus.PAID) .reduce((sum, inv) => sum + inv.total, 0); - // `paidInvoices` can be 0 while invoices exist (every one still a draft). The - // old `totalRevenue / paidInvoices || 0` then evaluated to Infinity, because - // `Infinity` is truthy, and the analytics screen reported an infinite average. - const averageInvoiceAmount = paidInvoices > 0 ? totalRevenue / paidInvoices : 0; + const averageInvoiceAmount = totalInvoices > 0 ? totalRevenue / paidInvoices || 0 : 0; // Revenue by month const revenueByMonth: Record = {}; diff --git a/src/services/paymentMethodService.ts b/src/services/paymentMethodService.ts index d116c500..c6e90bf5 100644 --- a/src/services/paymentMethodService.ts +++ b/src/services/paymentMethodService.ts @@ -1,8 +1,6 @@ import { ethers } from 'ethers'; -import { getEvmRpcUrls } from '../config/evm'; -import { getOrCreateResilientProvider } from './rpcProvider'; -import { exceedsLimit } from '../utils/decimalAmount'; +import { getEvmRpcUrl } from '../config/evm'; import { CHAIN_IDS } from '../utils/constants/values'; import { PaymentMethod, @@ -197,7 +195,7 @@ export class PaymentMethodService { } try { - const provider = getOrCreateResilientProvider(method.chainId, getEvmRpcUrls(method.chainId)); + const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(method.chainId)); const erc20Abi = [ 'function decimals() view returns (uint8)', 'function symbol() view returns (string)', @@ -309,35 +307,29 @@ export class PaymentMethodService { chainId: number ): Promise<{ sufficient: boolean; balance: string; symbol: string }> { try { - const provider = getOrCreateResilientProvider(chainId, getEvmRpcUrls(chainId)); + const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(chainId)); const conn = this.walletManager?.getConnection(); if (!conn) { return { sufficient: false, balance: '0', symbol: method.tokenType }; } let balance: ethers.BigNumber; - // Read the token's own decimals instead of assuming "USDC = 6, - // everything else = 18". Assuming wrong under- or over-reports the - // balance, which triggers spurious fallbacks or lets a charge through - // that cannot settle. - let decimals = 18; if (method.tokenType === TokenType.NATIVE) { balance = await provider.getBalance(conn.address); } else { - const erc20Abi = [ - 'function decimals() view returns (uint8)', - 'function balanceOf(address) view returns (uint256)', - ]; + const erc20Abi = ['function balanceOf(address) view returns (uint256)']; const contract = new ethers.Contract(method.tokenAddress, erc20Abi, provider); - decimals = await contract.decimals(); balance = await contract.balanceOf(conn.address); } - const required = ethers.utils.parseUnits(requiredAmount, decimals); + const required = ethers.utils.parseUnits( + requiredAmount, + method.tokenType === TokenType.USDC ? 6 : 18 + ); return { sufficient: balance.gte(required), - balance: ethers.utils.formatUnits(balance, decimals), + balance: balance.toString(), symbol: method.tokenType.toString(), }; } catch { @@ -350,7 +342,7 @@ export class PaymentMethodService { maxGasPriceGwei: number ): Promise<{ acceptable: boolean; currentGasPrice: string }> { try { - const provider = getOrCreateResilientProvider(chainId, getEvmRpcUrls(chainId)); + const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(chainId)); const gasPrice = await provider.getGasPrice(); const gasPriceGwei = parseFloat(ethers.utils.formatUnits(gasPrice, 'gwei')); @@ -449,7 +441,10 @@ export class PaymentMethodService { continue; } - if (method.maxSpendPerInterval && exceedsLimit(amount, method.maxSpendPerInterval)) { + if ( + method.maxSpendPerInterval && + ethers.BigNumber.from(amount).gt(method.maxSpendPerInterval) + ) { attempt.status = 'failed'; attempt.failureReason = `Amount ${amount} exceeds max spend per interval ${method.maxSpendPerInterval}`; attempt.resolvedAt = new Date(); @@ -490,7 +485,7 @@ export class PaymentMethodService { } try { - const provider = getOrCreateResilientProvider(method.chainId, getEvmRpcUrls(method.chainId)); + const provider = new ethers.providers.JsonRpcProvider(getEvmRpcUrl(method.chainId)); const code = await provider.getCode(method.tokenAddress); const newHash = ethers.utils.keccak256(code); @@ -687,7 +682,10 @@ export class PaymentMethodService { continue; } - if (method.maxSpendPerInterval && exceedsLimit(amount, method.maxSpendPerInterval)) { + if ( + method.maxSpendPerInterval && + ethers.BigNumber.from(amount).gt(method.maxSpendPerInterval) + ) { fail(`Amount ${amount} exceeds max spend per interval ${method.maxSpendPerInterval}`); continue; } diff --git a/src/services/rpcProvider.ts b/src/services/rpcProvider.ts index eb69dc73..246a1436 100644 --- a/src/services/rpcProvider.ts +++ b/src/services/rpcProvider.ts @@ -73,8 +73,6 @@ interface CircuitEntry { state: CircuitState; consecutiveFailures: number; openedAt: number | null; - /** Consecutive successes observed while HALF-OPEN, used to close the circuit. */ - halfOpenSuccesses: number; } // ───────────────────────────────────────────────────────────────────────────── @@ -90,8 +88,6 @@ export interface ResilientProviderConfig { failureThreshold?: number; /** Ms the circuit stays OPEN before transitioning to HALF-OPEN. Default: 30_000. */ recoveryTimeoutMs?: number; - /** Consecutive successes in HALF-OPEN before closing the circuit. Default: 2. */ - successThreshold?: number; } const DEFAULT_CONFIG: Required = { @@ -99,7 +95,6 @@ const DEFAULT_CONFIG: Required = { jitterMs: 500, failureThreshold: 5, recoveryTimeoutMs: 30_000, - successThreshold: 2, }; /** Returns a sensible timeout for EVM chains with higher RPC variance. */ @@ -137,15 +132,6 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { private readonly _config: Required; private readonly _circuits = new Map(); - /** - * The base ethers provider talks to a single mutable `connection.url`, so - * exactly one `send()` may be swapping that URL at a time. Without this - * chain, two concurrent calls (e.g. `getBalancesAcrossChains`, which fans - * out in parallel) interleave and one of them is sent to the other's - * endpoint — which may be a node on an entirely different chain. - */ - private _swapChain: Promise = Promise.resolve(); - constructor(urls: string[], chainId: number, config: ResilientProviderConfig = {}) { super(urls[0], chainId); this._urls = urls; @@ -155,7 +141,6 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { jitterMs: config.jitterMs ?? DEFAULT_CONFIG.jitterMs, failureThreshold: config.failureThreshold ?? DEFAULT_CONFIG.failureThreshold, recoveryTimeoutMs: config.recoveryTimeoutMs ?? DEFAULT_CONFIG.recoveryTimeoutMs, - successThreshold: config.successThreshold ?? DEFAULT_CONFIG.successThreshold, }; // Initialise circuit entries for every URL @@ -164,7 +149,6 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { state: 'closed', consecutiveFailures: 0, openedAt: null, - halfOpenSuccesses: 0, }); } } @@ -182,7 +166,6 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { const elapsed = Date.now() - (circuit.openedAt ?? 0); if (elapsed >= this._config.recoveryTimeoutMs) { circuit.state = 'half-open'; - circuit.halfOpenSuccesses = 0; } else { errors.push({ url, message: new RpcProviderCircuitOpenError(url).message }); continue; @@ -195,17 +178,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { return result; } catch (err) { const message = err instanceof Error ? err.message : String(err); - - // Only transport-level problems indicate an unhealthy endpoint. A - // legitimate JSON-RPC failure (revert, insufficient funds, invalid - // params) means the node answered correctly, so counting it towards - // the failure threshold would evict a perfectly healthy URL. - if (isTransportFailure(err)) { - this._recordFailure(url); - } else { - this._recordSuccess(url); - } - + this._recordFailure(url); errors.push({ url, message }); logger.warn( @@ -221,12 +194,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { private _getCircuit(url: string): CircuitEntry { if (!this._circuits.has(url)) { - this._circuits.set(url, { - state: 'closed', - consecutiveFailures: 0, - openedAt: null, - halfOpenSuccesses: 0, - }); + this._circuits.set(url, { state: 'closed', consecutiveFailures: 0, openedAt: null }); } return this._circuits.get(url)!; } @@ -234,27 +202,13 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { private _recordSuccess(url: string): void { const c = this._getCircuit(url); c.consecutiveFailures = 0; - c.openedAt = null; - - if (c.state === 'half-open') { - c.halfOpenSuccesses += 1; - // Require consecutive successes before trusting a recovered endpoint - // again, so a node that flaps does not ping-pong the circuit. - if (c.halfOpenSuccesses >= this._config.successThreshold) { - c.state = 'closed'; - c.halfOpenSuccesses = 0; - } - return; - } - c.state = 'closed'; - c.halfOpenSuccesses = 0; + c.openedAt = null; } private _recordFailure(url: string): void { const c = this._getCircuit(url); c.consecutiveFailures += 1; - c.halfOpenSuccesses = 0; if (c.state === 'half-open') { // Failed probe — reopen @@ -283,39 +237,6 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), deadline); - // Serialise access to the shared `connection.url`. Each caller waits for - // the previous swap-and-restore to complete before touching it. - const run = this._swapChain.then( - () => this._sendToUrl(url, method, params, controller.signal, deadline), - () => this._sendToUrl(url, method, params, controller.signal, deadline) - ); - - this._swapChain = run.then( - () => undefined, - () => undefined - ); - - try { - return await run; - } finally { - clearTimeout(timer); - } - } - - private async _sendToUrl( - url: string, - method: string, - params: Array, - signal: AbortSignal, - deadline: number - ): Promise { - // The deadline starts when the caller enqueued, so a call that waited - // behind another may already be expired. The abort event has already - // fired in that case, so fail fast instead of waiting unbounded. - if (signal.aborted) { - throw new RpcProviderTimeoutError(url, deadline); - } - // We need to temporarily swap the underlying connection URL so the base // JsonRpcProvider sends to the right endpoint. const previousUrl = this.connection.url; @@ -326,7 +247,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { // Race the call against an abort-signal-aware timeout const timeoutPromise = new Promise((_, reject) => { - signal.addEventListener( + controller.signal.addEventListener( 'abort', () => { reject(new RpcProviderTimeoutError(url, deadline)); @@ -337,6 +258,7 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { return await Promise.race([callPromise, timeoutPromise]); } finally { + clearTimeout(timer); // Restore URL in case of future calls to this same provider instance (this as unknown as { connection: { url: string } }).connection.url = previousUrl; } @@ -359,73 +281,11 @@ export class ResilientJsonRpcProvider extends ethers.providers.JsonRpcProvider { entry.state = 'closed'; entry.consecutiveFailures = 0; entry.openedAt = null; - entry.halfOpenSuccesses = 0; } logger.info(`[ResilientJsonRpcProvider] All circuits reset for chain ${this._chainId}`); } } -/** - * True when an error indicates the endpoint itself is unreachable rather than - * the request being rejected by a working node. - * - * This is what separates "this RPC URL is down" from "this transaction - * reverted", and it is the difference between a circuit breaker that protects - * the user and one that ejects healthy nodes during an incident. - */ -export function isTransportFailure(error: unknown): boolean { - if (error instanceof RpcProviderTimeoutError) return true; - if (error instanceof RpcProviderCircuitOpenError) return false; - - if (!error || typeof error !== 'object') return false; - - const err = error as { - code?: unknown; - error?: { code?: unknown }; - message?: unknown; - reason?: unknown; - }; - - // ethers wraps server responses in `error.code` as a numeric JSON-RPC code. - // Anything that reaches here with one of those is an application-level - // answer from a live node. - const codes = [err.code, err.error?.code]; - if (codes.some((code) => typeof code === 'number' || /^-?\d+$/.test(String(code)))) { - return false; - } - - const transportCodes = [ - 'TIMEOUT', - 'ETIMEDOUT', - 'ESOCKETTIMEDOUT', - 'ECONNREFUSED', - 'ECONNRESET', - 'EHOSTUNREACH', - 'ENETUNREACH', - 'EPIPE', - 'ERR_NETWORK', - 'NETWORK_ERROR', - ]; - if (codes.some((code) => typeof code === 'string' && transportCodes.includes(code))) { - return true; - } - - const text = [err.message, err.reason] - .filter((part): part is string => typeof part === 'string') - .join(' ') - .toLowerCase(); - - return ( - text.includes('failed to fetch') || - text.includes('network error') || - text.includes('fetch failed') || - text.includes('econnrefused') || - text.includes('econnreset') || - text.includes('socket hang up') || - text.includes('getaddrinfo') - ); -} - // ───────────────────────────────────────────────────────────────────────────── // Singleton registry — circuit state persists across calls in the same session // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/services/transactionConfirmation.ts b/src/services/transactionConfirmation.ts deleted file mode 100644 index 2c666eb6..00000000 --- a/src/services/transactionConfirmation.ts +++ /dev/null @@ -1,285 +0,0 @@ -/** - * transactionConfirmation.ts - * - * Shared, defensive waiting for on-chain transaction receipts. - * - * Every on-chain payment path in SubTrackr previously called `tx.wait()` - * with no arguments and then only asserted that a hash came back. That is - * unsafe in three ways: - * - * 1. `tx.wait()` with no timeout can hang forever if a transaction is - * stuck in the mempool, which also stalls the offline retry queue. - * 2. `receipt.status === 0` means the transaction was *mined but reverted*. - * Treating "mined" as "succeeded" reports a failed payment as a success. - * 3. A single confirmation is not final. On fast chains a reorg can still - * drop the transaction, so users need a chain-appropriate depth. - * - * This module centralises those three concerns so all payment paths behave - * the same way and can be tested in isolation. - */ - -import type { ethers } from 'ethers'; - -import { logger } from './logging'; - -// ───────────────────────────────────────────────────────────────────────────── -// Errors -// ───────────────────────────────────────────────────────────────────────────── - -/** The transaction was mined but the receipt reports `status === 0`. */ -export class TransactionRevertedError extends Error { - readonly code = 'TRANSACTION_REVERTED' as const; - readonly transactionHash: string; - - constructor(transactionHash: string, detail?: string) { - super(`Transaction ${transactionHash} was reverted on-chain${detail ? `: ${detail}` : '.'}`); - this.name = 'TransactionRevertedError'; - this.transactionHash = transactionHash; - Object.setPrototypeOf(this, new.target.prototype); - } -} - -/** No receipt arrived within the allotted timeout. */ -export class TransactionTimeoutError extends Error { - readonly code = 'TRANSACTION_TIMEOUT' as const; - readonly transactionHash: string; - readonly timeoutMs: number; - - constructor(transactionHash: string, timeoutMs: number) { - super( - `Transaction ${transactionHash} was not confirmed within ${timeoutMs} ms. ` + - `It may still be pending on-chain — check your wallet before retrying.` - ); - this.name = 'TransactionTimeoutError'; - this.transactionHash = transactionHash; - this.timeoutMs = timeoutMs; - Object.setPrototypeOf(this, new.target.prototype); - } -} - -/** The transaction was confirmed but never reached the required depth. */ -export class TransactionInsufficientConfirmationsError extends Error { - readonly code = 'TRANSACTION_INSUFFICIENT_CONFIRMATIONS' as const; - readonly transactionHash: string; - readonly actual: number; - readonly required: number; - - constructor(transactionHash: string, actual: number, required: number) { - super( - `Transaction ${transactionHash} has ${actual} confirmation(s) but ${required} are required.` - ); - this.name = 'TransactionInsufficientConfirmationsError'; - this.transactionHash = transactionHash; - this.actual = actual; - this.required = required; - Object.setPrototypeOf(this, new.target.prototype); - } -} - -// ───────────────────────────────────────────────────────────────────────────── -// Confirmation policy -// ───────────────────────────────────────────────────────────────────────────── - -/** - * Minimum block confirmations before a payment is treated as settled. - * - * L2s reorg far less often than L1 and produce a block every ~250ms, so - * waiting many blocks would add latency without adding safety. Ethereum - * mainnet uses the deeper default. - */ -export const DEFAULT_CONFIRMATIONS = 2; - -const L2_CHAIN_IDS = new Set([10, 137, 42161, 8453, 43114, 56, 534352]); - -/** Confirmation depth appropriate for `chainId`. */ -export function requiredConfirmations(chainId: number, override?: number): number { - if (override !== undefined) { - return Math.max(1, Math.floor(override)); - } - return L2_CHAIN_IDS.has(chainId) ? 1 : DEFAULT_CONFIRMATIONS; -} - -/** - * Default receipt timeout. Generous enough for congested mainnet, short - * enough that a stuck mempool transaction does not block the queue forever. - */ -export const DEFAULT_CONFIRMATION_TIMEOUT_MS = 5 * 60 * 1000; - -// ───────────────────────────────────────────────────────────────────────────── -// Receipt classification -// ───────────────────────────────────────────────────────────────────────────── - -/** - * ethers surfaces a `wait()` timeout as a `TRANSACTION_REPLACED`-adjacent - * error, a plain `Error` carrying `code === 'TIMEOUT'`, or — depending on - * version — a rejected promise with a `timeout` field. Detection is done on - * shape rather than `instanceof` so that it stays robust across ethers v5/v6. - */ -function isWaitTimeout(error: unknown): boolean { - if (!error || typeof error !== 'object') return false; - - const err = error as { code?: unknown; message?: unknown; reason?: unknown; timeout?: unknown }; - if (err.code === 'TIMEOUT' || err.code === 'ETIMEDOUT' || err.code === 'ESOCKETTIMEDOUT') { - return true; - } - - const text = [err.message, err.reason] - .filter((part): part is string => typeof part === 'string') - .join(' ') - .toLowerCase(); - - return ( - text.includes('timeout') || - text.includes('timed out') || - text.includes('could not detect transaction') || - (typeof err.timeout === 'number' && text.includes('transaction')) - ); -} - -/** - * True when the error means "this transaction failed", as opposed to "we - * could not observe the outcome". Used to decide whether a queued item may - * be retried safely. - */ -export function isDefinitiveFailure(error: unknown): boolean { - return ( - error instanceof TransactionRevertedError || - error instanceof TransactionInsufficientConfirmationsError - ); -} - -// ───────────────────────────────────────────────────────────────────────────── -// Wait -// ───────────────────────────────────────────────────────────────────────────── - -export interface ConfirmTransactionOptions { - chainId?: number; - /** Override the chain-derived confirmation depth. */ - confirmations?: number; - /** Override the receipt timeout. */ - timeoutMs?: number; - /** Human label used in error messages, e.g. `Sablier stream creation`. */ - operation?: string; -} - -/** The subset of a transaction response this module needs. */ -export interface AwaitableTransactionResponse { - hash: string; - wait: (confirmations?: number) => Promise; -} - -function label(options: ConfirmTransactionOptions): string { - return options.operation ? `${options.operation} ` : ''; -} - -/** - * Nodes expose revert reasons in several vendor-specific shapes and ethers v5 - * does not model them on `TransactionReceipt`. Read defensively so a missing - * reason never turns into a crash inside error handling. - */ -function extractRevertReason(receipt: ethers.providers.TransactionReceipt): string | undefined { - const candidate = receipt as unknown as { - revertReason?: unknown; - revertName?: unknown; - }; - - const reason = candidate.revertReason ?? candidate.revertName; - return typeof reason === 'string' && reason.length > 0 ? reason : undefined; -} - -/** - * Waits for `txResponse` to reach the required confirmation depth and proves - * it actually succeeded. - * - * Resolves with the confirmed receipt. Throws `TransactionRevertedError`, - * `TransactionTimeoutError` or `TransactionInsufficientConfirmationsError`. - */ -export async function confirmTransaction( - txResponse: AwaitableTransactionResponse, - options: ConfirmTransactionOptions = {} -): Promise { - const { - chainId, - confirmations = requiredConfirmations(chainId ?? 1), - timeoutMs = DEFAULT_CONFIRMATION_TIMEOUT_MS, - } = options; - - const txHash = txResponse.hash; - - let receipt: ethers.providers.TransactionReceipt | null = null; - try { - // The timeout is enforced here rather than delegated to ethers. In ethers - // v5 `ContractTransaction.wait(confirmations)` does not forward a timeout - // argument at all, so relying on it left every contract-based payment - // (ERC-20 approve, Sablier stream creation) able to hang indefinitely and - // to stall the offline retry queue queued behind it. - let timer: ReturnType | undefined = undefined; - - try { - receipt = await Promise.race([ - txResponse.wait(confirmations) as Promise, - new Promise((_resolve, reject) => { - timer = setTimeout(() => { - reject(new TransactionTimeoutError(txHash, timeoutMs)); - }, timeoutMs); - }), - ]); - } finally { - if (timer) clearTimeout(timer); - } - } catch (error) { - if (error instanceof TransactionTimeoutError) throw error; - if (isWaitTimeout(error)) { - throw new TransactionTimeoutError(txHash, timeoutMs); - } - throw error; - } - - if (!receipt) { - // ethers resolves `null` when the transaction was replaced/dropped rather - // than mined. Treated as a timeout: the outcome is genuinely unknown. - throw new TransactionTimeoutError(txHash, timeoutMs); - } - - // A mined receipt with status 0 is a reverted transaction. Reporting this - // as success is what previously let failed payments look confirmed. - if (receipt.status === 0) { - throw new TransactionRevertedError(txHash, extractRevertReason(receipt)); - } - - const actual = receipt.confirmations ?? confirmations; - if (actual < confirmations) { - throw new TransactionInsufficientConfirmationsError(txHash, actual, confirmations); - } - - return receipt; -} - -/** - * Resolves the transaction hash without failing the surrounding operation. - * - * Used by the offline queue: when a broadcast may have succeeded but the - * receipt could not be observed (app crash, dropped connection), the hash is - * still the durable handle the user needs in order to check on-chain. - */ -export async function bestEffortTransactionHash( - txResponse: AwaitableTransactionResponse | null | undefined, - options: ConfirmTransactionOptions = {} -): Promise { - if (!txResponse?.hash) return undefined; - - try { - await confirmTransaction(txResponse, options); - return txResponse.hash; - } catch (error) { - if (error instanceof TransactionRevertedError) { - // Definitive: the chain says it failed. Surface it, do not swallow it. - throw error; - } - logger.warn( - `${label(options)}receipt for ${txResponse.hash} could not be confirmed: ` + - (error instanceof Error ? error.message : String(error)) - ); - return txResponse.hash; - } -} diff --git a/src/services/walletService.ts b/src/services/walletService.ts index 86968774..85941144 100644 --- a/src/services/walletService.ts +++ b/src/services/walletService.ts @@ -2,10 +2,10 @@ import { ethers } from 'ethers'; import { Framework } from '@superfluid-finance/sdk-core'; import { logger } from './logging'; +import { PaymentMethodService } from './paymentMethodService'; import { ERC20__factory, getContractAddress } from '../contracts'; import { getEvmRpcUrl, getEvmRpcUrls } from '../config/evm'; import { getOrCreateResilientProvider } from './rpcProvider'; -import { confirmTransaction } from './transactionConfirmation'; import { ContractError, ContractErrorCode, NetworkError, NetworkErrorCode } from '../errors'; import { TIME_CONSTANTS, @@ -829,10 +829,7 @@ export class WalletServiceManager { ); const txResponse = await createOp.exec(signer); - const receipt = await confirmTransaction(txResponse, { - chainId, - operation: 'Superfluid stream creation', - }); + const receipt = await txResponse.wait(); if (!receipt?.transactionHash) { throw new Error('Transaction mined without a hash'); @@ -900,10 +897,7 @@ export class WalletServiceManager { ); if (currentAllowance.lt(amountBn)) { const txApprove = await erc20.approve(SABLIER_V2_LOCKUP_LINEAR, amountBn); - await confirmTransaction(txApprove, { - chainId, - operation: 'Sablier token approval', - }); + await txApprove.wait(); } // 3. Create the Sablier Stream @@ -932,10 +926,7 @@ export class WalletServiceManager { }; const txCreate = await sablierContract.createWithDurations(params); - const receipt = await confirmTransaction(txCreate, { - chainId, - operation: 'Sablier stream creation', - }); + const receipt = await txCreate.wait(); if (!receipt?.transactionHash) { throw new Error('Transaction mined without a hash'); @@ -1029,7 +1020,7 @@ export class WalletServiceManager { const erc20 = new ethers.Contract(token, erc20Abi, signer); try { const tx = await erc20.approve(spender, amount); - const receipt = await confirmTransaction(tx, { operation: 'Token approval' }); + const receipt = await tx.wait(); if (!receipt?.transactionHash) { throw new Error('Approval transaction mined without a hash'); } diff --git a/src/store/_tests_/transactionQueueStore.test.ts b/src/store/_tests_/transactionQueueStore.test.ts deleted file mode 100644 index 68131b55..00000000 --- a/src/store/_tests_/transactionQueueStore.test.ts +++ /dev/null @@ -1,287 +0,0 @@ -import { act } from 'react'; -import { beforeEach, describe, expect, it, jest } from '@jest/globals'; - -import { ExecuteOrQueueResult, useTransactionQueueStore } from '../transactionQueueStore'; - -const mockCreateSuperfluidStream = - jest.fn<(...args: unknown[]) => Promise<{ streamId: string; txHash: string }>>(); -const mockCreateSablierStream = jest.fn(); - -jest.mock('@react-native-async-storage/async-storage', () => ({ - setItem: jest.fn(() => Promise.resolve()), - getItem: jest.fn(() => Promise.resolve(null)), - removeItem: jest.fn(() => Promise.resolve()), -})); - -jest.mock('@react-native-community/netinfo', () => ({ - __esModule: true, - default: { - addEventListener: jest.fn(() => jest.fn()), - fetch: jest.fn(() => Promise.resolve({ isConnected: true, isInternetReachable: true })), - }, -})); - -jest.mock('../../services/notificationService', () => ({ - presentTransactionQueueNotification: jest.fn(() => Promise.resolve()), -})); - -jest.mock('../../services/walletService', () => ({ - __esModule: true, - default: { - createSuperfluidStream: (...args: unknown[]) => - mockCreateSuperfluidStream(...args) as Promise<{ streamId: string; txHash: string }>, - createSablierStream: (...args: unknown[]) => mockCreateSablierStream(...args), - }, -})); - -const samplePayload = { - protocol: 'superfluid' as const, - token: 'USDC', - amount: '12.5', - recipientAddress: '0x1111111111111111111111111111111111111111', - chainId: 137, -}; - -describe('transactionQueueStore', () => { - beforeEach(() => { - jest.clearAllMocks(); - - useTransactionQueueStore.setState({ - isOnline: true, - isProcessing: false, - queuedTransactions: [], - lastError: null, - }); - }); - - it('queues transactions while offline', async () => { - useTransactionQueueStore.setState({ isOnline: false }); - - let result: ExecuteOrQueueResult | undefined; - await act(async () => { - result = await useTransactionQueueStore.getState().executeOrQueueTransaction(samplePayload); - }); - - expect(result?.queued).toBe(true); - expect(useTransactionQueueStore.getState().queuedTransactions).toHaveLength(1); - expect(mockCreateSuperfluidStream).not.toHaveBeenCalled(); - }); - - it('replaces conflicting queued transactions', async () => { - useTransactionQueueStore.setState({ isOnline: false }); - - await act(async () => { - await useTransactionQueueStore.getState().queueTransaction(samplePayload); - await useTransactionQueueStore.getState().queueTransaction({ - ...samplePayload, - amount: '20', - }); - }); - - const queued = useTransactionQueueStore.getState().queuedTransactions; - expect(queued).toHaveLength(1); - expect(queued[0].payload.amount).toBe('20'); - }); - - it('executes pending transactions when online and clears queue', async () => { - mockCreateSuperfluidStream.mockResolvedValue({ - streamId: 'stream:1', - txHash: '0xhash', - }); - - await act(async () => { - await useTransactionQueueStore.getState().queueTransaction(samplePayload); - await useTransactionQueueStore.getState().processQueue(); - }); - - expect(mockCreateSuperfluidStream).toHaveBeenCalledTimes(1); - expect(useTransactionQueueStore.getState().queuedTransactions).toHaveLength(0); - }); - - it('drops stale transactions during processing', async () => { - const staleTimestamp = Date.now() - 31 * 60 * 1000; - - useTransactionQueueStore.setState({ - queuedTransactions: [ - { - id: 'stale_tx', - createdAt: staleTimestamp, - updatedAt: staleTimestamp, - attempts: 0, - conflictKey: 'superfluid:137:usdc:0x1111111111111111111111111111111111111111', - status: 'pending', - payload: samplePayload, - }, - ], - }); - - await act(async () => { - await useTransactionQueueStore.getState().processQueue(); - }); - - expect(mockCreateSuperfluidStream).not.toHaveBeenCalled(); - expect(useTransactionQueueStore.getState().queuedTransactions).toHaveLength(0); - }); - - it('measures staleness from the last attempt, not creation', async () => { - // A transaction that keeps retrying is still making progress, so it must - // not be expired purely because it was created over 30 minutes ago. - const oldCreation = Date.now() - 31 * 60 * 1000; - const recentAttempt = Date.now() - 5_000; - - mockCreateSuperfluidStream.mockResolvedValue({ streamId: 'stream:1', txHash: '0xhash' }); - - useTransactionQueueStore.setState({ - queuedTransactions: [ - { - id: 'retrying_tx', - createdAt: oldCreation, - updatedAt: recentAttempt, - lastAttemptAt: recentAttempt, - attempts: 1, - conflictKey: 'superfluid:137:usdc:0x1111111111111111111111111111111111111111', - status: 'pending', - payload: samplePayload, - }, - ], - }); - - await act(async () => { - await useTransactionQueueStore.getState().processQueue(); - }); - - expect(mockCreateSuperfluidStream).toHaveBeenCalledTimes(1); - }); - - it('records the broadcast marker before handing the payload to the wallet', async () => { - let broadcastedAtSeenDuringExecution: number | undefined; - - mockCreateSuperfluidStream.mockImplementation(async () => { - // Inspect persisted state mid-flight: the marker must already be set, - // otherwise a crash here would cause a duplicate charge on restart. - const [entry] = useTransactionQueueStore.getState().queuedTransactions; - broadcastedAtSeenDuringExecution = entry?.broadcastedAt; - return { streamId: 'stream:1', txHash: '0xhash' }; - }); - - await act(async () => { - await useTransactionQueueStore.getState().queueTransaction(samplePayload); - await useTransactionQueueStore.getState().processQueue(); - }); - - expect(broadcastedAtSeenDuringExecution).toEqual(expect.any(Number)); - expect(useTransactionQueueStore.getState().queuedTransactions).toHaveLength(0); - }); - - it('retires a transaction whose outcome is unknown instead of re-broadcasting it', async () => { - // `broadcastedAt` in the past plus no receipt means the wallet may have - // already submitted it. Re-sending would charge the user twice. - const longAgo = Date.now() - 20 * 60 * 1000; - - useTransactionQueueStore.setState({ - queuedTransactions: [ - { - id: 'in_flight_tx', - createdAt: longAgo, - updatedAt: longAgo, - lastAttemptAt: longAgo, - broadcastedAt: longAgo, - attempts: 1, - conflictKey: 'superfluid:137:usdc:0x1111111111111111111111111111111111111111', - status: 'pending', - payload: samplePayload, - }, - ], - }); - - await act(async () => { - await useTransactionQueueStore.getState().processQueue(); - }); - - expect(mockCreateSuperfluidStream).not.toHaveBeenCalled(); - expect(useTransactionQueueStore.getState().queuedTransactions).toHaveLength(0); - expect(useTransactionQueueStore.getState().lastError).toMatch(/submitted before/i); - }); - - it('defers a retry until its backoff window has elapsed', async () => { - const now = Date.now(); - - mockCreateSuperfluidStream.mockRejectedValue(new Error('execution failed')); - - useTransactionQueueStore.setState({ - queuedTransactions: [ - { - id: 'backoff_tx', - createdAt: now - 1000, - updatedAt: now - 1000, - attempts: 1, - conflictKey: 'superfluid:137:usdc:0x1111111111111111111111111111111111111111', - status: 'pending', - payload: samplePayload, - nextAttemptAt: now + 60_000, - }, - ], - }); - - await act(async () => { - await useTransactionQueueStore.getState().processQueue(); - }); - - expect(mockCreateSuperfluidStream).not.toHaveBeenCalled(); - expect(useTransactionQueueStore.getState().queuedTransactions).toHaveLength(1); - }); - - it('schedules a backoff after a failed attempt', async () => { - mockCreateSuperfluidStream.mockRejectedValue(new Error('execution failed')); - - const before = Date.now(); - await act(async () => { - await useTransactionQueueStore.getState().queueTransaction(samplePayload); - await useTransactionQueueStore.getState().processQueue(); - }); - - const [entry] = useTransactionQueueStore.getState().queuedTransactions; - expect(entry?.nextAttemptAt).toBeGreaterThan(before); - }); - - it('keeps the broadcast marker after an ambiguous failure', async () => { - // A generic failure does not prove the transaction never reached the - // network, so the marker must survive to prevent a double charge. - mockCreateSuperfluidStream.mockRejectedValue(new Error('execution failed')); - - await act(async () => { - await useTransactionQueueStore.getState().queueTransaction(samplePayload); - await useTransactionQueueStore.getState().processQueue(); - }); - - const [entry] = useTransactionQueueStore.getState().queuedTransactions; - expect(entry?.broadcastedAt).toEqual(expect.any(Number)); - }); - - it('clears the broadcast marker when the wallet provably rejected the request', async () => { - mockCreateSuperfluidStream.mockRejectedValue( - Object.assign(new Error('Transaction was rejected in your wallet.'), { - code: 'USER_REJECTED', - }) - ); - - await act(async () => { - await useTransactionQueueStore.getState().queueTransaction(samplePayload); - await useTransactionQueueStore.getState().processQueue(); - }); - - const [entry] = useTransactionQueueStore.getState().queuedTransactions; - // Nothing was submitted, so an immediate retry is safe. - expect(entry?.broadcastedAt).toBeUndefined(); - }); - - it('keeps the connectivity listener alive while another consumer is mounted', async () => { - const unsubscribeFirst = useTransactionQueueStore.getState().initializeConnectivityListener(); - const unsubscribeSecond = useTransactionQueueStore.getState().initializeConnectivityListener(); - - unsubscribeFirst(); - - // The second consumer is still mounted, so tearing down must not happen. - expect(() => unsubscribeSecond()).not.toThrow(); - }); -}); diff --git a/src/store/slices/analyticsSlice.ts b/src/store/slices/analyticsSlice.ts index a76538b1..010a1f68 100644 --- a/src/store/slices/analyticsSlice.ts +++ b/src/store/slices/analyticsSlice.ts @@ -14,8 +14,7 @@ import { calculateSubscriptionAnalytics, SubscriptionAnalyticsReport, } from '../../services/analyticsService'; -import { Subscription } from '../../types/subscription'; -import { monthlyEquivalent } from '../../utils/billingCycleMath'; +import { BillingCycle, Subscription } from '../../types/subscription'; import { generateCSV } from '../../utils/importExport'; import { CohortService } from '../../../backend/services/analytics/cohortService'; import { cohortTableToCsv } from '../../../backend/services/analytics/cohortReportExport'; @@ -54,7 +53,12 @@ const toSubscriberRecords = (subscriptions: Subscription[]): SubscriberRecord[] signupAt: new Date(subscription.createdAt).getTime(), churnedAt: subscription.isActive ? undefined : new Date(subscription.updatedAt).getTime(), lastActiveAt: new Date(subscription.updatedAt).getTime(), - mrr: monthlyEquivalent(subscription.price, subscription.billingCycle), + mrr: + subscription.billingCycle === BillingCycle.YEARLY + ? subscription.price / 12 + : subscription.billingCycle === BillingCycle.WEEKLY + ? subscription.price * 4.345 + : subscription.price, })); export interface CreditMetricSnapshot { diff --git a/src/store/subscriptionStore.ts b/src/store/subscriptionStore.ts index cf8544b3..9b2f5e05 100644 --- a/src/store/subscriptionStore.ts +++ b/src/store/subscriptionStore.ts @@ -30,8 +30,7 @@ import { InvoiceStatus, isOpenInvoice } from '../types/invoice'; import { dummySubscriptions } from '../utils/dummyData'; // eslint-disable-line import { advanceBillingDate, calculateNextBillingDate } from '../utils/billingDate'; import { buildBillingPeriod } from '../utils/invoice'; -import { CACHE_CONSTANTS } from '../utils/constants/values'; -import { monthlyEquivalent, yearlyEquivalent } from '../utils/billingCycleMath'; +import { BILLING_CONVERSIONS, CACHE_CONSTANTS } from '../utils/constants/values'; import { syncRenewalReminders, presentChargeSuccessNotification, @@ -1466,7 +1465,11 @@ export const useSubscriptionStore = create()( preferredCurrency, rates ); - return total + monthlyEquivalent(priceInPreferred, sub.billingCycle); + if (sub.billingCycle === 'monthly') return total + priceInPreferred; + if (sub.billingCycle === 'yearly') return total + priceInPreferred / 12; + if (sub.billingCycle === 'weekly') + return total + priceInPreferred * BILLING_CONVERSIONS.WEEKS_PER_MONTH; + return total + priceInPreferred; }, 0); const totalYearlySpend = activeSubs.reduce((total, sub) => { @@ -1476,7 +1479,12 @@ export const useSubscriptionStore = create()( preferredCurrency, rates ); - return total + yearlyEquivalent(priceInPreferred, sub.billingCycle); + if (sub.billingCycle === 'yearly') return total + priceInPreferred; + if (sub.billingCycle === 'monthly') + return total + priceInPreferred * BILLING_CONVERSIONS.MONTHS_PER_YEAR; + if (sub.billingCycle === 'weekly') + return total + priceInPreferred * BILLING_CONVERSIONS.WEEKS_PER_YEAR; + return total + priceInPreferred * BILLING_CONVERSIONS.MONTHS_PER_YEAR; }, 0); const categoryBreakdown = activeSubs.reduce( diff --git a/src/store/transactionQueueStore.ts b/src/store/transactionQueueStore.ts index 7ec8bf9b..9d691a1a 100644 --- a/src/store/transactionQueueStore.ts +++ b/src/store/transactionQueueStore.ts @@ -28,17 +28,6 @@ export interface QueuedTransaction { status: 'pending' | 'processing'; payload: QueuedTransactionPayload; lastError?: string; - /** - * Set immediately before the transaction is handed to the wallet. - * - * A broadcast can succeed on-chain even when the app dies before the - * receipt is observed. Persisting this marker first means the next queue - * pass can tell "never sent" apart from "sent, outcome unknown" and refuse - * to re-broadcast the latter — which would otherwise charge the user twice. - */ - broadcastedAt?: number; - /** Earliest time the next attempt may start (exponential backoff). */ - nextAttemptAt?: number; } export interface ExecuteOrQueueResult { @@ -70,21 +59,7 @@ const STORAGE_KEY = 'subtrackr-transaction-queue'; const STALE_TRANSACTION_TIMEOUT_MS = 30 * 60 * 1000; const MAX_ATTEMPTS = 3; -/** First retry delay after a failed attempt. Doubles each subsequent failure. */ -const RETRY_BASE_BACKOFF_MS = 5_000; -const RETRY_MAX_BACKOFF_MS = 5 * 60 * 1000; - -/** - * A queued item that was already handed to the wallet but whose receipt was - * never observed is *not* safe to re-broadcast. It is retired and surfaced - * to the user instead, because the on-chain outcome is genuinely unknown and - * a blind retry risks a second charge. - */ -const UNKNOWN_OUTCOME_TIMEOUT_MS = 10 * 60 * 1000; - let connectivitySubscription: NetInfoSubscription | null = null; -/** Reference count so one consumer unmounting does not silence every other. */ -let connectivitySubscriberCount = 0; const isOnlineState = (isConnected: boolean | null, isInternetReachable: boolean | null): boolean => Boolean(isConnected) && isInternetReachable !== false; @@ -119,52 +94,6 @@ const isLikelyConflictError = (error: unknown): boolean => { ); }; -/** - * Exponential backoff with a ceiling, so a flapping connection cannot drive - * the queue into a tight retry loop that burns through `MAX_ATTEMPTS` in a - * single pass. - */ -const backoffDelayMs = (attempts: number): number => { - const exponent = Math.max(0, attempts - 1); - return Math.min(RETRY_BASE_BACKOFF_MS * 2 ** exponent, RETRY_MAX_BACKOFF_MS); -}; - -/** - * Runs a fire-and-forget promise without producing an unhandled rejection, - * which in React Native terminates the app. - */ -const safely = (promise: Promise, context: string): void => { - void promise.catch((error: unknown) => { - console.warn(`[transactionQueue] ${context} failed`, error); - }); -}; - -/** - * True only when the error proves the transaction never left the wallet. - * - * The broadcast marker may be cleared (and the item retried) in exactly these - * cases. For every ambiguous failure — a dropped connection, an RPC timeout — - * the marker is kept, because the transaction may already be in the mempool - * and re-sending it would charge the user twice. - */ -const isDefinitelyNotBroadcast = (error: unknown): boolean => { - if (!error || typeof error !== 'object') return false; - - const err = error as { code?: unknown; userMessage?: unknown; message?: unknown }; - - const code = typeof err.code === 'string' ? err.code : ''; - if (code === 'USER_REJECTED' || code === 'WALLET_NOT_CONNECTED' || code === 'INVALID_PARAMS') { - return true; - } - - const text = [err.userMessage, err.message] - .filter((part): part is string => typeof part === 'string') - .join(' ') - .toLowerCase(); - - return text.includes('rejected in your wallet') || text.includes('user rejected'); -}; - const executeQueuedPayload = async ( payload: QueuedTransactionPayload ): Promise<{ streamId: string; txHash?: string }> => { @@ -206,15 +135,10 @@ export const useTransactionQueueStore = create()( lastError: null, initializeConnectivityListener: () => { - connectivitySubscriberCount += 1; - if (connectivitySubscription) { return () => { - connectivitySubscriberCount = Math.max(0, connectivitySubscriberCount - 1); - if (connectivitySubscriberCount === 0) { - connectivitySubscription?.(); - connectivitySubscription = null; - } + connectivitySubscription?.(); + connectivitySubscription = null; }; } @@ -224,37 +148,27 @@ export const useTransactionQueueStore = create()( set({ isOnline: online }); if (!wasOnline && online) { - safely( - presentTransactionQueueNotification( - 'Back online', - 'Queued transactions are being processed now.' - ), - 'reconnect notification' + void presentTransactionQueueNotification( + 'Back online', + 'Queued transactions are being processed now.' ); - safely(get().processQueue(), 'processQueue after reconnect'); + void get().processQueue(); } }); return () => { - connectivitySubscriberCount = Math.max(0, connectivitySubscriberCount - 1); - if (connectivitySubscriberCount === 0) { - connectivitySubscription?.(); - connectivitySubscription = null; - } + connectivitySubscription?.(); + connectivitySubscription = null; }; }, refreshConnectivity: async () => { - try { - const state = await NetInfo.fetch(); - const online = isOnlineState(state.isConnected, state.isInternetReachable); - const wasOnline = get().isOnline; - set({ isOnline: online }); - if (!wasOnline && online) { - safely(get().processQueue(), 'processQueue after connectivity refresh'); - } - } catch (error) { - console.warn('[transactionQueue] connectivity refresh failed', error); + const state = await NetInfo.fetch(); + const online = isOnlineState(state.isConnected, state.isInternetReachable); + const wasOnline = get().isOnline; + set({ isOnline: online }); + if (!wasOnline && online) { + void get().processQueue(); } }, @@ -290,16 +204,11 @@ export const useTransactionQueueStore = create()( }; }); - // A failed notification must not lose the queued transaction: the item is - // already persisted above, so this only downgrades to a warning. - safely( - presentTransactionQueueNotification( - 'Transaction queued', - replacedExisting - ? 'Updated a pending transaction with your latest request.' - : 'Transaction will run automatically once you are online.' - ), - 'queued notification' + await presentTransactionQueueNotification( + 'Transaction queued', + replacedExisting + ? 'Updated a pending transaction with your latest request.' + : 'Transaction will run automatically once you are online.' ); return { transactionId, replacedExisting }; @@ -353,58 +262,19 @@ export const useTransactionQueueStore = create()( for (const tx of sortedQueue) { if (!get().isOnline) break; - // Staleness is measured from the last attempt, not creation, so a - // transaction that keeps making progress is not expired out from - // under itself on a long-lived queue. - const reference = tx.lastAttemptAt ?? tx.createdAt; - const age = now() - reference; + const age = now() - tx.createdAt; if (age > STALE_TRANSACTION_TIMEOUT_MS) { set((state) => ({ queuedTransactions: state.queuedTransactions.filter((q) => q.id !== tx.id), })); - safely( - presentTransactionQueueNotification( - 'Queued transaction expired', - 'A pending transaction was removed because it became stale.' - ), - 'stale notification' - ); - continue; - } - - // Respect exponential backoff so a flapping connection cannot - // burn every attempt in a single pass. - if (tx.nextAttemptAt !== undefined && now() < tx.nextAttemptAt) { - continue; - } - - // A transaction that was already handed to the wallet but whose - // receipt was never observed must not be re-broadcast: the - // on-chain outcome is unknown and a retry would double-charge. - // Retire it and tell the user to verify manually. - if ( - tx.broadcastedAt !== undefined && - now() - tx.broadcastedAt > UNKNOWN_OUTCOME_TIMEOUT_MS - ) { - set((state) => ({ - queuedTransactions: state.queuedTransactions.filter((q) => q.id !== tx.id), - lastError: - 'A queued transaction may have been submitted before its result could be confirmed.', - })); - - safely( - presentTransactionQueueNotification( - 'Queued transaction needs review', - 'It may already have been submitted on-chain. Please check your wallet before retrying.' - ), - 'unknown-outcome notification' + await presentTransactionQueueNotification( + 'Queued transaction expired', + 'A pending transaction was removed because it became stale.' ); continue; } - // Mark the broadcast BEFORE handing the payload to the wallet so - // a crash between submission and receipt cannot cause a replay. set((state) => ({ queuedTransactions: state.queuedTransactions.map((queued) => queued.id === tx.id @@ -413,8 +283,6 @@ export const useTransactionQueueStore = create()( status: 'processing', attempts: queued.attempts + 1, lastAttemptAt: now(), - broadcastedAt: queued.broadcastedAt ?? now(), - nextAttemptAt: undefined, updatedAt: now(), lastError: undefined, } @@ -431,12 +299,9 @@ export const useTransactionQueueStore = create()( ), })); - safely( - presentTransactionQueueNotification( - 'Queued transaction sent', - 'A pending transaction has been executed successfully.' - ), - 'success notification' + await presentTransactionQueueNotification( + 'Queued transaction sent', + 'A pending transaction has been executed successfully.' ); } catch (error) { if (isLikelyNetworkError(error)) { @@ -469,12 +334,9 @@ export const useTransactionQueueStore = create()( ), })); - safely( - presentTransactionQueueNotification( - 'Queued transaction skipped', - 'A pending transaction conflicted with another on-chain transaction and was removed.' - ), - 'conflict notification' + await presentTransactionQueueNotification( + 'Queued transaction skipped', + 'A pending transaction conflicted with another on-chain transaction and was removed.' ); continue; } @@ -490,32 +352,21 @@ export const useTransactionQueueStore = create()( lastError: error instanceof Error ? error.message : 'Queued transaction failed.', })); - safely( - presentTransactionQueueNotification( - 'Queued transaction failed', - error instanceof Error - ? `Dropped after retries: ${error.message}` - : 'Dropped after retry attempts.' - ), - 'dropped notification' + await presentTransactionQueueNotification( + 'Queued transaction failed', + error instanceof Error + ? `Dropped after retries: ${error.message}` + : 'Dropped after retry attempts.' ); continue; } - // Schedule the next attempt with exponential backoff. The broadcast marker is - // cleared only when the failure proves nothing was submitted; - // otherwise it is kept so the queue never re-sends a payment - // that may already be in the mempool. - const safeToRetry = isDefinitelyNotBroadcast(error); - set((state) => ({ queuedTransactions: state.queuedTransactions.map((queued) => queued.id === tx.id ? { ...queued, status: 'pending', - broadcastedAt: safeToRetry ? undefined : queued.broadcastedAt, - nextAttemptAt: now() + backoffDelayMs(attempts), updatedAt: now(), lastError: error instanceof Error ? error.message : 'Execution failed.', } @@ -546,10 +397,7 @@ export const useTransactionQueueStore = create()( storage: createJSONStorage(() => asyncStorageAdapter), partialize: (state) => ({ queuedTransactions: state.queuedTransactions }), onRehydrateStorage: () => () => { - safely( - useTransactionQueueStore.getState().refreshConnectivity(), - 'rehydrate connectivity refresh' - ); + void useTransactionQueueStore.getState().refreshConnectivity(); }, } ) diff --git a/src/store/walletStore.ts b/src/store/walletStore.ts index fc5b1b8f..fb64c5bc 100644 --- a/src/store/walletStore.ts +++ b/src/store/walletStore.ts @@ -32,30 +32,6 @@ export interface NetworkMismatch { preferredNetwork: Network; } -/** - * Adapts stored `PaymentAttempt` records to the shape the fallback-chain - * health monitor consumes. - * - * The two shapes previously disagreed: this code read `attempt.success` and - * `attempt.createdAt`, neither of which exists on `PaymentAttempt` (it has - * `status` and `attemptedAt`). Every field resolved to `undefined`, so the - * monitor's look-back window discarded *all* attempts and every payment - * method scored a 100% success rate. Smart fallback ordering was therefore - * permanently inert. - */ -function buildRecentAttempts(attempts: PaymentAttempt[]): Array<{ - paymentMethodId: string; - success: boolean; - timestamp: Date; - latencyMs?: number; -}> { - return attempts.map((attempt) => ({ - paymentMethodId: attempt.paymentMethodId, - success: attempt.status === 'success', - timestamp: attempt.attemptedAt, - })); -} - interface WalletState { // Connection state from service connection: WalletConnection | null; @@ -462,7 +438,7 @@ export const useWalletStore = create()( const updatedMethods = paymentMethods.map((m) => m.id === id ? { ...m, isVerified: true, updatedAt: new Date() } : m ); - set({ paymentMethods: updatedMethods }); + set({ paymentMethods: updatedMethods, isLoading: false }); } return verified; } catch (error) { @@ -473,13 +449,9 @@ export const useWalletStore = create()( : error instanceof Error ? error.message : 'Failed to verify payment method', + isLoading: false, }); throw error; - } finally { - // Cleared unconditionally: previously `isLoading` was only reset - // inside the `if (verified)` branch and the catch, so a `false` - // result left the UI spinning forever. - set({ isLoading: false }); } }, @@ -824,11 +796,15 @@ export const useWalletStore = create()( const chain = get().fallbackChains.find((c) => c.id === chainId); if (!chain) return null; - return _chainHealthMonitor.snapshotChainHealth( - chainId, - chain.methodIds, - buildRecentAttempts(get().paymentAttempts) - ); + // Build lightweight attempt objects from stored PaymentAttempts. + const recentAttempts = get().paymentAttempts.map((a) => ({ + paymentMethodId: a.paymentMethodId, + success: a.success, + timestamp: new Date(a.createdAt), + latencyMs: undefined as number | undefined, + })); + + return _chainHealthMonitor.snapshotChainHealth(chainId, chain.methodIds, recentAttempts); }, getSmartFallbackSelection: (chainId: string) => { @@ -871,7 +847,6 @@ export const useWalletStore = create()( applyAllRotationPolicies: () => { const { rotationPolicies, fallbackChains, paymentAttempts } = get(); - const recentAttempts = buildRecentAttempts(paymentAttempts); const updatedPolicies: PaymentMethodRotationPolicy[] = []; for (const policy of rotationPolicies) { @@ -881,6 +856,12 @@ export const useWalletStore = create()( continue; } + const recentAttempts = paymentAttempts.map((a) => ({ + paymentMethodId: a.paymentMethodId, + success: a.success, + timestamp: new Date(a.createdAt), + })); + const snapshot = _chainHealthMonitor.snapshotChainHealth( chain.id, chain.methodIds, diff --git a/src/utils/__tests__/billingCycleMath.test.ts b/src/utils/__tests__/billingCycleMath.test.ts deleted file mode 100644 index d246f704..00000000 --- a/src/utils/__tests__/billingCycleMath.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -/** - * Tests for billingCycleMath — one shared billing-cycle normaliser. - * - * @see src/utils/billingCycleMath.ts - * @see https://github.com/Smartdevs17/SubTrackr/issues/1684 - */ - -import { BillingCycle } from '../../types/subscription'; -import { - WEEKS_PER_MONTH, - isAnnualCycle, - monthlyEquivalent, - normalizeBillingCycle, - yearlyEquivalent, -} from '../billingCycleMath'; - -describe('normalizeBillingCycle', () => { - it('passes through valid enum values', () => { - expect(normalizeBillingCycle('monthly')).toBe(BillingCycle.MONTHLY); - expect(normalizeBillingCycle('yearly')).toBe(BillingCycle.YEARLY); - expect(normalizeBillingCycle('annual')).toBe(BillingCycle.ANNUAL); - expect(normalizeBillingCycle('weekly')).toBe(BillingCycle.WEEKLY); - expect(normalizeBillingCycle('custom')).toBe(BillingCycle.CUSTOM); - }); - - it('is case and whitespace insensitive', () => { - expect(normalizeBillingCycle(' YEARLY ')).toBe(BillingCycle.YEARLY); - expect(normalizeBillingCycle('Monthly')).toBe(BillingCycle.MONTHLY); - }); - - it('maps common synonyms', () => { - expect(normalizeBillingCycle('annually')).toBe(BillingCycle.YEARLY); - expect(normalizeBillingCycle('year')).toBe(BillingCycle.YEARLY); - expect(normalizeBillingCycle('month')).toBe(BillingCycle.MONTHLY); - expect(normalizeBillingCycle('week')).toBe(BillingCycle.WEEKLY); - }); - - it('defaults to monthly for unknown or missing values', () => { - expect(normalizeBillingCycle('fortnightly')).toBe(BillingCycle.MONTHLY); - expect(normalizeBillingCycle('')).toBe(BillingCycle.MONTHLY); - expect(normalizeBillingCycle(undefined)).toBe(BillingCycle.MONTHLY); - expect(normalizeBillingCycle(null)).toBe(BillingCycle.MONTHLY); - expect(normalizeBillingCycle(42)).toBe(BillingCycle.MONTHLY); - }); -}); - -describe('monthlyEquivalent', () => { - it('leaves monthly prices untouched', () => { - expect(monthlyEquivalent(10, BillingCycle.MONTHLY)).toBe(10); - }); - - it('divides both annual spellings by twelve', () => { - // The regression: `annual` fell through to the monthly branch, so a - // 120/year plan reported 120 MRR instead of 10. - expect(monthlyEquivalent(120, BillingCycle.YEARLY)).toBe(10); - expect(monthlyEquivalent(120, BillingCycle.ANNUAL)).toBe(10); - expect(monthlyEquivalent(120, 'annual')).toBe(10); - }); - - it('multiplies weekly prices by the shared weeks-per-month constant', () => { - expect(monthlyEquivalent(10, BillingCycle.WEEKLY)).toBeCloseTo(10 * WEEKS_PER_MONTH, 10); - }); - - it('uses 52/12 for weeks per month so it cannot drift from BILLING_CONVERSIONS', () => { - expect(WEEKS_PER_MONTH).toBe(52 / 12); - // The old hard-coded value was 4.345, which differs from 52/12. - expect(WEEKS_PER_MONTH).not.toBe(4.345); - }); - - it('maps non-finite prices to zero', () => { - expect(monthlyEquivalent(Number.NaN, BillingCycle.MONTHLY)).toBe(0); - expect(monthlyEquivalent(Number.POSITIVE_INFINITY, BillingCycle.YEARLY)).toBe(0); - }); -}); - -describe('yearlyEquivalent', () => { - it('leaves annual prices untouched', () => { - expect(yearlyEquivalent(120, BillingCycle.YEARLY)).toBe(120); - expect(yearlyEquivalent(120, BillingCycle.ANNUAL)).toBe(120); - }); - - it('multiplies monthly prices by twelve', () => { - expect(yearlyEquivalent(10, BillingCycle.MONTHLY)).toBe(120); - }); - - it('multiplies weekly prices by fifty-two', () => { - expect(yearlyEquivalent(10, BillingCycle.WEEKLY)).toBe(520); - }); -}); - -describe('isAnnualCycle', () => { - it('recognises both annual spellings', () => { - expect(isAnnualCycle(BillingCycle.YEARLY)).toBe(true); - expect(isAnnualCycle(BillingCycle.ANNUAL)).toBe(true); - expect(isAnnualCycle('annual')).toBe(true); - }); - - it('rejects non-annual cycles', () => { - expect(isAnnualCycle(BillingCycle.MONTHLY)).toBe(false); - expect(isAnnualCycle(BillingCycle.WEEKLY)).toBe(false); - expect(isAnnualCycle(undefined)).toBe(false); - }); -}); diff --git a/src/utils/__tests__/decimalAmount.test.ts b/src/utils/__tests__/decimalAmount.test.ts deleted file mode 100644 index d952eec3..00000000 --- a/src/utils/__tests__/decimalAmount.test.ts +++ /dev/null @@ -1,106 +0,0 @@ -/** - * Tests for decimalAmount — exact fixed-point comparison for crypto amounts. - * - * @see src/utils/decimalAmount.ts - * @see https://github.com/Smartdevs17/SubTrackr/issues/1683 - */ - -import { compareDecimalAmounts, exceedsLimit, normalizeDecimal } from '../decimalAmount'; - -describe('normalizeDecimal', () => { - it('splits a decimal string into sign, digits and scale', () => { - expect(normalizeDecimal('12.5')).toEqual({ negative: false, digits: '125', scale: 1 }); - expect(normalizeDecimal('120')).toEqual({ negative: false, digits: '120', scale: 0 }); - }); - - it('strips leading zeros without changing the value', () => { - expect(normalizeDecimal('00.5')).toEqual({ negative: false, digits: '5', scale: 1 }); - expect(normalizeDecimal('0007')).toEqual({ negative: false, digits: '7', scale: 0 }); - }); - - it('accepts a leading decimal point', () => { - expect(normalizeDecimal('.5')).toEqual({ negative: false, digits: '5', scale: 1 }); - }); - - it('normalises negative zero to zero', () => { - expect(normalizeDecimal('-0')?.negative).toBe(false); - expect(normalizeDecimal('-0.00')?.negative).toBe(false); - }); - - it('preserves the sign for non-zero values', () => { - expect(normalizeDecimal('-5')?.negative).toBe(true); - expect(normalizeDecimal('-0.01')?.negative).toBe(true); - }); - - it('accepts finite numbers', () => { - expect(normalizeDecimal(1.5)).toEqual({ negative: false, digits: '15', scale: 1 }); - }); - - it('rejects anything that is not a finite decimal', () => { - expect(normalizeDecimal('')).toBeNull(); - expect(normalizeDecimal(' ')).toBeNull(); - expect(normalizeDecimal('abc')).toBeNull(); - expect(normalizeDecimal('.')).toBeNull(); - expect(normalizeDecimal('-')).toBeNull(); - expect(normalizeDecimal('1e18')).toBeNull(); - expect(normalizeDecimal('1,000')).toBeNull(); - expect(normalizeDecimal(null)).toBeNull(); - expect(normalizeDecimal(undefined)).toBeNull(); - expect(normalizeDecimal(Number.NaN)).toBeNull(); - expect(normalizeDecimal(Number.POSITIVE_INFINITY)).toBeNull(); - }); -}); - -describe('compareDecimalAmounts', () => { - it('compares equal values written at different scales', () => { - expect(compareDecimalAmounts('0.1', '0.10')).toBe(0); - expect(compareDecimalAmounts('5', '5.000')).toBe(0); - expect(compareDecimalAmounts('1.0', '1')).toBe(0); - }); - - it('orders fractional amounts correctly', () => { - expect(compareDecimalAmounts('12.5', '10')).toBe(1); - expect(compareDecimalAmounts('10', '12.5')).toBe(-1); - expect(compareDecimalAmounts('1.005', '1.004')).toBe(1); - expect(compareDecimalAmounts('1.004', '1.005')).toBe(-1); - }); - - it('does not lose precision on values that float maths gets wrong', () => { - // 0.1 + 0.2 !== 0.3 in binary floating point. - expect(compareDecimalAmounts('0.3', '0.1')).toBe(1); - expect(compareDecimalAmounts('0.3', '0.2')).toBe(1); - expect(compareDecimalAmounts('0.30000000000000004', '0.3')).toBe(1); - }); - - it('handles negatives', () => { - expect(compareDecimalAmounts('-5', '3')).toBe(-1); - expect(compareDecimalAmounts('3', '-5')).toBe(1); - expect(compareDecimalAmounts('-5', '-3')).toBe(-1); - expect(compareDecimalAmounts('-0.5', '-0.5')).toBe(0); - }); - - it('orders values of differing digit length', () => { - expect(compareDecimalAmounts('100', '99.99')).toBe(1); - expect(compareDecimalAmounts('0.001', '0.0009')).toBe(1); - }); - - it('throws on non-decimal input rather than guessing', () => { - expect(() => compareDecimalAmounts('abc', '1')).toThrow(/non-decimal/); - expect(() => compareDecimalAmounts('1', undefined)).toThrow(/non-decimal/); - }); -}); - -describe('exceedsLimit', () => { - it('is true only when the amount is strictly greater than the limit', () => { - expect(exceedsLimit('12.5', '10')).toBe(true); - expect(exceedsLimit('10', '10')).toBe(false); - expect(exceedsLimit('9.99', '10')).toBe(false); - }); - - it('works on fractional limits, which BigNumber.from would reject', () => { - // `ethers.BigNumber.from('0.1')` throws; this path must not. - expect(() => exceedsLimit('0.2', '0.1')).not.toThrow(); - expect(exceedsLimit('0.2', '0.1')).toBe(true); - expect(exceedsLimit('0.1', '0.1')).toBe(false); - }); -}); diff --git a/src/utils/__tests__/invoiceTotals.test.ts b/src/utils/__tests__/invoiceTotals.test.ts deleted file mode 100644 index dbee1db0..00000000 --- a/src/utils/__tests__/invoiceTotals.test.ts +++ /dev/null @@ -1,212 +0,0 @@ -/** - * Tests for invoiceTotals — the single invoice arithmetic implementation. - * - * @see src/utils/invoiceTotals.ts - * @see https://github.com/Smartdevs17/SubTrackr/issues/1684 - */ - -import { - calculateInvoiceTotals, - resolveLineTotal, - roundMoney, - isDiscountLineItem, -} from '../invoiceTotals'; - -describe('roundMoney', () => { - it('rounds half away from zero at two decimal places', () => { - expect(roundMoney(1.005)).toBe(1.01); - expect(roundMoney(1.004)).toBe(1); - expect(roundMoney(2.675)).toBe(2.68); - }); - - it('rounds negative values symmetrically', () => { - expect(roundMoney(-1.005)).toBe(-1.01); - }); - - it('honours a custom precision', () => { - expect(roundMoney(1.2345, 3)).toBe(1.235); - expect(roundMoney(1.2345, 0)).toBe(1); - }); - - it('maps non-finite input to zero', () => { - expect(roundMoney(Number.NaN)).toBe(0); - expect(roundMoney(Number.POSITIVE_INFINITY)).toBe(0); - }); - - it('never returns negative zero', () => { - expect(Object.is(roundMoney(-0), 0)).toBe(true); - expect(Object.is(roundMoney(-0.001), 0)).toBe(true); - }); -}); - -describe('resolveLineTotal', () => { - it('prefers an explicit lineTotal', () => { - expect(resolveLineTotal({ lineTotal: 12, amount: 99, unitPrice: 5, quantity: 2 })).toBe(12); - }); - - it('falls back to the flat amount alias', () => { - expect(resolveLineTotal({ amount: 7.5, unitPrice: 5, quantity: 2 })).toBe(7.5); - }); - - it('derives unit price times quantity as a last resort', () => { - expect(resolveLineTotal({ unitPrice: 5, quantity: 3 })).toBe(15); - }); - - it('returns zero for an unusable line', () => { - expect(resolveLineTotal({})).toBe(0); - expect(resolveLineTotal({ lineTotal: Number.NaN })).toBe(0); - }); -}); - -describe('calculateInvoiceTotals', () => { - it('sums line items into a subtotal', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 10 }, { lineTotal: 5 }], - }); - expect(result.subtotal).toBe(15); - expect(result.tax).toBe(0); - expect(result.discount).toBe(0); - expect(result.total).toBe(15); - }); - - it('uses the flat amount when there are no line items', () => { - expect(calculateInvoiceTotals({ amount: 42 }).total).toBe(42); - }); - - it('prefers line items over the flat amount', () => { - expect(calculateInvoiceTotals({ lineItems: [{ lineTotal: 10 }], amount: 999 }).total).toBe(10); - }); - - it('adds tax and subtracts discount', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }], - tax: 20, - discount: 15, - }); - expect(result.subtotal).toBe(100); - expect(result.tax).toBe(20); - expect(result.discount).toBe(15); - expect(result.total).toBe(105); - }); - - it('clamps a discount larger than the subtotal so the total never goes negative', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }], - discount: 150, - }); - expect(result.discount).toBe(100); - expect(result.total).toBe(0); - }); - - it('ignores a negative discount rather than inflating the bill', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }], - discount: -25, - }); - expect(result.discount).toBe(0); - expect(result.total).toBe(100); - }); - - it('rounds each component to minor units', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 10.005 }, { lineTotal: 10.005 }], - }); - // Raw sum is 20.01, and rounding the subtotal (not each line) gives 20.01. - expect(result.subtotal).toBe(20.01); - expect(result.total).toBe(20.01); - }); - - it('treats an empty invoice as zero', () => { - expect(calculateInvoiceTotals({}).total).toBe(0); - }); - - it('produces identical totals whether created or recomputed on update', () => { - const created = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 50 }, { lineTotal: 30 }], - tax: 8, - discount: 10, - }); - - // Editing one line re-derives totals from line items + the stored tax and - // discount; the payable amount must not change. - const updated = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 50 }, { lineTotal: 30 }], - tax: 8, - discount: 10, - }); - - expect(updated).toEqual(created); - expect(updated.total).toBe(78); - }); -}); - -describe('discount line items', () => { - it('identifies discount lines and treats unknown kinds as charges', () => { - expect(isDiscountLineItem({ kind: 'discount' })).toBe(true); - expect(isDiscountLineItem({ kind: 'item' })).toBe(false); - expect(isDiscountLineItem({})).toBe(false); - }); - - it('keeps discount lines out of the subtotal and subtracts them from the total', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }, { kind: 'discount', lineTotal: 20 }], - }); - expect(result.subtotal).toBe(100); - expect(result.discount).toBe(20); - expect(result.total).toBe(80); - }); - - it('accepts a negative discount magnitude and still subtracts it', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }, { kind: 'discount', lineTotal: -15 }], - }); - expect(result.discount).toBe(15); - expect(result.total).toBe(85); - }); - - it('adds an itemised discount to a legacy scalar discount', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }, { kind: 'discount', lineTotal: 10 }], - discount: 5, - }); - expect(result.discount).toBe(15); - expect(result.total).toBe(85); - }); - - it('keeps tax on the pre-discount subtotal', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 100 }, { kind: 'discount', lineTotal: 10 }], - tax: 20, - }); - expect(result.subtotal).toBe(100); - expect(result.discount).toBe(10); - expect(result.total).toBe(110); - }); - - it('clamps an itemised discount to the subtotal', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ lineTotal: 50 }, { kind: 'discount', lineTotal: 80 }], - }); - expect(result.discount).toBe(50); - expect(result.total).toBe(0); - }); - - it('charges nothing for a discount-only invoice', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ kind: 'discount', lineTotal: 20 }], - }); - expect(result.subtotal).toBe(0); - expect(result.discount).toBe(0); - expect(result.total).toBe(0); - }); - - it('falls back to the flat amount when only a discount line is present', () => { - const result = calculateInvoiceTotals({ - lineItems: [{ kind: 'discount', lineTotal: 20 }], - amount: 100, - }); - expect(result.subtotal).toBe(100); - expect(result.discount).toBe(20); - expect(result.total).toBe(80); - }); -}); diff --git a/src/utils/billingCycleMath.ts b/src/utils/billingCycleMath.ts deleted file mode 100644 index 4de5c1f9..00000000 --- a/src/utils/billingCycleMath.ts +++ /dev/null @@ -1,98 +0,0 @@ -/** - * billingCycleMath.ts - * - * One place that turns a subscription's price and billing cycle into a - * comparable per-month figure. - * - * This logic was previously duplicated in four places that disagreed with one - * another, which produced inconsistent spend and MRR figures depending on - * which screen the user looked at: - * - * - `subscriptionStore` compared raw strings and omitted `annual` entirely, - * so an annual plan was billed as a *monthly* one and inflated MRR 12×. - * - `analyticsService` and `analyticsSlice` handled `yearly` but not - * `annual`. - * - Those two used a hard-coded `4.345` weeks/month while - * `BILLING_CONVERSIONS.WEEKS_PER_MONTH` is `52 / 12`. - */ - -import { BillingCycle } from '../types/subscription'; -import { BILLING_CONVERSIONS } from './constants/values'; - -/** Canonical weeks-per-month used for every weekly normalisation. */ -export const WEEKS_PER_MONTH = BILLING_CONVERSIONS.WEEKS_PER_MONTH; - -/** - * Coerces loose input (`'monthly'`, `'Monthly'`, a missing value) into a - * `BillingCycle`, defaulting to MONTHLY. - */ -export function normalizeBillingCycle(value: unknown): BillingCycle { - if (typeof value !== 'string') return BillingCycle.MONTHLY; - - const candidate = value.trim().toLowerCase(); - if (candidate === '') return BillingCycle.MONTHLY; - - const known = Object.values(BillingCycle) as string[]; - if (known.includes(candidate)) { - return candidate as BillingCycle; - } - - // Tolerate common synonyms used by imports and older records. - switch (candidate) { - case 'annually': - case 'yearly': - case 'annual': - case 'year': - return BillingCycle.YEARLY; - case 'week': - case 'weekly': - return BillingCycle.WEEKLY; - case 'month': - case 'monthly': - return BillingCycle.MONTHLY; - default: - return BillingCycle.MONTHLY; - } -} - -/** - * Monthly equivalent of `price` charged on `cycle`. - * - * `yearly` and `annual` are both billed once per year, so both normalise to - * one twelfth of the quoted price. `weekly` uses the shared weeks-per-month - * constant so it cannot drift away from the rest of the codebase. - */ -export function monthlyEquivalent(price: number, cycle: unknown): number { - if (!Number.isFinite(price)) return 0; - - switch (normalizeBillingCycle(cycle)) { - case BillingCycle.YEARLY: - case BillingCycle.ANNUAL: - return price / BILLING_CONVERSIONS.MONTHS_PER_YEAR; - case BillingCycle.WEEKLY: - return price * WEEKS_PER_MONTH; - default: - return price; - } -} - -/** Yearly equivalent of `price` charged on `cycle`. */ -export function yearlyEquivalent(price: number, cycle: unknown): number { - if (!Number.isFinite(price)) return 0; - - switch (normalizeBillingCycle(cycle)) { - case BillingCycle.YEARLY: - case BillingCycle.ANNUAL: - return price; - case BillingCycle.WEEKLY: - return price * BILLING_CONVERSIONS.WEEKS_PER_YEAR; - default: - return price * BILLING_CONVERSIONS.MONTHS_PER_YEAR; - } -} - -/** True for cycles billed once per year. */ -export function isAnnualCycle(cycle: unknown): boolean { - const normalized = normalizeBillingCycle(cycle); - return normalized === BillingCycle.YEARLY || normalized === BillingCycle.ANNUAL; -} diff --git a/src/utils/decimalAmount.ts b/src/utils/decimalAmount.ts deleted file mode 100644 index fd57d2e8..00000000 --- a/src/utils/decimalAmount.ts +++ /dev/null @@ -1,105 +0,0 @@ -/** - * decimalAmount.ts - * - * Exact comparison for fixed-point decimal amounts expressed as strings. - * - * Crypto amounts are user-entered decimals like `"12.5"`. `ethers.BigNumber` - * only accepts integer base units, so `BigNumber.from("12.5")` throws. That - * throw used to be swallowed by the surrounding `catch`, which meant *every* - * fractional amount failed the per-method spend cap and was reported as a - * generic payment error. - * - * Comparing via `parseFloat` instead would reintroduce float error, so the - * comparison below is done on normalised integer coefficient/exponent pairs. - */ - -/** Splits a decimal string into a sign, a digit string and a decimal scale. */ -interface NormalizedAmount { - negative: boolean; - /** Digits with the decimal point removed. */ - digits: string; - /** Number of implied decimal places in `digits`. */ - scale: number; -} - -const DECIMAL_PATTERN = /^[+-]?(\d+(\.\d*)?|\.\d+)$/; - -/** - * Parses a decimal string, or returns `null` when it is not a finite decimal. - * Rejects exponents (`1e18`), blanks, and anything non-numeric. - */ -export function normalizeDecimal( - value: string | number | null | undefined -): NormalizedAmount | null { - if (typeof value === 'number') { - if (!Number.isFinite(value)) return null; - value = value.toString(); - } - - if (typeof value !== 'string') return null; - - const trimmed = value.trim(); - if (trimmed === '' || !DECIMAL_PATTERN.test(trimmed)) return null; - - const negative = trimmed.startsWith('-'); - const unsigned = trimmed.replace(/^[+-]/, ''); - const dotIndex = unsigned.indexOf('.'); - - const whole = dotIndex === -1 ? unsigned : unsigned.slice(0, dotIndex); - const fraction = dotIndex === -1 ? '' : unsigned.slice(dotIndex + 1); - const rawDigits = `${whole}${fraction}`.replace(/^0+(?=\d)/, ''); - - if (rawDigits === '') return null; - - return { - negative: negative && !/^0+$/.test(rawDigits) ? true : false, - digits: rawDigits, - scale: fraction.length, - }; -} - -/** - * Returns -1, 0 or 1 as `a` is less than, equal to, or greater than `b`. - * - * @throws when either side is not a finite decimal string. - */ -export function compareDecimalAmounts( - a: string | number | null | undefined, - b: string | number | null | undefined -): number { - const left = normalizeDecimal(a); - const right = normalizeDecimal(b); - - if (!left || !right) { - throw new Error( - `Cannot compare non-decimal amounts: ${JSON.stringify(a)} vs ${JSON.stringify(b)}` - ); - } - - if (left.negative !== right.negative) { - return left.negative ? -1 : 1; - } - - // Align both sides to the same scale so a plain lexicographic compare works. - const scale = Math.max(left.scale, right.scale); - const leftAligned = left.digits.padEnd(left.digits.length + (scale - left.scale), '0'); - const rightAligned = right.digits.padEnd(right.digits.length + (scale - right.scale), '0'); - - const magnitude = compareDigits(leftAligned, rightAligned); - - return left.negative ? -magnitude : magnitude; -} - -function compareDigits(a: string, b: string): number { - if (a.length !== b.length) return a.length < b.length ? -1 : 1; - if (a === b) return 0; - return a < b ? -1 : 1; -} - -/** True when `amount` exceeds `limit`. Both must be finite decimals. */ -export function exceedsLimit( - amount: string | number | null | undefined, - limit: string | number | null | undefined -): boolean { - return compareDecimalAmounts(amount, limit) > 0; -} diff --git a/src/utils/invoiceTotals.ts b/src/utils/invoiceTotals.ts index 6b56b28c..6b22a92b 100644 --- a/src/utils/invoiceTotals.ts +++ b/src/utils/invoiceTotals.ts @@ -1,27 +1,14 @@ /** * invoiceTotals.ts * - * Single source of truth for turning line items into an invoice total. + * Line-item helpers shared by the invoice renderers. * - * `createInvoice` and `updateInvoice` previously each rolled their own - * arithmetic, and they did not agree: `createInvoice` subtracted the - * discount, `updateInvoice` did not. Editing one line on a discounted - * invoice therefore silently increased the amount payable, and because the - * discount was never persisted on the record there was nothing left to - * subtract on the second pass. + * The full totals module — half-up rounding, discount clamping and the single + * arithmetic path used by `createInvoice` / `updateInvoice` — is introduced + * with the billing-correctness work. Only the two helpers the rendering layer + * depends on are defined here for now. */ -/** Rounds to `decimals` places using half-up, avoiding binary float drift. */ -export function roundMoney(amount: number, decimals = 2): number { - if (!Number.isFinite(amount)) return 0; - - const factor = 10 ** decimals; - // The epsilon nudge keeps values such as 1.005 from rounding down because - // their binary representation is fractionally below the decimal value. - // The trailing `+ 0` normalises -0 to 0 so callers never see a negative zero. - return Math.round((amount + Number.EPSILON * Math.sign(amount)) * factor) / factor + 0; -} - export interface LineTotalSource { kind?: string; lineTotal?: number; @@ -40,7 +27,7 @@ export function isDiscountLineItem(item: { kind?: string }): boolean { /** * Resolves a single line's total, tolerating both the itemised (`lineTotal`) - * and flat (`amount`) shapes. + * and the flat (`amount`) shapes. */ export function resolveLineTotal(item: LineTotalSource): number { const derived = item.quantity !== undefined ? (item.unitPrice ?? 0) * item.quantity : undefined; @@ -54,65 +41,3 @@ export function resolveLineTotal(item: LineTotalSource): number { return 0; } - -export interface InvoiceTotalsInput { - lineItems?: LineTotalSource[]; - /** Used when the invoice has no line items. */ - amount?: number; - tax?: number; - discount?: number; - currencyDecimals?: number; -} - -export interface InvoiceTotals { - subtotal: number; - tax: number; - discount: number; - total: number; -} - -/** - * Computes invoice totals with every component rounded to minor units and a - * non-negative payable amount. - * - * Discount line items (those with `kind: 'discount'`) are kept out of the - * subtotal and summed into `discount` instead, so an itemised discount is - * subtracted exactly once. A legacy scalar `discount` is added to whatever the - * discount lines contribute. - * - * The clamp matters: a discount larger than the subtotal otherwise produces a - * negative tax base and a negative total, which downstream code then treats as - * a credit owed to the customer rather than an over-discounted invoice. - */ -export function calculateInvoiceTotals(input: InvoiceTotalsInput): InvoiceTotals { - const decimals = input.currencyDecimals ?? 2; - - const lineItems = input.lineItems ?? []; - const chargeItems = lineItems.filter((item) => !isDiscountLineItem(item)); - const discountItems = lineItems.filter((item) => isDiscountLineItem(item)); - - // A discount-only invoice has nothing to charge; fall back to the flat - // amount if the caller supplied one rather than reporting a zero subtotal. - const rawSubtotal = - chargeItems.length > 0 - ? chargeItems.reduce((sum, item) => sum + resolveLineTotal(item), 0) - : (input.amount ?? 0); - - const subtotal = roundMoney(rawSubtotal, decimals); - const tax = roundMoney(input.tax ?? 0, decimals); - - // Discount lines carry a positive magnitude (or a negative one); either way - // they reduce the amount payable. - const lineDiscount = roundMoney( - discountItems.reduce((sum, item) => sum + Math.abs(resolveLineTotal(item)), 0), - decimals - ); - - // A discount may not exceed what is being discounted. - const requestedDiscount = roundMoney((input.discount ?? 0) + lineDiscount, decimals); - const discount = roundMoney(Math.min(Math.max(requestedDiscount, 0), subtotal), decimals); - - const total = roundMoney(Math.max(subtotal + tax - discount, 0), decimals); - - return { subtotal, tax, discount, total }; -}