Skip to content

feat: add support for consensys zkevm network (Linea) - #1126

Closed
VGau wants to merge 3 commits into
MetaMask:mainfrom
VGau:feat/add-consensys-zkevm-support
Closed

feat: add support for consensys zkevm network (Linea)#1126
VGau wants to merge 3 commits into
MetaMask:mainfrom
VGau:feat/add-consensys-zkevm-support

Conversation

@VGau

@VGau VGau commented Mar 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Title
Added support for ConsenSys zkEVM test network (Linea).

Description
It was added ConsenSys zkEVM test network (Linea) support to our Network Controller.

  • CHANGED:
    • The network controller has been updated to support Consensys zkEVM (Linea)
    • New test cases have been added to use ConsenSys zkEVM network (Linea)

Checklist

  • Tests are included if applicable
  • Any added code is fully documented

@VGau
VGau requested a review from a team as a code owner March 13, 2023 16:16
@VGau VGau mentioned this pull request Mar 13, 2023
Merged
3 tasks
@VGau VGau changed the title feat: add support for consensys zkevm network feat: add support for consensys zkevm network (Linea) Mar 14, 2023
@VGau
VGau force-pushed the feat/add-consensys-zkevm-support branch from ce939fd to 2a32fdd Compare March 15, 2023 13:18
sethkfman
sethkfman previously approved these changes Mar 16, 2023
View reviewed changes

@sethkfman sethkfman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

mcmire
mcmire reviewed Mar 16, 2023
View reviewed changes

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This makes sense. Looks like there's one block of tests missing though. I think you'll want to copy the describe starting at line 2222 to a new one and change instances of "localhost" to "lineatestnet".

Comment thread packages/network-controller/tests/NetworkController.test.ts Outdated
sethkfman
sethkfman previously approved these changes Mar 17, 2023
View reviewed changes

@sethkfman sethkfman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed typo. LGTM

@sethkfman

sethkfman commented Mar 17, 2023

Copy link
Copy Markdown
Contributor

@mcmire Can you confirm?

mcmire
mcmire previously approved these changes Mar 17, 2023
View reviewed changes

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, LGTM!

@VGau
VGau dismissed stale reviews from mcmire and sethkfman via 35f4930 March 18, 2023 15:06
@VGau
VGau force-pushed the feat/add-consensys-zkevm-support branch from 7ed667d to 35f4930 Compare March 18, 2023 15:06
@mcmire

mcmire commented Mar 20, 2023

Copy link
Copy Markdown
Collaborator

@Gudahtt I notice you left a comment in Slack about this PR, are you good with this?

@VGau

VGau commented Mar 20, 2023

Copy link
Copy Markdown
Contributor Author

@Gudahtt I notice you left a comment in Slack about this PR, are you good with this?

Yes I am good with this PR thanks 😄

@Gudahtt

Gudahtt commented Mar 23, 2023

Copy link
Copy Markdown
Member

Nope, no objections. But I will add a DO-NOT-MERGE label for now because since this was approved, the strategy used to support this network on mobile has changed. We need to revisit whether we need a change to the network controller at all, or whether this can be handled solely in mobile instead.

@mcmire

mcmire commented Jul 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Now that #1423 has been merged, should we close this?

@Gudahtt

Gudahtt commented Jun 24, 2024

Copy link
Copy Markdown
Member

I'm assuming we don't need this anymore; if it turns out we do, we can open a new PR.

@Gudahtt Gudahtt closed this Jun 24, 2024
@saustrie-consensys saustrie-consensys mentioned this pull request Aug 12, 2026
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcmire mcmire mcmire approved these changes
+1 more reviewer
@sethkfman sethkfman sethkfman left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@VGau @sethkfman @mcmire @Gudahtt