Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cp-12.12.0 CAIP-25 Permission Migration when eth_accounts has not been granted #30218

Merged
merged 6 commits into from
Feb 11, 2025

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented Feb 8, 2025

Description

Fixes a bug in the CAIP-25 Permission migration file that incorrectly aborting the migration if there were any subjects without eth_accounts.

Open in GitHub Codespaces

Related issues

See: #27847

Manual testing steps

  1. git checkout Version-v12.11.0, yarn, yarn start
  2. Reset your wallet state
  3. Permission a dapp with some accounts and chains
  4. Open extension console and run await chrome.storage.local.get() to get local state
  5. Check local state to ensure that the dapp has eth_accounts and permmitedChains permission AND that there are some subjects that you did not explicitly authorize that have also have permissions, but not eth_accounts permissions (should be just snaps related)
  6. git checkout this branch, yarn, yarn start
  7. Open extension console and run await chrome.storage.local.get() to get local state
  8. Check that your dapp has a endowment:caip25 permission now

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

@jiexi jiexi marked this pull request as ready for review February 8, 2025 00:07
Copy link
Contributor

github-actions bot commented Feb 8, 2025

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.

@metamaskbot
Copy link
Collaborator

Builds ready [cbc47b7]
Page Load Metrics (1600 ± 65 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14421974160013364
domContentLoaded13991966157612661
load14431977160013665
domInteractive22110372110
backgroundConnect975252110
firstReactRender1597282512
getState56517189
initialActions00000
loadScripts9591539113312459
setupStore67216199
uiStartup159426691848255123
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -2 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@jiexi jiexi changed the title Fix: fix CAIP-25 Permission Migration when eth_accounts has not been granted fix: fix CAIP-25 Permission Migration when eth_accounts has not been granted Feb 11, 2025
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@Gudahtt Gudahtt 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 enabled auto-merge February 11, 2025 17:25
@jiexi jiexi changed the title fix: fix CAIP-25 Permission Migration when eth_accounts has not been granted fix: cp-12.12.0 CAIP-25 Permission Migration when eth_accounts has not been granted Feb 11, 2025
@jiexi jiexi added this pull request to the merge queue Feb 11, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [b7860fb]
Page Load Metrics (1975 ± 120 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint28427011735624300
domContentLoaded156726791941249120
load160927051975251120
domInteractive24180433416
backgroundConnect889382412
firstReactRender1696482613
getState55913115
initialActions00000
loadScripts113521291450220106
setupStore882222211
uiStartup182429442264290139
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -2 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Merged via the queue into main with commit 93ec593 Feb 11, 2025
73 checks passed
@jiexi jiexi deleted the jl/fix-caip-25-permission-migration branch February 11, 2025 19:17
@github-actions github-actions bot locked and limited conversation to collaborators Feb 11, 2025
@metamaskbot metamaskbot added the release-12.13.0 Issue or pull request that will be included in release 12.13.0 label Feb 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.13.0 Issue or pull request that will be included in release 12.13.0 team-wallet-api-platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants