starknet_committer: support both layouts in fetch_patricia_paths

[ArielElp]

No description provided.

[reviewable-StarkWare]

This change is 

[ArielElp]

• apollo_committer: add new request handler #14003
• starknet_committer: delete patricia paths on reverts #14002
• 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
• 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 👈 (View in Graphite)
• main

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

[cursor]

PR Summary

Medium Risk
Touches core Patricia witness generation used for proofs by refactoring and changing traversal logic to handle both storage layouts; incorrect hashing/traversal could produce invalid witnesses.

Overview
Moves fetch_patricia_paths/fetch_patricia_paths_inner out of facts_db::traversal into trie_traversal and makes them generic over NodeLayout, so the same API can fetch witnesses for both facts and non-facts DB layouts.

Updates the traversal algorithm to cope with layouts where child hashes aren’t embedded in the parent value by queuing “hash-only” reads for unmodified children and deferring PreimageMap inserts until required child hashes are available.

Adjusts call sites in patricia_merkle_tree/tree.rs to pass FactsNodeLayout, and relocates the traversal tests accordingly.

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

[yoavGrs]

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

---

a discussion (no related file):
Please separate moving the file from the changes made inside it. It’ll make it easier for me to review the changes.

[yoavGrs]

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

---

crates/starknet_committer/src/db/facts_db/traversal.rs line 53 at r1 (raw file):

}

// TODO(Rotem): Match the python logic of fetching the nodes.

What about this TODO?
On this point, is there a note in the test that we are collecting too many nodes?

Code quote:

// TODO(Rotem): Match the python logic of fetching the nodes.

---

crates/starknet_committer/src/db/trie_traversal.rs line 57 at r1 (raw file):

#[cfg(test)]
#[path = "facts_db/traversal_test.rs"]

The test should be beside the corresponding module.

Code quote:

"facts_db/traversal_test.rs"

---

crates/starknet_committer/src/db/trie_traversal.rs line 61 at r1 (raw file):

/// Logs out a warning of a trivial modification.
macro_rules! log_trivial_modification {

Move it down, please.
(Top-down Ordering)

Code quote:

macro_rules! log_trivial_modification 

---

crates/starknet_committer/src/db/trie_traversal.rs line 67 at r1 (raw file):

}

/// Returns the Patricia inner nodes ([PreimageMap]) in the paths to the given `leaf_indices` in the

With the siblings?

Code quote:

in the paths to the given `leaf_indices`

---

crates/starknet_committer/src/db/trie_traversal.rs line 101 at r1 (raw file):

/// Required for `patricia_update` function in Cairo.
/// Extra preimages (more than the data required to verify merkle paths) are required to verify
/// correctness of final tree topology; for more details see 'traverse_edge' in 'patricia.cairo'.

Not in this repo

Code quote:

 see 'traverse_edge' in 'patricia.cairo'

---

crates/starknet_committer/src/db/trie_traversal.rs line 129 at r1 (raw file):

    while !current_traversal.is_empty() || !current_hash_only.is_empty() {
        read_hashes::<L, Layout>(&current_hash_only, storage, key_context, &mut hash_by_index)
            .await?;

Can you read all the hashes for the waiting hash-only vector outside the loop?

Code quote:

        read_hashes::<L, Layout>(&current_hash_only, storage, key_context, &mut hash_by_index)
            .await?;