starknet_committer: underlying logic of the new read witnesses and commit endpoint

[ArielElp]

No description provided.

[reviewable-StarkWare]

This change is 

[ArielElp]

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

• apollo_committer: add new request handler #14003
• starknet_committer: delete patricia paths on reverts #14002
• starknet_committer,apollo_committer: add timers for fetch patricia paths #14189
• starknet_committer: underlying logic of the new read witnesses and commit endpoint #14001 👈 (View in Graphite)
• 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
• starknet_committer: db keys for proofs and accessed keys digest #13997
• apollo_committer_types: add patricia proofs request and response types #13996
• 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.

[ArielElp]

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

---

crates/apollo_committer/src/committer.rs line 503 at r4 (raw file):

Previously, yoavGrs wrote…

I found this a bit hard to read.
Could you flatten the function out and use some named variables?

Done

---

crates/apollo_committer/src/committer.rs line 508 at r4 (raw file):

Previously, yoavGrs wrote…

Top-down Ordering

Done.

---

crates/apollo_committer/src/committer.rs line 521 at r4 (raw file):

Previously, yoavGrs wrote…

It makes sense that LeavesRequest will be part of the request type.

Maybe it's a bad name, I can rename in a different PR. LeavesRequest is essentially a conversion between concrete leaves to NodeIndex, so I don't think it should be part of the request.

---

crates/apollo_committer/src/committer.rs line 555 at r4 (raw file):

Previously, yoavGrs wrote…

Add some comments about the main stages in the process.

Done.

---

crates/apollo_committer/src/committer.rs line 571 at r4 (raw file):

Previously, yoavGrs wrote…

The main goal at this stage is measurements, so you also need to cover the witness-related parts.

done in next PR

---

crates/apollo_committer/src/committer.rs line 597 at r4 (raw file):

Previously, yoavGrs wrote…

It is better not to create a mutable object unnecessarily.

Changed the original variable to mut.

---

crates/apollo_committer/src/committer.rs line 602 at r4 (raw file):

Previously, yoavGrs wrote…

It may help us.

done in next PR

[cursor]

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

^{❌ 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 a660497. Configure here.}

[yoavGrs]

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

---

crates/apollo_committer/src/committer.rs line 626 at r6 (raw file):

                })
            })
            .transpose()

This is my suggestion - break it with a let in the middle.

Suggestion:

        let digest_raw = self.forest_storage
            .read_metadata(ForestMetadataType::AccessedKeysDigest(DbBlockNumber(block_number)))
            .await
            .map_err(|error| self.map_internal_error(error))?;
        digest_raw.0.as_slice().try_into().map_err(|_| CommitterError::Internal {
                    height: block_number,
                    message: format!(
                        "Invalid OS witnesses digest length {} (expected 32)",
                        digest_raw.0.len()
                    ),
                })
            })
            .transpose()

[ArielElp]

@ArielElp made 1 comment and resolved 1 discussion.
Reviewable status: 2 of 5 files reviewed, all discussions resolved (waiting on yoavGrs).

---

crates/apollo_committer/src/committer.rs line 626 at r6 (raw file):

Previously, yoavGrs wrote…

This is my suggestion - break it with a let in the middle.

Done

[yoavGrs]

@yoavGrs reviewed 2 files and all commit messages.
Reviewable status: 4 of 5 files reviewed, all discussions resolved.

[ArielElp]

@ArielElp dismissed @yoavGrs from a discussion.
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on yoavGrs).

[yoavGrs]

@yoavGrs partially reviewed 4 files and all commit messages.
Reviewable status: 4 of 5 files reviewed, all discussions resolved.

[yoavGrs]

Reviewable status: 4 of 5 files reviewed, all discussions resolved.