starknet_committer,starknet_patricia,starknet_patricia_storage: add context to prefix and serde

[ArielElp]

No description provided.

[reviewable-StarkWare]

This change is 

[ArielElp]

• 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
• 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 👈 (View in Graphite)
• 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 18 of 20 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 19 of 20 files reviewed, 5 unresolved discussions (waiting on @ArielElp)

---

crates/starknet_committer/src/db/facts_db/types.rs line 43 at r2 (raw file):

            PatriciaPrefix::InnerNode.into()
        }
    }

Please include this change in your previous PR, where you defined SubTreeTrait.

Code quote:

    fn get_root_prefix<L: Leaf>(
        &self,
        _key_context: &<L as HasStaticPrefix>::KeyContext,
    ) -> DbKeyPrefix {
        if self.is_leaf() {
            PatriciaPrefix::Leaf(L::get_static_prefix(_key_context)).into()
        } else {
            PatriciaPrefix::InnerNode.into()
        }
    }

---

crates/starknet_patricia_storage/src/db_object.rs line 5 at r2 (raw file):

pub trait HasDynamicPrefix {
    type KeyContext;

Please doc the type

Code quote:

type KeyContext;

---

crates/starknet_patricia_storage/src/db_object.rs line 22 at r2 (raw file):

    fn get_prefix(&self, key_context: &Self::KeyContext) -> DbKeyPrefix {
        T::get_static_prefix(key_context)
    }

new line + doc

Suggestion:

    /// ....
    type KeyContext = <T as HasStaticPrefix>::KeyContext;
    
    fn get_prefix(&self, key_context: &Self::KeyContext) -> DbKeyPrefix {
        T::get_static_prefix(key_context)
    }

---

crates/starknet_patricia_storage/src/db_object.rs line 32 at r2 (raw file):

        value: &DbValue,
        deserialize_context: &Self::DeserializeContext,
    ) -> Result<Self, DeserializationError>;

new line

Suggestion:

    fn serialize(&self) -> DbValue;
    
    fn deserialize(
        value: &DbValue,
        deserialize_context: &Self::DeserializeContext,
    ) -> Result<Self, DeserializationError>;

---

crates/starknet_patricia/src/patricia_merkle_tree/traversal.rs line 97 at r2 (raw file):

        &self,
        key_context: &<L as HasStaticPrefix>::KeyContext,
    ) -> DbKeyPrefix;

should be added in previous PR

Code quote:

    fn get_root_prefix<L: Leaf>(
        &self,
        key_context: &<L as HasStaticPrefix>::KeyContext,
    ) -> DbKeyPrefix;

[yoavGrs]

Reviewable status: 16 of 20 files reviewed, 6 unresolved discussions (waiting on @ArielElp and @nimrod-starkware)

---

a discussion (no related file):
Please break this PR.
Separate between adding the context type and the serde functions?

[ArielElp]

Reviewable status: 9 of 20 files reviewed, 6 unresolved discussions (waiting on @nimrod-starkware and @yoavGrs)

---

a discussion (no related file):

Previously, yoavGrs wrote…

Please break this PR.
Separate between adding the context type and the serde functions?

Done, this PR now only deals with KeyContext, next PR deals with serde

[ArielElp]

Reviewable status: 9 of 20 files reviewed, 6 unresolved discussions (waiting on @nimrod-starkware and @yoavGrs)

---

crates/starknet_committer/src/db/facts_db/types.rs line 43 at r2 (raw file):

Previously, nimrod-starkware wrote…

Please include this change in your previous PR, where you defined SubTreeTrait.

Done.

---

crates/starknet_patricia/src/patricia_merkle_tree/traversal.rs line 97 at r2 (raw file):

Previously, nimrod-starkware wrote…

should be added in previous PR

Done.

---

crates/starknet_patricia_storage/src/db_object.rs line 5 at r2 (raw file):

Previously, nimrod-starkware wrote…

Please doc the type

Done.

---

crates/starknet_patricia_storage/src/db_object.rs line 22 at r2 (raw file):

Previously, nimrod-starkware wrote…

new line + doc

Done.

---

crates/starknet_patricia_storage/src/db_object.rs line 32 at r2 (raw file):

Previously, nimrod-starkware wrote…

new line

moved to next PR (will fix there)

[dorimedini-starkware]

@dorimedini-starkware reviewed 4 of 4 files at r13, all commit messages.
Reviewable status: 21 of 22 files reviewed, 3 unresolved discussions (waiting on @ArielElp and @yoavGrs)

[ArielElp]

Reviewable status: 18 of 22 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)

---

crates/starknet_committer/src/db/external_test_utils.rs line 31 at r12 (raw file):

Previously, dorimedini-starkware wrote…

why do you force this test body to only work for leaves with empty key contexts?
Leaf already has a trait bound of HasStaticPrefix,
and I'm guessing create_original_skeleton_tree will need to be refactored to take key contexts as input anyway; how will this be done?

create_original_skeleton_tree will have a layout generic argument

Given it's only facts ATM I needed to limit it to Empty. In the PR that inttroduces layout, this test still only deals with facts (no PRs for tests yet), but this is easily changable there (tree_computation_flow will receive a layout arg, layout knows how to generate key context).

---

crates/starknet_committer/src/forest/filled_forest.rs line 36 at r12 (raw file):

Previously, dorimedini-starkware wrote…

how are the changes here related to this PR?

A gt sync accident, removed

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line at r12 (raw file):

Previously, dorimedini-starkware wrote…

how will you handle generalizing the code here?
if you plan on doing it in subsequent PRs, please at least add TODOs in this PR

In the top of the stack, code in this module (get_leaves closure) is only used by the os crates. Given that we don't serve storage proofs now (or store history), I think we don't need to generalize yet.

---

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

Previously, dorimedini-starkware wrote…

you will define different leaf types for indexed layout?

Yes, they will be trivial wrappers for different implementations of DBObject (which depends on HasDynamicPrefix, which will also be different).ff

[dorimedini-starkware]

@dorimedini-starkware reviewed 3 of 3 files at r14, 1 of 1 files at r15, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArielElp)

---

crates/starknet_committer/src/db/external_test_utils.rs line 31 at r12 (raw file):

Previously, ArielElp wrote…

create_original_skeleton_tree will have a layout generic argument

Given it's only facts ATM I needed to limit it to Empty. In the PR that inttroduces layout, this test still only deals with facts (no PRs for tests yet), but this is easily changable there (tree_computation_flow will receive a layout arg, layout knows how to generate key context).

please add a TODO here then

---

crates/starknet_committer/src/patricia_merkle_tree/tree.rs line at r12 (raw file):

Previously, ArielElp wrote…

In the top of the stack, code in this module (get_leaves closure) is only used by the os crates. Given that we don't serve storage proofs now (or store history), I think we don't need to generalize yet.

ok, please add some comment / docstring explaining why this code is layout-specific

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 of 1 files at r16, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

[ArielElp]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs line 45 at r12 (raw file):

Previously, dorimedini-starkware wrote…

consider this, if it reduces boilerplate (non-blocking)

Will consider when continuing the top of the stack with serialization.

[dorimedini-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs line 45 at r12 (raw file):

Previously, ArielElp wrote…

Will consider when continuing the top of the stack with serialization.

why not now? key context type is layout-dependent, not specific tree dependent

[ArielElp]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs line 45 at r12 (raw file):

Previously, dorimedini-starkware wrote…

why not now? key context type is layout-dependent, not specific tree dependent

Just because there will probably be more changes around that trait. Now Iv'e actually tried it and apparantly associated types defaults are not stable yet.

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 of 1 files at r17, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

[dorimedini-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)