From 835570a6e36de7213552ba74f0c6afb5ca34844a Mon Sep 17 00:00:00 2001 From: Tzahi Date: Wed, 24 Jul 2024 17:43:27 +0300 Subject: [PATCH 1/2] chore: committer mono_repo 02 - changes from 01 (#44) --- .github/workflows/committer_ci.yml | 142 +++++++++++++ Cargo.lock | 170 ++++++++++++++- Cargo.toml | 2 + crates/committer/Cargo.toml | 11 +- .../committer/src/block_committer/commit.rs | 54 ++--- crates/committer/src/block_committer/input.rs | 34 +-- crates/committer/src/felt.rs | 7 +- crates/committer/src/forest_errors.rs | 20 +- crates/committer/src/hash/hash_trait.rs | 3 +- .../src/patricia_merkle_tree/errors.rs | 7 +- .../external_test_utils.rs | 20 +- .../filled_tree/errors.rs | 14 +- .../filled_tree/forest.rs | 50 ++--- .../patricia_merkle_tree/filled_tree/node.rs | 4 +- .../filled_tree/node_serde.rs | 42 ++-- .../patricia_merkle_tree/filled_tree/tree.rs | 53 ++--- .../filled_tree/tree_test.rs | 164 +++++---------- .../internal_test_utils.rs | 107 ++++++++-- .../patricia_merkle_tree/node_data/errors.rs | 11 +- .../node_data/inner_node.rs | 43 ++-- .../node_data/inner_node_tests.rs | 4 +- .../patricia_merkle_tree/node_data/leaf.rs | 14 +- .../node_data/leaf_serde.rs | 43 ++-- .../node_data/leaf_serde_test.rs | 17 +- .../original_skeleton_tree/config.rs | 13 +- .../original_skeleton_tree/create_tree.rs | 154 ++++++-------- .../create_tree_test.rs | 135 +++++------- .../original_skeleton_tree/errors.rs | 10 +- .../original_skeleton_tree/skeleton_forest.rs | 58 +++--- .../skeleton_forest_test.rs | 60 +++--- .../original_skeleton_tree/tree.rs | 10 +- .../original_skeleton_tree/utils.rs | 10 +- .../original_skeleton_tree/utils_test.rs | 33 ++- .../src/patricia_merkle_tree/types.rs | 35 ++-- .../src/patricia_merkle_tree/types_test.rs | 27 +-- .../create_tree_helper.rs | 71 +++---- .../create_tree_helper_test.rs | 68 +++--- .../updated_skeleton_tree/hash_function.rs | 23 +- .../hash_function_test.rs | 27 +-- .../updated_skeleton_tree/skeleton_forest.rs | 16 +- .../updated_skeleton_tree/tree.rs | 8 +- .../updated_skeleton_tree/tree_test.rs | 32 +-- crates/committer/src/storage/db_object.rs | 17 +- crates/committer/src/storage/errors.rs | 9 +- crates/committer/src/storage/map_storage.rs | 3 +- crates/committer/src/storage/storage_trait.rs | 17 +- crates/committer_cli/Cargo.toml | 15 +- .../committer_cli/benches/committer_bench.rs | 56 ++--- crates/committer_cli/src/block_hash.rs | 11 +- crates/committer_cli/src/commands.rs | 25 +-- .../src/filled_tree_output/errors.rs | 7 +- crates/committer_cli/src/main.rs | 18 +- crates/committer_cli/src/parse_input/cast.rs | 25 ++- .../src/parse_input/raw_input.rs | 42 +++- crates/committer_cli/src/parse_input/read.rs | 6 +- .../src/parse_input/read_test.rs | 44 ++-- .../src/tests/flow_test_files_prefix | 2 +- .../committer_cli/src/tests/python_tests.rs | 196 +++++++----------- .../src/tests/regression_tests.rs | 85 ++++---- .../committer_cli/src/tests/utils/objects.rs | 40 ++-- .../src/tests/utils/parse_from_python.rs | 29 +-- .../src/tests/utils/random_structs.rs | 74 +++---- 62 files changed, 1329 insertions(+), 1218 deletions(-) create mode 100644 .github/workflows/committer_ci.yml diff --git a/.github/workflows/committer_ci.yml b/.github/workflows/committer_ci.yml new file mode 100644 index 0000000000..35511cb264 --- /dev/null +++ b/.github/workflows/committer_ci.yml @@ -0,0 +1,142 @@ +name: Committer-CI + +on: + push: + branches: + - main + - main-v[0-9].** + tags: + - v[0-9].** + paths: + - 'crates/committer/**' + - 'crates/committer_cli/**' + + pull_request: + types: + - opened + - reopened + - synchronize + - auto_merge_enabled + - edited + paths: + - 'crates/committer/**' + - 'crates/committer_cli/**' + +jobs: + run-regression-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + + - id: auth + uses: "google-github-actions/auth@v2" + with: + credentials_json: ${{ secrets.COMMITER_PRODUCTS_EXT_WRITER_JSON }} + - uses: 'google-github-actions/setup-gcloud@v2' + - run: echo "BENCH_INPUT_FILES_PREFIX=$(cat ./crates/committer_cli/src/tests/flow_test_files_prefix)" >> $GITHUB_ENV + - run: gcloud storage cp -r gs://committer-testing-artifacts/$BENCH_INPUT_FILES_PREFIX/* ./crates/committer_cli/benches + - run: cargo test --release -- --include-ignored test_regression + + benchmarking: + runs-on: ubuntu-latest + steps: + # Checkout the base branch to get the old code. + - uses: actions/checkout@v4 + with: + ref: ${{ github.base_ref }} + - uses: Swatinem/rust-cache@v2 + + # Download the old benchmark inputs. + - id: auth + uses: "google-github-actions/auth@v2" + with: + credentials_json: ${{ secrets.COMMITER_PRODUCTS_EXT_WRITER_JSON }} + - uses: 'google-github-actions/setup-gcloud@v2' + - run: echo "OLD_BENCH_INPUT_FILES_PREFIX=$(cat ./crates/committer_cli/src/tests/flow_test_files_prefix)" >> $GITHUB_ENV + - run: gcloud storage cp -r gs://committer-testing-artifacts/$OLD_BENCH_INPUT_FILES_PREFIX/* ./crates/committer_cli/benches + + # List the existing benchmarks. + - run: | + cargo bench -- --list | grep ': benchmark$' | sed -e "s/: benchmark$//" > benchmarks_list.txt + + # Benchmark the old code. + - run: cargo bench + + # Backup the downloaded files to avoid re-downloading them if they didn't change (overwritten by checkout). + - run: mv ./crates/committer_cli/benches/tree_flow_inputs.json ./crates/committer_cli/benches/tree_flow_inputs.json_bu + - run: mv ./crates/committer_cli/benches/committer_flow_inputs.json ./crates/committer_cli/benches/committer_flow_inputs.json_bu + + # Checkout the new code. + - uses: actions/checkout@v4 + with: + clean: false + - run: echo "NEW_BENCH_INPUT_FILES_PREFIX=$(cat ./crates/committer_cli/src/tests/flow_test_files_prefix)" >> $GITHUB_ENV + + # Input files didn't change. + - if: env.OLD_BENCH_INPUT_FILES_PREFIX == env.NEW_BENCH_INPUT_FILES_PREFIX + run: | + mv ./crates/committer_cli/benches/tree_flow_inputs.json_bu ./crates/committer_cli/benches/tree_flow_inputs.json + mv ./crates/committer_cli/benches/committer_flow_inputs.json_bu ./crates/committer_cli/benches/committer_flow_inputs.json + + # Input files did change, download new inputs. + - if: env.OLD_BENCH_INPUT_FILES_PREFIX != env.NEW_BENCH_INPUT_FILES_PREFIX + run: | + gcloud storage cp -r gs://committer-testing-artifacts/$NEW_BENCH_INPUT_FILES_PREFIX/* ./crates/committer_cli/benches + + # Benchmark the new code, splitting the benchmarks, and prepare the results for posting a comment. + - run: bash ./crates/committer_cli/benches/bench_split_and_prepare_post.sh benchmarks_list.txt bench_new.txt + + - run: echo BENCHES_RESULT=$(cat bench_new.txt) >> $GITHUB_ENV + + # Post comment in case of performance regression or improvement. + - run: npm install fs + - if: contains(env.BENCHES_RESULT, 'regressed') || contains(env.BENCHES_RESULT, 'improved') + uses: actions/github-script@v6 + with: + script: | + const fs = require('fs') + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: fs.readFileSync('bench_new.txt', 'utf8'), + path: 'Commits' + }) + + gcs-push: + runs-on: ubuntu-20.04 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + + # Commit hash on pull request event would be the head commit of the branch. + - name: Get commit hash prefix for PR update + if: ${{ github.event_name == 'pull_request' }} + env: + COMMIT_SHA: ${{ github.event.pull_request.head.sha }} + run: echo "SHORT_HASH=${COMMIT_SHA:0:7}" >> $GITHUB_ENV + + # On push event (to main, for example) we should take the commit post-push. + - name: Get commit hash prefix for merge + if: ${{ github.event_name != 'pull_request' }} + env: + COMMIT_SHA: ${{ github.event.after }} + run: echo "SHORT_HASH=${COMMIT_SHA:0:7}" >> $GITHUB_ENV + + - name: Build CLI binary + run: cargo build -p committer_cli -r --bin committer_cli --target-dir CLI_TARGET + + - id: auth + uses: "google-github-actions/auth@v2" + with: + credentials_json: ${{ secrets.COMMITER_PRODUCTS_EXT_WRITER_JSON }} + + - name: Upload binary to GCP + id: upload_file + uses: "google-github-actions/upload-cloud-storage@v2" + with: + path: "CLI_TARGET/release/committer_cli" + destination: "committer-products-external/${{ env.SHORT_HASH }}/release/" \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 847c1ccb2b..a67d9b80d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -106,6 +106,12 @@ dependencies = [ "libc", ] +[[package]] +name = "anes" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" + [[package]] name = "anstream" version = "0.6.14" @@ -535,6 +541,17 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "async-recursion" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.61", +] + [[package]] name = "async-signal" version = "0.2.6" @@ -976,7 +993,7 @@ dependencies = [ "cairo-lang-starknet-classes", "cairo-lang-utils", "cairo-vm", - "criterion", + "criterion 0.3.6", "derive_more", "glob", "indexmap 2.2.6", @@ -1772,6 +1789,33 @@ dependencies = [ "windows-targets 0.52.5", ] +[[package]] +name = "ciborium" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42e69ffd6f0917f5c029256a24d0161db17cea3997d185db0d35926308770f0e" +dependencies = [ + "ciborium-io", + "ciborium-ll", + "serde", +] + +[[package]] +name = "ciborium-io" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05afea1e0a06c9be33d539b876f1ce3692f4afea2cb41f740e7743225ed1c757" + +[[package]] +name = "ciborium-ll" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57663b653d948a338bfb3eeba9bb2fd5fcfaecb9e199e87e1eda4d9e8b240fd9" +dependencies = [ + "ciborium-io", + "half 2.4.1", +] + [[package]] name = "cipher" version = "0.4.4" @@ -1922,6 +1966,53 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "committer" +version = "0.1.0-rc.0" +dependencies = [ + "async-recursion", + "derive_more", + "ethnum", + "hex", + "log", + "pretty_assertions", + "rand 0.8.5", + "rstest", + "serde", + "serde_json", + "starknet-types-core", + "strum 0.25.0", + "strum_macros 0.25.3", + "thiserror", + "tokio", +] + +[[package]] +name = "committer_cli" +version = "0.1.0-rc.0" +dependencies = [ + "clap 4.5.4", + "committer", + "criterion 0.5.1", + "derive_more", + "ethnum", + "indexmap 2.2.6", + "log", + "pretty_assertions", + "rand 0.8.5", + "rand_distr", + "serde", + "serde_json", + "serde_repr", + "simplelog", + "starknet-types-core", + "starknet_api", + "strum 0.25.0", + "strum_macros 0.25.3", + "thiserror", + "tokio", +] + [[package]] name = "concurrent-queue" version = "2.5.0" @@ -2089,7 +2180,7 @@ dependencies = [ "atty", "cast", "clap 2.34.0", - "criterion-plot", + "criterion-plot 0.4.5", "csv", "itertools 0.10.5", "lazy_static", @@ -2106,6 +2197,32 @@ dependencies = [ "walkdir", ] +[[package]] +name = "criterion" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2b12d017a929603d80db1831cd3a24082f8137ce19c69e6447f54f5fc8d692f" +dependencies = [ + "anes", + "cast", + "ciborium", + "clap 4.5.4", + "criterion-plot 0.5.0", + "is-terminal", + "itertools 0.10.5", + "num-traits 0.2.19", + "once_cell", + "oorandom", + "plotters", + "rayon", + "regex", + "serde", + "serde_derive", + "serde_json", + "tinytemplate", + "walkdir", +] + [[package]] name = "criterion-plot" version = "0.4.5" @@ -2116,6 +2233,16 @@ dependencies = [ "itertools 0.10.5", ] +[[package]] +name = "criterion-plot" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1" +dependencies = [ + "cast", + "itertools 0.10.5", +] + [[package]] name = "crossbeam-deque" version = "0.8.5" @@ -3034,6 +3161,12 @@ dependencies = [ "yansi", ] +[[package]] +name = "ethnum" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b90ca2580b73ab6a1f724b76ca11ab632df820fd6040c336200d2c1df7b3c82c" + [[package]] name = "event-listener" version = "2.5.3" @@ -3676,6 +3809,16 @@ version = "1.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b43ede17f21864e81be2fa654110bf1e793774238d86ef8555c37e6519c0403" +[[package]] +name = "half" +version = "2.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dd08c532ae367adf81c312a4580bc67f1d0fe8bc9c460520283f4c0ff277888" +dependencies = [ + "cfg-if", + "crunchy", +] + [[package]] name = "hashbrown" version = "0.12.3" @@ -4243,6 +4386,17 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" +[[package]] +name = "is-terminal" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f23ff5ef2b80d608d61efee834934d862cd92461afc0560dedf493e4c033738b" +dependencies = [ + "hermit-abi 0.3.9", + "libc", + "windows-sys 0.52.0", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.0" @@ -7470,6 +7624,16 @@ dependencies = [ "getrandom", ] +[[package]] +name = "rand_distr" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32cb0b9bc82b0a0876c2dd994a7e7a2683d3e7390ca40e6886785ef0c7e3ee31" +dependencies = [ + "num-traits 0.2.19", + "rand 0.8.5", +] + [[package]] name = "rand_hc" version = "0.1.0" @@ -8222,7 +8386,7 @@ version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2bef2ebfde456fb76bbcf9f59315333decc4fda0b2b44b420243c11e0f5ec1f5" dependencies = [ - "half", + "half 1.8.3", "serde", ] diff --git a/Cargo.toml b/Cargo.toml index c042bcd341..9f74931072 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,8 @@ resolver = "2" members = [ "crates/blockifier", + "crates/committer", + "crates/committer_cli", "crates/gateway", "crates/mempool", "crates/mempool_infra", diff --git a/crates/committer/Cargo.toml b/crates/committer/Cargo.toml index 6a31c49470..b15a64288e 100644 --- a/crates/committer/Cargo.toml +++ b/crates/committer/Cargo.toml @@ -19,15 +19,20 @@ rstest.workspace = true [dependencies] async-recursion.workspace = true -bisection.workspace = true derive_more.workspace = true ethnum.workspace = true hex.workspace = true log.workspace = true rand.workspace = true -serde.workspace = true -serde_json.workspace = true +serde = { version = "1.0.197", features = ["derive"] } +serde_json = "1.0.116" starknet-types-core.workspace = true +strum.workspace = true strum_macros.workspace = true thiserror.workspace = true tokio.workspace = true + +# Optional dependencies required for tests and the testing feature. +# See [here](https://github.com/bnjbvr/cargo-machete/issues/128). +[package.metadata.cargo-machete] +ignored = ["strum"] diff --git a/crates/committer/src/block_committer/commit.rs b/crates/committer/src/block_committer/commit.rs index f5faa50950..a80806e22d 100644 --- a/crates/committer/src/block_committer/commit.rs +++ b/crates/committer/src/block_committer/commit.rs @@ -1,19 +1,17 @@ -use log::warn; use std::collections::HashMap; +use log::warn; + use crate::block_committer::errors::BlockCommitmentError; -use crate::block_committer::input::Config; -use crate::block_committer::input::ConfigImpl; -use crate::block_committer::input::ContractAddress; -use crate::block_committer::input::Input; -use crate::block_committer::input::StateDiff; +use crate::block_committer::input::{Config, ConfigImpl, ContractAddress, Input, StateDiff}; use crate::patricia_merkle_tree::filled_tree::forest::FilledForest; use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, Nonce}; use crate::patricia_merkle_tree::node_data::leaf::ContractState; -use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::ForestSortedIndices; -use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::OriginalSkeletonForest; -use crate::patricia_merkle_tree::types::NodeIndex; -use crate::patricia_merkle_tree::types::SortedLeafIndices; +use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::{ + ForestSortedIndices, + OriginalSkeletonForest, +}; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices}; use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl; use crate::patricia_merkle_tree::updated_skeleton_tree::skeleton_forest::UpdatedSkeletonForest; use crate::storage::map_storage::MapStorage; @@ -31,11 +29,14 @@ pub async fn commit_block(input: Input) -> BlockCommitmentResult) -> BlockCommitmentResult( updated_forest, - input.state_diff.actual_storage_updates(), - input.state_diff.actual_classes_updates(), + actual_storage_updates, + actual_classes_updates, &original_contracts_trie_leaves, &input.state_diff.address_to_class_hash, &input.state_diff.address_to_nonce, @@ -80,10 +81,7 @@ fn check_trivial_nonce_and_class_hash_updates( .get(&NodeIndex::from_contract_address(address)) .is_some_and(|previous_contract_state| previous_contract_state.nonce == *nonce) { - warn!( - "Encountered a trivial nonce update of contract {:?}", - address - ) + warn!("Encountered a trivial nonce update of contract {:?}", address) } } @@ -94,10 +92,7 @@ fn check_trivial_nonce_and_class_hash_updates( previous_contract_state.class_hash == *class_hash }) { - warn!( - "Encountered a trivial class hash update of contract {:?}", - address - ) + warn!("Encountered a trivial class hash update of contract {:?}", address) } } } @@ -109,11 +104,7 @@ type ClassesTrieIndices = Vec; /// Returns all modified indices in the given state diff. pub(crate) fn get_all_modified_indices( state_diff: &StateDiff, -) -> ( - StorageTriesIndices, - ContractsTrieIndices, - ClassesTrieIndices, -) { +) -> (StorageTriesIndices, ContractsTrieIndices, ClassesTrieIndices) { let accessed_addresses = state_diff.accessed_addresses(); let contracts_trie_indices: Vec = accessed_addresses .iter() @@ -128,18 +119,11 @@ pub(crate) fn get_all_modified_indices( .iter() .map(|address| { let indices: Vec = match state_diff.storage_updates.get(address) { - Some(updates) => updates - .keys() - .map(NodeIndex::from_starknet_storage_key) - .collect(), + Some(updates) => updates.keys().map(NodeIndex::from_starknet_storage_key).collect(), None => Vec::new(), }; (**address, indices) }) .collect(); - ( - storage_tries_indices, - contracts_trie_indices, - classes_trie_indices, - ) + (storage_tries_indices, contracts_trie_indices, classes_trie_indices) } diff --git a/crates/committer/src/block_committer/input.rs b/crates/committer/src/block_committer/input.rs index 6bb207d907..18023bfe45 100644 --- a/crates/committer/src/block_committer/input.rs +++ b/crates/committer/src/block_committer/input.rs @@ -1,18 +1,22 @@ +use std::collections::{HashMap, HashSet}; +use std::fmt::Debug; + +use log::LevelFilter; + use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; use crate::patricia_merkle_tree::node_data::leaf::{LeafModifications, SkeletonLeaf}; use crate::patricia_merkle_tree::types::NodeIndex; use crate::storage::storage_trait::{StorageKey, StorageValue}; -use std::collections::{HashMap, HashSet}; -use std::fmt::Debug; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] // TODO(Nimrod, 1/6/2025): Use the ContractAddress defined in starknet-types-core when available. pub struct ContractAddress(pub Felt); #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -// TODO(Nimrod, 1/6/2025): Use the StarknetStorageValue defined in starknet-types-core when available. +// TODO(Nimrod, 1/6/2025): Use the StarknetStorageValue defined in starknet-types-core when +// available. pub struct StarknetStorageKey(pub Felt); #[derive(Clone, Copy, Default, Debug, Eq, PartialEq)] @@ -33,24 +37,30 @@ pub trait Config: Debug + Eq + PartialEq { /// If the configuration is set, it requires that the storage will contain the original data for /// the modified leaves. Otherwise, it is not required. fn warn_on_trivial_modifications(&self) -> bool; + + /// Indicates from which log level output should be printed out to console. + fn logger_level(&self) -> LevelFilter; } #[derive(Debug, Eq, PartialEq)] pub struct ConfigImpl { warn_on_trivial_modifications: bool, + log_level: LevelFilter, } impl Config for ConfigImpl { fn warn_on_trivial_modifications(&self) -> bool { self.warn_on_trivial_modifications } + + fn logger_level(&self) -> LevelFilter { + self.log_level + } } impl ConfigImpl { - pub fn new(warn_on_trivial_modifications: bool) -> Self { - Self { - warn_on_trivial_modifications, - } + pub fn new(warn_on_trivial_modifications: bool, log_level: LevelFilter) -> Self { + Self { warn_on_trivial_modifications, log_level } } } @@ -85,10 +95,7 @@ impl StateDiff { Some(inner_updates) => inner_updates .iter() .map(|(key, value)| { - ( - NodeIndex::from_starknet_storage_key(key), - SkeletonLeaf::from(value.0), - ) + (NodeIndex::from_starknet_storage_key(key), SkeletonLeaf::from(value.0)) }) .collect(), None => HashMap::new(), @@ -102,10 +109,7 @@ impl StateDiff { self.class_hash_to_compiled_class_hash .iter() .map(|(class_hash, compiled_class_hash)| { - ( - NodeIndex::from_class_hash(class_hash), - SkeletonLeaf::from(compiled_class_hash.0), - ) + (NodeIndex::from_class_hash(class_hash), SkeletonLeaf::from(compiled_class_hash.0)) }) .collect() } diff --git a/crates/committer/src/felt.rs b/crates/committer/src/felt.rs index 85b9b1e970..e31e1a31e0 100644 --- a/crates/committer/src/felt.rs +++ b/crates/committer/src/felt.rs @@ -1,4 +1,5 @@ use core::fmt; + use ethnum::U256; use serde::{Deserialize, Serialize}; use starknet_types_core::felt::{Felt as StarknetTypesFelt, FromStrError}; @@ -121,10 +122,6 @@ impl Felt { // Convert to a 64-character hexadecimal string without the "0x" prefix. pub fn to_fixed_hex_string(&self) -> String { // Zero-pad the remaining string - self.0 - .to_fixed_hex_string() - .strip_prefix("0x") - .unwrap_or("0") - .to_string() + self.0.to_fixed_hex_string().strip_prefix("0x").unwrap_or("0").to_string() } } diff --git a/crates/committer/src/forest_errors.rs b/crates/committer/src/forest_errors.rs index 168ccad0ed..36549bfd53 100644 --- a/crates/committer/src/forest_errors.rs +++ b/crates/committer/src/forest_errors.rs @@ -1,13 +1,15 @@ +use thiserror::Error; +use tokio::task::JoinError; + use crate::block_committer::input::ContractAddress; use crate::patricia_merkle_tree::filled_tree::errors::{ - ClassesTrieError, ContractsTrieError, StorageTrieError, + ClassesTrieError, + ContractsTrieError, + StorageTrieError, }; use crate::patricia_merkle_tree::original_skeleton_tree::errors::OriginalSkeletonTreeError; use crate::patricia_merkle_tree::updated_skeleton_tree::errors::UpdatedSkeletonTreeError; -use thiserror::Error; -use tokio::task::JoinError; - pub(crate) type ForestResult = Result; #[derive(Debug, Error)] @@ -24,11 +26,17 @@ pub enum ForestError { ContractsTrie(#[from] ContractsTrieError), #[error("Missing input: Couldn't find the storage trie's current state of address {0:?}")] MissingContractCurrentState(ContractAddress), - #[error("Can't build storage trie's updated skeleton, because there is no original skeleton at address {0:?}")] + #[error( + "Can't build storage trie's updated skeleton, because there is no original skeleton at \ + address {0:?}" + )] MissingOriginalSkeleton(ContractAddress), #[error("Can't fill storage trie, because there is no updated skeleton at address {0:?}")] MissingUpdatedSkeleton(ContractAddress), - #[error("Can't build storage trie, because there are no sorted leaf indices of the contract at address {0:?}")] + #[error( + "Can't build storage trie, because there are no sorted leaf indices of the contract at \ + address {0:?}" + )] MissingSortedLeafIndices(ContractAddress), #[error(transparent)] JoinError(#[from] JoinError), diff --git a/crates/committer/src/hash/hash_trait.rs b/crates/committer/src/hash/hash_trait.rs index 43dc4f2ed6..c7ace47813 100644 --- a/crates/committer/src/hash/hash_trait.rs +++ b/crates/committer/src/hash/hash_trait.rs @@ -1,6 +1,7 @@ use starknet_types_core::felt::FromStrError; -use crate::{felt::Felt, impl_from_hex_for_felt_wrapper}; +use crate::felt::Felt; +use crate::impl_from_hex_for_felt_wrapper; #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] pub struct HashOutput(pub Felt); diff --git a/crates/committer/src/patricia_merkle_tree/errors.rs b/crates/committer/src/patricia_merkle_tree/errors.rs index e157db8934..334e6260d6 100644 --- a/crates/committer/src/patricia_merkle_tree/errors.rs +++ b/crates/committer/src/patricia_merkle_tree/errors.rs @@ -1,12 +1,9 @@ use std::fmt::Debug; + use thiserror::Error; #[derive(Debug, Error)] pub enum TypesError { #[error("Failed to convert type {from:?} to {to}. Reason: {reason}.")] - ConversionError { - from: T, - to: &'static str, - reason: &'static str, - }, + ConversionError { from: T, to: &'static str, reason: &'static str }, } diff --git a/crates/committer/src/patricia_merkle_tree/external_test_utils.rs b/crates/committer/src/patricia_merkle_tree/external_test_utils.rs index cae0132540..213cdc98e0 100644 --- a/crates/committer/src/patricia_merkle_tree/external_test_utils.rs +++ b/crates/committer/src/patricia_merkle_tree/external_test_utils.rs @@ -2,22 +2,21 @@ use std::collections::HashMap; use std::sync::Arc; use ethnum::U256; -use serde_json::json; - -use crate::block_committer::input::StarknetStorageValue; -use crate::felt::Felt; -use crate::hash::hash_trait::HashOutput; -use crate::patricia_merkle_tree::errors::TypesError; -use crate::storage::map_storage::MapStorage; use rand::Rng; +use serde_json::json; use super::filled_tree::tree::{FilledTree, StorageTrie}; -use super::node_data::leaf::{LeafData, LeafModifications, SkeletonLeaf}; +use super::node_data::leaf::{Leaf, LeafModifications, SkeletonLeaf}; use super::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig; use super::original_skeleton_tree::tree::{OriginalSkeletonTree, OriginalSkeletonTreeImpl}; use super::types::{NodeIndex, SortedLeafIndices}; use super::updated_skeleton_tree::hash_function::TreeHashFunctionImpl; use super::updated_skeleton_tree::tree::{UpdatedSkeletonTree, UpdatedSkeletonTreeImpl}; +use crate::block_committer::input::StarknetStorageValue; +use crate::felt::Felt; +use crate::hash::hash_trait::HashOutput; +use crate::patricia_merkle_tree::errors::TypesError; +use crate::storage::map_storage::MapStorage; impl TryFrom<&U256> for Felt { type Error = TypesError; @@ -55,10 +54,7 @@ pub fn get_random_u256(rng: &mut R, low: U256, high: U256) -> U256 { // 2. high_of_high == high_of_low + 1, and every possible low 128 bits value is valid either // when the high bits equal high_of_high, or when they equal high_of_low). let mut randomize = || { - U256::from_words( - rng.gen_range(*high_of_low..=*high_of_high), - rng.gen_range(0..=u128::MAX), - ) + U256::from_words(rng.gen_range(*high_of_low..=*high_of_high), rng.gen_range(0..=u128::MAX)) }; let mut result = randomize(); while result < low || result >= high { diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/errors.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/errors.rs index c611d95353..7bc74950c6 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/errors.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/errors.rs @@ -1,22 +1,18 @@ use tokio::task::JoinError; use crate::block_committer::input::StarknetStorageValue; -use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use crate::patricia_merkle_tree::filled_tree::node::FilledNode; +use crate::patricia_merkle_tree::filled_tree::node::{CompiledClassHash, FilledNode}; use crate::patricia_merkle_tree::node_data::errors::LeafError; -use crate::patricia_merkle_tree::node_data::leaf::ContractState; +use crate::patricia_merkle_tree::node_data::leaf::{ContractState, Leaf}; +use crate::patricia_merkle_tree::types::NodeIndex; use crate::patricia_merkle_tree::updated_skeleton_tree::errors::UpdatedSkeletonTreeError; -use crate::patricia_merkle_tree::{node_data::leaf::LeafData, types::NodeIndex}; #[derive(thiserror::Error, Debug)] -pub enum FilledTreeError { +pub enum FilledTreeError { #[error("Deleted leaf at index {0:?} appears in the updated skeleton tree.")] DeletedLeafInSkeleton(NodeIndex), #[error("Double update at node {index:?}. Existing value: {existing_value:?}.")] - DoubleUpdate { - index: NodeIndex, - existing_value: Box>, - }, + DoubleUpdate { index: NodeIndex, existing_value: Box> }, #[error(transparent)] Leaf(#[from] LeafError), #[error("Missing node at index {0:?}.")] diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/forest.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/forest.rs index 0c3c16e414..9dc41ea72b 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/forest.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/forest.rs @@ -1,11 +1,18 @@ +use std::collections::HashMap; +use std::sync::Arc; + +use tokio::task::JoinSet; + use crate::block_committer::input::{ContractAddress, StarknetStorageValue}; use crate::forest_errors::{ForestError, ForestResult}; use crate::hash::hash_trait::HashOutput; -use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, Nonce}; -use crate::patricia_merkle_tree::filled_tree::tree::FilledTree; +use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; use crate::patricia_merkle_tree::filled_tree::tree::{ - ClassesTrie, ContractsTrie, StorageTrie, StorageTrieMap, + ClassesTrie, + ContractsTrie, + FilledTree, + StorageTrie, + StorageTrieMap, }; use crate::patricia_merkle_tree::node_data::leaf::{ContractState, LeafModifications}; use crate::patricia_merkle_tree::types::NodeIndex; @@ -14,10 +21,6 @@ use crate::patricia_merkle_tree::updated_skeleton_tree::skeleton_forest::Updated use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl; use crate::storage::storage_trait::Storage; -use std::collections::HashMap; -use std::sync::Arc; -use tokio::task::JoinSet; - pub struct FilledForest { pub storage_tries: StorageTrieMap, pub contracts_trie: ContractsTrie, @@ -55,15 +58,13 @@ impl FilledForest { address_to_class_hash: &HashMap, address_to_nonce: &HashMap, ) -> ForestResult { - let classes_trie = ClassesTrie::create::( + let classes_trie_task = tokio::spawn(ClassesTrie::create::( Arc::new(updated_forest.classes_trie), Arc::new(classes_updates), - ) - .await?; - + )); let mut contracts_trie_modifications = HashMap::new(); let mut filled_storage_tries = HashMap::new(); - let mut tasks = JoinSet::new(); + let mut contracts_state_tasks = JoinSet::new(); for (address, inner_updates) in storage_updates { let updated_storage_trie = updated_forest @@ -74,11 +75,9 @@ impl FilledForest { let original_contract_state = original_contracts_trie_leaves .get(&NodeIndex::from_contract_address(&address)) .ok_or(ForestError::MissingContractCurrentState(address))?; - tasks.spawn(Self::new_contract_state::( + contracts_state_tasks.spawn(Self::new_contract_state::( address, - *(address_to_nonce - .get(&address) - .unwrap_or(&original_contract_state.nonce)), + *(address_to_nonce.get(&address).unwrap_or(&original_contract_state.nonce)), *(address_to_class_hash .get(&address) .unwrap_or(&original_contract_state.class_hash)), @@ -87,25 +86,22 @@ impl FilledForest { )); } - while let Some(result) = tasks.join_next().await { + while let Some(result) = contracts_state_tasks.join_next().await { let (address, new_contract_state, filled_storage_trie) = result??; - contracts_trie_modifications.insert( - NodeIndex::from_contract_address(&address), - new_contract_state, - ); + contracts_trie_modifications + .insert(NodeIndex::from_contract_address(&address), new_contract_state); filled_storage_tries.insert(address, filled_storage_trie); } - let contracts_trie = ContractsTrie::create::( + let contracts_trie_task = tokio::spawn(ContractsTrie::create::( Arc::new(updated_forest.contracts_trie), Arc::new(contracts_trie_modifications), - ) - .await?; + )); Ok(Self { storage_tries: filled_storage_tries, - contracts_trie, - classes_trie, + contracts_trie: contracts_trie_task.await??, + classes_trie: classes_trie_task.await??, }) } diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/node.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/node.rs index b83190292a..116209c558 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/node.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/node.rs @@ -4,7 +4,7 @@ use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::impl_from_hex_for_felt_wrapper; use crate::patricia_merkle_tree::node_data::inner_node::NodeData; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; +use crate::patricia_merkle_tree::node_data::leaf::Leaf; // TODO(Nimrod, 1/6/2024): Use the ClassHash defined in starknet-types-core when available. @@ -24,7 +24,7 @@ impl_from_hex_for_felt_wrapper!(CompiledClassHash); #[derive(Clone, Debug, PartialEq, Eq)] /// A node in a Patricia-Merkle tree which was modified during an update. -pub struct FilledNode { +pub struct FilledNode { pub hash: HashOutput, pub data: NodeData, } diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/node_serde.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/node_serde.rs index 3af27fd14d..fc7295fd7c 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/node_serde.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/node_serde.rs @@ -1,15 +1,20 @@ +use ethnum::U256; +use serde::{Deserialize, Serialize}; + use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::FilledNode; use crate::patricia_merkle_tree::node_data::inner_node::{ - BinaryData, EdgeData, EdgePathLength, NodeData, PathToBottom, + BinaryData, + EdgeData, + EdgePathLength, + NodeData, + PathToBottom, }; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; +use crate::patricia_merkle_tree::node_data::leaf::Leaf; use crate::storage::db_object::DBObject; use crate::storage::errors::DeserializationError; -use crate::storage::storage_trait::{StorageKey, StoragePrefix, StorageValue}; -use ethnum::U256; -use serde::{Deserialize, Serialize}; +use crate::storage::storage_trait::{StarknetPrefix, StorageKey, StorageValue}; // Const describe the size of the serialized node. pub(crate) const SERIALIZE_HASH_BYTES: usize = 32; @@ -27,7 +32,7 @@ pub(crate) struct LeafCompiledClassToSerialize { pub(crate) compiled_class_hash: Felt, } -impl FilledNode { +impl FilledNode { pub fn suffix(&self) -> [u8; SERIALIZE_HASH_BYTES] { self.hash.0.to_bytes_be() } @@ -37,17 +42,14 @@ impl FilledNode { } } -impl DBObject for FilledNode { +impl DBObject for FilledNode { /// This method serializes the filled node into a byte vector, where: /// - For binary nodes: Concatenates left and right hashes. /// - For edge nodes: Concatenates bottom hash, path, and path length. /// - For leaf nodes: use leaf.serialize() method. fn serialize(&self) -> StorageValue { match &self.data { - NodeData::Binary(BinaryData { - left_hash, - right_hash, - }) => { + NodeData::Binary(BinaryData { left_hash, right_hash }) => { // Serialize left and right hashes to byte arrays. let left: [u8; SERIALIZE_HASH_BYTES] = left_hash.0.to_bytes_be(); let right: [u8; SERIALIZE_HASH_BYTES] = right_hash.0.to_bytes_be(); @@ -57,10 +59,7 @@ impl DBObject for FilledNode { StorageValue(serialized) } - NodeData::Edge(EdgeData { - bottom_hash, - path_to_bottom, - }) => { + NodeData::Edge(EdgeData { bottom_hash, path_to_bottom }) => { // Serialize bottom hash, path, and path length to byte arrays. let bottom: [u8; SERIALIZE_HASH_BYTES] = bottom_hash.0.to_bytes_be(); let path: [u8; SERIALIZE_HASH_BYTES] = @@ -76,15 +75,17 @@ impl DBObject for FilledNode { } } - fn get_prefix(&self) -> StoragePrefix { + fn get_prefix(&self) -> Vec { match &self.data { - NodeData::Binary(_) | NodeData::Edge(_) => StoragePrefix::InnerNode, + NodeData::Binary(_) | NodeData::Edge(_) => { + StarknetPrefix::InnerNode.to_storage_prefix() + } NodeData::Leaf(leaf_data) => leaf_data.get_prefix(), } } } -impl FilledNode { +impl FilledNode { /// Deserializes filled nodes. pub(crate) fn deserialize( node_hash: HashOutput, @@ -92,10 +93,7 @@ impl FilledNode { is_leaf: bool, ) -> Result { if is_leaf { - return Ok(Self { - hash: node_hash, - data: NodeData::Leaf(L::deserialize(value)?), - }); + return Ok(Self { hash: node_hash, data: NodeData::Leaf(L::deserialize(value)?) }); } if value.0.len() == BINARY_BYTES { diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs index ce3d188cb1..70df1a268c 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs @@ -1,28 +1,20 @@ use std::collections::HashMap; -use std::sync::Arc; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use async_recursion::async_recursion; -use crate::block_committer::input::ContractAddress; -use crate::block_committer::input::StarknetStorageValue; +use crate::block_committer::input::{ContractAddress, StarknetStorageValue}; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::errors::FilledTreeError; -use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use crate::patricia_merkle_tree::filled_tree::node::FilledNode; -use crate::patricia_merkle_tree::node_data::inner_node::BinaryData; -use crate::patricia_merkle_tree::node_data::inner_node::EdgeData; -use crate::patricia_merkle_tree::node_data::inner_node::NodeData; -use crate::patricia_merkle_tree::node_data::leaf::ContractState; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; -use crate::patricia_merkle_tree::node_data::leaf::LeafModifications; +use crate::patricia_merkle_tree::filled_tree::node::{CompiledClassHash, FilledNode}; +use crate::patricia_merkle_tree::node_data::inner_node::{BinaryData, EdgeData, NodeData}; +use crate::patricia_merkle_tree::node_data::leaf::{ContractState, Leaf, LeafModifications}; use crate::patricia_merkle_tree::types::NodeIndex; use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunction; use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode; use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTree; use crate::storage::db_object::DBObject; -use crate::storage::storage_trait::StorageKey; -use crate::storage::storage_trait::StorageValue; +use crate::storage::storage_trait::{StorageKey, StorageValue}; #[cfg(test)] #[path = "tree_test.rs"] @@ -32,7 +24,7 @@ pub(crate) type FilledTreeResult = Result>; /// Consider a Patricia-Merkle Tree which has been updated with new leaves. /// FilledTree consists of all nodes which were modified in the update, including their updated /// data and hashes. -pub(crate) trait FilledTree: Sized { +pub(crate) trait FilledTree: Sized { /// Computes and returns the filled tree. async fn create<'a, TH: TreeHashFunction + 'static>( updated_skeleton: Arc + 'static>, @@ -48,7 +40,7 @@ pub(crate) trait FilledTree: Sized { } #[derive(Debug, Eq, PartialEq)] -pub struct FilledTreeImpl { +pub struct FilledTreeImpl { pub tree_map: HashMap>, pub root_hash: HashOutput, } @@ -58,7 +50,7 @@ pub type ClassesTrie = FilledTreeImpl; pub type ContractsTrie = FilledTreeImpl; pub type StorageTrieMap = HashMap; -impl FilledTreeImpl { +impl FilledTreeImpl { fn initialize_with_placeholders<'a>( updated_skeleton: &Arc>, ) -> HashMap>>> { @@ -170,10 +162,8 @@ impl FilledTreeImpl { Arc::clone(&output_map), ) .await?; - let data = NodeData::Edge(EdgeData { - path_to_bottom: *path_to_bottom, - bottom_hash, - }); + let data = + NodeData::Edge(EdgeData { path_to_bottom: *path_to_bottom, bottom_hash }); let hash_value = TH::compute_node_hash(&data); Self::write_to_output_map(output_map, index, hash_value, data)?; Ok(hash_value) @@ -199,21 +189,15 @@ impl FilledTreeImpl { let UpdatedSkeletonNode::UnmodifiedSubTree(root_hash) = root_node else { panic!("A root of tree without modifications is expected to be a unmodified subtree.") }; - Ok(Self { - tree_map: HashMap::new(), - root_hash: *root_hash, - }) + Ok(Self { tree_map: HashMap::new(), root_hash: *root_hash }) } fn create_empty() -> Self { - Self { - tree_map: HashMap::new(), - root_hash: HashOutput::ROOT_OF_EMPTY_TREE, - } + Self { tree_map: HashMap::new(), root_hash: HashOutput::ROOT_OF_EMPTY_TREE } } } -impl FilledTree for FilledTreeImpl { +impl FilledTree for FilledTreeImpl { async fn create<'a, TH: TreeHashFunction + 'static>( updated_skeleton: Arc + 'static>, leaf_modifications: Arc>, @@ -246,12 +230,9 @@ impl FilledTree for FilledTreeImpl { } fn serialize(&self) -> HashMap { - // This function iterates over each node in the tree, using the node's `db_key` as the hashmap key - // and the result of the node's `serialize` method as the value. - self.get_all_nodes() - .iter() - .map(|(_, node)| (node.db_key(), node.serialize())) - .collect() + // This function iterates over each node in the tree, using the node's `db_key` as the + // hashmap key and the result of the node's `serialize` method as the value. + self.get_all_nodes().iter().map(|(_, node)| (node.db_key(), node.serialize())).collect() } fn get_root_hash(&self) -> HashOutput { diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/tree_test.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/tree_test.rs index b7c08b61c8..058ca30267 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/tree_test.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/tree_test.rs @@ -1,30 +1,36 @@ use std::collections::HashMap; use std::sync::Arc; -use crate::block_committer::input::StarknetStorageValue; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::FilledNode; use crate::patricia_merkle_tree::filled_tree::tree::{FilledTree, FilledTreeImpl}; +use crate::patricia_merkle_tree::internal_test_utils::{MockLeaf, OriginalSkeletonMockTrieConfig}; use crate::patricia_merkle_tree::node_data::inner_node::{ - BinaryData, EdgeData, EdgePathLength, NodeData, PathToBottom, + BinaryData, + EdgeData, + EdgePathLength, + NodeData, + PathToBottom, }; use crate::patricia_merkle_tree::node_data::leaf::SkeletonLeaf; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig; use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl; use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices}; use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl; use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode; use crate::patricia_merkle_tree::updated_skeleton_tree::tree::{ - UpdatedSkeletonNodeMap, UpdatedSkeletonTree, UpdatedSkeletonTreeImpl, + UpdatedSkeletonNodeMap, + UpdatedSkeletonTree, + UpdatedSkeletonTreeImpl, }; use crate::storage::map_storage::MapStorage; #[tokio::test(flavor = "multi_thread")] -/// This test is a sanity test for computing the root hash of the patricia merkle tree with a single node that is a leaf with hash==1. +/// This test is a sanity test for computing the root hash of the patricia merkle tree with a single +/// node that is a leaf with hash==1. async fn test_filled_tree_sanity() { let mut skeleton_tree: UpdatedSkeletonNodeMap = HashMap::new(); - let new_filled_leaf = StarknetStorageValue(Felt::ONE); + let new_filled_leaf = MockLeaf(Felt::ONE); let new_leaf_index = NodeIndex::ROOT; skeleton_tree.insert(new_leaf_index, UpdatedSkeletonNode::Leaf); let modifications = HashMap::from([(new_leaf_index, new_filled_leaf)]); @@ -71,9 +77,7 @@ async fn test_small_filled_tree() { ] .into_iter() .chain( - new_leaves - .iter() - .map(|(index, _)| create_leaf_updated_skeleton_node_for_testing(*index)), + new_leaves.iter().map(|(index, _)| create_leaf_updated_skeleton_node_for_testing(*index)), ) .collect(); let skeleton_tree: UpdatedSkeletonNodeMap = nodes_in_skeleton_tree.into_iter().collect(); @@ -81,12 +85,7 @@ async fn test_small_filled_tree() { let updated_skeleton_tree = UpdatedSkeletonTreeImpl { skeleton_tree }; let modifications = new_leaves .iter() - .map(|(index, value)| { - ( - NodeIndex::from(*index), - StarknetStorageValue(Felt::from_hex(value).unwrap()), - ) - }) + .map(|(index, value)| (NodeIndex::from(*index), MockLeaf(Felt::from_hex(value).unwrap()))) .collect(); // Compute the hash values. @@ -100,53 +99,17 @@ async fn test_small_filled_tree() { let root_hash = filled_tree.get_root_hash(); // The expected hash values were computed separately. - let expected_root_hash = HashOutput( - Felt::from_hex("0xe8899e8c731a35f5e9ce4c4bc32aabadcc81c5cdcc1aeba74fa7509046c338").unwrap(), - ); + let expected_root_hash = HashOutput(Felt::from_hex("0x21").unwrap()); let expected_filled_tree_map = HashMap::from([ - create_binary_entry_for_testing( - 1, - "0xe8899e8c731a35f5e9ce4c4bc32aabadcc81c5cdcc1aeba74fa7509046c338", - "0x4e970ad06a06486b44fff5606c4f65486d31e05e323d65a618d4ef8cdf6d3a0", - "0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543", - ), - create_edge_entry_for_testing( - 2, - "0x4e970ad06a06486b44fff5606c4f65486d31e05e323d65a618d4ef8cdf6d3a0", - 0, - 1, - "0x5d36a1ae900ef417a5696417dde9a0244b873522f40b552e4a60acde0991bc9", - ), - create_edge_entry_for_testing( - 3, - "0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543", - 15, - 4, - "0x3", - ), - create_binary_entry_for_testing( - 4, - "0x5d36a1ae900ef417a5696417dde9a0244b873522f40b552e4a60acde0991bc9", - "0x582d984e4005c27b9c886cd00ec9a82ed5323aa629f6ea6b3ed7c0386ae6256", - "0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88", - ), - create_edge_entry_for_testing( - 8, - "0x582d984e4005c27b9c886cd00ec9a82ed5323aa629f6ea6b3ed7c0386ae6256", - 3, - 2, - "0x1", - ), - create_edge_entry_for_testing( - 9, - "0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88", - 0, - 2, - "0x2", - ), - create_leaf_entry_for_testing(35, "0x1"), - create_leaf_entry_for_testing(36, "0x2"), - create_leaf_entry_for_testing(63, "0x3"), + create_mock_binary_entry_for_testing(1, "0x21", "0xb", "0x16"), + create_mock_edge_entry_for_testing(2, "0xb", 0, 1, "0xa"), + create_mock_edge_entry_for_testing(3, "0x16", 15, 4, "0x3"), + create_mock_binary_entry_for_testing(4, "0xa", "0x6", "0x4"), + create_mock_edge_entry_for_testing(8, "0x6", 3, 2, "0x1"), + create_mock_edge_entry_for_testing(9, "0x4", 0, 2, "0x2"), + create_mock_leaf_entry_for_testing(35, "0x1"), + create_mock_leaf_entry_for_testing(36, "0x2"), + create_mock_leaf_entry_for_testing(63, "0x3"), ]); assert_eq!(filled_tree_map, &expected_filled_tree_map); assert_eq!(root_hash, expected_root_hash, "Root hash mismatch"); @@ -158,12 +121,12 @@ async fn test_small_filled_tree() { /// i=1: binary /// / \ /// i=2: edge i=3: unmodified -/// l=1, p=0 hash=0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543 +/// l=1, p=0 hash=0x3 /// / /// i=4: binary /// / \ /// i=8: edge i=9: unmodified -/// l=2, p=3 hash=0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88 +/// l=2, p=3 hash=0x4 /// \ /// \ /// i=35: leaf @@ -174,16 +137,10 @@ async fn test_small_tree_with_unmodified_nodes() { let nodes_in_skeleton_tree = [ create_binary_updated_skeleton_node_for_testing(1), create_path_to_bottom_edge_updated_skeleton_node_for_testing(2, 0, 1), - create_unmodified_updated_skeleton_node_for_testing( - 3, - "0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543", - ), + create_unmodified_updated_skeleton_node_for_testing(3, "0x3"), create_binary_updated_skeleton_node_for_testing(4), create_path_to_bottom_edge_updated_skeleton_node_for_testing(8, 3, 2), - create_unmodified_updated_skeleton_node_for_testing( - 9, - "0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88", - ), + create_unmodified_updated_skeleton_node_for_testing(9, "0x4"), create_leaf_updated_skeleton_node_for_testing(new_leaf_index), ]; let skeleton_tree: UpdatedSkeletonNodeMap = nodes_in_skeleton_tree.into_iter().collect(); @@ -191,7 +148,7 @@ async fn test_small_tree_with_unmodified_nodes() { let updated_skeleton_tree = UpdatedSkeletonTreeImpl { skeleton_tree }; let modifications = HashMap::from([( NodeIndex::from(new_leaf_index), - StarknetStorageValue(Felt::from_hex(new_leaf).unwrap()), + MockLeaf(Felt::from_hex(new_leaf).unwrap()), )]); // Compute the hash values. @@ -207,37 +164,13 @@ async fn test_small_tree_with_unmodified_nodes() { // The expected hash values were computed separately. Note that the unmodified nodes are not // computed in the filled tree, but the hash values are directly used. The hashes of unmodified // nodes should not appear in the filled tree. - let expected_root_hash = HashOutput( - Felt::from_hex("0xe8899e8c731a35f5e9ce4c4bc32aabadcc81c5cdcc1aeba74fa7509046c338").unwrap(), - ); + let expected_root_hash = HashOutput(Felt::from_hex("0xe").unwrap()); let expected_filled_tree_map = HashMap::from([ - create_binary_entry_for_testing( - 1, - "0xe8899e8c731a35f5e9ce4c4bc32aabadcc81c5cdcc1aeba74fa7509046c338", - "0x4e970ad06a06486b44fff5606c4f65486d31e05e323d65a618d4ef8cdf6d3a0", - "0x2955a96b09495fb2ce4ed65cf679c54e54aefc2c6972d7f3042590000bb7543", - ), - create_edge_entry_for_testing( - 2, - "0x4e970ad06a06486b44fff5606c4f65486d31e05e323d65a618d4ef8cdf6d3a0", - 0, - 1, - "0x5d36a1ae900ef417a5696417dde9a0244b873522f40b552e4a60acde0991bc9", - ), - create_binary_entry_for_testing( - 4, - "0x5d36a1ae900ef417a5696417dde9a0244b873522f40b552e4a60acde0991bc9", - "0x582d984e4005c27b9c886cd00ec9a82ed5323aa629f6ea6b3ed7c0386ae6256", - "0x39eb7b85bcc9deac314406d6b73154b09b008f8af05e2f58ab623f4201d0b88", - ), - create_edge_entry_for_testing( - 8, - "0x582d984e4005c27b9c886cd00ec9a82ed5323aa629f6ea6b3ed7c0386ae6256", - 3, - 2, - "0x1", - ), - create_leaf_entry_for_testing(35, "0x1"), + create_mock_binary_entry_for_testing(1, "0xe", "0xb", "0x3"), + create_mock_edge_entry_for_testing(2, "0b", 0, 1, "0xa"), + create_mock_binary_entry_for_testing(4, "0xa", "0x6", "0x4"), + create_mock_edge_entry_for_testing(8, "0x6", 3, 2, "0x1"), + create_mock_leaf_entry_for_testing(35, "0x1"), ]); assert_eq!(filled_tree_map, &expected_filled_tree_map); assert_eq!(root_hash, expected_root_hash, "Root hash mismatch"); @@ -246,18 +179,16 @@ async fn test_small_tree_with_unmodified_nodes() { #[tokio::test(flavor = "multi_thread")] /// Test that deleting a leaf that does not exist in the tree succeeds. async fn test_delete_leaf_from_empty_tree() { - let storage_modifications: HashMap = - HashMap::from([(NodeIndex::FIRST_LEAF, StarknetStorageValue(Felt::ZERO))]); + let storage_modifications: HashMap = + HashMap::from([(NodeIndex::FIRST_LEAF, MockLeaf(Felt::ZERO))]); let mut indices = [NodeIndex::FIRST_LEAF]; // Create an empty original skeleton tree with a single leaf modified. let mut original_skeleton_tree = OriginalSkeletonTreeImpl::create_impl( - &MapStorage { - storage: HashMap::new(), - }, + &MapStorage { storage: HashMap::new() }, HashOutput::ROOT_OF_EMPTY_TREE, SortedLeafIndices::new(&mut indices), - &OriginalSkeletonStorageTrieConfig::new(&storage_modifications, false), + &OriginalSkeletonMockTrieConfig::new(&storage_modifications, false), ) .unwrap(); @@ -268,8 +199,7 @@ async fn test_delete_leaf_from_empty_tree() { UpdatedSkeletonTreeImpl::create(&mut original_skeleton_tree, &skeleton_modifications) .unwrap(); - let leaf_modifications = - HashMap::from([(NodeIndex::FIRST_LEAF, StarknetStorageValue(Felt::ZERO))]); + let leaf_modifications = HashMap::from([(NodeIndex::FIRST_LEAF, MockLeaf(Felt::ZERO))]); // Compute the filled tree. let filled_tree = FilledTreeImpl::create::( updated_skeleton_tree.into(), @@ -318,12 +248,12 @@ fn create_leaf_updated_skeleton_node_for_testing(index: u128) -> (NodeIndex, Upd (NodeIndex::from(index), UpdatedSkeletonNode::Leaf) } -fn create_binary_entry_for_testing( +fn create_mock_binary_entry_for_testing( index: u128, hash: &str, left_hash: &str, right_hash: &str, -) -> (NodeIndex, FilledNode) { +) -> (NodeIndex, FilledNode) { ( NodeIndex::from(index), FilledNode { @@ -336,13 +266,13 @@ fn create_binary_entry_for_testing( ) } -fn create_edge_entry_for_testing( +fn create_mock_edge_entry_for_testing( index: u128, hash: &str, path: u128, length: u8, bottom_hash: &str, -) -> (NodeIndex, FilledNode) { +) -> (NodeIndex, FilledNode) { ( NodeIndex::from(index), FilledNode { @@ -359,15 +289,15 @@ fn create_edge_entry_for_testing( ) } -fn create_leaf_entry_for_testing( +fn create_mock_leaf_entry_for_testing( index: u128, hash: &str, -) -> (NodeIndex, FilledNode) { +) -> (NodeIndex, FilledNode) { ( NodeIndex::from(index), FilledNode { hash: HashOutput(Felt::from_hex(hash).unwrap()), - data: NodeData::Leaf(StarknetStorageValue(Felt::from_hex(hash).unwrap())), + data: NodeData::Leaf(MockLeaf(Felt::from_hex(hash).unwrap())), }, ) } diff --git a/crates/committer/src/patricia_merkle_tree/internal_test_utils.rs b/crates/committer/src/patricia_merkle_tree/internal_test_utils.rs index afbde3df11..688b3e4af5 100644 --- a/crates/committer/src/patricia_merkle_tree/internal_test_utils.rs +++ b/crates/committer/src/patricia_merkle_tree/internal_test_utils.rs @@ -1,16 +1,91 @@ +use std::sync::Arc; + +use ethnum::U256; +use rand::rngs::ThreadRng; +use rstest::{fixture, rstest}; + use crate::felt::Felt; +use crate::generate_trie_config; +use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::external_test_utils::get_random_u256; - -use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToBottom}; -use crate::patricia_merkle_tree::node_data::leaf::SkeletonLeaf; +use crate::patricia_merkle_tree::filled_tree::tree::FilledTreeImpl; +use crate::patricia_merkle_tree::node_data::errors::LeafResult; +use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, NodeData, PathToBottom}; +use crate::patricia_merkle_tree::node_data::leaf::{Leaf, LeafModifications, SkeletonLeaf}; +use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonTreeConfig; +use crate::patricia_merkle_tree::original_skeleton_tree::errors::OriginalSkeletonTreeError; use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; +use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeResult; use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight}; +use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::{ + HashFunction, + TreeHashFunction, + TreeHashFunctionImpl, +}; use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode; use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl; +use crate::storage::db_object::{DBObject, Deserializable}; +use crate::storage::storage_trait::StorageValue; -use ethnum::U256; -use rand::rngs::ThreadRng; -use rstest::{fixture, rstest}; +#[derive(Debug, PartialEq, Clone, Copy, Default, Eq)] +pub(crate) struct MockLeaf(pub(crate) Felt); + +impl DBObject for MockLeaf { + fn serialize(&self) -> StorageValue { + StorageValue(self.0.to_bytes_be().to_vec()) + } + + fn get_prefix(&self) -> Vec { + vec![0] + } +} + +impl Deserializable for MockLeaf { + fn deserialize( + value: &StorageValue, + ) -> Result { + Ok(Self(Felt::from_bytes_be_slice(&value.0))) + } + + fn prefix() -> Vec { + vec![0] + } +} + +impl Leaf for MockLeaf { + fn is_empty(&self) -> bool { + self.0 == Felt::ZERO + } + + async fn create( + index: &NodeIndex, + leaf_modifications: Arc>, + ) -> LeafResult { + Self::from_modifications(index, leaf_modifications) + } +} + +impl TreeHashFunction for TreeHashFunctionImpl { + fn compute_leaf_hash(leaf_data: &MockLeaf) -> HashOutput { + HashOutput(leaf_data.0) + } + + fn compute_node_hash(node_data: &NodeData) -> HashOutput { + Self::compute_node_hash_with_inner_hash_function::(node_data) + } +} + +generate_trie_config!(OriginalSkeletonMockTrieConfig, MockLeaf); + +pub(crate) type MockTrie = FilledTreeImpl; + +struct MockHashFunction; + +impl HashFunction for MockHashFunction { + fn hash(left: &Felt, right: &Felt) -> HashOutput { + HashOutput(*left + *right) + } +} impl From for SkeletonLeaf { fn from(value: u8) -> Self { @@ -21,9 +96,7 @@ impl From for SkeletonLeaf { impl From<&str> for PathToBottom { fn from(value: &str) -> Self { Self::new( - U256::from_str_radix(value, 2) - .expect("Invalid binary string") - .into(), + U256::from_str_radix(value, 2).expect("Invalid binary string").into(), EdgePathLength::new( (value.len() - if value.starts_with('+') { 1 } else { 0 }) .try_into() @@ -78,16 +151,12 @@ pub(crate) fn get_initial_updated_skeleton( .iter() .filter(|(_, leaf_val)| *leaf_val != 0) .map(|(index, _)| (*index, UpdatedSkeletonNode::Leaf)) - .chain( - original_skeleton - .iter() - .filter_map(|(index, node)| match node { - OriginalSkeletonNode::UnmodifiedSubTree(hash) => { - Some((*index, UpdatedSkeletonNode::UnmodifiedSubTree(*hash))) - } - OriginalSkeletonNode::Binary | OriginalSkeletonNode::Edge(_) => None, - }), - ) + .chain(original_skeleton.iter().filter_map(|(index, node)| match node { + OriginalSkeletonNode::UnmodifiedSubTree(hash) => { + Some((*index, UpdatedSkeletonNode::UnmodifiedSubTree(*hash))) + } + OriginalSkeletonNode::Binary | OriginalSkeletonNode::Edge(_) => None, + })) .collect(), } } diff --git a/crates/committer/src/patricia_merkle_tree/node_data/errors.rs b/crates/committer/src/patricia_merkle_tree/node_data/errors.rs index 4d1a729b38..9feae4beef 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/errors.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/errors.rs @@ -1,4 +1,5 @@ use std::fmt::Debug; + use thiserror::Error; use crate::patricia_merkle_tree::node_data::inner_node::{EdgePath, EdgePathLength}; @@ -7,15 +8,9 @@ use crate::patricia_merkle_tree::types::NodeIndex; #[derive(Debug, Error)] pub enum PathToBottomError { #[error("Tried to remove {n_edges:?} edges from a {length:?} length path.")] - RemoveEdgesError { - length: EdgePathLength, - n_edges: EdgePathLength, - }, + RemoveEdgesError { length: EdgePathLength, n_edges: EdgePathLength }, #[error("EdgePath {path:?} is too long for EdgePathLength {length:?}.")] - MismatchedLengthError { - path: EdgePath, - length: EdgePathLength, - }, + MismatchedLengthError { path: EdgePath, length: EdgePathLength }, } #[derive(Debug, Error)] diff --git a/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs b/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs index 2e59ea961a..4250b0f05e 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs @@ -1,12 +1,12 @@ +use ethnum::U256; +use strum_macros::{EnumDiscriminants, EnumIter}; + use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::node_data::errors::{EdgePathError, PathToBottomError}; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; +use crate::patricia_merkle_tree::node_data::leaf::Leaf; use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight}; -use ethnum::U256; -use strum_macros::{EnumDiscriminants, EnumIter}; - #[cfg(test)] #[path = "inner_node_tests.rs"] pub mod inner_node_test; @@ -15,7 +15,7 @@ pub mod inner_node_test; #[cfg_attr(any(test, feature = "testing"), derive(EnumDiscriminants))] #[cfg_attr(any(test, feature = "testing"), strum_discriminants(derive(EnumIter)))] // A Patricia-Merkle tree node's data, i.e., the pre-image of its hash. -pub enum NodeData { +pub enum NodeData { Binary(BinaryData), Edge(EdgeData), Leaf(L), @@ -36,10 +36,8 @@ impl EdgePath { /// [EdgePath] constant that represents the longest path (from some node) in a tree. #[allow(clippy::as_conversions)] - pub const MAX: Self = Self(U256::from_words( - u128::MAX >> (U256::BITS - Self::BITS as u32), - u128::MAX, - )); + pub const MAX: Self = + Self(U256::from_words(u128::MAX >> (U256::BITS - Self::BITS as u32), u128::MAX)); } impl From for EdgePath { @@ -115,11 +113,7 @@ impl PathToBottom { if bit_length > u8::from(length).into() { return Err(PathToBottomError::MismatchedLengthError { path, length }); } - Ok(Self { - path, - length, - _fake_field: (), - }) + Ok(Self { path, length, _fake_field: () }) } } @@ -130,17 +124,11 @@ pub struct EdgeData { } impl PathToBottom { - pub(crate) const LEFT_CHILD: Self = Self { - path: EdgePath(U256::ZERO), - length: EdgePathLength(1), - _fake_field: (), - }; - - pub(crate) const RIGHT_CHILD: Self = Self { - path: EdgePath(U256::ONE), - length: EdgePathLength(1), - _fake_field: (), - }; + pub(crate) const LEFT_CHILD: Self = + Self { path: EdgePath(U256::ZERO), length: EdgePathLength(1), _fake_field: () }; + + pub(crate) const RIGHT_CHILD: Self = + Self { path: EdgePath(U256::ONE), length: EdgePathLength(1), _fake_field: () }; pub(crate) fn bottom_index(&self, root_index: NodeIndex) -> NodeIndex { NodeIndex::compute_bottom_index(root_index, self) @@ -164,10 +152,7 @@ impl PathToBottom { /// Returns the path after removing the first steps (the edges from the path's origin node). pub(crate) fn remove_first_edges(&self, n_edges: EdgePathLength) -> PathToBottomResult { if self.length < n_edges { - return Err(PathToBottomError::RemoveEdgesError { - length: self.length, - n_edges, - }); + return Err(PathToBottomError::RemoveEdgesError { length: self.length, n_edges }); } Self::new( EdgePath(self.path.0 & ((U256::ONE << (self.length.0 - n_edges.0)) - 1)), diff --git a/crates/committer/src/patricia_merkle_tree/node_data/inner_node_tests.rs b/crates/committer/src/patricia_merkle_tree/node_data/inner_node_tests.rs index 2bbbd20475..5a8b43c352 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/inner_node_tests.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/inner_node_tests.rs @@ -16,9 +16,7 @@ fn test_remove_first_edges( #[case] expected: PathToBottom, ) { assert_eq!( - path_to_bottom - .remove_first_edges(EdgePathLength::new(n_edges).unwrap()) - .unwrap(), + path_to_bottom.remove_first_edges(EdgePathLength::new(n_edges).unwrap()).unwrap(), expected ); } diff --git a/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs b/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs index 1dcc6daeea..2855b41020 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs @@ -11,7 +11,7 @@ use crate::patricia_merkle_tree::node_data::errors::{LeafError, LeafResult}; use crate::patricia_merkle_tree::types::NodeIndex; use crate::storage::db_object::{DBObject, Deserializable}; -pub trait LeafData: Clone + Sync + Send + DBObject + Deserializable + Default + Debug + Eq { +pub trait Leaf: Clone + Sync + Send + DBObject + Deserializable + Default + Debug + Eq { /// Returns true if leaf is empty. fn is_empty(&self) -> bool; @@ -47,7 +47,7 @@ pub struct ContractState { pub class_hash: ClassHash, } -impl LeafData for StarknetStorageValue { +impl Leaf for StarknetStorageValue { fn is_empty(&self) -> bool { self.0 == Felt::ZERO } @@ -60,7 +60,7 @@ impl LeafData for StarknetStorageValue { } } -impl LeafData for CompiledClassHash { +impl Leaf for CompiledClassHash { fn is_empty(&self) -> bool { self.0 == Felt::ZERO } @@ -73,7 +73,7 @@ impl LeafData for CompiledClassHash { } } -impl LeafData for ContractState { +impl Leaf for ContractState { fn is_empty(&self) -> bool { self.nonce.0 == Felt::ZERO && self.class_hash.0 == Felt::ZERO @@ -102,11 +102,7 @@ impl SkeletonLeaf { impl From for SkeletonLeaf { fn from(value: Felt) -> Self { - if value == Felt::ZERO { - Self::Zero - } else { - Self::NonZero - } + if value == Felt::ZERO { Self::Zero } else { Self::NonZero } } } diff --git a/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde.rs b/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde.rs index 6883d2b94c..654a3a8944 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde.rs @@ -10,7 +10,7 @@ use crate::patricia_merkle_tree::node_data::leaf::ContractState; use crate::patricia_merkle_tree::types::SubTreeHeight; use crate::storage::db_object::{DBObject, Deserializable}; use crate::storage::errors::DeserializationError; -use crate::storage::storage_trait::{StoragePrefix, StorageValue}; +use crate::storage::storage_trait::{StarknetPrefix, StorageValue}; #[cfg(test)] #[path = "leaf_serde_test.rs"] @@ -22,8 +22,8 @@ impl DBObject for StarknetStorageValue { StorageValue(self.0.to_bytes_be().to_vec()) } - fn get_prefix(&self) -> StoragePrefix { - StoragePrefix::StorageLeaf + fn get_prefix(&self) -> Vec { + StarknetPrefix::StorageLeaf.to_storage_prefix() } } @@ -34,8 +34,8 @@ impl DBObject for CompiledClassHash { StorageValue(json_string.into_bytes()) } - fn get_prefix(&self) -> StoragePrefix { - StoragePrefix::CompiledClassLeaf + fn get_prefix(&self) -> Vec { + StarknetPrefix::CompiledClassLeaf.to_storage_prefix() } } @@ -52,8 +52,8 @@ impl DBObject for ContractState { StorageValue(json_string.into_bytes()) } - fn get_prefix(&self) -> StoragePrefix { - StoragePrefix::StateTreeLeaf + fn get_prefix(&self) -> Vec { + StarknetPrefix::StateTreeLeaf.to_storage_prefix() } } @@ -62,8 +62,8 @@ impl Deserializable for StarknetStorageValue { Ok(Self(Felt::from_bytes_be_slice(&value.0))) } - fn prefix() -> StoragePrefix { - StoragePrefix::StorageLeaf + fn prefix() -> Vec { + StarknetPrefix::StorageLeaf.to_storage_prefix() } } @@ -71,16 +71,14 @@ impl Deserializable for CompiledClassHash { fn deserialize(value: &StorageValue) -> Result { let json_str = std::str::from_utf8(&value.0)?; let map: HashMap = serde_json::from_str(json_str)?; - let hash_as_hex = - map.get("compiled_class_hash") - .ok_or(DeserializationError::NonExistingKey( - "compiled_class_hash".to_string(), - ))?; + let hash_as_hex = map + .get("compiled_class_hash") + .ok_or(DeserializationError::NonExistingKey("compiled_class_hash".to_string()))?; Ok(Self::from_hex(hash_as_hex)?) } - fn prefix() -> StoragePrefix { - StoragePrefix::CompiledClassLeaf + fn prefix() -> Vec { + StarknetPrefix::CompiledClassLeaf.to_storage_prefix() } } @@ -97,10 +95,8 @@ impl Deserializable for ContractState { }; let class_hash_as_hex = get_leaf_key(&deserialized_map, "contract_hash")?; let nonce_as_hex = get_leaf_key(&deserialized_map, "nonce")?; - let root_hash_as_hex = get_leaf_key( - get_key_from_map(&deserialized_map, "storage_commitment_tree")?, - "root", - )?; + let root_hash_as_hex = + get_leaf_key(get_key_from_map(&deserialized_map, "storage_commitment_tree")?, "root")?; Ok(Self { nonce: Nonce::from_hex(&nonce_as_hex)?, @@ -109,12 +105,11 @@ impl Deserializable for ContractState { }) } - fn prefix() -> StoragePrefix { - StoragePrefix::StateTreeLeaf + fn prefix() -> Vec { + StarknetPrefix::StateTreeLeaf.to_storage_prefix() } } fn get_key_from_map<'a>(map: &'a Value, key: &str) -> Result<&'a Value, DeserializationError> { - map.get(key) - .ok_or(DeserializationError::NonExistingKey(key.to_string())) + map.get(key).ok_or(DeserializationError::NonExistingKey(key.to_string())) } diff --git a/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde_test.rs b/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde_test.rs index 74dfbe4d38..446bc0aa0e 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde_test.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/leaf_serde_test.rs @@ -1,13 +1,12 @@ +use std::fmt::Debug; + +use rstest::rstest; + use crate::block_committer::input::StarknetStorageValue; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; -use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, Nonce}; -use crate::patricia_merkle_tree::node_data::leaf::ContractState; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; - -use rstest::rstest; -use std::fmt::Debug; +use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; +use crate::patricia_merkle_tree::node_data::leaf::{ContractState, Leaf}; #[rstest] #[case::zero_storage_leaf(StarknetStorageValue(Felt::ZERO))] @@ -26,7 +25,7 @@ use std::fmt::Debug; nonce: Nonce(Felt::from(23479515749555_u128)), storage_root_hash: HashOutput(Felt::from(2359743529034_u128)), class_hash: ClassHash(Felt::from(1349866415897798_u128)) }) ] -fn test_leaf_serde(#[case] leaf: L) { +fn test_leaf_serde(#[case] leaf: L) { let serialized = leaf.serialize(); let deserialized = L::deserialize(&serialized).unwrap(); assert_eq!(deserialized, leaf); @@ -36,6 +35,6 @@ fn test_leaf_serde(#[case] leaf: L) { #[case::storage_leaf(StarknetStorageValue::default())] #[case::compiled_class_leaf(CompiledClassHash::default())] #[case::contract_state_leaf(ContractState::default())] -fn test_default_is_empty(#[case] leaf: L) { +fn test_default_is_empty(#[case] leaf: L) { assert!(leaf.is_empty()) } diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/config.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/config.rs index feb4b53e60..c38b138b4b 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/config.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/config.rs @@ -1,17 +1,17 @@ use crate::block_committer::input::StarknetStorageValue; use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use crate::patricia_merkle_tree::node_data::leaf::{ContractState, LeafData, LeafModifications}; +use crate::patricia_merkle_tree::node_data::leaf::{ContractState, Leaf, LeafModifications}; use crate::patricia_merkle_tree::original_skeleton_tree::errors::OriginalSkeletonTreeError; use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeResult; use crate::patricia_merkle_tree::types::NodeIndex; /// Configures the creation of an original skeleton tree. -pub(crate) trait OriginalSkeletonTreeConfig { +pub(crate) trait OriginalSkeletonTreeConfig { /// Configures whether modified leaves should be compared to the previous leaves and log out a /// warning when encountering a trivial modification. fn compare_modified_leaves(&self) -> bool; - /// Compares the previous leaf to the modificated and returns true iff they are equal. + /// Compares the previous leaf to the modified and returns true iff they are equal. fn compare_leaf( &self, index: &NodeIndex, @@ -19,6 +19,7 @@ pub(crate) trait OriginalSkeletonTreeConfig { ) -> OriginalSkeletonTreeResult; } +#[macro_export] macro_rules! generate_trie_config { ($struct_name:ident, $leaf_type:ty) => { pub(crate) struct $struct_name<'a> { @@ -27,14 +28,12 @@ macro_rules! generate_trie_config { } impl<'a> $struct_name<'a> { + #[allow(dead_code)] pub(crate) fn new( modifications: &'a LeafModifications<$leaf_type>, compare_modified_leaves: bool, ) -> Self { - Self { - modifications, - compare_modified_leaves, - } + Self { modifications, compare_modified_leaves } } } diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs index df3145c34f..13594f3a75 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs @@ -1,28 +1,28 @@ +use std::borrow::Borrow; +use std::collections::HashMap; +use std::fmt::Debug; + +use log::warn; + use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::FilledNode; -use crate::patricia_merkle_tree::node_data::inner_node::BinaryData; -use crate::patricia_merkle_tree::node_data::inner_node::EdgeData; -use crate::patricia_merkle_tree::node_data::inner_node::NodeData; -use crate::patricia_merkle_tree::node_data::inner_node::PathToBottom; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; +use crate::patricia_merkle_tree::node_data::inner_node::{ + BinaryData, + EdgeData, + NodeData, + PathToBottom, +}; +use crate::patricia_merkle_tree::node_data::leaf::Leaf; use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonTreeConfig; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeResult; -use crate::patricia_merkle_tree::original_skeleton_tree::utils::split_leaves; -use crate::patricia_merkle_tree::types::SortedLeafIndices; -use crate::patricia_merkle_tree::types::SubTreeHeight; -use crate::patricia_merkle_tree::{ - original_skeleton_tree::node::OriginalSkeletonNode, types::NodeIndex, +use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; +use crate::patricia_merkle_tree::original_skeleton_tree::tree::{ + OriginalSkeletonTreeImpl, + OriginalSkeletonTreeResult, }; +use crate::patricia_merkle_tree::original_skeleton_tree::utils::split_leaves; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices, SubTreeHeight}; use crate::storage::errors::StorageError; -use crate::storage::storage_trait::create_db_key; -use crate::storage::storage_trait::Storage; -use crate::storage::storage_trait::StorageKey; -use crate::storage::storage_trait::StoragePrefix; -use log::warn; -use std::borrow::Borrow; -use std::collections::HashMap; -use std::fmt::Debug; +use crate::storage::storage_trait::{create_db_key, StarknetPrefix, Storage, StorageKey}; #[cfg(test)] #[path = "create_tree_test.rs"] @@ -31,10 +31,7 @@ pub mod create_tree_test; /// Logs out a warning of a trivial modification. macro_rules! log_trivial_modification { ($index:expr, $value:expr) => { - warn!( - "Encountered a trivial modification at index {:?}, with value {:?}", - $index, $value - ); + warn!("Encountered a trivial modification at index {:?}, with value {:?}", $index, $value); }; } @@ -73,9 +70,7 @@ impl<'a> SubTree<'a> { leftmost_in_subtree - NodeIndex::ROOT + (NodeIndex::ROOT << bottom_height.into()); let leftmost_index = self.sorted_leaf_indices.bisect_left(&leftmost_in_subtree); let rightmost_index = self.sorted_leaf_indices.bisect_right(&rightmost_in_subtree); - let bottom_leaves = self - .sorted_leaf_indices - .subslice(leftmost_index, rightmost_index); + let bottom_leaves = self.sorted_leaf_indices.subslice(leftmost_index, rightmost_index); let previously_empty_leaf_indices = self.sorted_leaf_indices.get_indices() [..leftmost_index] .iter() @@ -110,7 +105,7 @@ impl<'a> SubTree<'a> { } fn is_leaf(&self) -> bool { - u8::from(self.get_height()) == 0 + self.root_index.is_leaf() } } @@ -119,7 +114,7 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { /// Given a list of subtrees, traverses towards their leaves and fetches all non-empty, /// unmodified nodes. If `compare_modified_leaves` is set, function logs out a warning when /// encountering a trivial modification. Fills the previous leaf values if it is not none. - fn fetch_nodes( + fn fetch_nodes( &mut self, subtrees: Vec>, storage: &impl Storage, @@ -129,16 +124,14 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { if subtrees.is_empty() { return Ok(()); } - let should_fetch_leaves = config.compare_modified_leaves() || previous_leaves.is_some(); + let should_fetch_modified_leaves = + config.compare_modified_leaves() || previous_leaves.is_some(); let mut next_subtrees = Vec::new(); let filled_roots = Self::calculate_subtrees_roots::(&subtrees, storage)?; for (filled_root, subtree) in filled_roots.into_iter().zip(subtrees.iter()) { match filled_root.data { // Binary node. - NodeData::Binary(BinaryData { - left_hash, - right_hash, - }) => { + NodeData::Binary(BinaryData { left_hash, right_hash }) => { if subtree.is_unmodified() { self.nodes.insert( subtree.root_index, @@ -146,23 +139,25 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { ); continue; } - self.nodes - .insert(subtree.root_index, OriginalSkeletonNode::Binary); + self.nodes.insert(subtree.root_index, OriginalSkeletonNode::Binary); let (left_subtree, right_subtree) = subtree.get_children_subtrees(left_hash, right_hash); - self.handle_subtree(&mut next_subtrees, left_subtree, should_fetch_leaves); - self.handle_subtree(&mut next_subtrees, right_subtree, should_fetch_leaves) + self.handle_subtree( + &mut next_subtrees, + left_subtree, + should_fetch_modified_leaves, + ); + self.handle_subtree( + &mut next_subtrees, + right_subtree, + should_fetch_modified_leaves, + ) } // Edge node. - NodeData::Edge(EdgeData { - bottom_hash, - path_to_bottom, - }) => { - self.nodes.insert( - subtree.root_index, - OriginalSkeletonNode::Edge(path_to_bottom), - ); + NodeData::Edge(EdgeData { bottom_hash, path_to_bottom }) => { + self.nodes + .insert(subtree.root_index, OriginalSkeletonNode::Edge(path_to_bottom)); if subtree.is_unmodified() { self.nodes.insert( path_to_bottom.bottom_index(subtree.root_index), @@ -186,16 +181,16 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { config, )?; - self.handle_subtree(&mut next_subtrees, bottom_subtree, should_fetch_leaves); + self.handle_subtree( + &mut next_subtrees, + bottom_subtree, + should_fetch_modified_leaves, + ); } // Leaf node. NodeData::Leaf(previous_leaf) => { if subtree.is_unmodified() { - // Sibling leaf. - self.nodes.insert( - subtree.root_index, - OriginalSkeletonNode::UnmodifiedSubTree(filled_root.hash), - ); + warn!("Unexpectedly deserialized leaf sibling.") } else { // Modified leaf. if config.compare_modified_leaves() @@ -203,6 +198,7 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { { log_trivial_modification!(subtree.root_index, previous_leaf); } + // If previous values of modified leaves are requested, add this leaf. if let Some(ref mut leaves) = previous_leaves { leaves.insert(subtree.root_index, previous_leaf); } @@ -213,7 +209,8 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { self.fetch_nodes::(next_subtrees, storage, config, previous_leaves) } - fn calculate_subtrees_roots( + // TODO(Aviv, 17/07/2024): Split between storage prefix implementation and function logic. + fn calculate_subtrees_roots( subtrees: &[SubTree<'a>], storage: &impl Storage, ) -> OriginalSkeletonTreeResult>> { @@ -225,7 +222,7 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { if subtree.is_leaf() { L::prefix() } else { - StoragePrefix::InnerNode + StarknetPrefix::InnerNode.to_storage_prefix() }, &subtree.root_hash.0.to_bytes_be(), ) @@ -237,16 +234,12 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { subtrees.iter().zip(db_vals.iter()).zip(db_keys.into_iter()) { let val = optional_val.ok_or(StorageError::MissingKey(db_key))?; - subtrees_roots.push(FilledNode::deserialize( - subtree.root_hash, - val, - subtree.is_leaf(), - )?) + subtrees_roots.push(FilledNode::deserialize(subtree.root_hash, val, subtree.is_leaf())?) } Ok(subtrees_roots) } - pub(crate) fn create_impl( + pub(crate) fn create_impl( storage: &impl Storage, root_hash: HashOutput, sorted_leaf_indices: SortedLeafIndices<'a>, @@ -262,20 +255,13 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { )?; return Ok(Self::create_empty(sorted_leaf_indices)); } - let main_subtree = SubTree { - sorted_leaf_indices, - root_index: NodeIndex::ROOT, - root_hash, - }; - let mut skeleton_tree = Self { - nodes: HashMap::new(), - sorted_leaf_indices, - }; + let main_subtree = SubTree { sorted_leaf_indices, root_index: NodeIndex::ROOT, root_hash }; + let mut skeleton_tree = Self { nodes: HashMap::new(), sorted_leaf_indices }; skeleton_tree.fetch_nodes::(vec![main_subtree], storage, config, None)?; Ok(skeleton_tree) } - pub(crate) fn create_and_get_previous_leaves_impl( + pub(crate) fn create_and_get_previous_leaves_impl( storage: &impl Storage, root_hash: HashOutput, sorted_leaf_indices: SortedLeafIndices<'a>, @@ -288,22 +274,11 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { if root_hash == HashOutput::ROOT_OF_EMPTY_TREE { return Ok(( Self::create_empty(sorted_leaf_indices), - sorted_leaf_indices - .get_indices() - .iter() - .map(|idx| (*idx, L::default())) - .collect(), + sorted_leaf_indices.get_indices().iter().map(|idx| (*idx, L::default())).collect(), )); } - let main_subtree = SubTree { - sorted_leaf_indices, - root_index: NodeIndex::ROOT, - root_hash, - }; - let mut skeleton_tree = Self { - nodes: HashMap::new(), - sorted_leaf_indices, - }; + let main_subtree = SubTree { sorted_leaf_indices, root_index: NodeIndex::ROOT, root_hash }; + let mut skeleton_tree = Self { nodes: HashMap::new(), sorted_leaf_indices }; let mut leaves = HashMap::new(); skeleton_tree.fetch_nodes::(vec![main_subtree], storage, config, Some(&mut leaves))?; Ok((skeleton_tree, leaves)) @@ -320,10 +295,7 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { } fn create_empty(sorted_leaf_indices: SortedLeafIndices<'a>) -> Self { - Self { - nodes: HashMap::new(), - sorted_leaf_indices, - } + Self { nodes: HashMap::new(), sorted_leaf_indices } } /// Handles a subtree referred by an edge or a binary node. Decides whether we deserialize the @@ -332,9 +304,9 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { &mut self, next_subtrees: &mut Vec>, subtree: SubTree<'a>, - should_fetch_leaves: bool, + should_fetch_modified_leaves: bool, ) { - if !subtree.is_leaf() || should_fetch_leaves { + if !subtree.is_leaf() || (should_fetch_modified_leaves && !subtree.is_unmodified()) { next_subtrees.push(subtree); } else if subtree.is_unmodified() { // Leaf sibling. @@ -348,7 +320,7 @@ impl<'a> OriginalSkeletonTreeImpl<'a> { /// Given leaf indices that were previously empty leaves, logs out a warning for trivial /// modification if a leaf is modified to an empty leaf. /// If this check is suppressed by configuration, does nothing. - fn log_warning_for_empty_leaves + Debug>( + fn log_warning_for_empty_leaves + Debug>( leaf_indices: &[T], config: &impl OriginalSkeletonTreeConfig, ) -> OriginalSkeletonTreeResult<()> { diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs index 4ab2aab9ed..4436c4bd5a 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs @@ -1,29 +1,31 @@ +use std::collections::HashMap; + +use ethnum::U256; +use pretty_assertions::assert_eq; +use rstest::rstest; + use super::OriginalSkeletonTreeImpl; use crate::block_committer::input::StarknetStorageValue; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; -use crate::patricia_merkle_tree::internal_test_utils::small_tree_index_to_full; -use crate::patricia_merkle_tree::node_data::inner_node::EdgePath; -use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToBottom}; +use crate::patricia_merkle_tree::internal_test_utils::{ + small_tree_index_to_full, + MockLeaf, + OriginalSkeletonMockTrieConfig, +}; +use crate::patricia_merkle_tree::node_data::inner_node::{EdgePath, EdgePathLength, PathToBottom}; use crate::patricia_merkle_tree::node_data::leaf::{ContractState, LeafModifications}; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig; use crate::patricia_merkle_tree::original_skeleton_tree::create_tree::SubTree; use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTree; -use crate::patricia_merkle_tree::types::SubTreeHeight; -use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices}; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices, SubTreeHeight}; use crate::storage::db_object::DBObject; use crate::storage::map_storage::MapStorage; -use crate::storage::storage_trait::{create_db_key, StorageKey, StoragePrefix, StorageValue}; -use ethnum::U256; -use pretty_assertions::assert_eq; -use rstest::rstest; -use std::collections::HashMap; +use crate::storage::storage_trait::{create_db_key, StarknetPrefix, StorageKey, StorageValue}; #[rstest] // This test assumes for simplicity that hash is addition (i.e hash(a,b) = a + b). -/// /// Old tree structure: /// /// 50 @@ -45,8 +47,6 @@ use std::collections::HashMap; /// B E * /// / \ \ \ /// NZ 9 11 15 -/// -/// #[case::simple_tree_of_height_3( HashMap::from([ @@ -56,12 +56,12 @@ use std::collections::HashMap; create_binary_entry(17, 13), create_edge_entry(15, 3, 2), create_binary_entry(30, 20), - create_storage_leaf_entry(8), - create_storage_leaf_entry(9), - create_storage_leaf_entry(11), - create_storage_leaf_entry(15) + create_mock_leaf_entry(8), + create_mock_leaf_entry(9), + create_mock_leaf_entry(11), + create_mock_leaf_entry(15) ]).into(), - create_leaf_modifications(vec![(8, 8), (10, 3), (13, 2)]), + create_mock_leaf_modifications(vec![(8, 8), (10, 3), (13, 2)]), HashOutput(Felt::from(50_u128 + 248_u128)), create_expected_skeleton_nodes( vec![ @@ -99,7 +99,6 @@ use std::collections::HashMap; /// B E E /// / \ \ \ /// NZ 2 NZ NZ -/// #[case::another_simple_tree_of_height_3( HashMap::from([ @@ -110,13 +109,13 @@ use std::collections::HashMap; create_edge_entry(12, 0, 1), create_binary_entry(5, 11), create_binary_entry(13, 16), - create_storage_leaf_entry(10), - create_storage_leaf_entry(2), - create_storage_leaf_entry(3), - create_storage_leaf_entry(4), - create_storage_leaf_entry(7) + create_mock_leaf_entry(10), + create_mock_leaf_entry(2), + create_mock_leaf_entry(3), + create_mock_leaf_entry(4), + create_mock_leaf_entry(7) ]).into(), - create_leaf_modifications(vec![(8, 5), (11, 1), (13, 3)]), + create_mock_leaf_modifications(vec![(8, 5), (11, 1), (13, 3)]), HashOutput(Felt::from(29_u128 + 248_u128)), create_expected_skeleton_nodes( vec![ @@ -138,7 +137,7 @@ use std::collections::HashMap; /// / \ /// 26 90 /// / / \ -/// * 25 65 +/// * 25 65 /// / \ / \ /// 24 * 6 59 /// / \ \ / / \ @@ -152,12 +151,11 @@ use std::collections::HashMap; /// / \ /// E B /// / / \ -/// * E B +/// * E B /// / \ / \ /// 24 * E B /// \ / \ /// 20 5 40 -/// #[case::tree_of_height_4_with_long_edge( HashMap::from([ create_root_edge_entry(116, SubTreeHeight::new(4)), @@ -169,14 +167,14 @@ use std::collections::HashMap; create_edge_entry(24, 0, 2), create_binary_entry(25, 65), create_binary_entry(26, 90), - create_storage_leaf_entry(11), - create_storage_leaf_entry(13), - create_storage_leaf_entry(20), - create_storage_leaf_entry(5), - create_storage_leaf_entry(19), - create_storage_leaf_entry(40), + create_mock_leaf_entry(11), + create_mock_leaf_entry(13), + create_mock_leaf_entry(20), + create_mock_leaf_entry(5), + create_mock_leaf_entry(19), + create_mock_leaf_entry(40), ]).into(), - create_leaf_modifications(vec![(18, 5), (25, 1), (29, 15), (30, 19)]), + create_mock_leaf_modifications(vec![(18, 5), (25, 1), (29, 15), (30, 19)]), HashOutput(Felt::from(116_u128 + 247_u128)), create_expected_skeleton_nodes( vec![ @@ -198,21 +196,20 @@ use std::collections::HashMap; )] fn test_create_tree( #[case] storage: MapStorage, - #[case] leaf_modifications: LeafModifications, + #[case] leaf_modifications: LeafModifications, #[case] root_hash: HashOutput, #[case] expected_skeleton_nodes: HashMap, #[case] subtree_height: SubTreeHeight, #[values(true, false)] compare_modified_leaves: bool, ) { - let leaf_modifications: LeafModifications = leaf_modifications + let leaf_modifications: LeafModifications = leaf_modifications .into_iter() .map(|(idx, leaf)| (NodeIndex::from_subtree_index(idx, subtree_height), leaf)) .collect(); - let config = - OriginalSkeletonStorageTrieConfig::new(&leaf_modifications, compare_modified_leaves); + let config = OriginalSkeletonMockTrieConfig::new(&leaf_modifications, compare_modified_leaves); let mut sorted_leaf_indices: Vec = leaf_modifications.keys().copied().collect(); let sorted_leaf_indices = SortedLeafIndices::new(&mut sorted_leaf_indices); - let skeleton_tree = OriginalSkeletonTreeImpl::create::( + let skeleton_tree = OriginalSkeletonTreeImpl::create::( &storage, root_hash, sorted_leaf_indices, @@ -298,7 +295,7 @@ fn test_create_tree( /// /// 1 /// / \ -/// * * +/// * * /// / \ / /// 4 5 6 /// / \ / \ / @@ -310,7 +307,6 @@ fn test_create_tree( /// 5 /// / \ /// 11 10 -/// #[rstest] #[case::single_right_child( SubTreeHeight(1), @@ -389,11 +385,7 @@ fn test_get_bottom_subtree( ); // Create the input Subtree. - let tree = SubTree { - sorted_leaf_indices, - root_index, - root_hash: HashOutput(Felt::ONE), - }; + let tree = SubTree { sorted_leaf_indices, root_index, root_hash: HashOutput(Felt::ONE) }; // Get the bottom subtree. let (subtree, previously_empty_leaf_indices) = @@ -407,10 +399,7 @@ fn test_get_bottom_subtree( root_index: expected_root_index, root_hash: HashOutput(Felt::TWO), }; - assert_eq!( - previously_empty_leaf_indices, - expected_previously_empty_leaf_indices - ); + assert_eq!(previously_empty_leaf_indices, expected_previously_empty_leaf_indices); assert_eq!(subtree, expected_subtree); } @@ -418,6 +407,11 @@ pub(crate) fn create_32_bytes_entry(simple_val: u128) -> [u8; 32] { U256::from(simple_val).to_be_bytes() } +pub(crate) fn create_mock_leaf_entry(val: u128) -> (StorageKey, StorageValue) { + let leaf = MockLeaf(Felt::from(val)); + (leaf.get_db_key(&leaf.0.to_bytes_be()), leaf.serialize()) +} + pub(crate) fn create_storage_leaf_entry(val: u128) -> (StorageKey, StorageValue) { let leaf = StarknetStorageValue(Felt::from(val)); (leaf.get_db_key(&leaf.0.to_bytes_be()), leaf.serialize()) @@ -439,15 +433,12 @@ pub(crate) fn create_contract_state_leaf_entry(val: u128) -> (StorageKey, Storag } fn create_patricia_key(val: u128) -> StorageKey { - create_db_key(StoragePrefix::InnerNode, &U256::from(val).to_be_bytes()) + create_db_key(StarknetPrefix::InnerNode.to_storage_prefix(), &U256::from(val).to_be_bytes()) } fn create_binary_val(left: u128, right: u128) -> StorageValue { StorageValue( - (create_32_bytes_entry(left) - .into_iter() - .chain(create_32_bytes_entry(right))) - .collect(), + (create_32_bytes_entry(left).into_iter().chain(create_32_bytes_entry(right))).collect(), ) } @@ -461,27 +452,21 @@ fn create_edge_val(hash: u128, path: u128, length: u8) -> StorageValue { ) } -fn create_leaf_modifications( +fn create_mock_leaf_modifications( leaf_modifications: Vec<(u128, u128)>, -) -> LeafModifications { +) -> LeafModifications { leaf_modifications .into_iter() - .map(|(idx, val)| (NodeIndex::from(idx), StarknetStorageValue(Felt::from(val)))) + .map(|(idx, val)| (NodeIndex::from(idx), MockLeaf(Felt::from(val)))) .collect() } pub(crate) fn create_binary_entry(left: u128, right: u128) -> (StorageKey, StorageValue) { - ( - create_patricia_key(left + right), - create_binary_val(left, right), - ) + (create_patricia_key(left + right), create_binary_val(left, right)) } pub(crate) fn create_edge_entry(hash: u128, path: u128, length: u8) -> (StorageKey, StorageValue) { - ( - create_patricia_key(hash + path + u128::from(length)), - create_edge_val(hash, path, length), - ) + (create_patricia_key(hash + path + u128::from(length)), create_edge_val(hash, path, length)) } pub(crate) fn create_expected_skeleton_nodes( @@ -491,12 +476,7 @@ pub(crate) fn create_expected_skeleton_nodes( let subtree_height = SubTreeHeight::new(height); nodes .into_iter() - .map(|(node_index, node)| { - ( - NodeIndex::from_subtree_index(node_index, subtree_height), - node, - ) - }) + .map(|(node_index, node)| (NodeIndex::from_subtree_index(node_index, subtree_height), node)) .chain([( NodeIndex::ROOT, OriginalSkeletonNode::Edge( @@ -541,7 +521,7 @@ pub(crate) fn create_root_edge_entry( let length = SubTreeHeight::ACTUAL_HEIGHT.0 - subtree_height.0; let new_root = old_root + u128::from(length); let key = create_db_key( - StoragePrefix::InnerNode, + StarknetPrefix::InnerNode.to_storage_prefix(), &Felt::from(new_root).to_bytes_be(), ); let value = StorageValue( @@ -559,8 +539,5 @@ fn create_previously_empty_leaf_indices<'a>( tree_leaf_indices: &'a [NodeIndex], subtree_leaf_indices: &'a [NodeIndex], ) -> Vec<&'a NodeIndex> { - tree_leaf_indices - .iter() - .filter(|idx| !subtree_leaf_indices.contains(idx)) - .collect() + tree_leaf_indices.iter().filter(|idx| !subtree_leaf_indices.contains(idx)).collect() } diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/errors.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/errors.rs index 27f86272e3..f760ae16cd 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/errors.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/errors.rs @@ -2,10 +2,8 @@ use std::fmt::Debug; use thiserror::Error; -use crate::{ - patricia_merkle_tree::types::NodeIndex, - storage::errors::{DeserializationError, StorageError}, -}; +use crate::patricia_merkle_tree::types::NodeIndex; +use crate::storage::errors::{DeserializationError, StorageError}; #[derive(Debug, Error)] pub enum OriginalSkeletonTreeError { @@ -14,8 +12,8 @@ pub enum OriginalSkeletonTreeError { )] Deserialization(#[from] DeserializationError), #[error( - "Unable to read from storage the storage key: {0:?} while building the \ - original skeleton tree." + "Unable to read from storage the storage key: {0:?} while building the original skeleton \ + tree." )] StorageRead(#[from] StorageError), #[error("Failed to read the modified leaf at index {0:?}")] diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest.rs index 7298e0c809..8b8c09f4c1 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest.rs @@ -1,22 +1,21 @@ -use crate::block_committer::input::Config; -use crate::block_committer::input::ContractAddress; -use crate::block_committer::input::StarknetStorageValue; -use crate::block_committer::input::StateDiff; -use crate::forest_errors::ForestError; -use crate::forest_errors::ForestResult; +use std::collections::HashMap; + +use crate::block_committer::input::{Config, ContractAddress, StarknetStorageValue}; +use crate::forest_errors::{ForestError, ForestResult}; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use crate::patricia_merkle_tree::node_data::leaf::ContractState; -use crate::patricia_merkle_tree::node_data::leaf::LeafModifications; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonClassesTrieConfig; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonContractsTrieConfig; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTree; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl; -use crate::patricia_merkle_tree::types::NodeIndex; -use crate::patricia_merkle_tree::types::SortedLeafIndices; +use crate::patricia_merkle_tree::node_data::leaf::{ContractState, LeafModifications}; +use crate::patricia_merkle_tree::original_skeleton_tree::config::{ + OriginalSkeletonClassesTrieConfig, + OriginalSkeletonContractsTrieConfig, + OriginalSkeletonStorageTrieConfig, +}; +use crate::patricia_merkle_tree::original_skeleton_tree::tree::{ + OriginalSkeletonTree, + OriginalSkeletonTreeImpl, +}; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices}; use crate::storage::storage_trait::Storage; -use std::collections::HashMap; #[cfg(test)] #[path = "skeleton_forest_test.rs"] @@ -30,14 +29,15 @@ pub(crate) struct OriginalSkeletonForest<'a> { } impl<'a> OriginalSkeletonForest<'a> { - /// Creates an original skeleton forest that includes the storage tries of the modified contracts, - /// the classes trie and the contracts trie. Additionally, returns the original contract states that - /// are needed to compute the contract state tree. + /// Creates an original skeleton forest that includes the storage tries of the modified + /// contracts, the classes trie and the contracts trie. Additionally, returns the original + /// contract states that are needed to compute the contract state tree. pub(crate) fn create( storage: impl Storage, contracts_trie_root_hash: HashOutput, classes_trie_root_hash: HashOutput, - state_diff: &StateDiff, + storage_updates: &HashMap>, + classes_updates: &LeafModifications, forest_sorted_indices: &ForestSortedIndices<'a>, config: &impl Config, ) -> ForestResult<(Self, HashMap)> @@ -50,28 +50,21 @@ impl<'a> OriginalSkeletonForest<'a> { forest_sorted_indices.contracts_trie_sorted_indices, )?; let storage_tries = Self::create_storage_tries( - &state_diff.actual_storage_updates(), + storage_updates, &original_contracts_trie_leaves, &storage, config, &forest_sorted_indices.storage_tries_sorted_indices, )?; let classes_trie = Self::create_classes_trie( - &state_diff.actual_classes_updates(), + classes_updates, classes_trie_root_hash, &storage, config, forest_sorted_indices.classes_trie_sorted_indices, )?; - Ok(( - Self { - classes_trie, - contracts_trie, - storage_tries, - }, - original_contracts_trie_leaves, - )) + Ok((Self { classes_trie, contracts_trie, storage_tries }, original_contracts_trie_leaves)) } /// Creates the contracts trie original skeleton. @@ -80,10 +73,7 @@ impl<'a> OriginalSkeletonForest<'a> { contracts_trie_root_hash: HashOutput, storage: &impl Storage, contracts_trie_sorted_indices: SortedLeafIndices<'a>, - ) -> ForestResult<( - OriginalSkeletonTreeImpl<'a>, - HashMap, - )> { + ) -> ForestResult<(OriginalSkeletonTreeImpl<'a>, HashMap)> { Ok(OriginalSkeletonTreeImpl::create_and_get_previous_leaves( storage, contracts_trie_root_hash, diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs index 193137c8cb..c3c63ecd0f 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs @@ -1,31 +1,38 @@ +use std::collections::HashMap; + use pretty_assertions::assert_eq; use rstest::rstest; -use std::collections::HashMap; use super::OriginalSkeletonForest; use crate::block_committer::commit::get_all_modified_indices; use crate::block_committer::input::{ - ConfigImpl, ContractAddress, Input, StarknetStorageKey, StarknetStorageValue, StateDiff, + ConfigImpl, + ContractAddress, + Input, + StarknetStorageKey, + StarknetStorageValue, + StateDiff, }; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; use crate::patricia_merkle_tree::node_data::leaf::ContractState; use crate::patricia_merkle_tree::original_skeleton_tree::create_tree::create_tree_test::{ - create_32_bytes_entry, create_binary_entry, create_binary_skeleton_node, create_edge_entry, - create_edge_skeleton_node, create_expected_skeleton_nodes, - create_unmodified_subtree_skeleton_node, -}; -use crate::patricia_merkle_tree::original_skeleton_tree::create_tree::create_tree_test::{ - create_compiled_class_leaf_entry, create_contract_state_leaf_entry, create_root_edge_entry, + create_32_bytes_entry, + create_binary_entry, + create_binary_skeleton_node, + create_compiled_class_leaf_entry, + create_contract_state_leaf_entry, + create_edge_entry, + create_edge_skeleton_node, + create_expected_skeleton_nodes, + create_root_edge_entry, create_storage_leaf_entry, + create_unmodified_subtree_skeleton_node, }; -use crate::patricia_merkle_tree::types::NodeIndex; -use crate::patricia_merkle_tree::types::SubTreeHeight; -use crate::patricia_merkle_tree::{ - original_skeleton_tree::skeleton_forest::ForestSortedIndices, - original_skeleton_tree::tree::OriginalSkeletonTreeImpl, types::SortedLeafIndices, -}; +use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::ForestSortedIndices; +use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices, SubTreeHeight}; use crate::storage::map_storage::MapStorage; macro_rules! compare_skeleton_tree { @@ -85,7 +92,7 @@ macro_rules! compare_skeleton_tree { /// / \ / /// E E B /// / \ / \ -/// * B B E +/// * B B E /// / / \ \ /// 303 NZ 47 UB /// @@ -99,7 +106,6 @@ macro_rules! compare_skeleton_tree { /// B E E B E * /// / \ \ \ / \ \ \ /// NZ 2 NZ NZ NZ 9 16 15 -/// #[rstest] #[case( @@ -165,7 +171,7 @@ macro_rules! compare_skeleton_tree { }, contracts_trie_root_hash: HashOutput(Felt::from(861_u128 + 248_u128)), classes_trie_root_hash: HashOutput(Felt::from(155_u128 + 248_u128)), - config: ConfigImpl::new(true), + config: ConfigImpl::new(true, log::LevelFilter::Debug), }, OriginalSkeletonForest{ classes_trie: OriginalSkeletonTreeImpl { nodes: create_expected_skeleton_nodes( @@ -284,19 +290,17 @@ fn test_create_original_skeleton_forest( MapStorage::from(input.storage), input.contracts_trie_root_hash, input.classes_trie_root_hash, - &input.state_diff, + &input.state_diff.actual_storage_updates(), + &input.state_diff.actual_classes_updates(), &forest_sorted_indices, - &ConfigImpl::new(false), + &ConfigImpl::new(false, log::LevelFilter::Debug), ) .unwrap(); let expected_original_contracts_trie_leaves = expected_original_contracts_trie_leaves .into_iter() .map(|(address, state)| (NodeIndex::from_contract_address(&address), state)) .collect(); - assert_eq!( - original_contracts_trie_leaves, - expected_original_contracts_trie_leaves - ); + assert_eq!(original_contracts_trie_leaves, expected_original_contracts_trie_leaves); compare_skeleton_tree!( &actual_forest.classes_trie, @@ -375,15 +379,9 @@ fn create_original_skeleton_with_sorted_indices<'a>( indices: SortedLeafIndices<'a>, skeleton: &OriginalSkeletonTreeImpl<'_>, ) -> OriginalSkeletonTreeImpl<'a> { - OriginalSkeletonTreeImpl { - nodes: skeleton.nodes.clone(), - sorted_leaf_indices: indices, - } + OriginalSkeletonTreeImpl { nodes: skeleton.nodes.clone(), sorted_leaf_indices: indices } } fn create_expected_sorted_indices(indices: &[u128]) -> Vec { - indices - .iter() - .map(|idx| NodeIndex::FIRST_LEAF + NodeIndex::from(*idx)) - .collect() + indices.iter().map(|idx| NodeIndex::FIRST_LEAF + NodeIndex::from(*idx)).collect() } diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/tree.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/tree.rs index 33a2cfad71..68a195eb9c 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/tree.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/tree.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use crate::hash::hash_trait::HashOutput; -use crate::patricia_merkle_tree::node_data::leaf::LeafData; +use crate::patricia_merkle_tree::node_data::leaf::Leaf; use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonTreeConfig; use crate::patricia_merkle_tree::original_skeleton_tree::errors::OriginalSkeletonTreeError; use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; @@ -16,7 +16,7 @@ pub(crate) type OriginalSkeletonTreeResult = Result: Sized { - fn create( + fn create( storage: &impl Storage, root_hash: HashOutput, sorted_leaf_indices: SortedLeafIndices<'a>, @@ -27,7 +27,7 @@ pub(crate) trait OriginalSkeletonTree<'a>: Sized { fn get_nodes_mut(&mut self) -> &mut OriginalSkeletonNodeMap; - fn create_and_get_previous_leaves( + fn create_and_get_previous_leaves( storage: &impl Storage, root_hash: HashOutput, sorted_leaf_indices: SortedLeafIndices<'a>, @@ -46,7 +46,7 @@ pub(crate) struct OriginalSkeletonTreeImpl<'a> { } impl<'a> OriginalSkeletonTree<'a> for OriginalSkeletonTreeImpl<'a> { - fn create( + fn create( storage: &impl Storage, root_hash: HashOutput, sorted_leaf_indices: SortedLeafIndices<'a>, @@ -63,7 +63,7 @@ impl<'a> OriginalSkeletonTree<'a> for OriginalSkeletonTreeImpl<'a> { &mut self.nodes } - fn create_and_get_previous_leaves( + fn create_and_get_previous_leaves( storage: &impl Storage, root_hash: HashOutput, sorted_leaf_indices: SortedLeafIndices<'a>, diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs index 09a2046773..ac15f61476 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs @@ -23,8 +23,8 @@ pub(crate) fn split_leaves<'a>( let assert_descendant = |leaf_index: &NodeIndex| { if (*leaf_index >> u8::from(root_height)) != *root_index { panic!( - "Leaf {leaf_index:?} is not a descendant of the root {root_index:?} \ - (root height={root_height:?})." + "Leaf {leaf_index:?} is not a descendant of the root {root_index:?} (root \ + height={root_height:?})." ); } }; @@ -33,11 +33,7 @@ pub(crate) fn split_leaves<'a>( assert_descendant(first_leaf); if leaf_indices.len() > 1 { - assert_descendant( - leaf_indices - .last() - .expect("leaf_indices unexpectedly empty."), - ); + assert_descendant(leaf_indices.last().expect("leaf_indices unexpectedly empty.")); } let right_child_index = (*root_index << 1) + 1; diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs index 52590b4675..da55e02e75 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs +++ b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs @@ -1,15 +1,17 @@ -use super::split_leaves; -use crate::patricia_merkle_tree::external_test_utils::get_random_u256; -use crate::patricia_merkle_tree::internal_test_utils::as_fully_indexed; -use crate::patricia_merkle_tree::internal_test_utils::random; -use crate::patricia_merkle_tree::internal_test_utils::small_tree_index_to_full; -use crate::patricia_merkle_tree::types::SortedLeafIndices; -use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight}; use ethnum::{uint, U256}; use rand::rngs::ThreadRng; use rand::Rng; use rstest::rstest; +use super::split_leaves; +use crate::patricia_merkle_tree::external_test_utils::get_random_u256; +use crate::patricia_merkle_tree::internal_test_utils::{ + as_fully_indexed, + random, + small_tree_index_to_full, +}; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices, SubTreeHeight}; + /// Creates an array of increasing random U256 numbers, with jumps of up to 'jump' between two /// consecutive numbers. fn create_increasing_random_array( @@ -62,10 +64,7 @@ fn test_split_leaves( SortedLeafIndices::new(&mut left_full_indices), SortedLeafIndices::new(&mut right_full_indices), ]; - assert_eq!( - split_leaves(&root_index, &SortedLeafIndices::new(&mut leaf_indices)), - expected - ); + assert_eq!(split_leaves(&root_index, &SortedLeafIndices::new(&mut leaf_indices)), expected); } #[rstest] @@ -86,16 +85,8 @@ fn test_split_leaves_big_tree(mut random: ThreadRng) { SubTreeHeight::ACTUAL_HEIGHT.into(), NodeIndex::ROOT.into(), [ - left_leaf_indices - .clone() - .into_iter() - .map(NodeIndex::new) - .collect::>(), - right_leaf_indices - .clone() - .into_iter() - .map(NodeIndex::new) - .collect(), + left_leaf_indices.clone().into_iter().map(NodeIndex::new).collect::>(), + right_leaf_indices.clone().into_iter().map(NodeIndex::new).collect(), ] .concat(), left_leaf_indices.as_slice(), diff --git a/crates/committer/src/patricia_merkle_tree/types.rs b/crates/committer/src/patricia_merkle_tree/types.rs index 474112ce76..690de8e913 100644 --- a/crates/committer/src/patricia_merkle_tree/types.rs +++ b/crates/committer/src/patricia_merkle_tree/types.rs @@ -1,12 +1,11 @@ +use ethnum::U256; + use crate::block_committer::input::{ContractAddress, StarknetStorageKey}; use crate::felt::Felt; use crate::patricia_merkle_tree::errors::TypesError; use crate::patricia_merkle_tree::filled_tree::node::ClassHash; use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToBottom}; -use bisection::{bisect_left, bisect_right}; -use ethnum::U256; - #[cfg(test)] #[path = "types_test.rs"] pub mod types_test; @@ -50,10 +49,8 @@ impl NodeIndex { #[allow(clippy::as_conversions)] /// [NodeIndex] constant that represents the largest index in a tree. // TODO(Tzahi, 15/6/2024): Support height < 128 bits. - pub const MAX: Self = Self(U256::from_words( - u128::MAX >> (U256::BITS - Self::BITS as u32), - u128::MAX, - )); + pub const MAX: Self = + Self(U256::from_words(u128::MAX >> (U256::BITS - Self::BITS as u32), u128::MAX)); pub fn new(index: U256) -> Self { assert!(index <= Self::MAX.0, "Index {index} is too large."); @@ -127,11 +124,8 @@ impl NodeIndex { panic!("The descendant is not a really descendant of the node."); }; - PathToBottom::new( - delta.0.into(), - EdgePathLength::new(distance).expect("Illegal length"), - ) - .expect("Illegal PathToBottom") + PathToBottom::new(delta.0.into(), EdgePathLength::new(distance).expect("Illegal length")) + .expect("Illegal PathToBottom") } pub(crate) fn from_starknet_storage_key(key: &StarknetStorageKey) -> Self { @@ -230,12 +224,14 @@ pub(crate) struct SortedLeafIndices<'a>(&'a [NodeIndex]); impl<'a> SortedLeafIndices<'a> { /// Creates a new instance by sorting the given indices. + // TODO(Nimrod, 1/8/2024): Remove duplicates from the given indices. pub(crate) fn new(indices: &'a mut [NodeIndex]) -> Self { indices.sort(); Self(indices) } - /// Returns a subslice of the indices stored at self, at the range [leftmost_idx, rightmost_idx). + /// Returns a subslice of the indices stored at self, at the range [leftmost_idx, + /// rightmost_idx). pub(crate) fn subslice(&self, leftmost_idx: usize, rightmost_idx: usize) -> Self { Self(&self.0[leftmost_idx..rightmost_idx]) } @@ -270,11 +266,20 @@ impl<'a> SortedLeafIndices<'a> { self.0.first() } + /// Returns the leftmost position where `leftmost_value` can be inserted to the slice and + /// maintain sorted order. Assumes that the elements in the slice are unique. pub(crate) fn bisect_left(&self, leftmost_value: &NodeIndex) -> usize { - bisect_left(self.0, leftmost_value) + match self.0.binary_search(leftmost_value) { + Ok(pos) | Err(pos) => pos, + } } + /// Returns the rightmost position where `rightmost_value` can be inserted to the slice and + /// maintain sorted order. Assumes that the elements in the slice are unique. pub(crate) fn bisect_right(&self, rightmost_value: &NodeIndex) -> usize { - bisect_right(self.0, rightmost_value) + match self.0.binary_search(rightmost_value) { + Err(pos) => pos, + Ok(pos) => pos + 1, + } } } diff --git a/crates/committer/src/patricia_merkle_tree/types_test.rs b/crates/committer/src/patricia_merkle_tree/types_test.rs index 1db478d6a9..c5657d02cd 100644 --- a/crates/committer/src/patricia_merkle_tree/types_test.rs +++ b/crates/committer/src/patricia_merkle_tree/types_test.rs @@ -1,3 +1,8 @@ +use ethnum::{uint, U256}; +use rand::rngs::ThreadRng; +use rand::Rng; +use rstest::rstest; + use crate::block_committer::input::{ContractAddress, StarknetStorageKey}; use crate::felt::Felt; use crate::patricia_merkle_tree::external_test_utils::get_random_u256; @@ -5,11 +10,6 @@ use crate::patricia_merkle_tree::internal_test_utils::random; use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToBottom}; use crate::patricia_merkle_tree::types::NodeIndex; -use ethnum::{uint, U256}; -use rand::rngs::ThreadRng; -use rand::Rng; -use rstest::rstest; - #[rstest] #[case(1, 1, 1, 3)] #[case(1, 0, 2, 4)] @@ -76,11 +76,8 @@ fn test_get_lca(#[case] node_index: U256, #[case] other: U256, #[case] expected: #[rstest] fn test_get_lca_big(mut random: ThreadRng) { - let lca = NodeIndex::new(get_random_u256( - &mut random, - U256::ZERO, - (NodeIndex::MAX >> 1).into(), - )); + let lca = + NodeIndex::new(get_random_u256(&mut random, U256::ZERO, (NodeIndex::MAX >> 1).into())); let left_child = lca << 1; let right_child = left_child + 1; @@ -114,10 +111,7 @@ fn test_get_path_to_descendant( let descendant = NodeIndex::new(descendant.into()); let path_to_bottom = root_index.get_path_to_descendant(descendant); assert_eq!(path_to_bottom.path, U256::from(expected_path).into()); - assert_eq!( - path_to_bottom.length, - EdgePathLength::new(expected_length).unwrap() - ); + assert_eq!(path_to_bottom.length, EdgePathLength::new(expected_length).unwrap()); } #[rstest] @@ -130,10 +124,7 @@ fn test_get_path_to_descendant_big() { let descendant = (root_index << extension_index.bit_length()) + extension_index; let path_to_bottom = root_index.get_path_to_descendant(descendant); assert_eq!(path_to_bottom.path, extension.into()); - assert_eq!( - path_to_bottom.length, - EdgePathLength::new(extension_index.bit_length()).unwrap() - ); + assert_eq!(path_to_bottom.length, EdgePathLength::new(extension_index.bit_length()).unwrap()); } #[rstest] diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper.rs index b32b44147d..69d3a29dc7 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper.rs @@ -1,20 +1,21 @@ use std::collections::HashMap; -use crate::patricia_merkle_tree::node_data::inner_node::EdgePathLength; -use crate::patricia_merkle_tree::node_data::inner_node::PathToBottom; -use crate::patricia_merkle_tree::node_data::leaf::LeafModifications; -use crate::patricia_merkle_tree::node_data::leaf::SkeletonLeaf; +use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToBottom}; +use crate::patricia_merkle_tree::node_data::leaf::{LeafModifications, SkeletonLeaf}; use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonNodeMap; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTree; +use crate::patricia_merkle_tree::original_skeleton_tree::tree::{ + OriginalSkeletonNodeMap, + OriginalSkeletonTree, +}; use crate::patricia_merkle_tree::original_skeleton_tree::utils::split_leaves; -use crate::patricia_merkle_tree::types::NodeIndex; -use crate::patricia_merkle_tree::types::SortedLeafIndices; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices}; use crate::patricia_merkle_tree::updated_skeleton_tree::errors::UpdatedSkeletonTreeError; use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode; -use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonNodeMap; -use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl; -use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeResult; +use crate::patricia_merkle_tree::updated_skeleton_tree::tree::{ + UpdatedSkeletonNodeMap, + UpdatedSkeletonTreeImpl, + UpdatedSkeletonTreeResult, +}; #[cfg(test)] #[path = "create_tree_helper_test.rs"] @@ -63,9 +64,7 @@ fn has_leaves_on_both_sides(root_index: &NodeIndex, leaf_indices: &SortedLeafInd if leaf_indices.is_empty() { return false; } - split_leaves(root_index, leaf_indices) - .iter() - .all(|leaves_in_side| !leaves_in_side.is_empty()) + split_leaves(root_index, leaf_indices).iter().all(|leaves_in_side| !leaves_in_side.is_empty()) } impl UpdatedSkeletonTreeImpl { @@ -80,17 +79,12 @@ impl UpdatedSkeletonTreeImpl { .iter() .filter(|(_, leaf)| !leaf.is_zero()) .map(|(index, _)| (*index, UpdatedSkeletonNode::Leaf)) - .chain( - original_skeleton - .get_nodes() - .iter() - .filter_map(|(index, node)| match node { - OriginalSkeletonNode::UnmodifiedSubTree(hash) => { - Some((*index, UpdatedSkeletonNode::UnmodifiedSubTree(*hash))) - } - OriginalSkeletonNode::Binary | OriginalSkeletonNode::Edge(_) => None, - }), - ) + .chain(original_skeleton.get_nodes().iter().filter_map(|(index, node)| match node { + OriginalSkeletonNode::UnmodifiedSubTree(hash) => { + Some((*index, UpdatedSkeletonNode::UnmodifiedSubTree(*hash))) + } + OriginalSkeletonNode::Binary | OriginalSkeletonNode::Edge(_) => None, + })) .collect() } @@ -127,8 +121,9 @@ impl UpdatedSkeletonTreeImpl { "Unexpected leaf index (root_index={root_index:?}, leaf_indices={leaf_indices:?})." ); if !self.skeleton_tree.contains_key(root_index) { - // "Deletion" of an original empty leaf (as non-zero leaf modifications are finalized in `finalize_bottom_layer`). - // Supported but not expected. + // "Deletion" of an original empty leaf (as non-zero leaf modifications are + // finalized in `finalize_bottom_layer`). Supported but not + // expected. return TempSkeletonNode::Empty; } return TempSkeletonNode::Leaf; @@ -150,7 +145,8 @@ impl UpdatedSkeletonTreeImpl { self.node_from_edge_data(&path_to_lca, &bottom_index, &bottom) } - /// Updates the Patricia tree rooted at the given index, with the given leaves; returns the root. + /// Updates the Patricia tree rooted at the given index, with the given leaves; returns the + /// root. pub(crate) fn update_node_in_nonempty_tree( &mut self, root_index: &NodeIndex, @@ -288,7 +284,7 @@ impl UpdatedSkeletonTreeImpl { assert!( self.skeleton_tree.contains_key(bottom_index), "bottom {bottom_index:?} is a non-empty leaf but doesn't appear in the \ - skeleton." + skeleton." ); return TempSkeletonNode::Original(OriginalSkeletonNode::Edge(*path)); } @@ -301,8 +297,7 @@ impl UpdatedSkeletonTreeImpl { } OriginalSkeletonNode::Binary => { // Finalize bottom - a binary descendant cannot change form. - self.skeleton_tree - .insert(*bottom_index, UpdatedSkeletonNode::Binary); + self.skeleton_tree.insert(*bottom_index, UpdatedSkeletonNode::Binary); OriginalSkeletonNode::Edge(*path) } OriginalSkeletonNode::UnmodifiedSubTree(_) => OriginalSkeletonNode::Edge(*path), @@ -330,19 +325,9 @@ impl UpdatedSkeletonTreeImpl { empty_subtree_child_index, empty_subtree_leaf_indices, ) = if was_left_nonempty { - ( - left_child_index, - left_indices, - right_child_index, - right_indices, - ) + (left_child_index, left_indices, right_child_index, right_indices) } else { - ( - right_child_index, - right_indices, - left_child_index, - left_indices, - ) + (right_child_index, right_indices, left_child_index, left_indices) }; // 1. Handle the originally non-empty subtree, replacing the root with the child in the diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper_test.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper_test.rs index 6d0be5719e..520d6e54bc 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper_test.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/create_tree_helper_test.rs @@ -1,31 +1,38 @@ -use crate::block_committer::input::StarknetStorageValue; -use crate::patricia_merkle_tree::filled_tree::tree::FilledTree; -use crate::patricia_merkle_tree::filled_tree::tree::StorageTrie; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig; -use crate::patricia_merkle_tree::types::SortedLeafIndices; +use std::collections::HashMap; +use std::sync::Arc; + use ethnum::{uint, U256}; use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; -use std::collections::HashMap; -use std::sync::Arc; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; +use crate::patricia_merkle_tree::filled_tree::tree::FilledTree; use crate::patricia_merkle_tree::internal_test_utils::{ - as_fully_indexed, get_initial_updated_skeleton, small_tree_index_to_full, + as_fully_indexed, + get_initial_updated_skeleton, + small_tree_index_to_full, + MockLeaf, + MockTrie, + OriginalSkeletonMockTrieConfig, }; use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToBottom}; use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonNodeMap; -use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl; -use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight}; +use crate::patricia_merkle_tree::original_skeleton_tree::tree::{ + OriginalSkeletonNodeMap, + OriginalSkeletonTreeImpl, +}; +use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices, SubTreeHeight}; use crate::patricia_merkle_tree::updated_skeleton_tree::create_tree_helper::{ - get_path_to_lca, has_leaves_on_both_sides, TempSkeletonNode, + get_path_to_lca, + has_leaves_on_both_sides, + TempSkeletonNode, }; +use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl; use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode; -use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl; -use crate::patricia_merkle_tree::updated_skeleton_tree::{ - hash_function::TreeHashFunctionImpl, tree::UpdatedSkeletonTree, +use crate::patricia_merkle_tree::updated_skeleton_tree::tree::{ + UpdatedSkeletonTree, + UpdatedSkeletonTreeImpl, }; use crate::storage::map_storage::MapStorage; @@ -220,10 +227,7 @@ fn test_node_from_binary_data( expected_skeleton_tree.extend(expected_skeleton_additions.iter().cloned()); let temp_node = initial_updated_skeleton.node_from_binary_data(root_index, left, right); assert_eq!(temp_node, expected_node); - assert_eq!( - initial_updated_skeleton.skeleton_tree, - expected_skeleton_tree - ); + assert_eq!(initial_updated_skeleton.skeleton_tree, expected_skeleton_tree); } #[rstest] @@ -290,10 +294,7 @@ fn test_node_from_edge_data( expected_skeleton_tree.extend(expected_skeleton_additions.iter().cloned()); let temp_node = initial_updated_skeleton.node_from_edge_data(path, bottom_index, bottom); assert_eq!(temp_node, expected_node); - assert_eq!( - initial_updated_skeleton.skeleton_tree, - expected_skeleton_tree - ); + assert_eq!(initial_updated_skeleton.skeleton_tree, expected_skeleton_tree); } #[rstest] @@ -337,10 +338,7 @@ fn test_update_node_in_empty_tree( let temp_node = initial_updated_skeleton .update_node_in_empty_tree(root_index, &SortedLeafIndices::new(&mut leaf_indices)); assert_eq!(temp_node, expected_node); - assert_eq!( - initial_updated_skeleton.skeleton_tree, - expected_skeleton_tree - ); + assert_eq!(initial_updated_skeleton.skeleton_tree, expected_skeleton_tree); } #[rstest] @@ -490,10 +488,7 @@ fn test_update_node_in_nonempty_tree( &SortedLeafIndices::new(&mut leaf_indices), ); assert_eq!(temp_node, expected_node); - assert_eq!( - initial_updated_skeleton.skeleton_tree, - expected_skeleton_tree - ); + assert_eq!(initial_updated_skeleton.skeleton_tree, expected_skeleton_tree); } #[rstest] @@ -502,8 +497,8 @@ fn test_update_node_in_nonempty_tree( #[tokio::test] async fn test_update_non_modified_storage_tree(#[case] root_hash: HashOutput) { let empty_map = HashMap::new(); - let config = OriginalSkeletonStorageTrieConfig::new(&empty_map, false); - let mut original_skeleton_tree = OriginalSkeletonTreeImpl::create_impl::( + let config = OriginalSkeletonMockTrieConfig::new(&empty_map, false); + let mut original_skeleton_tree = OriginalSkeletonTreeImpl::create_impl::( &MapStorage::default(), root_hash, SortedLeafIndices::new(&mut []), @@ -512,9 +507,8 @@ async fn test_update_non_modified_storage_tree(#[case] root_hash: HashOutput) { .unwrap(); let updated = UpdatedSkeletonTreeImpl::create(&mut original_skeleton_tree, &HashMap::new()).unwrap(); - let filled = - StorageTrie::create::(Arc::new(updated), Arc::new(empty_map)) - .await - .unwrap(); + let filled = MockTrie::create::(Arc::new(updated), Arc::new(empty_map)) + .await + .unwrap(); assert_eq!(root_hash, filled.get_root_hash()); } diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs index 58cabf2332..0c2d17de95 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function.rs @@ -5,9 +5,12 @@ use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; use crate::patricia_merkle_tree::filled_tree::node::CompiledClassHash; use crate::patricia_merkle_tree::node_data::inner_node::{ - BinaryData, EdgeData, NodeData, PathToBottom, + BinaryData, + EdgeData, + NodeData, + PathToBottom, }; -use crate::patricia_merkle_tree::node_data::leaf::{ContractState, LeafData}; +use crate::patricia_merkle_tree::node_data::leaf::{ContractState, Leaf}; #[cfg(test)] #[path = "hash_function_test.rs"] @@ -35,7 +38,7 @@ impl HashFunction for PoseidonHashFunction { } } -pub(crate) trait TreeHashFunction { +pub(crate) trait TreeHashFunction { /// Computes the hash of the given leaf. fn compute_leaf_hash(leaf_data: &L) -> HashOutput; @@ -48,10 +51,9 @@ pub(crate) trait TreeHashFunction { node_data: &NodeData, ) -> HashOutput { match node_data { - NodeData::Binary(BinaryData { - left_hash, - right_hash, - }) => H::hash(&left_hash.0, &right_hash.0), + NodeData::Binary(BinaryData { left_hash, right_hash }) => { + H::hash(&left_hash.0, &right_hash.0) + } NodeData::Edge(EdgeData { bottom_hash: hash_output, path_to_bottom: PathToBottom { path, length, .. }, @@ -106,11 +108,8 @@ impl TreeHashFunction for TreeHashFunctionImpl { "could not parse hex string corresponding to b'CONTRACT_CLASS_LEAF_V0' to Felt", ); HashOutput( - Poseidon::hash( - &contract_class_leaf_version.into(), - &compiled_class_hash.0.into(), - ) - .into(), + Poseidon::hash(&contract_class_leaf_version.into(), &compiled_class_hash.0.into()) + .into(), ) } fn compute_node_hash(node_data: &NodeData) -> HashOutput { diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function_test.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function_test.rs index 8b80da1a31..a08b686d58 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function_test.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/hash_function_test.rs @@ -1,17 +1,22 @@ +use rstest::rstest; +use starknet_types_core::hash::{Pedersen, StarkHash}; + use crate::block_committer::input::StarknetStorageValue; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; -use crate::patricia_merkle_tree::filled_tree::node::Nonce; -use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash}; +use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; use crate::patricia_merkle_tree::node_data::inner_node::{ - BinaryData, EdgeData, EdgePathLength, NodeData, PathToBottom, + BinaryData, + EdgeData, + EdgePathLength, + NodeData, + PathToBottom, }; use crate::patricia_merkle_tree::node_data::leaf::ContractState; use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::{ - TreeHashFunction, TreeHashFunctionImpl, + TreeHashFunction, + TreeHashFunctionImpl, }; -use rstest::rstest; -use starknet_types_core::hash::{Pedersen, StarkHash}; #[rstest] #[case(Felt::ONE, Felt::TWO, Felt::from_hex("0x5bb9440e27889a364bcb678b1f679ecd1347acdedcbf36e83494f857cc58026").unwrap())] @@ -22,12 +27,10 @@ fn test_tree_hash_function_impl_binary_node( #[case] right_hash: Felt, #[case] expected_hash: Felt, ) { - let hash_output = TreeHashFunctionImpl::compute_node_hash( - &NodeData::::Binary(BinaryData { - left_hash: HashOutput(left_hash), - right_hash: HashOutput(right_hash), - }), - ); + let hash_output = + TreeHashFunctionImpl::compute_node_hash(&NodeData::::Binary( + BinaryData { left_hash: HashOutput(left_hash), right_hash: HashOutput(right_hash) }, + )); assert_eq!( hash_output, HashOutput(Pedersen::hash(&left_hash.into(), &right_hash.into()).into()) diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/skeleton_forest.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/skeleton_forest.rs index 3875f99177..d3257e1d1f 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/skeleton_forest.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/skeleton_forest.rs @@ -5,12 +5,16 @@ use crate::felt::Felt; use crate::forest_errors::{ForestError, ForestResult}; use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, Nonce}; use crate::patricia_merkle_tree::node_data::leaf::{ - ContractState, LeafModifications, SkeletonLeaf, + ContractState, + LeafModifications, + SkeletonLeaf, }; use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::OriginalSkeletonForest; use crate::patricia_merkle_tree::types::NodeIndex; -use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTree; -use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl; +use crate::patricia_merkle_tree::updated_skeleton_tree::tree::{ + UpdatedSkeletonTree, + UpdatedSkeletonTreeImpl, +}; pub(crate) struct UpdatedSkeletonForest { pub(crate) classes_trie: UpdatedSkeletonTreeImpl, @@ -71,11 +75,7 @@ impl UpdatedSkeletonForest { &contracts_trie_leaves, )?; - Ok(Self { - classes_trie, - contracts_trie, - storage_tries, - }) + Ok(Self { classes_trie, contracts_trie, storage_tries }) } /// Given the previous contract state, whether the contract's storage has become empty or not, diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs index 5e41f085da..5de0475a40 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs @@ -76,9 +76,7 @@ impl<'a> UpdatedSkeletonTree<'a> for UpdatedSkeletonTreeImpl { updated_skeleton_tree .skeleton_tree .insert(NodeIndex::ROOT, new_node) - .map_or((), |_| { - panic!("Root node already exists in the updated skeleton tree") - }) + .map_or((), |_| panic!("Root node already exists in the updated skeleton tree")) } }; Ok(updated_skeleton_tree) @@ -109,8 +107,6 @@ impl<'a> UpdatedSkeletonTree<'a> for UpdatedSkeletonTreeImpl { } fn get_nodes(&self) -> impl Iterator { - self.skeleton_tree - .iter() - .map(|(index, node)| (*index, node.clone())) + self.skeleton_tree.iter().map(|(index, node)| (*index, node.clone())) } } diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree_test.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree_test.rs index fefda311fc..8f9cb3d6ec 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree_test.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree_test.rs @@ -2,21 +2,25 @@ use std::collections::HashMap; use rstest::{fixture, rstest}; -use crate::block_committer::input::StarknetStorageValue; use crate::felt::Felt; use crate::hash::hash_trait::HashOutput; -use crate::patricia_merkle_tree::internal_test_utils::get_initial_updated_skeleton; +use crate::patricia_merkle_tree::internal_test_utils::{ + get_initial_updated_skeleton, + MockLeaf, + OriginalSkeletonMockTrieConfig, +}; use crate::patricia_merkle_tree::node_data::inner_node::PathToBottom; use crate::patricia_merkle_tree::node_data::leaf::{LeafModifications, SkeletonLeaf}; -use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig; use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode; use crate::patricia_merkle_tree::original_skeleton_tree::tree::{ - OriginalSkeletonTree, OriginalSkeletonTreeImpl, + OriginalSkeletonTree, + OriginalSkeletonTreeImpl, }; use crate::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices, SubTreeHeight}; use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode; use crate::patricia_merkle_tree::updated_skeleton_tree::tree::{ - UpdatedSkeletonTree, UpdatedSkeletonTreeImpl, + UpdatedSkeletonTree, + UpdatedSkeletonTreeImpl, }; use crate::storage::map_storage::MapStorage; @@ -124,10 +128,8 @@ fn test_updated_skeleton_tree_impl_create( #[with(original_skeleton, leaf_modifications)] initial_updated_skeleton: UpdatedSkeletonTreeImpl, ) { - let leaf_modifications: LeafModifications = leaf_modifications - .iter() - .map(|(index, val)| (*index, (*val).into())) - .collect(); + let leaf_modifications: LeafModifications = + leaf_modifications.iter().map(|(index, val)| (*index, (*val).into())).collect(); let mut leaf_indices: Vec = leaf_modifications.keys().copied().collect(); let sorted_leaf_indices = SortedLeafIndices::new(&mut leaf_indices); let mut original_skeleton = OriginalSkeletonTreeImpl { @@ -145,22 +147,20 @@ fn test_updated_skeleton_tree_impl_create( #[rstest] #[case::empty_modifications(HashMap::new())] -#[case::non_empty_modifications(HashMap::from([(NodeIndex::FIRST_LEAF + NodeIndex::from(7), StarknetStorageValue::default())]))] -fn test_updated_empty_tree(#[case] modifications: LeafModifications) { +#[case::non_empty_modifications(HashMap::from([(NodeIndex::FIRST_LEAF + NodeIndex::from(7), MockLeaf::default())]))] +fn test_updated_empty_tree(#[case] modifications: LeafModifications) { let storage: MapStorage = HashMap::new().into(); let mut indices: Vec = modifications.keys().copied().collect(); let mut original_skeleton = OriginalSkeletonTreeImpl::create( &storage, HashOutput::ROOT_OF_EMPTY_TREE, SortedLeafIndices::new(&mut indices), - &OriginalSkeletonStorageTrieConfig::new(&modifications, false), + &OriginalSkeletonMockTrieConfig::new(&modifications, false), ) .unwrap(); - let skeleton_modifications = modifications - .into_iter() - .map(|(idx, leaf)| (idx, leaf.0.into())) - .collect(); + let skeleton_modifications = + modifications.into_iter().map(|(idx, leaf)| (idx, leaf.0.into())).collect(); let updated_skeleton_tree = UpdatedSkeletonTreeImpl::create(&mut original_skeleton, &skeleton_modifications).unwrap(); assert!(updated_skeleton_tree.is_empty()); diff --git a/crates/committer/src/storage/db_object.rs b/crates/committer/src/storage/db_object.rs index 1bc54558f5..5c4d17b9b6 100644 --- a/crates/committer/src/storage/db_object.rs +++ b/crates/committer/src/storage/db_object.rs @@ -1,23 +1,17 @@ use crate::storage::errors::DeserializationError; -use crate::storage::storage_trait::{StorageKey, StoragePrefix, StorageValue}; +use crate::storage::storage_trait::{StorageKey, StorageValue}; pub trait DBObject { /// Serializes the given value. fn serialize(&self) -> StorageValue; + // TODO(Aviv, 17/07/2024): Define a trait `T` for storage prefix and return `impl T` here. /// Returns the storage key prefix of the DB object. - fn get_prefix(&self) -> StoragePrefix; + fn get_prefix(&self) -> Vec; /// Returns a `StorageKey` from a prefix and a suffix. fn get_db_key(&self, suffix: &[u8]) -> StorageKey { - StorageKey( - [ - self.get_prefix().to_bytes().to_vec(), - b":".to_vec(), - suffix.to_vec(), - ] - .concat(), - ) + StorageKey([self.get_prefix(), b":".to_vec(), suffix.to_vec()].concat()) } } @@ -25,6 +19,7 @@ pub trait Deserializable: Sized { /// Deserializes the given value. fn deserialize(value: &StorageValue) -> Result; + // TODO(Aviv, 17/07/2024): Define a trait `T` for storage prefix and return `impl T` here. /// The prefix used to store in DB. - fn prefix() -> StoragePrefix; + fn prefix() -> Vec; } diff --git a/crates/committer/src/storage/errors.rs b/crates/committer/src/storage/errors.rs index e931e5111f..2a85137d49 100644 --- a/crates/committer/src/storage/errors.rs +++ b/crates/committer/src/storage/errors.rs @@ -1,10 +1,10 @@ -use crate::patricia_merkle_tree::node_data::errors::{EdgePathError, PathToBottomError}; -use crate::storage::storage_trait::{StorageKey, StoragePrefix}; - use serde_json; use starknet_types_core::felt::FromStrError; use thiserror::Error; +use crate::patricia_merkle_tree::node_data::errors::{EdgePathError, PathToBottomError}; +use crate::storage::storage_trait::StorageKey; + #[derive(Debug, Error)] pub enum StorageError { #[error("The key {0:?} does not exist in storage.")] @@ -30,7 +30,8 @@ pub enum DeserializationError { #[error(transparent)] PathToBottomError(#[from] PathToBottomError), #[error("Unexpected prefix ({0:?}) variant when deserializing a leaf.")] - LeafPrefixError(StoragePrefix), + // TODO(Aviv, 17/07/2024): Define a trait `T` for storage prefix and return `impl T` here. + LeafPrefixError(Vec), #[error(transparent)] StringConversionError(#[from] std::str::Utf8Error), #[error(transparent)] diff --git a/crates/committer/src/storage/map_storage.rs b/crates/committer/src/storage/map_storage.rs index 4c35d547b6..1fac3b3486 100644 --- a/crates/committer/src/storage/map_storage.rs +++ b/crates/committer/src/storage/map_storage.rs @@ -1,8 +1,9 @@ use std::collections::HashMap; -use crate::storage::storage_trait::{Storage, StorageKey, StorageValue}; use serde::Serialize; +use crate::storage::storage_trait::{Storage, StorageKey, StorageValue}; + #[derive(Serialize, Debug, Default)] #[cfg_attr(any(test, feature = "testing"), derive(Clone))] pub struct MapStorage { diff --git a/crates/committer/src/storage/storage_trait.rs b/crates/committer/src/storage/storage_trait.rs index 88b5f95d94..190bf5f027 100644 --- a/crates/committer/src/storage/storage_trait.rs +++ b/crates/committer/src/storage/storage_trait.rs @@ -1,7 +1,8 @@ +use std::collections::HashMap; + use serde::{Serialize, Serializer}; use crate::felt::Felt; -use std::collections::HashMap; #[derive(Debug, Eq, Hash, PartialEq)] #[cfg_attr(any(test, feature = "testing"), derive(Clone))] @@ -30,8 +31,10 @@ pub trait Storage: From> { fn delete(&mut self, key: &StorageKey) -> Option; } +// TODO(Aviv, 17/07/2024); Split between Storage prefix representation (trait) and node +// specific implementation (enum). #[derive(Clone, Debug)] -pub enum StoragePrefix { +pub enum StarknetPrefix { InnerNode, StorageLeaf, StateTreeLeaf, @@ -39,7 +42,7 @@ pub enum StoragePrefix { } /// Describes a storage prefix as used in Aerospike DB. -impl StoragePrefix { +impl StarknetPrefix { pub(crate) fn to_bytes(&self) -> &'static [u8] { match self { Self::InnerNode => b"patricia_node", @@ -48,6 +51,10 @@ impl StoragePrefix { Self::CompiledClassLeaf => b"contract_class_leaf", } } + + pub(crate) fn to_storage_prefix(&self) -> Vec { + self.to_bytes().to_vec() + } } impl From for StorageKey { @@ -71,6 +78,6 @@ impl Serialize for StorageKey { } /// Returns a `StorageKey` from a prefix and a suffix. -pub(crate) fn create_db_key(prefix: StoragePrefix, suffix: &[u8]) -> StorageKey { - StorageKey([prefix.to_bytes().to_vec(), b":".to_vec(), suffix.to_vec()].concat()) +pub(crate) fn create_db_key(prefix: Vec, suffix: &[u8]) -> StorageKey { + StorageKey([prefix, b":".to_vec(), suffix.to_vec()].concat()) } diff --git a/crates/committer_cli/Cargo.toml b/crates/committer_cli/Cargo.toml index ce598eb0f7..9fc69e8e0e 100644 --- a/crates/committer_cli/Cargo.toml +++ b/crates/committer_cli/Cargo.toml @@ -10,11 +10,11 @@ description = "Cli for the committer package." workspace = true [dev-dependencies] -criterion = { workspace = true, features = ["html_reports"] } +criterion = { version = "0.5.1", features = ["html_reports"] } pretty_assertions.workspace = true [dependencies] -clap.workspace = true +clap = { version = "4.5.4", features = ["cargo", "derive"] } committer = { path = "../committer", features = ["testing"] } derive_more.workspace = true ethnum.workspace = true @@ -22,12 +22,14 @@ indexmap.workspace = true log.workspace = true rand.workspace = true rand_distr.workspace = true -serde.workspace = true -serde_json.workspace = true +serde = { version = "1.0.197", features = ["derive"] } +serde_json = "1.0.116" +serde_repr = "0.1.19" simplelog.workspace = true starknet-types-core.workspace = true starknet_api = { path = "../starknet_api", version = "0.13.0-rc.0"} strum.workspace = true +strum_macros.workspace = true thiserror.workspace = true tokio.workspace = true @@ -35,3 +37,8 @@ tokio.workspace = true harness = false name = "committer_bench" path = "benches/committer_bench.rs" + +# Optional dependencies required for tests and the testing feature. +# See [here](https://github.com/bnjbvr/cargo-machete/issues/128). +[package.metadata.cargo-machete] +ignored = ["hex", "strum_macros"] diff --git a/crates/committer_cli/benches/committer_bench.rs b/crates/committer_cli/benches/committer_bench.rs index 2ebbe94e50..90796c7cb8 100644 --- a/crates/committer_cli/benches/committer_bench.rs +++ b/crates/committer_cli/benches/committer_bench.rs @@ -1,18 +1,21 @@ #![allow(clippy::unwrap_used)] -use std::{collections::HashMap, sync::Arc}; +// This file is for benchmarking the committer flow. +// The input files for the different benchmarks are downloaded from GCS, using the prefix stored in +// committer_cli/src/tests/flow_test_files_prefix. In order to update them, generate a new random +// prefix (the hash of the initial new commit can be used) and update it in the mentioned file. +// Then upload the new files to GCS with this new prefix (run e.g., +// gcloud storage cp LOCAL_FILE gs://committer-testing-artifacts/NEW_PREFIX/tree_flow_inputs.json). -use committer::{ - block_committer::input::StarknetStorageValue, - patricia_merkle_tree::{ - external_test_utils::tree_computation_flow, node_data::leaf::LeafModifications, - types::NodeIndex, - }, -}; -use committer_cli::{ - commands::commit, parse_input::read::parse_input, - tests::utils::parse_from_python::TreeFlowInput, -}; +use std::collections::HashMap; +use std::sync::Arc; + +use committer::block_committer::input::StarknetStorageValue; +use committer::patricia_merkle_tree::external_test_utils::tree_computation_flow; +use committer::patricia_merkle_tree::node_data::leaf::LeafModifications; +use committer::patricia_merkle_tree::types::NodeIndex; +use committer_cli::commands::parse_and_commit; +use committer_cli::tests::utils::parse_from_python::TreeFlowInput; use criterion::{criterion_group, criterion_main, Criterion}; const CONCURRENCY_MODE: bool = true; @@ -21,17 +24,12 @@ const FLOW_TEST_INPUT: &str = include_str!("committer_flow_inputs.json"); const OUTPUT_PATH: &str = "benchmark_output.txt"; pub fn single_tree_flow_benchmark(criterion: &mut Criterion) { - let TreeFlowInput { - leaf_modifications, - storage, - root_hash, - } = serde_json::from_str(SINGLE_TREE_FLOW_INPUT).unwrap(); + let TreeFlowInput { leaf_modifications, storage, root_hash } = + serde_json::from_str(SINGLE_TREE_FLOW_INPUT).unwrap(); let runtime = match CONCURRENCY_MODE { true => tokio::runtime::Builder::new_multi_thread().build().unwrap(), - false => tokio::runtime::Builder::new_current_thread() - .build() - .unwrap(), + false => tokio::runtime::Builder::new_current_thread().build().unwrap(), }; let leaf_modifications = leaf_modifications @@ -54,31 +52,21 @@ pub fn single_tree_flow_benchmark(criterion: &mut Criterion) { pub fn full_committer_flow_benchmark(criterion: &mut Criterion) { let runtime = match CONCURRENCY_MODE { true => tokio::runtime::Builder::new_multi_thread().build().unwrap(), - false => tokio::runtime::Builder::new_current_thread() - .build() - .unwrap(), + false => tokio::runtime::Builder::new_current_thread().build().unwrap(), }; // TODO(Aner, 8/7/2024): use structs for deserialization. let input: HashMap = serde_json::from_str(FLOW_TEST_INPUT).unwrap(); let committer_input_string = input.get("committer_input").unwrap(); + // TODO(Aner, 27/06/2024): output path should be a pipe (file on memory) // to avoid disk IO in the benchmark. - // TODO(Aner, 11/7/24): consider moving function to production code. - async fn parse_and_commit(input_str: &str) { - let committer_input = parse_input(input_str).expect("Failed to parse the given input."); - commit(committer_input, OUTPUT_PATH.to_owned()).await; - } criterion.bench_function("full_committer_flow", |benchmark| { benchmark.iter(|| { - runtime.block_on(parse_and_commit(committer_input_string)); + runtime.block_on(parse_and_commit(committer_input_string, OUTPUT_PATH.to_owned())); }) }); } -criterion_group!( - benches, - single_tree_flow_benchmark, - full_committer_flow_benchmark -); +criterion_group!(benches, single_tree_flow_benchmark, full_committer_flow_benchmark); criterion_main!(benches); diff --git a/crates/committer_cli/src/block_hash.rs b/crates/committer_cli/src/block_hash.rs index ae4c4eea2f..d78f5b19da 100644 --- a/crates/committer_cli/src/block_hash.rs +++ b/crates/committer_cli/src/block_hash.rs @@ -1,10 +1,11 @@ use serde::Deserialize; -use starknet_api::{ - block::BlockHeaderWithoutHash, - block_hash::block_hash_calculator::{BlockHeaderCommitments, TransactionHashingData}, - data_availability::L1DataAvailabilityMode, - state::ThinStateDiff, +use starknet_api::block::BlockHeaderWithoutHash; +use starknet_api::block_hash::block_hash_calculator::{ + BlockHeaderCommitments, + TransactionHashingData, }; +use starknet_api::data_availability::L1DataAvailabilityMode; +use starknet_api::state::ThinStateDiff; #[derive(Clone, Debug, Deserialize, Eq, PartialEq)] pub struct BlockCommitmentsInput { diff --git a/crates/committer_cli/src/commands.rs b/crates/committer_cli/src/commands.rs index 2f55076006..66552325a9 100644 --- a/crates/committer_cli/src/commands.rs +++ b/crates/committer_cli/src/commands.rs @@ -1,18 +1,19 @@ -use committer::block_committer::{ - commit::commit_block, - input::{ConfigImpl, Input}, -}; +use committer::block_committer::commit::commit_block; +use committer::block_committer::input::{Config, ConfigImpl, Input}; -use crate::{ - filled_tree_output::filled_forest::SerializedForest, parse_input::read::write_to_file, -}; +use crate::filled_tree_output::filled_forest::SerializedForest; +use crate::parse_input::read::{parse_input, write_to_file}; + +pub async fn parse_and_commit(input_string: &str, output_path: String) { + let input = parse_input(input_string).expect("Failed to parse the given input."); + // Set the given log level. + log::set_max_level(input.config.logger_level()); + commit(input, output_path).await; +} pub async fn commit(input: Input, output_path: String) { - let serialized_filled_forest = SerializedForest( - commit_block(input) - .await - .expect("Failed to commit the given block."), - ); + let serialized_filled_forest = + SerializedForest(commit_block(input).await.expect("Failed to commit the given block.")); let output = serialized_filled_forest.forest_to_output(); write_to_file(&output_path, &output); } diff --git a/crates/committer_cli/src/filled_tree_output/errors.rs b/crates/committer_cli/src/filled_tree_output/errors.rs index c8191bfb9d..410cf07eb2 100644 --- a/crates/committer_cli/src/filled_tree_output/errors.rs +++ b/crates/committer_cli/src/filled_tree_output/errors.rs @@ -1,7 +1,10 @@ +use std::fmt::Debug; + use committer::patricia_merkle_tree::filled_tree::errors::{ - ClassesTrieError, ContractsTrieError, StorageTrieError, + ClassesTrieError, + ContractsTrieError, + StorageTrieError, }; -use std::fmt::Debug; #[derive(thiserror::Error, Debug)] pub enum FilledForestError { diff --git a/crates/committer_cli/src/main.rs b/crates/committer_cli/src/main.rs index a051b2fcb1..dec0e504ca 100644 --- a/crates/committer_cli/src/main.rs +++ b/crates/committer_cli/src/main.rs @@ -1,13 +1,12 @@ use clap::{Args, Parser, Subcommand}; use committer_cli::block_hash::{BlockCommitmentsInput, BlockHashInput}; -use committer_cli::commands::commit; -use committer_cli::parse_input::read::{ - load_from_stdin, parse_input, read_from_stdin, write_to_file, -}; +use committer_cli::commands::parse_and_commit; +use committer_cli::parse_input::read::{load_from_stdin, read_from_stdin, write_to_file}; use committer_cli::tests::python_tests::PythonTest; use simplelog::{ColorChoice, Config, LevelFilter, TermLogger, TerminalMode}; use starknet_api::block_hash::block_hash_calculator::{ - calculate_block_commitments, calculate_block_hash, + calculate_block_commitments, + calculate_block_hash, }; /// Committer CLI. @@ -73,14 +72,11 @@ async fn main() { match args.command { Command::Commit { output_path } => { - let input = parse_input(&read_from_stdin()).expect("Failed to parse the given input."); - commit(input, output_path).await; + // TODO(Aner, 15/7/24): try moving read_from_stdin into function. + parse_and_commit(&read_from_stdin(), output_path).await; } - Command::PythonTest { - output_path, - test_name, - } => { + Command::PythonTest { output_path, test_name } => { // Create PythonTest from test_name. let test = PythonTest::try_from(test_name) .unwrap_or_else(|error| panic!("Failed to create PythonTest: {}", error)); diff --git a/crates/committer_cli/src/parse_input/cast.rs b/crates/committer_cli/src/parse_input/cast.rs index 3092b01ddb..730db39c7a 100644 --- a/crates/committer_cli/src/parse_input/cast.rs +++ b/crates/committer_cli/src/parse_input/cast.rs @@ -1,6 +1,12 @@ -use crate::parse_input::raw_input::RawInput; +use std::collections::HashMap; + use committer::block_committer::input::{ - ConfigImpl, ContractAddress, Input, StarknetStorageKey, StarknetStorageValue, StateDiff, + ConfigImpl, + ContractAddress, + Input, + StarknetStorageKey, + StarknetStorageValue, + StateDiff, }; use committer::felt::Felt; use committer::hash::hash_trait::HashOutput; @@ -8,7 +14,7 @@ use committer::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClas use committer::storage::errors::DeserializationError; use committer::storage::storage_trait::{StorageKey, StorageValue}; -use std::collections::HashMap; +use crate::parse_input::raw_input::RawInput; pub type InputImpl = Input; @@ -17,12 +23,7 @@ impl TryFrom for InputImpl { fn try_from(raw_input: RawInput) -> Result { let mut storage = HashMap::new(); for entry in raw_input.storage { - add_unique( - &mut storage, - "storage", - StorageKey(entry.key), - StorageValue(entry.value), - )?; + add_unique(&mut storage, "storage", StorageKey(entry.key), StorageValue(entry.value))?; } let mut address_to_class_hash = HashMap::new(); @@ -89,7 +90,7 @@ impl TryFrom for InputImpl { classes_trie_root_hash: HashOutput(Felt::from_bytes_be_slice( &raw_input.classes_trie_root_hash, )), - config: ConfigImpl::new(raw_input.trivial_updates_config), + config: raw_input.config.into(), }) } } @@ -104,9 +105,7 @@ where K: std::cmp::Eq + std::hash::Hash + std::fmt::Debug, { if map.contains_key(&key) { - return Err(DeserializationError::KeyDuplicate(format!( - "{map_name}: {key:?}" - ))); + return Err(DeserializationError::KeyDuplicate(format!("{map_name}: {key:?}"))); } map.insert(key, value); Ok(()) diff --git a/crates/committer_cli/src/parse_input/raw_input.rs b/crates/committer_cli/src/parse_input/raw_input.rs index bbd0e3387c..d58dc863de 100644 --- a/crates/committer_cli/src/parse_input/raw_input.rs +++ b/crates/committer_cli/src/parse_input/raw_input.rs @@ -1,5 +1,7 @@ -use serde::Deserialize; - +use committer::block_committer::input::ConfigImpl; +use log::LevelFilter; +use serde::{Deserialize, Serialize}; +use serde_repr::Deserialize_repr; type RawFelt = [u8; 32]; #[derive(Deserialize, Debug)] @@ -10,7 +12,7 @@ pub(crate) struct RawInput { pub state_diff: RawStateDiff, pub contracts_trie_root_hash: RawFelt, pub classes_trie_root_hash: RawFelt, - pub trivial_updates_config: bool, + pub config: RawConfigImpl, } #[derive(Deserialize, Debug)] @@ -20,6 +22,40 @@ pub(crate) struct RawStorageEntry { pub value: Vec, } +#[derive(Deserialize, Debug)] +pub(crate) struct RawConfigImpl { + warn_on_trivial_modifications: bool, + log_level: PythonLogLevel, +} + +#[derive(Deserialize_repr, Debug, Default, Serialize)] +#[repr(usize)] +/// Describes a log level https://docs.python.org/3/library/logging.html#logging-levels +pub(crate) enum PythonLogLevel { + NotSet = 0, + Info = 20, + Warning = 30, + Error = 40, + Critical = 50, + // If an unknown variant is given, the default log level is Debug. + #[serde(other)] + #[default] + Debug = 10, +} + +impl From for ConfigImpl { + fn from(raw_config: RawConfigImpl) -> Self { + let log_level = match raw_config.log_level { + PythonLogLevel::NotSet => LevelFilter::Trace, + PythonLogLevel::Debug => LevelFilter::Debug, + PythonLogLevel::Info => LevelFilter::Info, + PythonLogLevel::Warning => LevelFilter::Warn, + PythonLogLevel::Error | PythonLogLevel::Critical => LevelFilter::Error, + }; + ConfigImpl::new(raw_config.warn_on_trivial_modifications, log_level) + } +} + #[derive(Deserialize, Debug)] pub(crate) struct RawFeltMapEntry { pub key: RawFelt, diff --git a/crates/committer_cli/src/parse_input/read.rs b/crates/committer_cli/src/parse_input/read.rs index f8f8e4df0c..4f7ddf1f67 100644 --- a/crates/committer_cli/src/parse_input/read.rs +++ b/crates/committer_cli/src/parse_input/read.rs @@ -1,7 +1,5 @@ -use std::{ - fs::File, - io::{self, BufWriter}, -}; +use std::fs::File; +use std::io::{self, BufWriter}; use committer::storage::errors::DeserializationError; use serde::{Deserialize, Serialize}; diff --git a/crates/committer_cli/src/parse_input/read_test.rs b/crates/committer_cli/src/parse_input/read_test.rs index 1cc977d3d5..ae8d550625 100644 --- a/crates/committer_cli/src/parse_input/read_test.rs +++ b/crates/committer_cli/src/parse_input/read_test.rs @@ -1,17 +1,19 @@ -use committer::{ - block_committer::input::{ - ConfigImpl, ContractAddress, Input, StarknetStorageKey, StarknetStorageValue, StateDiff, - }, - felt::Felt, - hash::hash_trait::HashOutput, - patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}, - storage::{ - errors::DeserializationError, - storage_trait::{StorageKey, StorageValue}, - }, +use std::collections::HashMap; + +use committer::block_committer::input::{ + ConfigImpl, + ContractAddress, + Input, + StarknetStorageKey, + StarknetStorageValue, + StateDiff, }; +use committer::felt::Felt; +use committer::hash::hash_trait::HashOutput; +use committer::patricia_merkle_tree::filled_tree::node::{ClassHash, CompiledClassHash, Nonce}; +use committer::storage::errors::DeserializationError; +use committer::storage::storage_trait::{StorageKey, StorageValue}; use pretty_assertions::assert_eq; -use std::collections::HashMap; use super::parse_input; @@ -80,19 +82,13 @@ fn test_simple_input_parsing() { ], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 19], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0], - true + {"warn_on_trivial_modifications": true, "log_level": 5} ] "#; let expected_storage = HashMap::from([ - ( - StorageKey([14, 6, 78, 90].to_vec()), - StorageValue([245, 90, 0, 0, 1].to_vec()), - ), - ( - StorageKey([14, 6, 43, 90].to_vec()), - StorageValue([9, 0, 0, 0, 1].to_vec()), - ), + (StorageKey([14, 6, 78, 90].to_vec()), StorageValue([245, 90, 0, 0, 1].to_vec())), + (StorageKey([14, 6, 43, 90].to_vec()), StorageValue([9, 0, 0, 0, 1].to_vec())), ]); let expected_address_to_class_hash = HashMap::from([ @@ -205,7 +201,7 @@ fn test_simple_input_parsing() { }, contracts_trie_root_hash: expected_contracts_trie_root_hash, classes_trie_root_hash: expected_classes_trie_root_hash, - config: ConfigImpl::new(true), + config: ConfigImpl::new(true, log::LevelFilter::Debug), }; assert_eq!(parse_input(input).unwrap(), expected_input); } @@ -232,7 +228,7 @@ fn test_input_parsing_with_storage_key_duplicate() { ], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 17, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 222, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 0, 3], - true + {"warn_on_trivial_modifications": true, "log_level": 20} ] "#; @@ -274,7 +270,7 @@ fn test_input_parsing_with_mapping_key_duplicate() { ], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 17, 0, 0, 0, 0, 0, 0, 0, 144, 0, 0, 0, 0, 0, 0, 0, 0, 5], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 222, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 0, 3], - false + {"warn_on_trivial_modifications": false, "log_level": 30} ] "#; diff --git a/crates/committer_cli/src/tests/flow_test_files_prefix b/crates/committer_cli/src/tests/flow_test_files_prefix index 62f44e9f32..af417ba7e0 100644 --- a/crates/committer_cli/src/tests/flow_test_files_prefix +++ b/crates/committer_cli/src/tests/flow_test_files_prefix @@ -1 +1 @@ -7c50039 +23ffcf5 diff --git a/crates/committer_cli/src/tests/python_tests.rs b/crates/committer_cli/src/tests/python_tests.rs index 8ba1341972..f4d1a55e2e 100644 --- a/crates/committer_cli/src/tests/python_tests.rs +++ b/crates/committer_cli/src/tests/python_tests.rs @@ -1,44 +1,57 @@ -use crate::filled_tree_output::errors::FilledForestError; -use crate::filled_tree_output::filled_forest::SerializedForest; -use crate::parse_input::cast::InputImpl; -use crate::parse_input::read::parse_input; -use crate::tests::utils::parse_from_python::parse_input_single_storage_tree_flow_test; -use crate::tests::utils::random_structs::DummyRandomValue; +use std::collections::HashMap; +use std::fmt::Debug; +use std::io; + use committer::block_committer::input::{ - ContractAddress, StarknetStorageKey, StarknetStorageValue, StateDiff, + ContractAddress, + StarknetStorageKey, + StarknetStorageValue, + StateDiff, }; use committer::felt::Felt; use committer::hash::hash_trait::HashOutput; +use committer::patricia_merkle_tree::external_test_utils::single_tree_flow_test; use committer::patricia_merkle_tree::filled_tree::forest::FilledForest; -use committer::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use committer::patricia_merkle_tree::filled_tree::node::{ClassHash, FilledNode, Nonce}; +use committer::patricia_merkle_tree::filled_tree::node::{ + ClassHash, + CompiledClassHash, + FilledNode, + Nonce, +}; use committer::patricia_merkle_tree::node_data::inner_node::{ - BinaryData, EdgeData, EdgePathLength, NodeData, PathToBottom, + BinaryData, + EdgeData, + EdgePathLength, + NodeData, + PathToBottom, }; use committer::patricia_merkle_tree::node_data::leaf::ContractState; use committer::patricia_merkle_tree::types::SubTreeHeight; -use log::error; - -use committer::patricia_merkle_tree::external_test_utils::single_tree_flow_test; use committer::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl; use committer::storage::db_object::DBObject; use committer::storage::errors::{DeserializationError, SerializationError}; use committer::storage::map_storage::MapStorage; use committer::storage::storage_trait::{Storage, StorageKey, StorageValue}; use ethnum::U256; +use log::error; use serde_json::json; use starknet_api::block_hash::block_hash_calculator::{ - TransactionHashingData, TransactionOutputForHash, + TransactionHashingData, + TransactionOutputForHash, }; use starknet_api::state::ThinStateDiff; use starknet_api::transaction::TransactionExecutionStatus; use starknet_types_core::hash::{Pedersen, StarkHash}; -use std::fmt::Debug; -use std::{collections::HashMap, io}; use thiserror; use super::utils::objects::{get_thin_state_diff, get_transaction_output_for_hash, get_tx_data}; use super::utils::parse_from_python::TreeFlowInput; +use crate::filled_tree_output::errors::FilledForestError; +use crate::filled_tree_output::filled_forest::SerializedForest; +use crate::parse_input::cast::InputImpl; +use crate::parse_input::read::parse_input; +use crate::tests::utils::parse_from_python::parse_input_single_storage_tree_flow_test; +use crate::tests::utils::random_structs::DummyRandomValue; // Enum representing different Python tests. pub enum PythonTest { @@ -181,11 +194,8 @@ impl PythonTest { } Self::ComputeHashSingleTree => { // 1. Get and deserialize input. - let TreeFlowInput { - leaf_modifications, - storage, - root_hash, - } = serde_json::from_str(Self::non_optional_input(input)?)?; + let TreeFlowInput { leaf_modifications, storage, root_hash } = + serde_json::from_str(Self::non_optional_input(input)?)?; // 2. Run the test. let output = single_tree_flow_test(leaf_modifications, storage, root_hash).await; // 3. Serialize and return output. @@ -212,16 +222,11 @@ impl PythonTest { // Test that the fetching of the input to flow test is working. // TODO(Aner, 8/7/2024): refactor using structs for deserialization and rename the function. fn serialize_for_rust_committer_flow_test(input: HashMap) -> String { - let TreeFlowInput { - leaf_modifications, - storage, - root_hash, - } = parse_input_single_storage_tree_flow_test(&input); + let TreeFlowInput { leaf_modifications, storage, root_hash } = + parse_input_single_storage_tree_flow_test(&input); // Serialize the leaf modifications to an object that can be JSON-serialized. - let leaf_modifications_to_print: HashMap> = leaf_modifications - .into_iter() - .map(|(k, v)| (k.0.to_string(), v.serialize().0)) - .collect(); + let leaf_modifications_to_print: HashMap> = + leaf_modifications.into_iter().map(|(k, v)| (k.0.to_string(), v.serialize().0)).collect(); // Create a json string to compare with the expected string in python. serde_json::to_string(&json!( @@ -283,12 +288,8 @@ pub(crate) fn felt_serialize_test(felt: u128) -> String { pub(crate) fn test_hash_function(hash_input: HashMap) -> String { // Fetch x and y from the input. - let x = hash_input - .get("x") - .expect("Failed to get value for key 'x'"); - let y = hash_input - .get("y") - .expect("Failed to get value for key 'y'"); + let x = hash_input.get("x").expect("Failed to get value for key 'x'"); + let y = hash_input.get("y").expect("Failed to get value for key 'y'"); // Convert x and y to Felt. let x_felt = Felt::from(*x); @@ -313,12 +314,8 @@ pub(crate) fn test_hash_function(hash_input: HashMap) -> String { /// A JSON string representing the value of serialized binary data. pub(crate) fn test_binary_serialize_test(binary_input: HashMap) -> String { // Extract left and right values from the input. - let left = binary_input - .get("left") - .expect("Failed to get value for key 'left'"); - let right = binary_input - .get("right") - .expect("Failed to get value for key 'right'"); + let left = binary_input.get("left").expect("Failed to get value for key 'left'"); + let right = binary_input.get("right").expect("Failed to get value for key 'right'"); // Create a map to store the serialized binary data. let mut map: HashMap> = HashMap::new(); @@ -330,10 +327,8 @@ pub(crate) fn test_binary_serialize_test(binary_input: HashMap) -> }; // Create a filled node (irrelevant leaf type) with binary data and zero hash. - let filled_node: FilledNode = FilledNode { - data: NodeData::Binary(binary_data), - hash: HashOutput(Felt::ZERO), - }; + let filled_node: FilledNode = + FilledNode { data: NodeData::Binary(binary_data), hash: HashOutput(Felt::ZERO) }; // Serialize the binary node and insert it into the map under the key "value". let value = filled_node.serialize(); @@ -369,10 +364,7 @@ fn create_output_to_python(actual_input: InputImpl) -> String { actual_input.storage.len(), actual_input.state_diff.address_to_class_hash.len(), actual_input.state_diff.address_to_nonce.len(), - actual_input - .state_diff - .class_hash_to_compiled_class_hash - .len(), + actual_input.state_diff.class_hash_to_compiled_class_hash.len(), actual_input.state_diff.storage_updates.len(), actual_input.contracts_trie_root_hash.0.to_bytes_be(), actual_input.classes_trie_root_hash.0.to_bytes_be(), @@ -396,23 +388,15 @@ fn hash_state_diff(state_diff: &StateDiff) -> (Vec, Vec) { ) = hash_class_hash_to_compiled_class_hash(&state_diff.class_hash_to_compiled_class_hash); let (storage_updates_keys_hash, storage_updates_values_hash) = hash_storage_updates(&state_diff.storage_updates); - let mut state_diff_keys_hash = xor_hash( - &address_to_class_hash_keys_hash, - &address_to_nonce_keys_hash, - ); - state_diff_keys_hash = xor_hash( - &state_diff_keys_hash, - &class_hash_to_compiled_class_hash_keys_hash, - ); + let mut state_diff_keys_hash = + xor_hash(&address_to_class_hash_keys_hash, &address_to_nonce_keys_hash); + state_diff_keys_hash = + xor_hash(&state_diff_keys_hash, &class_hash_to_compiled_class_hash_keys_hash); state_diff_keys_hash = xor_hash(&state_diff_keys_hash, &storage_updates_keys_hash); - let mut state_diff_values_hash = xor_hash( - &address_to_class_hash_values_hash, - &address_to_nonce_values_hash, - ); - state_diff_values_hash = xor_hash( - &state_diff_values_hash, - &class_hash_to_compiled_class_hash_values_hash, - ); + let mut state_diff_values_hash = + xor_hash(&address_to_class_hash_values_hash, &address_to_nonce_values_hash); + state_diff_values_hash = + xor_hash(&state_diff_values_hash, &class_hash_to_compiled_class_hash_values_hash); state_diff_values_hash = xor_hash(&state_diff_values_hash, &storage_updates_values_hash); (state_diff_keys_hash, state_diff_values_hash) } @@ -474,14 +458,13 @@ fn xor_hash(x: &[u8], y: &[u8]) -> Vec { /// Creates and serializes storage keys for different node types. /// -/// This function generates and serializes storage keys for various node types, including binary nodes, -/// edge nodes, storage leaf nodes, state tree leaf nodes, and compiled class leaf nodes. The resulting -/// keys are stored in a `HashMap` and serialized into a JSON string. +/// This function generates and serializes storage keys for various node types, including binary +/// nodes, edge nodes, storage leaf nodes, state tree leaf nodes, and compiled class leaf nodes. The +/// resulting keys are stored in a `HashMap` and serialized into a JSON string. /// /// # Returns /// /// A JSON string representing the serialized storage keys for different node types. -/// pub(crate) fn test_node_db_key() -> String { let zero = Felt::ZERO; @@ -489,28 +472,19 @@ pub(crate) fn test_node_db_key() -> String { let hash = HashOutput(zero); let binary_node: FilledNode = FilledNode { - data: NodeData::Binary(BinaryData { - left_hash: hash, - right_hash: hash, - }), + data: NodeData::Binary(BinaryData { left_hash: hash, right_hash: hash }), hash, }; let binary_node_key = binary_node.db_key().0; let edge_node: FilledNode = FilledNode { - data: NodeData::Edge(EdgeData { - bottom_hash: hash, - path_to_bottom: Default::default(), - }), + data: NodeData::Edge(EdgeData { bottom_hash: hash, path_to_bottom: Default::default() }), hash, }; let edge_node_key = edge_node.db_key().0; - let storage_leaf = FilledNode { - data: NodeData::Leaf(StarknetStorageValue(zero)), - hash, - }; + let storage_leaf = FilledNode { data: NodeData::Leaf(StarknetStorageValue(zero)), hash }; let storage_leaf_key = storage_leaf.db_key().0; let state_tree_leaf = FilledNode { @@ -523,10 +497,7 @@ pub(crate) fn test_node_db_key() -> String { }; let state_tree_leaf_key = state_tree_leaf.db_key().0; - let compiled_class_leaf = FilledNode { - data: NodeData::Leaf(CompiledClassHash(zero)), - hash, - }; + let compiled_class_leaf = FilledNode { data: NodeData::Leaf(CompiledClassHash(zero)), hash }; let compiled_class_leaf_key = compiled_class_leaf.db_key().0; // Store keys in a HashMap. @@ -536,24 +507,19 @@ pub(crate) fn test_node_db_key() -> String { map.insert("edge_node_key".to_string(), edge_node_key); map.insert("storage_leaf_key".to_string(), storage_leaf_key); map.insert("state_tree_leaf_key".to_string(), state_tree_leaf_key); - map.insert( - "compiled_class_leaf_key".to_string(), - compiled_class_leaf_key, - ); + map.insert("compiled_class_leaf_key".to_string(), compiled_class_leaf_key); // Serialize the map to a JSON string and handle serialization errors. serde_json::to_string(&map) .unwrap_or_else(|error| panic!("Failed to serialize storage prefix: {}", error)) } -/// This function storage_serialize_test generates a MapStorage containing StorageKey and StorageValue -/// pairs for u128 values in the range 0..=1000, +/// This function storage_serialize_test generates a MapStorage containing StorageKey and +/// StorageValue pairs for u128 values in the range 0..=1000, /// serializes it to a JSON string using Serde, /// and returns the serialized JSON string or panics with an error message if serialization fails. pub(crate) fn storage_serialize_test() -> Result { - let mut storage = MapStorage { - storage: HashMap::new(), - }; + let mut storage = MapStorage { storage: HashMap::new() }; for i in 0..=99_u128 { let key = StorageKey(Felt::from(i).to_bytes_be().to_vec()); let value = StorageValue(Felt::from(i).to_bytes_be().to_vec()); @@ -574,7 +540,8 @@ fn python_hash_constants_compare() -> String { } /// Processes a map containing JSON strings for different node data. -/// Creates `NodeData` objects for each node type, stores them in a storage, and serializes the map to a JSON string. +/// Creates `NodeData` objects for each node type, stores them in a storage, and serializes the map +/// to a JSON string. /// /// # Arguments /// * `data` - A map containing JSON strings for different node data: @@ -585,12 +552,11 @@ fn python_hash_constants_compare() -> String { /// - `"contract_class_leaf"`: Compiled class leaf data. /// /// # Returns -/// A `Result` containing a serialized map of all nodes on success, or an error if keys are missing or parsing fails. +/// A `Result` containing a serialized map of all nodes on success, or an +/// error if keys are missing or parsing fails. fn test_storage_node(data: HashMap) -> Result { // Create a storage to store the nodes. - let mut rust_fact_storage = MapStorage { - storage: HashMap::new(), - }; + let mut rust_fact_storage = MapStorage { storage: HashMap::new() }; // Parse the binary node data from the input. let binary_json = get_or_key_not_found(&data, "binary")?; @@ -639,10 +605,7 @@ fn test_storage_node(data: HashMap) -> Result) -> Result) -> Result); @@ -31,9 +29,7 @@ impl<'de> Deserialize<'de> for FactMap { where D: Deserializer<'de>, { - Ok(Self( - serde_json::from_str(&String::deserialize(deserializer)?).unwrap(), - )) + Ok(Self(serde_json::from_str(&String::deserialize(deserializer)?).unwrap())) } } @@ -44,9 +40,7 @@ impl<'de> Deserialize<'de> for CommitterInput { where D: Deserializer<'de>, { - Ok(Self( - parse_input(&String::deserialize(deserializer)?).unwrap(), - )) + Ok(Self(parse_input(&String::deserialize(deserializer)?).unwrap())) } } @@ -82,7 +76,8 @@ struct TreeRegressionInput { expected_storage_changes: Map, } -// TODO(Aner, 9/8/24): remove this impl and use the Deserialize derive, by changing the input format. +// TODO(Aner, 9/8/24): remove this impl and use the Deserialize derive, by changing the input +// format. impl<'de> Deserialize<'de> for TreeRegressionInput { fn deserialize(deserializer: D) -> Result where @@ -100,16 +95,11 @@ impl<'de> Deserialize<'de> for TreeRegressionInput { } } -#[ignore = "To avoid running the benchmark test in Coverage or without the --release flag."] +#[ignore = "To avoid running the regression test in Coverage or without the --release flag."] #[tokio::test(flavor = "multi_thread")] pub async fn test_regression_single_tree() { let TreeRegressionInput { - tree_flow_input: - TreeFlowInput { - leaf_modifications, - storage, - root_hash, - }, + tree_flow_input: TreeFlowInput { leaf_modifications, storage, root_hash }, expected_hash, expected_storage_changes, } = serde_json::from_str(SINGLE_TREE_FLOW_INPUT).unwrap(); @@ -120,10 +110,8 @@ pub async fn test_regression_single_tree() { let execution_time = std::time::Instant::now() - start; // Assert correctness of the output of the single tree flow test. - let TreeRegressionOutput { - root_hash, - storage_changes: Value::Object(actual_storage_changes), - } = serde_json::from_str(&output).unwrap() + let TreeRegressionOutput { root_hash, storage_changes: Value::Object(actual_storage_changes) } = + serde_json::from_str(&output).unwrap() else { panic!("Expected storage changes object to be an object."); }; @@ -135,29 +123,25 @@ pub async fn test_regression_single_tree() { assert!(execution_time.as_secs_f64() < MAX_TIME_FOR_SINGLE_TREE_BECHMARK_TEST); } -#[ignore = "To avoid running the benchmark test in Coverage or without the --release flag."] -#[tokio::test(flavor = "multi_thread")] -pub async fn test_regression_committer_flow() { +pub async fn test_single_committer_flow(input: &str, output_path: &str) { let CommitterRegressionInput { committer_input, contract_states_root: expected_contract_states_root, contract_classes_root: expected_contract_classes_root, expected_facts, - } = serde_json::from_str(FLOW_TEST_INPUT).unwrap(); + } = serde_json::from_str(input).unwrap(); let start = std::time::Instant::now(); // Benchmark the committer flow test. - commit(committer_input.0, OUTPUT_PATH.to_owned()).await; + commit(committer_input.0, output_path.to_owned()).await; let execution_time = std::time::Instant::now() - start; // Assert correctness of the output of the committer flow test. let CommitterRegressionOutput { contract_storage_root_hash, compiled_class_root_hash, - storage: StorageObject { - storage: Value::Object(storage_changes), - }, - } = serde_json::from_str(&std::fs::read_to_string(OUTPUT_PATH).unwrap()).unwrap() + storage: StorageObject { storage: Value::Object(storage_changes) }, + } = serde_json::from_str(&std::fs::read_to_string(output_path).unwrap()).unwrap() else { panic!("Expected the storage to be an object."); }; @@ -169,3 +153,26 @@ pub async fn test_regression_committer_flow() { // Assert the execution time does not exceed the threshold. assert!(execution_time.as_secs_f64() < MAX_TIME_FOR_COMMITTER_FLOW_BECHMARK_TEST); } +#[ignore = "To avoid running the regression test in Coverage or without the --release flag."] +#[tokio::test(flavor = "multi_thread")] +pub async fn test_regression_committer_flow() { + test_single_committer_flow(FLOW_TEST_INPUT, OUTPUT_PATH).await; +} + +#[ignore = "To avoid running the regression test in Coverage or without the --release flag."] +#[tokio::test(flavor = "multi_thread")] +pub async fn test_regression_committer_all_files() { + assert_eq!( + fs::read_dir("./benches/regression_files").unwrap().count(), + EXPECTED_NUMBER_OF_FILES + ); + let dir_path = fs::read_dir("./benches/regression_files").unwrap(); + for file_path in dir_path { + // TODO(Aner, 23/07/24): multi-thread the test. + test_single_committer_flow( + &fs::read_to_string(file_path.unwrap().path()).unwrap(), + OUTPUT_PATH, + ) + .await; + } +} diff --git a/crates/committer_cli/src/tests/utils/objects.rs b/crates/committer_cli/src/tests/utils/objects.rs index 7a8273c110..c75d6c1dc1 100644 --- a/crates/committer_cli/src/tests/utils/objects.rs +++ b/crates/committer_cli/src/tests/utils/objects.rs @@ -1,13 +1,30 @@ use indexmap::indexmap; -use starknet_api::{ - block_hash::block_hash_calculator::{TransactionHashingData, TransactionOutputForHash}, - core::{ClassHash, CompiledClassHash, ContractAddress, EthAddress, Nonce, PatriciaKey}, - state::{StorageKey, ThinStateDiff}, - transaction::{ - Event, EventContent, EventData, EventKey, Fee, GasVector, L2ToL1Payload, MessageToL1, - RevertedTransactionExecutionStatus, TransactionExecutionStatus, TransactionHash, - TransactionSignature, - }, +use starknet_api::block_hash::block_hash_calculator::{ + TransactionHashingData, + TransactionOutputForHash, +}; +use starknet_api::core::{ + ClassHash, + CompiledClassHash, + ContractAddress, + EthAddress, + Nonce, + PatriciaKey, +}; +use starknet_api::state::{StorageKey, ThinStateDiff}; +use starknet_api::transaction::{ + Event, + EventContent, + EventData, + EventKey, + Fee, + GasVector, + L2ToL1Payload, + MessageToL1, + RevertedTransactionExecutionStatus, + TransactionExecutionStatus, + TransactionHash, + TransactionSignature, }; use starknet_types_core::felt::Felt; @@ -32,10 +49,7 @@ pub(crate) fn get_transaction_output_for_hash( }, }], execution_status: expected_execution_status, - gas_consumed: GasVector { - l1_gas: 0, - l1_data_gas: 64, - }, + gas_consumed: GasVector { l1_gas: 0, l1_data_gas: 64 }, messages_sent: vec![MessageToL1 { from_address: ContractAddress(PatriciaKey::from(2_u128)), to_address: EthAddress::try_from(Felt::from_bytes_be_slice(&[1_u8])) diff --git a/crates/committer_cli/src/tests/utils/parse_from_python.rs b/crates/committer_cli/src/tests/utils/parse_from_python.rs index 3c574b6eb6..78ace87dac 100644 --- a/crates/committer_cli/src/tests/utils/parse_from_python.rs +++ b/crates/committer_cli/src/tests/utils/parse_from_python.rs @@ -1,17 +1,17 @@ -use crate::parse_input::cast::add_unique; -use crate::parse_input::raw_input::RawStorageEntry; +use std::collections::HashMap; + use committer::block_committer::input::StarknetStorageValue; use committer::felt::Felt; use committer::hash::hash_trait::HashOutput; use committer::patricia_merkle_tree::node_data::leaf::LeafModifications; use committer::patricia_merkle_tree::types::NodeIndex; use committer::storage::map_storage::MapStorage; -use committer::storage::storage_trait::StorageKey; -use committer::storage::storage_trait::StorageValue; +use committer::storage::storage_trait::{StorageKey, StorageValue}; use ethnum::U256; -use serde::Deserialize; -use serde::Deserializer; -use std::collections::HashMap; +use serde::{Deserialize, Deserializer}; + +use crate::parse_input::cast::add_unique; +use crate::parse_input::raw_input::RawStorageEntry; pub struct TreeFlowInput { pub leaf_modifications: LeafModifications, @@ -53,13 +53,8 @@ pub fn parse_input_single_storage_tree_flow_test(input: &HashMap let mut storage = HashMap::new(); for entry in raw_storage { - add_unique( - &mut storage, - "storage", - StorageKey(entry.key), - StorageValue(entry.value), - ) - .unwrap(); + add_unique(&mut storage, "storage", StorageKey(entry.key), StorageValue(entry.value)) + .unwrap(); } let map_storage = MapStorage { storage }; @@ -67,9 +62,5 @@ pub fn parse_input_single_storage_tree_flow_test(input: &HashMap // Fetch root_hash. let root_hash = HashOutput(Felt::from_hex(input.get("root_hash").unwrap()).unwrap()); - TreeFlowInput { - leaf_modifications, - storage: map_storage, - root_hash, - } + TreeFlowInput { leaf_modifications, storage: map_storage, root_hash } } diff --git a/crates/committer_cli/src/tests/utils/random_structs.rs b/crates/committer_cli/src/tests/utils/random_structs.rs index e9f777b102..caf0995b8c 100644 --- a/crates/committer_cli/src/tests/utils/random_structs.rs +++ b/crates/committer_cli/src/tests/utils/random_structs.rs @@ -1,22 +1,31 @@ -use committer::block_committer::input::ContractAddress; -use committer::block_committer::input::StarknetStorageValue; +use std::cmp::min; +use std::collections::HashMap; + +use committer::block_committer::input::{ContractAddress, StarknetStorageValue}; use committer::felt::Felt; use committer::hash::hash_trait::HashOutput; use committer::patricia_merkle_tree::external_test_utils::get_random_u256; use committer::patricia_merkle_tree::filled_tree::forest::FilledForest; -use committer::patricia_merkle_tree::filled_tree::node::ClassHash; -use committer::patricia_merkle_tree::filled_tree::node::CompiledClassHash; -use committer::patricia_merkle_tree::filled_tree::node::FilledNode; -use committer::patricia_merkle_tree::filled_tree::node::Nonce; -use committer::patricia_merkle_tree::filled_tree::tree::ClassesTrie; -use committer::patricia_merkle_tree::filled_tree::tree::ContractsTrie; -use committer::patricia_merkle_tree::filled_tree::tree::StorageTrie; -use committer::patricia_merkle_tree::filled_tree::tree::StorageTrieMap; -use committer::patricia_merkle_tree::node_data::inner_node::BinaryData; -use committer::patricia_merkle_tree::node_data::inner_node::EdgeData; -use committer::patricia_merkle_tree::node_data::inner_node::NodeDataDiscriminants as NodeDataVariants; +use committer::patricia_merkle_tree::filled_tree::node::{ + ClassHash, + CompiledClassHash, + FilledNode, + Nonce, +}; +use committer::patricia_merkle_tree::filled_tree::tree::{ + ClassesTrie, + ContractsTrie, + StorageTrie, + StorageTrieMap, +}; use committer::patricia_merkle_tree::node_data::inner_node::{ - EdgePath, EdgePathLength, NodeData, PathToBottom, + BinaryData, + EdgeData, + EdgePath, + EdgePathLength, + NodeData, + NodeDataDiscriminants as NodeDataVariants, + PathToBottom, }; use committer::patricia_merkle_tree::node_data::leaf::ContractState; use committer::patricia_merkle_tree::types::NodeIndex; @@ -25,8 +34,6 @@ use rand::prelude::IteratorRandom; use rand::Rng; use rand_distr::num_traits::ToPrimitive; use rand_distr::{Distribution, Geometric}; -use std::cmp::min; -use std::collections::HashMap; use strum::IntoEnumIterator; pub trait RandomValue { @@ -74,10 +81,7 @@ impl RandomValue for ContractState { impl RandomValue for BinaryData { fn random(rng: &mut R, max: Option) -> Self { - Self { - left_hash: HashOutput::random(rng, max), - right_hash: HashOutput::random(rng, max), - } + Self { left_hash: HashOutput::random(rng, max), right_hash: HashOutput::random(rng, max) } } } @@ -85,9 +89,10 @@ impl RandomValue for PathToBottom { fn random(rng: &mut R, max: Option) -> Self { // Crate a random path and than calculate the length of the path. let path = EdgePath::random(rng, max); - // TODO(Aviv, 27/6/2024): use a built in function once we migrate to a better big-integer library - // Randomly choose the number of real leading zeros in the path (up to the maximum possible). - // Real leading zero is a zero that refer to a left node, and not a padding zero. + // TODO(Aviv, 27/6/2024): use a built in function once we migrate to a better big-integer + // library Randomly choose the number of real leading zeros in the path (up to the + // maximum possible). Real leading zero is a zero that refer to a left node, and not + // a padding zero. let max_real_leading_zeros = path.0.leading_zeros() - EdgePath::MAX.0.leading_zeros(); let real_leading_zeros = std::cmp::min( Geometric::new(0.5) @@ -164,10 +169,7 @@ macro_rules! random_filled_node { ($leaf:ty) => { impl RandomValue for FilledNode<$leaf> { fn random(rng: &mut R, max: Option) -> Self { - Self { - data: NodeData::random(rng, max), - hash: HashOutput::random(rng, max), - } + Self { data: NodeData::random(rng, max), hash: HashOutput::random(rng, max) } } } }; @@ -195,12 +197,7 @@ macro_rules! random_filled_tree { .as_usize(); let mut nodes: Vec<(NodeIndex, FilledNode<$leaf>)> = (0..max_node_number) - .map(|_| { - ( - NodeIndex::random(rng, max_size), - FilledNode::random(rng, max_size), - ) - }) + .map(|_| (NodeIndex::random(rng, max_size), FilledNode::random(rng, max_size))) .collect(); nodes.push((NodeIndex::ROOT, FilledNode::random(rng, max_size))); @@ -234,20 +231,13 @@ impl DummyRandomValue for FilledForest { let storage_tries: StorageTrieMap = (0..max_trees_number) .map(|_| { - ( - ContractAddress::random(rng, max_size), - StorageTrie::dummy_random(rng, max_size), - ) + (ContractAddress::random(rng, max_size), StorageTrie::dummy_random(rng, max_size)) }) .collect::>(); let contracts_trie = ContractsTrie::dummy_random(rng, max_size); let classes_trie = ClassesTrie::dummy_random(rng, max_size); - Self { - storage_tries, - contracts_trie, - classes_trie, - } + Self { storage_tries, contracts_trie, classes_trie } } } From 2dd6ac36c3495ab9af565070b0317c45e069e860 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Wed, 24 Jul 2024 20:01:06 +0300 Subject: [PATCH 2/2] No conflicts in main-v0.13.2 -> main merge, this commit is for any change needed to pass the CI.