state: Implement fake BLOCKHASH resolution for testing

[chfast]

If explict block hashes are not provided, return predefined block hashes from Host::get_block_hash() as keccak256(str(block_number)). This is badly documented convention used in state tests.

Note: the readability of block hash values from BLOCKHASH instruction still applies. I.e. you will get 0 if you ask for a block from the future or more than 256 blocks older than the current one.

[codecov]

Codecov Report

Merging #698 (51ccacb) into master (778dfcb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #698   +/-   ##
=======================================
  Coverage   97.56%   97.56%           
=======================================
  Files          92       92           
  Lines        8535     8547   +12     
=======================================
+ Hits         8327     8339   +12     
  Misses        208      208           

Flag	Coverage Δ
blockchaintests	62.61% <ø> (ø)
statetests	73.46% <100.00%> (+0.01%)	⬆️
statetests-silkpre	22.63% <16.66%> (-0.02%)	⬇️
unittests	95.52% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
test/state/host.cpp	93.10% <100.00%> (+0.06%)	⬆️
test/unittests/evm_state_test.cpp	100.00% <100.00%> (ø)
test/unittests/state_transition_block_test.cpp	100.00% <100.00%> (ø)
test/utils/bytecode.hpp	96.00% <100.00%> (+0.03%)	⬆️

[winsvega]

Yes, since we decided that makes no sense. There should be no state tests using blockhash instruction as it is a blockchain logic. Otherwise it just tests a test logic.

[chfast]

Yes, since we decided that makes no sense. There should be no state tests using blockhash instruction as it is a blockchain logic. Otherwise it just tests a test logic.

This is true, however goevmlab may generate a state test that uses BLOCKHASH and expects this answer (although it may be limited to BLOCKHASH(0) only).

[holiman]

I can't meaningfully review this, but it sounds good. A better description is here: NethermindEth/nethermind#4967 (comment)

if we execute on BLOCKNUMBER 1 , in which case 0 is the only one "eligible" for blockhash. If we're on a later number, there are other blocks which are 'eligible'.

goevmlab only generates tests with blocknumber 1, but it is possible for a statetest to specify a later number. So if we execute on block 1000, and BLOCKHASH(1) is requested, then it should, of course, put zero on the stack, and not "look up (== keccak)" that number.

[chfast]

So if we execute on block 1000, and BLOCKHASH(1) is requested, then it should, of course, put zero on the stack, and not "look up (== keccak)" that number.

Yes, this is handled already closer to the instruction implementation (i.e. EVM will not ask anything the Host in this situation).

I noted this is not needed for regular state tests because they have a discipline to avoid BLOCKHASH entirely.

However, for goevmlab this is required. But as mentioned goevmlab generated tests are always running at block 1 therefore the Host will only be asked for BLOCKHASH(0). So my question to @holiman is mostly: should be keep the generic formula for the "fake" block hashes (as currently in this PR) or maybe we should limit ourselves only to

if (block_number == 0)
    return 0x044852b2a670ade5407e78fb2863c51de9fcb96542a07186fe3aeda6bb8a116d_bytes32;

[chfast]

I think this is the geth implementation so I will link to it. https://github.com/ethereum/go-ethereum/blob/v1.12.2/tests/state_test_util.go#L432

What is the preferred naming for this?

• fake
• cheat
• test