evm: Precompile Tests

[scorbajio]

This change increases coverage of evm precompiles by adding tests for the ripemd160 and blake2f precompile contracts as well as updating the ecrecover precompile tests.

[codecov]

Codecov Report

Merging #3189 (6f58777) into master (98fc3ab) will increase coverage by 0.37%.
The diff coverage is n/a.

Additional details and impacted files

Flag	Coverage Δ
block	87.52% <ø> (ø)
blockchain	91.60% <ø> (ø)
client	84.58% <ø> (+0.03%)	⬆️
common	98.25% <ø> (ø)
devp2p	82.11% <ø> (ø)
ethash	∅ <ø> (∅)
evm	76.64% <ø> (+2.75%)	⬆️
statemanager	85.72% <ø> (ø)
trie	89.26% <ø> (-0.50%)	⬇️
tx	95.73% <ø> (ø)
util	89.05% <ø> (ø)
vm	80.83% <ø> (ø)

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

[holgerd77]

Some general feedback here: yeah, I do think that it is really good if we have at least 1-2 simple test cases for each precompile in addition to the cross-client testing, also agree with @acolytec3 though that these particular precompiles have not such a high relevance (though they should nevertheless still not break consensus for sure), so we shouldn't take this too far.

Changing error messages in the EVM is always a breaking change and I think it would not really be worth it here to adopt. So as a practical measure I would suggest that we delete the error message testing from the test cases and just concentrate on value testing here and simply see and take this as a small useful addition to the test setup (without discovering anything new).

[jochem-brouwer]

I agree changing the error messages is not worth it. I would change the tests to check that there is an error and fail if it does not error out. For reference, in ethereum/tests (so our BlockchainTests and StateTests in VM) there is also an "expected error" string field, but we do not check this. It is also not worth it, since these errors are likely not checked among other clients as well: if there are 2 checks to be done (for instance, input length should be X, and gas should be higher than Y) then the order of these checks will thus change what error gets generated (if both checks would fail), but the end result should be the same: in most cases, all gas forwarded is used.

[acolytec3]

LGTM