added test for PUSH opcodes

[felix314159]

🗒️ Description

hi, this is my first PR for this repo. please let me know if the tests are faulty or not useful.

i still have a few questions:

1. i ran fill --fork=Cancun ./tests/frontier/opcodes/test_push.py to get 99 passed. However, it also says 'No tests executed - the test fixtures in "fixtures/" may now be executed against a client'. How can I do this?
2. i wanted to include 'negative tests' (e.g. PUSH4 value that requires more than 4 bytes) to see that it correctly fails, but I didn't know how to signal this in this Python script. I would also then have to somehow programmatically specify which failure reason is the 'correct' one. LMK if this is possible

🔗 Related Issues

Fixes #972

✅ 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.

[marioevz]

1. i ran `fill --fork=Cancun ./tests/frontier/opcodes/test_push.py` to get 99 passed. However, it also says 'No tests executed - the test fixtures in "fixtures/" may now be executed against a client'. How can I do this?

The easiest way to do this would be to compile geth's evm tool, which can be done by cloning their repo, installing golang, then doing go build -o ./build/bin/evm ./cmd/evm/ to build the command, and then add the generated binary (./build/bin/evm) to the PATH.

Then you can use the consume command to run the fixtures you generated:

uv run consume direct --input ./fixtures/

2. i wanted to include 'negative tests' (e.g. PUSH4 value that requires more than 4 bytes) to see that it correctly fails, but I didn't know how to signal this in this Python script. I would also then have to somehow programmatically specify which failure reason is the 'correct' one. LMK if this is possible

Yes I think it's always interesting to add negative tests cases, but let me explain a bit on how these data-portion opcodes such as PUSH4 work in the EVM:

The way these opcodes work is that the EVM interpreter already knows how many bytes it needs to read after PUSH4 to interpret its data, so when it encounters the PUSH4 opcode it figures "the next four bytes are not other opcodes but data instead". So if you add a fifth byte after PUSH4 it would be interpreted as the next opcode instead of an oversized data.

However, the interesting thing would be to crop the bytecode of the smart contract so there's not enough bytes to complete the 4 bytes required by PUSH4 😉

[felix314159]

Thanks for your reply! I tried running uv run consume direct --input ./fixtures/ but I get this error:

I was able to work around this by running hive with hive --dev --client go-ethereum , setting the HIVE_SIMULATOR variable and then running uv run consume engine --input ./fixtures/, it now shows that 49 passed. My question here is why only 49 passed when it previously (when generating the fixtures) said that 99 passed. Are some of these tests not consumable by geth?

[marioevz]

Thanks for your reply! I tried running uv run consume direct --input ./fixtures/ but I get this error

My bad! Reason is that the --evm-bin flag is missing, could you try adding --evm-bin /path/to/the/binary/evm with the correct path and see if that works.

I was able to work around this by running hive with hive --dev --client go-ethereum

Amazing! hive is one step further 😉

it now shows that 49 passed. My question here is why only 49 passed when it previously (when generating the fixtures) said that 99 passed. Are some of these tests not consumable by geth?

Reason being that fill generates multiple kinds of tests to be run in different environments.

Hive consumes two types of tests:

• Blockchain RLP, which is the blocks being fed to the client as serialized blocks, which is a similar format to what the client receives from other clients via P2P messages (when syncing for example).
• Blockchain Engine, which is a JSON format encoded block that the client receives from the consensus client, and this typically happens when the client is already in sync with its peers and a new block is proposed by a validator as the new head of the chain. More info on this JSON format here https://github.com/ethereum/execution-apis/tree/main/src/engine

So when doing uv run consume engine ... only half of the produced fixtures are processed, while the others should be processed with uv run consume rlp ....

[felix314159]

Sadly, I still can't get the --evm-bin flag to work. Seems to be an issue with a plugin:

The uv run consume rlp ... worked well, I now ran the other tests successfully. 👍

[danceratopz]

Sadly, I still can't get the --evm-bin flag to work. Seems to be an issue with a plugin:

Hi @felix314159, could you try the following (note the addition of = between the flag and the arguments) and let me know if it works?

uv run consume direct --input ./fixtures/ --evm-bin=$HOME/Documents/go-ethereum/build/bin/evm

The uv run consume rlp ... worked well, I now ran the other tests successfully. 👍

Nice!

[felix314159]

Hi @felix314159, could you try the following (note the addition of = between the flag and the arguments) and let me know if it works?

uv run consume direct --input ./fixtures/ --evm-bin=$HOME/Documents/go-ethereum/build/bin/evm

Thanks, this works! What also works is uv run consume direct --input=./fixtures/ --evm-bin=$HOME/Documents/go-ethereum/build/bin/evm, so it's probably best for me to make a habit of consistently using '=' as separator 😊

[raxhvl]

FYI - #975 adds the same test.

[felix314159]

FYI - #975 add the same test.

Good to know, but I would not call it the same test as you do not seem to test PUSH0.

[raxhvl]

Good to know, but I would not call it the same test as you do not seem to test PUSH0.

Thats correct. The test for PUSH0 has already been added.