feature: Use evmone-t8n to fill tests

[marioevz]

Use https://github.com/ethereum/evmone to fill tests.

Builds on top of work started by @spencer-tb.

Also implements transaction signing for all transaction types.

[marioevz]

@chfast @winsvega Evmone it's almost fully working now!

To run all tests using evmone-t8n use this branch and run:

fill --evm-bin=evmone-t8n

evmone-t8n should be available in path for it to work.

All but two pre-Cancun tests are filling correctly:

• tests/shanghai/eip4895_withdrawals/test_withdrawals.py::test_zero_amount[fork=Shanghai-four_withdrawals_one_with_value_one_with_max]
• tests/shanghai/eip4895_withdrawals/test_withdrawals.py::test_zero_amount[fork=Shanghai-four_withdrawals_one_with_value_one_with_max_reversed_order]

It seems like an account with an zero-value withdrawal remains in the state.

To run:

fill tests/shanghai/eip4895_withdrawals/test_withdrawals.py::test_zero_amount[fork=Shanghai-four_withdrawals_one_with_value_one_with_max] --evm-bin=evmone-t8n

and

fill tests/shanghai/eip4895_withdrawals/test_withdrawals.py::test_zero_amount[fork=Shanghai-four_withdrawals_one_with_value_one_with_max_reversed_order] --evm-bin=evmone-t8n

[danceratopz]

This is great! Love how easy it is to add a new transition tool.

I would suggest we split src/evm_transition_tool/__init__.py into multiple files. Typically, we've only defined the module/package namespaces in our init files; I think this is the only init that defines objects within it.

➜ find src -name __init__.py -exec wc -c '{}' \;
1270 src/ethereum_test_forks/__init__.py
50 src/ethereum_test_forks/tests/__init__.py
53 src/ethereum_test_forks/forks/__init__.py
467 src/ethereum_test_tools/spec/__init__.py
287 src/ethereum_test_tools/reference_spec/__init__.py
313 src/ethereum_test_tools/code/__init__.py
84 src/ethereum_test_tools/filling/__init__.py
1709 src/ethereum_test_tools/__init__.py
1520 src/ethereum_test_tools/common/__init__.py
50 src/ethereum_test_tools/tests/__init__.py
148 src/ethereum_test_tools/vm/__init__.py
186 src/pytest_plugins/spec_version_checker/__init__.py
86 src/pytest_plugins/test_filler/__init__.py
67 src/pytest_plugins/__init__.py
167 src/pytest_plugins/forks/__init__.py
13592 src/evm_transition_tool/__init__.py  <--------------------------------------------------------- !

How about moving:

• TransitionTool into transition_tool.py
• EvmTransitionTool into geth.py (and perhaps rename the class to GethTransitionTool
• EvmOneTransitionTool into evmone.py
  ?

Otherwise, couple of minor comments below.

[marioevz]

@danceratopz Thanks for the review! :)

Thanks to this handy stackoverflow response https://stackoverflow.com/a/66883553, I've added a dynamic method to load all transition tools automatically. Check it out: https://github.com/marioevz/execution-spec-tests/blob/3a28c9c50d3c905d39add0c7c2e12974d0043d22/src/evm_transition_tool/transition_tool.py#L37

[danceratopz]

Thanks to this handy stackoverflow response https://stackoverflow.com/a/66883553, I've added a dynamic method to load all transition tools automatically. Check it out: https://github.com/marioevz/execution-spec-tests/blob/3a28c9c50d3c905d39add0c7c2e12974d0043d22/src/evm_transition_tool/transition_tool.py#L37

This is awesome!

I went a step further and modified from_binary() to return the instantiated class directly: eb01d57.

Additionally:

• I cleaned up the types used for binary/binary_path: 6c60144
• Moved the common constructor code to the base class: 34cc32d
• Moved matches_binary_path to the base class: 7915b58
• Fixed --evm-bin=../evm and --evm-bin=~/bin/evm: 2fbf1cf

2ccb377 was necessary because i removed the tracing option when instantiating the t8n tool in pytest_plugins/forks.py (this snucked in in eb01d57). This inadvertently meant that every test raised the error instead of only once during pytest_configure().

Hope you don't mind me just jumping in on this?! 🙏

Just one comment. matches_binary_path doesn't seem like a very robust method. It looks like multiple tools call their binary evm. Ideally, they'd have a --name flag or similar. Or perhaps we can use the -v option or ask teams to add a unique "id" there?

[marioevz]

Just one comment. matches_binary_path doesn't seem like a very robust method. It looks like multiple tools call their binary evm. Ideally, they'd have a --name flag or similar. Or perhaps we can use the -v option or ask teams to add a unique "id" there?

I've harden the tool detection in 622f181.

Turns out, all current evm tools handle -v flag, so we can simply regex the result and detect the correct tool.

[danceratopz]

Really nice. My only concern is the silent exception, see below. I don't think it's too serious though, this shouldn't break too often.

[chfast]

This should fix it (but untested): ethereum/evmone#670.

Is this enough to fill --evm-bin=evmone-t8n to get test failure reports? If yes this should be good enough for us as the way of running these tests.

[danceratopz]

Hi @chfast,

This should fix it (but untested): ethereum/evmone#670.

Nice! I confirmed that the errors are indeed fixed in your branch (0.11.0-dev+commit.436f5f18).

Is this enough to fill --evm-bin=evmone-t8n to get test failure reports? If yes this should be good enough for us as the way of running these tests.

Yes, if evmone-t8n is in your path. Otherwise you can also provide an absolute or relative path to the evmone-t8n binary.

Just to clarify as to whether it's good enough, as I'm sure you're aware, fill only checks the limited post conditions defined in the test, in this case

execution-spec-tests/tests/shanghai/eip4895_withdrawals/test_withdrawals.py

Line 638 in ace89a1

all_post = {

It's not considered the full test (as defined by the corresponding JSON fixture that it generates).

[chfast]

For the reference, I can run it as described. It works very good (a lot of failures to fix) however debugging is difficult.