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: relax network symbol length validation #24872

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

bergeron
Copy link
Contributor

@bergeron bergeron commented May 29, 2024

Description

When adding a network with wallet_addEthereumChain, MetaMask was requiring the chain's symbol to be minimum of 2 characters. But there are networks with single character symbols. And MetaMask already lets you manually add a network with a single character symbol. And it renders fine in the UI. So relaxing the validation.

Open in GitHub Codespaces

Related issues

Manual testing steps

  1. Visit https://chainid.network/
  2. Connect wallet
  3. Search for chainid 35443
  4. Add chain
  5. Verify it added successfully

Screenshots/Recordings

Before

Screenshot 2024-05-29 at 8 10 39 AM

After

Screenshot 2024-05-29 at 8 11 25 AM

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • 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.

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.

@bergeron bergeron added team-assets needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. labels May 29, 2024
@bergeron bergeron requested review from sahar-fehri and salimtb May 29, 2024 15:19
@bergeron bergeron requested a review from a team as a code owner May 29, 2024 15:19
Copy link
Contributor

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.

Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 65.46%. Comparing base (cfe39c5) to head (fb4b859).
Report is 2 commits behind head on develop.

Files Patch % Lines
...method-middleware/handlers/ethereum-chain-utils.js 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #24872   +/-   ##
========================================
  Coverage    65.46%   65.46%           
========================================
  Files         1351     1351           
  Lines        53948    53948           
  Branches     14017    14017           
========================================
  Hits         35316    35316           
  Misses       18632    18632           

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

@metamaskbot
Copy link
Collaborator

Builds ready [6b3ec6c]
Page Load Metrics (1409 ± 638 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint691911183617
domContentLoaded10251342
load58302214091330638
domInteractive10251342
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [fb4b859]
Page Load Metrics (44 ± 3 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint60957594
domContentLoaded812910
load40584463
domInteractive812910
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@bergeron bergeron merged commit 923ce5f into develop Jun 5, 2024
77 checks passed
@bergeron bergeron deleted the brian/chain-symbol-length branch June 5, 2024 20:52
@github-actions github-actions bot locked and limited conversation to collaborators Jun 5, 2024
@gauthierpetetin gauthierpetetin added the release-12.0.0 Issue or pull request that will be included in release 12.0.0 label Jun 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants