fix: exact batch totals, payout retry worker and webhook hardening (#1163-#1166)

WEBHOOK_VERIFICATION.md

@@ -61,4 +61,9 @@ and default to the constants above when omitted.
 7. Create secret rotation mechanism
 8. Add comprehensive verification logging
 9. Update webhook routes to use verification
-10. Create admin interface for webhook management
+10. Create admin interface for webhook management
+
+## Replay window and signature comparison
+
+- A timestamp that does not parse as a date is rejected with `Timestamp is not a valid date`. An invalid date yields `NaN` in the tolerance check, and `NaN > tolerance` is false, so it previously passed the replay window.
+- Digests are compared with `crypto.timingSafeEqual` over the decoded bytes. A non-hex or wrong-length signature fails closed without throwing.

backend/src/index.ts

@@ -74,6 +74,7 @@ import { zapierRouter } from './routes/zapier.js';
 import { intercomRouter } from './routes/intercom.js';
 import { allowancesRouter } from './routes/allowances.js';
 import { payoutsRouter } from './routes/payouts.js';
+import { startPayoutRetryWorker } from './services/payouts/retry-worker.js';
 import { getPrismaReplicaClient } from './db/PrismaReplicaClient.js';
 import { cohortAnalyticsRouter } from './routes/cohort-analytics.js';
 import { churnPredictionRouter } from './routes/churn-prediction.js';
@@ -508,6 +509,9 @@ server.listen(config.server.port, () => {
     // Load Sentry profiling integration now that the process is warm
     loadProfilingIntegration();
 
+    // Payout retry worker: drives backed-off payout retries without a manual call
+    startPayoutRetryWorker();
+
     // Job scheduler
     if (config.jobs.enabled) {
       startJobs();

backend/src/services/batch.ts

@@ -3,6 +3,7 @@ import * as StellarSdk from '@stellar/stellar-sdk';
 import { config } from '../config/env.js';
 import { featureFlags } from '../config/featureFlags.js';
 import { server, getNonceManager, getGasEstimator, UnitOfWorkError } from './stellar.js';
+import { CRYPTO_DECIMALS, sumAmounts } from './money.js';
 
 const NETWORK = config().STELLAR_NETWORK;
 const networkPassphrase =
@@ -151,17 +152,8 @@ export function getBatchReport(id: string): object | undefined {
   if (!record) return undefined;
 
   const successRate = record.total > 0 ? ((record.succeeded / record.total) * 100).toFixed(2) : '0.00';
-  const totalAmount = record.results
-    .filter((r) => r.status === 'success')
-    .reduce((sum, r) => sum + parseFloat(r.amount), 0)
-    .toFixed(7);
-
-  const byAsset = record.results
-    .filter((r) => r.status === 'success')
-    .reduce<Record<string, number>>((acc, r) => {
-      acc[r.asset] = (acc[r.asset] ?? 0) + parseFloat(r.amount);
-      return acc;
-    }, {});
+  const byAsset = sumByAsset(record.results.filter((r) => r.status === 'success'));
+  const totalAmount = singleAssetTotal(byAsset);
 
   return {
     batchId: record.id, label: record.label, status: record.status,
@@ -186,39 +178,53 @@ export function generateCSVTemplate(): string {
 
 export interface BatchEstimate {
   totalPayments: number;
-  totalAmount: string;
+  /** Null when the batch spans more than one asset: incomparable assets are never summed. */
+  totalAmount: string | null;
   byAsset: Record<string, string>;
   estimatedGasUnits: number;
   duplicateCount: number;
   invalidAddressCount: number;
   estimatedDurationMs: number;
 }
 
+/** Sums amounts per asset in exact decimal arithmetic. */
+function sumByAsset(items: readonly Pick<BatchPaymentItem, 'asset' | 'amount'>[]): Record<string, string> {
+  const grouped: Record<string, string[]> = {};
+  for (const item of items) {
+    (grouped[item.asset] ??= []).push(item.amount);
+  }
+  const totals: Record<string, string> = {};
+  for (const [asset, amounts] of Object.entries(grouped)) {
+    totals[asset] = sumAmounts(amounts, CRYPTO_DECIMALS);
+  }
+  return totals;
+}
+
+/** Returns the total only when there is exactly one asset to total. */
+function singleAssetTotal(byAsset: Record<string, string>): string | null {
+  const assets = Object.keys(byAsset);
+  return assets.length === 1 ? byAsset[assets[0]] : null;
+}
+
 export function estimateBatch(payments: BatchPaymentItem[]): BatchEstimate {
-  const byAsset: Record<string, number> = {};
-  let totalAmount = 0;
   let invalidCount = 0;
+  const valid: BatchPaymentItem[] = [];
 
   for (const p of payments) {
     if (!/^G[A-Z2-7]{55}$/.test(p.recipient)) {
       invalidCount++;
       continue;
     }
-    const amount = parseFloat(p.amount) || 0;
-    totalAmount += amount;
-    byAsset[p.asset] = (byAsset[p.asset] ?? 0) + amount;
+    valid.push(p);
   }
 
+  const byAsset = sumByAsset(valid);
   const duplicateIndices = detectDuplicates(payments);
-  const byAssetStrings: Record<string, string> = {};
-  for (const [k, v] of Object.entries(byAsset)) {
-    byAssetStrings[k] = v.toFixed(7);
-  }
 
   return {
     totalPayments: payments.length,
-    totalAmount: totalAmount.toFixed(7),
-    byAsset: byAssetStrings,
+    totalAmount: singleAssetTotal(byAsset),
+    byAsset,
     estimatedGasUnits: payments.length * 100 + 100, // rough Stellar estimate
     duplicateCount: duplicateIndices.length,
     invalidAddressCount: invalidCount,

backend/src/services/payouts/__tests__/retry-worker.test.ts

@@ -0,0 +1,65 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { startPayoutRetryWorker } from '../retry-worker.js';
+
+describe('startPayoutRetryWorker', () => {
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it('runs a retry pass on every interval', async () => {
+    vi.useFakeTimers();
+    const retryDueBatches = vi.fn(async () => [{ id: 'b1' }]);
+    const worker = startPayoutRetryWorker({ retryDueBatches } as never, 1_000);
+
+    await vi.advanceTimersByTimeAsync(3_000);
+
+    expect(retryDueBatches).toHaveBeenCalledTimes(3);
+    worker.stop();
+  });
+
+  it('stops scheduling passes once stopped', async () => {
+    vi.useFakeTimers();
+    const retryDueBatches = vi.fn(async () => []);
+    const worker = startPayoutRetryWorker({ retryDueBatches } as never, 1_000);
+
+    worker.stop();
+    await vi.advanceTimersByTimeAsync(5_000);
+
+    expect(retryDueBatches).not.toHaveBeenCalled();
+  });
+
+  it('skips an interval while the previous pass is still running', async () => {
+    vi.useFakeTimers();
+    let release!: () => void;
+    const retryDueBatches = vi.fn(
+      () => new Promise<never[]>((resolve) => {
+        release = () => resolve([]);
+      }),
+    );
+    const worker = startPayoutRetryWorker({ retryDueBatches } as never, 1_000);
+
+    await vi.advanceTimersByTimeAsync(3_000);
+    expect(retryDueBatches).toHaveBeenCalledTimes(1);
+
+    release();
+    await vi.advanceTimersByTimeAsync(1_000);
+    expect(retryDueBatches).toHaveBeenCalledTimes(2);
+    worker.stop();
+  });
+
+  it('reports a failed pass and keeps the worker alive for the next one', async () => {
+    const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined);
+    const retryDueBatches = vi
+      .fn<() => Promise<{ id: string }[]>>()
+      .mockRejectedValueOnce(new Error('db down'))
+      .mockResolvedValueOnce([{ id: 'b1' }, { id: 'b2' }]);
+    const worker = startPayoutRetryWorker({ retryDueBatches } as never, 60_000);
+
+    await expect(worker.tick()).resolves.toBe(0);
+    await expect(worker.tick()).resolves.toBe(2);
+
+    expect(errorSpy).toHaveBeenCalledTimes(1);
+    worker.stop();
+  });
+});

backend/src/services/payouts/retry-worker.ts

@@ -0,0 +1,49 @@
+import { payoutEngine, type PayoutEngine } from './payout-engine.js';
+
+/** How often failed payout batches are checked for a due retry. */
+export const DEFAULT_PAYOUT_RETRY_INTERVAL_MS = 60_000;
+
+export interface PayoutRetryWorker {
+  /** Runs one retry pass now. Resolves to the number of batches retried; never rejects. */
+  tick(): Promise<number>;
+  stop(): void;
+}
+
+/**
+ * Drives `retryDueBatches` on an interval. Without this, a failed batch's backoff
+ * timestamp is recorded but nothing acts on it until an operator calls the retries route.
+ *
+ * Passes never overlap: if a pass is still running when the next interval fires, that
+ * tick is skipped. A rejected pass is reported and retried on the next interval.
+ */
+export function startPayoutRetryWorker(
+  engine: Pick<PayoutEngine, 'retryDueBatches'> = payoutEngine,
+  intervalMs: number = DEFAULT_PAYOUT_RETRY_INTERVAL_MS,
+): PayoutRetryWorker {
+  let running = false;
+
+  const tick = async (): Promise<number> => {
+    if (running) return 0;
+    running = true;
+    try {
+      const retried = await engine.retryDueBatches();
+      return retried.length;
+    } catch (error) {
+      console.error('[payouts] Retry pass failed', error);
+      return 0;
+    } finally {
+      running = false;
+    }
+  };
+
+  const timer = setInterval(() => {
+    void tick();
+  }, intervalMs);
+  // Do not keep the process alive solely for payout retries.
+  timer.unref();
+
+  return {
+    tick,
+    stop: () => clearInterval(timer),
+  };
+}

backend/src/services/webhooks/__tests__/verification.test.ts

@@ -140,6 +140,47 @@ describe('Webhook Signature Verification', () => {
     });
   });
 
+  describe('Replay and signature hardening', () => {
+    it('rejects an unparseable timestamp instead of passing the tolerance window', () => {
+      createWebhookSecret(testProvider, testSecret);
+      const result = verifyWebhookSignature({
+        signature: generateWebhookSignature(testPayload, testSecret, 'not-a-date'),
+        timestamp: 'not-a-date',
+        body: testPayload,
+        provider: testProvider,
+      });
+
+      expect(result.isValid).toBe(false);
+      expect(result.error).toBe('Timestamp is not a valid date');
+    });
+
+    it('rejects a non-hex signature without throwing', () => {
+      createWebhookSecret(testProvider, testSecret);
+      const result = verifyWebhookSignature({
+        signature: 'zz'.repeat(32),
+        timestamp: testTimestamp,
+        body: testPayload,
+        provider: testProvider,
+      });
+
+      expect(result.isValid).toBe(false);
+    });
+
+    it('rejects a signature that differs from the expected digest in one byte', () => {
+      createWebhookSecret(testProvider, testSecret);
+      const good = generateWebhookSignature(testPayload, testSecret, testTimestamp);
+      const flipped = (good[0] === '0' ? '1' : '0') + good.slice(1);
+      const result = verifyWebhookSignature({
+        signature: flipped,
+        timestamp: testTimestamp,
+        body: testPayload,
+        provider: testProvider,
+      });
+
+      expect(result.isValid).toBe(false);
+    });
+  });
+
   describe('Webhook Secret Management', () => {
     it('should create and retrieve webhook secrets', () => {
       const secret = createWebhookSecret(testProvider, testSecret);

backend/src/services/webhooks/verification.ts

@@ -1,4 +1,4 @@
-import { createHmac } from 'node:crypto';
+import { createHmac, timingSafeEqual } from 'node:crypto';
 import { z } from 'zod';
 import { AppError } from '../../middleware/errorHandler.js';
 
@@ -92,6 +92,16 @@ export function verifyWebhookSignature(
   const toleranceMs = parsed.toleranceSeconds * 1000;
   const timeDiff = Math.abs(now.getTime() - timestamp.getTime());
 
+  // NaN fails every comparison, so an unparseable timestamp would otherwise pass the window.
+  if (Number.isNaN(timeDiff)) {
+    return {
+      isValid: false,
+      provider: parsed.provider,
+      timestamp,
+      error: 'Timestamp is not a valid date',
+    };
+  }
+
   if (timeDiff > toleranceMs) {
     return {
       isValid: false,
@@ -125,19 +135,13 @@ export function verifyWebhookSignature(
 }
 
 /**
- * Constant-time string comparison to prevent timing attacks
+ * Constant-time comparison of hex digests to prevent timing attacks.
+ * Compares the decoded bytes, so a non-hex or wrong-length signature fails closed.
  */
 function constantTimeEquals(a: string, b: string): boolean {
-  if (a.length !== b.length) {
-    return false;
-  }
-
-  let result = 0;
-  for (let i = 0; i < a.length; i++) {
-    result |= a.charCodeAt(i) ^ b.charCodeAt(i);
-  }
-
-  return result === 0;
+  const aBuf = Buffer.from(a, 'hex');
+  const bBuf = Buffer.from(b, 'hex');
+  return aBuf.length > 0 && aBuf.length === bBuf.length && timingSafeEqual(aBuf, bBuf);
 }
 
 /**

docs/PAYMENT_CORRECTNESS.md

@@ -106,6 +106,15 @@ scheduled job to reclaim expired entries.
 
 Only responses below `500` are cached: a `5xx` may be transient.
 
+## Batch totals are exact and never span assets
+
+`estimateBatch` and `getBatchReport` (`backend/src/services/batch.ts`) sum amounts with
+`sumAmounts` at seven decimals (Stellar precision), not with `parseFloat`. Binary floating
+point turned `0.1 + 0.2` into `0.30000000000000004` in reports shown to operators.
+
+Totals are per asset. A batch that pays both XLM and USDC reports `totalAmount: null` and
+lists each asset under `byAsset`, because XLM and USDC are not comparable.
+
 ## Unknown states are programming errors, not branches
 
 `PayoutBatch.status` is only ever changed through the engine's transition table. A