From 38b152136b20e00698e59d995df8f737b9ec5f98 Mon Sep 17 00:00:00 2001 From: Sylvain Duchesne Date: Thu, 9 Jul 2026 13:47:07 +0200 Subject: [PATCH] =?UTF-8?q?refactor(client):=20retirer=20anti-fork=20?= =?UTF-8?q?=E2=80=94=20gap=20non=20exhib=C3=A9,=20resolveAccount=20simple?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preuve e2e (wallet frais, broker RÉEL rapide : 1er State 1-2 ms) : le private store est synchronisé au login, la lecture du shim réussit à froid — le « fork sur lag » que anti-fork compensait n'est PAS exhibé. Par le principe du polyfill (compenser un gap RÉEL, jamais du poids mort), et par la règle no-polling : - la version retry = polling (bannie) ; - la version barrière `ensureRepoOpen(privateStore)` = CASSÉE (un store n'émet pas de `State`, la barrière timeout systématiquement → CONTRAT 2 e2e échouait) ; - le gap = non exhibé. → `ensureAccount` fait un `resolveAccount(id)` SIMPLE (une lecture, provision si 0). `resolveAccountReliably`, `_forceOpenedSyncState` retirés ; `provisionRetry` gardé optionnel @deprecated (ignoré) pour ne pas casser les 8 tests qui le passent. `ensureRepoOpen`/`getSyncState` inchangés (chemin de lecture open-repo). gate : tsc 0 ; bun test 116 ; test:e2e 39 passed, CONTRAT 2 VERT (« same account, no second provisioning »). Le ~10s du re-resolve public est du scaling anchorless, pas de la lenteur broker (broker mesuré à 1-2 ms). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/client/src/open-repo.ts | 11 ---- packages/client/src/polyfill.ts | 27 +++----- packages/client/src/store-registry.ts | 85 +------------------------- packages/client/test/anti-fork.test.ts | 55 +++++------------ 4 files changed, 25 insertions(+), 153 deletions(-) diff --git a/packages/client/src/open-repo.ts b/packages/client/src/open-repo.ts index 4f200dd..f2dd85c 100644 --- a/packages/client/src/open-repo.ts +++ b/packages/client/src/open-repo.ts @@ -110,17 +110,6 @@ export function resetOpenedRepos(): void { boundSessionId = null; } -/** - * @internal TEST-ONLY — force a nuri into the opened registry with a given sync - * state, without going through `doc_subscribe`. Used by `anti-fork.test.ts` to - * simulate a timed-out barrier without waiting the full `OPEN_TIMEOUT_MS` (8s). - * Do NOT call from production code. - */ -export function _forceOpenedSyncState(nuri: Nuri, state: SyncState): void { - opened.add(nuri); - syncState.set(nuri, state); -} - /** * The bootstrap sync state of `nuri` (lib-internal accessor; NOT a reactive * app-facing hook — that is a later phase). Returns `"unknown"` if the repo was diff --git a/packages/client/src/polyfill.ts b/packages/client/src/polyfill.ts index b6bc90a..873299c 100644 --- a/packages/client/src/polyfill.ts +++ b/packages/client/src/polyfill.ts @@ -25,17 +25,9 @@ export interface StoreRegistryDeps { /** Normalize an identity id for shim keying. Default: trim (identity-ish). */ normalizeId?: (id: string) => string; /** - * ANTI-FORK budget for account resolution (polyfill-era). Before provisioning - * a "missing" account, the registry re-reads its shim record a bounded number - * of times with backoff, so a record merely lagging by broker sync (fresh - * session over a persistent wallet) is FOUND and REUSED instead of triggering - * a second set of scope docs (the account fork). See the note in - * store-registry.ts. Only if every attempt still reads 0 do we provision. - * - * Default (production): a real budget (~8 attempts / ≲8.5s). Set `attempts: 1` - * to disable the retry entirely — appropriate for a SYNCHRONOUS in-memory - * store (the unit fake `ng`) where a 0-row read is authoritative and the - * backoff would only add dead time; there is no sync lag to wait out there. + * @deprecated Removed. The anti-fork retry/barrier mechanism has been dropped + * (the private store is synchronised at login; the barrier was both unnecessary + * and broken for STORE NURIs). This field is accepted but silently ignored. */ provisionRetry?: { attempts?: number; baseMs?: number; maxStepMs?: number }; } @@ -64,7 +56,9 @@ export interface EventuallyConfig { let cfg: EventuallyConfig | null = null; let currentUser: PrincipalId | null = null; -let registryDeps: Required | null = null; +/** Required fields of StoreRegistryDeps after defaults are applied (excludes deprecated provisionRetry). */ +type ResolvedRegistryDeps = Required>; +let registryDeps: ResolvedRegistryDeps | null = null; /** The emulated ReadCap/WriteCap registry. Empty until the app declares caps; * while it has no read policy the read filter passes through (no regression). */ let caps = new CapRegistry(); @@ -98,18 +92,11 @@ export function configureStoreRegistry(deps: StoreRegistryDeps): void { registryDeps = { getSession: deps.getSession, normalizeId: deps.normalizeId ?? ((id: string) => id.trim()), - // Production default: a real anti-fork budget. Consumers over a synchronous - // store pass `{ attempts: 1 }` to opt out (see StoreRegistryDeps). - provisionRetry: { - attempts: deps.provisionRetry?.attempts ?? 8, - baseMs: deps.provisionRetry?.baseMs ?? 300, - maxStepMs: deps.provisionRetry?.maxStepMs ?? 1500, - }, }; } /** @internal — used by the storeRegistry to reach its injected dependencies. */ -export function getStoreRegistryDeps(): Required { +export function getStoreRegistryDeps(): ResolvedRegistryDeps { if (!registryDeps) { throw new Error("[ng-eventually] configureStoreRegistry() must be called before use"); } diff --git a/packages/client/src/store-registry.ts b/packages/client/src/store-registry.ts index 7cc772e..40b8286 100644 --- a/packages/client/src/store-registry.ts +++ b/packages/client/src/store-registry.ts @@ -35,31 +35,6 @@ import { docCreate, sparqlUpdate, sparqlQuery } from "./docs"; import { getStoreRegistryDeps } from "./polyfill"; import { ensureRepoOpen } from "./open-repo"; - -// --- provisioning anti-fork guard: barrier-first (polyfill-era) ------------ -// -// The shim (account→docs trust root) lives in the shared wallet's PRIVATE store. -// On a fresh session over a persistent wallet, that repo may not have synced yet -// from the broker: a targeted account resolve would then read 0 rows even though -// the account WAS persisted in an earlier session. If ensureAccount took that 0 -// at face value it would RE-PROVISION a second set of scope documents — an account -// FORK: one session writes/reads one set, another (or the same after a cache drop) -// the other, EMPTY set → the user "loses" their data on reconnect. -// -// Guard (barrier-first): BEFORE reading the shim, open/subscribe the private-store -// repo and AWAIT its first `State` push — the deterministic sync barrier (CONTRACT 3 -// in e2e/). After the barrier, presence is GUARANTEED and absence DEFINITIVE. Then -// read the shim ONCE: 0 rows = account genuinely absent → provision exactly once; -// rows present = account found → reuse it (NO-FORK). No retry loop, no poll. -// -// Timed-out barrier (conservative): if `ensureRepoOpen` exits via the 8-second -// fallback timeout (getSyncState → "timed-out") the sync is UNCONFIRMED — we do NOT -// provision blindly (that would re-fork). Instead we surface a clear error so the -// caller can retry the full flow rather than silently creating a duplicate account. -// The timeout is rare (pathological broker) and a hard error there is far safer than -// a silent fork. Disappears at the real multi-store migration (per-user store NURIs -// make this moot — barrier becomes the native store-open which is always confirmed). -import { getSyncState } from "./open-repo"; import { escapeLiteral, escapeIri, assertNuri } from "./sparql"; import type { Nuri, Scope } from "./types"; @@ -282,54 +257,6 @@ export async function resolveAccount(id: string): Promise } } -/** - * Resolve ONE account after waiting for the private-store sync barrier, so a - * 0-row result is DEFINITIVE (account genuinely absent) rather than ambiguous - * (broker lag). This is the ANTI-FORK guard for the polyfill-era shim. - * - * Flow: - * 1. Cache hit → return immediately (session-idempotent, no I/O). - * 2. Open/subscribe the private-store repo (`did:ng:${privateStoreId}`) and - * await the first `State` push — the deterministic sync barrier (CONTRACT 3). - * After the barrier, presence is guaranteed and absence definitive. - * 3. If the barrier TIMED-OUT (getSyncState → "timed-out"), the sync is - * unconfirmed — provisioning here would risk a fork. Throw a clear error - * instead so the caller can retry the full login flow. This is the - * conservative-safe choice: a hard error is recoverable; a silent fork is not. - * 4. Read the shim ONCE. 0 rows = genuinely absent → caller provisions exactly - * once. Rows present → reuse (NO-FORK preserved). - * - * With the unit fake `ng` (no `doc_subscribe`), `ensureRepoOpen` is a no-op - * (getSyncState → "unknown") and the single read is immediate — synchronous - * behaviour preserved, no lag to wait out. - */ -async function resolveAccountReliably(id: string): Promise { - // Cache hit → session-idempotent, no query, no fork risk. - const cached = accountCache.get(accountKey(id)); - if (cached) return cached; - - // The shim lives in the private store. Open/subscribe it and await the first - // `State` push (the sync barrier). NURI: `did:ng:${session.privateStoreId}`. - const nuri = await anchorNuri(); - await ensureRepoOpen(nuri); - - // Conservative timed-out guard: if the barrier did not confirm sync, - // provisioning blind risks creating a fork. Raise a clear error — the caller - // (or the app's retry-login flow) should re-attempt when the broker is reachable. - // "unknown" means the fake-ng no-op path: no barrier semantics → safe to read. - const syncResult = getSyncState(nuri); - if (syncResult === "timed-out") { - throw new Error( - "[storeRegistry] sync barrier timed out for the private store — " + - "shim state unconfirmed, refusing to provision to avoid account fork. " + - "Retry when the broker is reachable.", - ); - } - - // Barrier reached (or fake-ng "unknown" path): single read, result is definitive. - return resolveAccount(id); -} - /** All known accounts (from the shim). */ export async function allAccounts(): Promise { return [...(await loadShim()).values()]; @@ -351,15 +278,9 @@ export async function ensureAccount(id: string): Promise { const key = accountKey(id); // HOT PATH: targeted O(1) lookup — does THIS account already exist? — instead // of a full-shim scan (loadShim). Off the read/write hot path entirely. - // - // ANTI-FORK (polyfill-era): open the private-store repo and await its first - // `State` push (the sync barrier) BEFORE reading the shim, so a 0-row result - // is DEFINITIVE rather than a lag artifact. Only after the barrier do we treat - // 0 rows as "genuinely new" and provision. A cache hit inside - // resolveAccountReliably keeps same-session resolves free and deterministic - // (the cached record wins over any re-provision). Throws if the barrier timed - // out — safer than provisioning blind and creating a fork. - const existing = await resolveAccountReliably(id); + // The private store is synchronised at login, so a simple resolveAccount read + // is authoritative: 0 rows = genuinely new → provision; rows present → reuse. + const existing = await resolveAccount(id); if (existing) return existing; const [docPublic, docProtected, docPrivate] = await Promise.all([ diff --git a/packages/client/test/anti-fork.test.ts b/packages/client/test/anti-fork.test.ts index 2944ef4..f3dcaa4 100644 --- a/packages/client/test/anti-fork.test.ts +++ b/packages/client/test/anti-fork.test.ts @@ -1,18 +1,16 @@ /** - * anti-fork.test.ts — behavioral tests for the ANTI-FORK guard in ensureAccount / - * resolveAccountReliably (src/store-registry.ts). + * ensureAccount-idempotent.test.ts — behavioral tests for the NO-FORK guarantee + * of ensureAccount (src/store-registry.ts). * - * The guard is: BEFORE reading the shim, open the private-store repo and await - * the first `State` push (the deterministic sync barrier, CONTRACT 3). After the - * barrier, 0 rows is DEFINITIVE (account genuinely absent) → provision exactly - * once; rows present → reuse (NO-FORK). No retry loop, no polling. + * The guarantee: ensureAccount always resolves via a SIMPLE resolveAccount read + * (one targeted SPARQL query, no barrier, no retry). The private store is + * synchronised at login, so 0 rows is DEFINITIVE (account genuinely absent) → + * provision exactly once; rows present → reuse (NO-FORK). Idempotent within + * and across sessions. * * With the unit fake `ng` (no `doc_subscribe`), `ensureRepoOpen` is a no-op - * (getSyncState → "unknown") and the single shim read is immediate — no lag to - * wait out, synchronous behaviour preserved. - * - * Timed-out barrier: if the barrier expires without a `State`, we refuse to - * provision (throwing a clear error) rather than risk a fork. + * (getSyncState → "unknown") and the single shim read is immediate — + * synchronous behaviour preserved, no lag to wait out. */ import { describe, it, expect, mock, beforeEach, afterAll } from "bun:test"; @@ -27,7 +25,7 @@ import { resetStoreRegistry, resetConfig, } from "../src/polyfill"; -import { resetOpenedRepos, _forceOpenedSyncState } from "../src/open-repo"; +import { resetOpenedRepos } from "../src/open-repo"; afterAll(() => { resetConfig(); @@ -42,8 +40,7 @@ const SESSION: RegistrySession = { sessionId: "sid-af", privateStoreId: "PRIV-AF // Fake ng — simple in-memory store (no doc_subscribe → fake-ng no-op path) // // Queries return data from the `quads` array immediately. No lag simulation: -// the barrier mechanism (ensureRepoOpen) is a no-op in the fake-ng path, so -// the single shim read after "the barrier" is already authoritative. +// the single shim read is already authoritative (private store is synced at login). // --------------------------------------------------------------------------- interface Quad { g: string; s: string; p: string; o: string } @@ -150,8 +147,6 @@ function inject(fakeNg: ReturnType) { configureStoreRegistry({ getSession: async () => SESSION, normalizeId: (u) => u.trim().toLowerCase(), - // provisionRetry is now unused by the barrier mechanism; kept for API compat. - provisionRetry: { attempts: 1 }, }); resetRegistryCache(); resetOpenedRepos(); @@ -161,7 +156,7 @@ function inject(fakeNg: ReturnType) { // Tests // --------------------------------------------------------------------------- -describe("anti-fork: barrier-first resolveAccountReliably / ensureAccount", () => { +describe("ensureAccount: no-fork + idempotence", () => { beforeEach(() => { resetRegistryCache(); resetOpenedRepos(); @@ -175,7 +170,7 @@ describe("anti-fork: barrier-first resolveAccountReliably / ensureAccount", () = expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); // Simulate a fresh session: clear caches but keep quads intact (same fakeNg). - // In the barrier model the single post-barrier read finds the account immediately. + // The single resolveAccount read finds the account immediately. resetRegistryCache(); resetOpenedRepos(); @@ -215,9 +210,9 @@ describe("anti-fork: barrier-first resolveAccountReliably / ensureAccount", () = expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); }); - it("(d) fake-ng no-op barrier path: single shim read, no doc_subscribe calls", async () => { + it("(d) fake-ng no-op path: single shim read per call, no doc_subscribe calls", async () => { // The fake ng has no doc_subscribe → ensureRepoOpen is a no-op (getSyncState → "unknown"). - // resolveAccountReliably must proceed to the single read without waiting or throwing. + // ensureAccount must proceed to the single resolveAccount read without waiting or throwing. const fakeNg = makeFakeNg(); // no doc_subscribe inject(fakeNg); @@ -234,24 +229,4 @@ describe("anti-fork: barrier-first resolveAccountReliably / ensureAccount", () = // The account query was called exactly twice (once per ensureAccount, no retries) expect(fakeNg.getAccountQueryCount()).toBe(2); }); - - it("(e) timed-out barrier: resolveAccountReliably throws rather than provisioning", async () => { - // Simulate: the private store nuri is already in the `opened` set but with - // sync state "timed-out" (forced via _forceOpenedSyncState so the test - // does not have to wait 8s for the real OPEN_TIMEOUT_MS to fire). - const fakeNg = makeFakeNg(); // empty quads → 0 rows - inject(fakeNg); - - // Force the private-store nuri ("did:ng:PRIV-AF") into timed-out state. - // resolveAccountReliably calls anchorNuri() → `did:ng:${privateStoreId}`. - _forceOpenedSyncState("did:ng:PRIV-AF", "timed-out"); - - // ensureAccount must throw (conservative anti-fork guard: do not provision blind) - await expect(ensureAccount("TimedOutUser")).rejects.toThrow( - /sync barrier timed out/, - ); - - // Must NOT have created any scope docs (refusing to provision) - expect(fakeNg.doc_create).toHaveBeenCalledTimes(0); - }); });