Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(fw,forks,tests): Add EVM code type marker #610

Merged
merged 26 commits into from
Aug 8, 2024
Merged

Conversation

marioevz
Copy link
Member

@marioevz marioevz commented Jun 7, 2024

🗒️ Description

@pytest.mark.with_all_evm_code_types marker

Adds the with_all_evm_code_types marker that can be added to a test as follows:

@pytest.mark.with_all_evm_code_types
@pytest.mark.valid_from("Frontier")
def test_something_with_all_evm_code_types(evm_code_type: EVMCodeType):
    pass

and the test will be parameterized for parameter evm_code_type only with value [EVMCodeType.LEGACY] starting on fork Frontier, and eventually it will be parametrized with with values [EVMCodeType.LEGACY, EVMCodeType.EOF_V1] on the EOF activation fork.

Adding the evm_code_type parameter to the function signature is optional and can be done only if required.

In all calls to pre.deploy_contract, if the code parameter is Bytecode type, and evm_code_type==EVMCodeType.EOF_V1, the bytecode will be automatically wrapped in an EOF V1 container.

Code wrapping might fail in the following circumstances:

  • The code contains invalid EOF V1 opcodes.
  • The code does not end with a valid EOF V1 terminating opcode (such as Op.STOP or Op.REVERT or Op.RETURN).

In the case where the code wrapping fails, evm_code_type can be added as a parameter to the test and the bytecode can be dynamically modified to be compatible with the EOF V1 container.

@pytest.mark.with_all_call_opcodes marker

This marker is used to automatically parameterize a test with all EVM call opcodes that are valid for the fork being tested.

import pytest

@pytest.mark.with_all_call_opcodes
@pytest.mark.valid_from("Frontier")
def test_something_with_all_call_opcodes(pre: Alloc, call_opcode: Op):
    ...

In this example, the test will be parametrized for parameter call_opcode with values [Op.CALL, Op.CALLCODE] starting on fork Frontier, [Op.CALL, Op.CALLCODE, Op.DELEGATECALL] on fork Homestead, [Op.CALL, Op.CALLCODE, Op.DELEGATECALL, Op.STATICCALL] on fork Byzantium, and eventually it will be parametrized with with values [Op.CALL, Op.CALLCODE, Op.DELEGATECALL, Op.STATICCALL, Op.EXTCALL, Op.EXTSTATICCALL, Op.EXTDELEGATECALL] on the EOF activation fork.

Parameter evm_code_type will also be parametrized with the correct EVM code type for the opcode under test.

Code Generators Switch and Conditional

Code generators Switch and Conditional have been updated to take the evm_code_type and correctly generate EOF V1 code in case of EVMCodeType.EOF_V1.

--evm-code-type parameter for fill

Parameter --evm-code-type to force all tests to be filled with an specific EVM code type and, while most tests would currently fail because of their incompatibility with EOF, it can serve the purpose of exploring which tests could be easily updated for EOF.

call_return_code helper

This function returns the correct return code depending on the give opcode, whether a success is expected or not, and whether a revert is expected or not.

It is useful to automatically set the correct storage expectations depending on the type of call used, since all legacy *CALL opcodes and the EOF EXT*CALL opcodes return different values.

Fork Covariant Processor Changes

Changes were required to src/pytest_plugins/forks/forks.py to allow markers to parametrize two or more parameters at the same time, depending on the fork. It was necessary because the new marker with_all_call_opcodes parametrizes call_opcode and evm_code_type at the same time.

Example tests updated

The following tests were updated as an example of how an updated test would look like:

  • tests/cancun/eip4844_blobs/test_point_evaluation_precompile.py
  • tests/frontier/opcodes/test_dup.py

🔗 Related Issues

None

✅ 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: 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 marioevz mentioned this pull request Jun 17, 2024
7 tasks
@marioevz marioevz force-pushed the evm-code-type-marker branch from 2b9681a to 37a9edc Compare June 18, 2024 00:08
@marioevz marioevz changed the base branch from main to implement-bytecode June 18, 2024 00:09
@marioevz marioevz force-pushed the implement-bytecode branch 2 times, most recently from 5e89735 to 26d4729 Compare June 20, 2024 13:42
@marioevz marioevz force-pushed the evm-code-type-marker branch from 00c68fd to f36db6f Compare July 18, 2024 13:49
@marioevz marioevz force-pushed the evm-code-type-marker branch 2 times, most recently from 7f40a1b to f12d155 Compare July 30, 2024 22:06
@marioevz marioevz changed the base branch from implement-bytecode to main July 30, 2024 22:06
src/pytest_plugins/forks/forks.py Show resolved Hide resolved
src/pytest_plugins/forks/forks.py Show resolved Hide resolved
src/pytest_plugins/forks/forks.py Show resolved Hide resolved
@marioevz marioevz force-pushed the evm-code-type-marker branch from 0aa8389 to a87bb09 Compare July 31, 2024 16:17
@marioevz marioevz force-pushed the evm-code-type-marker branch from a87bb09 to 9ef68a4 Compare August 1, 2024 16:47
@marioevz marioevz marked this pull request as ready for review August 1, 2024 16:47
Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this PR. LGTM!

My only real comment relates to a potential exstension to the with_all_... markers.

Instead of having multiple test markers, i.e:

  • pytest.mark.with_all_evm_code_types
  • pytest.mark.with_all_call_opcodes
  • pytest.mark.with_all_tx_types
  • pytest.mark.with_all_precompiles

Should we instead create a single "with all" marker:

  • pytest.mark.with_all(...)

That accepts a list of potential input/s:

  • pytest.mark.with_all("tx_types", "evm_code_types")

docs/writing_tests/test_markers.md Show resolved Hide resolved
@marioevz
Copy link
Member Author

marioevz commented Aug 4, 2024

Should we instead create a single "with all" marker:

* `pytest.mark.with_all(...)`

That accepts a list of potential input/s:

* `pytest.mark.with_all("tx_types", "evm_code_types")`

This is a really nice idea, although it would require some refactoring to the covariant fork parameter logic, so I think this could be its own PR. Also would need to make some modifications because, as it is right now, with_all_evm_code_types and with_all_call_opcodes are mutually exclusive.

@pdobacz pdobacz mentioned this pull request Aug 6, 2024
8 tasks
Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@marioevz marioevz merged commit 16ea506 into main Aug 8, 2024
7 checks passed
@marioevz marioevz deleted the evm-code-type-marker branch August 8, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants