Gas limit for deployment

[graup]

web3-loader/index.js

Line 109 in 89e98e9

gas: config.gasLimit,

Hi, I am wondering if we could use estimateGas instead of this. We are trying to use it for deployment on a real (test) net and sometimes run into gas problems. I don't know if this is the problem, but it could be connected. I think the official Ethereum wallet deploys contracts with estimateGas() + 20000.

[uzyn]

What is the error that you got?

If I'm not mistaken, gasLimit >= estimateGas().

But yeah, I think set default to estimateGas() + 20000 is better than setting the default at block max.

[kyroy]

@graup said, we had some deployment problems, but they now randomly disappeared.

Module build failed: Error: The contract code couldn't be stored, please check your gas amount.

(This message doesn't mean that the problem is the gas amount.)

[uzyn]

Have you managed to identify this issue?

Could it be because the primary account does not have enough eth to cover the gas amount that's specified, especially since the original code is using max.

@graup it makes sense to set default gas to estimateGas() instead of max. You want to submit a PR?

[kyroy]

Here is our main account on the testnet
http://testnet.etherscan.io/address/0x78ab9626dff0dbd0b4a14eddad8f24d53c801a8a

[uzyn]

Is this issue still relevant?

If you have managed to solve it, please do submit a PR or share on how it's solved.

Thanks.

[graup]

For our tests, we solved it increasing the gas limit. Testrpc now offers an option for that. The raised limit is recognised by the loader.

At this point I am not sure if we should still change the implementation to estimateGas. Do you see any advantages?

[uzyn]

I think it's still beneficial to update estimateGas to be based on actual estimated gas than protocol's max gas limit for a few reasons:

1. testrpc or other simulator's broken gas limit.
2. The limit might be too high on actual live network that the deploying account may not have enough ETH for.

[graup]

I agree. I will look into implementing this change then.

There were, however, some issues with that in testrpc:
trufflesuite/ganache-cli-archive#86
trufflesuite/ganache-cli-archive#58

I need to test that this actually works.