feat: add getSessionProperties to chain-agnostic-permissions. Use it in multichain-api-middleware#9294
Merged
Merged
Conversation
jiexi
changed the title
chain-agnostic-permissions. Use it in multichain-api-middleware
Member
Author
|
@metamaskbot publish-previews |
Member
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
adonesky1
reviewed
Jun 29, 2026
adonesky1
reviewed
Jun 29, 2026
adonesky1
reviewed
Jun 29, 2026
Co-authored-by: Alex Donesky <adonesky@gmail.com>
…o jl/add-getSessionProperties
Member
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
adonesky1
reviewed
Jun 29, 2026
Comment on lines
+174
to
+178
| await Promise.all( | ||
| addresses.map(async (address) => { | ||
| eip155Capabilities[address] = await getCapabilities({ address }); | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
This is unguarded, so if getCapabilities rejects for any single address the whole Promise.all rejects, getSessionProperties throws, and both wallet_getSession and wallet_createSession fail. Should we make this best-effort, e.g. try/catch per address and default to {} on failure?
Member
Author
There was a problem hiding this comment.
yeah, i thought about if it was worth a try catch or not. I suppose we shouldn't cause the whole request to fail on this
adonesky1
reviewed
Jun 29, 2026
| }) => Promise<Record<Hex, Record<string, Json>>>; | ||
| }, | ||
| ): Promise<Record<string, Json>> => { | ||
| const addresses = getEthAccounts(caip25CaveatValue); |
Contributor
There was a problem hiding this comment.
How are you feeling about making returning capabilities conditional on callers passing a new sessionProperty param in the request?
Contributor
There was a problem hiding this comment.
Also Should we lowercase the addresses here? The real wallet_getCapabilities already normalizes to lowercase via validateAndNormalizeKeyholder, but getEthAccounts returns them in the caveat's casing (likely checksummed), so we'd be keying eip155Capabilities differently than how that method treats the same addresses. Seems worth standardizing on the lowercase form here (key + the address passed to getCapabilities) so both paths agree, otherwise anything resolving these caps by address has to know which casing to expect.
Member
Author
There was a problem hiding this comment.
How are you feeling about making returning capabilities conditional on callers passing a new sessionProperty param in the request?
Still feel like it isn't worth the trouble, but if you feel strongly still then we can choose a new property name and check it here
Also Should we lowercase the addresses here?
wallet_getCapabilities doesn't return the account address in its result, so I think this is up to us to interpret
Contributor
|
Do we need to include these capabilties in |
Member
Author
|
yeah, |
Member
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Adds
getSessionPropertiestochain-agnostic-permissionsand then uses it inwallet_getSessionandwallet_createSessionhandlers inmultichain-api-middleware. Effectively this change makes it so that CAIP sessionresult.sessionPropertiesnow include aeip155Capabilitiesfield which contains thewallet_getCapabilitiesresult for eacheip155account in the permission.This is needed to help make the CAIP Multichain API handshake truly a one request handshake as without this, many dapps need to make separate
wallet_getCapabilitiescalls. This is problematic for the MWP flow in MetaMask Connect mainly where we want to avoid making unnecessary calls as each call that must resolve in the wallet forces the user out of the dapp and into the wallet causing poor UX.References
Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1560
Checklist
Note
Medium Risk
Breaking middleware hook contract and changes session payload shape dapps consume; capability lookup failures are non-fatal per address, which may surprise callers expecting full maps.
Overview
Adds
getSessionPropertiesinchain-agnostic-permission, which merges stored CAIP-25sessionPropertieswith aneip155Capabilitiesmap: for each unique permitted EVM address (viagetEthAccounts), it calls an injectedgetCapabilitieshook and keys results by address. Failures for a single address are logged and that address is omitted from the map.multichain-api-middlewarenow uses this forwallet_getSessionandwallet_createSessioninstead of returning only persisted caveat properties. Both handlers require a newgetCapabilitieshook (breaking for integrators).wallet_getSessionalways includessessionPropertiesin the result (including{}when there is no active session).Reviewed by Cursor Bugbot for commit 43e6d65. Bugbot is set up for automated code reviews on this repo. Configure here.