web3-loader icon indicating copy to clipboard operation
web3-loader copied to clipboard

Published 20 hours ago • verified publisher icon uzyn

Gas limit for deployment

Open graup opened this issue 8 years ago • 8 comments

https://github.com/uzyn/web3-loader/blob/89e98e915948b6e7cf82639a2f070ddcdacfae77/index.js#L109

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.

graup avatar May 30 '16 11:05 graup

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.

uzyn avatar May 30 '16 12:05 uzyn

@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.)

kyroy avatar Jun 01 '16 09:06 kyroy

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?

uzyn avatar Jun 03 '16 08:06 uzyn

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

kyroy avatar Jun 03 '16 08:06 kyroy

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.

uzyn avatar Jul 07 '16 09:07 uzyn

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?

graup avatar Jul 07 '16 22:07 graup

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.

uzyn avatar Jul 08 '16 05:07 uzyn

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

There were, however, some issues with that in testrpc: https://github.com/ethereumjs/testrpc/issues/86 https://github.com/ethereumjs/testrpc/issues/58

I need to test that this actually works.

graup avatar Jul 08 '16 07:07 graup