diff --git a/test/e2e-live/helpers/docker-helpers.ts b/test/e2e-live/helpers/docker-helpers.ts index 8e3c841..6a6f58d 100644 --- a/test/e2e-live/helpers/docker-helpers.ts +++ b/test/e2e-live/helpers/docker-helpers.ts @@ -29,6 +29,7 @@ import type { StopContainer, WaitForContainerRunning, } from './contracts.js'; +import { sessionContainerPrefix } from './session.js'; // ---------------------------------------------------------------------------- // Errors @@ -170,8 +171,13 @@ export function buildRunArgs(opts: DockerRunOptions, name: string): string[] { function buildName(label: string | undefined): string { // Docker container names: [a-zA-Z0-9][a-zA-Z0-9_.-]+ // Label has already been validated against LABEL_RE if provided. + // + // Stem includes the per-process SESSION_ID so two concurrent invocations + // of `npm run test:e2e-live` produce disjoint name spaces. See + // session.ts for the rationale. const suffix = Math.random().toString(36).slice(2, 8); - const stem = label ? `trader-e2e-${label}` : 'trader-e2e'; + const sessionPrefix = sessionContainerPrefix(); + const stem = label ? `${sessionPrefix}-${label}` : sessionPrefix; return `${stem}-${suffix}`; } @@ -375,9 +381,15 @@ export async function listContainersByNamePrefix( ); } try { + // Docker's `name=` filter is a SUBSTRING match by default — `name=foo` + // matches any container whose name CONTAINS "foo", not just those that + // START with "foo". For our session-isolation guarantee (where another + // test run's session ID could in theory share leading hex digits with + // ours), we anchor the regex with `^` to force prefix semantics. + // Docker passes the value through to its regexp matcher, so `^` is honored. const { stdout } = await execFileImpl('docker', [ 'ps', - '--filter', `name=${namePrefix}`, + '--filter', `name=^${namePrefix}`, '--format', '{{.ID}}', ]); return stdout.split('\n').map((s) => s.trim()).filter((s) => s.length > 0); diff --git a/test/e2e-live/helpers/scenario-helpers.ts b/test/e2e-live/helpers/scenario-helpers.ts index 8dc752d..9ea03fa 100644 --- a/test/e2e-live/helpers/scenario-helpers.ts +++ b/test/e2e-live/helpers/scenario-helpers.ts @@ -23,6 +23,7 @@ import { pollUntil } from './polling.js'; import { SWAP_TIMEOUT_MS } from './constants.js'; import { getControllerWallet } from './tenant-fixture.js'; import { getContainerLogs, listContainersByNamePrefix } from './docker-helpers.js'; +import { sessionContainerPrefix } from './session.js'; const DEAL_POLL_INTERVAL_MS = 2_000; const CREATE_INTENT_TIMEOUT_MS = 30_000; @@ -234,24 +235,32 @@ export async function waitForDealInState( /* container may already be gone */ } - // Also dump every running escrow container's recent logs. The trader-side - // logs alone don't tell us whether the escrow received the deposit, paid - // out, or got stuck — without these, parallel-swap and unreachable-escrow - // failures are nearly impossible to diagnose post-mortem. + // Also dump every running container from THIS session's tenant pool — + // includes escrows + any sibling traders. The trader-side logs alone + // don't tell us whether the escrow received the deposit, paid out, or + // got stuck. Filter by `sessionContainerPrefix()` so a parallel + // session running on the same Docker daemon doesn't leak its logs into + // our diagnostic output (and vice versa). + // + // NOTE: docker-helpers names BOTH trader and escrow containers with + // the `trader-e2e--` prefix, so this single filter captures + // escrows. The previous `escrow-e2e` filter never matched anything. try { - const escrowContainerIds = await listContainersByNamePrefix('escrow-e2e'); - for (const id of escrowContainerIds) { + const sessionContainerIds = await listContainersByNamePrefix(sessionContainerPrefix()); + for (const id of sessionContainerIds) { + // Skip the tenant whose logs we already dumped above. + if (id === tenant.container.id) continue; try { const logs = await getContainerLogs(id, 1500); console.error( - `[waitForDealInState] last 1500 log lines from escrow container ${id}:\n${logs}`, + `[waitForDealInState] last 1500 log lines from session container ${id}:\n${logs}`, ); } catch { /* container may have just exited — skip */ } } } catch { - /* listing failed (docker error) — skip escrow logs */ + /* listing failed (docker error) — skip sibling logs */ } throw new Error( diff --git a/test/e2e-live/helpers/session.ts b/test/e2e-live/helpers/session.ts new file mode 100644 index 0000000..ca4d467 --- /dev/null +++ b/test/e2e-live/helpers/session.ts @@ -0,0 +1,64 @@ +/** + * session — process-wide identifier for a single `npm run test:e2e-live` run. + * + * Two independent invocations of the e2e-live suite (concurrent CI shards, + * two developers on the same host, etc.) MUST NOT interfere. The shared + * resources at risk are: + * + * - Docker container names (one daemon, flat name space) + * - /tmp directory layout (test-only, but `ls /tmp/trader-e2e-*` should + * scope cleanly to a single run) + * - Diagnostic queries that filter by name prefix + * (see scenario-helpers `listContainersByNamePrefix`) + * + * The Nostr-side identifiers (per-trader secp256k1 keypair, nametag derived + * from the instance UUID) already have ≥10⁹ entropy per resource so they + * do not need a session prefix to remain non-colliding — adding one would + * only reduce the 9-hex randomness in the nametag slice. + * + * SESSION_ID is generated ONCE at module load. Every helper that constructs + * a name targeting a shared resource must read it from here so it is + * uniformly attached. + * + * Override via `TRADER_E2E_SESSION_ID=...` if a CI driver wants to tag the + * run with its own job id for cross-tool log correlation. The override is + * sanitized to lowercase hex; non-hex characters are stripped. Empty values + * are ignored — a fresh ID is generated. + */ + +import { randomBytes } from 'node:crypto'; + +function readSessionOverride(): string | null { + const raw = process.env['TRADER_E2E_SESSION_ID']; + if (typeof raw !== 'string') return null; + // Sanitize: lowercase, [a-z0-9-] (hex + dash), max 32 chars. We accept `-` + // because Docker container names allow it and CI drivers commonly tag with + // values like "ci-job-1234-abc" — silently stripping the dashes would mangle + // a meaningful identifier into an indecipherable hex blob (review feedback + // PR-10 W6). Stripping characters that ARE invalid for Docker names (e.g. + // `/`, `:`, spaces) is still required to prevent injection through the + // value into the docker argv. + const cleaned = raw.toLowerCase().replace(/[^0-9a-z-]/g, '').slice(0, 32); + return cleaned.length > 0 ? cleaned : null; +} + +/** + * 16-hex-char session id (64 bits ≈ 1.8e19 distinct sessions). + * + * Increased from 32 bits per PR-10 review (W4): with 100-shard CI matrices the + * birthday-bound collision probability at 32 bits (~1.2e-6) was small but + * non-zero. 64 bits drops it to ~5e-15 even at thousands of concurrent runs — + * effectively impossible for any realistic deployment. The cost is 4 additional + * random bytes; trivial. + */ +export const SESSION_ID: string = readSessionOverride() ?? randomBytes(8).toString('hex'); + +/** + * Common prefix for every container/tmp-dir name produced by this run. + * Two simultaneous test sessions will end up with distinct prefixes and + * `docker ps --filter name=` lists only the + * current run's containers. + */ +export function sessionContainerPrefix(): string { + return `trader-e2e-${SESSION_ID}`; +} diff --git a/test/e2e-live/helpers/tenant-fixture.ts b/test/e2e-live/helpers/tenant-fixture.ts index a9631b4..bf99502 100644 --- a/test/e2e-live/helpers/tenant-fixture.ts +++ b/test/e2e-live/helpers/tenant-fixture.ts @@ -23,12 +23,22 @@ import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { randomUUID } from 'node:crypto'; +import { randomBytes, randomUUID } from 'node:crypto'; import { - generatePrivateKey, getPublicKey, Sphere, } from '@unicitylabs/sphere-sdk'; + +/** + * A secp256k1 private key is 32 random bytes (any value < curve order; the + * probability of generating an invalid one is ~2^-128, vanishingly small). + * Inlined so the test fixture doesn't depend on sphere-sdk's internal L1 + * helper, which has been moved into the L1 sub-namespace and is no longer + * exported at the package root. + */ +function generatePrivateKey(): string { + return randomBytes(32).toString('hex'); +} import { createNodeProviders } from '@unicitylabs/sphere-sdk/impl/nodejs'; import type { @@ -47,6 +57,7 @@ import { runTraderCtl } from './trader-ctl-driver.js'; import { RELAYS, TRADER_IMAGE, ESCROW_IMAGE } from './constants.js'; import { pollUntil } from './polling.js'; import { fundWallet } from './funding.js'; +import { SESSION_ID } from './session.js'; // --------------------------------------------------------------------------- // Local extensions to ProvisionTraderOptions @@ -141,7 +152,9 @@ let controllerCache: Promise | null = null; export async function getControllerWallet(): Promise { if (controllerCache !== null) return controllerCache; controllerCache = (async () => { - const root = mkdtempSync(join(tmpdir(), 'trader-e2e-controller-')); + // Per-session prefix so two concurrent test runs don't share a cache dir + // and so `ls /tmp/trader-e2e-controller--*` scopes cleanly. + const root = mkdtempSync(join(tmpdir(), `trader-e2e-controller-${SESSION_ID}-`)); const dataDir = join(root, 'wallet'); const tokensDir = join(root, 'tokens'); mkdirSync(dataDir, { recursive: true }); @@ -228,7 +241,8 @@ function realSecp256k1Pubkey(): string { */ function materializeWalletDir(label: string): string { const safeLabel = label.replace(/[^a-zA-Z0-9-_]/g, '-').slice(0, 24); - const root = mkdtempSync(join(tmpdir(), `trader-e2e-${safeLabel}-`)); + // Per-session prefix isolates two concurrent runs' wallet dirs in /tmp. + const root = mkdtempSync(join(tmpdir(), `trader-e2e-${SESSION_ID}-${safeLabel}-`)); const walletDir = join(root, 'wallet'); const tokensDir = join(root, 'tokens'); mkdirSync(walletDir, { recursive: true }); @@ -694,35 +708,118 @@ export async function provisionTrader( } // ============================================================================ -// provisionTradersStaggered — concurrent provisioning with kickoff stagger +// provisionTradersStaggered — bounded-parallel provisioning // ============================================================================ +const DEFAULT_PROVISION_CONCURRENCY = 3; + +function readProvisionConcurrency(): number { + const raw = process.env['TRADER_E2E_PROVISION_CONCURRENCY']; + if (raw === undefined || raw === '') return DEFAULT_PROVISION_CONCURRENCY; + const n = Number.parseInt(raw, 10); + // Per PR-10 review W5: silently floor invalid values to the default mask + // typos. A user setting `TRADER_E2E_PROVISION_CONCURRENCY=0` (intent: + // "force sequential, cc=1") got cc=3 instead — surprising. Reject loudly + // for invalid forms so the operator notices the typo. We DO honor cc=1 + // explicitly as "sequential" and cc=0 is now an explicit error. + if (!Number.isFinite(n) || Number.isNaN(n)) { + throw new Error( + `Invalid TRADER_E2E_PROVISION_CONCURRENCY="${raw}": must be a positive integer.`, + ); + } + if (n < 1) { + throw new Error( + `Invalid TRADER_E2E_PROVISION_CONCURRENCY="${raw}" (parsed as ${n}): must be >= 1. ` + + `Use TRADER_E2E_PROVISION_CONCURRENCY=1 to force sequential provisioning.`, + ); + } + return n; +} + /** - * Provisions multiple traders sequentially. + * Provision multiple traders with bounded parallelism. * - * **Why sequential, not parallel:** the testnet's single Nostr relay is - * the bottleneck. When N traders run `Sphere.init` concurrently, all of - * them publish nametag binding events and then each does a self-verify - * `sphere.resolve()` query. The relay's queryEvents subscription queue - * serializes — under N>=3 concurrent load, queries time out at 15s and - * each trader's verify loop runs out of budget. We've seen all 3 traders - * hang at sphere_initialized for 180s under pure Promise.all. + * **Why bounded, not unbounded Promise.all:** the testnet's single Nostr + * relay is the bottleneck. Under unbounded concurrency we have observed + * (N≥4) one trader hanging at `sphere_initialized` for 3 minutes while its + * `sphere.resolve()` self-verify times out. Bounded parallelism with a + * small cap keeps the simultaneous nametag-publish + self-verify load + * within the relay's serving budget. * - * Sequential gives each trader's binding event time to propagate to the - * relay's query index before the next trader starts hitting it. Cost is - * ~30-60s per trader (dominated by sphere.init + verify); total wall - * time scales linearly with N. + * Concurrency is configurable via `TRADER_E2E_PROVISION_CONCURRENCY` (env + * var, default 3). Set to 1 to fall back to fully sequential behavior on + * a degraded relay; set higher when the relay is healthy and you want to + * cut wall-clock provisioning time. The 2026-04-30 measurement in + * `provisioning-load-investigation.e2e-live.test.ts` validates 3-way + * parallel provisioning is reliable on a healthy relay. * - * If the testnet relay's subscription throughput improves, this can be - * revisited as parallel-with-stagger. + * **Result ordering** matches the input order regardless of completion + * order — callers commonly destructure `[alice, bob, carol] = await ...` + * and rely on positional alignment with the factory list. */ export async function provisionTradersStaggered( factories: Array<() => Promise>, ): Promise { - const results: ProvisionedTenant[] = []; - for (const factory of factories) { - if (factory === undefined) continue; - results.push(await factory()); + const tasks = factories.filter((f): f is () => Promise => f !== undefined); + const concurrency = Math.min(readProvisionConcurrency(), Math.max(tasks.length, 1)); + const results: ProvisionedTenant[] = new Array(tasks.length); + const errors: unknown[] = []; + + let nextIndex = 0; + // Worker pool: each worker pulls the next task index and runs it. When the + // index pointer crosses the task list, the worker exits. With `concurrency` + // workers we cap simultaneous in-flight provisions. + // + // CRITICAL: workers MUST NOT abort on first error — sequential provisioning's + // failure semantics are that earlier-completed tenants stay alive for the + // caller's afterAll to dispose. With Promise.all-style fail-fast, in-flight + // workers continue spawning containers AFTER the function rejects, and those + // containers never reach `results` so afterAll never sees them. The leak is + // strictly worse than sequential. We catch each task's error, record it, and + // let every started worker drain to completion. Once all workers settle, we + // dispose any tenants that did succeed (caller's afterAll won't see them + // through the rejected promise either) and re-throw the FIRST recorded error. + async function worker(): Promise { + for (;;) { + const i = nextIndex++; + if (i >= tasks.length) return; + try { + // Non-null assertion is safe: i < tasks.length guarantees defined. + results[i] = await tasks[i]!(); + } catch (err) { + errors.push(err); + // Continue — let other workers drain so we can clean up any + // late-arriving tenants below. + } + } + } + + const workers: Array> = []; + for (let i = 0; i < concurrency; i++) { + workers.push(worker()); + } + await Promise.all(workers); + + if (errors.length > 0) { + // Dispose every tenant that DID succeed before re-throwing — they are + // unreachable to the caller through the rejected promise. + for (let i = 0; i < results.length; i++) { + const tenant = results[i]; + if (tenant !== undefined) { + try { + await tenant.dispose(); + } catch { + // Best-effort cleanup; don't mask the original error. + } + } + } + // Re-throw the first error. If multiple workers failed, the rest are + // captured via `cause` so they aren't silently swallowed. + const primary = errors[0]; + if (errors.length > 1 && primary instanceof Error) { + (primary as Error & { otherErrors?: unknown[] }).otherErrors = errors.slice(1); + } + throw primary; } return results; } diff --git a/vitest.e2e-live.config.ts b/vitest.e2e-live.config.ts index bfcae6c..7ddd649 100644 --- a/vitest.e2e-live.config.ts +++ b/vitest.e2e-live.config.ts @@ -18,13 +18,62 @@ */ import { defineConfig } from 'vitest/config'; +/** + * VITEST_MAX_FORKS — opt-in knob for parallelizing e2e-live test FILES + * across vitest worker forks. Default is 1 (sequential, one file at a + * time) which preserves the historical behavior of `singleFork: true`. + * + * Two forks would run two test files concurrently. Each fork spawns its + * own set of Docker containers (the test/e2e-live helpers session-prefix + * everything) so cross-fork resource collisions are designed-out, but the + * shared testnet Nostr relay is the bottleneck. Don't raise this above + * what the relay can handle (empirically 2-3 today). + * + * Note: each fork spawns its own controller wallet via `Sphere.init`, + * which performs a relay handshake. With N forks you do N concurrent + * Sphere.init calls against the same relay during startup. If the relay + * sustains 2-3 concurrent inits today, plan accordingly. + * + * The session-isolation work in test/e2e-live/helpers/session.ts is what + * makes cross-fork (and cross-process) parallelism safe — see that file + * for the per-resource analysis. + * + * Per PR-10 review W5: validate explicitly rather than silently floor on + * NaN/0/negative — typos used to be hidden by `|| 1`, now they fail loudly. + */ +function readMaxForks(): number { + const raw = process.env['VITEST_MAX_FORKS']; + if (raw === undefined || raw === '') return 1; + const n = Number.parseInt(raw, 10); + if (!Number.isFinite(n) || Number.isNaN(n)) { + throw new Error( + `Invalid VITEST_MAX_FORKS="${raw}": must be a positive integer.`, + ); + } + if (n < 1) { + throw new Error( + `Invalid VITEST_MAX_FORKS="${raw}" (parsed as ${n}): must be >= 1.`, + ); + } + return n; +} +const MAX_FORKS = readMaxForks(); + export default defineConfig({ test: { include: ['test/e2e-live/**/*.test.ts'], testTimeout: 180_000, hookTimeout: 300_000, pool: 'forks', - poolOptions: { forks: { singleFork: true } }, + poolOptions: { + forks: { + // singleFork preserved as default for backward compatibility. + // Setting VITEST_MAX_FORKS>1 flips to a multi-fork pool. + singleFork: MAX_FORKS === 1, + maxForks: MAX_FORKS, + minForks: 1, + }, + }, sequence: { concurrent: false }, retry: 0, bail: 1,