gasLimit should be gas

[MicahZoltu]

ethers.js/src.ts/utils/transaction.ts

Line 71 in 9b118af

{ name: 'gasLimit', maxLength: 32 },

https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sendtransaction
https://wiki.parity.io/JSONRPC-eth-module#eth_sendtransaction
https://wiki.parity.io/JSONRPC-eth-module#eth_signtransaction

Notice that both Geth and Parity documentation call the parameter gas, but Ethers calls it gasLimit. This means if I construct a transaction such that Geth or Parity will accept it, I cannot use Ethers wallet to sign it. Inversely, if I construct a transaction such that Ethers can sign it, Geth/Parity will not accept it as valid (I believe, I haven't tested giving them gasLimit instead of gas).

I believe MetaMask also expects gas for signing, but I have not confirmed this.

[ricmoo]

This is intentional and was part of the original library. The term gas has led to confusion and “back in the day” I had many discussion with many developers about this, and the consensus was gasLimit was more obvious what it does.

If you look at the JSON-RPC code, you will see it does the conversion and contracts will through if invalid fields are given in the overrides.

[MicahZoltu]

I agree that gasLimit is a better variable name, but the problem is that I sometimes want/need a transaction to be valid whether it is given to ethers or directly to a node or to web3 or some other library. In this case, if I give a transaction that is valid elsewhere to Wallet.sign, it will not produce a proper signature because that function implicitly convert gas to gasLimit.

If the intent is to support both (standard way and better way), then this may just be a bug in Wallet.sign.

[ricmoo]

There is no intention of supporting both in ethers.js, since that just adds more confusion to people that are not using both Web3 and ethers.js.

There is the ethers-web3-bridge, which I'm half done updating for v4 along with TypeScript, which allows a Web3 instance to use a ethers.js provider, which manages these conversions for you. This is mainly intended for migration between ethers.js and Web3 (either direction of migration is supported). :)

I should ensure that any un-recognized parameters passed into Wallet.prototype.sign throw an exception though; fail early and fail loud. :)

[ricmoo]

Version 4.0.5 will ensure any invalid properties passed in will throw an error, which should prevent people from incorrectly passing gas instead or gasLimit.

[MicahZoltu]

This is a pretty major change, at the least it should be considered a breaking change IMO (major version bump). Code that reasonably worked before will no longer work as of this change, so I think patch version is incorrect.

Also, I'm pretty strongly against this change for a couple reasons:

1. It violates the principle of "be permissive in what you accept, as long as it is fully valid".
2. It makes transactions that follow the defacto standard hard fail, even if those transactions are also fully compatible with ethers.js.

This change assumes that the transaction being passed in is an ethers Transaction, not something constructed externally and being utilized by ethers (and possibly other systems). As an example, I have a system that integrates with many other things (not just ethers.js) and those other things expect a different format of transactions (the standard gas instead of the non-standard gasLimit). Already I have to construct my transaction to be compatible with both styles at the same time in order to satisfy ethers' non-standard use of gasLimit, and now this change is making it so I can't even do that (include both gas and gasLimit) on the transaction, I have to construct separate transaction objects for each signing tool. This makes the library notably more complicated to use, and is most likely going to result in me having to write a wrapper around ethers that trims off transaction parameters that ethers doesn't expect prior to passing through to ethers.

It is also possible that the system integrating with ethers is dropping additional variables onto the transaction object for its own internal tracking, like correlation ids, supplamental details, client-specific details, or even details that are incredibly common yet not part of signing like from.

I just want to emphasize, if I am reading this change correctly, this code will throw if your transactionRequest contains all of the parameters on the TransactionRequest interface (from). While this specific issue is easily fixed, it is a great example of how sometimes having an extra property on an object beyond what is needed for the function you are calling can be valuable.

[ricmoo]

Any code that used to work was heavily relying on a bug in the library.

The gas was never intended to work (and never has since version 0.0.0); the fact it was passed through the JsonRpcProvider was the bug, as any code that relied on that functionality would fail if used with contracts, or any other Provider. The Contract already throws if that key was provided. And it was ignored in any other Provider.

The documentation has also indicated the correct usage since it was created (probably around version 0.9 or v1.0).

Cross-platform transactions already require tweaking, as there are various BigNumber libraries, ethers supports Uint8Arrays for many properties, and so on. Also, ethers works with Checksum addresses and verifies all address, some platforms require addresses to be in lowercase format (which ethers performs on the to address in JSON-RPC calls). It probably is not safe to be using most transactions across multiple libraries.

Allowing two different keys that do the same thing can also lead to many issues, and more complex code logic, for example if both are specified; if they match, then it is ok to collapse them, if not then what?

If you need to use a transaction across a Web3 and an Ethers system, you can use:

let web3Provider = new providers.Web3Provider(web.currentProvider);
let web3Signer = web3Provider.getSigner(0);

let wallet = new ethers.Wallet(privateKey, ethers.getDefaultProvider());
let tx = {
    to: someAddress,
    gasLimit: 21000
};

// This will automatically update the tx from ethers to Web3 for you
web3Signer.sendTransaction(tx);

wallet.sendTransaction(tx);

Allowing additional properties can also lead to bugs, typos are not caught and in general TypeScript would not work. With a functional language like JavaScript, there are far easier ways to include correlation ID's, by storing them in the dictionary above the transaction object.

There are already quite a few differenced between ethers.js and Web3, many of which are because I am trying to escape legacy decisions made early in the eco-system. For the most part I've made attempts to provide migration easy, but some things are and will be difficult to reconcile.