Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RLP Block importer does not validate block headers against engine API rules #8452

Open
macfarla opened this issue Mar 24, 2025 · 2 comments
Open
Assignees
Labels
hive relating to hive tests testing

Comments

@macfarla
Copy link
Contributor

eg this test https://hive.ethpandaops.io/pectra/suite.html?suiteid=1742771993-dd8981844476302696757189d4fe81e8.json&suitename=eest%2Fconsume-rlp#

there are 3 failing for the same reason - eg
tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py::test_invalid_pre_fork_block_with_blob_fields[fork_ShanghaiToCancunAtTime15k-blockchain_test-excess_blob_gas_present_False-blob_gas_used_present_True]

expects that if the block header has blobGasUsed present, it will fail import. This validation occurs within EngineNewPayloadV2 but because these hive tests use RLPBlockImporter, they don't go through the EngineAPI. Would be nice to refactor so we can have the same validation happen here.

Description:
Test id: tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py::test_invalid_pre_fork_block_with_blob_fields[fork_ShanghaiToCancunAtTime15k-blockchain_test-excess_blob_gas_present_False-blob_gas_used_present_True]

Test source: https://github.com/ethereum/execution-spec-tests/tree/v4.1.0/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py#L207

Test function documentation:

Test block rejection when `excessBlobGas` and/or `blobGasUsed` fields are present on a pre-fork
block.

Blocks sent by NewPayloadV2 (Shanghai) that contain `excessBlobGas` and `blobGasUsed` fields
must be rejected with the appropriate `EngineAPIError.InvalidParams` error error.
@macfarla macfarla added the hive relating to hive tests label Mar 25, 2025
@jflo
Copy link
Contributor

jflo commented Mar 25, 2025

Fork specific validation rules should be pushed down for maximum reuse based on the protocol schedule, and failures need to be finely grained enough that the EngineAPI using it can convey correct errors back up to the client.

@jflo
Copy link
Contributor

jflo commented Mar 25, 2025

Needs design work.

It is also possible that this test, as implemented is overly dependent on a reference implementation instead of a spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hive relating to hive tests testing
Projects
None yet
Development

No branches or pull requests

2 participants