Compile time evaluation of constant hashes

[benjaminion]

There's room for improvement in optimising compile-time evaluation of constant hashes when optimisation is enabled.

contract Hash{

    bytes32 constant HASH1 = keccak256('');
    bytes32 constant HASH2 = keccak256('12345678901234567890123456789012');
    bytes32 constant HASH3 = keccak256(uint256(0));
    
    function foo1() public pure returns (bytes32) {
        return HASH1;
    }

    function foo2() public pure returns (bytes32) {
        return HASH2;
    }

    function foo3() public pure returns (bytes32) {
        return HASH3;
    }
}

With optimisation off, none of the constant hashes is evaluated at compile-time. In each case the keccak256 operation is being done at run time. This is somewhat expected.

With optimisation enabled, only the first (keccak256('')) is evaluated at compile time. The remainder (keccak256('12345678901234567890123456789012'), keccak256(uint256(0))) are evaluated at run time, as if optimisation were off. This is unexpected. Expected behaviour would be to evaluate all constant hashes at compile time.

Test environment: Remix, Solidity version 0.4.23+commit.124ca40d
Conclusions reached by inspecting the generated byte code. It's quite obvious if the hash is present or not.

[chriseth]

This has to do with the trade-off between code deployment costs and runtime costs. There is a setting in the optimizer called "runs" where you can adjust it.

[chriseth]

Note that storing a single byte in code costs at least 200+68 gas.

[benjaminion]

I understand this, which is why the second example (keccak256('12345678901234567890123456789012')) is constructed so that it is always cheaper to embed the hash in the deployment code, irrespective of the runs parameter.

This also does not explain why keccak256('') is compile-time executed when optimised, but the others are not. Your explanation would suggest the opposite should be true.

[chriseth]

OK, perhaps this is not at all related to the compiler finding the most efficient way, but rather to the fact that the value of keccak256('') does not depend on the state of memory when calculating the hash.

[chriseth]

A place to start would be to check if the check just after // If all arguments are known constants, compute the Keccak-256 here in KnownState.cpp turns out to be true for non-empty keccak256 calls.

[benjaminion]

Considering the two test cases, keccak256('12345678901234567890123456789012') and keccak256(uint256(0)).

A place to start would be to check if the check just after // If all arguments are known constants, compute the Keccak-256 here in KnownState.cpp turns out to be true for non-empty keccak256 calls.

In both these cases, execution does not reach that line (although it does and is correctly handled for the empty hash).

In both of the test cases, applyKeccak256() is called three times (which is a surprise - it is called only once for the empty hash).

Each time, applyKeccak256() exits at the check if (!l || *l > 128) since !l is true at this point. That is, m_expressionClasses->knownConstant(_length) is (incorrectly?) returning a false value each time for these hash operands.

This is as far as my ability to debug C++ can take me...

Setup:

> solc/solc --version
solc, the solidity compiler commandline interface
Version: 0.4.24-develop.2018.5.1+commit.5cce2e55.mod.Linux.g++

> solc/solc --optimize const-keccak.sol

[chriseth]

OK, I will take a look later this week!

[axic]

@chriseth have you checked this by any chance?

[chriseth]

No, sorry.

[nventuro]

Are there plans to improve this in the near future? From @chriseth's description above it sounds like the tools to achieve this are already present, just not being used in all scenarios.

I find myself recommending people to use hard-coded bytes32 constants to save gas, instead of having the keccak256 call in the source code. This is far from ideal: computing said hash outside of Solidity is annoying, and having a 66-character constant is error-prone, specially in situations where a value must be the result of a hash for security reasons. This also comes in while auditing, since readers must verify manually that the constant matches the expected value.

[chriseth]

The problem is that 1) it is dangerous to re-implement this outside of the EVM and 2) it is not clear how we should handle memory objects at compile-time.

Concerning hashes: The optimizer should actually remove the keccak256 opcode. It might not do it because it is cheaper depending on the "runs" setting (a 32 byte constant is rather expensive to deploy).

[axic]

Could we make one exception and evaluate keccak256(string)? That is used in quite a few contracts.

[chriseth]

This could be a good exception. I would propose to put the value in the annotation object of the constant as opposed to fully supporting this in the type system as we do for constants.

[dmihal]

Do creationCode hashes get compile-time-hashed as well? Such as this:

bytes32 public constant CONTRACT_BYTECODE_HASH = keccak256(type(MyContract).creationCode);

These hashes are useful for optimizing Create2 calculation

[chriseth]

I don't think this is optimized. There is a proposal to add a function to type(MyContract) instead to directly retrieve the create2 address given a salt.

[axic]

There is a proposal to add a function to type(MyContract) instead to directly retrieve the create2 address given a salt.

It is covered in #8798.