core
core copied to clipboard
Merge the two network controllers
Merge the network controller in this repository with the extension network controller.
General guidelines:
- Write comprehensive unit tests for both current network controllers
- This is the first step so that we can be certain how each controller behaves before we merge them. This will help ensure nothing changes unintentionally, especially breaking changes.
- We can test the "configuration management" and the "RPC message handling" layers separately. For both current controllers, most of the controller API is for configuration management. The RPC message handling is solely in the "network client" object, which we can test independently from the controller API.
- Document all functional differences between each controller
- Extract any API interactions or complex utility functions
- We want to minimize the direct responsibilities that our controllers have, so that they are simpler and easier to maintain. They need to be responsible for ensuring the integrity of our state, so that the wallet is always in a "valid" state, and to accomplish this goal the controllers will include most of the "business logic" of the wallet. But we can extract parts of this logic to other modules/libraries/middleware/etc. as appropriate, to keep the controllers focused on high-level operations and state management.
- At the moment, there are many RPC middleware steps setup by the extension network controller that have nothing to do with the network (see
createMetamaskMiddleware.js). These can be extracted to be setup alongside the rest of our RPC pipeline. - The extension network controller is also responsible at the moment for creating the "provider proxy" that we use internally. This could be extracted from the controller, it doesn't need to have this responsibility.
- Normalize functionality across both controllers
- We can proceed one-at-a-time through the list of functional differences prepared earlier. For each difference, we can change one or both controllers to have the same behaviour. The fewer differences we have, the easier the final migration will be.
- It may not be practical to remove 100% of differences at this stage. Use your best judgement to decide whether a difference can be eliminated before the merge or not.
- Migrate the
@metamask/controllersnetwork controller to BaseControllerV2- This will serve as the basis for the merged controller.
- Migrate the extension to use the
@metamask/controllersnetwork controller.
Notes from standup today:
- Currently we are using
web3-provider-enginein mobile andjson-rpc-enginein extension, which relies onweb3. We should usejson-rpc-enginein the shared version to remove that dependency. - Currently we use
eth-queryto talk to the network. We should consider usingethersinstead, as it has a simpler, more piecemeal API and supports TypeScript. This requires thateth-json-rpc-middlewarereturn a provider that is EIP-1194-compatible, so that it can be passed toethers.
To do both of these, we should first write unit tests in the extension and in mobile for the JSON-RPC methods that are usable by the provider that NetworkController exposes. This way we can ensure that the behavior that our new version of NetworkController exposes supports all of the same functionality as the individual network controllers, and therefore it should be easier for the new version to be a drop-in replacement for the old.