Skip to content

starknet_committer: abstract fetch_all_patricia_paths#13991

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

starknet_committer: abstract fetch_all_patricia_paths#13991
ArielElp merged 1 commit into
mainfrom
ariel/abstract_fetch_all_patricia_paths

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

Medium Risk
Touches Merkle proof / witness path assembly for block commitment; behavior should match FactsNodeLayout but layout abstraction increases the chance of subtle type or conversion mismatches for other layouts later.

Overview
fetch_all_patricia_paths is now public and generic over DbLayout, so Patricia path fetching for classes, contracts, and per-contract storage tries uses each layout’s DB leaf types and node layout instead of hardcoding FactsNodeLayout and concrete leaf types.

Contract trie leaves are read through AsRef<ContractState> for storage roots and converted with Into<ContractState> when building proof maps. fetch_previous_and_new_patricia_paths is unchanged in behavior but calls fetch_all_patricia_paths::<FactsNodeLayout> and is documented as facts-layout storage only.

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

@ArielElp ArielElp changed the base branch from ariel/add_two_layer_storage to graphite-base/13991 May 14, 2026 11:56
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 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ArielElp).


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

        // prove regarding the contract storage.
        let Some(storage_root_hash) =
            leaves.get(idx).map(|leaf| leaf.clone().into().storage_root_hash)

Can you avoid cloning?

Code quote:

leaf.clone().into().storage_root_hash

@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from 526b5f0 to b6e6d57 Compare May 18, 2026 11:36
@ArielElp ArielElp force-pushed the graphite-base/13991 branch from d746081 to 1fd7795 Compare May 18, 2026 11:36
@ArielElp ArielElp changed the base branch from graphite-base/13991 to ariel/add_two_layer_storage May 18, 2026 11:36
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 partially reviewed 1 file and made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on yoavGrs).


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

Previously, yoavGrs wrote…

Can you avoid cloning?

Replaced the Clone bound with AsRef to circumvent cloning

@ArielElp ArielElp force-pushed the ariel/add_two_layer_storage branch from 1fd7795 to eb0084f Compare May 18, 2026 13:52
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from b6e6d57 to 815bf53 Compare May 18, 2026 13:52
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.

:lgtm:

@yoavGrs reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
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 815bf53 to 8acac5f Compare May 19, 2026 12:37
@ArielElp ArielElp force-pushed the ariel/add_two_layer_storage branch from eb0084f to 7f68595 Compare May 19, 2026 12:37
@ArielElp ArielElp force-pushed the ariel/add_two_layer_storage branch from 7f68595 to adada4b Compare May 20, 2026 09:03
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch 2 times, most recently from 9cc1634 to 56288b6 Compare May 20, 2026 09:25
@ArielElp ArielElp force-pushed the ariel/add_two_layer_storage branch from adada4b to 260ae4f 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.
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 4 times, most recently from d962c4f to b25db53 Compare May 26, 2026 10:08
@ArielElp ArielElp force-pushed the ariel/add_two_layer_storage branch from 827da80 to 55283c6 Compare May 26, 2026 10:08
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from b25db53 to bbe1511 Compare May 26, 2026 13:03
@ArielElp ArielElp force-pushed the ariel/add_two_layer_storage branch 2 times, most recently from 9d10dee to dc4126c Compare May 26, 2026 13:11
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from bbe1511 to 823105d Compare May 26, 2026 13:11
@ArielElp ArielElp changed the base branch from ariel/add_two_layer_storage to main May 27, 2026 07:13
@ArielElp ArielElp force-pushed the ariel/abstract_fetch_all_patricia_paths branch from 823105d to 780c725 Compare May 27, 2026 07:23
@ArielElp ArielElp added this pull request to the merge queue May 27, 2026
Merged via the queue into main with commit 8f4a522 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