(feat) Additional precompile tests for blake2b, identity #1244
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reedsa
force-pushed
the
eip152-blake2-delegatecall
branch
2 times, most recently
from
19ae03c to
b10bd8f
Compare
reedsa
commented
Feb 21, 2025
There was a problem hiding this comment.
@marioevz @winsvega I'm working on several more precompile tests and double checking that all cases are covered.
- Blake2b should be covered with these additions, but one case is causing timeouts (see separate comment below).
- This PR includes a couple Identity test cases. There are numerous others for CALL and CALLCODE that I will add here.
I want to clarify the other test cases since you mentioned skipping a few from the stPreCompiledContracts2 directory.
- Am I remembering that you said to ignore the ecrecover, ripemd and sha256 tests? I dont know if there might be others, but wanted to check back before I proceed further.
- I noticed a single test case in /~https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stPreCompiledContracts/sec80Filler.json that appears to be related to the ecrecover precompile (I found it runs
CALLCODEto address 0x1). Should this one be ignored? - The precompile EIP2929 tests should probably be converted. I wanted to check before implementing those, they appear to check gas consumption of certain opcodes. /~https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stPreCompiledContracts/precompsEIP2929CancunFiller.yml
| ), | ||
| # pytest.param( | ||
| # Blake2bInput( | ||
| # rounds=8000000, |
There was a problem hiding this comment.
This huge number of rounds unsurprisingly causes time outs. Is there another way to run this to keep this case covered?
| @@ -0,0 +1,57 @@ | |||
| """abstract: Test delegatecall to Blake2B Precompile before and after it was added.""" | |||
There was a problem hiding this comment.
| state_test(env=env, pre=pre, post=post, tx=tx) | ||
|
|
||
|
|
||
| @pytest.mark.valid_from("Istanbul") |
There was a problem hiding this comment.
Additional tests that follow cover cases found in /~https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stPreCompiledContracts2/CALLBlake2fFiller.json
| ["args_size", "output_size", "returndatasize"], | ||
| [ | ||
| pytest.param(16, 32, 16, id="output_16"), | ||
| pytest.param(32, 16, 32, id="output_32"), |
There was a problem hiding this comment.
| @pytest.mark.parametrize( | ||
| ["args_size", "output_size", "returndatasize"], | ||
| [ | ||
| pytest.param(16, 32, 16, id="output_16"), |
There was a problem hiding this comment.
This was referenced Feb 25, 2025
reedsa
force-pushed
the
eip152-blake2-delegatecall
branch
from
c55d8a3 to
5c29b09
Compare
reedsa
force-pushed
the
eip152-blake2-delegatecall
branch
from
5c29b09 to
7882fc1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🗒️ Description
Convert remaining Blake2 precompile test cases for call and delegatecall
Convert identity precompile tests
🔗 Related Issues
✅ Checklist
mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.