Skip to content

central_systest_blobs: refactor skeleton into structured factory#13962

Merged
dorimedini-starkware merged 1 commit intomainfrom
04-30-central_systest_blobs_refactor_skeleton_into_structured_factory
May 5, 2026
Merged

central_systest_blobs: refactor skeleton into structured factory#13962
dorimedini-starkware merged 1 commit intomainfrom
04-30-central_systest_blobs_refactor_skeleton_into_structured_factory

Conversation

@dorimedini-starkware
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator Author

dorimedini-starkware commented May 5, 2026

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

@cursor
Copy link
Copy Markdown

cursor Bot commented May 5, 2026

PR Summary

Low Risk
Changes are limited to test/data-generation scaffolding and dependency wiring; core runtime logic isn’t modified. Main risk is build/test fallout from the new starknet_committer dev-dependency and the refactor leaving placeholder unimplemented!() paths for future work.

Overview
Refactors the blob regression test data generation from a set of free functions into a BlobFactory/BlockData structure that will own chain info, state, and per-block artifacts, with finalize() producing blobs plus a preconfirmed block.

Adds starknet_committer (and related storage types) as a new dev-dependency and updates the test to derive chain_info from the factory instead of a global static; most generation logic remains as TODO/unimplemented placeholders.

Reviewed by Cursor Bugbot for commit 839ea7c. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Contributor

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

@nimrod-starkware reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on dorimedini-starkware and yoavGrs).


crates/central_systest_blobs/src/cende_blob_regression_test.rs line 137 at r1 (raw file):

    #[expect(dead_code)]
    fn current_state_roots(&self) -> StateRoots {
        self.blocks.last().map(|block| block.state_roots).unwrap_or_default()

unrelated to this PR, but consider adding this constant to StateRoots

Suggestion:

self.blocks.last().map(|block| block.state_roots).unwrap_or(StateRoots::ROOTS_OF_EMPTY_STATE)

crates/central_systest_blobs/src/cende_blob_regression_test.rs line 138 at r1 (raw file):

    fn current_state_roots(&self) -> StateRoots {
        self.blocks.last().map(|block| block.state_roots).unwrap_or_default()
    }

Lets use the same terminology

Suggestion:

    #[expect(dead_code)]
    fn last_finalized_block_hash(&self) -> BlockHash {
        self.blocks.last().map(|block| block.block_hash).unwrap_or(BlockHash::GENESIS_PARENT_HASH)
    }

    #[expect(dead_code)]
    fn last_finalized_state_roots(&self) -> StateRoots {
        self.blocks.last().map(|block| block.state_roots).unwrap_or_default()
    }

@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_refactor_skeleton_into_structured_factory branch from 6f48b03 to b2772b7 Compare May 5, 2026 11:56
Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware made 2 comments and resolved 1 discussion.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on nimrod-starkware and yoavGrs).


crates/central_systest_blobs/src/cende_blob_regression_test.rs line 137 at r1 (raw file):

Previously, nimrod-starkware wrote…

unrelated to this PR, but consider adding this constant to StateRoots

PTAL


crates/central_systest_blobs/src/cende_blob_regression_test.rs line 138 at r1 (raw file):

Previously, nimrod-starkware wrote…

Lets use the same terminology

Done.

Copy link
Copy Markdown
Contributor

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@nimrod-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on yoavGrs).

@dorimedini-starkware dorimedini-starkware changed the base branch from 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs to main May 5, 2026 13:28
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_refactor_skeleton_into_structured_factory branch from b2772b7 to 839ea7c Compare May 5, 2026 13:29
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented May 5, 2026

Merge activity

  • May 5, 1:29 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Artifacts upload workflows:

@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue May 5, 2026
Merged via the queue into main with commit f5db387 May 5, 2026
40 checks passed
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