Skip to content

feat(Snaps Dynamic Permissions): Adds support for Snaps dynamic permissions#22878

Closed
david0xd wants to merge 4 commits intodevelopfrom
dd/snaps-dynamic-permissions
Closed

feat(Snaps Dynamic Permissions): Adds support for Snaps dynamic permissions#22878
david0xd wants to merge 4 commits intodevelopfrom
dd/snaps-dynamic-permissions

Conversation

@david0xd
Copy link
Copy Markdown
Contributor

@david0xd david0xd commented Feb 8, 2024

Description

The purpose of this PR is to enable Snaps Dynamic Permissions feature.
This PR adds missing hooks required by the Dynamic Permissions feature and adds additional changes to the UI to correctly support Dynamic Permissions requests in approval popup.

Related PR: MetaMask/snaps#2088

Dynamic Permissions feature is specified in SIP-14.

Rules to follow before merging this PR

Merge this PR only into the release PR of Snaps packages that contain changes on the related PR for Snaps packages: MetaMask/snaps#2088

Target PR: Snaps packages release.

TODO: Change target PR to the one that releases Snaps packages with Dynamic Permissions.

Related issues

Related task: MetaMask/snaps#1821

Manual testing steps

  1. Create a Snap example that invokes following RPC methods: snap_requestPermissions, snap_getPermissions, snap_revokePermissions.
  2. Test each of the method invocations.
  3. When requesting permissions, approval request popup should appear.
  4. When revoking permissions, approval request popup should not appear.
  5. Requested permissions should appear in the Snaps settings page only after approval.
  6. Getting granted Snaps permissions is reserved for Snap only and will not be shown anywhere within MetaMask extension.

Snap code examples:

  • snap_requestPermissions
const premissionRequestResult = await snap.request({
    method: 'snap_requestPermissions',
    params: {
      'endowment:network-access': {},
      'endowment:webassembly': {},
      'endowment:ethereum-provider': {},
      snap_getBip44Entropy: [
        {
          coinType: 3,
        },
        {
          coinType: 1,
        },
      ],
    },
  });
  • snap_getPermissions
const permissions = await snap.request({
    method: 'snap_getPermissions',
});
  • snap_revokePermissions
const revokePermissionsResult = await snap.request({
    method: 'snap_revokePermissions',
    params: {
      'endowment:network-access': {},
      'endowment:webassembly': {},
      'endowment:ethereum-provider': {},
      snap_getBip44Entropy: [
        {
          coinType: 3,
        },
        {
          coinType: 1,
        },
      ],
    },
  });

Screenshots/Recordings

Before

TODO: Show to related screens which have the code living in the same components, etc.

After

Screenshot 2024-02-14 at 15 26 29

End to end testing TODO

  • Create new example Snap that uses Dynamic Permissions features (all three methods).
  • Create e2e tests for testing Dynamic Permissions requests.

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@david0xd david0xd added the team-snaps-deprecated DEPRECATED: please use "team-core-platform" instead label Feb 8, 2024
@david0xd david0xd self-assigned this Feb 8, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 8, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@david0xd david0xd added the DO-NOT-MERGE Pull requests that should not be merged label Feb 8, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 8, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (727ccfe) 68.51% compared to head (8ab3ece) 68.48%.

❗ Current head 8ab3ece differs from pull request most recent head cc930be. Consider uploading reports for the commit cc930be to get more accurate results

Files Patch % Lines
app/scripts/metamask-controller.js 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #22878      +/-   ##
===========================================
- Coverage    68.51%   68.48%   -0.03%     
===========================================
  Files         1088     1088              
  Lines        42915    42903      -12     
  Branches     11427    11421       -6     
===========================================
- Hits         29401    29378      -23     
- Misses       13514    13525      +11     

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

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [8ab3ece]
Page Load Metrics (874 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint801781143014
domContentLoaded95418136
load725154787417885
domInteractive95418136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 196 Bytes (0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@david0xd david0xd force-pushed the dd/snaps-dynamic-permissions branch from 466a8d0 to cc930be Compare February 13, 2024 11:15
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [cc930be]
Page Load Metrics (1004 ± 29 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1352501893115
domContentLoaded1094352613
load850111710046129
domInteractive1094352613
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 184 Bytes (0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@david0xd david0xd changed the title Add changes required for Snaps dynamic permissions feat(Snaps Dynamic Permissions): Add changes to support Snaps dynamic permissions integration Feb 14, 2024
@david0xd david0xd changed the title feat(Snaps Dynamic Permissions): Add changes to support Snaps dynamic permissions integration feat(Snaps Dynamic Permissions): Adds support for Snaps dynamic permissions Feb 14, 2024
@ziedbrini ziedbrini added team-snaps-platform-deprecated DEPRECATED: please use "team-core-platform" instead (or "team-new-networks" for Solana snap issues) and removed team-snaps-deprecated DEPRECATED: please use "team-core-platform" instead labels Mar 6, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 5, 2024

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label May 5, 2024
@github-actions
Copy link
Copy Markdown
Contributor

This PR was closed because there has been no follow up activity in the last 14 days. Thank you for your contributions.

@github-actions github-actions bot closed this May 19, 2024
@HowardBraham HowardBraham deleted the dd/snaps-dynamic-permissions branch January 19, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO-NOT-MERGE Pull requests that should not be merged stale issues and PRs marked as stale team-snaps-platform-deprecated DEPRECATED: please use "team-core-platform" instead (or "team-new-networks" for Solana snap issues)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants