feat: add linea networks support by VGau · Pull Request #1423 · MetaMask/core

VGau · 2023-06-11T09:12:30Z

Description

Added linea networks support in controller utils
Added linea networks support in network controller
Added linea networks support in transaction controller
Updated @metamask/eth-json-rpc-infura package version to 8.1.0 to support linea networks

Changes

`@metamask/controller-utils`

ADDED: Add Linea networks to TESTNET_TICKER_SYMBOLS, BUILT_IN_NETWORKS, NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP, InfuraNetworkType, NetworkType, BuiltInNetworkName, ChainId, NetworkId, and NetworksTicker
- isNetworkType will now return true when given linea-mainnet or linea-goerli

`@metamask/network-controller`

ADDED: Add support for Infura provider types linea-mainnet and linea-goerli
CHANGED: Update @metamask/eth-json-rpc-infura package version to 8.1.0 to support Linea networks

`@metamask/transaction-controller`

CHANGED: Update approveTransaction, stopTransaction, and speedUpTransaction to use proper chain ID and network ID when creating transaction for Linea networks

References

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
I've highlighted breaking changes using the "BREAKING" category above as appropriate

Arschieqwe

Thanks

mcmire

Looks good, but I just had one suggestion.

packages/transaction-controller/src/TransactionController.ts

mcmire · 2023-06-12T17:21:04Z

Also, there are some differences between the "Description" and "Changes" sections in the PR description that perhaps we ought to clarify better:

The Description is intended for you to explain the purpose and scope of your changes and share knowledge that people might not be able to see just from reading the PR. The audience for this section is other developers who are maintaining this code. They might want to know: What is motivating this change (is there a ticket or can you provide some more context)? How do the changes work? If you had to upgrade a package, why?
The Changes is intended to be a list of your changes from the perspective of each package in the monorepo. The audience is consumers of the package (i.e. developers working on either the extension or mobile app) who will scan the changelog when they upgrade to a new version. They want to know: What are the exact changes to the API I need to be aware of (types, interfaces, functions, methods)? What are the effects on whichever platform might want to make use of these changes?

With that in mind, something you might want to highlight in the Description is, why did you have to upgrade @metamask/eth-json-rpc-infura? Why did you have to exclude Linea from the getCommonConfiguration method?

As for Changes, I think you could borrow from what you have in the Description. What if you updated Changes to the following?

### `@metamask/controller-utils`

- **ADDED:** Add Linea networks to `TESTNET_TICKER_SYMBOLS`, `BUILT_IN_NETWORKS`, `NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP`, `InfuraNetworkType`, `NetworkType`, `BuiltInNetworkName`, `ChainId`, `NetworkId`, and `NetworksTicker`
  - `isNetworkType` will now return true when given `linea-mainnet` or `linea-goerli`

### `@metamask/network-controller`

- **ADDED:** Add support for Infura provider types `linea-mainnet` and `linea-goerli`
- **CHANGED:** Update `@metamask/eth-json-rpc-infura` package version to 8.1.0 to support Linea networks

### `@metamask/transaction-controller`

- **CHANGED:** Update `approveTransaction`, `stopTransaction`, and `speedUpTransaction` to use proper chain ID and network ID when creating transaction for Linea networks

I filled in what I thought was a proper summary of the changes to the transaction controller from what I could guess, but feel free to clarify that further if you have more knowledge.

This should help us when building the changelog for a new release.

VGau · 2023-06-13T08:11:55Z

Also, there are some differences between the "Description" and "Changes" sections in the PR description that perhaps we ought to clarify better:

The Description is intended for you to explain the purpose and scope of your changes and share knowledge that people might not be able to see just from reading the PR. The audience for this section is other developers who are maintaining this code. They might want to know: What is motivating this change (is there a ticket or can you provide some more context)? How do the changes work? If you had to upgrade a package, why?

The Changes is intended to be a list of your changes from the perspective of each package in the monorepo. The audience is consumers of the package (i.e. developers working on either the extension or mobile app) who will scan the changelog when they upgrade to a new version. They want to know: What are the exact changes to the API I need to be aware of (types, interfaces, functions, methods)? What are the effects on whichever platform might want to make use of these changes?

With that in mind, something you might want to highlight in the Description is, why did you have to upgrade @metamask/eth-json-rpc-infura? Why did you have to exclude Linea from the getCommonConfiguration method?

As for Changes, I think you could borrow from what you have in the Description. What if you updated Changes to the following?
### `@metamask/controller-utils`

- **ADDED:** Add Linea networks to `TESTNET_TICKER_SYMBOLS`, `BUILT_IN_NETWORKS`, `NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP`, `InfuraNetworkType`, `NetworkType`, `BuiltInNetworkName`, `ChainId`, `NetworkId`, and `NetworksTicker`
  - `isNetworkType` will now return true when given `linea-mainnet` or `linea-goerli`

### `@metamask/network-controller`

- **ADDED:** Add support for Infura provider types `linea-mainnet` and `linea-goerli`
- **CHANGED:** Update `@metamask/eth-json-rpc-infura` package version to 8.1.0 to support Linea networks

### `@metamask/transaction-controller`

- **CHANGED:** Update `approveTransaction`, `stopTransaction`, and `speedUpTransaction` to use proper chain ID and network ID when creating transaction for Linea networks
I filled in what I thought was a proper summary of the changes to the transaction controller from what I could guess, but feel free to clarify that further if you have more knowledge.

This should help us when building the changelog for a new release.

Thanks for you comment @mcmire :) I updated "Changes" part in the PR description.

Regarding this Why did you have to exclude Linea from the getCommonConfiguration method?:
I needed to exclude Linea because @ethereumjs/common does not support Linea networks for now.

mcmire · 2023-06-13T17:56:02Z

@VGau Thanks! I think it would be good to add a test, even if it's just for approveTransaction, that guarantees that a transaction for Linea looks the way we expect, and then I can approve this PR.

socket-security · 2023-06-30T18:21:52Z

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@metamask/eth-json-rpc-infura	8.0.0...8.1.0	None	`+0/-0`	47.6 kB	gudahtt

mcmire · 2023-07-06T18:07:52Z

@VGau Is this ready for another review?

VGau · 2023-07-07T06:39:29Z

@VGau Is this ready for another review?

@mcmire Yes it is ready for another review :)

…ea networks

mcmire

This looks good to me! Sorry for the long wait on this.

mcmire · 2023-07-07T16:46:24Z

The test failures for assets-controller look intermittent — try re-running CI and they should work.

packages/controller-utils/src/types.ts

mcmire · 2023-07-11T17:25:28Z

@metamaskbot publish-preview

mcmire · 2023-07-11T17:39:03Z

Ah, I see, this is from a fork, so we can't create a preview-build. Temp PR created here: #1478

mcmire · 2023-07-11T18:23:55Z

Here are the preview builds for anyone who wants to test this in extension (note: you'll need to first add @metamask/eth-json-rpc-infura to package.json to populate the cache, then you can upgrade @metamask/controller-utils and @metamask/network-controller):

{
  "@metamask/address-book-controller": "3.0.0-preview.be1b962",
  "@metamask/announcement-controller": "4.0.0-preview.be1b962",
  "@metamask/approval-controller": "3.4.0-preview.be1b962",
  "@metamask/assets-controllers": "9.2.0-preview.be1b962",
  "@metamask/base-controller": "3.0.0-preview.be1b962",
  "@metamask/composable-controller": "3.0.0-preview.be1b962",
  "@metamask/controller-utils": "4.1.0-preview.be1b962",
  "@metamask/ens-controller": "4.0.0-preview.be1b962",
  "@metamask/gas-fee-controller": "6.0.1-preview.be1b962",
  "@metamask/keyring-controller": "6.0.0-preview.be1b962",
  "@metamask/logging-controller": "1.0.0-preview.be1b962",
  "@metamask/message-manager": "7.0.2-preview.be1b962",
  "@metamask/network-controller": "10.3.0-preview.be1b962",
  "@metamask/notification-controller": "3.0.0-preview.be1b962",
  "@metamask/permission-controller": "4.0.1-preview.be1b962",
  "@metamask/phishing-controller": "5.0.0-preview.be1b962",
  "@metamask/preferences-controller": "4.2.0-preview.be1b962",
  "@metamask/rate-limit-controller": "3.0.0-preview.be1b962",
  "@metamask/signature-controller": "5.1.0-preview.be1b962",
  "@metamask/transaction-controller": "7.0.0-preview.be1b962"
}

mcmire

@VGau I added some more tests yesterday so be sure to do a pull on your side if you need to add any more changes.

Regardless, everything here looks good to me.

Requested change won't be addressed at this time, but may be considered in the future

- Added linea networks support in controller utils - Added linea networks support in network controller - Added linea networks support in transaction controller - Updated `@metamask/eth-json-rpc-infura` package version to 8.1.0 to support linea networks Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

VGau requested a review from a team as a code owner June 11, 2023 09:12

This was referenced Jun 11, 2023

feat(551): add Linea Mainnet MetaMask/metamask-mobile#6496

Merged

fix: refactor linea testnet implementation MetaMask/metamask-mobile#6491

Merged

Arschieqwe approved these changes Jun 12, 2023

View reviewed changes

mcmire reviewed Jun 12, 2023

View reviewed changes

packages/transaction-controller/src/TransactionController.ts Show resolved Hide resolved

VGau force-pushed the feat/add-linea-networks branch from b0314d4 to 45672b5 Compare June 30, 2023 18:20

VGau added 5 commits July 7, 2023 08:39

feat: add linea mainnet and linea goerli to controller utils

f6f2e94

feat: add linea networks to transaction controller

2c6b15f

upgrade @metamask/eth-json-rpc-infura version to 8.1.0 to support lin…

90c0d76

…ea networks

feat: add unit tests for getCommonConfiguration

bb5ab1f

fix: reorder imports in transaction controller test file

17a9fef

VGau force-pushed the feat/add-linea-networks branch from 45672b5 to 17a9fef Compare July 7, 2023 06:54

mcmire previously approved these changes Jul 7, 2023

View reviewed changes

legobeat previously requested changes Jul 10, 2023

View reviewed changes

packages/controller-utils/src/types.ts Show resolved Hide resolved

mcmire mentioned this pull request Jul 11, 2023

[Bug]: Unknown Infura provider type "linea-goerli". MetaMask/metamask-extension#19906

Closed

mcmire added 2 commits July 11, 2023 10:44

Update NetworkController unit tests to account for Linea networks

d91976d

Merge branch 'main' into feat/add-linea-networks

be1b962

mcmire dismissed their stale review via be1b962 July 11, 2023 16:45

mcmire approved these changes Jul 12, 2023

View reviewed changes

Merge branch 'main' into feat/add-linea-networks

5043c23

mcmire merged commit ea3b5e9 into MetaMask:main Jul 12, 2023

mcmire mentioned this pull request Jul 21, 2023

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

Closed

2 tasks

Gudahtt mentioned this pull request Jul 25, 2023

chore: Update @metamask/controller-utils to v3.4 MetaMask/metamask-mobile#6884

Merged

3 tasks

Uh oh!

Conversation

VGau commented Jun 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

@metamask/controller-utils

@metamask/network-controller

@metamask/transaction-controller

References

Checklist

Uh oh!

Arschieqwe left a comment

Choose a reason for hiding this comment

Uh oh!

mcmire left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcmire commented Jun 12, 2023

Uh oh!

VGau commented Jun 13, 2023

Uh oh!

mcmire commented Jun 13, 2023

Uh oh!

socket-security bot commented Jun 30, 2023

Uh oh!

mcmire commented Jul 6, 2023

Uh oh!

VGau commented Jul 7, 2023

Uh oh!

mcmire left a comment

Choose a reason for hiding this comment

Uh oh!

mcmire commented Jul 7, 2023

Uh oh!

Uh oh!

mcmire commented Jul 11, 2023

Uh oh!

mcmire commented Jul 11, 2023

Uh oh!

mcmire commented Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcmire left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VGau commented Jun 11, 2023 •

edited

Loading

`@metamask/controller-utils`

`@metamask/network-controller`

`@metamask/transaction-controller`

mcmire commented Jul 11, 2023 •

edited

Loading