feat: Add Multichain API to @metamask/multichain
#4813
Conversation
## Explanation This PR fixes a lot of the linting and typescript errors. still some left but this covers a lot of it. <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/package-a` - **<CATEGORY>**: Your change here - **<CATEGORY>**: Your change here ### `@metamask/package-b` - **<CATEGORY>**: Your change here - **<CATEGORY>**: Your change here ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --------- Co-authored-by: Jiexi Luan <[email protected]>
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> Added ESM exports for multichain package ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/package-a` - **<CATEGORY>**: Your change here - **<CATEGORY>**: Your change here ### `@metamask/package-b` - **<CATEGORY>**: Your change here - **<CATEGORY>**: Your change here ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
| @@ -43,6 +43,9 @@ | |||
| "pre-push": "yarn lint" | |||
| }, | |||
| "resolutions": { | |||
| "cross-spawn@^7.0.0": "^7.0.5", | |||
There was a problem hiding this comment.
These resolutions seems unnecessary - deduping should do the same thing, right?
There was a problem hiding this comment.
unless these are very specific versions (and there are other versions in the dep tree) that we want to force resolve...? But I'll defer to Jiexi on this!
| _next: JsonRpcEngineNextCallback, | ||
| end: JsonRpcEngineEndCallback, | ||
| hooks: { | ||
| revokePermission: (origin: string, permissionName: string) => void; |
There was a problem hiding this comment.
Usually we bind all hooks to the origin for unrestricted RPC methods, would recommend we do that here too
(this applies to multiple files here)
| if ( | ||
| !methodToCheck || | ||
| !isObject(methodToCheck) || | ||
| !('params' in methodToCheck) |
There was a problem hiding this comment.
Generally I would say that hasProperty() is preferred over 'key' in object unless the types don't permit it in rare instances or if we are actually interested in the prototype.
There was a problem hiding this comment.
It seems that Typescript prefers 'key' in object in this case
| ) { | ||
| return [ | ||
| { | ||
| code: -32601, |
There was a problem hiding this comment.
Can we use rpcErrors so we use the standard codes, error messages etc and don't have to duplicate those?
There was a problem hiding this comment.
pushing a PR to address this
| /** | ||
| * All MetaMask methods, except for ones we have specified in the constants above. | ||
| */ | ||
| const Eip155Methods = MetaMaskOpenRPCDocument.methods | ||
| .map(({ name }: { name: string }) => name) | ||
| .filter((method: string) => !WalletEip155Methods.includes(method)) | ||
| .filter((method: string) => !KnownWalletRpcMethods.includes(method)); | ||
| .filter((method: string) => !KnownWalletRpcMethods.includes(method)) | ||
| .filter((method: string) => !Eip1193OnlyMethods.includes(method)); |
There was a problem hiding this comment.
I thought we wanted to include some of these in the multichain via the wallet:eip155 scope?
There was a problem hiding this comment.
currently the only method that lives in the wallet:eip155 scope is wallet_addEthereumChain and this const is just substractively deriving the methods that should go in all eip155:<chain_id> scopes
| * @param hooks.isChainIdSupported - A helper that returns true if an eth chainId is currently supported by the wallet. | ||
| * @returns a NormalizedScopesObject with only scopes that are currently supported. | ||
| */ | ||
| export const getSupportedScopes = ( |
There was a problem hiding this comment.
I can't find any uses of this 🤔
There was a problem hiding this comment.
This function was going to be used for helping to organize different categories of scopes in the wallet_createSession connection flow for when we enable adding scopes as part of the flow. Though looking at it now I'm not sure if its been superceded by getSupportedScopeObject... cc @jiexi ?
| return next(); | ||
| } | ||
|
|
||
| const { chainId } = |
There was a problem hiding this comment.
This adapter warrants more explanation, especially because it seems incompatible with non-EVM
There was a problem hiding this comment.
When we were doing this work the non-EVM milestones felt like a long ways off.
I suppose its worth considering whether we should make this more agnostic at this point.
My feeling though is this could be a fast(ish) follow since our first targeted release is evm only.
WDYT @jiexi
| const scopeString = _scopeString as keyof typeof normalizedScopesObject; | ||
|
|
||
| internalScopes[scopeString] = { | ||
| accounts, |
There was a problem hiding this comment.
Wait, so we are throwing away all of the methods etc? What is the plan then if we want to do more granular permissioning down the line, then we will require another potentially tricky migration?
There was a problem hiding this comment.
So we originally were including the methods in the serialized permission itself (and granularly enforcing each metho), but after discussions w/ @Gudahtt we realized that approach would
- actually require a migration for each and any new methods added.
- doesn't really match how we represent permissions (in connections flows) to users. We don't represent good "buckets" of types of permissions currently. We considered creating these kinds of buckets as we removed the individual methods but decided we probably weren't going to get it exactly right the first time and so would probably need a later migration no matter what. Getting these "buckets" right requires more in depth discussion and we didn't want to make that a blocker for this work
| if (middlewareEntry) { | ||
| middlewareEntry.middleware(req, res, next, end); | ||
| } else { | ||
| return next(); |
There was a problem hiding this comment.
Is there actually a use-case for calling next here? That would lead to sending the request to the node. But if only multichain API requests are allowed in this pipeline anyway, will that ever be a real use-case?
There was a problem hiding this comment.
I'll defer to @jiexi on this one
| isChainIdSupported, | ||
| isChainIdSupportable, | ||
| }: { | ||
| isChainIdSupported: (chainId: Hex) => boolean; |
There was a problem hiding this comment.
What's the plan for making this non-EVM compatible?
There was a problem hiding this comment.
I think this will merit some conversation. We will possibly/probably want to figure out the right strategy for giving dapps the same capability of suggesting the user add EVM networks for non EVM networks (by handing us the snap information as part of the CAIP-25 request). But since this feature itself is not part of V1 I think its okay to leave it as TBD for now?
| } | ||
| } else { | ||
| methods = | ||
| KnownRpcMethods[namespace as NonWalletKnownCaipNamespace] ?? []; |
There was a problem hiding this comment.
Seems incompatible with non-EVM as well? 🤔
| request.params, | ||
| ); | ||
| if (errors.length > 0) { | ||
| throw rpcErrors.invalidParams<JsonRpcError[]>({ data: errors }); |
There was a problem hiding this comment.
I understand the reasoning for wrapping the errors here, but it might be more ergonomic API-wise to not require the unwrapping on the dapp side? 🤔
There was a problem hiding this comment.
yeah I feel that. Kindof nice that we can batch send a bunch of errors though
Explanation
This PR updates
@metamask/multichainto add method handlers and middleware specific to the new Multichain API and which can be shared across the extension & mobile clients. The package includes implementations for managing multichain sessions, handling multichain RPC methods, and integrating multichain functionalities into the MetaMask extension. Key features of this package include:wallet_createSession,wallet_invokeMethod,wallet_revokeSession, andwallet_getSession.NormalizedScopesObjectandInternalScopesObjectThese tools and utilities will be used in both clients (mobile + extension)'s multichain API implementations.
File Overview
packages/multichain/src/adapters/caip-permission-adapter-middleware.ts: Middleware for the EIP-1193 API that enforces a CAIP-25 permission for each request if that CAIP-25 permission was granted viawallet_createSessionpackages/multichain/src/handlers/wallet-getSession.ts: Handlers for CAIP Multichain lifecycle methods except forwallet_createSessionwhich seemed a little too platform specific to belong in a shared package currentlypackages/multichain/src/middlewares/: Middleware for the Multichain API that helps facilitate concurrent eth subscriptions and for using@metamask/api-specsfor method param validation for new CAIP Multichain methodspackages/multichain/src/scope/authorization.ts: Adds helpers that sort scopes based on if they are currently supported by the wallet (i.e. a network already exists the eip155 scope), if they could be supported by the wallet (i.e. the network does not already exist for the eip155 scope, but the dapp has provided EIP-3085 details for adding the network in thescopedPropertiesproperty of thewallet_createSessionrequest), or if they cannot be served.packages/multichain/src/scope/filter.ts: provides helpers used for the bucketing above inauthorization.tstypes/@metamask/eth-json-rpc-filters.d.ts: Typedef for missing types in@metamask/eth-json-rpc-filters/subscriptionManagerReferences
Upstream: #4784
Downstream: None. This is the end.
Key Multichain API Standards implemented here:
wallet_creatSessionwallet_invokeMethodwallet_revokeSessionwallet_sessionChangedwallet_getSessionOpen PR that uses this new package for exposing the multichain API in the extension: MetaMask/metamask-extension#27782
Changelog
@metamask/multichaingetInternalScopesObjectandgetSessionScopeshelpers for transforming betweenNormalizedScopesObjectandInternalScopesObject.caipPermissionAdapterMiddlewarefor enforcing CAIP-25 permission on the EIP-1193 API.walletGetSession,walletInvokeMethod, andwalletRevokeSessionhandlers.multichainMethodCallValidatorMiddlewarefor validating Multichain API method params as defined in@metamask/api-specs.MultichainMiddlewareManagerto multiplex a request to other middleware based on requested scope.MultichainSubscriptionManagerto handle concurrent subscriptions across multiple scopes.bucketScopeswhich groups the scopes in aNormalizedScopesObjectbased on if the scopes are already supported, could be supported, or are not supportable.getSupportedScopeObjectshelper for getting only the supported methods and notifications from eachNormalizedScopeObjectin aNormalizedScopesObject.Checklist