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

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

Open
mcmire opened this issue Oct 31, 2024 · 0 comments

Comments

@mcmire
Copy link
Contributor

mcmire commented Oct 31, 2024

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:

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:

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant