From cf43d77e21acedb4747e53ee5a199eb195977e7a Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 11:29:21 +0200 Subject: [PATCH 01/27] feat(wallet-integration): rewrite on @icp-sdk/signer, integration only Replaces the @dfinity/oisy-wallet-signer material with @icp-sdk/signer, the relying-party client, and narrows the skill to integrating a signer. The "Wallet Side (Signer)" section, prompt registration and the ICRC-21 consent-message machinery are gone -- that is implementing a wallet, and consent rendering belongs to the wallet. Generic over ICRC-25 signers with OISY as the worked example: transport choice (ICRC-29 popup, ICRC-167 redirect, ICRC-94 extension discovery), capability negotiation, permissions and accounts, then the two interaction models -- per-action approval via SignerAgent (ICRC-49) and session delegation (ICRC-34). Two claims from the old skill are retracted rather than ported: - "concurrent requests return 503 BUSY" -- no such code in ICRC-25, and the library does not serialize. It was an oisy-only extension. - "not a session system / no ICRC-34" -- requestDelegation exists, so the premise the old When-NOT-to-Use section rested on is false. Every API call in the file typechecks against @icp-sdk/signer 6.0.0, @icp-sdk/core 6.1.0 and @icp-sdk/canisters 4.0.0 under strict with skipLibCheck disabled. --- skills/wallet-integration/SKILL.md | 608 +++++++++++++---------------- 1 file changed, 270 insertions(+), 338 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 33a26f83..a707824f 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -1,8 +1,8 @@ --- name: wallet-integration -description: "Integrate wallets with IC dApps using ICRC signer standards (ICRC-21/25/27/29/49). Covers the popup-based signer model, consent messages, permission lifecycle, and transaction approval flows. Implementation uses @dfinity/oisy-wallet-signer. Do NOT use for Internet Identity login, delegation-based auth (ICRC-34/46), or threshold signing (chain-key). Use when the developer mentions wallet integration, OISY, oisy-wallet-signer, wallet signer, relying party, consent messages, wallet popup, or transaction approval." +description: "Integrate an external wallet (signer) into an IC dapp with @icp-sdk/signer — the relying-party side of the ICRC signer standards. Covers picking a transport (popup via ICRC-29 / top-level redirect via ICRC-167 / browser extension via ICRC-94) and then negotiating capabilities and driving the permission and account lifecycle. Shows both interaction models: per-action approval through SignerAgent (ICRC-49) and session delegation (ICRC-34). Uses OISY as the worked example but applies to any ICRC-25 signer. Do NOT use for Internet Identity login (use the internet-identity skill) or for implementing a wallet yourself. Use when the developer mentions wallet integration or OISY or @icp-sdk/signer or approving a transaction in a wallet popup." license: Apache-2.0 -compatibility: "Node.js >= 22" +compatibility: "Node.js >= 22, a browser (secure context: HTTPS, localhost, or 127.0.0.1)" metadata: title: Wallet Integration category: DeFi @@ -12,444 +12,376 @@ metadata: ## What This Is -Wallet integration on the Internet Computer uses the ICRC signer standards — a popup-based model where every action requires explicit user approval via JSON-RPC 2.0 over `window.postMessage`. +Connecting an **external wallet** to your dapp so the user approves actions in the wallet rather than handing your app a key. `@icp-sdk/signer` is the **relying-party** client: your app is the relying party, the wallet is the signer, and they exchange JSON-RPC 2.0 messages over a transport defined by the ICRC signer standards. -This skill covers integration using `@dfinity/oisy-wallet-signer`. Other integration paths (IdentityKit, signer-js) exist but are not covered here. +This skill covers **integrating a signer**. Implementing one is out of scope — consent screens, prompt registration and account custody are the wallet's job. -**The signer model = explicit per-action approval.** `connect()` establishes a channel. Nothing more. +Examples use [OISY](https://oisy.com) (`https://oisy.com/sign`), but nothing here is OISY-specific: any ICRC-25 signer works by swapping the transport URL, and [`BrowserExtensionTransport`](#extension-icrc-94) discovers extension signers you never hardcoded. -**It is not:** +| Standard | What it gives you | API | +|----------|-------------------|-----| +| ICRC-25 | Capability discovery + permission lifecycle | `getSupportedStandards`, `requestPermissions`, `getPermissions` | +| ICRC-27 | The user's accounts | `getAccounts` | +| ICRC-29 | Popup transport over `postMessage` | `PostMessageTransport` | +| ICRC-34 | Session delegation | `requestDelegation` | +| ICRC-49 | Execute a canister call | `callCanister`, `SignerAgent` | +| ICRC-94 | Browser-extension discovery | `BrowserExtensionTransport.discover` | +| ICRC-95 | Identity derivation origin | `derivationOrigin` option | +| ICRC-167 | Top-level redirect transport | `UrlTransport` (**new in signer 6**) | -- A session system -- A delegated identity (no ICRC-34) -- A background executor +## Choose an interaction model first -**ICRC standards implemented:** +Everything downstream follows from this choice. -- ICRC-21 — Canister call consent messages -- ICRC-25 — Signer interaction standard (permissions) -- ICRC-27 — Accounts -- ICRC-29 — Window PostMessage transport -- ICRC-49 — Call canister +| | **Path A — per-action approval** (ICRC-49) | **Path B — session delegation** (ICRC-34) | +|---|---|---| +| The user approves | every write, individually | once, at sign-in | +| Your app holds | no key — the wallet signs | a session key the wallet delegated to | +| You end up with | a `SignerAgent` | an ordinary `HttpAgent` + `DelegationIdentity` | +| Good for | transfers, approvals, mints — deliberate, high-value acts | games, social, frequent writes, background work | +| Bad for | anything frequent; each call is a popup | acts a user should consciously confirm | -**Not implemented:** +A signer may support one and not the other — negotiate before you commit (see below). You can also combine them: a delegation for your own canister, per-action approval for token transfers. -- ICRC-46 — Session-based delegation (not supported; use a delegation-capable model if you need sessions) +### When NOT to use this skill -## When to Use - -- Clear, intentional, high-value actions: token transfers (ICP / ICRC-1 / ICRC-2), NFT mint/claim, single approvals -- Funding / deposit flows: "Top up", "Deposit into protocol" -- Any action where a confirmation dialogue per operation feels natural - -## When NOT to Use - -- **Delegation or sessions**: sign once / act many times, background execution, autonomous behaviour -- **High-frequency interactions**: games, social actions, rapid write operations -- **Invisible writes**: autosave, cron jobs, auto-compounding - -> **Decision test:** If your app still feels good when every meaningful update shows a confirmation dialogue, this library is appropriate. If not, use a delegation-capable model instead. +- **Internet Identity sign-in** → the **internet-identity** skill. II is an identity provider, not an ICRC-25 signer. +- **Letting an agent or CLI act as the user** → the **agent-web-identity** skill. +- **Building a wallet** → out of scope, as above. ## Prerequisites -- `@dfinity/oisy-wallet-signer` (>= 4.1.0) -- Peer dependencies: `@dfinity/utils` (>= 4.2.0), `@dfinity/zod-schemas` (>= 3.2.0), `@icp-sdk/canisters` (>= 3.5.0), `@icp-sdk/core` (>= 5.0.0), `zod` -- A non-anonymous identity on the signer side (e.g. `Ed25519KeyIdentity`) - ```bash -npm i @dfinity/oisy-wallet-signer @dfinity/utils @dfinity/zod-schemas @icp-sdk/canisters @icp-sdk/core zod +npm i '@icp-sdk/signer@^6' '@icp-sdk/core@^6' ``` -## How It Works +Add `@icp-sdk/canisters@^4` if you call ICP/ICRC ledgers (it brings `@dfinity/utils@^5` as a peer). Pin `@icp-sdk/core` to `^6`: signer 6 peers it, and so do `@icp-sdk/auth@^10` and `@icp-sdk/canisters@^4`. -### End-to-End Lifecycle +The transport URL must be a **secure context** — HTTPS, `localhost`, or `127.0.0.1`. -```text -1. dApp: IcrcWallet.connect({url}) → opens popup, polls icrc29_status -2. dApp: wallet.requestPermissionsNotGranted() → prompts user if needed -3. dApp: wallet.accounts() → signer prompts, returns accounts -4. dApp: wallet.transfer({...}) → signer fetches ICRC-21 consent message - → signer prompts user with consent - → signer executes canister call - → returns block index -5. dApp: wallet.disconnect() → closes popup, cleans up -``` +## Pick a transport -## Pitfalls +The transport is the only part that knows *how* the wallet is reached. The `Signer` API above it is identical either way. -1. **Importing classes from the wrong entry point.** `Signer`, `RelyingParty`, `IcpWallet`, and `IcrcWallet` are **not** exported from the main entry point. Import them from their dedicated subpaths or you get `undefined`. +| Transport | Standard | Mechanism | Use when | +|-----------|----------|-----------|----------| +| `PostMessageTransport` | ICRC-29 | Opens a popup, handshakes with `icrc29_status`, then `postMessage` | Default for web wallets like OISY | +| `UrlTransport` | ICRC-167 | Navigates the top-level window; wallet returns to your `callbackUrl` | Mobile, or anywhere popups are blocked | +| `BrowserExtensionTransport` | ICRC-94 | Extensions announce themselves on `window` events | Extension wallets; discovering unknown signers | - ```typescript - // WRONG — will fail - import {Signer} from '@dfinity/oisy-wallet-signer'; +```typescript +import { Signer } from '@icp-sdk/signer'; +import { PostMessageTransport } from '@icp-sdk/signer/web'; - // CORRECT - import {Signer} from '@dfinity/oisy-wallet-signer/signer'; - import {IcpWallet} from '@dfinity/oisy-wallet-signer/icp-wallet'; - import {IcrcWallet} from '@dfinity/oisy-wallet-signer/icrc-wallet'; - ``` +// One Signer per wallet connection; safe to create at module scope. +const signer = new Signer({ + transport: new PostMessageTransport({ url: 'https://oisy.com/sign' }) +}); +``` -2. **Using `IcrcWallet` without `ledgerCanisterId`.** Unlike `IcpWallet` (which defaults to the ICP ledger `ryjl3-tyaaa-aaaaa-aaaba-cai`), `IcrcWallet.transfer()`, `.approve()`, and `.transferFrom()` all **require** `ledgerCanisterId`. Omitting it causes a runtime error. +### Extension (ICRC-94) -3. **Forgetting to register prompts on the signer side.** The signer returns error 501 (`PERMISSIONS_PROMPT_NOT_REGISTERED`) if a request arrives and no prompt handler is registered for it. Register all four prompts (`ICRC25_REQUEST_PERMISSIONS`, `ICRC27_ACCOUNTS`, `ICRC21_CALL_CONSENT_MESSAGE`, `ICRC49_CALL_CANISTER`) before the signer can handle any relying party traffic. +```typescript +import { BrowserExtensionTransport } from '@icp-sdk/signer/extension'; -4. **Sending concurrent requests to the signer.** The signer processes one request at a time. A second request while one is in-flight returns error 503 (`BUSY`). Serialize your calls — wait for each response before sending the next. Read-only methods (`icrc29_status`, `icrc25_supported_standards`) are exempt. +async function pickExtensionSigner() { + // Each provider carries { uuid, name, icon, rdns } — enough to render a picker. + const providers = await BrowserExtensionTransport.discover(); + if (providers.length === 0) return null; -5. **Assuming `connect()` = authenticated session.** `connect()` only opens a `postMessage` channel. The user has not pre-authorized anything. Permissions default to `ask_on_use` — the signer will prompt the user on first use of each method. Call `requestPermissionsNotGranted()` after connecting to request all permissions upfront in a single prompt instead of per-method prompts. + const transport = await BrowserExtensionTransport.findTransport({ uuid: providers[0].uuid }); + return new Signer({ transport }); +} +``` -6. **Not handling the consent message state machine.** The `ICRC21_CALL_CONSENT_MESSAGE` prompt fires multiple times with different statuses: `loading` → `result` | `error`. If you only handle `result`, the UI breaks on loading and error states. Always branch on `payload.status`. +### Redirect (ICRC-167) -7. **`sender` not matching `owner`.** The signer validates that `sender` in every `icrc49_call_canister` request matches the signer's `owner` identity. A mismatch returns error 502 (`SENDER_NOT_ALLOWED`). Always use the `owner` from `accounts()`. +`UrlTransport` unloads your page on every request, so it keeps a call-order journal in `sessionStorage` and replays it when the wallet returns. Two rules make or break it: -8. **Not calling `disconnect()`.** Both `Signer.disconnect()` and `wallet.disconnect()` must be called on clean-up. Forgetting this leaks event listeners and leaves popup windows open. +1. **Issue the same requests, in the same order, on every load.** Branch only on values recovered from earlier results. A divergence guard rejects a replay that does not match. +2. **`memoize()` is the only place a flow may await anything that is not a signer request.** Its result is journaled, so a value stays stable across the redirect. -9. **Ignoring permission expiration.** Permissions default to a 7-day validity period. After expiry, they silently revert to `ask_on_use`. Don't cache permission state client-side beyond a session. +```typescript +import { Signer } from '@icp-sdk/signer'; +import { UrlTransport } from '@icp-sdk/signer/web'; +import { DelegationIdentity, Ed25519KeyIdentity } from '@icp-sdk/core/identity'; +import type { Principal } from '@icp-sdk/core/principal'; + +const transport = new UrlTransport({ + url: 'https://id.ai/icrc-167', + // Absolute, fragment-free, on an origin you control, and listed in that + // origin's /.well-known/ii-auth-callbacks allow-list. + callbackUrl: 'https://app.example.com/signer-callback' +}); -10. **Auto-triggering signing on connect.** Never fire a canister call immediately after `connect()`. Let the user initiate the action. The signer is designed for intentional, user-driven operations. +// Run this on the load of the callback route: a fresh arrival starts the flow, +// the wallet's return replays it. No separate resume or cleanup call. +async function runRedirectFlow(backend: Principal) { + const signer = new Signer({ transport }); + + // The session key must survive the redirect, so journal it. Ed25519KeyIdentity + // is used because toJSON() is JSON-serializable; ECDSAKeyIdentity holds + // CryptoKeys that are not (see pitfall 6). + const sessionKeyJson = await transport.memoize(() => + JSON.stringify(Ed25519KeyIdentity.generate().toJSON()) + ); + const sessionKey = Ed25519KeyIdentity.fromJSON(sessionKeyJson); + + const chain = await signer.requestDelegation({ + publicKey: sessionKey.getPublicKey(), + targets: [backend] + }); + return DelegationIdentity.fromDelegation(sessionKey, chain); +} +``` -## Implementation +## Negotiate capabilities -### Import Map +Skip this only if you hardcode one wallet and know what it supports. For generic integration it is the step that keeps you honest — ICRC-34 and ICRC-49 are independent, and a signer may offer either, both, or neither. ```typescript -// Constants, errors, and types — from main entry point -import { - ICRC25_REQUEST_PERMISSIONS, - ICRC25_PERMISSION_GRANTED, - ICRC25_PERMISSION_DENIED, - ICRC25_PERMISSION_ASK_ON_USE, - ICRC27_ACCOUNTS, - ICRC21_CALL_CONSENT_MESSAGE, - ICRC49_CALL_CANISTER, - DEFAULT_SIGNER_WINDOW_CENTER, - DEFAULT_SIGNER_WINDOW_TOP_RIGHT, - RelyingPartyResponseError, - RelyingPartyDisconnectedError -} from '@dfinity/oisy-wallet-signer'; - -import type { - PermissionsPromptPayload, - AccountsPromptPayload, - ConsentMessagePromptPayload, - CallCanisterPromptPayload, - IcrcAccounts, - SignerOptions, - RelyingPartyOptions -} from '@dfinity/oisy-wallet-signer'; - -// Classes — from dedicated subpaths -import {Signer} from '@dfinity/oisy-wallet-signer/signer'; -import {RelyingParty} from '@dfinity/oisy-wallet-signer/relying-party'; -import {IcpWallet} from '@dfinity/oisy-wallet-signer/icp-wallet'; -import {IcrcWallet} from '@dfinity/oisy-wallet-signer/icrc-wallet'; +async function capabilities(signer: Signer) { + const standards = await signer.getSupportedStandards(); // [{ name: 'ICRC-27', url }, ...] + const names = new Set(standards.map(({ name }) => name)); + return { + canCallCanisters: names.has('ICRC-49'), // Path A + canDelegate: names.has('ICRC-34') // Path B + }; +} ``` -### dApp Side (Relying Party) +`getSupportedStandards` needs no permission, so it is safe as a first call. + +## Permissions and accounts + +Permissions default to `ask_on_use`: the wallet prompts the first time each method is used. `requestPermissions` is **optional** — it trades several later prompts for one up front. -#### Choosing the Right Class +| State | Behaviour | +|-------|-----------| +| `granted` | Proceeds without prompting | +| `denied` | Rejected immediately with error `3000` | +| `ask_on_use` | Prompts on first use (the default) | -| Class | Use for | -| -------------- | ---------------------------------------------------------------------------- | -| `IcpWallet` | ICP ledger operations — `ledgerCanisterId` optional (defaults to ICP ledger) | -| `IcrcWallet` | Any ICRC ledger — `ledgerCanisterId` **required** | -| `RelyingParty` | Low-level custom canister calls via protected `call()` | +```typescript +// Optional: ask once, up front, instead of per method. +await signer.requestPermissions([ + { method: 'icrc27_accounts' }, + { method: 'icrc49_call_canister' } +]); + +const accounts = await signer.getAccounts(); +const account = accounts[0].owner; // a Principal, already decoded +const subaccount = accounts[0].subaccount; // Uint8Array | undefined +``` + +`getPermissions()` reads the current state without prompting. Do not cache it across sessions — a wallet may expire grants, after which they silently revert to `ask_on_use`. -#### Connect, Permissions, Accounts +## Path A — per-action approval -All wallet operations are async. Wrap them in functions — do not use top-level `await`, which fails with Vite's default `es2020` build target. +`SignerAgent` implements `Agent`, so it drops into anything that takes one: a ledger client from `@icp-sdk/canisters`, or an actor from `@icp-sdk/bindgen` for your own canister. Each call becomes a wallet prompt. ```typescript -// Wrapping in an async function avoids top-level await, which requires -// build.target >= es2022. This works with any bundler target. -async function connectWallet() { - const wallet = await IcrcWallet.connect({ - url: 'https://your-wallet.example.com/sign', // URL of the wallet implementing the signer - host: 'https://icp-api.io', - windowOptions: {width: 576, height: 625, position: 'center'}, - connectionOptions: {timeoutInMilliseconds: 120_000}, - onDisconnect: () => { - /* wallet popup closed */ - } - }); +import { SignerAgent } from '@icp-sdk/signer/agent'; +import { IcrcLedgerCanister } from '@icp-sdk/canisters/ledger/icrc'; +import { HttpAgent } from '@icp-sdk/core/agent'; +import { Principal } from '@icp-sdk/core/principal'; + +const ICP_LEDGER = Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'); - const {allPermissionsGranted} = await wallet.requestPermissionsNotGranted(); +async function transfer(signer: Signer, account: Principal, to: Principal, amount: bigint) { + // Reuse one HttpAgent for root key + status so SignerAgent does not build its own. + const agent = await HttpAgent.create({ host: 'https://icp-api.io' }); + const signerAgent = await SignerAgent.create({ signer, account, agent }); - const accounts = await wallet.accounts(); - const {owner} = accounts[0]; - return {wallet, owner}; + const ledger = IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ICP_LEDGER }); + return ledger.transfer({ to: { owner: to, subaccount: [] }, amount }); // → block index } ``` -#### IcpWallet — ICP Transfers and Approvals - -Uses `{owner, request}` — no `ledgerCanisterId` needed. +**Read with a plain agent, write with the signer agent.** `SignerAgent.query()` upgrades every query into a full canister call routed through the wallet — so a balance check becomes a user prompt and costs cycles. Build two clients against the same ledger: ```typescript -async function icpWalletTransfers() { - const wallet = await IcpWallet.connect({url: 'https://your-wallet.example.com/sign'}); - const accounts = await wallet.accounts(); - const {owner} = accounts[0]; - - await wallet.icrc1Transfer({ - owner, - request: {to: {owner: recipientPrincipal, subaccount: []}, amount: 100_000_000n} - }); +// Reads: anonymous agent, no prompt, no cycles. +const readLedger = IcrcLedgerCanister.create({ agent, canisterId: ICP_LEDGER }); +const balance = await readLedger.balance({ owner: account }); - await wallet.icrc2Approve({ - owner, - request: {spender: {owner: spenderPrincipal, subaccount: []}, amount: 500_000_000n} - }); -} +// Writes: signer agent, one prompt per call. +const writeLedger = IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ICP_LEDGER }); ``` -#### IcrcWallet — Any ICRC Ledger +`signerAgent.replaceAccount(principal)` switches the account for later calls without rebuilding the agent. -Uses `{owner, ledgerCanisterId, params}` — `ledgerCanisterId` is **required**. +## Path B — session delegation -```typescript -async function icrcWalletTransfers() { - const wallet = await IcrcWallet.connect({url: 'https://your-wallet.example.com/sign'}); - const accounts = await wallet.accounts(); - const {owner} = accounts[0]; - - await wallet.transfer({ - owner, - ledgerCanisterId: 'mxzaz-hqaaa-aaaar-qaada-cai', - params: {to: {owner: recipientPrincipal, subaccount: []}, amount: 1_000_000n} - }); +The wallet delegates to a key your app generates. Afterwards you hold an ordinary `HttpAgent` and the wallet is not involved again until the delegation expires. - await wallet.approve({ - owner, - ledgerCanisterId: 'mxzaz-hqaaa-aaaar-qaada-cai', - params: {spender: {owner: spenderPrincipal, subaccount: []}, amount: 5_000_000n} +```typescript +import { HttpAgent } from '@icp-sdk/core/agent'; +import { DelegationIdentity, ECDSAKeyIdentity } from '@icp-sdk/core/identity'; + +async function startSession(signer: Signer, backend: Principal) { + // Non-extractable keys cannot be exfiltrated; prefer ECDSA when you do not + // need to serialize the key (see pitfall 6 for the redirect case). + const sessionKey = await ECDSAKeyIdentity.generate(); + + const chain = await signer.requestDelegation({ + publicKey: sessionKey.getPublicKey(), + // Scope it. Omitting targets asks for a delegation valid for ANY canister. + targets: [backend], + maxTimeToLive: BigInt(8) * BigInt(3_600_000_000_000) // 8 hours, in nanoseconds }); - await wallet.transferFrom({ - owner, - ledgerCanisterId: 'mxzaz-hqaaa-aaaar-qaada-cai', - params: {from: {owner: fromPrincipal, subaccount: []}, to: {owner: toPrincipal, subaccount: []}, amount: 1_000_000n} - }); + const identity = DelegationIdentity.fromDelegation(sessionKey, chain); + return HttpAgent.create({ identity }); } ``` -#### Query Methods and Disconnect +`requestDelegation` **validates the wallet's response before returning** and throws if the chain does not terminate at your public key, if `targets` come back broader than requested, or if it outlives `maxTimeToLive`. Do not reimplement those checks, and do not swallow the throw — it is the guard against a malicious or buggy signer widening your delegation. -```typescript -async function queryAndDisconnect(wallet: IcrcWallet) { - const standards = await wallet.supportedStandards(); - const currentPermissions = await wallet.permissions(); +## Channel lifecycle and page reloads - await wallet.disconnect(); +`autoCloseTransportChannel` defaults to `true`: the channel closes ~200 ms after each response, so the popup does not linger. For a multi-step flow that awaits your own async work between requests, turn it off or the channel closes underneath you. + +```typescript +signer.autoCloseTransportChannel = false; +try { + const accounts = await signer.getAccounts(); + await recordSomethingOnYourServer(accounts); // channel stays open + await transfer(/* ... */); +} finally { + signer.autoCloseTransportChannel = true; + await signer.closeChannel(); } ``` -#### Error Handling (dApp Side) +**A connection does not survive a page reload.** There is no persistent session to restore — the channel is a live `postMessage` link to a popup that is gone. The workable pattern is to persist only the principal, render read-only state from it with an anonymous agent, and re-establish the signer lazily on the first write: ```typescript -async function safeTransfer(wallet: IcrcWallet) { - try { - await wallet.transfer({...}); - } catch (err) { - if (err instanceof RelyingPartyResponseError) { - switch (err.code) { - case 3000: /* PERMISSION_NOT_GRANTED */ break; - case 3001: /* ACTION_ABORTED — user rejected */ break; - case 4000: /* NETWORK_ERROR */ break; - } - } - if (err instanceof RelyingPartyDisconnectedError) { - /* popup closed unexpectedly */ - } - } -} -``` +const SESSION_KEY = 'wallet-principal'; -### Wallet Side (Signer) +// On connect: remember who, not the channel. +sessionStorage.setItem(SESSION_KEY, account.toText()); -#### Initialise and Register All Prompts +// On reload: balances and history render immediately, with no popup. +const stored = sessionStorage.getItem(SESSION_KEY); +const account = stored ? Principal.fromText(stored) : null; -```typescript -const signer = Signer.init({ - owner: identity, - host: 'https://icp-api.io', - sessionOptions: { - sessionPermissionExpirationInMilliseconds: 7 * 24 * 60 * 60 * 1000 - } -}); +// On the first write after a reload: this reopens the popup briefly. +async function ensureSignerAgent(signer: Signer, account: Principal, agent: HttpAgent) { + await signer.getAccounts(); // re-establishes the channel + return SignerAgent.create({ signer, account, agent }); +} +``` -signer.register({ - method: ICRC25_REQUEST_PERMISSIONS, - prompt: ({requestedScopes, confirm, origin}: PermissionsPromptPayload) => { - confirm( - requestedScopes.map(({scope}) => ({ - scope, - state: userApproved ? ICRC25_PERMISSION_GRANTED : ICRC25_PERMISSION_DENIED - })) - ); - } -}); +Treat "disconnect" as clearing your own state — there is no wallet-side logout to call. -signer.register({ - method: ICRC27_ACCOUNTS, - prompt: ({approve, reject, origin}: AccountsPromptPayload) => { - approve([{owner: identity.getPrincipal().toText()}]); - } -}); +## Error handling -signer.register({ - method: ICRC21_CALL_CONSENT_MESSAGE, - prompt: (payload: ConsentMessagePromptPayload) => { - if (payload.status === 'loading') { - // show spinner - } else if (payload.status === 'result') { - // payload.consentInfo: { Ok: ... } (from canister) or { Warn: ... } (signer-generated fallback) - // show consent UI, then: payload.approve() or payload.reject() - } else if (payload.status === 'error') { - // show error, optionally payload.details +```typescript +import { Signer, SignerError } from '@icp-sdk/signer'; +import { PostMessageTransportError } from '@icp-sdk/signer/web'; + +try { + await transfer(/* ... */); +} catch (err) { + if (err instanceof SignerError) { + switch (err.code) { + case 3001: return; // user cancelled — not a failure + case 3000: showPermissionHelp(); return; // permission denied + case 2000: showUnsupported(); return; // wallet does not support the method + case 4001: promptReconnect(); return; // channel closed + default: throw err; } } -}); - -signer.register({ - method: ICRC49_CALL_CANISTER, - prompt: (payload: CallCanisterPromptPayload) => { - if (payload.status === 'executing') { - /* show progress */ - } else if (payload.status === 'result') { - /* call succeeded */ - } else if (payload.status === 'error') { - /* call failed */ - } + if (err instanceof PostMessageTransportError) { + // Popup blocked, or the ICRC-29 handshake timed out. + promptReconnect(); + return; } -}); - + throw err; +} ``` -#### Consent Message: `Ok` vs `Warn` +These are the ICRC-25 codes — the only ones portable across wallets: -- `{ Ok: consentInfo }` — canister implements ICRC-21; message is canister-verified -- `{ Warn: { consentInfo, canisterId, method, arg } }` — signer generated a fallback (for `icrc1_transfer`, `icrc2_approve`, `icrc2_transfer_from`) +| Code | Meaning | Handle by | +|------|---------|-----------| +| `1000` | Generic error | Surfacing `err.data` to developers | +| `2000` | Not supported | Negotiating capabilities first | +| `3000` | Permission not granted | Explaining what to re-grant | +| `3001` | **Action aborted — the user cancelled** | Returning quietly; this is normal | +| `4000` | Network error | Retrying | +| `4001` | Transport channel closed | Reconnecting | -Always distinguish these in the UI — warn the user when the message is signer-generated. +Transport-level failures arrive as `PostMessageTransportError`, `UrlTransportError`, `BrowserExtensionTransportError`, or `SignerAgentError` — not as `SignerError`, because no wallet response was involved. -#### Disconnect +## Pitfalls -```typescript -signer.disconnect(); -``` +1. **Opening the popup outside a click handler.** `PostMessageTransport` rejects establishment that is not user-initiated (`detectNonClickEstablishment`, default `true`) because Safari and others block such popups. Connect from an event handler, never on mount or in a `useEffect`. -### Error Code Reference + ```typescript + // WRONG — blocked, and the transport detects it + useEffect(() => { signer.getAccounts(); }, []); -| Code | Name | Meaning | -| ---- | ----------------------------------- | --------------------------- | -| 500 | `ORIGIN_ERROR` | Origin mismatch | -| 501 | `PERMISSIONS_PROMPT_NOT_REGISTERED` | Missing prompt handler | -| 502 | `SENDER_NOT_ALLOWED` | `sender` ≠ `owner` | -| 503 | `BUSY` | Concurrent request rejected | -| 504 | `NOT_INITIALIZED` | Owner identity not set | -| 1000 | `GENERIC_ERROR` | Catch-all | -| 2000 | `REQUEST_NOT_SUPPORTED` | Method not supported | -| 3000 | `PERMISSION_NOT_GRANTED` | Permission denied | -| 3001 | `ACTION_ABORTED` | User cancelled | -| 4000 | `NETWORK_ERROR` | IC call failure | + // CORRECT + button.addEventListener('click', () => signer.getAccounts()); + ``` -### Permission States +2. **Reading through `SignerAgent`.** `query()` is upgraded to an update call routed through the wallet, so every read prompts the user and costs cycles. Reads go through a plain `HttpAgent`; only writes go through `SignerAgent`. -| State | Constant | Behavior | -| ---------- | ------------------------------ | --------------------------------- | -| Granted | `ICRC25_PERMISSION_GRANTED` | Proceeds without prompting | -| Denied | `ICRC25_PERMISSION_DENIED` | Rejected immediately (error 3000) | -| Ask on use | `ICRC25_PERMISSION_ASK_ON_USE` | Prompts user on access (default) | +3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the principal for read-only rendering and reconnect on first write — see above. -Permissions stored in `localStorage` as `oisy_signer_{origin}_{owner}` with timestamps. Default validity: 7 days. +4. **Assuming a wallet's capabilities.** Call `getSupportedStandards()`. A wallet that executes canister calls (ICRC-49) may not issue delegations (ICRC-34), and vice versa. -## Deploy & Test +5. **Coding against one wallet's non-standard error codes.** Only `1000`/`2000`/`3000`/`3001`/`4000`/`4001` are ICRC-25. Vendor extensions outside that range are not portable — earlier revisions of this skill documented a `503 BUSY` code that exists only in `@dfinity/oisy-wallet-signer` and in no standard. Branch on the standard codes and treat the rest as generic. -### Local Development — Your Own Signer +6. **Journaling a non-serializable session key through `memoize()`.** `memoize` persists via JSON, so `ECDSAKeyIdentity` cannot cross a redirect — its `getKeyPair()` returns `CryptoKey`s that `JSON.stringify` silently reduces to `{}`, and the flow fails on return with a key it cannot sign with. Use `Ed25519KeyIdentity` (`toJSON`/`fromJSON`) for `UrlTransport` flows; prefer non-extractable `ECDSAKeyIdentity` for popup flows, where nothing needs serializing. -If you are building both the dApp and the wallet/signer, start a local network and pass `host` to both sides: +7. **Requesting an unscoped delegation.** Omitting `targets` asks for a delegation valid against *any* canister. Always pass the canisters you actually call. -```bash -icp network start -d -``` +8. **Diverging on a redirect replay.** With `UrlTransport`, issue the same requests and `memoize` steps in the same order on every load, and route anything a request depends on — a nonce above all — through `memoize`. Re-fetching a single-use value on the return load invalidates the flow. -```typescript -// dApp side — point to your local wallet's /sign route -async function connectLocalWallet() { - const wallet = await IcrcWallet.connect({ - url: 'http://localhost:5174/sign', - host: 'http://localhost:8000' - }); - return wallet; -} +9. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. -// Wallet/signer side — same local network host -const signer = Signer.init({ - owner: identity, - host: 'http://localhost:8000' -}); -``` +10. **Top-level `await` in wallet code.** Every call here is async. Vite's default `es2020` target rejects top-level `await`; wrap calls in functions rather than raising `build.target`. -### Local Development — Using the Pseudo Wallet Signer +11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5`, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it installs two copies of core, which degrades silently instead of failing. -If you are building a dApp (relying party) and need a signer to test against locally, the library provides a pseudo wallet signer in its demo: +12. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. -```bash -git clone https://github.com/dfinity/oisy-wallet-signer -cd oisy-wallet-signer -npm ci - -cd demo -npm ci -npm run sync:all -npm run dev:wallet # starts the pseudo wallet on port 5174 -``` +## Testing against a real wallet -Then connect from your dApp: +There is no local signer to run: OISY is a hosted wallet, and a locally deployed frontend can talk to it because `localhost` is a secure context. Test on testnet tokens rather than mainnet value. -```typescript -async function connectPseudoWallet() { - const wallet = await IcpWallet.connect({ - url: 'http://localhost:5174/sign', - host: 'http://localhost:8000' // match your local network port - }); - return wallet; -} +```bash +icp network start -d +icp deploy ``` -### Mainnet +Get free testnet tokens from the [ICP Faucet](https://faucet.internetcomputer.org) and switch OISY to the **IC (testnet tokens)** network to see them. Useful ledgers: -On mainnet, point to the wallet's production signer URL and omit `host` (defaults to `https://icp-api.io`): +| Token | Ledger canister | +|-------|-----------------| +| TESTICP | `xafvr-biaaa-aaaai-aql5q-cai` | +| TICRC1 | `3jkp5-oyaaa-aaaaj-azwqa-cai` | -```typescript -async function connectMainnetWallet() { - const wallet = await IcpWallet.connect({ - url: 'https://your-wallet.example.com/sign' - }); - return wallet; -} -``` +Set `host: 'https://icp-api.io'` on the agent even when serving from `localhost` — `host` is the API endpoint calls go to, not the origin your app is served from. ## Expected Behavior -### Connection - -- `connect()` resolves with a wallet instance; throws `RelyingPartyDisconnectedError` on timeout -- `wallet.supportedStandards()` returns an array containing at least ICRC-21, ICRC-25, ICRC-27, ICRC-29, ICRC-49 - -### Permissions - -- `requestPermissionsNotGranted()` triggers the signer's permissions prompt -- After approval, `wallet.permissions()` returns scopes with state `granted` -- A second call returns `{allPermissionsGranted: true}` without prompting again - -### Accounts - -- `wallet.accounts()` returns at least one `{owner: string}` (principal as text) -- The returned `owner` matches the signer's identity principal - -### Transfers and Approvals - -- `icrc1Transfer()` / `transfer()`, `icrc2Approve()` / `approve()`, and `transferFrom()` all resolve with a `bigint` block index -- Each triggers the consent message prompt on the signer before execution - +- `getSupportedStandards()` resolves without a prompt and lists at least ICRC-25 and the transport's own standard. +- The first `getAccounts()` opens the wallet, the user approves, and it resolves with one or more `{ owner: Principal, subaccount?: Uint8Array }`. +- With `autoCloseTransportChannel` at its default, the popup closes shortly after each response. +- A ledger `transfer` through `SignerAgent` prompts once and resolves with a `bigint` block index. +- Cancelling any prompt rejects with `SignerError` and `code === 3001`. +- `requestDelegation` resolves with a `DelegationChain`, or throws if the wallet returned one broader or longer-lived than requested. +- After a reload, read-only state renders with no popup; the first write reopens one. + +## Additional References + +- **internet-identity** — II sign-in, delegation-based auth for your own app +- **agent-web-identity** — letting an agent or CLI act as the user in an II app +- **icp-cli** — `@icp-sdk/bindgen` actors to call your own canister through a `SignerAgent` +- **canister-security** — verifying `msg.caller` on the backend once calls arrive +- [OISY signer demo](https://github.com/dfinity/examples/tree/master/hosting/oisy-signer-demo) — a working relying party (React) built on `@icp-sdk/signer` +- [ICRC signer standards](https://github.com/dfinity/wg-identity-authentication) — the specifications behind every method above From 05a56e9d6b2d566ab79abb17c4708320305a064e Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 11:29:22 +0200 Subject: [PATCH 02/27] test(wallet-integration): replace the oisy-specific eval suite All four previous cases tested @dfinity/oisy-wallet-signer specifics (IcpWallet vs IcrcWallet, signer-side implementation) and no longer describe the skill. Six cases replace them, weighted toward what an agent gets wrong unaided: library choice, reads through SignerAgent, popup on mount, interaction-model selection, the canisters ^3/^4 split, and reconnect-after-reload. --- evaluations/wallet-integration.json | 75 +++++++++++++++++------------ 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 429bb131..18039182 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -1,64 +1,79 @@ { "skill": "wallet-integration", - "description": "Evaluation cases for the wallet-integration skill. Tests whether agents produce correct ICRC signer protocol code, avoid top-level await, and use the right wallet classes.", - + "description": "Evaluation cases for the wallet-integration skill. Tests whether agents integrate an external signer with @icp-sdk/signer correctly: the right library and pins, reads off the SignerAgent, user-initiated popups, the right interaction model, and reconnect-after-reload.", "output_evals": [ { - "name": "No top-level await in wallet code", - "prompt": "Show me just the JavaScript code to connect an ICRC wallet and make a single token transfer. I'm using Vite with default settings. Keep it minimal — no signer-side code, no deploy steps.", + "name": "Adversarial: reaches for the superseded oisy library", + "prompt": "Give me the npm install command to add OISY wallet support to my IC dapp. No integration code.", "expected_behaviors": [ - "All await calls are inside async functions — no bare top-level await at module scope", - "Uses IcrcWallet or IcpWallet connect pattern", - "Shows wallet.transfer or wallet.icrc1Transfer inside an async function", - "Does NOT recommend changing build.target to 'esnext' or 'es2022' in Vite config" + "Installs @icp-sdk/signer as the client library", + "Pins @icp-sdk/core to ^6", + "Does NOT recommend @dfinity/oisy-wallet-signer" ] }, { - "name": "IcpWallet vs IcrcWallet selection", - "prompt": "I want to send ICP tokens from my frontend using a wallet. Which class should I use? Just the class name, import path, and a one-line explanation of when to use each.", + "name": "Adversarial: reading a balance through SignerAgent", + "prompt": "I connected OISY with @icp-sdk/signer and built a SignerAgent. Show me how to read the user's ICRC-1 balance and how to send a transfer — just those two calls, no connection or setup boilerplate.", "expected_behaviors": [ - "Recommends IcpWallet for ICP ledger operations", - "Explains that IcpWallet does not require ledgerCanisterId (defaults to ICP ledger)", - "Explains that IcrcWallet is for any ICRC ledger and requires ledgerCanisterId", - "Shows the correct import from 'oisy-wallet-signer'" + "Reads the balance through a plain HttpAgent, NOT through the SignerAgent", + "Explains that SignerAgent turns a query into a full canister call routed through the wallet, so a read would prompt the user and cost cycles", + "Sends the transfer through the SignerAgent" ] }, { - "name": "Error handling pattern", - "prompt": "How do I handle errors when the user rejects a wallet transaction? Just show the try/catch pattern with the relevant error types.", + "name": "Adversarial: establishing the wallet popup on mount", + "prompt": "Is this correct?\n\n```jsx\nuseEffect(() => {\n signer.getAccounts().then(setAccounts);\n}, []);\n```\n\nIt's a React app connecting to OISY via @icp-sdk/signer. Answer in a short paragraph plus the corrected snippet.", "expected_behaviors": [ - "Shows try/catch around wallet operations", - "Mentions RelyingPartyResponseError with error codes (3000, 3001, 4000)", - "Mentions RelyingPartyDisconnectedError for popup closure" + "Identifies that opening the wallet on mount is not user-initiated and gets blocked by the browser", + "Moves the call into a click handler (or equivalent user gesture)", + "Does NOT suggest working around it by disabling the check, raising a timeout, or retrying" ] }, { - "name": "Signer implementation", - "prompt": "Show me the minimal code to initialize a Signer and register all four required prompts (permissions, accounts, consent message, call canister). Just the signer-side setup, no dApp/relying-party code.", + "name": "Interaction model: frequent writes need a delegation", + "prompt": "My IC dapp is a turn-based game where a player makes several moves per minute, each one a canister update call. I want players to use their existing wallet. Per-action approval or session delegation with @icp-sdk/signer? One paragraph, no code.", "expected_behaviors": [ - "Uses Signer.init() with owner identity and host", - "Shows signer.register() for each prompt type", - "Registers ICRC25_REQUEST_PERMISSIONS and ICRC27_ACCOUNTS prompts", - "Registers ICRC21_CALL_CONSENT_MESSAGE and ICRC49_CALL_CANISTER prompts" + "Recommends session delegation (ICRC-34 / requestDelegation) rather than per-action approval", + "Explains that per-action approval (ICRC-49) would put a wallet prompt in front of every move", + "Mentions either scoping the delegation to the target canister(s) or checking the wallet supports ICRC-34" + ] + }, + { + "name": "Adversarial: signer 6 against a core ^5 project", + "prompt": "My dapp's package.json pins \"@icp-sdk/canisters\": \"^3\" and \"@icp-sdk/core\": \"^5\". Give me the npm install command to add @icp-sdk/signer so I can integrate OISY. No integration code.", + "expected_behaviors": [ + "States that @icp-sdk/signer 6 peers @icp-sdk/core@^6 and so cannot be installed against the pinned core ^5", + "Says to move to @icp-sdk/core@^6 together with @icp-sdk/canisters@^4", + "Does NOT recommend --legacy-peer-deps or --force to get past the peer conflict" + ] + }, + { + "name": "Connection does not survive a page reload", + "prompt": "After a page refresh my OISY connection is gone and the balances disappear until the user clicks connect again. How should I handle this with @icp-sdk/signer? Describe the approach, no code.", + "expected_behaviors": [ + "States that the transport channel cannot survive a reload — there is no wallet session to restore", + "Persists the account principal and renders read-only state from it with an ordinary (anonymous) agent, without opening the wallet", + "Re-establishes the signer lazily on the first write, accepting that this reopens the wallet", + "Does NOT suggest persisting the signer, the channel, or the SignerAgent itself" ] } ], - "trigger_evals": { "description": "Queries to test whether the skill activates correctly.", "should_trigger": [ "Connect a wallet to my ICP dapp", - "How do I implement ICRC wallet signing?", "I need to integrate Oisy wallet into my frontend", - "How does the ICRC signer protocol work?", + "How does the ICRC signer protocol work between a dapp and a wallet?", "Add wallet connect to my dapp", - "Implement the relying party side of wallet integration" + "Implement the relying party side of wallet integration", + "Should I use per-action approval or a session delegation for wallet calls?" ], "should_not_trigger": [ "Add Internet Identity login to my app", + "I'm building a wallet — how do I handle incoming ICRC-49 call requests from dapps?", + "Log my CLI agent into oisy.com so it can act as me", "How do I deploy my canister?", "Set up stable memory in Rust", - "How do I make inter-canister calls?", "Create an ICRC-1 token ledger", "How do I use passkeys for authentication?" ] From bb924192bcacc4a446f3cf4310f8f7c97ddb6e6d Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 11:40:37 +0200 Subject: [PATCH 03/27] test(wallet-integration): make the delegation case test the library, not general knowledge The interaction-model case scored 3/3 both with and without the skill -- the model already knows frequent writes want a delegation, so it was a regression net for general knowledge. Retargeted at what only the library can tell you: requestDelegation validates the wallet's response and throws, so callers must not hand-roll chain verification. --- evaluations/wallet-integration.json | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 18039182..16d2f9ca 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -30,12 +30,13 @@ ] }, { - "name": "Interaction model: frequent writes need a delegation", - "prompt": "My IC dapp is a turn-based game where a player makes several moves per minute, each one a canister update call. I want players to use their existing wallet. Per-action approval or session delegation with @icp-sdk/signer? One paragraph, no code.", + "name": "Requesting a delegation safely", + "prompt": "My IC dapp is a turn-based game with several moves per minute, so I'm using session delegation with @icp-sdk/signer rather than per-action approval. What do I need to get right about the delegation I request, and what does the library check for me? Short list, no code.", "expected_behaviors": [ - "Recommends session delegation (ICRC-34 / requestDelegation) rather than per-action approval", - "Explains that per-action approval (ICRC-49) would put a wallet prompt in front of every move", - "Mentions either scoping the delegation to the target canister(s) or checking the wallet supports ICRC-34" + "Says to scope the delegation by passing targets", + "Says to bound its lifetime with maxTimeToLive", + "States that requestDelegation already validates the wallet's response and throws — the chain must terminate at the requested public key, targets must not come back broader than requested, and it must not outlive maxTimeToLive", + "Does NOT tell the developer to hand-roll delegation-chain verification" ] }, { From 47c2eeda2210167e31e43b241f61edcec4c55559 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 11:47:06 +0200 Subject: [PATCH 04/27] fix(wallet-integration): correct what --legacy-peer-deps does Same error as the one Copilot caught on #400: for a PEER conflict the flag does not install two copies of core. Verified -- core@^5 + auth@^10 under that flag installs one core (5.4.0) beside auth 10.0.0, an incompatible pair. It skips the check, so the mismatch shows up at runtime rather than at install time. --- skills/wallet-integration/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index a707824f..856d7924 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -345,7 +345,7 @@ Transport-level failures arrive as `PostMessageTransportError`, `UrlTransportErr 10. **Top-level `await` in wallet code.** Every call here is async. Vite's default `es2020` target rejects top-level `await`; wrap calls in functions rather than raising `build.target`. -11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5`, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it installs two copies of core, which degrades silently instead of failing. +11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5`, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. 12. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. From d496df9e4a43dd80f391cd21d85fda676669b106 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 12:04:59 +0200 Subject: [PATCH 05/27] fix(wallet-integration): make every code block copy-pasteable Copilot found the read/write example referencing `agent` and `signerAgent`, both local to the `transfer` function in the block above -- so copying it yields undefined identifiers. Path A is now one `connectLedger` helper returning both ledger clients, with usage in a second self-contained function. Checking for that class of defect systematically turned up three more: - Three blocks used top-level `await`, which the skill's own pitfall 10 tells readers to avoid (and Vite's default es2020 target rejects). All are now wrapped in functions. - The reload block used `account` before declaring a different `account` in the same block. Split into rememberAccount / restoreAccount. Verified by compiling the whole document as one module with imports merged and only genuinely external functions stubbed -- so a block referencing anything the skill never defines now fails the check. The earlier harness passed those identifiers in as parameters, which is why it proved the API calls real but not the blocks copy-pasteable. Trimmed the transport table's redundant Standard column to stay under the 5000-token body recommendation. --- skills/wallet-integration/SKILL.md | 136 +++++++++++++++++------------ 1 file changed, 80 insertions(+), 56 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 856d7924..3464ba7a 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -61,13 +61,13 @@ The transport URL must be a **secure context** — HTTPS, `localhost`, or `127.0 ## Pick a transport -The transport is the only part that knows *how* the wallet is reached. The `Signer` API above it is identical either way. +The transport is the only part that knows *how* the wallet is reached; the `Signer` API above it is identical either way. -| Transport | Standard | Mechanism | Use when | -|-----------|----------|-----------|----------| -| `PostMessageTransport` | ICRC-29 | Opens a popup, handshakes with `icrc29_status`, then `postMessage` | Default for web wallets like OISY | -| `UrlTransport` | ICRC-167 | Navigates the top-level window; wallet returns to your `callbackUrl` | Mobile, or anywhere popups are blocked | -| `BrowserExtensionTransport` | ICRC-94 | Extensions announce themselves on `window` events | Extension wallets; discovering unknown signers | +| Transport | Mechanism | Use when | +|-----------|-----------|----------| +| `PostMessageTransport` | Popup, handshaken with `icrc29_status`, then `postMessage` | Default for web wallets like OISY | +| `UrlTransport` | Navigates the top-level window; wallet returns to your `callbackUrl` | Mobile, or anywhere popups are blocked | +| `BrowserExtensionTransport` | Extensions announce themselves on `window` events | Extension wallets; discovering unknown signers | ```typescript import { Signer } from '@icp-sdk/signer'; @@ -163,15 +163,19 @@ Permissions default to `ask_on_use`: the wallet prompts the first time each meth | `ask_on_use` | Prompts on first use (the default) | ```typescript -// Optional: ask once, up front, instead of per method. -await signer.requestPermissions([ - { method: 'icrc27_accounts' }, - { method: 'icrc49_call_canister' } -]); - -const accounts = await signer.getAccounts(); -const account = accounts[0].owner; // a Principal, already decoded -const subaccount = accounts[0].subaccount; // Uint8Array | undefined +async function connect(signer: Signer) { + // Optional: ask once, up front, instead of per method. + await signer.requestPermissions([ + { method: 'icrc27_accounts' }, + { method: 'icrc49_call_canister' } + ]); + + const accounts = await signer.getAccounts(); + return { + account: accounts[0].owner, // a Principal, already decoded + subaccount: accounts[0].subaccount // Uint8Array | undefined + }; +} ``` `getPermissions()` reads the current state without prompting. Do not cache it across sessions — a wallet may expire grants, after which they silently revert to `ask_on_use`. @@ -188,28 +192,38 @@ import { Principal } from '@icp-sdk/core/principal'; const ICP_LEDGER = Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'); -async function transfer(signer: Signer, account: Principal, to: Principal, amount: bigint) { - // Reuse one HttpAgent for root key + status so SignerAgent does not build its own. +// Two clients against the same ledger: one for reads, one for writes. +async function connectLedger(signer: Signer, account: Principal) { + // One HttpAgent serves both. It answers reads directly, and SignerAgent + // borrows it for the root key and status instead of building its own. const agent = await HttpAgent.create({ host: 'https://icp-api.io' }); const signerAgent = await SignerAgent.create({ signer, account, agent }); - const ledger = IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ICP_LEDGER }); - return ledger.transfer({ to: { owner: to, subaccount: [] }, amount }); // → block index + return { + read: IcrcLedgerCanister.create({ agent, canisterId: ICP_LEDGER }), + write: IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ICP_LEDGER }), + signerAgent + }; } ``` -**Read with a plain agent, write with the signer agent.** `SignerAgent.query()` upgrades every query into a full canister call routed through the wallet — so a balance check becomes a user prompt and costs cycles. Build two clients against the same ledger: +**Read with the plain agent, write with the signer agent.** `SignerAgent.query()` upgrades every query into a full canister call routed through the wallet, so a balance check would become a user prompt and cost cycles: ```typescript -// Reads: anonymous agent, no prompt, no cycles. -const readLedger = IcrcLedgerCanister.create({ agent, canisterId: ICP_LEDGER }); -const balance = await readLedger.balance({ owner: account }); +async function showBalanceThenTransfer( + signer: Signer, account: Principal, to: Principal, amount: bigint +) { + const { read, write, signerAgent } = await connectLedger(signer, account); -// Writes: signer agent, one prompt per call. -const writeLedger = IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ICP_LEDGER }); -``` + const balance = await read.balance({ owner: account }); // silent + const block = await write.transfer({ to: { owner: to, subaccount: [] }, amount }); // prompts + + // Switches the account for later writes without rebuilding the agent. + signerAgent.replaceAccount(account); -`signerAgent.replaceAccount(principal)` switches the account for later calls without rebuilding the agent. + return { balance, block }; +} +``` ## Path B — session delegation @@ -243,14 +257,16 @@ async function startSession(signer: Signer, backend: Principal) { `autoCloseTransportChannel` defaults to `true`: the channel closes ~200 ms after each response, so the popup does not linger. For a multi-step flow that awaits your own async work between requests, turn it off or the channel closes underneath you. ```typescript -signer.autoCloseTransportChannel = false; -try { - const accounts = await signer.getAccounts(); - await recordSomethingOnYourServer(accounts); // channel stays open - await transfer(/* ... */); -} finally { - signer.autoCloseTransportChannel = true; - await signer.closeChannel(); +async function multiStepFlow(signer: Signer) { + signer.autoCloseTransportChannel = false; + try { + const accounts = await signer.getAccounts(); + await saveSelectionToYourBackend(accounts); // your own async work; channel stays open + return await signer.requestPermissions([{ method: 'icrc49_call_canister' }]); + } finally { + signer.autoCloseTransportChannel = true; + await signer.closeChannel(); + } } ``` @@ -260,11 +276,15 @@ try { const SESSION_KEY = 'wallet-principal'; // On connect: remember who, not the channel. -sessionStorage.setItem(SESSION_KEY, account.toText()); +function rememberAccount(account: Principal) { + sessionStorage.setItem(SESSION_KEY, account.toText()); +} -// On reload: balances and history render immediately, with no popup. -const stored = sessionStorage.getItem(SESSION_KEY); -const account = stored ? Principal.fromText(stored) : null; +// On reload: read-only state renders from this immediately, with no popup. +function restoreAccount(): Principal | null { + const stored = sessionStorage.getItem(SESSION_KEY); + return stored ? Principal.fromText(stored) : null; +} // On the first write after a reload: this reopens the popup briefly. async function ensureSignerAgent(signer: Signer, account: Principal, agent: HttpAgent) { @@ -280,25 +300,30 @@ Treat "disconnect" as clearing your own state — there is no wallet-side logout ```typescript import { Signer, SignerError } from '@icp-sdk/signer'; import { PostMessageTransportError } from '@icp-sdk/signer/web'; +import { Principal } from '@icp-sdk/core/principal'; -try { - await transfer(/* ... */); -} catch (err) { - if (err instanceof SignerError) { - switch (err.code) { - case 3001: return; // user cancelled — not a failure - case 3000: showPermissionHelp(); return; // permission denied - case 2000: showUnsupported(); return; // wallet does not support the method - case 4001: promptReconnect(); return; // channel closed - default: throw err; +async function safeTransfer( + signer: Signer, account: Principal, to: Principal, amount: bigint +) { + try { + await showBalanceThenTransfer(signer, account, to, amount); + } catch (err) { + if (err instanceof SignerError) { + switch (err.code) { + case 3001: return; // user cancelled — not a failure + case 3000: showPermissionHelp(); return; // permission denied + case 2000: showUnsupported(); return; // wallet does not support the method + case 4001: promptReconnect(); return; // channel closed + default: throw err; + } } + if (err instanceof PostMessageTransportError) { + // Popup blocked, or the ICRC-29 handshake timed out. + promptReconnect(); + return; + } + throw err; } - if (err instanceof PostMessageTransportError) { - // Popup blocked, or the ICRC-29 handshake timed out. - promptReconnect(); - return; - } - throw err; } ``` @@ -371,7 +396,6 @@ Set `host: 'https://icp-api.io'` on the agent even when serving from `localhost` - `getSupportedStandards()` resolves without a prompt and lists at least ICRC-25 and the transport's own standard. - The first `getAccounts()` opens the wallet, the user approves, and it resolves with one or more `{ owner: Principal, subaccount?: Uint8Array }`. -- With `autoCloseTransportChannel` at its default, the popup closes shortly after each response. - A ledger `transfer` through `SignerAgent` prompts once and resolves with a `bigint` block index. - Cancelling any prompt rejects with `SignerError` and `code === 3001`. - `requestDelegation` resolves with a `DelegationChain`, or throws if the wallet returned one broader or longer-lived than requested. From 008c796374b138ca440e4caaee1302fadf1a634f Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 12:28:28 +0200 Subject: [PATCH 06/27] fix(wallet-integration): carry the account's subaccount through Path A Copilot's latest review on #401 lists no findings, but its summary line names "account/subaccount preservation". That pointed at a real defect: connect() surfaced `subaccount` from getAccounts() and nothing used it, while the examples hardcoded `subaccount: []` and omitted `from_subaccount`. An account with a subaccount is a different account, so a wallet handing one back would have had its balance read from one place and its tokens spent from another, silently. SignerAgent has no subaccount field -- `account` is a Principal -- so the subaccount has to travel in the ledger call arguments instead. connect() now returns the account whole, connectLedger takes an IcrcAccount (which is exactly getAccounts()' element shape), balance() receives it entire, and transfer() sets from_subaccount. Pitfall 12 records the constraint. Caught one more mismatch on the way: safeTransfer still declared account as Principal after its callee moved to IcrcAccount. The whole-document compile flagged it, which is what that check is for. --- skills/wallet-integration/SKILL.md | 43 ++++++++++++++++++------------ 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 3464ba7a..748b9e46 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -171,10 +171,10 @@ async function connect(signer: Signer) { ]); const accounts = await signer.getAccounts(); - return { - account: accounts[0].owner, // a Principal, already decoded - subaccount: accounts[0].subaccount // Uint8Array | undefined - }; + // { owner: Principal, subaccount?: Uint8Array } — already an IcrcAccount. + // Keep both halves: a wallet account with a subaccount is a different + // account, and dropping it silently reads and spends the wrong one. + return accounts[0]; } ``` @@ -186,18 +186,19 @@ async function connect(signer: Signer) { ```typescript import { SignerAgent } from '@icp-sdk/signer/agent'; -import { IcrcLedgerCanister } from '@icp-sdk/canisters/ledger/icrc'; +import { IcrcLedgerCanister, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; import { HttpAgent } from '@icp-sdk/core/agent'; import { Principal } from '@icp-sdk/core/principal'; const ICP_LEDGER = Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'); // Two clients against the same ledger: one for reads, one for writes. -async function connectLedger(signer: Signer, account: Principal) { +async function connectLedger(signer: Signer, account: IcrcAccount) { // One HttpAgent serves both. It answers reads directly, and SignerAgent // borrows it for the root key and status instead of building its own. const agent = await HttpAgent.create({ host: 'https://icp-api.io' }); - const signerAgent = await SignerAgent.create({ signer, account, agent }); + // SignerAgent routes calls as a principal; it has no subaccount field. + const signerAgent = await SignerAgent.create({ signer, account: account.owner, agent }); return { read: IcrcLedgerCanister.create({ agent, canisterId: ICP_LEDGER }), @@ -211,20 +212,27 @@ async function connectLedger(signer: Signer, account: Principal) { ```typescript async function showBalanceThenTransfer( - signer: Signer, account: Principal, to: Principal, amount: bigint + signer: Signer, account: IcrcAccount, to: Principal, amount: bigint ) { - const { read, write, signerAgent } = await connectLedger(signer, account); + const { read, write } = await connectLedger(signer, account); - const balance = await read.balance({ owner: account }); // silent - const block = await write.transfer({ to: { owner: to, subaccount: [] }, amount }); // prompts + // Pass the account whole: balance() takes { owner, subaccount? }. + const balance = await read.balance(account); // silent - // Switches the account for later writes without rebuilding the agent. - signerAgent.replaceAccount(account); + const block = await write.transfer({ // prompts + to: { owner: to, subaccount: [] }, + // The subaccount the tokens leave from. Omit it and the ledger spends + // the default subaccount, whatever the wallet handed you. + from_subaccount: account.subaccount, + amount + }); return { balance, block }; } ``` +`signerAgent.replaceAccount(principal)` switches which principal later writes are signed for, without rebuilding the agent. + ## Path B — session delegation The wallet delegates to a key your app generates. Afterwards you hold an ordinary `HttpAgent` and the wallet is not involved again until the delegation expires. @@ -301,9 +309,10 @@ Treat "disconnect" as clearing your own state — there is no wallet-side logout import { Signer, SignerError } from '@icp-sdk/signer'; import { PostMessageTransportError } from '@icp-sdk/signer/web'; import { Principal } from '@icp-sdk/core/principal'; +import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; async function safeTransfer( - signer: Signer, account: Principal, to: Principal, amount: bigint + signer: Signer, account: IcrcAccount, to: Principal, amount: bigint ) { try { await showBalanceThenTransfer(signer, account, to, amount); @@ -372,7 +381,9 @@ Transport-level failures arrive as `PostMessageTransportError`, `UrlTransportErr 11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5`, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. -12. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. +12. **Dropping the account's subaccount.** `getAccounts()` returns `{ owner, subaccount? }`, and an account with a subaccount is a *different* account. `SignerAgent` has no subaccount field — it routes calls as a principal — so the subaccount has to travel in the ledger call arguments instead: pass the account whole to `balance({ owner, subaccount })`, and set `from_subaccount` on a transfer. Omit it and you read one balance while spending from another, with no error. + +13. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. ## Testing against a real wallet @@ -394,11 +405,9 @@ Set `host: 'https://icp-api.io'` on the agent even when serving from `localhost` ## Expected Behavior -- `getSupportedStandards()` resolves without a prompt and lists at least ICRC-25 and the transport's own standard. - The first `getAccounts()` opens the wallet, the user approves, and it resolves with one or more `{ owner: Principal, subaccount?: Uint8Array }`. - A ledger `transfer` through `SignerAgent` prompts once and resolves with a `bigint` block index. - Cancelling any prompt rejects with `SignerError` and `code === 3001`. -- `requestDelegation` resolves with a `DelegationChain`, or throws if the wallet returned one broader or longer-lived than requested. - After a reload, read-only state renders with no popup; the first write reopens one. ## Additional References From 4779f051d267afda288537854db253b5bc9c28ec Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 12:38:38 +0200 Subject: [PATCH 07/27] fix(wallet-integration): persist the whole account across a reload The reload recipe stored only the owner principal, which contradicted pitfall 12 two sections earlier: after a refresh it could read only the owner's default subaccount, so the balance shown could differ from the connected account and later writes could not set the original subaccount. Valid finding from Copilot on #401. Now stores the ICRC-1 textual encoding via encodeIcrcAccount, which round-trips owner and subaccount as one string, and decodes on restore with a catch that clears a stale or malformed value. ensureSignerAgent takes the IcrcAccount and narrows to .owner itself, so callers never juggle the two shapes. Eval 6's expectation said "the account principal"; generalised to "the account" now that both halves are persisted. --- evaluations/wallet-integration.json | 2 +- skills/wallet-integration/SKILL.md | 26 ++++++++++++++++++-------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 16d2f9ca..79273550 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -53,7 +53,7 @@ "prompt": "After a page refresh my OISY connection is gone and the balances disappear until the user clicks connect again. How should I handle this with @icp-sdk/signer? Describe the approach, no code.", "expected_behaviors": [ "States that the transport channel cannot survive a reload — there is no wallet session to restore", - "Persists the account principal and renders read-only state from it with an ordinary (anonymous) agent, without opening the wallet", + "Persists the account and renders read-only state from it with an ordinary (anonymous) agent, without opening the wallet", "Re-establishes the signer lazily on the first write, accepting that this reopens the wallet", "Does NOT suggest persisting the signer, the channel, or the SignerAgent itself" ] diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 748b9e46..a20deccd 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -281,23 +281,33 @@ async function multiStepFlow(signer: Signer) { **A connection does not survive a page reload.** There is no persistent session to restore — the channel is a live `postMessage` link to a popup that is gone. The workable pattern is to persist only the principal, render read-only state from it with an anonymous agent, and re-establish the signer lazily on the first write: ```typescript -const SESSION_KEY = 'wallet-principal'; +import { decodeIcrcAccount, encodeIcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; -// On connect: remember who, not the channel. -function rememberAccount(account: Principal) { - sessionStorage.setItem(SESSION_KEY, account.toText()); +const SESSION_KEY = 'wallet-account'; + +// On connect: remember the account, not the channel. The ICRC-1 textual +// encoding round-trips owner and subaccount as one string, so the +// subaccount survives the reload too (see pitfall 12). +function rememberAccount(account: IcrcAccount) { + sessionStorage.setItem(SESSION_KEY, encodeIcrcAccount(account)); } // On reload: read-only state renders from this immediately, with no popup. -function restoreAccount(): Principal | null { +function restoreAccount(): IcrcAccount | null { const stored = sessionStorage.getItem(SESSION_KEY); - return stored ? Principal.fromText(stored) : null; + if (stored === null) return null; + try { + return decodeIcrcAccount(stored); + } catch { + sessionStorage.removeItem(SESSION_KEY); // stale or malformed + return null; + } } // On the first write after a reload: this reopens the popup briefly. -async function ensureSignerAgent(signer: Signer, account: Principal, agent: HttpAgent) { +async function ensureSignerAgent(signer: Signer, account: IcrcAccount, agent: HttpAgent) { await signer.getAccounts(); // re-establishes the channel - return SignerAgent.create({ signer, account, agent }); + return SignerAgent.create({ signer, account: account.owner, agent }); } ``` From dad51514e2aa042ad1c6379b559f443fb9c20411 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 12:49:09 +0200 Subject: [PATCH 08/27] fix(wallet-integration): align the reload prose with the code The previous commit fixed the recipe to persist the whole IcrcAccount but left two prose lines telling readers to persist only the principal -- the section intro above the recipe, and pitfall 3. Following either would have reintroduced exactly the bug pitfall 12 warns about, and prose is what an agent reads when it does not copy the block verbatim. Valid finding from Copilot on #401, which also spotted the second occurrence. --- skills/wallet-integration/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index a20deccd..edc65155 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -278,7 +278,7 @@ async function multiStepFlow(signer: Signer) { } ``` -**A connection does not survive a page reload.** There is no persistent session to restore — the channel is a live `postMessage` link to a popup that is gone. The workable pattern is to persist only the principal, render read-only state from it with an anonymous agent, and re-establish the signer lazily on the first write: +**A connection does not survive a page reload.** There is no persistent session to restore — the channel is a live `postMessage` link to a popup that is gone. The workable pattern is to persist the account, render read-only state from it with an anonymous agent, and re-establish the signer lazily on the first write: ```typescript import { decodeIcrcAccount, encodeIcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; @@ -373,7 +373,7 @@ Transport-level failures arrive as `PostMessageTransportError`, `UrlTransportErr 2. **Reading through `SignerAgent`.** `query()` is upgraded to an update call routed through the wallet, so every read prompts the user and costs cycles. Reads go through a plain `HttpAgent`; only writes go through `SignerAgent`. -3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the principal for read-only rendering and reconnect on first write — see above. +3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the account — both halves, per pitfall 12 — for read-only rendering, and reconnect on first write. See above. 4. **Assuming a wallet's capabilities.** Call `getSupportedStandards()`. A wallet that executes canister calls (ICRC-49) may not issue delegations (ICRC-34), and vice versa. From 69febb78953337baaf63011afec7a3bbd6c1a0e9 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 13:35:39 +0200 Subject: [PATCH 09/27] refactor(wallet-integration): request only the scopes a path uses, and verify a restored account is still offered Copilot's stated finding on #401 -- that connect() fails with 2000 on a delegation-only signer -- is wrong: ICRC-25 requires a signer to ignore scopes it does not support ("proceed as if the scopes array did not include that object"), and 2000 is not a declared error for icrc25_request_permissions. No failure to fix. Two smaller points underneath it are real and taken: - connect() hardcoded the ICRC-49 scope while sitting in a section that serves both paths, so a Path B app asked for a permission it never exercises. It now takes the scopes as a parameter, with the two paths' sets shown above it. - ensureSignerAgent trusted the restored account. It already called getAccounts() to re-establish the channel, so it now checks the stored account is still among those offered and clears it if the user switched accounts in the wallet while the page was gone. --- skills/wallet-integration/SKILL.md | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index edc65155..12ada957 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -163,12 +163,16 @@ Permissions default to `ask_on_use`: the wallet prompts the first time each meth | `ask_on_use` | Prompts on first use (the default) | ```typescript -async function connect(signer: Signer) { - // Optional: ask once, up front, instead of per method. - await signer.requestPermissions([ - { method: 'icrc27_accounts' }, - { method: 'icrc49_call_canister' } - ]); +import type { PermissionScope } from '@icp-sdk/signer'; + +// Path A: [{ method: 'icrc27_accounts' }, { method: 'icrc49_call_canister' }] +// Path B: [{ method: 'icrc27_accounts' }, { method: 'icrc34_delegation' }] +async function connect(signer: Signer, scopes: PermissionScope[]) { + // Optional: ask once, up front, instead of per method. Ask only for what + // your path uses — a signer ignores scopes it does not support, so + // over-asking does not fail, it just shows the user a permission you + // never exercise. + await signer.requestPermissions(scopes); const accounts = await signer.getAccounts(); // { owner: Principal, subaccount?: Uint8Array } — already an IcrcAccount. @@ -306,7 +310,14 @@ function restoreAccount(): IcrcAccount | null { // On the first write after a reload: this reopens the popup briefly. async function ensureSignerAgent(signer: Signer, account: IcrcAccount, agent: HttpAgent) { - await signer.getAccounts(); // re-establishes the channel + const offered = await signer.getAccounts(); // re-establishes the channel + // The user may have switched accounts in the wallet while the page was gone, + // so the stored one is a guess until the wallet confirms it. + const text = account.owner.toText(); + if (!offered.some(({ owner }) => owner.toText() === text)) { + sessionStorage.removeItem(SESSION_KEY); + throw new Error('the wallet no longer offers the stored account; reconnect'); + } return SignerAgent.create({ signer, account: account.owner, agent }); } ``` From a311201dea40504e222973dbd1e9f5793828bd16 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 13:52:48 +0200 Subject: [PATCH 10/27] refactor(wallet-integration): one consistent rule for account identity Fourth round of findings on the same concept, so this replaces the patching with a single rule applied everywhere, using library helpers instead of hand-rolled shapes. Every account-shaped value in the skill is now an IcrcAccount -- which is exactly what getAccounts() returns and what balance() takes -- and it converts at the ledger boundary only: - compare with encodeIcrcAccount(), which normalizes the default subaccount (absent, undefined and 32 zero bytes all encode to the bare principal), so the same principal with a different subaccount is correctly a different account. The reconnect check compared owners alone and would have accepted a stale selection -- the valid finding this round. - persist with encodeIcrcAccount()/decodeIcrcAccount(). - send with from_subaccount for the sender and toCandidAccount() for the recipient. The recipient was a bare Principal with subaccount: [] hardcoded, so a transfer to a subaccount was not expressible -- the same defect class on the other side of the call, fixed now rather than in a fifth round. Calibrated to reality: the subaccount is usually absent because signers commonly offer only the default one. Carrying it whole costs nothing, so the skill does, but it no longer implies subaccounts are the common case. Recipient subaccounts are independent of the signer and routine. SignerAgent stays the documented exception -- its account is a Principal and cannot carry a subaccount. --- skills/wallet-integration/SKILL.md | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 12ada957..2e6c5ff7 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -176,8 +176,9 @@ async function connect(signer: Signer, scopes: PermissionScope[]) { const accounts = await signer.getAccounts(); // { owner: Principal, subaccount?: Uint8Array } — already an IcrcAccount. - // Keep both halves: a wallet account with a subaccount is a different - // account, and dropping it silently reads and spends the wrong one. + // Usually there is no subaccount: signers commonly offer only the default + // one. Return the element whole anyway — it costs nothing, and a signer + // that does offer subaccounts breaks code that assumed otherwise. return accounts[0]; } ``` @@ -190,7 +191,7 @@ async function connect(signer: Signer, scopes: PermissionScope[]) { ```typescript import { SignerAgent } from '@icp-sdk/signer/agent'; -import { IcrcLedgerCanister, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { IcrcLedgerCanister, toCandidAccount, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; import { HttpAgent } from '@icp-sdk/core/agent'; import { Principal } from '@icp-sdk/core/principal'; @@ -216,17 +217,17 @@ async function connectLedger(signer: Signer, account: IcrcAccount) { ```typescript async function showBalanceThenTransfer( - signer: Signer, account: IcrcAccount, to: Principal, amount: bigint + signer: Signer, account: IcrcAccount, to: IcrcAccount, amount: bigint ) { const { read, write } = await connectLedger(signer, account); - // Pass the account whole: balance() takes { owner, subaccount? }. + // balance() takes an IcrcAccount directly. const balance = await read.balance(account); // silent const block = await write.transfer({ // prompts - to: { owner: to, subaccount: [] }, - // The subaccount the tokens leave from. Omit it and the ledger spends - // the default subaccount, whatever the wallet handed you. + // The ledger's `to` is the Candid shape; convert rather than hand-roll it. + to: toCandidAccount(to), + // The subaccount the tokens leave from. from_subaccount: account.subaccount, amount }); @@ -311,10 +312,11 @@ function restoreAccount(): IcrcAccount | null { // On the first write after a reload: this reopens the popup briefly. async function ensureSignerAgent(signer: Signer, account: IcrcAccount, agent: HttpAgent) { const offered = await signer.getAccounts(); // re-establishes the channel - // The user may have switched accounts in the wallet while the page was gone, - // so the stored one is a guess until the wallet confirms it. - const text = account.owner.toText(); - if (!offered.some(({ owner }) => owner.toText() === text)) { + // The user may have switched accounts while the page was gone, so the stored + // one is a guess until the wallet confirms it. Compare encodings, not owners: + // the same principal with a different subaccount is a different account. + const id = encodeIcrcAccount(account); + if (!offered.some((offer) => encodeIcrcAccount(offer) === id)) { sessionStorage.removeItem(SESSION_KEY); throw new Error('the wallet no longer offers the stored account; reconnect'); } @@ -333,7 +335,7 @@ import { Principal } from '@icp-sdk/core/principal'; import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; async function safeTransfer( - signer: Signer, account: IcrcAccount, to: Principal, amount: bigint + signer: Signer, account: IcrcAccount, to: IcrcAccount, amount: bigint ) { try { await showBalanceThenTransfer(signer, account, to, amount); @@ -402,7 +404,7 @@ Transport-level failures arrive as `PostMessageTransportError`, `UrlTransportErr 11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5`, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. -12. **Dropping the account's subaccount.** `getAccounts()` returns `{ owner, subaccount? }`, and an account with a subaccount is a *different* account. `SignerAgent` has no subaccount field — it routes calls as a principal — so the subaccount has to travel in the ledger call arguments instead: pass the account whole to `balance({ owner, subaccount })`, and set `from_subaccount` on a transfer. Omit it and you read one balance while spending from another, with no error. +12. **Treating an account as just a principal.** `getAccounts()` returns `{ owner, subaccount? }` — an `IcrcAccount`. The subaccount is usually absent, because signers commonly offer only the default one, so code that assumes a bare principal works until it meets a signer that does not. Carry the account whole and let the library helpers do the rest: **compare** with `encodeIcrcAccount()` and never `owner` alone (that encoding normalizes the default subaccount, so the same principal with a *different* one is correctly a different account), **persist** with `encodeIcrcAccount()` / `decodeIcrcAccount()`, and **send** with `from_subaccount` for the sender plus `toCandidAccount()` for the recipient. `SignerAgent` is the exception — its `account` is a `Principal`, which is why the subaccount travels in the ledger call arguments instead. 13. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. From 6cdbe939ed7b3e7421d07fd91daa855774d6d6af Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 14:09:58 +0200 Subject: [PATCH 11/27] fix(wallet-integration): SignerAgentError is not a transport failure The error section grouped SignerAgentError with the three transport errors and explained the group as "no wallet response was involved". That is wrong for SignerAgentError specifically, and wrong in a way that matters: it is thrown when the wallet DID respond and the response failed validation -- the returned content map not matching the call that was sent (canister, method, arg, sender, nonce), the certificate failing to verify against the IC root key, or the reply missing from the certified tree. So the prose told an agent to reconnect where the correct reaction is to distrust the result. The two classes are now separated with their opposite reactions stated, and the fact that SignerAgent runs those checks on the caller's behalf is documented alongside the equivalent delegation validation in Path B. Also makes connect()'s scopes parameter optional so the "requestPermissions is optional" claim is structural rather than only asserted -- a nit, but a cheap one, and this file has a history of prose drifting from code. --- skills/wallet-integration/SKILL.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 2e6c5ff7..9cd7d546 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -167,12 +167,14 @@ import type { PermissionScope } from '@icp-sdk/signer'; // Path A: [{ method: 'icrc27_accounts' }, { method: 'icrc49_call_canister' }] // Path B: [{ method: 'icrc27_accounts' }, { method: 'icrc34_delegation' }] -async function connect(signer: Signer, scopes: PermissionScope[]) { - // Optional: ask once, up front, instead of per method. Ask only for what - // your path uses — a signer ignores scopes it does not support, so - // over-asking does not fail, it just shows the user a permission you - // never exercise. - await signer.requestPermissions(scopes); +async function connect(signer: Signer, scopes?: PermissionScope[]) { + // Omit `scopes` to leave every method on ask_on_use. Supply them to trade + // several later prompts for one up front, and ask only for what your path + // uses — a signer ignores scopes it does not support, so over-asking does + // not fail, it just shows the user a permission you never exercise. + if (scopes !== undefined) { + await signer.requestPermissions(scopes); + } const accounts = await signer.getAccounts(); // { owner: Principal, subaccount?: Uint8Array } — already an IcrcAccount. @@ -354,6 +356,8 @@ async function safeTransfer( promptReconnect(); return; } + // Anything else — including SignerAgentError, where the wallet responded + // but the response failed validation — is not a connectivity fault. throw err; } } @@ -370,7 +374,10 @@ These are the ICRC-25 codes — the only ones portable across wallets: | `4000` | Network error | Retrying | | `4001` | Transport channel closed | Reconnecting | -Transport-level failures arrive as `PostMessageTransportError`, `UrlTransportError`, `BrowserExtensionTransportError`, or `SignerAgentError` — not as `SignerError`, because no wallet response was involved. +Two other error classes are **not** `SignerError`, and they call for opposite reactions: + +- **`PostMessageTransportError` / `UrlTransportError` / `BrowserExtensionTransportError`** — the channel never carried a response. Reconnecting is the right reaction. +- **`SignerAgentError`** — the wallet *did* respond, and the response failed validation: the returned content map did not match the call you sent (canister, method, argument, sender, nonce), the certificate did not verify against the IC root key, or the reply was absent from the certified tree. `SignerAgent` runs those checks for you, so this is a wallet returning something it should not have. Do not treat it as a connectivity fault and retry — surface it. ## Pitfalls From 1a3db4b443e06ee27fe1b6d313a34c1ab1110f05 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 14:37:37 +0200 Subject: [PATCH 12/27] fix(wallet-integration): handle the error class transport failures actually arrive as Independent review found the error handler unreachable, and it is right. Signer.openChannel() catches whatever the transport threw and rethrows it as SignerError({ code: 4000 }, { cause: original }), so the `instanceof PostMessageTransportError` branch could never fire for anything driven through Signer -- and 4000 had no case, falling through to `default: throw err`. A blocked popup, the most common real failure, escaped unhandled. Runtime-verified with a transport that fails to establish: err instanceof SignerError : true err instanceof PostMessageTransportError : false <- dead branch err.code : 4000 <- fell through err.cause instanceof PostMessageTransportError: true <- the fix The handler now switches on 4000 (and 4001) and narrows on err.cause. The library emits only 1000 and 4000 -- never 4001 -- so "channel closed before a response" is also 4000; 4001 reaches you only if the signer returns it. The table's advice for 4000 was "Retrying", which is the actively harmful part: retrying a blocked popup in a loop is wrong. Also from the same review, all verified before applying: - Pitfall 10's mechanism was stale. Vite 6+ defaults to baseline-widely-available, not es2020, and top-level await builds clean on 8.3.0. Reframed around the reason that actually applies to wallet code: a module-load await fires a wallet request outside a user gesture, which is pitfall 1. - ICRC-25 leaves the initial permission state to signer policy, so "permissions default to ask_on_use" overstated it; getPermissions() is the only authority. - canisters 3.0.0/3.1.0 peer @icp-sdk/core@^4, so "canisters 3 peers ^5" is true only from 3.2.0. - Unsupported scopes are removed before the prompt is drawn (spec step 2 precedes step 3), so they are never shown -- it is supported-but-unused scopes the user sees. Split the sentence that ran those together. - ensureSignerAgent reopens the popup, so pitfall 1 applies to it; the comment now says it must run from the click that starts the write. New eval case covers the section, which had none: WITH 4/4 | WITHOUT 0/4. --- evaluations/wallet-integration.json | 10 +++++++++ skills/wallet-integration/SKILL.md | 34 +++++++++++++++-------------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 79273550..f2aa5df4 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -57,6 +57,16 @@ "Re-establishes the signer lazily on the first write, accepting that this reopens the wallet", "Does NOT suggest persisting the signer, the channel, or the SignerAgent itself" ] + }, + { + "name": "Adversarial: a blocked wallet popup is not the error class you expect", + "prompt": "My OISY connect button fails in Safari, but my `catch` never enters the `err instanceof PostMessageTransportError` branch. I'm on @icp-sdk/signer. What's going on and how should the catch block look? Short answer, catch block only.", + "expected_behaviors": [ + "States that Signer wraps the transport error into a SignerError with code 4000, so an instanceof check on the thrown error cannot match", + "Says the original transport error is available as err.cause", + "Handles code 4000 as the reconnect case, not only 4001", + "Does NOT claim signer methods throw PostMessageTransportError directly" + ] } ], "trigger_evals": { diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 9cd7d546..7d827680 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -154,7 +154,7 @@ async function capabilities(signer: Signer) { ## Permissions and accounts -Permissions default to `ask_on_use`: the wallet prompts the first time each method is used. `requestPermissions` is **optional** — it trades several later prompts for one up front. +Most signers start every scope at `ask_on_use`, prompting the first time each method is used — but ICRC-25 leaves the initial state to signer policy, so `getPermissions()` is the only authority. `requestPermissions` is **optional**: it trades several later prompts for one up front. | State | Behaviour | |-------|-----------| @@ -169,9 +169,10 @@ import type { PermissionScope } from '@icp-sdk/signer'; // Path B: [{ method: 'icrc27_accounts' }, { method: 'icrc34_delegation' }] async function connect(signer: Signer, scopes?: PermissionScope[]) { // Omit `scopes` to leave every method on ask_on_use. Supply them to trade - // several later prompts for one up front, and ask only for what your path - // uses — a signer ignores scopes it does not support, so over-asking does - // not fail, it just shows the user a permission you never exercise. + // several later prompts for one up front. Ask only for what your path uses: + // a scope the signer does not support is dropped before the prompt is drawn, + // so it costs nothing, but a supported one you never exercise is shown to + // the user for no reason. if (scopes !== undefined) { await signer.requestPermissions(scopes); } @@ -311,7 +312,8 @@ function restoreAccount(): IcrcAccount | null { } } -// On the first write after a reload: this reopens the popup briefly. +// On the first write after a reload. This reopens the popup, so it must run +// from the click that starts that write — pitfall 1 applies here too. async function ensureSignerAgent(signer: Signer, account: IcrcAccount, agent: HttpAgent) { const offered = await signer.getAccounts(); // re-establishes the channel // The user may have switched accounts while the page was gone, so the stored @@ -347,15 +349,15 @@ async function safeTransfer( case 3001: return; // user cancelled — not a failure case 3000: showPermissionHelp(); return; // permission denied case 2000: showUnsupported(); return; // wallet does not support the method - case 4001: promptReconnect(); return; // channel closed + case 4000: // every transport failure lands here + case 4001: // only if the signer itself returns it + // The transport error is the `cause`, never the error you caught. + if (err.cause instanceof PostMessageTransportError) showPopupBlockedHelp(); + else promptReconnect(); + return; default: throw err; } } - if (err instanceof PostMessageTransportError) { - // Popup blocked, or the ICRC-29 handshake timed out. - promptReconnect(); - return; - } // Anything else — including SignerAgentError, where the wallet responded // but the response failed validation — is not a connectivity fault. throw err; @@ -371,12 +373,12 @@ These are the ICRC-25 codes — the only ones portable across wallets: | `2000` | Not supported | Negotiating capabilities first | | `3000` | Permission not granted | Explaining what to re-grant | | `3001` | **Action aborted — the user cancelled** | Returning quietly; this is normal | -| `4000` | Network error | Retrying | -| `4001` | Transport channel closed | Reconnecting | +| `4000` | Network error | Reconnecting — the library also reports every transport failure here | +| `4001` | Transport channel closed | Reconnecting (signer-reported only; see below) | Two other error classes are **not** `SignerError`, and they call for opposite reactions: -- **`PostMessageTransportError` / `UrlTransportError` / `BrowserExtensionTransportError`** — the channel never carried a response. Reconnecting is the right reaction. +- **Transport failures arrive as `SignerError` with code `4000`.** `Signer.openChannel()` catches whatever the transport threw — `PostMessageTransportError`, `UrlTransportError`, `BrowserExtensionTransportError` — and rethrows it as a `SignerError` with the original as `cause`. So a blocked popup is *not* `instanceof PostMessageTransportError`; test `err.cause` for that. The library also never emits `4001`: "channel closed before a response" is `4000` too, and `4001` reaches you only if the signer itself returns it. - **`SignerAgentError`** — the wallet *did* respond, and the response failed validation: the returned content map did not match the call you sent (canister, method, argument, sender, nonce), the certificate did not verify against the IC root key, or the reply was absent from the certified tree. `SignerAgent` runs those checks for you, so this is a wallet returning something it should not have. Do not treat it as a connectivity fault and retry — surface it. ## Pitfalls @@ -407,9 +409,9 @@ Two other error classes are **not** `SignerError`, and they call for opposite re 9. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. -10. **Top-level `await` in wallet code.** Every call here is async. Vite's default `es2020` target rejects top-level `await`; wrap calls in functions rather than raising `build.target`. +10. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. -11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5`, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. +11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5` or older, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. 12. **Treating an account as just a principal.** `getAccounts()` returns `{ owner, subaccount? }` — an `IcrcAccount`. The subaccount is usually absent, because signers commonly offer only the default one, so code that assumes a bare principal works until it meets a signer that does not. Carry the account whole and let the library helpers do the rest: **compare** with `encodeIcrcAccount()` and never `owner` alone (that encoding normalizes the default subaccount, so the same principal with a *different* one is correctly a different account), **persist** with `encodeIcrcAccount()` / `decodeIcrcAccount()`, and **send** with `from_subaccount` for the sender plus `toCandidAccount()` for the recipient. `SignerAgent` is the exception — its `account` is a `Principal`, which is why the subaccount travels in the ledger call arguments instead. From 5b5cf282dfc997cc3f0eff39088eb3a683c57f4e Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 14:48:37 +0200 Subject: [PATCH 13/27] fix(wallet-integration): stop the prose contradicting the code it introduces The sentence at :379 still said "Two other error classes are not SignerError" while the bullet under it now says transport failures arrive AS SignerError with code 4000. Reviewer is right, and right that this is the fourth time in this PR that code was corrected and the prose beside it was left stale. So this was swept rather than point-fixed, and the sweep found a fifth instance I would otherwise have shipped: the testing section said a localhost frontend can reach OISY "because localhost is a secure context". Wrong mechanism -- isSecureContextUrl is applied to the SIGNER's url (urlTransport.js:54, urlFlow.js:258), not the relying party's origin, and https://oisy.com/sign is what satisfies it. The localhost part matters for WebCrypto, which is now what it says. Also fixed "identical either way" -> "whichever you choose"; there are three transports, not two. Two checks now exist for this defect class: every API identifier named in prose is resolved against the libraries and the skill's own code blocks (63 identifiers, 0 unresolved), and every prose assertion about an API is enumerated for review (26 of them). The identifier check is mechanical; the assertion list is not, and is what caught the secure-context claim. --- skills/wallet-integration/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 7d827680..2469d0b1 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -61,7 +61,7 @@ The transport URL must be a **secure context** — HTTPS, `localhost`, or `127.0 ## Pick a transport -The transport is the only part that knows *how* the wallet is reached; the `Signer` API above it is identical either way. +The transport is the only part that knows *how* the wallet is reached; the `Signer` API above it is identical whichever you choose. | Transport | Mechanism | Use when | |-----------|-----------|----------| @@ -376,7 +376,7 @@ These are the ICRC-25 codes — the only ones portable across wallets: | `4000` | Network error | Reconnecting — the library also reports every transport failure here | | `4001` | Transport channel closed | Reconnecting (signer-reported only; see below) | -Two other error classes are **not** `SignerError`, and they call for opposite reactions: +Two failures do not arrive as the class you would expect, and they call for opposite reactions: - **Transport failures arrive as `SignerError` with code `4000`.** `Signer.openChannel()` catches whatever the transport threw — `PostMessageTransportError`, `UrlTransportError`, `BrowserExtensionTransportError` — and rethrows it as a `SignerError` with the original as `cause`. So a blocked popup is *not* `instanceof PostMessageTransportError`; test `err.cause` for that. The library also never emits `4001`: "channel closed before a response" is `4000` too, and `4001` reaches you only if the signer itself returns it. - **`SignerAgentError`** — the wallet *did* respond, and the response failed validation: the returned content map did not match the call you sent (canister, method, argument, sender, nonce), the certificate did not verify against the IC root key, or the reply was absent from the certified tree. `SignerAgent` runs those checks for you, so this is a wallet returning something it should not have. Do not treat it as a connectivity fault and retry — surface it. @@ -419,7 +419,7 @@ Two other error classes are **not** `SignerError`, and they call for opposite re ## Testing against a real wallet -There is no local signer to run: OISY is a hosted wallet, and a locally deployed frontend can talk to it because `localhost` is a secure context. Test on testnet tokens rather than mainnet value. +There is no local signer to run: OISY is hosted, and the transport's secure-context requirement applies to the *signer's* URL, not to your origin — `https://oisy.com/sign` satisfies it. Serving your own frontend from `localhost` is fine, and it is a browser secure context, so WebCrypto key generation works there too. Test on testnet tokens rather than mainnet value. ```bash icp network start -d From 1a04ec9141a2ec3986ec8d267c2a94da24865a3b Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 15:04:15 +0200 Subject: [PATCH 14/27] fix(wallet-integration): give every code block its own imports Copilot's latest overview says two examples fail as copied for want of imports. Directionally right, and understated: 9 of 12 blocks referenced a symbol imported only by some other block. It also exposes a gap in how I was checking. The whole-document compile MERGES imports across blocks, so a block missing its own import passes -- the same shape as the earlier harness that passed agents in as parameters and so could not see an out-of-scope reference. Two checks now exist instead of one: - merged: no block references an identifier the skill never defines - isolated: no block references a symbol it does not import itself, and each block compiles alone with only genuinely external helpers and the narrative's earlier declarations stubbed Both clean. The repo asks for copy-paste-correct blocks because agents lift them directly, and an agent copying the ledger example alone got Signer, IcrcAccount and toCandidAccount undefined. Costs ~485 tokens in repeated import lines, taking the body to 6257. Worth it: the soft limit is already exceeded by six skills in this repo at 7.8k-12.9k, and an example that does not compile is the failure this skill exists to prevent. --- skills/wallet-integration/SKILL.md | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 2469d0b1..9fc872d2 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -82,6 +82,7 @@ const signer = new Signer({ ### Extension (ICRC-94) ```typescript +import { Signer } from '@icp-sdk/signer'; import { BrowserExtensionTransport } from '@icp-sdk/signer/extension'; async function pickExtensionSigner() { @@ -102,10 +103,10 @@ async function pickExtensionSigner() { 2. **`memoize()` is the only place a flow may await anything that is not a signer request.** Its result is journaled, so a value stays stable across the redirect. ```typescript +import { DelegationIdentity, ECDSAKeyIdentity, Ed25519KeyIdentity } from '@icp-sdk/core/identity'; +import type { Principal } from '@icp-sdk/core/principal'; import { Signer } from '@icp-sdk/signer'; import { UrlTransport } from '@icp-sdk/signer/web'; -import { DelegationIdentity, Ed25519KeyIdentity } from '@icp-sdk/core/identity'; -import type { Principal } from '@icp-sdk/core/principal'; const transport = new UrlTransport({ url: 'https://id.ai/icrc-167', @@ -140,6 +141,8 @@ async function runRedirectFlow(backend: Principal) { Skip this only if you hardcode one wallet and know what it supports. For generic integration it is the step that keeps you honest — ICRC-34 and ICRC-49 are independent, and a signer may offer either, both, or neither. ```typescript +import { Signer } from '@icp-sdk/signer'; + async function capabilities(signer: Signer) { const standards = await signer.getSupportedStandards(); // [{ name: 'ICRC-27', url }, ...] const names = new Set(standards.map(({ name }) => name)); @@ -163,7 +166,9 @@ Most signers start every scope at `ask_on_use`, prompting the first time each me | `ask_on_use` | Prompts on first use (the default) | ```typescript -import type { PermissionScope } from '@icp-sdk/signer'; +import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { Principal } from '@icp-sdk/core/principal'; +import type { PermissionScope, Signer } from '@icp-sdk/signer'; // Path A: [{ method: 'icrc27_accounts' }, { method: 'icrc49_call_canister' }] // Path B: [{ method: 'icrc27_accounts' }, { method: 'icrc34_delegation' }] @@ -193,10 +198,11 @@ async function connect(signer: Signer, scopes?: PermissionScope[]) { `SignerAgent` implements `Agent`, so it drops into anything that takes one: a ledger client from `@icp-sdk/canisters`, or an actor from `@icp-sdk/bindgen` for your own canister. Each call becomes a wallet prompt. ```typescript -import { SignerAgent } from '@icp-sdk/signer/agent'; import { IcrcLedgerCanister, toCandidAccount, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; import { HttpAgent } from '@icp-sdk/core/agent'; import { Principal } from '@icp-sdk/core/principal'; +import { Signer } from '@icp-sdk/signer'; +import { SignerAgent } from '@icp-sdk/signer/agent'; const ICP_LEDGER = Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'); @@ -219,6 +225,9 @@ async function connectLedger(signer: Signer, account: IcrcAccount) { **Read with the plain agent, write with the signer agent.** `SignerAgent.query()` upgrades every query into a full canister call routed through the wallet, so a balance check would become a user prompt and cost cycles: ```typescript +import { type IcrcAccount, toCandidAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { Signer } from '@icp-sdk/signer'; + async function showBalanceThenTransfer( signer: Signer, account: IcrcAccount, to: IcrcAccount, amount: bigint ) { @@ -248,6 +257,8 @@ The wallet delegates to a key your app generates. Afterwards you hold an ordinar ```typescript import { HttpAgent } from '@icp-sdk/core/agent'; import { DelegationIdentity, ECDSAKeyIdentity } from '@icp-sdk/core/identity'; +import { Principal } from '@icp-sdk/core/principal'; +import { Signer } from '@icp-sdk/signer'; async function startSession(signer: Signer, backend: Principal) { // Non-extractable keys cannot be exfiltrated; prefer ECDSA when you do not @@ -273,6 +284,8 @@ async function startSession(signer: Signer, backend: Principal) { `autoCloseTransportChannel` defaults to `true`: the channel closes ~200 ms after each response, so the popup does not linger. For a multi-step flow that awaits your own async work between requests, turn it off or the channel closes underneath you. ```typescript +import { Signer } from '@icp-sdk/signer'; + async function multiStepFlow(signer: Signer) { signer.autoCloseTransportChannel = false; try { @@ -289,7 +302,10 @@ async function multiStepFlow(signer: Signer) { **A connection does not survive a page reload.** There is no persistent session to restore — the channel is a live `postMessage` link to a popup that is gone. The workable pattern is to persist the account, render read-only state from it with an anonymous agent, and re-establish the signer lazily on the first write: ```typescript -import { decodeIcrcAccount, encodeIcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { type IcrcAccount, decodeIcrcAccount, encodeIcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { HttpAgent } from '@icp-sdk/core/agent'; +import { Signer } from '@icp-sdk/signer'; +import { SignerAgent } from '@icp-sdk/signer/agent'; const SESSION_KEY = 'wallet-account'; From 46dc7e2a057f8a574fa098317efcbddd47d92586 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 16:21:46 +0200 Subject: [PATCH 15/27] refactor(wallet-integration): drop delegation, and fix three things the signer maintainer flagged sea-snake (maintains @icp-sdk/signer) reviewed and asked for ICRC-34 to come out: a delegation is scoped and issued for auth purposes, not for wallet purposes with explicit approval. That is the right boundary and it removes the tension this skill had with itself. - ICRC-34 and Path B are gone. "Choose an interaction model" becomes "What the model is": every write is an individual, user-approved ICRC-49 call. Wanting a session means wanting authentication, which is now an explicit When-NOT-to-use pointing at internet-identity. This restores the boundary the old skill drew, with the right reason: ICRC-34 exists, it just is not a wallet mechanism. - The redirect example no longer uses https://id.ai/icrc-167. He noted the skill said II is not a wallet and then used II as its example; it now journals a nonce and makes an ICRC-49 call against a neutral signer URL. Pitfall 6 keeps the memoize/JSON lesson without the session key. - "Costs cycles" is wrong for reads through SignerAgent. The cost is the user approval interaction; public data (a ledger balance) needs no wallet at all, just an HttpAgent, which is anonymous by default. - The error handler now does explicit codes then a range fallback. ICRC-25 owns 1xxx/2xxx/3xxx/4xxx and names a few codes in each, so a signer may return 3002 or 4002 and the old `default: throw` would have mishandled it. Evals: the delegation case is removed (6 remain). Two expectations were encoding my own errors -- one asserted reads "cost cycles", the other demanded 4000 route to reconnect even though the skill deliberately splits it by err.cause. Full suite: WITH 20/20 | WITHOUT 7/20, triggers 6/6 and 7/7. --- evaluations/wallet-integration.json | 16 +-- skills/wallet-integration/SKILL.md | 178 ++++++++++++---------------- 2 files changed, 78 insertions(+), 116 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index f2aa5df4..9f17e2e2 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -1,6 +1,6 @@ { "skill": "wallet-integration", - "description": "Evaluation cases for the wallet-integration skill. Tests whether agents integrate an external signer with @icp-sdk/signer correctly: the right library and pins, reads off the SignerAgent, user-initiated popups, the right interaction model, and reconnect-after-reload.", + "description": "Evaluation cases for the wallet-integration skill. Tests whether agents integrate an external signer with @icp-sdk/signer correctly: the right library and pins, reads off the SignerAgent, user-initiated popups, the error class transport failures actually arrive as, and reconnect-after-reload.", "output_evals": [ { "name": "Adversarial: reaches for the superseded oisy library", @@ -16,7 +16,7 @@ "prompt": "I connected OISY with @icp-sdk/signer and built a SignerAgent. Show me how to read the user's ICRC-1 balance and how to send a transfer — just those two calls, no connection or setup boilerplate.", "expected_behaviors": [ "Reads the balance through a plain HttpAgent, NOT through the SignerAgent", - "Explains that SignerAgent turns a query into a full canister call routed through the wallet, so a read would prompt the user and cost cycles", + "Explains that SignerAgent turns a query into a full canister call routed through the wallet, so a read would cost the user an approval interaction", "Sends the transfer through the SignerAgent" ] }, @@ -29,16 +29,6 @@ "Does NOT suggest working around it by disabling the check, raising a timeout, or retrying" ] }, - { - "name": "Requesting a delegation safely", - "prompt": "My IC dapp is a turn-based game with several moves per minute, so I'm using session delegation with @icp-sdk/signer rather than per-action approval. What do I need to get right about the delegation I request, and what does the library check for me? Short list, no code.", - "expected_behaviors": [ - "Says to scope the delegation by passing targets", - "Says to bound its lifetime with maxTimeToLive", - "States that requestDelegation already validates the wallet's response and throws — the chain must terminate at the requested public key, targets must not come back broader than requested, and it must not outlive maxTimeToLive", - "Does NOT tell the developer to hand-roll delegation-chain verification" - ] - }, { "name": "Adversarial: signer 6 against a core ^5 project", "prompt": "My dapp's package.json pins \"@icp-sdk/canisters\": \"^3\" and \"@icp-sdk/core\": \"^5\". Give me the npm install command to add @icp-sdk/signer so I can integrate OISY. No integration code.", @@ -64,7 +54,7 @@ "expected_behaviors": [ "States that Signer wraps the transport error into a SignerError with code 4000, so an instanceof check on the thrown error cannot match", "Says the original transport error is available as err.cause", - "Handles code 4000 as the reconnect case, not only 4001", + "Handles code 4000, not only 4001 (either as reconnect, or split by err.cause into popup-blocked vs reconnect)", "Does NOT claim signer methods throw PostMessageTransportError directly" ] } diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 9fc872d2..9fa13b40 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -1,6 +1,6 @@ --- name: wallet-integration -description: "Integrate an external wallet (signer) into an IC dapp with @icp-sdk/signer — the relying-party side of the ICRC signer standards. Covers picking a transport (popup via ICRC-29 / top-level redirect via ICRC-167 / browser extension via ICRC-94) and then negotiating capabilities and driving the permission and account lifecycle. Shows both interaction models: per-action approval through SignerAgent (ICRC-49) and session delegation (ICRC-34). Uses OISY as the worked example but applies to any ICRC-25 signer. Do NOT use for Internet Identity login (use the internet-identity skill) or for implementing a wallet yourself. Use when the developer mentions wallet integration or OISY or @icp-sdk/signer or approving a transaction in a wallet popup." +description: "Integrate an external wallet (signer) into an IC dapp with @icp-sdk/signer — the relying-party side of the ICRC signer standards. Covers picking a transport (popup via ICRC-29 / top-level redirect via ICRC-167 / browser extension via ICRC-94) then negotiating capabilities, the permission and account lifecycle, and executing canister calls the user approves through SignerAgent (ICRC-49). Uses OISY as the worked example but applies to any ICRC-25 signer. Do NOT use for Internet Identity login (use the internet-identity skill) or for implementing a wallet yourself. Use when the developer mentions wallet integration or OISY or @icp-sdk/signer or approving a transaction in a wallet popup." license: Apache-2.0 compatibility: "Node.js >= 22, a browser (secure context: HTTPS, localhost, or 127.0.0.1)" metadata: @@ -23,28 +23,20 @@ Examples use [OISY](https://oisy.com) (`https://oisy.com/sign`), but nothing her | ICRC-25 | Capability discovery + permission lifecycle | `getSupportedStandards`, `requestPermissions`, `getPermissions` | | ICRC-27 | The user's accounts | `getAccounts` | | ICRC-29 | Popup transport over `postMessage` | `PostMessageTransport` | -| ICRC-34 | Session delegation | `requestDelegation` | | ICRC-49 | Execute a canister call | `callCanister`, `SignerAgent` | | ICRC-94 | Browser-extension discovery | `BrowserExtensionTransport.discover` | | ICRC-95 | Identity derivation origin | `derivationOrigin` option | | ICRC-167 | Top-level redirect transport | `UrlTransport` (**new in signer 6**) | -## Choose an interaction model first +## What the model is -Everything downstream follows from this choice. +Every write is an individual, user-approved act. Your app asks the wallet to execute one canister call (ICRC-49); the wallet shows the user what it is about to do, the user approves, and the wallet signs and submits it. Your app never holds a key and never acts on the user's behalf unattended. -| | **Path A — per-action approval** (ICRC-49) | **Path B — session delegation** (ICRC-34) | -|---|---|---| -| The user approves | every write, individually | once, at sign-in | -| Your app holds | no key — the wallet signs | a session key the wallet delegated to | -| You end up with | a `SignerAgent` | an ordinary `HttpAgent` + `DelegationIdentity` | -| Good for | transfers, approvals, mints — deliberate, high-value acts | games, social, frequent writes, background work | -| Bad for | anything frequent; each call is a popup | acts a user should consciously confirm | - -A signer may support one and not the other — negotiate before you commit (see below). You can also combine them: a delegation for your own canister, per-action approval for token transfers. +That is the whole shape, and it is what makes wallet integration appropriate for deliberate, high-value actions — transfers, approvals, mints — and inappropriate for anything frequent or invisible. If a user should not see a prompt per action, you do not want a wallet; you want authentication. ### When NOT to use this skill +- **A session — sign in once, then act many times.** That is authentication, not wallet integration: a delegation is scoped and issued by an identity provider. Use the **internet-identity** skill. ICRC-34 exists, but it is an auth mechanism and out of scope here. - **Internet Identity sign-in** → the **internet-identity** skill. II is an identity provider, not an ICRC-25 signer. - **Letting an agent or CLI act as the user** → the **agent-web-identity** skill. - **Building a wallet** → out of scope, as above. @@ -103,13 +95,13 @@ async function pickExtensionSigner() { 2. **`memoize()` is the only place a flow may await anything that is not a signer request.** Its result is journaled, so a value stays stable across the redirect. ```typescript -import { DelegationIdentity, ECDSAKeyIdentity, Ed25519KeyIdentity } from '@icp-sdk/core/identity'; +import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; import type { Principal } from '@icp-sdk/core/principal'; import { Signer } from '@icp-sdk/signer'; import { UrlTransport } from '@icp-sdk/signer/web'; const transport = new UrlTransport({ - url: 'https://id.ai/icrc-167', + url: 'https://wallet.example.com/icrc-167', // Absolute, fragment-free, on an origin you control, and listed in that // origin's /.well-known/ii-auth-callbacks allow-list. callbackUrl: 'https://app.example.com/signer-callback' @@ -117,28 +109,31 @@ const transport = new UrlTransport({ // Run this on the load of the callback route: a fresh arrival starts the flow, // the wallet's return replays it. No separate resume or cleanup call. -async function runRedirectFlow(backend: Principal) { +async function runRedirectFlow( + account: IcrcAccount, canisterId: Principal, arg: Uint8Array +) { const signer = new Signer({ transport }); - // The session key must survive the redirect, so journal it. Ed25519KeyIdentity - // is used because toJSON() is JSON-serializable; ECDSAKeyIdentity holds - // CryptoKeys that are not (see pitfall 6). - const sessionKeyJson = await transport.memoize(() => - JSON.stringify(Ed25519KeyIdentity.generate().toJSON()) + // Non-request async work goes through memoize() so its result survives the + // redirect. It persists via JSON, so hand it something serializable — a + // Uint8Array is not (see pitfall 6). + const nonceBytes = await transport.memoize(async () => + Array.from(await fetchNonceFromYourBackend()) ); - const sessionKey = Ed25519KeyIdentity.fromJSON(sessionKeyJson); - const chain = await signer.requestDelegation({ - publicKey: sessionKey.getPublicKey(), - targets: [backend] + return signer.callCanister({ + canisterId, + sender: account.owner, + method: 'icrc1_transfer', + arg, + nonce: Uint8Array.from(nonceBytes) }); - return DelegationIdentity.fromDelegation(sessionKey, chain); } ``` ## Negotiate capabilities -Skip this only if you hardcode one wallet and know what it supports. For generic integration it is the step that keeps you honest — ICRC-34 and ICRC-49 are independent, and a signer may offer either, both, or neither. +Skip this only if you hardcode one wallet and know what it supports. For generic integration it is the step that keeps you honest: a signer may expose accounts without executing calls, or support a transport you have not built for. ```typescript import { Signer } from '@icp-sdk/signer'; @@ -147,8 +142,8 @@ async function capabilities(signer: Signer) { const standards = await signer.getSupportedStandards(); // [{ name: 'ICRC-27', url }, ...] const names = new Set(standards.map(({ name }) => name)); return { - canCallCanisters: names.has('ICRC-49'), // Path A - canDelegate: names.has('ICRC-34') // Path B + canListAccounts: names.has('ICRC-27'), + canCallCanisters: names.has('ICRC-49') }; } ``` @@ -170,8 +165,8 @@ import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; import { Principal } from '@icp-sdk/core/principal'; import type { PermissionScope, Signer } from '@icp-sdk/signer'; -// Path A: [{ method: 'icrc27_accounts' }, { method: 'icrc49_call_canister' }] -// Path B: [{ method: 'icrc27_accounts' }, { method: 'icrc34_delegation' }] +// The two scopes this skill uses: +// [{ method: 'icrc27_accounts' }, { method: 'icrc49_call_canister' }] async function connect(signer: Signer, scopes?: PermissionScope[]) { // Omit `scopes` to leave every method on ask_on_use. Supply them to trade // several later prompts for one up front. Ask only for what your path uses: @@ -193,7 +188,7 @@ async function connect(signer: Signer, scopes?: PermissionScope[]) { `getPermissions()` reads the current state without prompting. Do not cache it across sessions — a wallet may expire grants, after which they silently revert to `ask_on_use`. -## Path A — per-action approval +## Executing a call the user approves `SignerAgent` implements `Agent`, so it drops into anything that takes one: a ledger client from `@icp-sdk/canisters`, or an actor from `@icp-sdk/bindgen` for your own canister. Each call becomes a wallet prompt. @@ -215,6 +210,7 @@ async function connectLedger(signer: Signer, account: IcrcAccount) { const signerAgent = await SignerAgent.create({ signer, account: account.owner, agent }); return { + // Anonymous by default — public reads need no wallet and no prompt. read: IcrcLedgerCanister.create({ agent, canisterId: ICP_LEDGER }), write: IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ICP_LEDGER }), signerAgent @@ -222,7 +218,7 @@ async function connectLedger(signer: Signer, account: IcrcAccount) { } ``` -**Read with the plain agent, write with the signer agent.** `SignerAgent.query()` upgrades every query into a full canister call routed through the wallet, so a balance check would become a user prompt and cost cycles: +**Read with the plain agent, write with the signer agent.** `SignerAgent.query()` upgrades every query into a full canister call routed through the wallet, so a balance check would put an approval prompt in front of the user. Public data does not need the wallet at all — read it with an ordinary `HttpAgent`, which is anonymous by default: ```typescript import { type IcrcAccount, toCandidAccount } from '@icp-sdk/canisters/ledger/icrc'; @@ -234,7 +230,7 @@ async function showBalanceThenTransfer( const { read, write } = await connectLedger(signer, account); // balance() takes an IcrcAccount directly. - const balance = await read.balance(account); // silent + const balance = await read.balance(account); // no prompt const block = await write.transfer({ // prompts // The ledger's `to` is the Candid shape; convert rather than hand-roll it. @@ -250,35 +246,6 @@ async function showBalanceThenTransfer( `signerAgent.replaceAccount(principal)` switches which principal later writes are signed for, without rebuilding the agent. -## Path B — session delegation - -The wallet delegates to a key your app generates. Afterwards you hold an ordinary `HttpAgent` and the wallet is not involved again until the delegation expires. - -```typescript -import { HttpAgent } from '@icp-sdk/core/agent'; -import { DelegationIdentity, ECDSAKeyIdentity } from '@icp-sdk/core/identity'; -import { Principal } from '@icp-sdk/core/principal'; -import { Signer } from '@icp-sdk/signer'; - -async function startSession(signer: Signer, backend: Principal) { - // Non-extractable keys cannot be exfiltrated; prefer ECDSA when you do not - // need to serialize the key (see pitfall 6 for the redirect case). - const sessionKey = await ECDSAKeyIdentity.generate(); - - const chain = await signer.requestDelegation({ - publicKey: sessionKey.getPublicKey(), - // Scope it. Omitting targets asks for a delegation valid for ANY canister. - targets: [backend], - maxTimeToLive: BigInt(8) * BigInt(3_600_000_000_000) // 8 hours, in nanoseconds - }); - - const identity = DelegationIdentity.fromDelegation(sessionKey, chain); - return HttpAgent.create({ identity }); -} -``` - -`requestDelegation` **validates the wallet's response before returning** and throws if the chain does not terminate at your public key, if `targets` come back broader than requested, or if it outlives `maxTimeToLive`. Do not reimplement those checks, and do not swallow the throw — it is the guard against a malicious or buggy signer widening your delegation. - ## Channel lifecycle and page reloads `autoCloseTransportChannel` defaults to `true`: the channel closes ~200 ms after each response, so the popup does not linger. For a multi-step flow that awaits your own async work between requests, turn it off or the channel closes underneath you. @@ -311,7 +278,7 @@ const SESSION_KEY = 'wallet-account'; // On connect: remember the account, not the channel. The ICRC-1 textual // encoding round-trips owner and subaccount as one string, so the -// subaccount survives the reload too (see pitfall 12). +// subaccount survives the reload too (see pitfall 11). function rememberAccount(account: IcrcAccount) { sessionStorage.setItem(SESSION_KEY, encodeIcrcAccount(account)); } @@ -360,37 +327,44 @@ async function safeTransfer( try { await showBalanceThenTransfer(signer, account, to, amount); } catch (err) { - if (err instanceof SignerError) { - switch (err.code) { - case 3001: return; // user cancelled — not a failure - case 3000: showPermissionHelp(); return; // permission denied - case 2000: showUnsupported(); return; // wallet does not support the method - case 4000: // every transport failure lands here - case 4001: // only if the signer itself returns it - // The transport error is the `cause`, never the error you caught. - if (err.cause instanceof PostMessageTransportError) showPopupBlockedHelp(); - else promptReconnect(); - return; - default: throw err; - } + // Anything that is not a SignerError — SignerAgentError above all, where the + // wallet responded and the response failed validation — is not handled here. + if (!(err instanceof SignerError)) throw err; + + switch (err.code) { + case 3001: return; // user cancelled — not a failure + case 3000: showPermissionHelp(); return; // permission denied + case 2000: showUnsupported(); return; // wallet does not support the method + case 4000: // every transport failure lands here + case 4001: // only if the signer itself returns it + // The transport error is the `cause`, never the error you caught. + if (err.cause instanceof PostMessageTransportError) showPopupBlockedHelp(); + else promptReconnect(); + return; + } + + // ICRC-25 numbers errors by range and a signer may return a code this + // switch has never seen, so fall back on the range rather than rethrowing. + switch (Math.floor(err.code / 1000)) { + case 3: return; // 3xxx user action — nothing broke + case 2: showUnsupported(); return; // 2xxx not supported + case 4: promptReconnect(); return; // 4xxx transport channel + default: throw err; // 1xxx generic, and anything else } - // Anything else — including SignerAgentError, where the wallet responded - // but the response failed validation — is not a connectivity fault. - throw err; } } ``` -These are the ICRC-25 codes — the only ones portable across wallets: +ICRC-25 groups errors into ranges and names a few codes inside each. Handle the codes you know, then fall back on the range — a signer may return `3002` or `4002`, and a bare `default: throw` would mishandle it: -| Code | Meaning | Handle by | -|------|---------|-----------| -| `1000` | Generic error | Surfacing `err.data` to developers | -| `2000` | Not supported | Negotiating capabilities first | -| `3000` | Permission not granted | Explaining what to re-grant | -| `3001` | **Action aborted — the user cancelled** | Returning quietly; this is normal | -| `4000` | Network error | Reconnecting — the library also reports every transport failure here | -| `4001` | Transport channel closed | Reconnecting (signer-reported only; see below) | +| Range | Code | Meaning | Handle by | +|-------|------|---------|-----------| +| `1xxx` general | `1000` | Generic error | Surfacing `err.data` to developers | +| `2xxx` not supported | `2000` | Not supported | Negotiating capabilities first | +| `3xxx` user action | `3000` | Permission not granted | Explaining what to re-grant | +| | `3001` | **Action aborted — the user cancelled** | Returning quietly; this is normal | +| `4xxx` transport | `4000` | Network error | Reconnecting — the library also reports every transport failure here | +| | `4001` | Transport channel closed | Reconnecting (signer-reported only; see below) | Two failures do not arrive as the class you would expect, and they call for opposite reactions: @@ -409,29 +383,27 @@ Two failures do not arrive as the class you would expect, and they call for oppo button.addEventListener('click', () => signer.getAccounts()); ``` -2. **Reading through `SignerAgent`.** `query()` is upgraded to an update call routed through the wallet, so every read prompts the user and costs cycles. Reads go through a plain `HttpAgent`; only writes go through `SignerAgent`. - -3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the account — both halves, per pitfall 12 — for read-only rendering, and reconnect on first write. See above. +2. **Reading through `SignerAgent`.** `query()` is upgraded to an update call routed through the wallet, so every read costs the user an approval interaction. Public data — a ledger balance, token metadata — needs no wallet: read it with a plain `HttpAgent`, anonymous by default. Only writes go through `SignerAgent`. -4. **Assuming a wallet's capabilities.** Call `getSupportedStandards()`. A wallet that executes canister calls (ICRC-49) may not issue delegations (ICRC-34), and vice versa. +3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the account — both halves, per pitfall 11 — for read-only rendering, and reconnect on first write. See above. -5. **Coding against one wallet's non-standard error codes.** Only `1000`/`2000`/`3000`/`3001`/`4000`/`4001` are ICRC-25. Vendor extensions outside that range are not portable — earlier revisions of this skill documented a `503 BUSY` code that exists only in `@dfinity/oisy-wallet-signer` and in no standard. Branch on the standard codes and treat the rest as generic. +4. **Assuming a wallet's capabilities.** Call `getSupportedStandards()`. A signer may list accounts (ICRC-27) without executing calls (ICRC-49), or speak a transport you have not built for. -6. **Journaling a non-serializable session key through `memoize()`.** `memoize` persists via JSON, so `ECDSAKeyIdentity` cannot cross a redirect — its `getKeyPair()` returns `CryptoKey`s that `JSON.stringify` silently reduces to `{}`, and the flow fails on return with a key it cannot sign with. Use `Ed25519KeyIdentity` (`toJSON`/`fromJSON`) for `UrlTransport` flows; prefer non-extractable `ECDSAKeyIdentity` for popup flows, where nothing needs serializing. +5. **Coding against one wallet's non-standard error codes.** ICRC-25 owns `1xxx`–`4xxx` and names `1000`/`2000`/`3000`/`3001`/`4000`/`4001` inside them. A code outside those ranges is a vendor extension and does not port — earlier revisions of this skill documented a `503 BUSY` that exists only in `@dfinity/oisy-wallet-signer` and in no standard. Branch on the named codes, fall back on the range, and treat anything outside the ranges as generic. -7. **Requesting an unscoped delegation.** Omitting `targets` asks for a delegation valid against *any* canister. Always pass the canisters you actually call. +6. **Journaling something non-serializable through `memoize()`.** It persists via JSON, so a `Uint8Array` round-trips as `{"0":1,"1":2,…}` and a `CryptoKey` as `{}` — silently, with the flow failing on return rather than at the call. Convert to a plain array (or hex) before memoizing and back afterwards. -8. **Diverging on a redirect replay.** With `UrlTransport`, issue the same requests and `memoize` steps in the same order on every load, and route anything a request depends on — a nonce above all — through `memoize`. Re-fetching a single-use value on the return load invalidates the flow. +7. **Diverging on a redirect replay.** With `UrlTransport`, issue the same requests and `memoize` steps in the same order on every load, and route anything a request depends on — a nonce above all — through `memoize`. Re-fetching a single-use value on the return load invalidates the flow. -9. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. +8. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. -10. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. +9. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. -11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5` or older, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. +10. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5` or older, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. -12. **Treating an account as just a principal.** `getAccounts()` returns `{ owner, subaccount? }` — an `IcrcAccount`. The subaccount is usually absent, because signers commonly offer only the default one, so code that assumes a bare principal works until it meets a signer that does not. Carry the account whole and let the library helpers do the rest: **compare** with `encodeIcrcAccount()` and never `owner` alone (that encoding normalizes the default subaccount, so the same principal with a *different* one is correctly a different account), **persist** with `encodeIcrcAccount()` / `decodeIcrcAccount()`, and **send** with `from_subaccount` for the sender plus `toCandidAccount()` for the recipient. `SignerAgent` is the exception — its `account` is a `Principal`, which is why the subaccount travels in the ledger call arguments instead. +11. **Treating an account as just a principal.** `getAccounts()` returns `{ owner, subaccount? }` — an `IcrcAccount`. The subaccount is usually absent, because signers commonly offer only the default one, so code that assumes a bare principal works until it meets a signer that does not. Carry the account whole and let the library helpers do the rest: **compare** with `encodeIcrcAccount()` and never `owner` alone (that encoding normalizes the default subaccount, so the same principal with a *different* one is correctly a different account), **persist** with `encodeIcrcAccount()` / `decodeIcrcAccount()`, and **send** with `from_subaccount` for the sender plus `toCandidAccount()` for the recipient. `SignerAgent` is the exception — its `account` is a `Principal`, which is why the subaccount travels in the ledger call arguments instead. -13. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. +12. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. ## Testing against a real wallet @@ -460,7 +432,7 @@ Set `host: 'https://icp-api.io'` on the agent even when serving from `localhost` ## Additional References -- **internet-identity** — II sign-in, delegation-based auth for your own app +- **internet-identity** — II sign-in, and the place to go if you need a session rather than per-action approval - **agent-web-identity** — letting an agent or CLI act as the user in an II app - **icp-cli** — `@icp-sdk/bindgen` actors to call your own canister through a `SignerAgent` - **canister-security** — verifying `msg.caller` on the backend once calls arrive From 0f8503630cc918886cde4a64a5774214732ff7d3 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 16:30:14 +0200 Subject: [PATCH 16/27] fix(wallet-integration): stop indexing getAccounts() blindly connect() ended in `return accounts[0]`. ICRC-27 defines `accounts` as a vec with no minimum, and says the signer MAY prompt the user to select which accounts to share, returning "the list of accounts selected by the user" -- so an empty list means declined, not failed, and a list of several means the user should pick. `accounts[0]` on an empty list is undefined and the crash lands later at `.owner`, away from the cause. connect() now returns the list, and pitfall 4 covers both the empty and the multiple case. This is the "empty-account handling" an earlier Copilot overview named without ever rendering a finding for it; I had dismissed it as part of the subaccount thread. --- skills/wallet-integration/SKILL.md | 40 ++++++++++++++++-------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 9fa13b40..50ae3c1b 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -116,7 +116,7 @@ async function runRedirectFlow( // Non-request async work goes through memoize() so its result survives the // redirect. It persists via JSON, so hand it something serializable — a - // Uint8Array is not (see pitfall 6). + // Uint8Array is not (see pitfall 7). const nonceBytes = await transport.memoize(async () => Array.from(await fetchNonceFromYourBackend()) ); @@ -177,12 +177,14 @@ async function connect(signer: Signer, scopes?: PermissionScope[]) { await signer.requestPermissions(scopes); } - const accounts = await signer.getAccounts(); - // { owner: Principal, subaccount?: Uint8Array } — already an IcrcAccount. - // Usually there is no subaccount: signers commonly offer only the default - // one. Return the element whole anyway — it costs nothing, and a signer - // that does offer subaccounts breaks code that assumed otherwise. - return accounts[0]; + // ICRC-27 returns the accounts the user chose to share, as a list: it can be + // empty (they declined) and it can hold several. Hand it back whole and let + // the user pick rather than indexing blindly — see pitfall 4. + // + // Each element is { owner: Principal, subaccount?: Uint8Array } — already an + // IcrcAccount. Usually there is no subaccount, since signers commonly offer + // only the default one, but keep both halves anyway: it costs nothing. + return signer.getAccounts(); } ``` @@ -278,7 +280,7 @@ const SESSION_KEY = 'wallet-account'; // On connect: remember the account, not the channel. The ICRC-1 textual // encoding round-trips owner and subaccount as one string, so the -// subaccount survives the reload too (see pitfall 11). +// subaccount survives the reload too (see pitfall 12). function rememberAccount(account: IcrcAccount) { sessionStorage.setItem(SESSION_KEY, encodeIcrcAccount(account)); } @@ -385,25 +387,27 @@ Two failures do not arrive as the class you would expect, and they call for oppo 2. **Reading through `SignerAgent`.** `query()` is upgraded to an update call routed through the wallet, so every read costs the user an approval interaction. Public data — a ledger balance, token metadata — needs no wallet: read it with a plain `HttpAgent`, anonymous by default. Only writes go through `SignerAgent`. -3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the account — both halves, per pitfall 11 — for read-only rendering, and reconnect on first write. See above. +3. **Expecting a connection to survive a reload.** No channel outlives the page. Persist the account — both halves, per pitfall 12 — for read-only rendering, and reconnect on first write. See above. -4. **Assuming a wallet's capabilities.** Call `getSupportedStandards()`. A signer may list accounts (ICRC-27) without executing calls (ICRC-49), or speak a transport you have not built for. +4. **Indexing `getAccounts()` blindly.** It returns a *list* of the accounts the user chose to share. ICRC-27 lets the signer prompt for that selection, so the list can be **empty** — the user declined, which is not an error — and it can hold **several**, where `[0]` silently picks for them. `accounts[0]` on an empty list is `undefined`, so the crash lands later at `.owner` rather than at the call. Check the length, and offer a picker when there is more than one. -5. **Coding against one wallet's non-standard error codes.** ICRC-25 owns `1xxx`–`4xxx` and names `1000`/`2000`/`3000`/`3001`/`4000`/`4001` inside them. A code outside those ranges is a vendor extension and does not port — earlier revisions of this skill documented a `503 BUSY` that exists only in `@dfinity/oisy-wallet-signer` and in no standard. Branch on the named codes, fall back on the range, and treat anything outside the ranges as generic. +5. **Assuming a wallet's capabilities.** Call `getSupportedStandards()`. A signer may list accounts (ICRC-27) without executing calls (ICRC-49), or speak a transport you have not built for. -6. **Journaling something non-serializable through `memoize()`.** It persists via JSON, so a `Uint8Array` round-trips as `{"0":1,"1":2,…}` and a `CryptoKey` as `{}` — silently, with the flow failing on return rather than at the call. Convert to a plain array (or hex) before memoizing and back afterwards. +6. **Coding against one wallet's non-standard error codes.** ICRC-25 owns `1xxx`–`4xxx` and names `1000`/`2000`/`3000`/`3001`/`4000`/`4001` inside them. A code outside those ranges is a vendor extension and does not port — earlier revisions of this skill documented a `503 BUSY` that exists only in `@dfinity/oisy-wallet-signer` and in no standard. Branch on the named codes, fall back on the range, and treat anything outside the ranges as generic. -7. **Diverging on a redirect replay.** With `UrlTransport`, issue the same requests and `memoize` steps in the same order on every load, and route anything a request depends on — a nonce above all — through `memoize`. Re-fetching a single-use value on the return load invalidates the flow. +7. **Journaling something non-serializable through `memoize()`.** It persists via JSON, so a `Uint8Array` round-trips as `{"0":1,"1":2,…}` and a `CryptoKey` as `{}` — silently, with the flow failing on return rather than at the call. Convert to a plain array (or hex) before memoizing and back afterwards. -8. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. +8. **Diverging on a redirect replay.** With `UrlTransport`, issue the same requests and `memoize` steps in the same order on every load, and route anything a request depends on — a nonce above all — through `memoize`. Re-fetching a single-use value on the return load invalidates the flow. -9. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. +9. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. -10. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5` or older, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. +10. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. -11. **Treating an account as just a principal.** `getAccounts()` returns `{ owner, subaccount? }` — an `IcrcAccount`. The subaccount is usually absent, because signers commonly offer only the default one, so code that assumes a bare principal works until it meets a signer that does not. Carry the account whole and let the library helpers do the rest: **compare** with `encodeIcrcAccount()` and never `owner` alone (that encoding normalizes the default subaccount, so the same principal with a *different* one is correctly a different account), **persist** with `encodeIcrcAccount()` / `decodeIcrcAccount()`, and **send** with `from_subaccount` for the sender plus `toCandidAccount()` for the recipient. `SignerAgent` is the exception — its `account` is a `Principal`, which is why the subaccount travels in the ledger call arguments instead. +11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5` or older, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. -12. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. +12. **Treating an account as just a principal.** `getAccounts()` returns `{ owner, subaccount? }` — an `IcrcAccount`. The subaccount is usually absent, because signers commonly offer only the default one, so code that assumes a bare principal works until it meets a signer that does not. Carry the account whole and let the library helpers do the rest: **compare** with `encodeIcrcAccount()` and never `owner` alone (that encoding normalizes the default subaccount, so the same principal with a *different* one is correctly a different account), **persist** with `encodeIcrcAccount()` / `decodeIcrcAccount()`, and **send** with `from_subaccount` for the sender plus `toCandidAccount()` for the recipient. `SignerAgent` is the exception — its `account` is a `Principal`, which is why the subaccount travels in the ledger call arguments instead. + +13. **Firing a call immediately after connecting.** Let the user initiate. An unprompted approval dialog straight after connect reads as an attack, and wallets are within their rights to reject it. ## Testing against a real wallet From dd98b8fe0a527e59803bac870df8f4d5bacf5e9c Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 16:31:33 +0200 Subject: [PATCH 17/27] fix(wallet-integration): give an unnamed 3xxx feedback instead of silence The range fallback returned bare on any 3xxx. 3001 is the one code where silence is correct -- there you know the user cancelled deliberately. For an unnamed 3xxx the user did something the app has no specific message for, and returning silently leaves the UI unchanged after they pressed the button. It now tells them the action did not go through. Resolves the open question left on sea-snake's error-range thread. --- skills/wallet-integration/SKILL.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 50ae3c1b..27a6aa9c 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -348,7 +348,13 @@ async function safeTransfer( // ICRC-25 numbers errors by range and a signer may return a code this // switch has never seen, so fall back on the range rather than rethrowing. switch (Math.floor(err.code / 1000)) { - case 3: return; // 3xxx user action — nothing broke + case 3: + // 3xxx is "user action", so nothing broke — but 3001 is the only code + // where silence is right, because there you know they cancelled on + // purpose. For an unnamed 3xxx say the action did not go through, or + // the UI sits unchanged after the user pressed the button. + showActionNotCompleted(); + return; case 2: showUnsupported(); return; // 2xxx not supported case 4: promptReconnect(); return; // 4xxx transport channel default: throw err; // 1xxx generic, and anything else From 47e202960844ec0aeb6a550b32a97c18fba792bb Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 16:51:09 +0200 Subject: [PATCH 18/27] fix(wallet-integration): note that callCanister returns unverified output Independent review, and the important one of three findings. Signer.callCanister validates shape only -- that contentMap and certificate are present and base64-decodable (signer.js:519-539). The content-map match against the request sent and the certificate check against the root key live exclusively in SignerAgent (agent.js:130,143), which is why a SignerAgent call can raise SignerAgentError at all. The skill said that correctly in the error section and then had runRedirectFlow hand raw { contentMap, certificate } back to its caller -- teaching an agent to trust exactly what that section warns about. Same shape as the earlier prose/code splits, but security-relevant rather than cosmetic. Now stated at the call site and in a note after the block, with SignerAgent named as the alternative that verifies. Two nits from the same review: - the permission-state table still called ask_on_use "the default" two lines under the prose saying ICRC-25 leaves it to signer policy - three blocks carried unused imports, because the script that gave every block its own imports matched identifiers inside comments. The per-block check now strips comments, and tsc --noUnusedLocals is clean. Evals: the suite had lost coverage while gaining behaviour. Two cases added -- getAccounts() returning a list (3/3 vs 0/3) and an unnamed ICRC-25 code (3/3 vs 0/3). --- evaluations/wallet-integration.json | 18 ++++++++++++++++++ skills/wallet-integration/SKILL.md | 12 +++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 9f17e2e2..2aaaa314 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -38,6 +38,15 @@ "Does NOT recommend --legacy-peer-deps or --force to get past the peer conflict" ] }, + { + "name": "Adversarial: getAccounts() returns a list, not an account", + "prompt": "I'm connecting OISY with @icp-sdk/signer and I need the user's account so I can show their balance. Show me just the connect function — no ledger setup, no transfer.", + "expected_behaviors": [ + "Does NOT index getAccounts() as accounts[0] without handling the list being empty", + "States or handles that the list can be empty because the user may decline to share any account", + "Treats more than one account as possible rather than silently picking the first" + ] + }, { "name": "Connection does not survive a page reload", "prompt": "After a page refresh my OISY connection is gone and the balances disappear until the user clicks connect again. How should I handle this with @icp-sdk/signer? Describe the approach, no code.", @@ -57,6 +66,15 @@ "Handles code 4000, not only 4001 (either as reconnect, or split by err.cause into popup-blocked vs reconnect)", "Does NOT claim signer methods throw PostMessageTransportError directly" ] + }, + { + "name": "Adversarial: an ICRC-25 code the table does not name", + "prompt": "My wallet returned a SignerError with code 3002, which isn't one of the codes listed in ICRC-25. I'm using @icp-sdk/signer. How should my catch block deal with codes it doesn't recognise? Short answer.", + "expected_behaviors": [ + "Explains that ICRC-25 assigns meaning by range, not only by the named codes, so 3002 inherits the 3xxx meaning", + "Handles 3002 as a user-action outcome rather than as a failure to rethrow", + "Recommends falling back on the range after the named codes, instead of a bare default that rethrows" + ] } ], "trigger_evals": { diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 27a6aa9c..ec3163cc 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -121,6 +121,9 @@ async function runRedirectFlow( Array.from(await fetchNonceFromYourBackend()) ); + // callCanister is the raw ICRC-49 primitive: it checks only that the reply + // carries a contentMap and a certificate. Verifying them is the caller's + // job — see below. return signer.callCanister({ canisterId, sender: account.owner, @@ -131,6 +134,8 @@ async function runRedirectFlow( } ``` +**`callCanister` returns unverified wallet output.** It resolves with the CBOR `{ contentMap, certificate }` and validates only that both are present and decodable — it does not check the content map against the call you sent, and it does not verify the certificate against the IC root key. Those checks live in `SignerAgent`, which is why a `SignerAgent` call can raise `SignerAgentError`. If you use `callCanister` directly, verify the certificate before you trust the reply; if you would rather not, route the call through `SignerAgent` and let it do this for you. + ## Negotiate capabilities Skip this only if you hardcode one wallet and know what it supports. For generic integration it is the step that keeps you honest: a signer may expose accounts without executing calls, or support a transport you have not built for. @@ -158,11 +163,9 @@ Most signers start every scope at `ask_on_use`, prompting the first time each me |-------|-----------| | `granted` | Proceeds without prompting | | `denied` | Rejected immediately with error `3000` | -| `ask_on_use` | Prompts on first use (the default) | +| `ask_on_use` | Prompts on first use (the usual initial state) | ```typescript -import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; -import { Principal } from '@icp-sdk/core/principal'; import type { PermissionScope, Signer } from '@icp-sdk/signer'; // The two scopes this skill uses: @@ -195,7 +198,7 @@ async function connect(signer: Signer, scopes?: PermissionScope[]) { `SignerAgent` implements `Agent`, so it drops into anything that takes one: a ledger client from `@icp-sdk/canisters`, or an actor from `@icp-sdk/bindgen` for your own canister. Each call becomes a wallet prompt. ```typescript -import { IcrcLedgerCanister, toCandidAccount, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { IcrcLedgerCanister, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; import { HttpAgent } from '@icp-sdk/core/agent'; import { Principal } from '@icp-sdk/core/principal'; import { Signer } from '@icp-sdk/signer'; @@ -320,7 +323,6 @@ Treat "disconnect" as clearing your own state — there is no wallet-side logout ```typescript import { Signer, SignerError } from '@icp-sdk/signer'; import { PostMessageTransportError } from '@icp-sdk/signer/web'; -import { Principal } from '@icp-sdk/core/principal'; import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; async function safeTransfer( From 3271c1c43d7cb43879296a8b4860c2b3d2a08d61 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 17:10:14 +0200 Subject: [PATCH 19/27] fix(wallet-integration): omitting scopes does not set ask_on_use Copilot review, and it is right. connect()'s comment said "Omit `scopes` to leave every method on ask_on_use", which contradicted the prose two lines up saying ICRC-25 leaves the initial state to signer policy -- and was wrong on its own terms. Omitting requestPermissions sets nothing: it leaves whatever states the signer already holds for your origin, and ICRC-25 keeps those per relying party (:149, :235), so a previous session may have left a scope granted or denied. The comment now says that, and that getPermissions() is the only way to know. Two eval findings from the same review, both valid: - The reload expectation said "persists the account", which an answer keeping only the owner principal could satisfy -- so it would not have caught a regression of the subaccount-loss bug it exists to guard. Now requires the whole account to round-trip: WITH 4/4 | WITHOUT 2/4, and the baseline dropping from 3/4 shows the tightening is real. - No eval exercised the redirect rules. Added one for the two failure modes that survive whatever happens to the worked example: a value re-fetched instead of journaled, and memoize persisting via JSON: WITH 2/2 | WITHOUT 0/2. --- evaluations/wallet-integration.json | 10 +++++++++- skills/wallet-integration/SKILL.md | 7 +++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 2aaaa314..5d9c60de 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -52,7 +52,7 @@ "prompt": "After a page refresh my OISY connection is gone and the balances disappear until the user clicks connect again. How should I handle this with @icp-sdk/signer? Describe the approach, no code.", "expected_behaviors": [ "States that the transport channel cannot survive a reload — there is no wallet session to restore", - "Persists the account and renders read-only state from it with an ordinary (anonymous) agent, without opening the wallet", + "Persists the whole account rather than just the owner principal, so a subaccount survives the reload, and renders read-only state from it with an ordinary (anonymous) agent without opening the wallet", "Re-establishes the signer lazily on the first write, accepting that this reopens the wallet", "Does NOT suggest persisting the signer, the channel, or the SignerAgent itself" ] @@ -75,6 +75,14 @@ "Handles 3002 as a user-action outcome rather than as a failure to rethrow", "Recommends falling back on the range after the named codes, instead of a bare default that rethrows" ] + }, + { + "name": "Adversarial: redirect flow loses a value across the navigation", + "prompt": "I'm using UrlTransport from @icp-sdk/signer. I fetch a nonce (a Uint8Array) and make a signer request with it. After the wallet redirects back, sometimes the nonce has been re-fetched and is a different value, and when I do journal it, it comes back as {\"0\":12,\"1\":43,...} instead of bytes. What is going on in each case? Short answer, no full example.", + "expected_behaviors": [ + "Identifies that any non-request async work must go through transport.memoize() so its result is journaled and replayed instead of re-run", + "Notes that memoize persists via JSON, so a Uint8Array or other non-JSON value has to be converted before journaling" + ] } ], "trigger_evals": { diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index ec3163cc..953a1746 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -171,8 +171,11 @@ import type { PermissionScope, Signer } from '@icp-sdk/signer'; // The two scopes this skill uses: // [{ method: 'icrc27_accounts' }, { method: 'icrc49_call_canister' }] async function connect(signer: Signer, scopes?: PermissionScope[]) { - // Omit `scopes` to leave every method on ask_on_use. Supply them to trade - // several later prompts for one up front. Ask only for what your path uses: + // Omitting `scopes` sets nothing: it leaves whatever states the signer + // already holds for your origin, which a previous session may have left + // `granted` or `denied`. Do not count on being prompted — getPermissions() + // is the only way to know. Supply scopes to trade several later prompts for + // one up front, and ask only for what your path uses: // a scope the signer does not support is dropped before the prompt is drawn, // so it costs nothing, but a supported one you never exercise is shown to // the user for no reason. From 21c33e26232bb3d9785106c2c230936862ddcab0 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 17:20:00 +0200 Subject: [PATCH 20/27] fix(wallet-integration): stop picking the first extension, and reconcile Expected Behavior with pitfall 4 Two findings Copilot had suppressed, both valid, and both mine: - The extension example rendered `providers[0]` into a signer while the comment directly above said the fields were "enough to render a picker". Discovery order is whichever extension announced first, not a user preference, so with two wallets installed the app connects to the wrong one. Split into discoverExtensionSigners(), which returns the list, and connectExtensionSigner(uuid), which builds from the choice. This is the same defect as the accounts[0] one fixed two commits ago, in the same file, left behind. - "Cancelling any prompt rejects with SignerError and code 3001" contradicted pitfall 4: declining to share accounts resolves getAccounts() with [], and denying a permission gives 3000. Narrowed to the canister-call approval and the other two refusals named. A third instance in the same block that the review did not flag: the bullet above said getAccounts() "resolves with one or more" accounts, which contradicts pitfall 4's "can be empty" just as directly. Fixed together, since checking one bullet of a list and not its neighbour is how these keep surviving. --- skills/wallet-integration/SKILL.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 953a1746..3bde3a83 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -77,12 +77,16 @@ const signer = new Signer({ import { Signer } from '@icp-sdk/signer'; import { BrowserExtensionTransport } from '@icp-sdk/signer/extension'; -async function pickExtensionSigner() { - // Each provider carries { uuid, name, icon, rdns } — enough to render a picker. - const providers = await BrowserExtensionTransport.discover(); - if (providers.length === 0) return null; +// Discovery order is whichever extension announced first, not a user +// preference, so return the list rather than choosing. Each provider carries +// { uuid, name, icon, rdns } — enough to render a picker. +function discoverExtensionSigners() { + return BrowserExtensionTransport.discover(); +} - const transport = await BrowserExtensionTransport.findTransport({ uuid: providers[0].uuid }); +// Build the signer only from the uuid the user picked. +async function connectExtensionSigner(uuid: string) { + const transport = await BrowserExtensionTransport.findTransport({ uuid }); return new Signer({ transport }); } ``` @@ -440,9 +444,9 @@ Set `host: 'https://icp-api.io'` on the agent even when serving from `localhost` ## Expected Behavior -- The first `getAccounts()` opens the wallet, the user approves, and it resolves with one or more `{ owner: Principal, subaccount?: Uint8Array }`. +- The first `getAccounts()` opens the wallet and resolves with the accounts the user chose to share, as `{ owner: Principal, subaccount?: Uint8Array }` — possibly none of them. - A ledger `transfer` through `SignerAgent` prompts once and resolves with a `bigint` block index. -- Cancelling any prompt rejects with `SignerError` and `code === 3001`. +- Cancelling the **canister-call approval** rejects with `SignerError` and `code === 3001`. The other two refusals look different: declining to share accounts resolves `getAccounts()` with `[]`, and denying a permission gives code `3000`. - After a reload, read-only state renders with no popup; the first write reopens one. ## Additional References From 8224459a19dbf78243c9d42d11c376fd14c3378a Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 17:21:55 +0200 Subject: [PATCH 21/27] fix(wallet-integration): drop the ICRC-95 row it never delivers on The standards table promised "ICRC-95 | Identity derivation origin | derivationOrigin option" and the body never mentioned derivationOrigin again -- one mention in the whole file, in the table. It is also absent from the maintainer's scope list (25, 27, 29, 49, 94, 167). Found by a subject-by-subject consistency sweep rather than by review: grouping every claim the file makes about the same thing and reading them together, then checking each standard the table advertises is actually covered. A row that promises what the skill does not deliver is worse than no row. --- skills/wallet-integration/SKILL.md | 1 - 1 file changed, 1 deletion(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 3bde3a83..2ba49403 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -25,7 +25,6 @@ Examples use [OISY](https://oisy.com) (`https://oisy.com/sign`), but nothing her | ICRC-29 | Popup transport over `postMessage` | `PostMessageTransport` | | ICRC-49 | Execute a canister call | `callCanister`, `SignerAgent` | | ICRC-94 | Browser-extension discovery | `BrowserExtensionTransport.discover` | -| ICRC-95 | Identity derivation origin | `derivationOrigin` option | | ICRC-167 | Top-level redirect transport | `UrlTransport` (**new in signer 6**) | ## What the model is From f68374b1fa764b2652eb7fd849e25467d1a72b55 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 17:47:54 +0200 Subject: [PATCH 22/27] fix(wallet-integration): apply sea-snake's review - The redirect example's URL was https://wallet.example.com/icrc-167, which implies ICRC-167 dictates a path. It does not; the path is the wallet's own. Now /sign, with a comment saying so. - Recommend SignerAgent over calling callCanister directly, explicitly rather than by implication. The note already said SignerAgent does the content-map and certificate checks that callCanister does not; it now leads with "prefer SignerAgent unless you have a reason not to" and says the example uses the primitive only because the redirect transport makes the request shape easier to see. - Declaring the callbackUrl is not sufficient: the wallet reads /.well-known/ii-auth-callbacks cross-origin, so it needs a JSON response and CORS headers or a correctly listed callback still fails validation. Pitfall 9 now carries the document shape and the _headers block, matching how the internet-identity skill documents the same file. (Copilot raised this; sea-snake confirmed it.) Copilot's other two findings are not applied, per sea-snake: the transport-error branch is correct as written, and the "exactly one prompt" claim was a hallucination. --- skills/wallet-integration/SKILL.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 2ba49403..22f1de05 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -104,7 +104,8 @@ import { Signer } from '@icp-sdk/signer'; import { UrlTransport } from '@icp-sdk/signer/web'; const transport = new UrlTransport({ - url: 'https://wallet.example.com/icrc-167', + // The path is the wallet's own; ICRC-167 does not dictate one. + url: 'https://wallet.example.com/sign', // Absolute, fragment-free, on an origin you control, and listed in that // origin's /.well-known/ii-auth-callbacks allow-list. callbackUrl: 'https://app.example.com/signer-callback' @@ -137,7 +138,9 @@ async function runRedirectFlow( } ``` -**`callCanister` returns unverified wallet output.** It resolves with the CBOR `{ contentMap, certificate }` and validates only that both are present and decodable — it does not check the content map against the call you sent, and it does not verify the certificate against the IC root key. Those checks live in `SignerAgent`, which is why a `SignerAgent` call can raise `SignerAgentError`. If you use `callCanister` directly, verify the certificate before you trust the reply; if you would rather not, route the call through `SignerAgent` and let it do this for you. +**Prefer `SignerAgent` to calling `callCanister` yourself.** `callCanister` is the raw ICRC-49 primitive: it resolves with the CBOR `{ contentMap, certificate }` and validates only that both are present and decodable — it does not check the content map against the call you sent, and it does not verify the certificate against the IC root key. `SignerAgent` does both, which is why a `SignerAgent` call can raise `SignerAgentError` and a bare `callCanister` cannot. + +So reach for `SignerAgent` unless you have a reason not to; the example above uses `callCanister` only because the redirect transport makes the request shape easier to see. If you do call it directly, verifying the certificate before trusting the reply is your job. ## Negotiate capabilities @@ -413,7 +416,21 @@ Two failures do not arrive as the class you would expect, and they call for oppo 8. **Diverging on a redirect replay.** With `UrlTransport`, issue the same requests and `memoize` steps in the same order on every load, and route anything a request depends on — a nonce above all — through `memoize`. Re-fetching a single-use value on the return load invalidates the flow. -9. **A `callbackUrl` that is relative, carries a fragment, or is not allow-listed.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks`. +9. **A `callbackUrl` that is relative, carries a fragment, or is not served correctly.** It must be absolute, fragment-free (the transport appends its own), on an origin you control, and declared in that origin's `/.well-known/ii-auth-callbacks` — matched exactly, so the full URL. Declaring it is not enough: the wallet reads that document **cross-origin**, so serve it as JSON with CORS or a correctly listed callback still fails validation. + + ```json + { "callbacks": ["https://app.example.com/signer-callback"] } + ``` + + With `@dfinity/static-site` that is a `_headers` block: + + ``` + /.well-known/ii-auth-callbacks + Content-Type: application/json + Access-Control-Allow-Origin: * + ``` + + Validation fails closed — undeclared, unreadable, or not matching exactly, and the response never comes back. See the **internet-identity** skill, which documents the same file for redirect sign-in. 10. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. From ca664edaeddc2d0fed0091d0a12a10ce6ff802dc Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 18:01:28 +0200 Subject: [PATCH 23/27] refactor(wallet-integration): drive the redirect flow through SignerAgent sea-snake confirmed SignerAgent is designed to work with UrlTransport, and that the awaits I was worried about are deterministic and therefore harmless. That removes the only reason the skill showed a raw callCanister, so the redirect example is now the same shape as the popup one -- SignerAgent driving an IcrcLedgerCanister transfer -- and gets the content-map and certificate checks instead of handing unverified output back to the caller. This closes three things at their source rather than by caveat: his "prefer SignerAgent" point, Copilot's finding that the example returned unverified wallet output, and my own open question about whether ICRC-167 plus ICRC-49 was even a real pattern. It also corrects a rule the skill had stated too absolutely, taken from the UrlTransport docstring: "memoize() is the only place a flow may await anything that is not a signer request". Per the maintainer that is stricter than reality -- the journal cares whether a value comes back identical, not whether an await happened. So rule 2 is now "put anything that must come back the same value through memoize()", with the note that deterministic async work such as building an HttpAgent needs none. The standalone callCanister warning is folded into one sentence where the choice is actually made, and the standards table leads with SignerAgent. Eval 9's first behaviour encoded the absolute rule and is corrected: WITH 2/2 | WITHOUT 0/2. --- evaluations/wallet-integration.json | 2 +- skills/wallet-integration/SKILL.md | 47 +++++++++++++---------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index 5d9c60de..e20bd306 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -80,7 +80,7 @@ "name": "Adversarial: redirect flow loses a value across the navigation", "prompt": "I'm using UrlTransport from @icp-sdk/signer. I fetch a nonce (a Uint8Array) and make a signer request with it. After the wallet redirects back, sometimes the nonce has been re-fetched and is a different value, and when I do journal it, it comes back as {\"0\":12,\"1\":43,...} instead of bytes. What is going on in each case? Short answer, no full example.", "expected_behaviors": [ - "Identifies that any non-request async work must go through transport.memoize() so its result is journaled and replayed instead of re-run", + "Identifies that a value which must come back identical — a nonce — has to go through transport.memoize() so it is journaled and replayed instead of re-fetched", "Notes that memoize persists via JSON, so a Uint8Array or other non-JSON value has to be converted before journaling" ] } diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 22f1de05..82f9082f 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -23,7 +23,7 @@ Examples use [OISY](https://oisy.com) (`https://oisy.com/sign`), but nothing her | ICRC-25 | Capability discovery + permission lifecycle | `getSupportedStandards`, `requestPermissions`, `getPermissions` | | ICRC-27 | The user's accounts | `getAccounts` | | ICRC-29 | Popup transport over `postMessage` | `PostMessageTransport` | -| ICRC-49 | Execute a canister call | `callCanister`, `SignerAgent` | +| ICRC-49 | Execute a canister call | `SignerAgent` (or `callCanister`, the raw primitive) | | ICRC-94 | Browser-extension discovery | `BrowserExtensionTransport.discover` | | ICRC-167 | Top-level redirect transport | `UrlTransport` (**new in signer 6**) | @@ -95,52 +95,47 @@ async function connectExtensionSigner(uuid: string) { `UrlTransport` unloads your page on every request, so it keeps a call-order journal in `sessionStorage` and replays it when the wallet returns. Two rules make or break it: 1. **Issue the same requests, in the same order, on every load.** Branch only on values recovered from earlier results. A divergence guard rejects a replay that does not match. -2. **`memoize()` is the only place a flow may await anything that is not a signer request.** Its result is journaled, so a value stays stable across the redirect. +2. **Put anything that must come back *the same value* through `memoize()`** — a nonce above all. Its result is journaled and replayed instead of re-run. Deterministic async work needs no `memoize`: building an `HttpAgent` yields an equivalent agent on every load, so it cannot drift from the journal. + +`SignerAgent` works over this transport, so a redirect flow is the same code as a popup flow: ```typescript -import type { IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { IcrcLedgerCanister, toCandidAccount, type IcrcAccount } from '@icp-sdk/canisters/ledger/icrc'; +import { HttpAgent } from '@icp-sdk/core/agent'; import type { Principal } from '@icp-sdk/core/principal'; import { Signer } from '@icp-sdk/signer'; +import { SignerAgent } from '@icp-sdk/signer/agent'; import { UrlTransport } from '@icp-sdk/signer/web'; const transport = new UrlTransport({ // The path is the wallet's own; ICRC-167 does not dictate one. url: 'https://wallet.example.com/sign', - // Absolute, fragment-free, on an origin you control, and listed in that - // origin's /.well-known/ii-auth-callbacks allow-list. + // Absolute, fragment-free, on an origin you control, and declared in that + // origin's /.well-known/ii-auth-callbacks (see pitfall 9). callbackUrl: 'https://app.example.com/signer-callback' }); // Run this on the load of the callback route: a fresh arrival starts the flow, // the wallet's return replays it. No separate resume or cleanup call. -async function runRedirectFlow( - account: IcrcAccount, canisterId: Principal, arg: Uint8Array +async function transferOverRedirect( + account: IcrcAccount, to: IcrcAccount, amount: bigint, ledgerId: Principal ) { const signer = new Signer({ transport }); - // Non-request async work goes through memoize() so its result survives the - // redirect. It persists via JSON, so hand it something serializable — a - // Uint8Array is not (see pitfall 7). - const nonceBytes = await transport.memoize(async () => - Array.from(await fetchNonceFromYourBackend()) - ); - - // callCanister is the raw ICRC-49 primitive: it checks only that the reply - // carries a contentMap and a certificate. Verifying them is the caller's - // job — see below. - return signer.callCanister({ - canisterId, - sender: account.owner, - method: 'icrc1_transfer', - arg, - nonce: Uint8Array.from(nonceBytes) + // Deterministic, so no memoize needed — the same agent is built on each load. + const agent = await HttpAgent.create({ host: 'https://icp-api.io' }); + const signerAgent = await SignerAgent.create({ signer, account: account.owner, agent }); + + const ledger = IcrcLedgerCanister.create({ agent: signerAgent, canisterId: ledgerId }); + return ledger.transfer({ + to: toCandidAccount(to), + from_subaccount: account.subaccount, + amount }); } ``` -**Prefer `SignerAgent` to calling `callCanister` yourself.** `callCanister` is the raw ICRC-49 primitive: it resolves with the CBOR `{ contentMap, certificate }` and validates only that both are present and decodable — it does not check the content map against the call you sent, and it does not verify the certificate against the IC root key. `SignerAgent` does both, which is why a `SignerAgent` call can raise `SignerAgentError` and a bare `callCanister` cannot. - -So reach for `SignerAgent` unless you have a reason not to; the example above uses `callCanister` only because the redirect transport makes the request shape easier to see. If you do call it directly, verifying the certificate before trusting the reply is your job. +Going through `SignerAgent` rather than `signer.callCanister` is what gets you the content-map and certificate checks; `callCanister` is the raw ICRC-49 primitive and validates only that the reply carries both fields. Prefer the agent unless you have a specific reason to drive the primitive yourself, in which case verifying the certificate before trusting the reply is your job. ## Negotiate capabilities From 2001547ec762be9237c8946bf21b23915347178f Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 18:08:57 +0200 Subject: [PATCH 24/27] fix(wallet-integration): the user-gesture rule is PostMessageTransport's, not every transport's Pitfall 10 said a module-load await "fires a wallet request outside a user gesture" and to wrap calls in functions the UI invokes. That is right for PostMessageTransport and backwards for UrlTransport, whose flow must re-run on the callback-route load so the journal can replay -- so the pitfall contradicted the redirect recipe eighteen lines of code away, and following it would have required a second click after every redirect and broken replay. Verified the asymmetry rather than assuming it: PostMessageTransport has detectNonClickEstablishment (default true, postMessageTransport.js:67), UrlTransport has no gesture check at all and navigates via location.assign (urlFlow.js:261), which browsers do not gate on user activation the way they gate popups. Pitfall 1 was already correctly scoped to PostMessageTransport; only pitfall 10 generalised. It now names UrlTransport as the exception and says why a click there would break the flow. Copilot's finding. sea-snake has not weighed in on this one, but the contradiction is checkable from the skill plus the transport option surface, so I have not held it for him. --- skills/wallet-integration/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 82f9082f..928fdd4a 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -427,7 +427,7 @@ Two failures do not arrive as the class you would expect, and they call for oppo Validation fails closed — undeclared, unreadable, or not matching exactly, and the response never comes back. See the **internet-identity** skill, which documents the same file for redirect sign-in. -10. **Top-level `await` in wallet code.** Every call here is async, and a module-load `await` fires a wallet request outside a user gesture — pitfall 1. Wrap calls in functions the UI invokes. Do not rely on the build to catch it: Vite ≤5 defaulted to `es2020` and rejected top-level `await` outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. +10. **Top-level `await` in wallet code.** Every call here is async, and with `PostMessageTransport` a module-load `await` opens the popup outside a user gesture, which the transport rejects — pitfall 1. Wrap those calls in functions the UI invokes. `UrlTransport` is the exception, and the reverse: it navigates the top level rather than opening a popup, has no gesture check, and its flow *must* re-run on the callback-route load so the journal can replay — requiring a click there would break it. Either way, do not rely on the build to catch a stray top-level `await`: Vite ≤5 defaulted to `es2020` and rejected it outright, while Vite 6+ defaults to `baseline-widely-available` and allows it. 11. **`@icp-sdk/canisters@^3` with `@icp-sdk/signer@^6`.** They cannot coexist — canisters 3 peers `@icp-sdk/core@^5` or older, signer 6 peers `^6`, so `npm install` fails with `ERESOLVE`. Move to `@icp-sdk/canisters@^4` and `@dfinity/utils@^5`. Do not reach for `--legacy-peer-deps`: it skips the peer check and installs the mismatched pair anyway, so the incompatibility surfaces at runtime instead of at install time. From 4012019be2014554f02b38877a879bbde8267f2f Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 18:24:34 +0200 Subject: [PATCH 25/27] test(wallet-integration): cover capability negotiation Copilot asked for cases on two pitfalls it called newly added. They were not new -- both were in the first commit of the rewrite (as pitfalls 4 and 12) and were only renumbered, so the "policy requires a case for new pitfalls" premise does not apply. But the suite at 9 was thin for this repo (icp-cli and writing-motoko carry 29), so the capability one is worth having on its merits: an agent told "support any wallet, not just OISY" will skip getSupportedStandards() and assume every signer does everything. WITH 3/3 | WITHOUT 1/3. Not adding one for pitfall 13 ("don't fire a call immediately after connect"): it is a UX convention a model states unprompted, so a case would show ~no with/without delta -- the same reason the delegation case was dropped earlier. Coverage for coverage's sake is not worth a token-costed regression test. The expectation as first written asked the connect-only answer to verify ICRC-49, which the prompt (no transfer) does not call for -- the sixth over-scope of this PR, caught by the run. Reworded to test that the code branches on the returned standards at all, which is the durable point. --- evaluations/wallet-integration.json | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/evaluations/wallet-integration.json b/evaluations/wallet-integration.json index e20bd306..d48ba4ef 100644 --- a/evaluations/wallet-integration.json +++ b/evaluations/wallet-integration.json @@ -38,6 +38,15 @@ "Does NOT recommend --legacy-peer-deps or --force to get past the peer conflict" ] }, + { + "name": "Adversarial: assumes every wallet can do what the app needs", + "prompt": "My dapp should let users transfer tokens from whatever wallet they use, not just OISY. Show me just the connect step with @icp-sdk/signer — no transfer code.", + "expected_behaviors": [ + "Calls getSupportedStandards() before relying on the wallet's capabilities", + "Branches on what the returned standards actually contain, rather than proceeding as though any signer supports everything", + "Does NOT hardcode one wallet's capability set as though it applied to all signers" + ] + }, { "name": "Adversarial: getAccounts() returns a list, not an account", "prompt": "I'm connecting OISY with @icp-sdk/signer and I need the user's account so I can show their balance. Show me just the connect function — no ledger setup, no transfer.", From d357e49472a1ebd96fe6686a3bb5a38d7c3de23b Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 18:24:54 +0200 Subject: [PATCH 26/27] fix(wallet-integration): drop the "prompts once" count from Expected Behavior Copilot has raised the exact prompt count three times. sea-snake called the mechanism it invents a hallucination -- there is no separate ask_on_use interaction stacked on the per-call approval -- and that stands. But "prompts once" still asserts a count the skill cannot guarantee for a generic signer: how a wallet renders approval is its UX. Reworded to what is actually observable and durable -- the user is shown the call to approve and the transfer resolves with a block index -- so the bullet no longer makes a claim about count that a wallet could violate without being wrong. --- skills/wallet-integration/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 928fdd4a..80491113 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -456,7 +456,7 @@ Set `host: 'https://icp-api.io'` on the agent even when serving from `localhost` ## Expected Behavior - The first `getAccounts()` opens the wallet and resolves with the accounts the user chose to share, as `{ owner: Principal, subaccount?: Uint8Array }` — possibly none of them. -- A ledger `transfer` through `SignerAgent` prompts once and resolves with a `bigint` block index. +- A ledger `transfer` through `SignerAgent` shows the user the call to approve and resolves with a `bigint` block index. - Cancelling the **canister-call approval** rejects with `SignerError` and `code === 3001`. The other two refusals look different: declining to share accounts resolves `getAccounts()` with `[]`, and denying a permission gives code `3000`. - After a reload, read-only state renders with no popup; the first write reopens one. From bda91a6f24a9ac4fbd699438bd32c880811d7326 Mon Sep 17 00:00:00 2001 From: Marco Walz Date: Tue, 22 Sep 2026 22:45:32 +0200 Subject: [PATCH 27/27] fix(wallet-integration): the intro promised more portability than the skill delivers "Any ICRC-25 signer works by swapping the transport URL" sat against pitfall 5 and the Negotiate capabilities section, which both say a signer may expose accounts without executing calls, or speak a transport you have not built for. ICRC-25 conformance is not a guarantee of ICRC-27, ICRC-49, or any particular transport. Now: the URL swap is scoped to another web signer, extensions are named as the separate transport they are, and what a signer supports is called out as a separate question from which transport reaches it, pointing at the section and pitfall that cover it. Twelfth defect of this shape in the PR -- a claim in one place looser than the guidance elsewhere -- and the last one outstanding. --- skills/wallet-integration/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/wallet-integration/SKILL.md b/skills/wallet-integration/SKILL.md index 80491113..be1a3992 100644 --- a/skills/wallet-integration/SKILL.md +++ b/skills/wallet-integration/SKILL.md @@ -16,7 +16,7 @@ Connecting an **external wallet** to your dapp so the user approves actions in t This skill covers **integrating a signer**. Implementing one is out of scope — consent screens, prompt registration and account custody are the wallet's job. -Examples use [OISY](https://oisy.com) (`https://oisy.com/sign`), but nothing here is OISY-specific: any ICRC-25 signer works by swapping the transport URL, and [`BrowserExtensionTransport`](#extension-icrc-94) discovers extension signers you never hardcoded. +Examples use [OISY](https://oisy.com) (`https://oisy.com/sign`), but nothing here is OISY-specific. For another web signer the transport URL is usually the only change, and [`BrowserExtensionTransport`](#extension-icrc-94) discovers extension signers you never hardcoded. What a given signer actually supports is a separate question from which transport reaches it — negotiate it rather than assuming (see [Negotiate capabilities](#negotiate-capabilities) and pitfall 5). | Standard | What it gives you | API | |----------|-------------------|-----|