Skip to content

starknet_committer: add utility that returns sorted leaf indices#13992

Merged
ArielElp merged 1 commit into
mainfrom
ariel/get_leaf_indicies_utility
May 27, 2026
Merged

starknet_committer: add utility that returns sorted leaf indices#13992
ArielElp merged 1 commit into
mainfrom
ariel/get_leaf_indicies_utility

Conversation

@ArielElp
Copy link
Copy Markdown
Contributor

@ArielElp ArielElp commented May 7, 2026

No description provided.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

@cursor
Copy link
Copy Markdown

cursor Bot commented May 14, 2026

PR Summary

Low Risk
Mechanical extraction of existing index-building logic with no change to traversal or proof semantics.

Overview
Introduces LeavesRequest and SortedLeavesRequest in tree.rs so class, contract, and per-contract storage leaf NodeIndex buffers—and their SortedLeafIndices views—are built in one place instead of duplicated inside fetch_previous_and_new_patricia_paths.

LeavesRequest::from_accessed_leaves maps accessed class hashes, contract addresses, and storage keys into the index structures fetch_all_patricia_paths expects; SortedLeavesRequest is derived via From<&mut LeavesRequest> by sorting those buffers. fetch_previous_and_new_patricia_paths now builds a single mutable request, sorts once, and passes the same sorted views to both previous- and new-root fetches. SortedLeafIndices is re-exported from the patricia crate for callers of this module.

Reviewed by Cursor Bugbot for commit 5d766f1. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

@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 changed the base branch from ariel/abstract_fetch_all_patricia_paths to graphite-base/13992 May 18, 2026 11:36
@ArielElp ArielElp force-pushed the graphite-base/13992 branch from 526b5f0 to b6e6d57 Compare May 18, 2026 11:56
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch from b925279 to 13ea978 Compare May 18, 2026 11:56
@ArielElp ArielElp changed the base branch from graphite-base/13992 to ariel/abstract_fetch_all_patricia_paths May 18, 2026 11:56
Copy link
Copy Markdown
Contributor Author

@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 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 force-pushed the ariel/get_leaf_indicies_utility branch from 13ea978 to d58523d Compare May 18, 2026 13:52
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch 2 times, most recently from 70039f0 to c579a4b Compare May 20, 2026 09:25
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from 9cc1634 to 56288b6 Compare May 20, 2026 09:25
Copy link
Copy Markdown
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, 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 force-pushed the ariel/abstract_fetch_all_patricia_paths branch from 56288b6 to d8a881d Compare May 25, 2026 13:32
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch from c579a4b to 869a81f Compare May 25, 2026 13:32
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from d8a881d to da84e37 Compare May 25, 2026 13:45
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch from 869a81f to 96c5bcb Compare May 25, 2026 13:45
Copy link
Copy Markdown
Contributor Author

@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 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

Copy link
Copy Markdown
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, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ArielElp).

@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from da84e37 to d962c4f Compare May 25, 2026 13:58
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch from 96c5bcb to 2e17706 Compare May 25, 2026 13:58
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from d962c4f to b25db53 Compare May 26, 2026 10:08
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch 2 times, most recently from 300a545 to c381bc4 Compare May 26, 2026 13:03
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch 2 times, most recently from bbe1511 to 823105d Compare May 26, 2026 13:11
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch 2 times, most recently from 1b083d2 to 9885436 Compare May 27, 2026 07:23
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from 823105d to 780c725 Compare May 27, 2026 07:23
@ArielElp ArielElp changed the base branch from ariel/abstract_fetch_all_patricia_paths to main May 27, 2026 07:53
@ArielElp ArielElp force-pushed the ariel/get_leaf_indicies_utility branch from 9885436 to 5d766f1 Compare May 27, 2026 07:54
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented May 27, 2026

Merge activity

  • May 27, 7:54 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@ArielElp ArielElp added this pull request to the merge queue May 27, 2026
Merged via the queue into main with commit a2ea4e1 May 27, 2026
26 checks passed
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.

4 participants