feat(tests): EIP-152 precompiled contracts with Blake2

[reedsa]

🗒️ Description

Add tests for EIP-152: BLAKE2 compression function F precompile.

Test cases pulled from https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stPreCompiledContracts/blake2BFiller.yml

🔗 Related Issues

ethereum/tests#1431

✅ Checklist

• All: Set appropriate labels for the changes.
• All: Considered squashing commits to improve commit history.
• All: Added an entry to CHANGELOG.md.
• All: Considered updating the online docs in the ./docs/ directory.
• Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
• Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
• Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
• Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

[reedsa]

I wasn't familiar with the opcodes needed to successfully write the pre state contract, so I ended up copying over the yul from the original test. I may not have it all correct since the output state indicates there is no data returned in each case. I have the test cases written out so it'd be great if I could get some eyes on the potential issues with my implementation. Thanks!

[winsvega]

rewrite the yul contract using Op

Also since you start blake test, here are the existing blake tests that need to be converted
https://github.com/search?q=repo%3Aethereum%2Ftests%20blake&type=code

[danceratopz]

Nice! Looking good so far, leaving the pre-state contract as Yul is a good first approach to getting the tests working up front.

After all the tests fill successfully with this Yul contract, we could consider splitting it up and deploying test case-specific contracts matching the pytest parameterization.

Below are some cosmetic suggestions. Happy to help debug the fails, just let me know. But a starting point could be to fill using geth and enable traces (these are currently unavailable with EELS, but coming soon). Something like:

uv run pytest ./tests/istanbul/eip152_blake2/test_blake2.py -k "not case8" --evm-bin=~/bin/evm --traces -x

[reedsa]

rewrite the yul contract using Op

Also since you start blake test, here are the existing blake tests that need to be converted https://github.com/search?q=repo%3Aethereum%2Ftests%20blake&type=code

Should I keep all of these tests together in the same file but as separate tests? I could also keep them as separate files entirely, but maybe it's better to keep them all in one spot?

[reedsa]

@danceratopz @winsvega Just pushed up the latest revisions. As you can see there are test failures but I am not seeing anything from the trace output that would indicate an issue. It seems the inputs are not producing the same outputs as in the original ethereum/tests cases.

From what I can tell, the t_0 value should be set to the offset of the message input. In the cases where there are 16 bytes of data, this should be 0x10. The cases which set the message to 16 bytes in length (https://github.com/ethereum/tests/blob/ae4791077e8fcf716136e70fe8392f1a1f1495fb/src/GeneralStateTestsFiller/stPreCompiledContracts/blake2BFiller.yml#L145) do not seem to update this t_0 value.

For example, in case 9, the inputs have a t_0 value set as 0x05. For these tests using 16 bytes of data, shouldn't the t_0 value be changed to 0x10? I'm not certain if the original test is sending in the wrong offset value but based on the spec and what was done for case 9 that would make the most sense. Case 9 was created based on this thread: ethereum/tests#948 (comment)

It's possible the output values should be changed based on this logic. I'm not sure what that means for the original test cases or the clients which rely on those. Let me know if my logic is sound, or if there's still a problem with the test cases I've created here.

[marioevz]

A few comments but it looks pretty good overall, thanks for implementing this!

[reedsa]

@marioevz @winsvega I've updated the code based on your feedback, let me know if there's anything else. All cases are consistent with the cases that are in the original precompiled tests.

It's hard to tell if any edge cases may be missing. A code search shows other json files but I think most of those cases are already covered by the new tests. I believe the json fixture https://github.com/ethereum/tests/blob/c30599fb23d0470301f68bc69188ef444dcbc861/src/GeneralStateTestsFiller/stPreCompiledContracts2/CALLBlake2fFiller.json#L2 is already covered in this PR.

There are several files such as https://github.com/ethereum/tests/blob/c30599fb23d0470301f68bc69188ef444dcbc861/GeneralStateTests/stPreCompiledContracts2/CALLCODEBlake2f.json#L2 which are checking hash, logs and txbytes. How are these generated? Should those json fixtures be captured as a separate test module since they use a different opcode CALLCODE?

Thanks for checking these and helping me understand if I missed anything!

[winsvega]

@marioevz @winsvega I've updated the code based on your feedback, let me know if there's anything else. All cases are consistent with the cases that are in the original precompiled tests.

It's hard to tell if any edge cases may be missing. A code search shows other json files but I think most of those cases are already covered by the new tests. I believe the json fixture https://github.com/ethereum/tests/blob/c30599fb23d0470301f68bc69188ef444dcbc861/src/GeneralStateTestsFiller/stPreCompiledContracts2/CALLBlake2fFiller.json#L2 is already covered in this PR.

There are several files such as https://github.com/ethereum/tests/blob/c30599fb23d0470301f68bc69188ef444dcbc861/GeneralStateTests/stPreCompiledContracts2/CALLCODEBlake2f.json#L2 which are checking hash, logs and txbytes. How are these generated? Should those json fixtures be captured as a separate test module since they use a different opcode CALLCODE?

Thanks for checking these and helping me understand if I missed anything!

hash logs and txbytes generated by the framework, you don't need to worry about it.
the copy of test you provided is just the parametrization of same test but using a different callcode to the blake precompile

[marioevz]

Looks great, just the comments about the storage initial state that Dimitry suggested.

[marioevz]

LGTM, thanks for the changes!

I went over the coverage report and it seems related to the different approach we use in the python tests where we generate a different contract per test case instead of one big contract and handle the cases via jumps, and the lost coverage comes from the jump instructions not being covered anymore, which is completely reasonable to me since we are not looking to test this area with these cases.