Skip to content

fix(monad): exclude blob suites from MONAD_EIGHT#15

Open
haythemsellami wants to merge 1 commit intomonad-developers:forks/monad_ninefrom
haythemsellami:monad_eight_disable_blob_suites
Open

fix(monad): exclude blob suites from MONAD_EIGHT#15
haythemsellami wants to merge 1 commit intomonad-developers:forks/monad_ninefrom
haythemsellami:monad_eight_disable_blob_suites

Conversation

@haythemsellami
Copy link

Summary

  • add a blob-fork validity marker and apply it to the EIP-4844 blob suites so they do not collect for blobless forks like MONAD_EIGHT
  • make MONAD_EIGHT stop advertising blob transaction support while keeping the rest of the fork behavior intact
  • carry sender authority history through fill so the forks/monad_eight branch can fill state fixtures again

Verification

  • UV_CACHE_DIR=/tmp/uv-cache uv run --no-sync pytest packages/testing/src/execution_testing/cli/pytest_commands/plugins/forks/tests/test_markers.py -q
  • UV_CACHE_DIR=/tmp/uv-cache uv run --no-sync pytest packages/testing/src/execution_testing/forks/tests/test_forks.py -q -k monad_eight_disables_blob_support
  • UV_CACHE_DIR=/tmp/uv-cache uv run --no-sync fill --fork MONAD_EIGHT --clean --no-html --single-fixture-per-file --output /tmp/monad-fill-berlin-sample -m state_test tests/berlin/eip2929_gas_cost_increases/test_call.py -k test_call_insufficient_balance -q
  • UV_CACHE_DIR=/tmp/uv-cache uv run --no-sync fill --fork MONAD_EIGHT --clean --no-html --single-fixture-per-file --output /tmp/monad-fill-blob-sample -m state_test tests/cancun/eip4844_blobs/test_excess_blob_gas.py -q # no tests ran

@greptile-apps
Copy link

greptile-apps bot commented Mar 9, 2026

Greptile Summary

This PR introduces a valid_for_blob_forks pytest marker to exclude blob-specific EIP-4844 test suites from blobless forks like MONAD_EIGHT, adds comprehensive blob-related method overrides to the MONAD_EIGHT fork class so it no longer advertises blob transaction support, and fixes a senders_authorities propagation gap in make_hive_fixture that was breaking state-fixture fills on the forks/monad_eight branch.

Key changes:

  • New ValidForBlobForks flag-type validity marker in forks.py that, when applied, subtracts all blobless forks (and their transition forks) from the test's collected fork set.
  • MONAD_EIGHT now overrides eight blob-related class methods to return False/None, while retaining the KZG point-evaluation precompile (address 0x0a).
  • senders_authorities dict is now properly initialised and threaded through each generate_block_data call in make_hive_fixture, matching the existing behaviour in make_fixture.
  • The valid_for_blob_forks() marker is applied to all seven EIP-4844 blob test modules.

Coverage concern — KZG precompile tests incorrectly excluded:
The valid_for_blob_forks() marker has been applied to test_point_evaluation_precompile.py and test_point_evaluation_precompile_gas.py, which will suppress them entirely when running against MONAD_EIGHT. However, these tests only issue regular (non-blob) transactions that call the KZG precompile directly — they do not require blob transaction support. Because MONAD_EIGHT explicitly retains the point-evaluation precompile, this coverage gap is unintentional and should be reviewed.

Confidence Score: 2/5

  • Core logic is sound but has an unintended test coverage gap: KZG precompile tests are excluded from MONAD_EIGHT despite that fork supporting the precompile.
  • The marker mechanism, MONAD_EIGHT fork overrides, and senders_authorities propagation are all correctly implemented and well-tested. However, applying valid_for_blob_forks() to the KZG precompile test files is too broad — these tests don't require blob transactions, only the KZG precompile which MONAD_EIGHT explicitly retains. This creates a silent test coverage loss that directly contradicts the stated goal of keeping the precompile functional on MONAD_EIGHT.
  • tests/cancun/eip4844_blobs/test_point_evaluation_precompile.py (line 63) and tests/cancun/eip4844_blobs/test_point_evaluation_precompile_gas.py (line 31) — consider removing valid_for_blob_forks() or introducing a narrower marker that distinguishes blob transaction requirements from blob infrastructure support.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pytest collects test] --> B{has valid_for_blob_forks marker?}
    B -- No --> C[ValidForBlobForks returns empty set\nforks - empty = unchanged fork set]
    B -- Yes --> D[ValidForBlobForks._process_with_marker_args\nbuilds set of blobless forks]
    D --> E[fork.supports_blobs == False?]
    E -- Yes --> F[Add fork + its transition forks to exclusion set]
    E -- No --> G[Skip fork]
    F --> H[process: forks - exclusion set]
    G --> H
    C --> I[Test runs for all selected forks]
    H --> I

    subgraph MONAD_EIGHT overrides
        J[supports_blobs → False]
        K[header_excess_blob_gas_required → False]
        L[header_blob_gas_used_required → False]
        M[tx_types → Prague types minus type-3]
        N[blob_schedule → None]
        O[engine_get_blobs_version → None]
        P[engine_new_payload_blob_hashes → False]
        Q[full_blob_tx_wrapper_version → None]
    end

    D -->|checks| J
Loading

Last reviewed commit: 6c37550

Copy link
Collaborator

@pdobacz pdobacz left a comment

Choose a reason for hiding this comment

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

Nice, thank you so much for this!

Can you switch the target to the default forks/monad_nine. I believe some of the diff should disappear.

Also, does this setup now allow to fill cancun/eip4844... tests with some meaningful result? if so, we may want to un-exclude them in the .github/configs/feature.yaml fill commands.

@haythemsellami haythemsellami force-pushed the monad_eight_disable_blob_suites branch from 6c37550 to 31f6b47 Compare March 13, 2026 15:51
@haythemsellami haythemsellami changed the base branch from forks/monad_eight to forks/monad_nine March 13, 2026 15:53
@haythemsellami
Copy link
Author

Nice, thank you so much for this!

Can you switch the target to the default forks/monad_nine. I believe some of the diff should disappear.

Also, does this setup now allow to fill cancun/eip4844... tests with some meaningful result? if so, we may want to un-exclude them in the .github/configs/feature.yaml fill commands.

@pdobacz I addressed all the issues/bot's comments.

For cancun/eip4844..., I don’t think we should un-exclude the whole group in .github/configs/feature.yaml yet. The actual blob-tx/blob-header suites are still not meaningful for Monad because MONAD_EIGHT/MONAD_NINE do not support blobs. The point-evaluation precompile suites are meaningful now, but they currently fail on both MONAD_EIGHT and MONAD_NINE with real behavior mismatches, so I’d keep the broad exclusion for now and narrow it only after we fix those KZG failures.

@haythemsellami haythemsellami requested a review from pdobacz March 13, 2026 15:58
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.

2 participants