refactor: remove dependency on a given tree height of original skeletons

[nimrod-starkware]

First stage of the tree height refactor.
Small trees are now 'large' with an edge node to the root of the small tree.
Next stage is to remove the tree_height parameter inside original skeleton

---

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.22%. Comparing base (89b1bca) to head (7a3df56).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #179      +/-   ##
==========================================
+ Coverage   62.11%   62.22%   +0.11%     
==========================================
  Files          36       36              
  Lines        1668     1673       +5     
  Branches     1668     1673       +5     
==========================================
+ Hits         1036     1041       +5     
  Misses        583      583              
  Partials       49       49              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dorimedini-starkware]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @nimrod-starkware and @TzahiTaub)

---

crates/committer/src/patricia_merkle_tree/types.rs line 165 at r1 (raw file):

    #[cfg(test)]
    #[allow(dead_code)]
    /// Assumes self represnts an index in a smaller tree height. Returns a node index represents the

typo

Code quote:

represnts

---

crates/committer/src/patricia_merkle_tree/types.rs line 166 at r1 (raw file):

    #[allow(dead_code)]
    /// Assumes self represnts an index in a smaller tree height. Returns a node index represents the
    /// same index in tree of height 251 as if the smaller tree was 'planted' at the lowest leftmost

rephrase to refer to the starknet tree height, not to the number 251

Code quote:

of height 251

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs line 220 at r1 (raw file):

    /// 'Plants' the given smaller tree at the lowest leftmost node in order to create a tree of
    /// height TREE_HEIGHT::MAX.
    pub(crate) fn create_actual_sized_tree_from_smaller_tree(smaller_tree: Self) -> Self {

this function isn't used in this PR, correct?
if so, then it doesn't belong in this PR; please remove it (open another PR with a usage later if it's useful)

Code quote:

create_actual_sized_tree_from_smaller_tree

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 185 at r1 (raw file):

    let mut sorted_leaf_indices: Vec<NodeIndex> = leaf_modifications
        .keys()
        .map(|idx| idx.to_actual_tree_index(tree_height))

"actual" is confusing, please rename

Suggestion:

from_subtree_index

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 201 at r1 (raw file):

        .into_iter()
        .map(|(node_idx, node)| (node_idx.to_actual_tree_index(tree_height), node))
        .collect();

this mapping conversion is also done below; please make a test util out of it

Code quote:

    let mut expected_nodes: HashMap<NodeIndex, OriginalSkeletonNode> = expected_nodes
        .into_iter()
        .map(|(node_idx, node)| (node_idx.to_actual_tree_index(tree_height), node))
        .collect();

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 209 at r1 (raw file):

            length: EdgePathLength(TreeHeight::MAX.0 - tree_height.0),
        }),
    );

duplicated logic; write a test util for it

Suggestion:

    add_containing_tree_root(&mut expected_nodes, tree_height);

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 285 at r1 (raw file):

                    path: 0.into(),
                    length: EdgePathLength(251 - height),
                }),

1. no hard coded 251
2. reuse the logc in add_containing_tree_root (see above)

Code quote:

                NodeIndex::ROOT,
                OriginalSkeletonNode::Edge(PathToBottom {
                    path: 0.into(),
                    length: EdgePathLength(251 - height),
                }),

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 338 at r1 (raw file):

pub(crate) fn create_root_edge_entry(
    old_root: u8,
    small_tree_height: TreeHeight,

Suggestion:

subtree_height

[nimrod-starkware]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @dorimedini-starkware and @TzahiTaub)

---

crates/committer/src/patricia_merkle_tree/types.rs line 165 at r1 (raw file):

Previously, dorimedini-starkware wrote…

typo

Done

---

crates/committer/src/patricia_merkle_tree/types.rs line 166 at r1 (raw file):

Previously, dorimedini-starkware wrote…

rephrase to refer to the starknet tree height, not to the number 251

Done, hope that's more obvious

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs line 220 at r1 (raw file):

Previously, dorimedini-starkware wrote…

this function isn't used in this PR, correct?
if so, then it doesn't belong in this PR; please remove it (open another PR with a usage later if it's useful)

Done. removed

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 185 at r1 (raw file):

Previously, dorimedini-starkware wrote…

"actual" is confusing, please rename

Done.

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 201 at r1 (raw file):

Previously, dorimedini-starkware wrote…

this mapping conversion is also done below; please make a test util out of it

I used the function create_expected_skeleton which included the logic for the forest.

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 209 at r1 (raw file):

Previously, dorimedini-starkware wrote…

duplicated logic; write a test util for it

I used the function create_expected_skeleton which included the logic for the forest.

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 285 at r1 (raw file):

Previously, dorimedini-starkware wrote…

1. no hard coded 251
2. reuse the logc in add_containing_tree_root (see above)

Done.

---

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs line 338 at r1 (raw file):

pub(crate) fn create_root_edge_entry(
    old_root: u8,
    small_tree_height: TreeHeight,

Done.

[dorimedini-starkware]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware and @TzahiTaub)

---

crates/committer/src/patricia_merkle_tree/types.rs line 167 at r2 (raw file):

    /// the same index in the starknet state tree as if the smaller tree was 'planted' at the lowest
    /// leftmost node from the root.
    pub(crate) fn from_subtree_index(sub_tree_index: Self, subtree_height: TreeHeight) -> Self {

subtree is one word on our repo (not english but ok)

Suggestion:

subtree_index: Self, subtree_height: TreeHeight

[nimrod-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @TzahiTaub)

---

crates/committer/src/patricia_merkle_tree/types.rs line 167 at r2 (raw file):

Previously, dorimedini-starkware wrote…

subtree is one word on our repo (not english but ok)

Done.

[dorimedini-starkware]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)