starknet_committer: add utility that returns sorted leaf indices

[ArielElp]

No description provided.

[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
• 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 👈 (View in Graphite)
• 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.

[reviewable-StarkWare]

This change is 

[cursor]

PR Summary

Low Risk
Mechanical refactor with the same index-building and sorting logic; no change to trie traversal or proof semantics beyond clearer structure.

Overview
Introduces LeavesRequest and SortedLeavesRequest in tree.rs so class, contract, and per-contract storage leaf indices are built once from accessed leaves and then wrapped in SortedLeafIndices for Patricia path fetching.

fetch_previous_and_new_patricia_paths is refactored to call LeavesRequest::from_accessed_leaves and SortedLeavesRequest::from instead of duplicating index construction and sorting inline; the same sorted views are reused for both previous- and new-root fetch_all_patricia_paths calls. SortedLeafIndices is re-exported from starknet_patricia, and the contracts/storage assumption comment is clarified slightly.

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

[yoavGrs]

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

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 28 at r1 (raw file):

    StarknetForestProofs,
};

Restore the new line.

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 85 at r1 (raw file):

    contract_addresses: &[ContractAddress],
    contract_storage_keys: &HashMap<ContractAddress, Vec<StarknetStorageKey>>,
) -> LeavesRequest {

Move it into impl LeavesRequest?

Suggestion:

pub fn from...(
    class_hashes: &[ClassHash],
    contract_addresses: &[ContractAddress],
    contract_storage_keys: &HashMap<ContractAddress, Vec<StarknetStorageKey>>,
) -> Self {

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 247 at r1 (raw file):

    .await?;

    let (class_sorted, contract_sorted, storage_sorted) = leaves_request.get_sorted();

WDYT?
It’s logically equivalent, but I think this way makes it clearer that the code isn't doing any unnecessary work.

Suggestion:

    let prev_proofs = fetch_all_patricia_paths::<FactsNodeLayout>(
        storage,
        classes_trie_root_hashes.previous_root_hash,
        contracts_trie_root_hashes.previous_root_hash,
        class_sorted.clone(),
        contract_sorted.clone(),
        &storage_sorted.clone(),
    )
    .await?;

[ArielElp]

@ArielElp reviewed 1 file, made 3 comments, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on yoavGrs).

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 28 at r1 (raw file):

Previously, yoavGrs wrote…

Restore the new line.

Done.

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 85 at r1 (raw file):

Previously, yoavGrs wrote…

Move it into impl LeavesRequest?

Done

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 247 at r1 (raw file):

Previously, yoavGrs wrote…

WDYT?
It’s logically equivalent, but I think this way makes it clearer that the code isn't doing any unnecessary work.

Done (deleted the second unnecessary sorting)

[ArielElp]

@ArielElp resolved 1 discussion.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on yoavGrs).

[yoavGrs]

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

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 71 at r3 (raw file):

impl LeavesRequest {
    /// Builds index buffers expected by [`fetch_all_patricia_paths`].
    pub fn from(

Sorry for misleading you about the "..." I mentioned

Suggestion:

fn from_index_buffers(

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 96 at r3 (raw file):

    /// Sorts class, contract, and per-contract storage leaf index buffers in place, then returns
    /// a single [`SortedLeavesRequest`].
    pub fn get_sorted(&mut self) -> SortedLeavesRequest<'_> {

Use impl From please.

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 241 at r3 (raw file):

        LeavesRequest::from(class_hashes, contract_addresses, contract_storage_keys);

    let sorted_leaves = leaves_request.get_sorted();

Destruct here?

Suggestion:

SortedLeavesRequest{ ... } = leaves_request.get_sorted();

[ArielElp]

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

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 71 at r3 (raw file):

Previously, yoavGrs wrote…

Sorry for misleading you about the "..." I mentioned

Done.

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 96 at r3 (raw file):

Previously, yoavGrs wrote…

Use impl From please.

Done.

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 241 at r3 (raw file):

Previously, yoavGrs wrote…

Destruct here?

Done.

[yoavGrs]

@yoavGrs reviewed 1 file and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on ArielElp).

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on ArielElp).

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 57 at r4 (raw file):

/// per-contract storage leaves). Built via [`LeavesRequest::from_accessed_leaves`].
#[derive(Clone)]
pub struct LeavesRequest {

we had an equivalent concept on the py side called StateSelector. consider renaming, for SN OGs to feel at home (non-blocking)

Code quote:

pub struct LeavesRequest {

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 137 at r4 (raw file):

        "contract_sorted_leaf_indices is missing an address with requested storage witnesses. \
         contract_sorted_leaf_indices: {:?}, storage addresses: {:?}",
        contract_sorted_leaf_indices,

why the diff here? seems like it didn't have to change

Code quote:

         contract_sorted_leaf_indices: {:?}, storage addresses: {:?}",
        contract_sorted_leaf_indices,

[ArielElp]

@ArielElp made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on dorimedini-starkware and yoavGrs).

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 57 at r4 (raw file):

Previously, dorimedini-starkware wrote…

we had an equivalent concept on the py side called StateSelector. consider renaming, for SN OGs to feel at home (non-blocking)

I'm not sure if they fill the same role:

class StateSelector(StateSelectorBase):

"""
A class that contains a set of StarkNet contract addresses (sub-commitment tree root IDs)
and a set of hashes of relevant contract classes
affected by one/many transaction(s).
Used for fetching those sub-trees and classes from storage before transaction(s) processing.
"""

In any case, I'm for independence from our pythonic chains

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line 137 at r4 (raw file):

Previously, dorimedini-starkware wrote…

why the diff here? seems like it didn't have to change

It didn't, reverted

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on ArielElp).