starknet_committer,starknet_committer_and_os_cli,starknet_patricia: make FilledNode generic

[ArielElp]

No description provided.

[reviewable-StarkWare]

This change is 

[ArielElp]

• starknet_committer,starknet_patricia: move node_serde to facts db #11644
• starknet_committer: reorganize db modules #11640
• starknet_committer: add commit e2e tests #11331
• starknet_committer: index layout forest creation tests #11286
• starknet_committer,starknet_patricia: layout-based original tree creation tests #11236
• starknet_committer: add original tree creation test templates #11715
• starknet_committer,starknet_patricia: layout-dependent db-key separator #11641
• starknet_committer,starknet_patricia: remove FilledNode impl #11637
• starknet_committer: db layout trait #11621
• starknet_committer,starknet_patricia: layout-dependent FilledTree serialization #10988
• starknet_committer,starknet_patricia: refactor key context #11592
• starknet_committer,starknet_patricia: add layout for base trait #11566
• 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: enrich layout trait to abstract skeleton creation #11182
• starknet_committer: move trie creation logic to trie_traversal #11180
• 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 👈 (View in Graphite)
• 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 4 of 6 files at r1, all commit messages.
Reviewable status: 4 of 6 files reviewed, 5 unresolved discussions (waiting on @ArielElp and @yoavGrs)

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree_test.rs line 29 at r1 (raw file):

};

type FactDbFilledNode<L> = FilledNode<L, HashOutput>;

you already defined this alias. please delete here, define the alias as pub and use it here.

Code quote:

type FactDbFilledNode<L> = FilledNode<L, HashOutput>;

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/node_serde.rs line 58 at r1 (raw file):

impl<L: Leaf> HasDynamicPrefix for FilledNode<L, HashOutput> {
    type KeyContext = <L as HasStaticPrefix>::KeyContext;

Did you intentionally delete the comment?
// Inherit the KeyContext from the HasStaticPrefix implementation of the leaf.

Code quote:

impl<L: Leaf> HasDynamicPrefix for FilledNode<L, HashOutput> {
    type KeyContext = <L as HasStaticPrefix>::KeyContext;

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/node_serde.rs line 66 at r1 (raw file):

        .into()
    }
}

does that work? I don't see a specific usage of the HashOutput type here.

Suggestion:

impl<L: Leaf, ChildData> FilledNode<L, ChildData> {
    pub fn suffix(&self) -> [u8; SERIALIZE_HASH_BYTES] {
        self.hash.0.to_bytes_be()
    }

    pub fn db_key(&self, key_context: &<L as HasStaticPrefix>::KeyContext) -> DbKey {
        self.get_db_key(key_context, &self.suffix())
    }
}

impl<L: Leaf, ChildData> HasDynamicPrefix for FilledNode<L, ChildData> {
    type KeyContext = <L as HasStaticPrefix>::KeyContext;
    fn get_prefix(&self, _key_context: &Self::KeyContext) -> DbKeyPrefix {
        match &self.data {
            NodeData::Binary(_) | NodeData::Edge(_) => PatriciaPrefix::InnerNode,
            NodeData::Leaf(_) => PatriciaPrefix::Leaf(L::get_static_prefix(_key_context)),
        }
        .into()
    }
}

---

crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/node_serde.rs line 75 at r1 (raw file):

}

impl<L: Leaf> DBObject for FilledNode<L, HashOutput> {

use the alias you defined

Suggestion:

impl<L: Leaf> DBObject for FactDbFilledNode<L>

---

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

use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTree;

type FactDbFilledNode<L> = FilledNode<L, HashOutput>;

Please use this type wherever possible.
I actually think this should be defined somewhere in the facts_db module. is that possible? if currently not due to crate dependencies, please add a todo

Code quote:

type FactDbFilledNode<L> = FilledNode<L, HashOutput>;

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 of 6 files at r1, 5 of 6 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArielElp)

---

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

    pub tree_map: HashMap<NodeIndex, FactDbFilledNode<L>>,
    pub root_hash: HashOutput,
}

planning on making this generic...? or a separate type?

Code quote:

pub struct FilledTreeImpl<L: Leaf> {
    pub tree_map: HashMap<NodeIndex, FactDbFilledNode<L>>,
    pub root_hash: HashOutput,
}

[ArielElp]

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

---

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

Previously, dorimedini-starkware wrote…

planning on making this generic...? or a separate type?

Probably generic, but the code for serialization is mostly not written yet (there is an early draft)

[dorimedini-starkware]

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

---

crates/starknet_committer/src/db/facts_db/traversal.rs line 123 at r5 (raw file):

                    if !right_subtree.is_unmodified() {
                        next_subtrees.push(right_subtree);
                    }

move this to the prev PR

Code quote:

                    if !left_subtree.is_unmodified() {
                        next_subtrees.push(left_subtree);
                    }
                    if !right_subtree.is_unmodified() {
                        next_subtrees.push(right_subtree);
                    }

[ArielElp]

Reviewable status: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware, @nimrod-starkware, and @yoavGrs)

---

crates/starknet_committer/src/db/facts_db/traversal.rs line 123 at r5 (raw file):

Previously, dorimedini-starkware wrote…

move this to the prev PR

Done.

[dorimedini-starkware]

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

[dorimedini-starkware]

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

[graphite-app]

Merge activity

• Dec 16, 11:05 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
• Dec 16, 11:05 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

[dorimedini-starkware]

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