From f4a2587f1ce0d0351957a691e00d168e61712539 Mon Sep 17 00:00:00 2001 From: Alexander Khrushkov Date: Sat, 8 Aug 2026 22:49:18 +0300 Subject: [PATCH] fix: correct three false SDK claims and three real defects from the #23 audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bump PR shipped documentation that teaches code which cannot work, plus three defects no test could see. Nothing crashed, which is why it landed. False claims, each verified against the pinned SDK: - The refusal a pre-flip dApp receives names its version. ConnectClient has sent `sdkVersion` in the handshake since 0.10.1, so a 0.13.1 client is refused with `actualSdk: "0.13.1"`, not `null`. The docs presented the null/"unknown (not reported)" case as what an old dApp sees, and the browser and backend-auth tests were written around that unreachable branch. A reader branching on `actualSdk == null` gets dead code. - "Every pre-0.14 name still fires" is wrong. The flip removed 38 event names and gave 16 an adapter; the other 26 are accepted by `subscribe` and then never emit. 24 of them were live emitters in 0.13.1 — every `invoice:*`, every `swap:*`, `sync:started`/`:error`/`:provider` and more. CONNECT.md now carries the full removed-with-no-adapter table. - `payment_request:accepted` and `:response` were not fabrications: both are declared in 0.13.1's SphereEventType, and `:response` is emitted by PaymentsModule. They were removed by the flip without an adapter. Defects: - badgeFor painted an in-flight `submitted` transfer with the success green, as it did any payload with no `status`. Colour is now driven by the settled set (confirmed/delivered/completed) rather than a blocklist, so an unknown or absent status can no longer answer "did it go through?" with yes. - The bot registered its `inventory:updated` listener after the mint and after the first `assets()` round trip, so the credit event it exists to catch could fire with nobody attached. Subscribe first, mint second, read third — the boot read now goes through the same epoch guard. - mockSphere's paymentsV2 lacked `requests`, which the host dereferences in `sphere.paymentsV2?.requests.list()` — the optional chain guards paymentsV2, not requests, so the first live `payment_request:updated` would throw a TypeError inside ConnectHost. Masked only by `on` being a no-op stub. Also: - sendSafety.test.ts could not detect a code the SDK ADDS, which is the direction that costs money. It now sweeps the whole SphereErrorCode universe through the SDK predicate, with a type-level guard that fails `tsc` and names any code a future SDK adds. - formatAssets moved to bot/src/balance.ts so it is reachable by tests at all; index.ts calls main() at module scope. - nodejs describeConnectFailure and backend-auth describeVersionFloor each handled one of the three handshake-refusal shapes. Both now cover the SDK floor, the protocol floor and the 4008 network mismatch — the last being what a dApp that omits `network` actually hits. The latter is renamed describeHandshakeRefusal to match what it does. - CI runs npm ci instead of npm install. The comment justifying npm install described a file: link that no lockfile carries any more, and npm install will not fail when package.json and package-lock.json disagree. --- .github/workflows/ci.yml | 16 ++-- CLAUDE.md | 17 ++-- backend-auth/README.md | 3 +- backend-auth/frontend/src/errors.test.ts | 66 +++++++++++++-- backend-auth/frontend/src/errors.ts | 63 ++++++++++++--- bot/src/balance.test.ts | 53 ++++++++++++ bot/src/balance.ts | 37 +++++++++ bot/src/index.ts | 81 ++++++++----------- bot/src/sendSafety.test.ts | 79 +++++++++++++++--- browser/CONNECT.md | 37 ++++++--- browser/README.md | 2 +- .../components/events/EventLogPanel.test.ts | 69 +++++++++++++--- .../src/components/events/EventLogPanel.tsx | 28 +++++-- browser/src/hooks/useWalletConnect.test.ts | 24 +++++- nodejs/src/lockResume.test.ts | 64 +++++++++++++++ nodejs/src/lockResume.ts | 54 +++++++++++++ nodejs/src/mockSphere.test.ts | 55 +++++++++++++ nodejs/src/mockSphere.ts | 38 +++++++-- 18 files changed, 663 insertions(+), 123 deletions(-) create mode 100644 bot/src/balance.test.ts create mode 100644 bot/src/balance.ts create mode 100644 nodejs/src/mockSphere.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c96cd73..f70df69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,9 +21,11 @@ jobs: node-version: 20 cache: npm cache-dependency-path: browser/package-lock.json - # npm install, not npm ci: the lockfile carries a file: link to the local SDK - # until the last commit on this branch restores the published pin. - - run: npm install + # npm ci, not npm install: every lockfile here resolves @unicitylabs/sphere-sdk from + # registry.npmjs.org at the pinned version, so the install must be reproducible AND + # must fail when package.json and package-lock.json disagree. `npm install` silently + # re-resolves instead — on an SDK-bump PR that is exactly the regression to catch. + - run: npm ci - run: npm test # tsc -b covers src/**, which now includes the test files. - run: npx tsc -b @@ -40,7 +42,7 @@ jobs: node-version: 20 cache: npm cache-dependency-path: nodejs/package-lock.json - - run: npm install + - run: npm ci - run: npm test - run: npx tsc --noEmit @@ -56,7 +58,7 @@ jobs: node-version: 20 cache: npm cache-dependency-path: backend-auth/frontend/package-lock.json - - run: npm install + - run: npm ci - run: npm test - run: npx tsc -b @@ -72,7 +74,7 @@ jobs: node-version: 22 cache: npm cache-dependency-path: bot/package-lock.json - - run: npm install + - run: npm ci - run: npm test - run: npm run typecheck @@ -88,6 +90,6 @@ jobs: node-version: 22 cache: npm cache-dependency-path: backend-auth/backend/package-lock.json - - run: npm install + - run: npm ci - run: npm test - run: npm run typecheck diff --git a/CLAUDE.md b/CLAUDE.md index 92c3fcf..4ae5658 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,9 +3,10 @@ > **SDK floor:** every package pins `@unicitylabs/sphere-sdk` **0.14.2** exactly. Wallet hosts > from 0.14.1 enforce an SDK version floor at the Connect handshake (`ConnectHost`'s built-in > default is `0.14.1-0`, overridable via `ConnectHostConfig.minSdkVersion`): a client on an older -> SDK — or one too old to report a version, i.e. anything before 0.14.1 — is refused with -> `UNSUPPORTED_PROTOCOL_VERSION` (4007) carrying `data.requiredSdk` / `data.actualSdk`. The -> Connect protocol is unchanged at **2.1**. +> SDK is refused with `UNSUPPORTED_PROTOCOL_VERSION` (4007) carrying `data.requiredSdk` / +> `data.actualSdk`. `ConnectClient` has reported its version since **0.10.1**, so `actualSdk` is +> the reported string (`"0.13.1"`) — `null` / `"unknown (not reported)"` only reaches a host from +> 0.9.x or 0.10.0. The Connect protocol is unchanged at **2.1**. Demonstration project with four runnable examples of working with a Sphere wallet: a **browser dApp** and a **Node.js dApp** (both use the Connect protocol to drive a user's wallet), a **bot** that runs its own wallet (direct SDK, no Connect), and a **backend-auth** flow (a frontend brokers a wallet signature, a backend verifies it and issues a JWT). The Connect module enables dApps to interact with Sphere wallets through a transport-agnostic, permission-based RPC interface. @@ -342,8 +343,14 @@ Subscribable events (via `client.on()`), using the **sphere-sdk 0.14 names**: - `nametag:registered` / `nametag:recovered` — Nametag lifecycle - `address:activated` — New address tracked -Every pre-0.14 name still fires: the host re-emits each one from the new event through a -compatibility adapter, so no dApp subscription silently went dead. New code uses the names above. +The **16** pre-0.14 names listed in the host's `COMPAT_ATTACHERS` (`connect/host/payments-compat.ts`) +still fire — the host re-emits each from the new event through a compatibility adapter. The other +**26** removed names do NOT, and they fail silently: `Sphere.on()` accepts any string, so the +subscribe succeeds and then never delivers. Whole families went that way — every `invoice:*` and +every `swap:*`, plus `sync:started` / `:error` / `:provider`, `inventory:conflict`, +`send:partial-remainder`, `transfer:invalid`, `walletapi:session`, `payment_request:accepted` / +`:response` / `:settling`. Auditing a pre-0.14 dApp means checking its subscriptions against the +adapter list, not assuming they carried over. New code uses the names above; `browser/src/components/events/EventLogPanel.tsx` holds the canonical list this repo subscribes to. ## Connect Module Source (in sphere-sdk) diff --git a/backend-auth/README.md b/backend-auth/README.md index 5687bec..26770c2 100644 --- a/backend-auth/README.md +++ b/backend-auth/README.md @@ -10,7 +10,8 @@ and issues a session JWT. > 0.14.1 enforce an SDK version floor at the handshake and refuse older clients > with `UNSUPPORTED_PROTOCOL_VERSION` (4007) before any approval UI appears — > so an un-bumped dApp simply stops signing anybody in. `src/errors.ts` -> (`describeVersionFloor`) turns that refusal into copy that names the required +> (`describeHandshakeRefusal`) turns that refusal — the SDK floor, the protocol floor +> or a network mismatch — into copy that names the required > version. The **backend** is unaffected: it only calls > `recoverPubkeyFromSignature` / `verifySignedMessage`, which are unchanged. diff --git a/backend-auth/frontend/src/errors.test.ts b/backend-auth/frontend/src/errors.test.ts index dabfb33..da37421 100644 --- a/backend-auth/frontend/src/errors.test.ts +++ b/backend-auth/frontend/src/errors.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; import { ERROR_CODES } from '@unicitylabs/sphere-sdk/connect'; -import { describeError, describeVersionFloor, isConnectErrorCode, isWalletLocked } from './errors'; +import { describeError, describeHandshakeRefusal, isConnectErrorCode, isWalletLocked } from './errors'; const coded = (code: number, message = 'refused') => Object.assign(new Error(message), { code }); const codedWithData = (code: number, message: string, data: unknown) => @@ -33,7 +33,23 @@ describe('describeError', () => { ); }); - it('names the required SDK version when the wallet enforces its floor', () => { + // The refusal a real pre-flip dApp receives: `ConnectClient` has reported its version + // since sphere-sdk 0.10.1, so the wallet names it. This is the reachable case. + it('names both SDK versions when the wallet enforces its floor', () => { + const text = describeError( + codedWithData( + ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, + 'SDK version 0.13.1 is below the required minimum 0.14.1-0', + { reason: 'protocol_incompatible', requiredSdk: '0.14.1-0', actualSdk: '0.13.1' }, + ), + ); + expect(text).toContain('0.14.1-0'); + expect(text).toContain('0.13.1'); + }); + + // `actualSdk: null` reaches a host only from 0.9.x / 0.10.0, which predate the + // handshake's sdkVersion field. Still handled — just not the common case. + it('says so when the client reported no version at all', () => { const text = describeError( codedWithData( ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, @@ -54,20 +70,20 @@ describe('describeError', () => { }); }); -describe('describeVersionFloor', () => { +describe('describeHandshakeRefusal', () => { it('is null for any other code', () => { - expect(describeVersionFloor(coded(ERROR_CODES.USER_REJECTED))).toBeNull(); + expect(describeHandshakeRefusal(coded(ERROR_CODES.USER_REJECTED))).toBeNull(); }); it('is null when the host sent no versions to name', () => { expect( - describeVersionFloor(codedWithData(ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, 'nope', { reason: 'x' })), + describeHandshakeRefusal(codedWithData(ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, 'nope', { reason: 'x' })), ).toBeNull(); }); it('names both versions when the host reported them', () => { expect( - describeVersionFloor( + describeHandshakeRefusal( codedWithData(ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, 'nope', { requiredSdk: '0.14.1-0', actualSdk: '0.13.1', @@ -78,6 +94,44 @@ describe('describeVersionFloor', () => { 'Upgrade @unicitylabs/sphere-sdk and rebuild.', ); }); + + // A protocol-floor 4007 carries no requiredSdk — a describer written for the SDK branch + // alone returns null here and the UI falls back to a message that names nothing. + it('names both protocol versions on a protocol-floor refusal', () => { + const text = describeHandshakeRefusal( + codedWithData(ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, 'Connect protocol 2.0 is below the required minimum 2.1', { + reason: 'protocol_incompatible', + clientProtocol: '2.0', + requiredProtocol: '2.1', + }), + ); + expect(text).toContain('2.0'); + expect(text).toContain('2.1'); + }); + + // 4008 is the refusal a dApp that omits `network` actually hits, and its bare message + // ('dApp targets a different network than the wallet') names neither side. + it('names both networks on a mismatch', () => { + const text = describeHandshakeRefusal( + codedWithData(ERROR_CODES.INCOMPATIBLE_NETWORK, 'dApp targets a different network than the wallet', { + walletNetwork: { id: 4, name: 'testnet2' }, + clientNetwork: { id: 1, name: 'mainnet' }, + }), + ); + expect(text).toContain('testnet2'); + expect(text).toContain('mainnet'); + }); + + it('tells a dApp that declared no network what to pass', () => { + const text = describeHandshakeRefusal( + codedWithData(ERROR_CODES.INCOMPATIBLE_NETWORK, 'dApp targets a different network than the wallet', { + walletNetwork: { id: 4, name: 'testnet2' }, + clientNetwork: null, + }), + ); + expect(text).toContain('testnet2'); + expect(text).toContain('network'); + }); }); describe('isConnectErrorCode', () => { diff --git a/backend-auth/frontend/src/errors.ts b/backend-auth/frontend/src/errors.ts index 80c5219..8026670 100644 --- a/backend-auth/frontend/src/errors.ts +++ b/backend-auth/frontend/src/errors.ts @@ -16,31 +16,68 @@ function text(value: unknown): string | null { return typeof value === 'string' && value.length > 0 ? value : null; } +/** A network as `error.data` carries it: `{ id, name }`, a bare id, or nothing. */ +function networkName(value: unknown): string | null { + if (typeof value === 'number') return String(value); + if (typeof value !== 'object' || value === null) return null; + const bag = value as Record; + return text(bag.name) ?? (typeof bag.id === 'number' ? String(bag.id) : text(bag.id)); +} + /** - * A wallet host on sphere-sdk >= 0.14.1 refuses any dApp built on an older SDK with - * UNSUPPORTED_PROTOCOL_VERSION (4007), before any approval UI appears. It publishes the two - * versions it compared in `error.data`, so say which version is needed rather than rendering - * a bare "incompatible". Read defensively — `data` crosses postMessage from a peer on an SDK - * version this app does not control. + * A refused handshake, turned into copy that names what to change. + * + * There are THREE shapes behind the two codes, and handling only one leaves the others + * rendering a bare message that names nothing: + * 4007 + requiredSdk/actualSdk — the SDK floor + * 4007 + requiredProtocol/clientProtocol — the Connect protocol floor + * 4008 + walletNetwork/clientNetwork — a network mismatch, which is what a dApp + * that omits `network` hits on first connect + * + * Read defensively — `data` crosses postMessage from a peer on an SDK version this app + * does not control. */ -export function describeVersionFloor(err: unknown): string | null { - if (!isConnectErrorCode(err, ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION)) return null; +export function describeHandshakeRefusal(err: unknown): string | null { + const isVersion = isConnectErrorCode(err, ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION); + const isNetwork = isConnectErrorCode(err, ERROR_CODES.INCOMPATIBLE_NETWORK); + if (!isVersion && !isNetwork) return null; const data = (err as { data?: unknown }).data; if (typeof data !== 'object' || data === null) return null; const bag = data as Record; + + if (isNetwork) { + const wallet = networkName(bag.walletNetwork); + if (!wallet) return null; + const client = networkName(bag.clientNetwork); + return client + ? `This app targets network ${client}, but the wallet is on ${wallet}.` + : `This app declared no network — the wallet is on ${wallet}. Pass \`network\` to ConnectClient.`; + } + const requiredSdk = text(bag.requiredSdk); - if (!requiredSdk) return null; - const actualSdk = text(bag.actualSdk); - const has = actualSdk ? `is built on sphere-sdk ${actualSdk}` : 'reported no sphere-sdk version'; - return `This app ${has} — the wallet requires ${requiredSdk} or newer. Upgrade @unicitylabs/sphere-sdk and rebuild.`; + if (requiredSdk) { + // `actualSdk` is a version STRING for any client on sphere-sdk >= 0.10.1; it is null + // only for 0.9.x / 0.10.0, the releases predating the handshake's sdkVersion field. + const actualSdk = text(bag.actualSdk); + const has = actualSdk ? `is built on sphere-sdk ${actualSdk}` : 'reported no sphere-sdk version'; + return `This app ${has} — the wallet requires ${requiredSdk} or newer. Upgrade @unicitylabs/sphere-sdk and rebuild.`; + } + + const clientProtocol = text(bag.clientProtocol); + const requiredProtocol = text(bag.requiredProtocol); + if (clientProtocol && requiredProtocol) { + return `This app speaks Connect protocol ${clientProtocol} — the wallet requires ${requiredProtocol} or newer. Upgrade @unicitylabs/sphere-sdk and rebuild.`; + } + + return null; } export function describeError(err: unknown): string { if (isConnectErrorCode(err, ERROR_CODES.USER_REJECTED) || isConnectErrorCode(err, ERROR_CODES.INTENT_CANCELLED)) { return 'You declined the signature request in your wallet.'; } - const versionFloor = describeVersionFloor(err); - if (versionFloor) return versionFloor; + const refusal = describeHandshakeRefusal(err); + if (refusal) return refusal; if (isWalletLocked(err)) { return 'Your wallet is locked. Unlock it and press Sign in again — you are still connected, so no re-approval is needed.'; } diff --git a/bot/src/balance.test.ts b/bot/src/balance.test.ts new file mode 100644 index 0000000..91e8445 --- /dev/null +++ b/bot/src/balance.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from 'vitest'; +import { type Asset } from '@unicitylabs/sphere-sdk'; +import { formatAssets } from './balance'; + +const asset = (overrides: Partial): Asset => + ({ + symbol: 'UCT', + totalAmount: '100000000', + decimals: 8, + tokenCount: 1, + ...overrides, + }) as Asset; + +describe('formatAssets', () => { + it('renders an empty inventory as a placeholder, not an empty line', () => { + expect(formatAssets([])).toBe('(empty)'); + }); + + it('puts the decimal point back — the value arrives in BASE units', () => { + expect(formatAssets([asset({ totalAmount: '150000000' })])).toBe('1.5 UCT (1 token(s))'); + }); + + it('renders one line per asset', () => { + const out = formatAssets([ + asset({ symbol: 'UCT', totalAmount: '100000000', tokenCount: 2 }), + asset({ symbol: 'ALPHA', totalAmount: '250', decimals: 2, tokenCount: 1 }), + ]); + expect(out.split('\n ')).toEqual(['1 UCT (2 token(s))', '2.5 ALPHA (1 token(s))']); + }); + + // `Asset.decimals` is a required `number` in the SDK types, but the value crosses a + // network boundary from wallet-api before anything types it. A malformed one must + // degrade to the raw base units, never throw the bot's print loop down. + it.each([ + ['undefined decimals', { decimals: undefined as unknown as number }], + ['negative decimals', { decimals: -1 }], + ['fractional decimals', { decimals: 1.5 }], + ['non-numeric amount', { totalAmount: 'not-a-number' }], + ])('falls back to base units on %s instead of throwing', (_label, overrides) => { + const out = formatAssets([asset(overrides)]); + expect(out).toContain('(base units)'); + expect(out).toContain('UCT'); + }); + + it('keeps rendering the good assets when one is malformed', () => { + const out = formatAssets([ + asset({ symbol: 'BAD', decimals: -1 }), + asset({ symbol: 'GOOD', totalAmount: '100000000' }), + ]); + expect(out).toContain('BAD'); + expect(out).toContain('1 GOOD (1 token(s))'); + }); +}); diff --git a/bot/src/balance.ts b/bot/src/balance.ts new file mode 100644 index 0000000..7438733 --- /dev/null +++ b/bot/src/balance.ts @@ -0,0 +1,37 @@ +/** + * Balance rendering, extracted from `index.ts` so it can be unit-tested. + * + * `index.ts` calls `main()` at module scope — importing it from a test boots a real + * wallet — so anything worth asserting on has to live outside it. That is why + * `coins.ts`, `sendSafety.ts` and `aggregatorKey.ts` are separate modules, and this + * is the same rule applied to the one piece of money formatting the bot does. + * + * Verified against `@unicitylabs/sphere-sdk` **0.14.2**: `Asset` (types/index.ts) + * carries `symbol: string`, `totalAmount: string` in BASE units, `decimals: number` + * and `tokenCount: number`. + */ +import { type Asset } from '@unicitylabs/sphere-sdk'; +import { fromBaseUnits } from './coins'; + +/** + * `Asset.totalAmount` is in BASE units — `fromBaseUnits` puts the decimal point back. + * + * Falls back to the raw base-unit string when the decimal point cannot be placed. + * `Asset.decimals` is typed as a required `number`, so in a well-behaved SDK this + * never fires — but the value crosses a network boundary from wallet-api before it + * is typed, and printing a balance must never be able to kill the bot. + */ +export function formatAssets(assets: Asset[]): string { + if (assets.length === 0) return '(empty)'; + return assets + .map((a) => { + let amount: string; + try { + amount = fromBaseUnits(a.totalAmount, a.decimals); + } catch { + amount = `${a.totalAmount} (base units)`; + } + return `${amount} ${a.symbol} (${a.tokenCount} token(s))`; + }) + .join('\n '); +} diff --git a/bot/src/index.ts b/bot/src/index.ts index 692419e..e559006 100644 --- a/bot/src/index.ts +++ b/bot/src/index.ts @@ -32,39 +32,18 @@ */ import readline from 'readline'; import { - type Asset, type DirectMessage, type IncomingTransfer, type TransferResult, } from '@unicitylabs/sphere-sdk'; import { createBotSphere } from './sphere'; import { mayBeCommitted } from './sendSafety'; -import { fromBaseUnits, resolveCoin, toBaseUnits } from './coins'; +import { resolveCoin, toBaseUnits } from './coins'; +import { formatAssets } from './balance'; const MINT_SYMBOL = 'UCT'; const MINT_AMOUNT_HUMAN = '100'; -/** - * `Asset.totalAmount` is in BASE units — `fromBaseUnits` puts the decimal point back. - * - * Falls back to the raw base-unit string if the coin's `decimals` is missing or - * nonsensical. Printing a balance must never be able to kill the bot. - */ -function formatAssets(assets: Asset[]): string { - if (assets.length === 0) return '(empty)'; - return assets - .map((a) => { - let amount: string; - try { - amount = fromBaseUnits(a.totalAmount, a.decimals); - } catch { - amount = `${a.totalAmount} (base units)`; - } - return `${amount} ${a.symbol} (${a.tokenCount} token(s))`; - }) - .join('\n '); -} - async function main() { const { sphere, identity } = await createBotSphere(); console.log('Bot identity:', identity); @@ -82,7 +61,32 @@ async function main() { } }); - // --- 2. Self-mint a float, once on boot (best-effort — minting may be unavailable) --- + // --- 2. Balance: subscribe BEFORE the first read, then mint --- + // The server credits a fresh mint asynchronously and signals it with + // `inventory:updated`. Registering the listener after the mint (or after the first + // `assets()` round trip, which is hundreds of ms) loses that event outright: it + // fires with nobody attached, and the bot prints an empty balance it never revises. + // Subscribe first, mint second, read third. + // + // Two inventory updates in flight means two overlapping assets() reads, which can + // resolve out of order — printing a stale balance AFTER a fresher one. The epoch + // guard drops any result that a later read already superseded, and the boot read + // goes through the same guard so it cannot outrun an update either. A UI refreshing + // on this event needs the same rule. + let balanceEpoch = 0; + const refreshBalance = () => { + const epoch = ++balanceEpoch; + void sphere.payments + .assets() + .then((assets) => { + if (epoch !== balanceEpoch) return; // superseded by a newer read + console.log('[balance]\n ' + formatAssets(assets)); + }) + .catch((err) => console.error('[balance] read failed:', err instanceof Error ? err.message : err)); + }; + sphere.on('inventory:updated', refreshBalance); + + // --- 3. Self-mint a float, once on boot (best-effort — minting may be unavailable) --- try { const { coinId, decimals } = resolveCoin(MINT_SYMBOL); const amount = BigInt(toBaseUnits(MINT_AMOUNT_HUMAN, decimals)); @@ -96,28 +100,13 @@ async function main() { console.error('[mint] self-mint threw:', err instanceof Error ? err.message : err); } // `assets()` is a view over the wallet-api inventory, and the server credits a - // fresh mint asynchronously — so this first read can legitimately come back empty - // even though the mint above certified on-chain. `inventory:updated` is the signal - // that the server's view caught up; that is what a UI refreshes on. - console.log('[balance]\n ' + formatAssets(await sphere.payments.assets())); - - // Two inventory updates in flight means two overlapping assets() reads, which can - // resolve out of order — printing a stale balance AFTER a fresher one. The epoch - // guard drops any result that a later read already superseded. A UI refreshing on - // this event needs the same rule. - let balanceEpoch = 0; - sphere.on('inventory:updated', () => { - const epoch = ++balanceEpoch; - void sphere.payments - .assets() - .then((assets) => { - if (epoch !== balanceEpoch) return; // superseded by a newer read - console.log('[balance]\n ' + formatAssets(assets)); - }) - .catch((err) => console.error('[balance] read failed:', err instanceof Error ? err.message : err)); - }); + // fresh mint asynchronously — so this boot read can legitimately come back empty + // even though the mint above certified on-chain. The `inventory:updated` listener + // registered before the mint is what prints the corrected balance when the + // server's view catches up; that is what a UI refreshes on. + refreshBalance(); - // --- 3. Incoming transfers (tokens are delivered to the wallet-api mailbox) --- + // --- 4. Incoming transfers (tokens are delivered to the wallet-api mailbox) --- sphere.on('transfer:incoming', (t: IncomingTransfer) => { console.log(`[receive] incoming transfer ${t.id} from ${t.senderNametag ?? t.senderPubkey}`); sphere.communications.sendDM(t.senderPubkey, 'thanks for the tokens!').catch((err) => { @@ -137,7 +126,7 @@ async function main() { console.log(`[connection] ${status}`); }); - // --- 4. Demo command loop: send / balance / pending / resume / help / exit --- + // --- 5. Demo command loop: send / balance / pending / resume / help / exit --- const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); function showPrompt() { diff --git a/bot/src/sendSafety.test.ts b/bot/src/sendSafety.test.ts index c5276ca..5c4e037 100644 --- a/bot/src/sendSafety.test.ts +++ b/bot/src/sendSafety.test.ts @@ -1,7 +1,58 @@ import { describe, expect, it } from 'vitest'; import { SphereError, isPossiblyCommittedSendOutcome } from '@unicitylabs/sphere-sdk'; +import type { SphereErrorCode } from '@unicitylabs/sphere-sdk'; import { POSSIBLY_COMMITTED_CODES, mayBeCommitted } from './sendSafety'; +/** + * Every `SphereErrorCode` in the pinned SDK. + * + * The SDK's own `POSSIBLY_COMMITTED_SEND_CODES` is module-private, so there is no set to + * import and diff against. This array is the substitute universe: sweeping it through + * `isPossiblyCommittedSendOutcome` asks the SDK about each code in turn, which is what makes + * the SDK -> us direction checkable at all. Testing only our own set can never notice a code + * the SDK ADDS — and that is the direction that costs money, because an unrecognised + * possibly-committed code falls through to the retryable branch and invites a double pay. + * + * The `Missing` guard below is what keeps this honest: when an SDK bump grows the union, + * `tsc --noEmit` fails HERE and names the new code, instead of the sweep quietly skipping it. + */ +const ALL_ERROR_CODES = [ + 'NOT_INITIALIZED', 'ALREADY_INITIALIZED', 'INVALID_CONFIG', 'INVALID_IDENTITY', + 'INSUFFICIENT_BALANCE', 'INVALID_RECIPIENT', 'TRANSFER_FAILED', 'TRANSFER_CONFLICT', + 'CERTIFICATION_UNCONFIRMED', 'CHECKPOINT_PERSIST_FAILED', 'SPLIT_CHECKPOINT_LOST', + 'CHECKPOINT_TRUSTBASE_MISMATCH', 'STORAGE_ERROR', 'SEND_SYNC_PENDING', + 'SEND_PARTIALLY_COMPLETED', 'TRANSPORT_ERROR', 'AGGREGATOR_ERROR', 'VALIDATION_ERROR', + 'INVALID_AMOUNT', 'NETWORK_ERROR', 'TIMEOUT', 'DECRYPTION_ERROR', 'MODULE_NOT_AVAILABLE', + 'SIGNING_ERROR', 'SEND_QUEUE_TIMEOUT', 'SEND_INSUFFICIENT_BALANCE', + 'SEND_RESERVATION_CANCELLED', 'SEND_QUEUE_FULL', 'MODULE_DESTROYED', 'REENTRANT_GATE', + 'RATE_LIMITED', 'COMMUNICATIONS_UNAVAILABLE', + // Invoice + swap codes: the modules were deleted by the payments-v2 flip, but the code + // union still carries them, so the universe has to include them to stay complete. + 'INVOICE_NO_TARGETS', 'INVOICE_INVALID_ADDRESS', 'INVOICE_NO_ASSETS', 'INVOICE_INVALID_ASSET', + 'INVOICE_INVALID_AMOUNT', 'INVOICE_INVALID_COIN', 'INVOICE_INVALID_NFT', + 'INVOICE_PAST_DUE_DATE', 'INVOICE_DUPLICATE_ADDRESS', 'INVOICE_DUPLICATE_COIN', + 'INVOICE_DUPLICATE_NFT', 'INVOICE_MINT_FAILED', 'INVOICE_INVALID_PROOF', + 'INVOICE_WRONG_TOKEN_TYPE', 'INVOICE_INVALID_DATA', 'INVOICE_ALREADY_EXISTS', + 'INVOICE_NOT_FOUND', 'INVOICE_NOT_TARGET', 'INVOICE_ALREADY_CLOSED', + 'INVOICE_ALREADY_CANCELLED', 'INVOICE_ORACLE_REQUIRED', 'INVOICE_TERMINATED', + 'INVOICE_INVALID_TARGET', 'INVOICE_INVALID_ASSET_INDEX', 'INVOICE_RETURN_EXCEEDS_BALANCE', + 'INVOICE_INVALID_DELIVERY_METHOD', 'INVOICE_INVALID_REFUND_ADDRESS', 'INVOICE_INVALID_CONTACT', + 'INVOICE_INVALID_ID', 'INVOICE_TOO_MANY_TARGETS', 'INVOICE_TOO_MANY_ASSETS', + 'INVOICE_MEMO_TOO_LONG', 'INVOICE_TERMS_TOO_LARGE', 'INVOICE_NOT_TERMINATED', + 'INVOICE_NOT_CANCELLED', 'INVOICE_STORAGE_FAILED', + 'SWAP_INVALID_DEAL', 'SWAP_INVALID_MANIFEST', 'SWAP_NOT_FOUND', 'SWAP_WRONG_STATE', + 'SWAP_RESOLVE_FAILED', 'SWAP_DM_SEND_FAILED', 'SWAP_ESCROW_REJECTED', 'SWAP_DEPOSIT_FAILED', + 'SWAP_PAYOUT_VERIFICATION_FAILED', 'SWAP_ALREADY_EXISTS', 'SWAP_ALREADY_COMPLETED', + 'SWAP_ALREADY_CANCELLED', 'SWAP_TIMEOUT', 'SWAP_LIMIT_EXCEEDED', 'SWAP_ALREADY_INITIALIZED', + 'SWAP_MODULE_DESTROYED', 'SWAP_NOT_INITIALIZED', +] as const satisfies readonly SphereErrorCode[]; + +// A code the SDK declares that this file has not enumerated. Must be `never`: if it is not, +// the assignment below fails to compile and the error text names the code that was added. +type Missing = Exclude; +const _everyCodeEnumerated: [Missing] extends [never] ? true : Missing = true; +void _everyCodeEnumerated; + describe('mayBeCommitted', () => { it('agrees with the SDK predicate on a real SphereError', () => { for (const code of POSSIBLY_COMMITTED_CODES) { @@ -45,18 +96,26 @@ describe('mayBeCommitted', () => { }); /** - * Pins our mirrored set against the SDK's own. If a future SDK adds a - * possibly-committed code, this fails instead of silently letting the new - * code fall through to the retryable branch. + * Pins our mirrored set against the SDK's own, in BOTH directions, over the whole code + * universe — not just over our own set, which by construction can only confirm what we + * already believe. A code the SDK starts treating as possibly-committed shows up here as + * a concrete failure ("SDK says X is committed, we say retryable"), which is the double-pay + * this module exists to prevent. */ - it('mirrors the SDK set exactly', () => { - const notCommitted = ['SEND_INSUFFICIENT_BALANCE', 'INVALID_RECIPIENT', 'TRANSFER_CONFLICT', 'TIMEOUT']; - for (const code of notCommitted) { - expect(isPossiblyCommittedSendOutcome(new SphereError('x', code as never))).toBe(false); - expect(POSSIBLY_COMMITTED_CODES.has(code)).toBe(false); - } + it('mirrors the SDK set exactly, in both directions, across every error code', () => { + const sdkSays = ALL_ERROR_CODES.filter((code) => + isPossiblyCommittedSendOutcome(new SphereError('x', code)), + ); + const weSay = ALL_ERROR_CODES.filter((code) => POSSIBLY_COMMITTED_CODES.has(code)); + expect([...weSay].sort()).toEqual([...sdkSays].sort()); + }); + + // Our set must not contain anything outside the SDK's union either — a typo'd code would + // sit in the set forever, matching nothing, while looking like coverage. + it('contains no code the SDK does not declare', () => { + const declared = new Set(ALL_ERROR_CODES); for (const code of POSSIBLY_COMMITTED_CODES) { - expect(isPossiblyCommittedSendOutcome(new SphereError('x', code as never))).toBe(true); + expect(declared.has(code)).toBe(true); } }); }); diff --git a/browser/CONNECT.md b/browser/CONNECT.md index 5712cfb..138202b 100644 --- a/browser/CONNECT.md +++ b/browser/CONNECT.md @@ -6,17 +6,21 @@ This guide explains how to integrate a browser dApp with the Sphere wallet using > > Wallet hosts from 0.14.1 onward enforce an **SDK version floor at the handshake**. `ConnectHost` > applies a built-in default of `0.14.1-0` (a host may raise it via -> `ConnectHostConfig.minSdkVersion`). `ConnectClient` reports its own npm version -> in the handshake; a client below the floor — or one old enough not to report a version at all, -> which is every release before 0.14.1 — is refused with `UNSUPPORTED_PROTOCOL_VERSION` (**4007**) -> before any approval UI appears: +> `ConnectHostConfig.minSdkVersion`). `ConnectClient` has reported its own npm version in the +> handshake since **0.10.1**, so a client below the floor is refused by version, with that version +> named — `UNSUPPORTED_PROTOCOL_VERSION` (**4007**), before any approval UI appears: > > ```json > { "code": 4007, -> "message": "SDK version unknown (not reported) is below the required minimum 0.14.1-0", -> "data": { "reason": "protocol_incompatible", "requiredSdk": "0.14.1-0", "actualSdk": null } } +> "message": "SDK version 0.13.1 is below the required minimum 0.14.1-0", +> "data": { "reason": "protocol_incompatible", "requiredSdk": "0.14.1-0", "actualSdk": "0.13.1" } } > ``` > +> `actualSdk` is `null` (and the message reads `unknown (not reported)`) only for a client that +> sent no version at all — 0.9.x and 0.10.0, the two releases predating the handshake field. +> **Do not branch on `actualSdk == null` as the "old SDK" case:** every SDK a dApp is realistically +> built on reports a string, so that branch never fires. +> > The fix is a dependency bump and a rebuild — there is no protocol change to make. Connect is > still **2.1**. Read `data.requiredSdk` / `data.actualSdk` and put them in your error copy; > `describeConnectFailure()` in `src/lib/connectErrors.ts` does exactly that, so the user is told @@ -246,9 +250,24 @@ names in new code** — they are what the wallet actually emits: | `payment_request:paid`, `:rejected`, `:expired` | `payment_request:updated` `{ id, status }` | | `transfer:incoming` | unchanged | -Every old name still works: the wallet host re-emits each one from the new event through a -compatibility adapter, so a dApp built before 0.14 keeps receiving them. Nothing a dApp -subscribes to silently stopped firing. +The old names in the table above still work: the wallet host re-emits each from the new event +through a compatibility adapter, so a dApp built before 0.14 keeps receiving them. + +**The table is the whole list.** The payments-v2 flip removed 38 event names and gave 16 of them +an adapter; the remaining 26 are gone for good, and they fail *silently* — `Sphere.on()` accepts +any string, so the subscribe succeeds and then delivers nothing forever. If your dApp listens for +any of these, it is already dead code: + +| Removed with no adapter | | +|---|---| +| `invoice:created`, `:payment`, `:covered`, `:closed`, `:overpayment`, `:expired`, `:cancelled`, `:irrelevant` | the invoicing module was deleted | +| `swap:proposed`, `:accepted`, `:rejected`, `:cancelled`, `:concluding`, `:completed`, `:failed`, `:announced` | the swap module was deleted | +| `sync:started`, `sync:error`, `sync:provider` | only `sync:completed` / `sync:remote-update` map to `inventory:updated` | +| `payment_request:accepted`, `:response`, `:settling` | only `:paid` / `:rejected` / `:expired` have adapters | +| `inventory:conflict`, `send:partial-remainder`, `transfer:invalid`, `walletapi:session` | no equivalent | + +Auditing a pre-0.14 dApp means checking every subscription against the table above — a silent +subscription looks identical to one that simply has not fired yet. The four events in `AUTO_PUSHED_EVENTS` — `wallet:locked`, `wallet:unlocked`, `wallet:disconnected`, `identity:changed` — are pushed by `ConnectHost` unconditionally. Never route them through `sphere_subscribe`: `Sphere.on()` accepts any string and would silently never emit, so the subscribe would succeed and deliver nothing forever. See [Wallet Lock Handling](#wallet-lock-handling-wallet_eventslocked) below. diff --git a/browser/README.md b/browser/README.md index b1742ab..5c5a9f8 100644 --- a/browser/README.md +++ b/browser/README.md @@ -64,7 +64,7 @@ each panel drives one query / intent / event. > `deliveryPending: true` and **no `transferId`** — see the Send panel; never > re-send that (it would pay twice). -**Events** (real-time push): auto-pushed `wallet:locked` · `wallet:unlocked` · `wallet:disconnected` · `identity:changed`; subscribable `transfer:incoming` · `transfer:updated` · `transfer:attention` · `inventory:updated` · `payment_request:updated` · `connection:status` · and more. Those are the sphere-sdk 0.14 names — the pre-0.14 ones (`transfer:confirmed`, `sync:*`, …) still fire, re-emitted by the host's compatibility adapter. A lock does **not** disconnect — see [CONNECT.md](CONNECT.md#wallet-lock-handling-wallet_eventslocked). +**Events** (real-time push): auto-pushed `wallet:locked` · `wallet:unlocked` · `wallet:disconnected` · `identity:changed`; subscribable `transfer:incoming` · `transfer:updated` · `transfer:attention` · `inventory:updated` · `payment_request:updated` · `connection:status` · and more. Those are the sphere-sdk 0.14 names. Sixteen pre-0.14 names (`transfer:confirmed`, `sync:completed`, …) still fire, re-emitted by the host's compatibility adapter — but 26 others (every `invoice:*`, every `swap:*`, `sync:started`, …) were removed without one and fail *silently*, since `Sphere.on()` accepts any string. See the compat table in [CONNECT.md](CONNECT.md#events). A lock does **not** disconnect — see [CONNECT.md](CONNECT.md#wallet-lock-handling-wallet_eventslocked). ## How the connection is made diff --git a/browser/src/components/events/EventLogPanel.test.ts b/browser/src/components/events/EventLogPanel.test.ts index 64f8655..b71d3ed 100644 --- a/browser/src/components/events/EventLogPanel.test.ts +++ b/browser/src/components/events/EventLogPanel.test.ts @@ -23,9 +23,10 @@ describe('EventLogPanel event list', () => { }); // The demo is teaching material: it must subscribe to the names a current wallet - // actually emits. The pre-0.14 names still work through the host's compat adapter, - // so a stale list fails silently — nothing here would break, the panel would just - // quietly teach the wrong API. Pin both directions. + // actually emits. A stale list fails silently either way — the 16 names the host's + // COMPAT_ATTACHERS covers keep arriving through the adapter, and the other 26 are + // accepted by `subscribe` and then never fire. Neither breaks anything here; the + // panel would just quietly teach the wrong API. Pin both directions. it('uses the sphere-sdk 0.14 payments event names', () => { for (const event of [ 'transfer:incoming', @@ -59,13 +60,47 @@ describe('EventLogPanel event list', () => { 'payment_request:paid', 'payment_request:rejected', 'payment_request:expired', - // Never existed in any SDK release — the old list carried them anyway. + // Both WERE real in 0.13.1 (declared in SphereEventType/SphereEventMap; + // `:response` is emitted by PaymentsModule). The flip removed them and gave + // them no compat attacher, so unlike the names above they do not come back. 'payment_request:accepted', 'payment_request:response', ]) { expect(ALL_EVENTS).not.toContain(event); } }); + + // The 26 pre-0.14 names with no COMPAT_ATTACHERS entry are the dangerous ones: a + // subscription is ACCEPTED and then silently never fires, so listing one here would + // look like a working demo of an event that can no longer arrive. Whole families went + // this way — every `invoice:*` and every `swap:*`. + it('lists no pre-0.14 name the compat adapter does not re-emit', () => { + for (const event of [ + 'invoice:created', + 'invoice:payment', + 'invoice:covered', + 'invoice:closed', + 'invoice:overpayment', + 'invoice:expired', + 'invoice:cancelled', + 'invoice:irrelevant', + 'swap:proposed', + 'swap:accepted', + 'swap:rejected', + 'swap:cancelled', + 'swap:concluding', + 'swap:completed', + 'swap:failed', + 'swap:announced', + 'inventory:conflict', + 'send:partial-remainder', + 'transfer:invalid', + 'walletapi:session', + 'payment_request:settling', + ]) { + expect(ALL_EVENTS).not.toContain(event); + } + }); }); describe('badgeFor — transfer:updated is the COMBINED outcome event', () => { @@ -80,15 +115,31 @@ describe('badgeFor — transfer:updated is the COMBINED outcome event', () => { expect(badgeFor('transfer:updated', { status: 'pending' })).toContain('amber'); }); - it('keeps the success colour for a delivered transfer and for every other event', () => { - expect(badgeFor('transfer:updated', { status: 'delivered', deliveryPending: false })) - .toBe(EVENT_COLORS['transfer:updated']); + // `submitted` is certification IN FLIGHT — the money has not settled. It is the one + // status a blocklist implementation gets wrong, because it is neither 'failed' nor + // 'pending' and so falls through to the success colour. + it('colours a SUBMITTED transfer amber — it has not settled yet', () => { + expect(badgeFor('transfer:updated', { status: 'submitted' })).toContain('amber'); + }); + + // Green must be earned, not defaulted into: a payload with no status has told us + // nothing about the outcome, so it cannot answer "did it go through?" with yes. + it('does not paint an outcome-less payload green', () => { + expect(badgeFor('transfer:updated', {})).toContain('amber'); + expect(badgeFor('transfer:updated', { status: 'something-new' })).toContain('amber'); + }); + + it('keeps the success colour only for a settled transfer, and for every other event', () => { + for (const status of ['confirmed', 'delivered', 'completed']) { + expect(badgeFor('transfer:updated', { status, deliveryPending: false })) + .toBe(EVENT_COLORS['transfer:updated']); + } expect(badgeFor('transfer:incoming', {})).toBe(EVENT_COLORS['transfer:incoming']); expect(badgeFor('unknown:event', {})).toBe('bg-white/3 text-white/55'); }); it('does not crash on a null/undefined payload', () => { - expect(badgeFor('transfer:updated', null)).toBe(EVENT_COLORS['transfer:updated']); - expect(badgeFor('transfer:updated', undefined)).toBe(EVENT_COLORS['transfer:updated']); + expect(badgeFor('transfer:updated', null)).toContain('amber'); + expect(badgeFor('transfer:updated', undefined)).toContain('amber'); }); }); diff --git a/browser/src/components/events/EventLogPanel.tsx b/browser/src/components/events/EventLogPanel.tsx index 4e110fd..eb34a77 100644 --- a/browser/src/components/events/EventLogPanel.tsx +++ b/browser/src/components/events/EventLogPanel.tsx @@ -22,9 +22,12 @@ interface Props { * realtime:status + storage:degraded -> connection:status * payment_request:paid / :rejected / :expired -> payment_request:updated * - * The old names still resolve — a wallet host re-emits every one of them from the new event - * through a compatibility adapter, so a dApp built before 0.14 keeps working. New code should - * use the names below; they are what the wallet actually emits. + * The 16 old names listed in the host's COMPAT_ATTACHERS still resolve — the wallet re-emits + * them from the new event through a compatibility adapter. The other 26 pre-0.14 names do NOT: + * `subscribe` accepts any string, so a stale subscription to e.g. `invoice:payment`, + * `swap:failed`, `sync:started`, `send:partial-remainder`, `transfer:invalid` or + * `payment_request:response` is ACCEPTED and then silently never fires. Use the names below; + * they are what the wallet actually emits. */ export const ALL_EVENTS = [ // Transfers @@ -117,18 +120,27 @@ export const EVENT_COLORS: Record = { * Badge style for one logged event. The name alone is not enough for * `transfer:updated`: a FAILED send carries the same event name as a confirmed * one, and green-for-failed is the miscolour that actively misleads — a dApp - * dev reads this log to answer "did it go through?". Failed → red; - * still-converging (deliveryPending / pending) → amber; else the table colour. + * dev reads this log to answer "did it go through?". + * + * The colour is driven by the SETTLED set, not by a blocklist: `TransferStatus` + * is `pending | submitted | confirmed | delivered | completed | failed`, and only + * the last three mean the money has landed. `submitted` is certification IN FLIGHT + * — painting it green answers "did it go through?" with yes before it is true — + * and a payload carrying no `status` at all has told us nothing, so neither may + * fall through to the green table colour. */ +const SETTLED_TRANSFER_STATUSES = new Set(['confirmed', 'delivered', 'completed']); + export function badgeFor(event: string, data: unknown): string { const fallback = EVENT_COLORS[event] ?? 'bg-white/3 text-white/55'; if (event !== 'transfer:updated') return fallback; const result = data as { status?: unknown; deliveryPending?: unknown } | null | undefined; if (result?.status === 'failed') return 'bg-red-500/15 text-red-400'; - if (result?.deliveryPending === true || result?.status === 'pending') { - return 'bg-amber-500/15 text-amber-400'; + if (result?.deliveryPending === true) return 'bg-amber-500/15 text-amber-400'; + if (typeof result?.status === 'string' && SETTLED_TRANSFER_STATUSES.has(result.status)) { + return fallback; } - return fallback; + return 'bg-amber-500/15 text-amber-400'; } diff --git a/browser/src/hooks/useWalletConnect.test.ts b/browser/src/hooks/useWalletConnect.test.ts index 403cc10..74f6b17 100644 --- a/browser/src/hooks/useWalletConnect.test.ts +++ b/browser/src/hooks/useWalletConnect.test.ts @@ -467,8 +467,27 @@ describe('useWalletConnect — a dApp reload must not reload the wallet', () => describe('useWalletConnect — a refused handshake says which version to move to', () => { it('surfaces the SDK floor the wallet compared, not just its bare message', async () => { - // Exactly what a 0.14.1 wallet sends a pre-0.14.1 app. A client that old reports no - // sdkVersion at all, so `actualSdk` comes back null and only `requiredSdk` is nameable. + // Exactly what a 0.14.1 wallet sends a pre-flip app. `ConnectClient` has reported its + // npm version in the handshake since sphere-sdk 0.10.1, so the wallet names it: this + // is the refusal a real un-bumped dApp receives, and both versions must reach the UI. + FakeConnectClient.nextConnectError = new ConnectError( + 'SDK version 0.13.1 is below the required minimum 0.14.1-0', + ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, + { reason: 'protocol_incompatible', requiredSdk: '0.14.1-0', actualSdk: '0.13.1' }, + ); + + const hook = renderHook(() => useWalletConnect()); + await waitFor(() => expect(hook.result.current.isAutoConnecting).toBe(false)); + await connectPopup(hook.result); + + expect(hook.result.current.isConnected).toBe(false); + expect(hook.result.current.error).toContain('0.13.1'); + expect(hook.result.current.error).toContain('0.14.1-0'); + }); + + // `actualSdk: null` only reaches a host from 0.9.x / 0.10.0, the releases predating the + // handshake's sdkVersion field. Kept covered, but it is not the case to design copy around. + it('still says something useful when the client reported no version', async () => { FakeConnectClient.nextConnectError = new ConnectError( 'SDK version unknown (not reported) is below the required minimum 0.14.1-0', ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, @@ -479,7 +498,6 @@ describe('useWalletConnect — a refused handshake says which version to move to await waitFor(() => expect(hook.result.current.isAutoConnecting).toBe(false)); await connectPopup(hook.result); - expect(hook.result.current.isConnected).toBe(false); expect(hook.result.current.error).toContain('reported no sphere-sdk version'); expect(hook.result.current.error).toContain('0.14.1-0'); }); diff --git a/nodejs/src/lockResume.test.ts b/nodejs/src/lockResume.test.ts index 4ba86ae..ab1f831 100644 --- a/nodejs/src/lockResume.test.ts +++ b/nodejs/src/lockResume.test.ts @@ -35,6 +35,70 @@ describe('describeConnectFailure', () => { 'Query timeout: sphere_getBalance', ); }); + + // The SDK floor (4007). `actualSdk` is a version STRING for any client on sphere-sdk + // >= 0.10.1 — a wallet naming the version is the normal case, not the exotic one. + it('names both versions on an SDK-floor refusal', () => { + const text = describeConnectFailure( + new ConnectError('SDK version 0.13.1 is below the required minimum 0.14.1-0', ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, { + reason: 'protocol_incompatible', + requiredSdk: '0.14.1-0', + actualSdk: '0.13.1', + }), + ); + expect(text).toContain('0.13.1'); + expect(text).toContain('0.14.1-0'); + }); + + it('says so plainly when the client reported no version at all', () => { + const text = describeConnectFailure( + new ConnectError('SDK version unknown (not reported) is below the required minimum 0.14.1-0', ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, { + reason: 'protocol_incompatible', + requiredSdk: '0.14.1-0', + actualSdk: null, + }), + ); + expect(text).toContain('reported no sphere-sdk version'); + expect(text).toContain('0.14.1-0'); + }); + + // The protocol floor is a DIFFERENT 4007 payload — no requiredSdk, so a describer that + // only handles the SDK branch silently degrades to the bare message. + it('names both protocol versions on a protocol-floor refusal', () => { + const text = describeConnectFailure( + new ConnectError('Connect protocol 2.0 is below the required minimum 2.1', ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION, { + reason: 'protocol_incompatible', + clientProtocol: '2.0', + requiredProtocol: '2.1', + }), + ); + expect(text).toContain('2.0'); + expect(text).toContain('2.1'); + }); + + // 4008 is what a dApp that forgets `network` actually hits, and the bare message + // ('dApp targets a different network than the wallet') names neither side. + it('names both networks on a network mismatch', () => { + const text = describeConnectFailure( + new ConnectError('dApp targets a different network than the wallet', ERROR_CODES.INCOMPATIBLE_NETWORK, { + walletNetwork: { id: 4, name: 'testnet2' }, + clientNetwork: { id: 1, name: 'mainnet' }, + }), + ); + expect(text).toContain('testnet2'); + expect(text).toContain('mainnet'); + }); + + it('tells a dApp that declared no network what to pass', () => { + const text = describeConnectFailure( + new ConnectError('dApp targets a different network than the wallet', ERROR_CODES.INCOMPATIBLE_NETWORK, { + walletNetwork: { id: 4, name: 'testnet2' }, + clientNetwork: null, + }), + ); + expect(text).toContain('testnet2'); + expect(text).toContain('network'); + }); }); describe('isSameWallet', () => { diff --git a/nodejs/src/lockResume.ts b/nodejs/src/lockResume.ts index 71b582a..7b8281c 100644 --- a/nodejs/src/lockResume.ts +++ b/nodejs/src/lockResume.ts @@ -13,11 +13,39 @@ function connectErrorCode(err: unknown): number | undefined { return typeof code === 'number' ? code : undefined; } +/** `data` crosses the wire from a peer on an SDK version this process does not control. */ +function errorData(err: unknown): Record | undefined { + if (typeof err !== 'object' || err === null) return undefined; + const data = (err as { data?: unknown }).data; + return typeof data === 'object' && data !== null ? (data as Record) : undefined; +} + +function text(value: unknown): string | undefined { + return typeof value === 'string' && value !== '' ? value : undefined; +} + +function networkName(value: unknown): string | undefined { + if (typeof value === 'number') return String(value); + if (typeof value !== 'object' || value === null) return undefined; + const bag = value as Record; + return text(bag.name) ?? (typeof bag.id === 'number' ? String(bag.id) : text(bag.id)); +} + /** Discriminate on the CODE. The refusal text is a recommendation, not a contract. */ export function isWalletLocked(err: unknown): boolean { return connectErrorCode(err) === ERROR_CODES.WALLET_LOCKED; } +/** + * Turn a Connect failure into copy a human can act on. + * + * The handshake refusals are the ones worth unpacking: 4007 and 4008 both carry the numbers + * that say WHAT to change in `error.data`, and a message that only repeats "incompatible" + * sends a developer hunting. All three shapes are handled — the SDK floor (`requiredSdk` / + * `actualSdk`), the protocol floor (`requiredProtocol` / `clientProtocol`) and the network + * mismatch (`walletNetwork` / `clientNetwork`) — because a dApp that omits `network` hits the + * last one on its very first connect. + */ export function describeConnectFailure(err: unknown): string { const code = connectErrorCode(err); const message = err instanceof Error ? err.message : String(err); @@ -28,6 +56,32 @@ export function describeConnectFailure(err: unknown): string { 'Type "unlock" in the mock wallet server (or unlock the real wallet) and run the command again.' ); } + + const data = errorData(err); + if (data) { + if (code === ERROR_CODES.INCOMPATIBLE_NETWORK) { + const client = networkName(data.clientNetwork); + const wallet = networkName(data.walletNetwork); + if (client && wallet) return `This app targets network ${client}, but the wallet is on ${wallet}.`; + if (wallet) return `This app declared no network — the wallet is on ${wallet}. Pass \`network\` to ConnectClient.`; + } + if (code === ERROR_CODES.UNSUPPORTED_PROTOCOL_VERSION) { + const requiredSdk = text(data.requiredSdk); + if (requiredSdk) { + // `actualSdk` is the reported version string for any client on sphere-sdk >= 0.10.1; + // null only for the two releases that predate the handshake field. + const actualSdk = text(data.actualSdk); + const has = actualSdk ? `is built on sphere-sdk ${actualSdk}` : 'reported no sphere-sdk version'; + return `This app ${has} — the wallet requires ${requiredSdk} or newer. Upgrade @unicitylabs/sphere-sdk and rebuild.`; + } + const clientProtocol = text(data.clientProtocol); + const requiredProtocol = text(data.requiredProtocol); + if (clientProtocol && requiredProtocol) { + return `This app speaks Connect protocol ${clientProtocol} — the wallet requires ${requiredProtocol} or newer.`; + } + } + } + if (code === undefined) return message; return `${message} (code ${code})`; } diff --git a/nodejs/src/mockSphere.test.ts b/nodejs/src/mockSphere.test.ts new file mode 100644 index 0000000..2535f05 --- /dev/null +++ b/nodejs/src/mockSphere.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest'; +import { mockSphere } from './mockSphere'; + +/** + * The mock is advertised as "shaped like a real sphere-sdk 0.14 wallet", and a ConnectHost + * is built around it. Everything the host DEREFERENCES has to be present — a gap does not + * degrade gracefully, it throws inside the host the first time the path is taken. + * + * Today `mockSphere.on` is a no-op, so the event-compat paths never run and a gap stays + * invisible. These assertions are the thing that notices instead. + */ +describe('mockSphere implements what ConnectHost dereferences', () => { + it('exposes paymentsV2 as the same object as payments (the v2-wallet signal)', () => { + expect(mockSphere.paymentsV2).toBe(mockSphere.payments); + }); + + // connect/host/payments-compat.ts: `sphere.paymentsV2?.requests.list()` for the + // payment_request:paid / :rejected / :expired adapters, and `paymentsV2.tokens()` + // for sync:completed. The optional chain guards `paymentsV2`, NOT `requests`. + it.each(['assets', 'tokens', 'history', 'requests'])( + 'has facade member %s', + (member) => { + expect(mockSphere.paymentsV2[member as keyof typeof mockSphere.paymentsV2]).toBeDefined(); + }, + ); + + it.each(['list', 'create', 'pay', 'decline', 'dismissProcessed'])( + 'has requests.%s', + (member) => { + const requests = mockSphere.paymentsV2.requests as Record; + expect(typeof requests[member]).toBe('function'); + }, + ); + + it('survives the exact expression the payment_request compat adapter evaluates', () => { + const update = { id: 'preq-001', status: 'paid' }; + expect(() => + mockSphere.paymentsV2?.requests.list().find((request) => request.id === update.id), + ).not.toThrow(); + }); + + it('returns a listed request whose shape can rebuild the legacy payload', () => { + const [view] = mockSphere.paymentsV2.requests.list(); + for (const field of ['id', 'requestId', 'senderPubkey', 'amount', 'coinId', 'timestamp', 'status']) { + expect(view).toHaveProperty(field); + } + }); + + it('has the non-payments members the host reads', () => { + expect(mockSphere.identity.chainPubkey).toBeTruthy(); + expect(typeof mockSphere.resolve).toBe('function'); + expect(typeof mockSphere.on).toBe('function'); + expect(typeof mockSphere.communications.getConversations).toBe('function'); + }); +}); diff --git a/nodejs/src/mockSphere.ts b/nodejs/src/mockSphere.ts index baf0de2..812589e 100644 --- a/nodejs/src/mockSphere.ts +++ b/nodejs/src/mockSphere.ts @@ -5,12 +5,16 @@ * mock-wallet-server.ts, which starts a WebSocket server as a side effect of being imported. * * It mirrors the shape a REAL sphere-sdk 0.14 wallet hands to `ConnectHost`: `payments` is the - * payments-v2 facade (`assets()` / `tokens()` / `history()`), and `paymentsV2` is the same - * object — the deprecated alias the host reads to decide it is talking to a v2 wallet. + * payments-v2 facade (`assets()` / `tokens()` / `history()` / `requests`), and `paymentsV2` is + * the same object — the deprecated alias the host reads to decide it is talking to a v2 wallet. * - * The host only ever reads those three members plus `identity`, `resolve`, `on` and - * `communications`, so this mock implements exactly those. Money movement never reaches the - * facade in a Connect wallet: it arrives as an intent and is answered by `onIntent`. + * Money MOVEMENT never reaches the facade in a Connect wallet — it arrives as an intent and is + * answered by `onIntent`. Facade READS are a different matter: besides the four wire mappings + * below, the host's event-compat adapters read `requests.list()` and `tokens()` while rebuilding + * legacy event payloads. `mockSphere.on` is a no-op here, so no event can fire and those reads + * stay dormant — but a mock that omits what the host dereferences is a trap for the next person + * who gives it a real emitter, so the facade is implemented whole rather than to today's + * reachable subset. * * Wire mapping the host performs on top of this (dApps see it, so it's worth knowing): * sphere_getBalance / sphere_getAssets -> assets(coinId?) @@ -82,6 +86,30 @@ const payments = { more: false, cursor: null, }), + // NOT optional, despite money never reaching the facade in a Connect wallet: the host's + // payment_request compat adapter calls `sphere.paymentsV2?.requests.list()` to rebuild the + // legacy `IncomingPaymentRequest` payload whenever a `payment_request:updated` arrives. + // The optional chain stops at `paymentsV2`, so a missing `requests` is not a graceful + // degradation — it is `TypeError: Cannot read properties of undefined (reading 'list')` + // thrown inside ConnectHost. Processed requests stay listed until dismissProcessed(), + // which is why the adapter can still find a just-paid one here. + requests: { + list: () => [ + { + id: 'preq-001', requestId: 'preq-001', + senderPubkey: '03fedcba09876543210fedcba09876543210fedcba09876543210fedcba0987654321', + senderNametag: 'bob', amount: '250000000', coinId: 'UCT', symbol: 'UCT', + message: 'lunch', timestamp: now - 1800000, status: 'pending', + }, + ], + create: async (_to: string, _terms: { coinId: string; amount: string; memo?: string }) => ({ + success: true, + requestId: 'preq-002', + }), + pay: async (_id: string) => ({ id: 'xfer-preq-001', status: 'completed', deliveryPending: false }), + decline: async (_id: string) => {}, + dismissProcessed: () => {}, + }, }; export const mockSphere = {