test: test_tree_with_sibling_nodes

[aner-starkware]

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.09%. Comparing base (d3642d1) to head (c75ad75).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
- Coverage   72.68%   72.09%   -0.60%     
==========================================
  Files          19       19              
  Lines         637      688      +51     
  Branches      637      688      +51     
==========================================
+ Hits          463      496      +33     
- Misses        144      163      +19     
+ Partials       30       29       -1     

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

[dorimedini-starkware]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @amosStarkware, @aner-starkware, and @AvivYossef-starkware)

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree_test.rs line 185 at r2 (raw file):

///            \            
///         i=35: leaf   
///            v=1    

PTAL at create_X_entry_for_testing functions (for X in {binary, edge, leaf}).
These test utils are for filled nodes, but it would help if you create helper functions for skeleton nodes as well - will be useful here, may reduce boilerplate and lines in this current test

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree_test.rs line 198 at r2 (raw file):

        },
    );
    // A sibling node with hash value 0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543

redundant comment IMO, and it's a shame to write the hash value several times (comment maintenance).
non blocking

Code quote:

// A sibling node with hash value 0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree_test.rs line 216 at r2 (raw file):

        },
    );
    // A sibling node with hash value 0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88

redundant

Code quote:

// A sibling node with hash value 0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88

[aner-starkware]

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree_test.rs line 185 at r2 (raw file):

Previously, dorimedini-starkware wrote…

PTAL at create_X_entry_for_testing functions (for X in {binary, edge, leaf}).
These test utils are for filled nodes, but it would help if you create helper functions for skeleton nodes as well - will be useful here, may reduce boilerplate and lines in this current test

Like this? Should I modify the test above the same way?

[dorimedini-starkware]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware, @aner-starkware, and @AvivYossef-starkware)

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/node.rs line 6 at r3 (raw file):

#[allow(dead_code)]
#[derive(Clone, Copy)]

since you had to add the derive I'm guessing this code isn't dead anymore? non-blocking

Suggestion:

#[derive(Clone, Copy)]

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree_test.rs line 185 at r2 (raw file):

Previously, aner-starkware wrote…

Like this? Should I modify the test above the same way?

if it makes sense and reduces boilerplate, yes please :)

[dorimedini-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware, @aner-starkware, and @AvivYossef-starkware)

[aner-starkware]

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/node.rs line 6 at r3 (raw file):

Previously, dorimedini-starkware wrote…

since you had to add the derive I'm guessing this code isn't dead anymore? non-blocking

I used it in a test, so I don't think it would have been related, but actually, I didn't need the clone in the end.

[aner-starkware]

Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @amosStarkware, @AvivYossef-starkware, and @dorimedini-starkware)

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree_test.rs line 185 at r2 (raw file):

Previously, dorimedini-starkware wrote…

if it makes sense and reduces boilerplate, yes please :)

OK, done.