Skip to content

feat(pytest,tests): tests added by fork introduced #423

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/writing_tests/adding_a_new_test.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ All test cases are located underneath the `tests` directory, which are then orga
| | ├── 📄 test_excess_blob_gas.py
| | └── 📄 ...
| ├── 📁 shanghai
| | ├── 📄 __init__.py
| | ├── 📁 eip3651_warm_coinbase
| | | ├── 📄 __init__.py
| | | └── 📄 test_warm_coinbase.py
Expand All @@ -22,13 +23,25 @@ All test cases are located underneath the `tests` directory, which are then orga
| | | └── 📄 test_push0.py
| | ├── 📁...
| | ...
| ├── 📁 frontier
| | ├── 📄 __init__.py
| | ├── 📁 opcodes
| | | ├── 📄 __init__.py
| | | └── 📄 test_dup.py
| | | └── 📄 test_call.py
| | | └── 📄 ...
| | ├── 📁...
│ └── 📁 ...
```

Each category/sub-directory may have multiple Python test modules (`*.py`) which in turn may contain many test functions. The test functions themselves are always parametrized by fork (by the framework).
Each fork/sub-directory may have multiple Python test modules (`*.py`) which in turn may contain many test functions. The test functions themselves are always parametrized by fork (by the framework). In general, sub-directories can be feature categories such as opcodes or eips.

A new test can be added by either:

- Adding a new `test_` python function to an existing file in any of the existing category subdirectories within `tests`.
- Creating a new source file in an existing category, and populating it with the new test function(s).
- Creating an entirely new category by adding a subdirectory in `tests` with the appropriate source files and test functions.

!!! note "Which fork does my test belong?"
Tests should be added to the fork folder where the feature or EIP was introduced. For example, if the callcode opcode is being tested it should be added to the
Frontier fork folder as this opcode was first introduced in Frontier. This remains true for cases where the test is [valid from](../writing_a_new_test/#specifying-which-forks-tests-are-valid-for) a later fork, such as a callcode specific bug introduced in London.
18 changes: 17 additions & 1 deletion src/pytest_plugins/forks/forks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@
import pytest
from pytest import Metafunc

from ethereum_test_forks import (
from ethereum_test_forks import ( # noqa: F401
Berlin,
Byzantium,
Cancun,
Constantinople,
Comment on lines +13 to +17
Copy link
Contributor Author

@spencer-tb spencer-tb Feb 1, 2024

Choose a reason for hiding this comment

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

I would like to find a better way to this, but its needed forglobals()[fork]

Fork,
ForkAttribute,
Frontier,
Homestead,
Istanbul,
London,
Paris,
Shanghai,
forks_from_until,
get_deployed_forks,
get_forks,
Expand Down Expand Up @@ -345,6 +355,12 @@ def get_validity_marker_args(
f"'{validity_marker_name}'. "
f"List of valid forks: {', '.join(metafunc.config.fork_names)}" # type: ignore
)
fork_folder = metafunc.module.__name__.split(".")[1].capitalize()
if globals()[fork_name] < globals()[fork_folder]:
pytest.fail(
f"Incorrect usage of '{validity_marker_name}'. The validity marker fork "
f"'{fork_name}' must be greater than or equal to the test folder fork '{fork_folder}'."
)
Comment on lines +358 to +363
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Validity marker check. Note this required us to move some existing tests or change the fork name.


return fork_name

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def env(): # noqa: D103
)


@pytest.mark.valid_from("Paris")
@pytest.mark.valid_from("Cancun")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note this is still in Draft. I changed these to Cancun for now such that these tests pass the validity marker check.

What do we do in cases like this? To me if a test is valid before the fork where the eip or feature is introduced, it should not exist within that fork folder.

Are they specific to eip6780 or selfdestruct? If its the latter, I think we should move them to Homestead (where selfdestruct was introduced, well renamed from SUICIDE -> SELFDESTRUCT). Then mark them valid from the forks as is currently without the change.

@pytest.mark.parametrize(
"create2_dest_already_in_state",
(True, False),
Expand Down Expand Up @@ -230,7 +230,7 @@ def test_dynamic_create2_selfdestruct_collision(
state_test(env=env, pre=pre, post=post, tx=tx)


@pytest.mark.valid_from("Paris")
@pytest.mark.valid_from("Cancun")
@pytest.mark.parametrize(
"selfdestruct_on_first_tx,recreate_on_first_tx",
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def env(): # noqa: D103
)


@pytest.mark.valid_from("Paris")
@pytest.mark.valid_from("Cancun")
@pytest.mark.parametrize("first_suicide", [Op.CALL, Op.CALLCODE, Op.DELEGATECALL])
@pytest.mark.parametrize("second_suicide", [Op.CALL, Op.CALLCODE, Op.DELEGATECALL])
def test_reentrancy_selfdestruct_revert(
Expand Down
16 changes: 8 additions & 8 deletions tests/cancun/eip6780_selfdestruct/test_selfdestruct.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def pre(
],
)
@pytest.mark.parametrize("selfdestruct_contract_initial_balance", [0, 100_000])
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_create_selfdestruct_same_tx(
state_test: StateTestFiller,
env: Environment,
Expand Down Expand Up @@ -418,7 +418,7 @@ def test_create_selfdestruct_same_tx(
@pytest.mark.parametrize("call_times", [0, 1])
@pytest.mark.parametrize("selfdestruct_contract_initial_balance", [0, 100_000])
@pytest.mark.parametrize("self_destructing_initcode", [True], ids=[""])
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_self_destructing_initcode(
state_test: StateTestFiller,
env: Environment,
Expand Down Expand Up @@ -546,7 +546,7 @@ def test_self_destructing_initcode(
@pytest.mark.parametrize("selfdestruct_contract_initial_balance", [0, 100_000])
@pytest.mark.parametrize("selfdestruct_contract_address", [compute_create_address(TestAddress, 0)])
@pytest.mark.parametrize("self_destructing_initcode", [True], ids=[""])
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_self_destructing_initcode_create_tx(
state_test: StateTestFiller,
env: Environment,
Expand Down Expand Up @@ -614,7 +614,7 @@ def test_self_destructing_initcode_create_tx(
@pytest.mark.parametrize("selfdestruct_contract_initial_balance", [0, 100_000])
@pytest.mark.parametrize("recreate_times", [1])
@pytest.mark.parametrize("call_times", [1])
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_recreate_self_destructed_contract_different_txs(
blockchain_test: BlockchainTestFiller,
env: Environment,
Expand Down Expand Up @@ -753,7 +753,7 @@ def test_recreate_self_destructed_contract_different_txs(
@pytest.mark.parametrize(
"selfdestruct_contract_address", [PRE_EXISTING_SELFDESTRUCT_ADDRESS], ids=["pre_existing"]
)
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_selfdestruct_pre_existing(
state_test: StateTestFiller,
eip_enabled: bool,
Expand Down Expand Up @@ -880,7 +880,7 @@ def test_selfdestruct_pre_existing(
"selfdestruct_contract_address,entry_code_address",
[(compute_create_address(TestAddress, 0), compute_create_address(TestAddress, 1))],
)
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_selfdestruct_created_same_block_different_tx(
blockchain_test: BlockchainTestFiller,
eip_enabled: bool,
Expand Down Expand Up @@ -1018,7 +1018,7 @@ def test_selfdestruct_created_same_block_different_tx(
@pytest.mark.parametrize("call_times", [1])
@pytest.mark.parametrize("selfdestruct_contract_initial_balance", [0, 1])
@pytest.mark.parametrize("create_opcode", [Op.CREATE])
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_delegatecall_from_new_contract_to_pre_existing_contract(
state_test: StateTestFiller,
env: Environment,
Expand Down Expand Up @@ -1154,7 +1154,7 @@ def test_delegatecall_from_new_contract_to_pre_existing_contract(
@pytest.mark.parametrize("call_opcode", [Op.DELEGATECALL, Op.CALLCODE])
@pytest.mark.parametrize("call_times", [1])
@pytest.mark.parametrize("selfdestruct_contract_initial_balance", [0, 1])
@pytest.mark.valid_from("Shanghai")
@pytest.mark.valid_from("Cancun")
def test_delegatecall_from_pre_existing_contract_to_new_contract(
state_test: StateTestFiller,
eip_enabled: bool,
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
)


@pytest.mark.valid_from("Homestead")
@pytest.mark.valid_from("Frontier")
def test_yul(state_test: StateTestFiller, yul: YulCompiler, fork: Fork):
"""
Test YUL compiled bytecode.
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

@pytest.mark.compile_yul_with("Paris") # Shanghai refuses to compile SELFDESTRUCT
@pytest.mark.valid_from("Constantinople")
def test_tx_selfdestruct_balance_bug(blockchain_test: BlockchainTestFiller, yul: YulCompiler):
def test_selfdestruct_contract_balance(blockchain_test: BlockchainTestFiller, yul: YulCompiler):
"""
Test that the vulnerability is not present by checking the balance of the
`0xaa` contract after executing specific transactions:
Expand Down
2 changes: 1 addition & 1 deletion tests/shanghai/eip3651_warm_coinbase/test_warm_coinbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def test_warm_coinbase_call_out_of_gas(
]


@pytest.mark.valid_from("Paris") # these tests fill for fork >= Berlin
@pytest.mark.valid_from("Shanghai")
@pytest.mark.parametrize(
"opcode,code_gas_measure",
gas_measured_opcodes,
Expand Down