[refactor] Accept only ChannelProvider objects

[pedrouid]

Introduction

Right now, the client accepts three ways to be instantiated. It could either be through a mnemonic, xpub + keygen or ChannelProvider. The first two are essentially the same as on connect the mnemonic is used to generate an xpub + keygen to be passed to the CFCore module.

In the current codebase a lot of the methods include switch statements to handle RPC methods to be called using either the ChannelProvider or CounterfactualNode. Hence it would be a better practice to abstract the CounterfactualNode as a ChannelProvider that accesses CFCore module. This why we could remove all switch statements and remove the RpcType from the client.

Additionally, the Connext client connect method always calls the ChannelProvider enable method and it would be better practice to do this externally.

ClientOptions

Using the WalletConnect ChannelProvider the client options would be as follows

const channelProvider = new WalletConnectChannelProvider({ ...walletConnectConfig });

const options: ClientOptions = {
  ethProviderUrl: `https://daicard.io/api/ethprovider`,
  channelProvider,
}

const channel: Channel = await connext.connect(options)

Therefore the refactor would move the CounterfactualNode logic from the client's channelRouter to a channelProvider as follows

Before

const options: ClientOptions = {
  mnemonic: 'Apple Banana ...',
  ethProviderUrl: `https://daicard.io/api/ethprovider`,
  nodeUrl: `wss://daicard.io/api/messaging`,
  store
}

After

const channelProvider = new CounterfactualNodeChannelProvider({
  mnemonic: 'Apple Banana ...',
  store
});

const options: ClientOptions = {
  ethProviderUrl: `https://daicard.io/api/ethprovider`,
  nodeUrl: `wss://daicard.io/api/messaging`,
}

const channel: Channel = await connext.connect(options)

This not only simplify the connext client logic but have better seperation of concerns focusing on only connext exclusive logic inside the client without requiring unnecessary configuration like menmonic and store which are not used by other channel providers like WalletConnect for example.

Thus the ClientOptions will only require connext specific parameters like ethProviderUrl and nodeUrl.

CounterfactualNode ChannelProvider

As also described in the introduction the two possible configuration options for CounterfactualNode ChannelProvider would be mnemonic and xpub + keygen.

// using mnemonic
const channelProvider = new CounterfactualNodeChannelProvider({
  mnemonic: 'Apple Banana ...',
  store
});

// using xpub + keygen
const channelProvider = new CounterfactualNodeChannelProvider({
  xpub,
  keygen,
  store
});

const channel: Channel = await connext.connect(options)

The mnemonic option would also generate an xpub + keygen as well and this would be done on the ChannelProvider enable method.

ChannelProvider enable method

Another useful improvement to be included in this refactor is to not call the enable method for the ChannelProvider on the connect method for the client. This is not a good separation of concerns IMO and it also makes it harder for the developer integrating the Connext client to control the user experience. There will be scenarios, for example with WalletConnect, where the developer may or may not want to establish the connection with the provider separately to when the connext method is called for the client.

// for ChannelProvider 
const channelProvider = new ChannelProvider({ ...channelProviderConfig });

await channelProvider.enable()

const options: ClientOptions = {
  ethProviderUrl: `https://daicard.io/api/ethprovider`,
  channelProvider,
}

const channel: Channel = await connext.connect(options)

[rhlsthrm]

Totally agree with this proposal, great stuff @pedrouid!

[LayneHaber]

Yeah, this would be a lot cleaner for sure

[ArjunBhuptani]

Let's chat about this before you start working on it.

I like that this will separate concerns better, but most developers (especially those outside the space) will not know what a provider is. Aside from breaking our interfaces (which is in itself a problem), I'm worried that this refactor will make the process of creating a client much more complex because devs will have to think about the new CounterfactualNodeChannelProvider and what that means.

[pedrouid]

You can alternatively keep the same ClientOptions as is and instantiate a CounterfactualNodeChannelProvider internally making it backwards compatible. Thus on the client constructor logic we can check if a channelProvider is provided and if not just assume that developer wants a CFNode and provided the required parameters for it.

Similar to how you can configure a Web3 object with a simple string (rpc url) when in fact you are instantiating a HTTPProvider internally

[LayneHaber]

That's essentially just a cleaner way of doing what we are already doing and helps avoid a lot of switch statements within the code (which would be helpful)

[rhlsthrm]

Yeah, I think we should model it after libraries like Ethers. They have a way to get a default provider:

let provider = ethers.getDefaultProvider('ropsten');

Or you can go in and create your own providers:

let provider = new ethers.providers.Web3Provider(web3.currentProvider);

[rhlsthrm]

We can keep the APIs very simple for users by providing these interfaces but also having very nice APIs that wrap everything.

[ArjunBhuptani]

Ongoing work here: https://github.com/ConnextProject/indra/pull/618

[LayneHaber]

merged in #618 , will not only accept channel providers in connect to maintain backwards compatibility.

ideally will accept just a network string, but will address that in a separate issue