starknet_committer,apollo_committer: add timers for fetch patricia paths

[ArielElp]

No description provided.

[reviewable-StarkWare]

This change is 

[ArielElp]

• starknet_committer: fetch patricia paths concurrently #14283
• starknet_committer: define fetch_contract_storage_paths_concurrently #14282
• starknet_committer: define StoragePathsReadTask #14281
• starknet_patricia_storage: move is_gatherable to ReadOnlyStorage #14280
• apollo_committer,starknet_committer: use blockifier's AccessedKeys #14252
• apollo_committer: add new request handler #14003
• apollo_committer: add request_paths_and_commit_block tests #14208
• starknet_committer: delete patricia paths on reverts #14002
• starknet_committer,apollo_committer: add timers for fetch patricia paths #14189 👈 (View in Graphite)
• starknet_committer: underlying logic of the new read witnesses and commit endpoint #14001
• starknet_committer: extract metadata construction to a function #14000
• starknet_committer: extract commit plan to commit_or_load #13999
• starknet_committer: read and write patricia paths to index db #13998
• apollo_committer_types: add patricia proofs request and response types #13996 : 1 other dependent PR (#13997 )
• starknet_committer: add patricia paths forest reader/writer traits #13995
• starknet_committer: define StarknetForestProofs serde #13994
• starknet_committer: compute and store accessed keys digest #13993
• starknet_committer: add utility that returns sorted leaf indices #13992
• starknet_committer: abstract fetch_all_patricia_paths #13991
• starknet_patricia_storage: two layer storage #13990
• starknet_committer: move fetch_patricia_paths tests #13960
• starknet_committer: fetch patricia paths tests #13959
• starknet_committer: support both layouts in fetch_patricia_paths #13958
• main

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

[cursor]

PR Summary

Low Risk
Observability-only changes behind os_input; no changes to commit, witness merge, or persistence logic.

Overview
Adds os_input-gated timing around the two Patricia witness fetches in read_paths_and_commit_block (pre-commit vs post-commit roots), and moves the end-to-end block timer so it covers those passes as well as commit/write.

starknet_committer gains new Action variants, timers, durations, and a first-pass witness entry count on BlockMeasurement; apollo_committer starts/stops measurements around each fetch_patricia_witnesses call and extends block debug logs with pre/post fetch milliseconds and witness entry totals. Witness size logging uses a new StarknetForestProofs::get_nodes_count() instead of inline counting.

starknet_committer_cli exposes an os_input feature that forwards to starknet_committer/os_input. A TODO notes Prometheus metrics for witness fetch are not added yet.

^{Reviewed by Cursor Bugbot for commit 0556455. Bugbot is set up for automated code reviews on this repo. Configure here.}

[ArielElp]

@ArielElp made 2 comments.
Reviewable status: 2 of 7 files reviewed, 3 unresolved discussions (waiting on yoavGrs).

---

crates/starknet_committer/src/block_committer/commit.rs line 242 at r7 (raw file):

Previously, yoavGrs wrote…

We don't have the number of witnesses?

Yeah, it was a bug, overridden here.

I made some changes; in retrospect, the number of witnesses fetched does not fit in BlockModificationsCount, since the commit_block flow sets all these numbers once. Reintroduced the witness count in BlockMeasurement and added it to the debug log.

---

crates/starknet_committer_cli/src/utils_test.rs line 20 at r7 (raw file):

Previously, yoavGrs wrote…

Give an interesting value.

Refactored around it.

[yoavGrs]

@yoavGrs reviewed 5 files and all commit messages, made 3 comments, and resolved 2 discussions.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on ArielElp).

---

crates/apollo_committer/src/committer.rs line 682 at r9 (raw file):

#[allow(clippy::as_conversions)]
// TODO: Consider adding fetch witnesses measurements.

You are doing it here, right?

Code quote:

// TODO: Consider adding fetch witnesses measurements.

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 134 at r9 (raw file):

    pub durations: BlockDurations,
    pub modifications_counts: BlockModificationsCounts,
    pub fetched_witnesses_count: usize,

It was in BlockModificationsCounts.
I see that it's not only about the first pass now.

Suggestion:

#[cfg(feature = "os_input")]
 // Number of witnesses fetched in the first pass (pre-commit).
 pub fetched_witnesses_count: usize,

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 167 at r9 (raw file):

            Action::FetchWitnessesSecondPass => {
                self.durations.fetch_witnesses_second_pass = duration_in_seconds;
                self.fetched_witnesses_count += entries_count;

The intersection between the witnesses in the first and second pass isn't empty...

Code quote:

 self.fetched_witnesses_count += entries_count;

[ArielElp]

@ArielElp made 3 comments and resolved 4 discussions.
Reviewable status: 6 of 7 files reviewed, 4 unresolved discussions (waiting on yoavGrs).

---

crates/apollo_committer/src/committer.rs line 682 at r9 (raw file):

Previously, yoavGrs wrote…

You are doing it here, right?

It only goes to the log, no dedicated metric

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 134 at r9 (raw file):

Previously, yoavGrs wrote…

It was in BlockModificationsCounts.
I see that it's not only about the first pass now.

Reintroduced

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 167 at r9 (raw file):

Previously, yoavGrs wrote…

The intersection between the witnesses in the first and second pass isn't empty...

Right... reverting to the previous state (two counts won't be very informative ATM, unless I do something a bit more sophisticated than .len)

[cursor]

Second pass witness count silently discarded in measurement

Low Severity

The FetchWitnessesSecondPass handler in update_after_action records the duration but does not accumulate entries_count into fetched_witnesses_count, unlike FetchWitnessesFirstPass. The caller passes proof_after.len() as the entry count, but it's silently ignored. The log then reports "witness entries" using fetched_witnesses_count, which only reflects the first pass, making the logged metric misleading.

Additional Locations (1)

• crates/apollo_committer/src/committer.rs#L772-L778

 

^{Reviewed by Cursor Bugbot for commit fdd5e5d. Configure here.}

[yoavGrs]

@yoavGrs made 2 comments and resolved 2 discussions.
Reviewable status: 5 of 7 files reviewed, 3 unresolved discussions (waiting on ArielElp).

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 134 at r9 (raw file):

Previously, ArielElp wrote…

Reintroduced

With the feature flag

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 167 at r9 (raw file):

Previously, ArielElp wrote…

Right... reverting to the previous state (two counts won't be very informative ATM, unless I do something a bit more sophisticated than .len)

You can take the length of the union of the witness sets, but I'm fine with taking the first one.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 7 total unresolved issues (including 6 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 701d19d. Configure here.}

[ArielElp]

@ArielElp resolved 3 discussions.
Reviewable status: 4 of 7 files reviewed, 1 unresolved discussion (waiting on yoavGrs).

[ArielElp]

@ArielElp made 1 comment.
Reviewable status: 4 of 7 files reviewed, 1 unresolved discussion (waiting on yoavGrs).

---

crates/starknet_committer/src/block_committer/measurements_util.rs line 134 at r9 (raw file):

Previously, yoavGrs wrote…

With the feature flag

Done.

[yoavGrs]

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

---

crates/starknet_committer/src/patricia_merkle_tree/types.rs line 72 at r13 (raw file):

        self.classes_trie_proof.len()
            + self.contracts_trie_proof.nodes.len()
            + self.contracts_trie_proof.leaves.len()

You did both modifications. Does clippy complain if you rename it back to len?

Code quote:

    pub fn get_nodes_count(&self) -> usize {
        self.classes_trie_proof.len()
            + self.contracts_trie_proof.nodes.len()
            + self.contracts_trie_proof.leaves.len()