central_systest_blobs: refactor skeleton into structured factory#13962
Conversation
PR SummaryLow Risk Overview Adds Reviewed by Cursor Bugbot for commit 839ea7c. Bugbot is set up for automated code reviews on this repo. Configure here. |
nimrod-starkware
left a comment
There was a problem hiding this comment.
@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()
}6f48b03 to
b2772b7
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@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
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.
nimrod-starkware
left a comment
There was a problem hiding this comment.
@nimrod-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on yoavGrs).
b2772b7 to
839ea7c
Compare
Merge activity
|
|
Artifacts upload workflows: |

No description provided.