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(551): add Linea Mainnet #6496

Merged
merged 14 commits into from
Jun 20, 2023
Merged

Conversation

VGau
Copy link
Contributor

@VGau VGau commented May 30, 2023

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

Note: ⚠️ For now, the linea mainnet url is not working so I cannot record a successful transaction for the new linea mainnet network.

Screenshots/Recordings

zEsRhpdUna.mp4

If applicable, add screenshots and/or recordings to visualize the before and after of your change

Issue

Progresses #???

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@VGau VGau requested a review from a team as a code owner May 30, 2023 16:06
@github-actions
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.

@VGau VGau force-pushed the feat/551-add-linea-mainnet branch from 2c41a8c to 97b6b03 Compare June 2, 2023 12:21
@sethkfman
Copy link
Contributor

@VGau are these ready for dev-review?

@VGau VGau force-pushed the feat/551-add-linea-mainnet branch 2 times, most recently from 7350705 to e4d2119 Compare June 8, 2023 08:01
@sethkfman sethkfman added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jun 9, 2023
@VGau
Copy link
Contributor Author

VGau commented Jun 11, 2023

@VGau are these ready for dev-review?

@sethkfman Yes it is ready for dev review :)

@VGau VGau force-pushed the feat/551-add-linea-mainnet branch from e4d2119 to 1ce6576 Compare June 13, 2023 08:33
@sethkfman
Copy link
Contributor

Do you know when the functional url is going to be in place so you can show a successful transaction?

@VGau
Copy link
Contributor Author

VGau commented Jun 13, 2023

Do you know when the functional url is going to be in place so you can show a successful transaction?

I don't have the answer yet sorry. For now there is a feature toggle for linea mainnet so that it won't be displayed :)

@VGau VGau force-pushed the feat/551-add-linea-mainnet branch from b0978ef to 88f2296 Compare June 13, 2023 19:16
@sethkfman
Copy link
Contributor

sethkfman commented Jun 13, 2023

@VGau Can you add a video showing this there is no functional impact to displaying the current networks?

Also, can you add a description how you plan to test the network and if it fails how it will be disabled? This will help QA understand the feature is triggered from a specific response on infura endpoint.

sethkfman
sethkfman previously approved these changes Jun 13, 2023
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

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

LGTM

@sethkfman sethkfman added release-7.2.0 needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Jun 13, 2023
@sethkfman
Copy link
Contributor

@cortisiko This is a dormant feature that is enabled via infura flag. You will not see any functional difference. It should only need regression/e2e QA but I wanted you to confirm.

@cortisiko
Copy link
Member

hey @VGau a couple of things:

  • What should be the behavior when linea mainnet is turned on? Should the network appear in the default network list like mainnet or any other test net?

  • is there a way to verify that the feature flag is working? We don’tt necessarily have to use the linea mainnet rpc url to test. what if we use another rpc url just to make sure when we toggle the feature on the app doesnt break.

  • Can you do some smoke tests to verify that mainnet, an infura-supported custom net like polygon is still working?

@VGau
Copy link
Contributor Author

VGau commented Jun 14, 2023

hey @VGau a couple of things:

  • What should be the behavior when linea mainnet is turned on? Should the network appear in the default network list like mainnet or any other test net?
  • is there a way to verify that the feature flag is working? We don’tt necessarily have to use the linea mainnet rpc url to test. what if we use another rpc url just to make sure when we toggle the feature on the app doesnt break.
  • Can you do some smoke tests to verify that mainnet, an infura-supported custom net like polygon is still working?

Hey @cortisiko :

  • The linea mainnet network will appear in the default network list like mainnet
  • If you want to display the Linea mainnet network for your tests you can reverse the boolean result of this function shouldShowLineaMainnetNetwork in app/util/networks/index.js
  • Yes sure I can do a smoke test :)

@VGau
Copy link
Contributor Author

VGau commented Jun 14, 2023

@VGau Can you add a video showing this there is no functional impact to displaying the current networks?

Also, can you add a description how you plan to test the network and if it fails how it will be disabled? This will help QA understand the feature is triggered from a specific response on infura endpoint.

The feature toggle for the linea mainnet network performs two checks:

  • check if the current time is higher than the release date time
  • check if the linea mainnet rpc url is ready by calling the following endpoint: eth_chainid

If one of those two checks is false, the linea mainnet won't be displayed.

@VGau
Copy link
Contributor Author

VGau commented Jun 14, 2023

@VGau Can you add a video showing this there is no functional impact to displaying the current networks?

Also, can you add a description how you plan to test the network and if it fails how it will be disabled? This will help QA understand the feature is triggered from a specific response on infura endpoint.

By default the linea mainnet network won't be displayed on MM. It will be displayed only if the current date time is higher than July 11th and if the linea mainnet rpc url is working.

@sethkfman
Copy link
Contributor

@VGau Can you resolve the conflicts? It looks like the testnet PR is causing issues. We will get the PR merge on Tuesday.

@VGau VGau force-pushed the feat/551-add-linea-mainnet branch from e7b5cdc to d721f0b Compare June 17, 2023 07:54
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

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

LGTM

@sethkfman sethkfman merged commit 9300ff9 into MetaMask:main Jun 20, 2023
9 of 10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 20, 2023
@sethkfman sethkfman removed the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Jun 20, 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.

None yet

3 participants