Solidity: failed calls due to address.call() gas computation

Created on 1 Oct 2017  路  10Comments  路  Source: ethereum/solidity

Solidity version

Observed on 4.17+commit.bdeb9e52.Emscripten.clang. The current nightly build (0.4.18-nightly.2017.9.29+commit.b9218468.Emscripten.clang) is also affected.

Issue description

We came up with a minimal example demonstrating the issue and deployed it on the mainnet. The example consists of two contracts:

contract Callee {
    event ReceivedCall();

    function () {
        ReceivedCall();
    }
}
contract Caller {
    function callAddress(address a) {
        a.call();
    }
}

The issue occurs in transactions that invoke Caller.callAddress(<address of Callee>):
Everything works when the transaction is run with between 24402 and 48574 gas.
The internal transaction/call fails when it is run with between 48575 and 49388 gas. When the transaction is run with at least 49389 gas, the internal call succeeds again.

This behaviour is highly counterintuitive: If the transaction succeeds with x gas, it should also succeed with >x gas.

Cause

The cause of this behaviour is the code that solc generates for a.call():
For the EVM's CALL opcode, the top of the stack specifies the maximum amount of gas that should be available in the new callframe. The generated binary computes this amount by doing

PUSH2 0x646e
GAS
SUB

, i.e. as <amount of available gas> - 25710.

This code in ExpressionCompiler.cpp seems to be responsible for generating the above code.

In the above example, the call in the transaction with 48574 gas succeeds because the amount of available gas before the call is 25709; the subtraction hence underflows and provides an upper limit of 2**256-1 gas to the call.

When the amount of gas for the transaction lies between 48575 and 49388, the internal call runs out of gas because the subtraction no longer underflows and the maximum amount of gas hence lies between 0 and 813, while the call requires at least 814 gas to succeed.

Once the amount of gas for the transaction is at least 49389, the internal call succeeds again because now a sufficient amount of gas is supplied for the call even after subtracting 25710.

Suggested fixes

Most importantly, the behavior of address.call() is undocumented. We feel that users should be made aware of this behavior.

It is not clear to us what the benefit of retaining 25710 gas in the calling context is. The comment in ExpressionCompiler.cpp

send all gas except the amount needed to execute "SUB" and "CALL"

leads us to believe that solc wants to make sure that the maximum amount of gas supplied to the call isn't higher than the amount of gas available in the calling context. However, since the EVM will happily truncate the amount of gas down to what is actually available, this seems unnecessary.

Finally, if retaining the gas for the calling context is actually desirable, then the underflow issue should be fixed.

Attribution

This issue was jointly discovered by

  • Lorenz Breidenbach
  • Phil Daian
  • Florian Tramer

Most helpful comment

Reducing the code path to GAS is exactly what Viper does; I'm definitely in favor of Solidity doing the same.

All 10 comments

Thanks for the detailed investigation!

That particular piece of code originates way before the Tangerine Whistle hardfork which introduced the new rules for gas calculation in messages. Before, one had to make sure that enough gas is left to cover the possible account creation and value transfer fees, after subtracting the message gas. After the fork, due to truncation rules, this is not needed anymore.

See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md where compustate.gas is the actual gas left, gas is the argument supplied, submsg_gas is the final limit passed to the call and extra_gas is the additional required gas.

The 25710 comprises of 25000 for possible account creation, 700 for the call and 10 for the cost of PUSH, GAS, SUB.

If we consider the last Solidity version to only output Tangerine Whistle compatible code, we could reduce that code path to a single instruction of GAS. Otherwise we'd need to introduce a setting to chose the EVM version, such as #1117.

Reducing the code path to GAS is exactly what Viper does; I'm definitely in favor of Solidity doing the same.

I agree, we can assume tangerine whistle, the semantics are much more friendly at least. Also note that there might be some built-in lll functions that have the same behaviour. Finally, we might completely remove .call in 0.5.0.

Actually we've realised in a call with @chriseth that shouldn't we just use a large number, such as -1 (all bits set) instead of gas as that will ultimately result in the same effect?

It can also be optimised as PUSH 0 NOT and as a benefit more optimisations can happen around that piece of code where the usage of the gas opcode would prohibit it.

@vbuterin any thoughts?

Great bug research,
That answer my question:
https://ethereum.stackexchange.com/questions/28840/why-address-call-function-saves-unnecessary-gas-for-after-the-internal-execu

Is there any workaround for the time being? Maybe an older compiler version that works?

@bejavu use pragma experimental "0.5.0"; in Solidity 0.4.18.

MultiSigWallet.sol:3:1: SyntaxError: Unsupported experimental feature name. pragma experimental "0.5.0";

nvm, its supposed to be "v0.5.0"

Ah yes, sorry!

Great it worked!
https://ropsten.etherscan.io/tx/0xf096c54824c852de722361c8af9464b9ecca6ce0ad7aa8fc1b82e7c52370b104

one small problem:
I couldn't verify the contract in etherscan.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chriseth picture chriseth  路  3Comments

bshastry picture bshastry  路  3Comments

madvas picture madvas  路  3Comments

leviadam picture leviadam  路  4Comments

area picture area  路  3Comments