Skip to content

Conversation

@nimrod-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@nimrod-starkware nimrod-starkware self-assigned this Jan 21, 2026
@nimrod-starkware nimrod-starkware marked this pull request as ready for review January 21, 2026 13:19
Copy link
Contributor

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

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

@ArielElp reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware).


crates/starknet_committer/src/db/index_db/db.rs line 38 at r1 (raw file):

pub struct IndexDb<S: Storage> {
    pub storage: S,

Dori preferred not to give arbitrary access to the forest storage. Maybe we should add a get_storage_for_testing function?

Copy link
Contributor Author

@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 made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArielElp).


crates/starknet_committer/src/db/index_db/db.rs line 38 at r1 (raw file):

Previously, ArielElp wrote…

Dori preferred not to give arbitrary access to the forest storage. Maybe we should add a get_storage_for_testing function?

Done

@nimrod-starkware nimrod-starkware force-pushed the nimrod/index-DB-benchmarks branch from 297cc85 to 20e0f90 Compare January 21, 2026 14:22
Copy link
Contributor

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

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

@ArielElp reviewed 2 files and resolved 1 discussion.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @nimrod-starkware).

Copy link
Contributor

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

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

:lgtm:

@ArielElp made 1 comment.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @nimrod-starkware).

Copy link
Collaborator

@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 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware).


crates/starknet_committer/src/db/index_db/db.rs line 54 at r2 (raw file):

    pub fn get_mut_storage(&mut self) -> &mut S {
        &mut self.storage
    }

consider

  1. implementing get_stats and reset_stats, no need for the feature
  2. behind the feature flag, get_async_underlying_storage
    non-blocking

Code quote:

    #[cfg(any(feature = "testing", test))]
    pub fn get_storage(&self) -> &S {
        &self.storage
    }

    #[cfg(any(feature = "testing", test))]
    pub fn get_mut_storage(&mut self) -> &mut S {
        &mut self.storage
    }

Copy link
Collaborator

@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 reviewed 1 file and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware).

Copy link
Collaborator

@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 reviewed 1 file and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware).

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.

5 participants