new(tests): EOF - EIP-7069: Call Gas Testing for EXT*CALL

[shemnon]

🗒️ Description

Test call gas for EXT*CALL, using a legacy harness.

Signed-off-by: Danno Ferrin [email protected]

🔗 Related Issues

✅ 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: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
• 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.

[pdobacz]

I was opposed at first, but this looks very practical. Any ideas how to resolve the complaints I had?

• philosophically against the grain of removing gas introspection (not so important)
• ignores EOF-only EVMs, should such exist
• ignores post-legacy-EVM-ban worlds
• OOG testing - I suppose with some complication of your harness this could be done: if the harness stored the setup/teardown gas usage, added cold/warm gas to it and subtracted 1, or sth along these lines? For a healthy test though, this should probably be generated as a separate state_test, so 1 gas_test => 2 state_tests, is that even possible?

Are you proposing this more as a long-term or short-term approach?

[marioevz]

What I had in mind to not require gas introspection is to use a single contract, pre-calculate the gas and use that as the gas limit value of the transaction (plus the 21,000).

With that in mind, cold and warm could be two separate tests, and in the warm test, the subject code becomes the same code twice and the calculated gas is cold+warm.

Wdyt @pdobacz @shemnon

[shemnon]

How do we assert the gas within the test and durring a fill? If all we want is observed gas then we can get the same effect by filling the desired behavior and having the fixtures pass/fail based on total consumed gas.

What I want is a test that will fail only because of gas calculations so an implementor can look at the test and know it's a gas problem.

We need to detect and fail on both overcharging and undercharging. A test with a strict gas limit only gets us overcharging.

[pdobacz]

What I had in mind to not require gas introspection is to use a single contract, pre-calculate the gas and use that as the gas limit value of the transaction (plus the 21,000).

The approach from MCOPY mem expansion test correct? I initially preferred that to using GAS, but this PR here makes a strong case for using GAS. I actually only now realized it solves the problem of dynamic gas allowances of EXT*CALL I think we'd run into with the "MCOPY approach"

[marioevz]

Yes I think this approach is really nice too, I think we could plan on doing a decorator around this, even if it uses Op.GAS, as long as that opcode is available we can use it.

I have a branch with a decorator for the MCOPY type of gas test, #719, we could use that decorator as a template to create another decorator for this type of gas tests.

[pdobacz]

@marioevz are we maybe closer to a decision whether we'll adopt this or #719 moving forward (or both?). Also, I'm wondering which approach would lend itself better to the EOF marker? (for an example MCOPY gas test using the EOF marker, without the decoration, is in #754).

Another question still up is the OOG testing part (see previous comment) - is such an extension to the harness here feasible?

@shemnon Re:

We need to detect and fail on both overcharging and undercharging. A test with a strict gas limit only gets us overcharging.

In #719 both strict gas limit and strict gas limit - 1 are tested, former expected to work, latter expected to OOG, so undercharging is also covered.

[marioevz]

Also, I'm wondering which approach would lend itself better to the EOF marker? (for an example MCOPY gas test using the EOF marker, without the decoration, is in #754).

I'm taking a look at gas_test in this PR and it lends itself pretty well to the EOF decorator without that many changes.

pre.deploy_contract has the evm_code_type keyword argument which can force a deployment of a contract to be in legacy mode, even when the test has been parametrized to run in EOF mode, so we can use this to force address_legacy_harness to always be a legacy contract, regardless of the context, and address_baseline/address_subject can be dynamically deployed as EOF of Legacy depending on the context.

[marioevz]

Looks good! We could move gas_test to src/ethereum_test_tools/code/generators.py and export it to use it anywhere, although I'm thinking of any other cases (other than *CALL opcode testing) where the cold-warm dynamic would come into play.

[shemnon]

I'm going to punt on moving it. Mostly because of the storage slot allocation needed to store data(slot_warm_gas and slot_cold_gas values), I'm not sure what we want to introduce to the framework as a whole vs what is just a pattern for EOF tests.

[marioevz]

LGTM, thanks!

[marioevz]

Pushed a commit to make tox happy, I'll merge in a bit.