Byzantium - `eth_estimateGas` call no longer provides correct estimates

[LefterisJP]

System information

Geth version: 1.7.0
OS & Version: Linux

Expected behaviour

Right before byzantium the eth_estimateGas call provided reasonably accurate estimates for transactions with multiple stack frames containing a REVERT. Presumably because that used to be just an invalid operation equaling to a throw.

Actual behaviour

In ropsten, after block 1.7 million, with Byzantium rules activated it no longer provides correct estimates.

Steps to reproduce the behaviour

Try to estimate gas for a transaction with multiple calls, where one of the calls deeper in the stack frame does REVERT and reverts operation to its parent. Do that with pre-byzantium rules and then post-byzantium rules and you will see the difference.

As a live example on ropsten we can observe raiden's channel manager for a given token. Raiden used to set the gas limit of a transactions as GAS_ESTIMATE * 2.

A newChannel() call before the byzantium fork. Raiden provided a gas limit of 2368060 and the gas used by the transaction was 1168023. 1168023 is close to what estimateGas used to return.

Now let's check a post-Byzantium fork newChannel transaction where Raiden still relies on estimateGas. The gas limit provided by Raiden is 58070 which means the estimate was about 29035 for the same call which above took 1168023.

[karalabe]

Hmmm, could you describe the timeline of events (call/reverts) that happen? Internal reverts/throws should not impact gas estimation, only the outermost. AFAIK a revert is a valid termination, so gas will not be increased if your code REVERTs.

[karalabe]

Looking at the opcode trace https://ropsten.etherscan.io/vmtrace?txhash=0xc47a839ac039302a8951d3d84f43b543bb31547bada1ca1ec5ff3c78c2619395, you are going OOG internally, but on the outside you just REVERT.

IMHO revert should never be used for OOG cases, that should be throw if you want to handle it properly. REVERT just says some precondition failed (e.g. gas price is wrong), not that something blew up inside.

[karalabe]

We had quite a lot of internal debates about this exact topic with @Arachnid, whether gas estimation should take into consideration internal failures or not, and the consensus was that the smart contract needs to be smart enough to figure out how it wants to react to internal failures, and if it chooses to ignore internal failures, we should not try to override it from the outside.

TL;DR I'd say there's a bug in your code :P

[LefterisJP]

Looking at the opcode trace https://ropsten.etherscan.io/vmtrace?txhash=0xc47a839ac039302a8951d3d84f43b543bb31547bada1ca1ec5ff3c78c2619395, you are going OOG internally, but on the outside you just REVERT.

It's a kind of a chicken and egg problem. We were using estimate gas in order to give a gas estimate for the entire transaction. So since estimate gas is wrong, giving a small value we gave it to the transaction and then the inner transaction failed due to running out of gas.

Can we check if an internal call does revert?

[LefterisJP]

Note that all require() in solidity are now REVERT in metropolis so this may change how contracts behave with dapps. This is how it happened in our case.

[LefterisJP]

that should be throw if you want to handle it properly.

throw is deprecated in solidity but it also does a REVERT in newer solidity versions:
http://solidity.readthedocs.io/en/develop/control-structures.html?highlight=revert#error-handling-assert-require-revert-and-exceptions

[karalabe]

Hey @LefterisJP, could you try this PR? #15030

[karalabe]

This issue should be fixed with latest code on master. Please check Lefteris and report if you still see issues.

[LefterisJP]

Looking at it right now.

[LefterisJP]

@karalabe With latest master (commit hash: f4c49bc) I believe I can say the problem seems mitigated in byzantium code.

For a newChannel() transaction which cost 1168023 gas, estimateGas() now returns: 1184030 which is a bit higher but still fine and closer to reality than 29035 was with geth 1.7.0

I would say the issue is resolved.

[karalabe]

Note, that a CALL forwards 63/64 gas to the sub contract, so such txs will require more gas than consumed, hence the higher amount. Try sending with 1 gas less than estimated and it should OOG somewhere internally.

…

On Oct 2, 2017 19:23, "Lefteris Karapetsas" ***@***.***> wrote: @karalabe <https://github.com/karalabe> With latest master (commit hash: f4c49bc <f4c49bc>) I believe I can say the problem seems mitigated in byzantium code. For a newChannel() <https://ropsten.etherscan.io/tx/0x7c32ea26cc91666e3e0ded151b25f8c2b1407f9e1bddf987f2af812b1d7617c5> transaction which cost 1168023 gas, estimateGas() now returns: 1184030 which is a bit higher but still fine and closer to reality than 29035 was with geth 1.7.0 I would say the issue is resolved. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15186 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAH6GSv5dW6ruYAKcatY42ycfOi-Ikxpks5soQ3mgaJpZM4PgYo0> .