Skip to content

Conversation

CPerezz
Copy link
Contributor

@CPerezz CPerezz commented Oct 1, 2025

🗒️ Description

This PR completes the work started in #2186 whose goal was to implement all the variants (and orderings) of BALANCE <-> EXTCODE_ opcode abuse. No new setup scripts nor anything is required.

It's important to highlight that this PR SHOULD NOT be merged until #2186 is. Also, is highly encouraged for the review to review only the lattest commit (which is the only addition on top of the aforementioned PR). So c044cb3 should be the only thing to review.

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
  • Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
  • Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

@CPerezz
Copy link
Contributor Author

CPerezz commented Oct 1, 2025

cc: @LouisTsai-Csie as you're the one with the most background on this, you might be the best reviewer.

Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie left a comment

Choose a reason for hiding this comment

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

The only difference between this PR and PR #2186 is the order of operations. Instead of duplicating tests for each case, I recommend using parametrize feature in pytest framework to avoid repetitive code.

For example, in test_worst_zero_param, the test logic is shared across multiple opcodes. Rather than rewriting the test for each opcode, the implementation parametrizes the opcode argument and passes it into the test function, making it available inside the test:

# Parametrization example
@pytest.mark.parametrize(
    "opcode",
    [
        Op.ADDRESS,
        Op.ORIGIN,
        Op.CALLER,
        Op.CODESIZE,
        Op.GASPRICE,
        Op.COINBASE,
        Op.TIMESTAMP,
        Op.NUMBER,
        Op.PREVRANDAO,
        Op.GASLIMIT,
        Op.CHAINID,
        Op.BASEFEE,
        Op.BLOBBASEFEE,
        Op.GAS,
    ],
)
def test_worst_zero_param(
    state_test: StateTestFiller,
    pre: Alloc,
    opcode: Op,   # The parametrized opcode is passed here
    fork: Fork,
    gas_benchmark_value: int,
):
    # The opcode can be accessed directly in the test

We can also parametrize multiple values. For instance, test_worst_callvalue parametrizes both non_zero_value and from_origin. With two distinct values each, pytest generates 4 test combinations automatically.

For the current case, we can do the same with the operation order by introducing an op_order parameter:

@pytest.mark.parametrize("op_order", [True, False])
@pytest.mark.valid_from("Prague")
def test_bloatnet_balance_extcodecopy(
    blockchain_test: BlockchainTestFiller,
    pre: Alloc,
    fork: Fork,
    gas_benchmark_value: int,
    op_order: bool,  # Pass in the function
):

Then, extract the operations whose order changes:

extcodecopy_op = (
    Op.PUSH1(1)  # size (1 byte)
    + Op.PUSH2(max_contract_size - 1)  # code offset (last byte)
    + Op.ADD(Op.MLOAD(32), 96)         # unique memory offset
    + Op.DUP4                          # address (duplicated earlier)
    + Op.EXTCODECOPY
    + Op.POP                           # clean up address
)

balance_op = Op.POP(Op.BALANCE)  # cold access

benchmark_op = extcodecopy_op + balance_op if op_order else balance_op + extcodecopy_op

Finally, replace the sequence in the implementation with benchmark_op.

@LouisTsai-Csie
Copy link
Collaborator

Hi @CPerezz, PR #2186 was merged, please rebase your changes!

Some additional notes regarding the comments: if you want to run a specific subset of parameters, you can use the -k flag to filter by parameterized values.

For example, in test_worst_zero_param:

@pytest.mark.parametrize(
    "opcode",   # variable name
    [
        Op.ADDRESS,  # value
        ...
    ],
)

If you only want to run the ADDRESS opcode, concatenate the variable name and value with an underscore. Here the variable is opcode and the value is ADDRESS, so the full identifier is opcode_ADDRESS:

fill -v tests/benchmark/test_worst_compute.py::test_worst_zero_param -m benchmark --clean --gas-benchmark-values 1 -k "opcode_ADDRESS"

For multiple conditions, you can combine them with logical operators. For instance, in test_worst_callvalue, if you only want the case (non_zero_value=True, from_origin=True), you can run:

-k "non_zero_value_True and from_origin_True"

@CPerezz
Copy link
Contributor Author

CPerezz commented Oct 3, 2025

Thanks for the suggestion @LouisTsai-Csie ! I appreciate a lot the help with this one! It made everything much better!

Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie left a comment

Choose a reason for hiding this comment

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

Hi @CPerezz , thanks for the changes. There are some folder restructuring due to recent PR merge (#2169 and #2186 ). It would be nice if you could help migrate the file in the PR to new location, so we would not have duplicate tests here.

The CI is failing due to linting issue, you could run uvx --with=tox-uv tox -e lint, and resolve the error displayed, thanks

For the bloatnet marker, now you should run -m stateful instead of -m benchmark, for all the tests under the stateful folder. This is required for future CI/CD integration for stateful cases, including bloatnet and XEN.

Otherwise, there are no more suggested changes!

… BloatNet tests

Replace duplicate test functions with parametrized versions to avoid repetitive
code. Each test now accepts a `balance_first` parameter that controls the order
of operations, eliminating the need for separate `_extcodesize_balance`,
`_extcodecopy_balance`, and `_extcodehash_balance` variants.

Changes:
- Add @pytest.mark.parametrize to test_bloatnet_balance_extcodesize,
  test_bloatnet_balance_extcodecopy, and test_bloatnet_balance_extcodehash
- Each test now generates two variants via parametrization with descriptive IDs
  (e.g., "balance_extcodesize" and "extcodesize_balance")
- Extract operation sequences into variables and conditionally compose them
  based on balance_first parameter
- Remove test_bloatnet_extcodesize_balance, test_bloatnet_extcodecopy_balance,
  and test_bloatnet_extcodehash_balance (now covered by parametrization)

This reduces the file from 793 lines to 462 lines while maintaining the same
test coverage (6 tests total: 3 test functions × 2 parametrization values).

To run specific parameter variants, use the -k flag:
  fill -k "balance_extcodesize" tests/benchmark/bloatnet/test_multi_opcode.py
  fill -k "extcodesize_balance" tests/benchmark/bloatnet/test_multi_opcode.py

Co-Authored-By: LouisTsai-Csie
<72684086+LouisTsai-Csie@users.noreply.github.com>
@CPerezz CPerezz force-pushed the feat/bloatnet-multi-opcode-completion branch from ed323b0 to 2ad25ea Compare October 3, 2025 10:25
@CPerezz CPerezz requested a review from LouisTsai-Csie October 3, 2025 11:09
Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@LouisTsai-Csie LouisTsai-Csie merged commit 14d7a5d into ethereum:main Oct 3, 2025
16 checks passed
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.

2 participants