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 bring back exception if invalid address passed in eth_accounts #30213

Merged
merged 7 commits into from
Feb 11, 2025

Conversation

ffmcgee725
Copy link
Member

@ffmcgee725 ffmcgee725 commented Feb 7, 2025

This commit fixes the following issue which is currently a release blocker for v12.12.0

The eth accounts and permitted chains caveat validation in this PR was originally removed as part of the CAIP-25 Permission Migration PR here, but is being restored in order to fix a regression in the API behavior. Originally, making a wallet_requestPermissions request with invalid caveat values for eth_accounts or endowment:permitted-chains would result in an immediate error returned back to the dapp specifying which value was invalid, i.e. not found in the wallet. This PR restores that behavior.
Additionally, the changes in this PR are temporary and will be removed as part of a follow up refactor that cleans up the CAIP-25 approval/request permission flow.

Description

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

Unlock MM
Open test dapp console
Paste the following command
See error

Screenshots/Recordings

Before

After

Screen.Recording.2025-02-07.at.19.54.15.mov

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.

@ffmcgee725 ffmcgee725 requested a review from jiexi February 7, 2025 18:59
@ffmcgee725 ffmcgee725 marked this pull request as ready for review February 7, 2025 19:00
@ffmcgee725 ffmcgee725 requested a review from jiexi February 7, 2025 19:15
@metamaskbot
Copy link
Collaborator

❌ API Spec Test Failed. View the report here.

@metamaskbot
Copy link
Collaborator

❌ API Spec Test Failed. View the report here.

@metamaskbot
Copy link
Collaborator

Builds ready [84df286]
Page Load Metrics (1743 ± 70 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint49420381674308148
domContentLoaded14872021171815173
load15342040174314770
domInteractive2593372110
backgroundConnect116027168
firstReactRender1695362512
getState558232010
initialActions00000
loadScripts10681516124912560
setupStore663212110
uiStartup17192404202520598
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 363 Bytes (0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 841 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [6a7ef36]
Page Load Metrics (1725 ± 42 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint23118771652336161
domContentLoaded1590182316867737
load1604187917258742
domInteractive2486392010
backgroundConnect10109422813
firstReactRender1793342512
getState55820199
initialActions01000
loadScripts1101136912138239
setupStore75917178
uiStartup17942320197413665
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 365 Bytes (0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 841 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [7a9a4a3]
Page Load Metrics (1515 ± 45 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29317751461283136
domContentLoaded1361171814968642
load1369177915159545
domInteractive1795472512
backgroundConnect86323157
firstReactRender1482402512
getState3509105
initialActions01000
loadScripts959129910917838
setupStore7511095
uiStartup15392171172214369
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 365 Bytes (0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 841 Bytes (0.01%)

@jiexi
Copy link
Contributor

jiexi commented Feb 11, 2025

Manually validated

@ffmcgee725 ffmcgee725 enabled auto-merge February 11, 2025 16:41
@ffmcgee725 ffmcgee725 added this pull request to the merge queue Feb 11, 2025
@itsyoboieltr itsyoboieltr removed this pull request from the merge queue due to a manual request Feb 11, 2025
@itsyoboieltr itsyoboieltr changed the title fix: bring back exception if invalid address passed in eth_accounts fix: cp-12.12.0 bring back exception if invalid address passed in eth_accounts Feb 11, 2025
@itsyoboieltr itsyoboieltr added this pull request to the merge queue Feb 11, 2025
Merged via the queue into main with commit 74d8431 Feb 11, 2025
78 checks passed
@itsyoboieltr itsyoboieltr deleted the fix/value-must-be-hex-wallet-error branch February 11, 2025 17:25
@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
Archived in project
Development

Successfully merging this pull request may close these issues.

[Bug]: Permissions - Wallet breaks with error Message: Value must be a hexadecimal string, starting with "0x".
4 participants