feat: add getSessionProperties to chain-agnostic-permissions. Use it in multichain-api-middleware#9294
Conversation
chain-agnostic-permissions. Use it in multichain-api-middleware
|
@metamaskbot publish-previews |
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Co-authored-by: Alex Donesky <adonesky@gmail.com>
…o jl/add-getSessionProperties
|
@metamaskbot publish-previews |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
| await Promise.all( | ||
| addresses.map(async (address) => { | ||
| eip155Capabilities[address] = await getCapabilities({ address }); | ||
| }), | ||
| ); |
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?
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
| }) => Promise<Record<Hex, Record<string, Json>>>; | ||
| }, | ||
| ): Promise<Record<string, Json>> => { | ||
| const addresses = getEthAccounts(caip25CaveatValue); |
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?
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.
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
|
Do we need to include these capabilties in |
|
yeah, |
|
@metamaskbot publish-previews |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
## Explanation Releases the following packages: - **`@metamask/chain-agnostic-permission`**: `1.6.2` → `1.7.0` (minor) - Adds the `getSessionProperties` async function (from [MetaMask#9294](MetaMask#9294)). - **`@metamask/multichain-api-middleware`**: `4.0.0` → `4.0.1` (patch) - Bumps its `@metamask/chain-agnostic-permission` dependency from `^1.6.2` to `^1.7.0`. `4.0.0` already consumes `getSessionProperties` (introduced in [MetaMask#9294](MetaMask#9294)), which is only available from `chain-agnostic-permission@1.7.0`, so this aligns the declared dependency range with what the package actually requires. - **`@metamask/perps-controller`**: `9.2.0` → `9.2.1` (patch) - Bumps `@metamask/messenger` from `^1.2.0` to `^2.0.0`. - Fix `adaptOrderFromSDK` dropping `takeProfitPrice`/`stopLossPrice` for child TP/SL orders whose `triggerPx` is an empty string ## References - Follow-up to [MetaMask#9294](MetaMask#9294) - Mobile: MetaMask/metamask-mobile#32660 ## Checklist - [x] Versions bumped following SemVer - [x] Changelog entries migrated to new version sections - [x] All changelogs pass validation - [x] `yarn constraints` passes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Version, changelog, and dependency-range-only changes with no new runtime code; fixes a packaging mismatch for an API already used in 4.0.0. > > **Overview** > **Monorepo release 1097.0.0** cuts new package versions and updates lockfile entries; there is no application logic change in this diff. > > **`@metamask/chain-agnostic-permission` 1.7.0** moves the existing Unreleased changelog (including **`getSessionProperties`**) into the **1.7.0** section and bumps the package version. > > **`@metamask/multichain-api-middleware` 4.0.1** raises its declared **`@metamask/chain-agnostic-permission`** range from **`^1.6.2`** to **`^1.7.0`**, because **4.0.0** already calls **`getSessionProperties`** but did not declare that minimum version. The **4.0.0** changelog section is marked **[DEPRECATED]**. > > **`@metamask/eip1193-permission-middleware`** gets the same **`chain-agnostic-permission`** dependency bump and changelog note. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a02bb3e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
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.