Skip to content

Conversation

@spencer-tb
Copy link
Collaborator

@spencer-tb spencer-tb commented Feb 6, 2024

🗒️ Description

This PR aims to start creating a structure within src/ethereum_test_tools/ for usage within consume.

Its predominant aim is to define a types file specifically for the engine api spec, that can be used within the tests_consume/test_via_engine_api.py EEST simulator (and potentially many more). This allows us to:

  1. Separate from the existing FixtureEngineNewPayload etc types, apprehending a future deprecation of the hive blockchain fixture format. We can delete these with ease.
  2. Internally start a python specification for the engine api, that can be built upon for future EEST simulators.
  3. Opens the option to directly create the engine new payload json (and dataclasses) during filling procedures. This may be of use at somepoint, where we don't need to read in the fixture json directly -> convert it to dataclasses -> and then serialize it back to json for engine new payloads.

Key Changes

  • Updates various field names to be more concretely related to the engine api spec. This helps cleanly extract fields from a FixtureBlock type. Changes incl:
    • base_fee -> base_fee_per_gas,
    • reciept_root -> reciepts_root,
    • bloom -> logs_bloom,
    • mix_digest -> prev_randao,
    • hash -> block_hash,
    • Only in FixtureBlock: txs -> transactions
  • Moves src/ethereum_test_tools/rpc/* into src/ethereum_test_tools/consume/.
  • Adds src/ethereum_test_tools/consume/types.py, containing engine new payload and execution payload types, specific to the engine api fork version - aligning cleanly with the spec.
  • Adds tests to check that we can:
    • Extract a FixtureBlock into their respective Engine<fork>.NewPayloadV<version> dataclasses.
    • Convert the Engine<fork>.NewPayloadV<version> dataclasses to the correct json representation, expected within engine_rpc.py.

Remaining Todos

  • Add more complex tests for valid Cancun and Shanghai blockchain test fixtures, properly testing the conversion from a FixtureBlock to a correct engine new payload json representation:
    • Withdrawals extractions
    • ExpectedBlobVersionedHashes & ParentBeaconBlockRoot extractions
  • Delete the existing src/ethereum_test_tools/rpc/ folder.
  • Integrate the new types within test_consume_via_engine.py and engine_rpc.py, cleaning up more code.
  • Rebase back onto Dan's branch, after his is rebased to main.
  • Clean up tox.

Other Considerations

  • Change the name of coinbase to feeRecipent to align more with the PoS terminology (also makes the engine spec cleaner).
  • Like the above, change the name of number to block_number.
  • Use Base and Meta classes within the engine spec types to reduce duplication of functions.

Copy link
Collaborator

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

I could not even get it to work lol. How do you set HIVE_SIMULATOR in the vscode config?

I think one problem right now is on the src/pytest_plugins/consume/consume.py file with the pytest_configure method, because I don't think this file should try to generate the test cases, and instead this functionality should be moved to a custom pytest_generate_tests in each plugin, consume_direct, consume_via_rlp and consume_via_engine_api.

Method create_test_cases_from_json and class TestCase in src/pytest_plugins/consume/consume.py should be moved and customized too.

Also, I think a rebase is long due, because we are working on top of very old code, and we are duplicating efforts.


import time

import jwt
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to add PyJWT==2.8.0 (is this the right one?) to setup.cfg.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could not even get it to work lol. How do you set HIVE_SIMULATOR in the vscode config?
I wonder is we should set this automatically, right now its manual:

set -x HIVE_SIMULATOR http://127.0.0.1:3000

I think one problem right now is on the src/pytest_plugins/consume/consume.py file with the pytest_configure method, because I don't think this file should try to generate the test cases, and instead this functionality should be moved to a custom pytest_generate_tests in each plugin, consume_direct, consume_via_rlp and consume_via_engine_api.

Nice! This sounds like the move forward - I think currently its this way as we can apply each test case for every consume method.

Copy link
Owner

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Regarding the location of the new RPC classes: I'd prefer not to have them under a consume namespace, rather as they were in ethereum_test_tools.rpc. I think they'll get used in tools outside of the scope of consume sooner or later, e.g., ethereum#323.

@spencer-tb
Copy link
Collaborator Author

Closing in favor of: #31

@spencer-tb spencer-tb closed this Feb 8, 2024
@danceratopz
Copy link
Owner

I think one problem right now is on the src/pytest_plugins/consume/consume.py file with the pytest_configure method, because I don't think this file should try to generate the test cases, and instead this functionality should be moved to a custom pytest_generate_tests in each plugin, consume_direct, consume_via_rlp and consume_via_engine_api.

Method create_test_cases_from_json and class TestCase in src/pytest_plugins/consume/consume.py should be moved and customized too.

Thanks for taking a look, @marioevz!

I agree, that intuitively, you'd expect the fixtures to be loaded in pytest_generate_tests() and not in pytest_configure(). The reason is to allow piping of fixtures from fill AND using them in multiple consume tests within one pytest session, e.g.,

fill -k "chainid and blockchain_test and not hive" --fork=Shanghai --output=stdout | consume all -v

image

The problem is that pytest_generate_tests() gets called once per test function. So if you process the input on stdin in pytest_generate_tests(), it's unavailable in subsequent executions of the hook. Of course we could save it the first time it's called in a global... Happy to discuss the best solution!

With regards to having all the logic in the base consume plugin logic (instead of the separate consume sub-plugins), I need to revisit this in order to explain it, but there were some idiosyncrasies with pytest test parametrization. It was also related with executing the tests from multiple consumers within one pytest session.

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.

3 participants