Load gas and gasPrice from networkConfig

[nventuro]

Fixes #1227

The change itself is rather simple: the only thing I'm unsure is whether we should test this as a unit test, integration, or not at all.

That said, I want to discuss my findings while looking into this issue to provide some context that may help inform future decisions.

lib defines a TxParams object that is passed around when sending transactions and serves as a default for these. This object is populated by ConfigManager from the network configuration file, and before this PR, it only includes from.

Because of this, I was surprised to see that the correct values for gas are nonetheless used in all transactions: it turns out lib/utils/Transactions handles a null value for gas by loading it from Contracts.getArtifactsDefault, which acts as a sort of global store of configuration. Notice that it was ConfigManager who called Contracts.setArtifactsDefaults, duplicating these values.

The bug surfaced in transfer because this is the first instance of the SDK sending transactions that are not contract function calls. On those, because the gas price was never specified, web3 used the default one that is set when creating the contract objects.

There seems to be a tension here between global configuration that is accessed directly (either via the Contracts.setDefaultArtifacts or the implicit default values used in the web3 objects), and explicit configuration objects that are passed down the call stack. We may want to revisit this at some point.

From the description above, it should be clear that gas and gasPrice were always obtained from getArtifactDefault, when available. Because of this, I'm relatively confident that this PR will not break current behaviour.

[nventuro]

The explanation ended up being a bit too wall-of-texty. Sorry about that!

[spalladino]

Can we change this so these keys are not set if their value is empty? I want to avoid a case where we merge txParams using an object splat, which would end up overriding an existing value with undefined if we're not careful.

[spalladino]

As an example, in the following code, if txParams = { gasPrice: undefined }, the default 10e9 value would get overwritten with undefined (which is not the desired behaviour).

function foo(txParams) {
  const txParamsDefaults = { gasPrice: 10e9 }
  const txParamsToUse = { ...txParamsDefaults, ...txParams }
  ...
}

[nventuro]

This is currently not an issue due to how these values are created (empty entries are removed), but I addressed this nonetheless in the style suggested in #1231.

I also created #1346 to track this general issue.

[spalladino]

There seems to be a tension here between global configuration that is accessed directly (either via the Contracts.setDefaultArtifacts or the implicit default values used in the web3 objects), and explicit configuration objects that are passed down the call stack. We may want to revisit this at some point.

Agree. This is one of the things I want to fix when reviewing the programmatic API: no more global state.