MetaMask / MetaMask/core

[NetworkController] Give consumers a way to retrieve network configuration alongside network client

Open
#4,883 0 comments 0 reactions 0 assignees View on GitHub
enhancement team-core-platform team-wallet-framework wf-network-controller-improvements
Dominant language
TypeScript
Stars
413
Forks
308
Avg merge
1d 4h
Merged PRs (30d)
253

Description

## Problem

As we slowly multichain-ize our code, we are converting code that implicitly uses the global network to instead require a reference to a network client ID. In some cases, we need the chain ID associated with a network. The typical way to do this is:

``` typescript
function someFunctionThatUsesTheNetwork({ networkClientId }: { networkClientId: NetworkClientId }) {
const networkClient = messenger.call(
'NetworkController:getNetworkClientById',
networkClientId,
);
const chainId = networkClient.configuration.chainId;
// ...
}
```

However, this is a bit of a hack. The reason for this is that `configuration` property on a network client holds data that is used to initialize the client for a chain. But we do not need a chain ID to initialize a network client, since a chain already knows its ID. We only added `chainId` to `NetworkClientConfiguration` for convenience, but it really shouldn't be there. Instead, the _network configuration_ is where consumers should get the chain ID.

The NetworkController does have a `getNetworkConfigurationByNetworkClientId`, but unfortunately it is not as convenient to use as it should, as it is possible for this method to return `undefined`. So while it would be more accurate to say this, I imagine consumers would probably rather not say it:

``` typescript
function someFunctionThatUsesTheNetwork({ networkClientId }: { networkClientId: NetworkClientId }) {
const networkClient = messenger.call(
'NetworkController:getNetworkClientById',
networkClientId,
);
const networkConfiguration = messenger.call(
'NetworkController:getNetworkConfigurationByNetworkClientId',
networkClientId,
);
if (networkConfiguration === undefined) {
throw new Error("Network configuration not found for network client ID '${networkClientId}'");
}
const chainId = networkConfiguration.chainId;
// ...
}
```

We can make this better for consumers.

## Acceptance Criteria

- A method exists in NetworkController that allows consumers to pass a network client ID and get the network client _and_ the network configuration in one go. This method should throw if a network configuration cannot be found so we don't have to return `undefined` (or we should find a way to avoid the `undefined` altogether).
- The `getNetworkConfigurationByNetworkClientId` method and action are deprecated.
- The `configuration` property on NetworkClient is deprecated.

## Considerations

- Consider calling the new method `findNetworkByNetworkClientId`, and the combination of a network client + network configuration a `Network`. The reasoning here is that we already have a light concept of a "network" because we have the methods `addNetwork`, `updateNetwork` and `removeNetwork`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with NetworkController and its existing getNetworkConfigurationByNetworkClientId and getNetworkClientById methods and actions. Trace how network clients and network configurations are exposed to consumers, then implement the combined lookup with the requested missing-configuration behavior. Done means the old method and action, plus NetworkClient.configuration, are deprecated and the acceptance criteria are covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.