Implement Keyring API v2 - #151
Conversation
529fdab to
00fcbf1
Compare
00fcbf1 to
03fdea9
Compare
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on:
|
|
@SocketSecurity ignore-all |
There was a problem hiding this comment.
Pull request overview
This PR migrates the Stellar snap’s keyring handler to the remaining MetaMask Keyring API v2 surface, adding support for getAccounts and secure private-key export via exportAccount, along with required permissions/manifest updates.
Changes:
- Implemented Keyring API v2 semantics in the keyring handler (
KeyringRpc), includinggetAccounts,exportAccount, and updatedgetAccountmissing-id behavior. - Added wallet-level raw seed export helpers and tests for hex/base58 export round-trips.
- Updated snap permissions/manifest and dependency resolution to support v2 types and keyring capabilities.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile to reflect new/updated dependency graph needed for v2 support. |
| packages/snap/tsconfig.json | Enables moduleResolution: bundler (and module: preserve) to resolve /v2 subpath types. |
| packages/snap/src/services/wallet/Wallet.ts | Adds exportKey for raw seed export in hex/base58. |
| packages/snap/src/services/wallet/Wallet.test.ts | Adds tests validating exportKey output and round-trip behavior. |
| packages/snap/src/permissions.ts | Adds v2 RPC method permissions (GetAccounts, ExportAccount) and updates selected method constants. |
| packages/snap/src/handlers/keyring/keyring.ts | Migrates dispatcher to @metamask/keyring-snap-sdk/v2, implements KeyringRpc, adds getAccounts + exportAccount, and redacts export results from debug logs. |
| packages/snap/src/handlers/keyring/keyring.test.ts | Updates tests for v2 dispatcher usage, new semantics, export behavior, and log redaction. |
| packages/snap/src/handlers/keyring/api.ts | Adds Base58Struct and handler-level request struct for exportAccount. |
| packages/snap/src/handlers/keyring/api.test.ts | Adds tests for Base58Struct. |
| packages/snap/src/context.ts | Wires AccountResolver into KeyringHandler construction. |
| packages/snap/snap.manifest.json | Declares keyring capabilities for scopes, private-key export formats, and bip44 features. |
| packages/snap/package.json | Adds @scure/base dependency for base58 encoding. |
| packages/snap/CHANGELOG.md | Documents added v2 functionality and the getAccount breaking change. |
| package.json | Pins @metamask/snaps-utils via resolutions to support capabilities validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
03fdea9 to
c267be1
Compare
c267be1 to
99030f1
Compare
74127c4 to
cd42af5
Compare
cd42af5 to
d2f45f1
Compare
c3cdaf2 to
e36f177
Compare
cb7e0d6 to
960162d
Compare
|
The description mentions |
|
i think @jeremytsng didnt update the description yet |
960162d to
b920552
Compare
- Remove all Keyring API v1 methods, dual dispatch, and permissions - Implement the full KeyringSnapRpc interface without Omit - Use the snap manifest scopes as the source of truth for account scopes - Restrict createAccounts to the MetaMask origin - Remove the unneeded @metamask/snaps-utils resolution
b920552 to
c38f5d0
Compare
Explanation
The Stellar snap implemented the Keyring API v1 surface. MetaMask now drives Snap accounts through Keyring API v2, so the snap needs to expose the v2 method set and declare its capabilities in the manifest.
This migrates the keyring handler to v2:
onKeyringRequestnow dispatches throughhandleKeyringRequestfrom@metamask/keyring-snap-sdk/v2, andKeyringHandlerimplementsKeyringSnapRpc.getAccounts.getAccountreturns aKeyringAccountand throwsAccountNotFoundExceptionfor an unknown id, rather than returningundefined.listAccountAssets→getAccountAssetsandlistAccountTransactions→getAccountTransactions. These are the names the v2 dispatcher binds; it continues to accept the previous method names on the wire, so existing callers are unaffected.createAccountshandlesbip44:discoverin addition tobip44:derive-indexandbip44:derive-index-range. For discovery the account is only created when it has on-chain activity; otherwise an empty array is returned and the client stops discovering. All three options now resolve to a single index range and go through onebatchCreatecall.submitRequestreturns the handler result directly. v2 declaresPromise<Json>, so the previous{ pending: false, result }wrapper has been removed.createAccount,listAccounts,discoverAccounts,filterAccountChainsandupdateAccount, along with theAccountCreatedandAccountDeletedevents.AccountService.createand#getLowestUnusedIndexbecame unreachable once account creation went throughbatchCreate, so they are removed too, together with three exceptions that only those paths raised.Supporting changes:
snap.manifest.jsondeclaresendowment:keyring.capabilitieswithscopes: ["stellar:pubnet"]andbip44: { deriveIndex, deriveIndexRange, discover }, so the client knows which account operations the snap supports.SUPPORTED_SCOPESis derived from the manifest and used for account creation and discovery, so the manifest is the single source of truth for scopes rather than a separate config value.permissions.tsallows the v2 method names. The dapp origin is limited to the methods a dapp may legitimately call; account creation and address resolution remain MetaMask-only.packages/snap/tsconfig.jsonsetsmoduleResolution: "bundler", which is required to resolve the/v2subpath exports. This affects type checking only; the bundle is produced by webpack.yarn.lockmoves@metamask/snaps-utilsto a version that recognises thecapabilitiesfield. The version previously resolved by the lockfile rejects it withUnknown key: initialPermissions.endowment:keyring.capabilities, so the manifest would not validate without this.Manual testing
Verified against a local MetaMask Flask build with this snap preinstalled:
changeTrustopt-in, confirmed, asset appears in the token listmainReferences
Aligns the Stellar snap with the Keyring API v2 migration already applied to the Solana snap in MetaMask/snap-solana-wallet#606.
Checklist