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

feat: add the ConsenSys zkEVM (Linea) as a default network #17875

Merged
merged 10 commits into from
Mar 21, 2023

Conversation

VGau
Copy link
Contributor

@VGau VGau commented Feb 23, 2023

Explanation

The goal of the PR is to add the ConsenSys zkEVM network (Linea) as a default test network like Goerli and Sepolia.
I've updated all translation files.
This change can only be tested when this PR (MetaMask/design-tokens#408) is merged and the new design tokens library version is updated on metamask-extension repo.

Screenshots/Screencaps

Before

Screenshot 2023-02-23 at 11 56 19 AM

After

Screenshot 2023-03-14 at 6 01 16 PM

Manual Testing Steps

Step 1:
Go to the metamask home screen and select Goerli test network in the dropdown.
Screenshot 2023-03-14 at 6 01 16 PM

Step 2:
Go to this website: https://bridge.goerli.zkevm.consensys.net/send?token=ETH and send some ETH (minimum 0.1) from Goerli to Consensys zkEVM .
Screenshot 2023-02-23 at 12 04 31 PM

Wait for transaction to be finished.

Step 3:
Go to the metamask home screen and select then new ConsenSys zkEVM test network in the dropdown.
Screenshot 2023-03-14 at 6 01 16 PM

Step 4:
Go to this website: https://bridge.goerli.zkevm.consensys.net/send?token=ETH and send some ETH (minimum 0.1) from Consensys zkEVM to Goerli.
Screenshot 2023-02-23 at 12 07 30 PM

Wait for transaction to be finished.

Final step:
Check if all transactions succeed on Metamask.

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@VGau VGau requested review from a team and kumavis as code owners February 23, 2023 11:10
@github-actions
Copy link
Contributor

github-actions bot commented Feb 23, 2023

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.

@VGau
Copy link
Contributor Author

VGau commented Feb 23, 2023

I have read the CLA Document and I hereby sign the CLA

shared/constants/network.ts Outdated Show resolved Hide resolved
shared/constants/network.ts Outdated Show resolved Hide resolved
shared/constants/network.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

Hey @VGau, this is looking good from a UI perspective. Do you know if #234FD5 is the brand color for Consensys? We could add this too our design tokens and replace it so consensys zkevm has it's own color and doesn't need to use the Goerli color

@VGau VGau changed the title feat: add the ConsenSys zkEVM as a default network feat: add the ConsenSys zkEVM (Linea) as a default network Mar 14, 2023
@georgewrmarshall
Copy link
Contributor

georgewrmarshall commented Mar 15, 2023

Hey @VGau, I was wondering instead of using color we could add these images for test networks?? I think it's a lot more scalable

logo-test-network-goerli
logo-test-network-linea
logo-test-network-sepolia

@VGau
Copy link
Contributor Author

VGau commented Mar 15, 2023

Hey @VGau, I was wondering instead of using color we could add these images for test networks?? I think it's a lot more scalable

logo-test-network-goerli logo-test-network-linea logo-test-network-sepolia

Yes sure ! I will add these images. Should I add these images in the networks dropdown too ? 😄

@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch 2 times, most recently from d37ac5d to ceb2d63 Compare March 15, 2023 12:39
@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #17875 (dd72990) into develop (a854cfd) will decrease coverage by 0.04%.
The diff coverage is 43.14%.

@@             Coverage Diff             @@
##           develop   #17875      +/-   ##
===========================================
- Coverage    64.19%   64.15%   -0.04%     
===========================================
  Files          908      908              
  Lines        35211    35256      +45     
  Branches      8989     9006      +17     
===========================================
+ Hits         22601    22617      +16     
- Misses       12610    12639      +29     
Impacted Files Coverage Δ
...ethod-middleware/handlers/switch-ethereum-chain.js 15.49% <0.00%> (-0.22%) ⬇️
...network-screen/loading-network-screen.component.js 0.00% <0.00%> (ø)
...t-original/signature-request-original.component.js 52.21% <0.00%> (-0.94%) ⬇️
...p/signature-request/signature-request.component.js 74.68% <0.00%> (-1.94%) ⬇️
ui/components/ui/typography/typography.js 100.00% <ø> (ø)
ui/helpers/constants/design-system.ts 100.00% <ø> (ø)
ui/pages/routes/routes.component.js 0.00% <0.00%> (ø)
...es/settings/networks-tab/networks-tab.constants.js 100.00% <ø> (ø)
ui/helpers/constants/settings.js 33.77% <33.33%> (-0.01%) ⬇️
ui/components/app/dropdowns/network-dropdown.js 38.52% <42.31%> (-0.22%) ⬇️
... and 3 more

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch 5 times, most recently from 6ff785b to 2c44b0e Compare March 15, 2023 15:38
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

UI approved!

I think it's fine, but it might be worth checking if prod/your design contact is fine with the color in dark mode

Screenshot 2023-03-15 at 8 41 17 AM

Screenshot 2023-03-15 at 8 41 52 AM

@@ -4,6 +4,8 @@
--mainnet: #29b6af;
--inherit: inherit;
--transparent: transparent;
--color-network-linea-testnet-default: #000000;
--color-network-linea-testnet-inverse: #fcfcfc;
// DO NOT CHANGE
Copy link
Contributor

@georgewrmarshall georgewrmarshall Mar 15, 2023

Choose a reason for hiding this comment

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

Note to other reviewers: Usually we wouldn't allow static hex values but I think we can refactor the test networks to use images like other token icons during the multichain v0.5 work so this will be temporary.

@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch from 2c44b0e to f99cf8a Compare March 15, 2023 17:21
@danjm
Copy link
Contributor

danjm commented Mar 16, 2023

I believe there is a requirement that this network not be shown until March 28. Can you add some logic to the network dropdown and the settings sections to ensure that is the case.

roughly something like:

const shouldShowZKEVM = Date.now() > (new Date('March 28, 2023 12:00:00'))

And use that to conditionally render the network in the dropdown and in the network settings pages

@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch 3 times, most recently from 7bf78e7 to b9457fa Compare March 17, 2023 15:56
@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch 4 times, most recently from c6dade4 to b765608 Compare March 20, 2023 14:23
@danjm
Copy link
Contributor

danjm commented Mar 20, 2023

this branch probably needs to be updated to latest develop to avoid the prep-build-desktop failure

@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch 2 times, most recently from 98f6535 to 4a63413 Compare March 20, 2023 15:57
@VGau VGau force-pushed the feat/add-consensys-zkevm-network branch from 4a63413 to 9b30ce0 Compare March 20, 2023 17:27
Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

These changes look good to me

@danjm danjm merged commit a04fa20 into MetaMask:develop Mar 21, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants