starknet_committer,starknet_patricia: remove generic config

[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
• 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 👈 (View in Graphite)
• 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
• 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.

[dorimedini-starkware]

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

---

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

        Self { compare_modified_leaves: should_compare_modified_leaves }
    }
}

to make it clearer to the user, I would suggest this approach.
I don't think the user of this config should control the boolean directly - the user should know what the underlying DB layout is, not how to properly configure this struct

Suggestion:

impl OriginalSkeletonTrieConfig {
    pub fn new_for_facts_db() -> Self {
        Self { compare_modified_leaves: false }
    }

    pub fn new_for_node_index_db() -> Self {
        Self { compare_modified_leaves: true }
    }
}

---

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

}

pub(crate) struct OriginalSkeletonTrieDontCompareConfig;

why is this type needed...? seems redundant

Code quote:

OriginalSkeletonTrieDontCompareConfig;

[ArielElp]

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

---

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

Previously, dorimedini-starkware wrote…

to make it clearer to the user, I would suggest this approach.
I don't think the user of this config should control the boolean directly - the user should know what the underlying DB layout is, not how to properly configure this struct

It's not layout related, we use false for contracts trie where we still don't know about all the modified leaves (we don't yet have the new storage roots).

---

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

Previously, dorimedini-starkware wrote…

why is this type needed...? seems redundant

it is used in get_leaves in facts db, and up the stack also in create_contracts_trie

[dorimedini-starkware]

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

---

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

Previously, ArielElp wrote…

It's not layout related, we use false for contracts trie where we still don't know about all the modified leaves (we don't yet have the new storage roots).

ok, so new_for_contracts_trie and new_for_class_or_storage_trie then?

---

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

Previously, ArielElp wrote…

it is used in get_leaves in facts db, and up the stack also in create_contracts_trie

why not use OriginalSkeletonTrieConfig::new_X (where X depends on the context) up the stack?

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArielElp).

[dorimedini-starkware]

@dorimedini-starkware reviewed 7 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp).

[ArielElp]

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

---

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

Previously, dorimedini-starkware wrote…

ok, so new_for_contracts_trie and new_for_class_or_storage_trie then?

Done, but the latter is taken a should_warn_on_trivial_modifications_argument

I also ended up needing:

1. new_for_tests - there is at least one test where the value (compare_modified_leaves) is explicitly tested
2. default - get_leaves doesn't receive the config with should_warn, it's always false regardless of the trie we're querying

---

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

Previously, dorimedini-starkware wrote…

why not use OriginalSkeletonTrieConfig::new_X (where X depends on the context) up the stack?

Done.