Skip to content

Conversation

pdobacz
Copy link
Contributor

@pdobacz pdobacz commented Oct 4, 2025

🗒️ Description

The blobSchedule.json is required by evmone-t8n tool now, in order to fill tests correctly. We add a new structure intended to be used as a JSON generator for the filesystem evaluation inputs - I guess trying to mimic what TransitionToolRequest does, lmk if I misread the idea here.

Appropriate change on the evmone end: ipsilon/evmone#1330

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).

@pdobacz pdobacz force-pushed the feat/t8n-filesystem-blob-schedule branch from 220b7cf to 030733b Compare October 6, 2025 08:11
@pdobacz pdobacz marked this pull request as draft October 6, 2025 09:01
@pdobacz
Copy link
Contributor Author

pdobacz commented Oct 6, 2025

coverted to draft b/c maybe we should send more in that JSON (to include network and (future) ...Time fields to be able to t8n in e.g. BPO1)

@pdobacz pdobacz marked this pull request as ready for review October 7, 2025 08:13
@pdobacz
Copy link
Contributor Author

pdobacz commented Oct 7, 2025

Ready to review. The version pushed now will process (in line with ipsilon/evmone#1330) an --input.config argument and JSON, so that end-game schedule (with ...Time) fields is support-able, doesn't worry about handling fill --fork OsakaToBPO1AtTime15k, for now. AFAIK this doesn't break any existing fill-capabilities of evmone, so we can maybe think about providing them as a 2nd step

Copy link
Member

@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.

Thanks for implementing this, some comments.

Comment on lines +216 to +221
class TransitionToolConfig(CamelModel):
"""Transition tool config."""

fork: Fork = Field(..., alias="network")
chain_id: int = Field(..., alias="chainid")
blob_schedule: BlobSchedule
Copy link
Member

Choose a reason for hiding this comment

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

This one is very similar to the already-existing TransitionToolContext, so we can just repurpose that one instead.


fork: Fork = Field(..., alias="network")
chain_id: int = Field(..., alias="chainid")
blob_schedule: BlobSchedule
Copy link
Member

Choose a reason for hiding this comment

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

As discussed in chat group, let's pass instead:

Suggested change
blob_schedule: BlobSchedule
blob_params: BlobParams

Also, we already have a definition that fits our needs in here:

class ForkBlobSchedule(CamelModel):
"""Representation of the blob schedule of a given fork."""
target_blobs_per_block: HexNumber = Field(..., alias="target")
max_blobs_per_block: HexNumber = Field(..., alias="max")
base_fee_update_fraction: HexNumber = Field(...)

We could even rename that type to BlobParams because the current name ForkBlobSchedule doesn't make much sense.

Comment on lines +253 to +254
"--input.config",
input_paths["config"],
Copy link
Member

Choose a reason for hiding this comment

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

This will break client tools that do not support this parameter yet.

The way we handle this is by implementing a ClassVar in TransitionTool that could default to False (example here:

supports_xdist: ClassVar[bool] = True
), and then override it to True only for evmone.

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