-
Notifications
You must be signed in to change notification settings - Fork 454
fix(traces): tracing consistency between input and output alloc formatting #2832
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
Open
felix314159
wants to merge
2
commits into
ethereum:forks/amsterdam
Choose a base branch
from
felix314159:forks/amsterdam
base: forks/amsterdam
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
139 changes: 139 additions & 0 deletions
139
...c/execution_testing/cli/pytest_commands/plugins/filler/tests/test_alloc_dump_canonical.py
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| """ | ||
| Test that ``input/alloc.json`` written under ``--evm-dump-dir`` uses the | ||
| same canonical JSON format as the t8n-produced ``output/alloc.json``. | ||
|
|
||
| When both files share the same conventions (32-byte-padded storage keys, | ||
| minimal-hex nonces, and omission of empty/default fields), a textual | ||
| diff between them surfaces only real state transitions instead of | ||
| formatting noise. Any account whose semantic state is unchanged between | ||
| pre and post must therefore serialize byte-identically in both files. | ||
| """ | ||
|
|
||
| import json | ||
| import textwrap | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
|
|
||
| CONTRACT_ADDR = "0x000000000000000000000000000000000000c0de" | ||
|
|
||
| TEST_MODULE = textwrap.dedent( | ||
| f"""\ | ||
| import pytest | ||
|
|
||
| from execution_testing import ( | ||
| Account, | ||
| Environment, | ||
| TestAddress, | ||
| Transaction, | ||
| ) | ||
|
|
||
| CONTRACT = "{CONTRACT_ADDR}" | ||
|
|
||
| @pytest.mark.valid_at("Cancun") | ||
| def test_alloc_canonical(state_test) -> None: | ||
| state_test( | ||
| env=Environment(), | ||
| pre={{ | ||
| TestAddress: Account(balance=10**18), | ||
| CONTRACT: Account( | ||
| nonce=1, | ||
| code=b"", | ||
| storage={{0x22: 0xabc}}, | ||
| ), | ||
| }}, | ||
| post={{}}, | ||
| tx=Transaction(), | ||
| ) | ||
| """ | ||
| ) | ||
|
|
||
|
|
||
| def _load(path: Path) -> Any: | ||
| with path.open() as f: | ||
| return json.load(f) | ||
|
|
||
|
|
||
| def test_input_alloc_matches_output_alloc_format( | ||
| pytester: pytest.Pytester, tmp_path: Path | ||
| ) -> None: | ||
| """ | ||
| Filling a one-tx test with ``--traces`` must produce an | ||
| ``input/alloc.json`` whose formatting matches the t8n's | ||
| ``output/alloc.json``: storage keys padded to 32 bytes, empty fields | ||
| omitted, nonces in minimal hex. The contract account at | ||
| ``CONTRACT_ADDR`` is untouched by the transaction, so its JSON | ||
| representation must be byte-identical between the two files. | ||
| """ | ||
| tests_dir = pytester.mkdir("tests") | ||
| cancun_dir = tests_dir / "cancun" | ||
| cancun_dir.mkdir() | ||
| case_dir = cancun_dir / "alloc_canonical" | ||
| case_dir.mkdir() | ||
| (case_dir / "test_alloc_canonical.py").write_text(TEST_MODULE) | ||
|
|
||
| pytester.copy_example( | ||
| name="src/execution_testing/cli/pytest_commands/pytest_ini_files/pytest-fill.ini" | ||
| ) | ||
| dump_dir = tmp_path / "dump" | ||
|
|
||
| result = pytester.runpytest( | ||
| "-c", | ||
| "pytest-fill.ini", | ||
| "--fork", | ||
| "Cancun", | ||
| "tests/cancun/alloc_canonical/", | ||
| "--traces", | ||
| "--evm-dump-dir", | ||
| str(dump_dir), | ||
| "--clean", | ||
| "-q", | ||
| ) | ||
| assert result.ret == 0, f"fill failed: {result.outlines}" | ||
|
|
||
| inputs = sorted(dump_dir.rglob("input/alloc.json")) | ||
| outputs = sorted(dump_dir.rglob("output/alloc.json")) | ||
| assert inputs and outputs, f"no alloc dumps under {dump_dir}" | ||
|
|
||
| input_alloc = _load(inputs[0]) | ||
| output_alloc = _load(outputs[0]) | ||
|
|
||
| # Format invariants on the input — all aligned with the output's | ||
| # canonical conventions. | ||
| for addr, account in input_alloc.items(): | ||
| # Empty storage / zero balance / empty code must be omitted. | ||
| assert "storage" not in account or account["storage"], ( | ||
| f"{addr}: empty storage dict not omitted" | ||
| ) | ||
| assert "balance" not in account or account["balance"] != "0x00", ( | ||
| f"{addr}: zero-padded balance not omitted" | ||
| ) | ||
| assert "code" not in account or account["code"] != "0x", ( | ||
| f"{addr}: empty code not omitted" | ||
| ) | ||
| # Storage keys must be 32-byte zero-padded. | ||
| for key in account.get("storage", {}): | ||
| assert len(key) == 66 and key.startswith("0x"), ( | ||
| f"{addr}: storage key not 32-byte-padded: {key!r}" | ||
| ) | ||
| # Nonce uses minimal hex (no leading zero unless value is 0). | ||
| if "nonce" in account: | ||
| n = account["nonce"] | ||
| assert n == "0x0" or not n.startswith("0x0"), ( | ||
| f"{addr}: nonce uses zero-padding: {n}" | ||
| ) | ||
|
|
||
| # The contract is never touched by the transaction, so its | ||
| # representation must be identical in input and output. | ||
| assert CONTRACT_ADDR in input_alloc, ( | ||
| f"contract {CONTRACT_ADDR} missing from input alloc" | ||
| ) | ||
| assert CONTRACT_ADDR in output_alloc, ( | ||
| f"contract {CONTRACT_ADDR} missing from output alloc" | ||
| ) | ||
| assert input_alloc[CONTRACT_ADDR] == output_alloc[CONTRACT_ADDR], ( | ||
| "contract account differs between input and output:\n" | ||
| f" input: {input_alloc[CONTRACT_ADDR]}\n" | ||
| f" output: {output_alloc[CONTRACT_ADDR]}" | ||
| ) |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small downside of this function is that it's decoupled from the
Allocmodel, so if we change it (very unlikely) this function would break.