fix: fix bug in tree hash function definition

[aner-starkware]

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.01%. Comparing base (8e24699) to head (b064108).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #121      +/-   ##
==========================================
+ Coverage   69.98%   73.01%   +3.02%     
==========================================
  Files          27       29       +2     
  Lines        1036     1156     +120     
  Branches     1036     1156     +120     
==========================================
+ Hits          725      844     +119     
- Misses        277      280       +3     
+ Partials       34       32       -2     

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

[dorimedini-starkware]

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

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs line 28 at r2 (raw file):

pub const CONTRACT_STATE_HASH_VERSION: Felt = Felt::ZERO;
// The hex string corresponding to b'CONTRACT_CLASS_LEAF_V0' in big-endian.
pub const CONTRACT_CLASS_LEAF_V0: &str = "0x434f4e54524143545f434c4153535f4c4541465f5630";

add a tiny unit test that asserts this is true

Code quote:

// The hex string corresponding to b'CONTRACT_CLASS_LEAF_V0' in big-endian.
pub const CONTRACT_CLASS_LEAF_V0: &str = "0x434f4e54524143545f434c4153535f4c4541465f5630";

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function_test.rs line 65 at r2 (raw file):

#[rstest]
// expected hash value was computed independently.

Suggestion:

Exp

[aner-starkware]

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

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs line 28 at r2 (raw file):

Previously, dorimedini-starkware wrote…

add a tiny unit test that asserts this is true

Done.

[dorimedini-starkware]

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

[dorimedini-starkware]

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

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs line 49 at r4 (raw file):

                    "could not parse hex string corresponding to b'CONTRACT_CLASS_LEAF_V0' to Felt",
                );
                PoseidonHashFunction::compute_hash(HashInputPair(

I don't like this... trees need two hash functions now? because the compiled class tree uses pedersen on internal nodes and poseidon on leaves?

Code quote:

PoseidonHashFunction

[aner-starkware]

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs line 49 at r4 (raw file):

Previously, dorimedini-starkware wrote…

I don't like this... trees need two hash functions now? because the compiled class tree uses pedersen on internal nodes and poseidon on leaves?

That's as far as I understand. Maybe we could leave it as an implementation detail (i.e., without generics) for now - we don't know if the second hash function (which is already Poseidon) will need to be changed.

[dorimedini-starkware]

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

---

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs line 49 at r4 (raw file):

Previously, aner-starkware wrote…

That's as far as I understand. Maybe we could leave it as an implementation detail (i.e., without generics) for now - we don't know if the second hash function (which is already Poseidon) will need to be changed.

I think we need to get rid of HashFunction and only keep TreeHashFunction, and all implementation details / low-level hash function choices should be in the tree hash function.
Out of scope of this PR anyway;
please add a TODO over this line and I'll unblock

[aner-starkware]

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs line 49 at r4 (raw file):

Previously, dorimedini-starkware wrote…

I think we need to get rid of HashFunction and only keep TreeHashFunction, and all implementation details / low-level hash function choices should be in the tree hash function.
Out of scope of this PR anyway;
please add a TODO over this line and I'll unblock

Done.

[dorimedini-starkware]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @amosStarkware, @nimrod-starkware, and @TzahiTaub)