starknet_committer: hash leaves of both layouts

[ArielElp]

No description provided.

[reviewable-StarkWare]

This change is 

[ArielElp]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• starknet_committer,starknet_patricia: layout-dependent FilledTree serialization #10988
• starknet_committer,starknet_patricia: add serialization utilities to node layout #10987
• starknet_committer,starknet_patricia: move db_layout #10986
• starknet_committer: add index db #10685
• starknet_committer: abstract original tree creation #10684
• starknet_committer: group generic layout logic #10852
• starknet_committer,starknet_patricia: remove generic config #10683
• starknet_committer: add index layout nodes #10682
• starknet_committer: add index layout subtree #10919
• starknet_committer: hash leaves of both layouts #10918 👈 (View in Graphite)
• starknet_committer: add index layout leaves #10681
• starknet_committer,starknet_patricia,starknet_patricia_storage: support contract address prefix #10858
• starknet_committer,starknet_patricia: add serde errors #10810
• starknet_committer,starknet_committer_and_os_cli,starknet_patricia: layout dependent fetch_nodes #10680
• starknet_committer,starknet_patricia: enrich subtree trait to support fetch nodes #10751
• starknet_committer,starknet_committer_and_os_cli,starknet_patricia: make FilledNode generic #10750
• starknet_committer,starknet_os,starknet_patricia: make node data generic #10679
• starknet_committer,starknet_patricia: add deserialization context #10704
• starknet_committer,starknet_patricia,starknet_patricia_storage: add context to prefix and serde #10678
• starknet_committer,starknet_patricia: add subtree trait #10694
• starknet_committer: reorganize facts db files #10677
• main-v0.14.1-committer

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

[nimrod-starkware]

@nimrod-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs).

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArielElp).

---

crates/starknet_committer/src/hash_function/hash.rs line 83 at r1 (raw file):

    let compiled_class_hash: &CompiledClassHash = compiled_class_hash_leaf.as_ref();
    let contract_class_leaf_version: Felt = Felt::from_hex(
        TreeHashFunctionImpl::CONTRACT_CLASS_LEAF_V0,

In the "global" namespace (TreeHashFunctionImpl), there is a constant defined only for compiled class hashes. In the same way, utils for compiled class hashes should be defined in the same place.

In addition, I prefer that the common place won't beTreeHashFunctionImpl. WDYT?

Code quote:

TreeHashFunctionImpl::CONTRACT_CLASS_LEAF_V0

---

crates/starknet_committer/src/hash_function/hash.rs line 84 at r1 (raw file):

    let contract_class_leaf_version: Felt = Felt::from_hex(
        TreeHashFunctionImpl::CONTRACT_CLASS_LEAF_V0,
    )

It's worth using the const function Felt::from_hex_unchecked for const strings.

Code quote:

    let contract_class_leaf_version: Felt = Felt::from_hex(
        TreeHashFunctionImpl::CONTRACT_CLASS_LEAF_V0,
    )

[ArielElp]

@ArielElp made 2 comments.
Reviewable status: 2 of 6 files reviewed, 2 unresolved discussions (waiting on @nimrod-starkware and @yoavGrs).

---

crates/starknet_committer/src/hash_function/hash.rs line 83 at r1 (raw file):

Previously, yoavGrs wrote…

In the "global" namespace (TreeHashFunctionImpl), there is a constant defined only for compiled class hashes. In the same way, utils for compiled class hashes should be defined in the same place.

In addition, I prefer that the common place won't beTreeHashFunctionImpl. WDYT?

Done.

---

crates/starknet_committer/src/hash_function/hash.rs line 84 at r1 (raw file):

Previously, yoavGrs wrote…

It's worth using the const function Felt::from_hex_unchecked for const strings.

Done.

[yoavGrs]

@yoavGrs reviewed 4 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArielElp).

---

crates/starknet_committer/src/hash_function/hash.rs line 84 at r1 (raw file):

Previously, ArielElp wrote…

Done.

I want this Felt to be calculated at compilation time. For this, the variable should be const (or static).
Please define at the top of the module:

const CONTRACT_CLASS_LEAF_VERSION: Felt = Felt::from_hex_unchecked(CONTRACT_CLASS_LEAF_V0);

[ArielElp]

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

---

crates/starknet_committer/src/hash_function/hash.rs line 84 at r1 (raw file):

Previously, yoavGrs wrote…

I want this Felt to be calculated at compilation time. For this, the variable should be const (or static).
Please define at the top of the module:

const CONTRACT_CLASS_LEAF_VERSION: Felt = Felt::from_hex_unchecked(CONTRACT_CLASS_LEAF_V0);

Done.

[yoavGrs]

@yoavGrs reviewed 4 files and all commit messages, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp).

[dorimedini-starkware]

@dorimedini-starkware reviewed 6 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArielElp).

---

crates/starknet_committer/src/hash_function/hash.rs line 18 at r5 (raw file):

// The hex string corresponding to b'CONTRACT_CLASS_LEAF_V0' in big-endian.
pub const CONTRACT_CLASS_LEAF_V0: Felt =
    Felt::from_hex_unchecked("0x434f4e54524143545f434c4153535f4c4541465f5630");

this move of constants should have been a separate PR - broad change (many small diffs in different places), very easy to LGTM quickly, and self-contained

Code quote:

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: Felt =
    Felt::from_hex_unchecked("0x434f4e54524143545f434c4153535f4c4541465f5630");

---

crates/starknet_committer/src/patricia_merkle_tree/leaf/leaf_impl.rs line 28 at r5 (raw file):

        self
    }
}

ditto

Code quote:

impl AsRef<ContractState> for ContractState {
    fn as_ref(&self) -> &ContractState {
        self
    }
}

---

crates/starknet_committer/src/patricia_merkle_tree/types.rs line 29 at r5 (raw file):

    fn as_ref(&self) -> &CompiledClassHash {
        self
    }

also, can't you derive_more::AsRef it?

Suggestion:

    fn as_ref(&self) -> &Self {
        self
    }

[ArielElp]

@ArielElp made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware).

---

crates/starknet_committer/src/patricia_merkle_tree/types.rs line 29 at r5 (raw file):

Previously, dorimedini-starkware wrote…

also, can't you derive_more::AsRef it?

Nope, derive_more is for trivial wrappers Outer(inner).

---

crates/starknet_committer/src/patricia_merkle_tree/leaf/leaf_impl.rs line 28 at r5 (raw file):

Previously, dorimedini-starkware wrote…

ditto

Same as above