Skip to content

docs: wallet integration guide#85

Merged
marc0olo merged 2 commits into
mainfrom
docs/guides-defi-wallet-integration
Apr 16, 2026
Merged

docs: wallet integration guide#85
marc0olo merged 2 commits into
mainfrom
docs/guides-defi-wallet-integration

Conversation

@marc0olo
Copy link
Copy Markdown
Member

Summary

  • ICRC signer protocol overview and @icp-sdk/signer integration pattern
  • IcpWallet/IcrcWallet classes from @dfinity/oisy-wallet-signer
  • Signer + SignerAgent pattern from @icp-sdk/signer
  • Internet Identity authentication integration
  • Transfer and approval flows for ICP and ICRC tokens

Sync recommendation

informed by dfinity/icskills — skills/wallet-integration/SKILL.md

@marc0olo
Copy link
Copy Markdown
Member Author

Review: Wallet Integration

Must fix

  • signer.disconnect() does not exist in @icp-sdk/signer: The page calls await signer.disconnect() in the Disconnect section and describes it as "closes the wallet popup and removes any cached session state." The actual Signer class in @icp-sdk/signer has no disconnect() method — the correct method is signer.closeChannel() which returns Promise<void>. Verified against index/classes/Signer.md in icp-js-sdk-docs/public/signer/latest.zip. The actual oisy-signer-demo example also does not call signer.disconnect(). Fix: replace await signer.disconnect() with await signer.closeChannel() and update the description accordingly.

  • Wrong library referenced in local development section: The page covers @icp-sdk/signer throughout, but the "Local development" section states: "The @dfinity/oisy-wallet-signer repository includes a pseudo wallet signer you can run locally." This conflates two entirely different packages — @icp-sdk/signer (used in all code examples) and @dfinity/oisy-wallet-signer (a separate library with a completely different API, covering IcrcWallet, IcpWallet, RelyingParty, etc.). The pseudo wallet signer instructions (npm run sync:all, npm run dev:wallet) come from @dfinity/oisy-wallet-signer's demo and are irrelevant to @icp-sdk/signer-based dApps. If the intent is to point readers to a test signer, clarify that this is a separate tool and describe how to actually connect @icp-sdk/signer to it. If the pseudo wallet is genuinely useful, it deserves a concrete example using the @icp-sdk/signer API.

  • Working example deploy steps are missing the clone step: The deploy snippet starts with cd examples/hosting/oisy-signer-demo but there is no prior git clone step. The reader will not have this directory. The example's own README shows git clone https://github.com/dfinity/examples before the cd. Fix: add the clone step or add a note like "After cloning the examples repo:".

Suggestions

  • No permissions request step before getAccounts(): The page calls signer.getAccounts() directly without requesting permissions first. The icrc27_accounts permission scope is required and the signer will prompt per-method if not requested upfront. This means users may see an unexpected permissions prompt before the accounts prompt, without context about why. Consider adding a note that signer.requestPermissions(['icrc27_accounts']) can be called first to batch the prompt, and that the signer handles per-method prompts automatically if permissions are skipped.

  • Error handling section lacks error codes: The SignerError class has a .code property described as "numeric error code from the ICRC-25 standard" but no codes are listed. Common codes a developer needs to handle: 3001 (user cancelled / rejected consent), 3000 (permission not granted). Without these, the catch block example is not actionable. A 2-row inline table or a link to the error code reference would help significantly.

  • Session persistence code block doesn't show the Principal import: Principal.fromText(stored) appears in the session persistence block without an import statement. Earlier sections do show this import, but code blocks should be self-contained or explicitly note what was imported earlier.

  • icp0.io vs icp-api.io: The page uses https://icp0.io in the HttpAgent.create() example and the local dev default note. This matches the actual oisy-signer-demo example. No issue — just noting this for future consistency: the SKILL.md and SDK agent docs use https://icp-api.io. Both are valid IC boundary node URLs.

Verified

  • All three internal links resolve: ../authentication/internet-identity.md resolves to internet-identity.mdx (Astro resolves .md to .mdx); token-ledgers.md resolves to token-ledgers.mdx; ../../reference/token-standards.md exists as .md. All valid.
  • Signer import from @icp-sdk/signer is correct (confirmed via signer SDK docs).
  • PostMessageTransport import from @icp-sdk/signer/web is correct.
  • SignerAgent import from @icp-sdk/signer/agent is correct.
  • HttpAgent import from @icp-sdk/core/agent is correct (confirmed via core SDK docs).
  • Principal import from @icp-sdk/core/principal is correct.
  • IcrcLedgerCanister import from @icp-sdk/canisters/ledger/icrc is correct (confirmed via canisters SDK quick-start).
  • SignerAgent.create({ signer, account, agent? }) matches SignerAgentOptions interface exactly.
  • IcrcLedgerCanister.create({ agent, canisterId }) and .balance({ owner }), .transfer({ to, amount }) all match SDK docs.
  • oisy-signer-demo example exists at examples/hosting/oisy-signer-demo in .sources/examples and the code patterns (Signer, SignerAgent, IcrcLedgerCanister) match the page's approach.
  • icp network start -d and icp deploy commands confirmed correct against the example's own build instructions.
  • No dfx references present.
  • Frontmatter complete (title, description, sidebar.order).
  • File is .md (correct — no interactive components needed).
  • <\!-- Upstream: --> comment present at end of file.
  • No links to internetcomputer.org/docs/ or docs.internetcomputer.org.
  • No inline code blocks exceed 30 lines.
  • No mo:base imports (JavaScript-only page).

@marc0olo
Copy link
Copy Markdown
Member Author

Please note that @icp-sdk/signer is the library that we want to promote going forward.

In the examples we have the oisy-signer-demo, see .sources/examples/hosting/oisy-signer-demo which already uses it.

The wallet-integration skill will be updated in that regard. For client side (relayer) integration, we recommend to use @icp-sdk/signer going forward. The docs can be found here in .sources/icp-js-sdk-docs/public/signer/latest.zip.

we should also take a note about the following new libraries:

we don't have to document these for now. it is very likely that both will also move to the @icp-sdk namespace on npm soon. both will probably also be covered in the wallet-integration sill going forward.

- Replace signer.disconnect() with signer.closeChannel() (correct API per SDK docs)
- Fix local dev section: remove conflation with @dfinity/oisy-wallet-signer
- Add git clone step to working example deploy instructions
- Add requestPermissions() note before getAccounts()
- Add error code table to error handling section
- Add Principal import to session persistence code block
- Add forward-looking note about @dfinity/ledger-wallet-identity and @dfinity/icrc21-agent
@marc0olo
Copy link
Copy Markdown
Member Author

<!-- feedback-addressed -->

Feedback addressed for PR #85 — Wallet Integration

Changes applied

Must fix (all applied):

  1. signer.disconnect() replaced with signer.closeChannel() — The Signer class in @icp-sdk/signer has no disconnect() method. The correct method is closeChannel(): Promise<void>. Verified against index/classes/Signer.md in icp-js-sdk-docs/public/signer/latest.zip. Updated both the code block and description text in the Disconnect section.

  2. Local development section — removed @dfinity/oisy-wallet-signer conflation — The page uses @icp-sdk/signer throughout but the local dev section referenced setup steps from @dfinity/oisy-wallet-signer's demo (npm run sync:all, npm run dev:wallet), which belong to a completely different package. Replaced with a generic note: any ICRC-25-compliant wallet exposing a /sign endpoint works.

  3. Working example — added git clone step — The deploy instructions started with cd examples/hosting/oisy-signer-demo but no clone step was present. Added git clone https://github.com/dfinity/examples first, matching the example's own README. Also removed the incorrect cd frontend && npm install step — root npm install suffices (verified via package.json workspaces config).

Suggestions (applied):

  1. requestPermissions() note added before getAccounts() — Added a code example calling signer.requestPermissions([{ method: 'icrc27_accounts' }]) before getAccounts(), with an explanation that skipping it leads to per-method permission prompts. API verified against SDK docs.

  2. Error codes added to error handling section — Replaced the generic comment with a switch statement on err.code for 3001 (ACTION_ABORTED) and 3000 (PERMISSION_NOT_GRANTED), plus an error code table for 3000, 3001, and 4000.

  3. Principal import added to session persistence block — Added import { Principal } from '@icp-sdk/core/principal'; to make the code block self-contained.

  4. Forward-looking ecosystem note added — Added an "Ecosystem libraries" section before "Next steps" noting @dfinity/ledger-wallet-identity and @dfinity/icrc21-agent with npm links, and a note that both are expected to move to the @icp-sdk namespace and will be documented in the wallet-integration skill going forward.

Items skipped

  • icp0.io vs icp-api.io consistency — Reviewer explicitly noted "no issue." The oisy-signer-demo reference implementation uses icp0.io, so the page is consistent with its source. No change needed.

Build

No errors in docs/guides/defi/wallet-integration.md. A pre-existing build error in docs/guides/backends/https-outcalls.mdx (missing Motoko submodule file in this worktree) is unrelated to this PR and present on the original branch too.

@marc0olo marc0olo merged commit 67e18cc into main Apr 16, 2026
1 check passed
@marc0olo marc0olo deleted the docs/guides-defi-wallet-integration branch April 16, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant