Investigate function impact of replacing web3-provider-engine

[Gudahtt]

We should investigate the functional changes that will result from replacing web3-provider-engine with the extension's network client.

Scope:

• "network methods", i.e. anything the network client tests cover

Action plan:

• Discover differences by running network client tests against web3-provider-engine
• Document differences
• Decide what to do on a case-by-case basis

[xinnanyemm]

Hey team! Please add your planning poker estimate with Zenhub @Gudahtt @mcmire @xinnanyemm

[Gudahtt]

Please add your planning poker estimate with Zenhub @cryptodev-2s

[kenhkan]

Only remaining item: the mobile difference list

[BelfordZ]

eth_chainId

Currently

• On mobile, calls to eth_chainId are handled in RPCMethodMiddleware, using the value from providerConfig
• On extension, the network is not hit. When the provider type is infura, chainId is used from a constant. For custom providers, it uses the value from the providerConfig. For localhost, it would either be statically set to return 0x1337, or if you use _setProvider directly, it would be what ever chainId you passed in.

After web3-provider-engine removal

• If the provider type is infura, chainId is used from a constant. If provider type is localhost or rpc, chainId will be the value in the provider config. If you use setProviderType with providerType localhost (which doesn't allow setting chainId), chainId will be 0x0.
• If the chainId in providerConfig is alphanumeric, but not valid hex, it will now throw.
• in mobile, this method is overridden anyways.

net_version

Currently

• On mobile, calls to net_version use a static result if they are included in the list of 'allNetworks' (mainnet, ropsten, kovan, rinkeby, goerli). Otherwise, they will hit the network to resolve this.
• On extension. when the providerType is infura, net_version uses a static result. When its a custom rpc, the network is hit to fetch the value.

After web3-provider-engine removal

• when the providerType is infura, net_version uses a static result. When its a custom rpc, the network is hit to fetch the value.
• in mobile, this method is overridden anyways, but the same check will happen twice.

BlockRef

Currently

• on mobile, block tags ('latest', 'pending', etc) are left as is
• on extension, they are re-written based on the value held by the blockTracker

After web3-provider-engine removal

• blockrefs are re-written with the values from blockTracker.

Sanitization Middleware

Currently

• on mobile, requests who have transaction-like or filter-like params have 0x prefixes added if they are missing. We assume that the value is already hex, though. In practice, only eth_call seems to hit this middleware, since all the other methods are handled by earlier middleware
• on extension, no such behaviour exists.

After web3-provider-engine removal

• network controller no longer handles this behaviour. If we want to preserve this functionality, we can merge Refactor sanitization middleware metamask-mobile#6079

NonceTracker

Currently

• on mobile, requests that impact the nonce for an address are updated and maintained inside of the noncetracker middleware.
• on extension, we use a package called nonce-tracker in the transaction controller.

After web3-provider-engine removal

• calls to getTransactionCount made to the provider are not handled by nonce-tracker.

[mcmire]

@BelfordZ For the nonce tracker, does #1147 being merged now change anything going forward? i.e. you say after web3-provider-engine is removed, calls to getTransactionCount are no longer cached, but would that still be the case?

[BelfordZ]

@mcmire yes, thank you for pointing that out. #1147 will resolve the difference as it pertains to the transaction controller. More broadly speaking, other calls to eth_getTransactionCount that might exist (or received through dapp api) still wont be cached, though. I will update with the above considerations.

edit: let me double check this...

getTransactionCount gets cached by block cache middleware from with a strategy of 'Block' - so it is cached, but not for long. web3-provider-engine actually uses the same block cache, so the behaviour doesnt change.

edit again: I think cache is the wrong word to use. I should say, handled by the nonce-tracker middleware inside web3-provider-engine/zero.

[BelfordZ]

@mcmire upon further inspection, it seems we still need to update mobile to add some middleware that calls the transactionControllers nonce tracker when handling getTransactionCount, similar to how we do it in extension:

• metmask-controller calls createMetamaskMiddleware
• createMetamaskMiddleware handles eth_getTransactionCount

With just #1147 - the nonce tracker wont get used when the requests to the method eth_getTransactionCount are coming through the provider.

[mcmire]

Makes sense @BelfordZ!

[Gudahtt]

Regarding the eth_getTransactionCount method, I have a draft branch on mobile that should ensure that behaves the same as before. I've just left a comment here about it: https://github.com/MetaMask/mobile-planning/issues/798#issuecomment-1505919649
I can handle it as part of that issue

[Gudahtt]

If provider type is localhost or rpc, chainId will be the value in the provider config. If you use setProviderType with providerType localhost (which doesn't allow setting chainId), chainId will be 0x0.

If the chainId in providerConfig is alphanumeric, but not valid hex, it will now throw.

Note that as of #1064, we have validation in place to ensure custom networks have a valid hex chain ID set. So this scenario should no longer be possible.

The situation with Localhost is weird though. That built-in localhost network isn't used by mobile, nor would it be useful for the extension. I'll make a note to remove that at some point.