Skip to content

feat: Auto grant EVM permissions to preinstalled Snaps#3410

Merged
FrederikBolding merged 8 commits intomainfrom
fb/auto-grant-evm-permissions
Jul 1, 2025
Merged

feat: Auto grant EVM permissions to preinstalled Snaps#3410
FrederikBolding merged 8 commits intomainfrom
fb/auto-grant-evm-permissions

Conversation

@FrederikBolding
Copy link
Copy Markdown
Member

@FrederikBolding FrederikBolding commented May 20, 2025

For preinstalled Snaps that use endowment:ethereum-provider, we want to automatically grant permissions to EVM accounts so that the Snap can propose signatures without connecting first. This PR adds a middleware that automatically grants the required permissions when all accounts are not permitted. Otherwise it is a no-op.

@FrederikBolding FrederikBolding force-pushed the fb/auto-grant-evm-permissions branch from b0c1713 to b7bf665 Compare May 30, 2025 08:29
@codecov
Copy link
Copy Markdown

codecov bot commented May 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.24%. Comparing base (a9f0277) to head (9d50d65).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3410   +/-   ##
=======================================
  Coverage   98.23%   98.24%           
=======================================
  Files         406      408    +2     
  Lines       11456    11513   +57     
  Branches     1778     1791   +13     
=======================================
+ Hits        11254    11311   +57     
  Misses        202      202           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FrederikBolding FrederikBolding force-pushed the fb/auto-grant-evm-permissions branch from b7bf665 to 52984e6 Compare June 10, 2025 11:26
@FrederikBolding FrederikBolding marked this pull request as ready for review June 10, 2025 11:29
@FrederikBolding FrederikBolding requested a review from a team as a code owner June 10, 2025 11:29
@FrederikBolding FrederikBolding changed the title feat: Auto grant EVM permissions feat: Auto grant EVM permissions to preinstalled Snaps Jun 10, 2025
@jiexi
Copy link
Copy Markdown
Member

jiexi commented Jun 10, 2025

is this a permanent thing? This may be a bad suggestion, but could we do the following:

  1. Add a new permission with origin endowment:ethereum-provider
  2. Give this permisssion subject entry all the permissions necessary (and keep it up to date with state change listeners)
  3. Rather than granting the permission, rewrite the origin of the request to be endowment:ethereum-provider
  4. Allow the request to continue down the pipeline

I imagine there are unwanted side effects of doing the above though

Comment on lines +57 to +60
'endowment:caip25': {
caveats: [
{
type: 'authorizedScopes',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] we could add @metamask/chain-agnostic-permission package as devDependency and use Caip25CaveatType and Caip25EndowmentPermissionName consts here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally skipped out on adding it as a dependency to keep our dependency tree small. Otherwise it often makes releasing painful. I think in this case it doesn't matter too much for just these two strings.

@FrederikBolding FrederikBolding force-pushed the fb/auto-grant-evm-permissions branch from 52984e6 to 2f7ca4e Compare June 20, 2025 07:57
@FrederikBolding
Copy link
Copy Markdown
Member Author

FrederikBolding commented Jun 20, 2025

is this a permanent thing? This may be a bad suggestion, but could we do the following:

  1. Add a new permission with origin endowment:ethereum-provider
  2. Give this permisssion subject entry all the permissions necessary (and keep it up to date with state change listeners)
  3. Rather than granting the permission, rewrite the origin of the request to be endowment:ethereum-provider
  4. Allow the request to continue down the pipeline

I imagine there are unwanted side effects of doing the above though

I would strongly prefer not modifying the origin of the requests.

But yeah, I would see this as a permanent thing, we want preinstalled Snaps to be able to use our APIs without having to connect.

@adonesky1
Copy link
Copy Markdown
Contributor

I would strongly prefer not modifying the origin of the requests.

But yeah, I would see this as a permanent thing, we want preinstalled Snaps to be able to use our APIs without having to connect.

I also prefer what @FrederikBolding has here to your suggestion Jiexi. I think modifying the origin of the request is a very dangerous pattern to introduce at all.

adonesky1
adonesky1 previously approved these changes Jun 26, 2025
Copy link
Copy Markdown
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jiexi
jiexi previously approved these changes Jun 26, 2025
@FrederikBolding FrederikBolding dismissed stale reviews from jiexi and adonesky1 via fd9189f June 27, 2025 07:20
@FrederikBolding FrederikBolding force-pushed the fb/auto-grant-evm-permissions branch from ce9ca02 to fd9189f Compare June 27, 2025 07:20
GuillaumeRx
GuillaumeRx previously approved these changes Jun 27, 2025
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@FrederikBolding FrederikBolding requested a review from Mrtenz June 27, 2025 10:05
Mrtenz
Mrtenz previously approved these changes Jun 27, 2025
@FrederikBolding FrederikBolding added this pull request to the merge queue Jul 1, 2025
Merged via the queue into main with commit 7a1340b Jul 1, 2025
119 checks passed
@FrederikBolding FrederikBolding deleted the fb/auto-grant-evm-permissions branch July 1, 2025 08:16
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.

6 participants