feat(fw): Gas test generator

[marioevz]

🗒️ Description

Creates a gas test generator based off the MCOPY memory copy tests.

It can be applied to *any test and the test writer only needs to provide a couple of fixtures for it to work.

*It only works on legacy EVM code at the moment and not EOF, but I have a couple of ideas of how to make it work.

🔗 Related Issues

None

✅ 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]

*It only works on legacy EVM code at the moment and not EOF, but I have a couple of ideas of how to make it work.

Why? I tried and just wrapped test_code in a container and it seems to just have worked. It would need to "know" whether it should generate legacy or EOF or both (either directly or by combining with #610). Were there any other issues?

[marioevz]

*It only works on legacy EVM code at the moment and not EOF, but I have a couple of ideas of how to make it work.

Why? I tried and just wrapped test_code in a container and it seems to just have worked. It would need to "know" whether it should generate legacy or EOF or both (either directly or by combining with #610). Were there any other issues?

No other issues, I meant working automatically, and yes I agree #610 should solve this.

[marioevz]

@pdobacz @shemnon I rebased to main and renamed this generator to exact_gas_test so it differentiates from the one in #713.

I also added the EOF marker @pytest.mark.with_all_evm_code_types to the MCOPY test and it works out of the box in conjunction with this generator to produce legacy and EOF tests automatically.

[pdobacz]

@shemnon @marioevz I'm totally on the fence with the #713 vs #719, ever so slightly in favor of this #719, just because it already has OOG testing and easier integration with the EOF marker and no GAS opcode dependency. Or is the plan to have both? Maybe this is the way, and we just write both kinds for each case to test, serving two slightly different purposes.

Would other people on the testing team have a preference here?

[marioevz]

@shemnon @marioevz I'm totally on the fence with the #713 vs #719, ever so slightly in favor of this #719, just because it already has OOG testing and easier integration with the EOF marker and no GAS opcode dependency. Or is the plan to have both? Maybe this is the way, and we just write both kinds for each case to test, serving two slightly different purposes.

Would other people on the testing team have a preference here?

I think #713 is not generalized at the moment, and we could generalize it eventually, but it has been merged as a one-off function for the time being.

I tagged other members of the testing team for them to have a look at the implementation but in my opinion having many generators for different purposes should be ok.

[spencer-tb]

Really like this idea!! There are so many ways we can extend this for other types of state test generators.

The refactor in the test is very clean! My only concern is that we are not being verbose enough with respect how we define the test, as we don't know where the required fixtures are going. Maybe decorating the test function like so:

@exact_gas_test(
    state_test: StateTestFiller,
    ...
    tx: Transaction,
    gas_test_types=GasTestType.OOG,
    with_data=True,
)
def test_mcopy_huge_memory_expansion():
    pass

This could be a me problem though, and maybe its more obvious when writing the test.

[spencer-tb]

I think this still works if we move the with_data logic witih the decorator:

def decorator(func: Callable[..., Any], with_data: bool) -> Callable[..., Any]:
    @pytest.mark.parametrize(
        "gas_test_type",
        gas_test_types,
        ids=lambda x: x.value,
    )
    def generated_gas_test(
        pre: Alloc,
        state_test: StateTestFiller,
        exact_gas_cost: int,
        gas_test_type: GasTestType,
        bytecode: Bytecode,
        data: bytes = None,
    ):
        if with_data:
            generator(
                pre=pre,
                state_test=state_test,
                exact_gas_cost=exact_gas_cost,
                gas_test_type=gas_test_type,
                bytecode=bytecode,
                data=data,
            )
        else:
            generator(
                pre=pre,
                state_test=state_test,
                exact_gas_cost=exact_gas_cost,
                gas_test_type=gas_test_type,
                bytecode=bytecode,
            )

    generated_gas_test.__name__ = func.__name__
    generated_gas_test.__doc__ = func.__doc__

    return generated_gas_test

[spencer-tb]

Should we assert all the required fixtures?

[danceratopz]

This is so nice! This automatic test generation is going to be so valuable.

However, I tend to prefer a pytest plugin approach instead of a library decorator that returns @pytest.mark.parametrize:

1. I'm not fond of moving the state_test call out of the test function (it's now directly called in the library helper). Apart from being unexpected, it means that the implementer can't modify any of the objects going to the state test, post, for example (as it's set in the library function, it can't be extended by the implementer).
2. I'm also not keen on moving test parametrization to custom decorators that aren't exposed to pytest as we don't get a mark for these custom decorators (this means we can't filter by this type of test, for example). I.e., I'd prefer something like @pytest.mark.generate_gas_test(). I suspect we'll also run into problems applying xfail to subsets of these parametrized tests (but we could add a custom marks keyword as suggested for the with_all_... markers).

I created a rough POC for a pytest plugin, it doesn't modify post yet, but I think it's possible: #760