refactor(test-tests): port EXTCODEHASH dynamicAccountOverwriteEmpty

[chfast]

🗒️ Description

Port the legacy static test:
stExtCodeHash/dynamicAccountOverwriteEmpty_Paris
for EIP-1052 EXTCODEHASH testing.

The test verifies EXTCODEHASH correctly updates when an empty account is overwritten by CREATE2. Modified from original: target account has no storage to avoid EIP-7610 collision behavior.

🔗 Related Issues or PRs

N/A.

✅ Checklist

• All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:

  uvx tox -e static

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

Cute Animal Picture

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.18%. Comparing base (c3813a5) to head (31eefbd).

Additional details and impacted files

@@                 Coverage Diff                 @@
##           forks/amsterdam    #2032      +/-   ##
===================================================
+ Coverage            86.14%   86.18%   +0.03%     
===================================================
  Files                  599      599              
  Lines                39472    39472              
  Branches              3780     3780              
===================================================
+ Hits                 34002    34017      +15     
+ Misses                4848     4838      -10     
+ Partials               622      617       -5     

Flag	Coverage Δ
unittests	86.18% <ø> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[LouisTsai-Csie]

Thanks for the PR! I’ve left a few comments suggesting some refactoring using helper functions. Overall, the test logic looks great.

[LouisTsai-Csie]

Suggested change

	@pytest.mark.ported_from(
	[
	"https://github.com/ethereum/tests/tree/v13.3/src/GeneralStateTestsFiller/stExtCodeHash/dynamicAccountOverwriteEmpty_ParisFiller.yml", # noqa: E501
	]
	)
	@pytest.mark.ported_from(
	[
	"https://github.com/ethereum/tests/tree/v13.3/src/GeneralStateTestsFiller/stExtCodeHash/dynamicAccountOverwriteEmpty_ParisFiller.yml", # noqa: E501
	],
	pr=["https://github.com/ethereum/execution-specs/pull/2032"],
	)

Adding this might be helpful, i discovered that other porting PR also have this marker.

[chfast]

Added.

[LouisTsai-Csie]

Suggested change

@pytest.mark.pre_alloc_modify

I’m not sure if we need this for the test, maybe I’m missing something here.

[chfast]

The caller_address is fixed, so you will not be able to deploy it in a live network. Maybe we should make it random: I don't think it will modify the main flow of the test.

[chfast]

This is resolved now by creating an account with CALL.

[LouisTsai-Csie]

Some suggestion for this call sequence:

• We could use the Storage object here instead of specifying the slot for SSTORE.

Example:

from execution_testing import Storage

storage = Storage()

Op.SSTORE(storage.store_next(EMPTY_KECCAK), Op.EXTCODEHASH(target_address))

store_next would handle the slot management for us. And the benefit is we could simplify the post storage by passing the entire Storage object:

caller_address: Account(storage=storage),

• In the original test, it do the EXTCODEHASH, EXTCODESIZE, EXTCODECOPY, CALLCODE to the target address before / after the contract creation. We might need to do the same check here to avoid any coverage loss.

• In the original test, it performs a CALLCODE to the target account. Since CALLCODE executes the callee’s code in the context of the caller, any code present at the target account could potentially modify the caller’s storage. The test currently checks whether the call succeeds and whether the caller’s storage remains unchanged, but this is insufficient here because there are two possible scenarios:

  • There is no code at the target account, so nothing is executed.
  • Code execution succeeds, but the logic does not modify storage.
    To distinguish these cases, I suggest passing zero gas to the call. This ensures that if a success value is returned, no execution could have taken place at all, making the test behavior unambiguous.

This is my suggested changes, please let me know if it is unclear or does not make sense, thanks:

caller_code = (
    # EXTCODEHASH Check
    Op.SSTORE(
        storage.store_next(EMPTY_KECCAK), Op.EXTCODEHASH(target_address)
    )
    # EXTCODESIZE Check
    + Op.SSTORE(storage.store_next(0), Op.EXTCODESIZE(target_address))
    # EXTCODECOPY Check
    + Op.EXTCODECOPY(target_address, 0, 0, 32)
    + Op.SSTORE(storage.store_next(0), Op.MLOAD(0))
    # CALL* Ops Check
    + Op.SSTORE(
        storage.store_next(1),
        Op.CALLCODE(
            gas=0,  # Passing zero gas to ensure no execution
            address=target_address,
        ),
    )
    # Create the contract via CREATE2
    + Op.MSTORE(0, bytes(create2_initcode).ljust(32, b"\0"))
    + Op.SSTORE(
        storage.store_next(target_address),
        Op.CREATE2(size=len(create2_initcode)),
    )
    # EXTCODEHASH Check
    + Op.SSTORE(
        storage.store_next(deploy_code.keccak256()),
        Op.EXTCODEHASH(target_address),
    )
    # EXTCODESIZE Check
    + Op.SSTORE(
        storage.store_next(len(deploy_code)),
        Op.EXTCODESIZE(target_address),
    )
    # EXTCODECOPY Check
    + Op.EXTCODECOPY(target_address, 0, 0, 32)
    + Op.SSTORE(
        storage.store_next(bytes(deploy_code).ljust(32, b"\0")),
        Op.MLOAD(0),
    )
    # CALL* Ops Check
    + Op.SSTORE(
        storage.store_next(1),
        Op.CALL(gas=gas_cost, address=target_address),
    )
)

[chfast]

I applied suggestions and even extended the workflow. In the end the check hash, size, and code itself in 3 states: non-existient, no-code and CREATE2-deployed. The no-code account is created via CALL so we can avoid fixed-address pre-state.

We also call the target account code via CALL and DELEGATECALL.

[LouisTsai-Csie]

Continue the last comment, we could simplify this storage comparison to

Suggested change

	storage={
	# Before CREATE2: empty account has keccak256("") hash
	0: EMPTY_KECCAK,
	1: 0, # EXTCODESIZE = 0
	# CALL to deployed contract succeeded
	3: 1,
	# CREATE2 returned the new address
	10: target_address,
	# After CREATE2: account has code
	4: deploy_code.keccak256(),
	5: len(deploy_code),
	},
	storage=storage

[chfast]

Done.

[LouisTsai-Csie]

This works under the fill mode, but i would suggest using a more dynamic approach, so that we could run the test under execute remote in a live network.

We could wait until PR #1996 merged to do the refactor again, and we would no longer need the hardcoded caller_address:

caller_address = Address(0x095E7BAEA6A6C7C4C2DFEB977EFAC326AF552D87)

[chfast]

Resolved. See above.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.