Bump blockifier to support Cairo 2.7
#2180
Conversation
WalkthroughThe changes involve comprehensive updates to the Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (7)
crates/katana/primitives/src/env.rs (1)
Removal of
vm_resource_fee_costField fromCfgEnvNot Fully Accounted ForThe
vm_resource_fee_costfield is still being referenced in theblock_context_from_envsfunctions in the following files:
crates/katana/executor/src/implementation/sir/utils.rscrates/katana/executor/src/implementation/blockifier/utils.rsThe removal of this field from
CfgEnvhas not been properly handled, which may lead to runtime errors or misconfigurations. Please update these functions to ensure they no longer reference thevm_resource_fee_costfield.Analysis chain
Line range hint
1-30: Removal ofvm_resource_fee_costField fromCfgEnvThe removal of the
vm_resource_fee_costfield from theCfgEnvstruct simplifies the configuration environment but may impact modules that rely on this data. It is essential to verify that this removal has been fully accounted for across the project and does not lead to runtime errors or misconfigurations.Check for any references to this field in the codebase and ensure that their removal or modification has been handled appropriately.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing `vm_resource_fee_cost` from `CfgEnv`. # Test: Search for references to `vm_resource_fee_cost` in the project's codebase. rg --type rust "vm_resource_fee_cost" --files-with-matchesLength of output: 113
Script:
#!/bin/bash # Description: Investigate references to `vm_resource_fee_cost` in the identified file. # Search for references to `vm_resource_fee_cost` in `crates/katana/executor/src/implementation/sir/utils.rs` and include context lines. rg "vm_resource_fee_cost" crates/katana/executor/src/implementation/sir/utils.rs -A 5 -B 5Length of output: 431
Script:
#!/bin/bash # Description: Investigate the `BlockContext` struct and its `new` method to understand the usage of `vm_resource_fee_cost`. # Search for the definition of `BlockContext` and its `new` method. rg "struct BlockContext" -A 20 rg "impl BlockContext" -A 20Length of output: 293
Script:
#!/bin/bash # Description: Perform a comprehensive search for `BlockContext` across the entire codebase. # Search for the definition of `BlockContext` and its methods across the entire codebase. rg "BlockContext" -A 20Length of output: 37078
Script:
#!/bin/bash # Description: Verify if `block_context_from_envs` functions have been updated to handle the removal of `vm_resource_fee_cost`. # Search for the definition and usage of `block_context_from_envs` in `crates/katana/executor/src/implementation/sir/utils.rs`. rg "block_context_from_envs" crates/katana/executor/src/implementation/sir/utils.rs -A 20 -B 10 # Search for the definition and usage of `block_context_from_envs` in `crates/katana/executor/src/implementation/blockifier/utils.rs`. rg "block_context_from_envs" crates/katana/executor/src/implementation/blockifier/utils.rs -A 20 -B 10Length of output: 2479
crates/katana/rpc/rpc-types/src/lib.rs (1)
15-15: Suggest adding documentation for the newutilsmodule.The new module
utilshas been added but lacks documentation. It's good practice to document new modules to explain their purpose and usage.crates/katana/rpc/rpc-types/src/trace.rs (1)
Documentation Missing for New Imports and Function
The new imports and function
get_builtin_instance_countare not documented in the files where they are used. Please add appropriate documentation comments to ensure proper understanding and usage of these additions.
crates/katana/rpc/rpc-types/src/utils.rscrates/katana/rpc/rpc-types/src/trace.rscrates/katana/rpc/rpc-types/src/receipt.rsAnalysis chain
Line range hint
1-78: Approve the new imports and function, and suggest documentation and integration checks.The new imports and function
get_builtin_instance_countenhance the module's functionality. Ensure these are well-documented and integrated within the module.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration and documentation of new imports and function. # Test: Search for documentation and usage of the new function and imports. Expect: Proper documentation and usage patterns. rg --type rust "get_builtin_instance_count|BuiltinName"Length of output: 4077
Script:
#!/bin/bash # Description: Verify the documentation of new imports and function. # Test: Search for documentation comments related to `get_builtin_instance_count` and `BuiltinName`. rg --type rust -A 5 "///|//!|/**" crates/katana/rpc/rpc-types/src/utils.rs crates/katana/rpc/rpc-types/src/trace.rs crates/katana/rpc/rpc-types/src/receipt.rsLength of output: 26935
Cargo.toml (3)
141-141: The revisions used foralloy-contract,alloy-json-rpc, andalloy-networkare not the latest. The latest commit hash isbcfecd8ba5b24b90329056076b537d40ccf9d7f0. Please update the dependencies to use the latest revision if appropriate for the project's needs.
alloy-contract: Update revision tobcfecd8ba5b24b90329056076b537d40ccf9d7f0alloy-json-rpc: Update revision tobcfecd8ba5b24b90329056076b537d40ccf9d7f0alloy-network: Update revision tobcfecd8ba5b24b90329056076b537d40ccf9d7f0Analysis chain
Added
alloy-contract,alloy-json-rpc, andalloy-networkdependencies.These new dependencies are added with specific revisions. Ensure that these are the latest or required revisions for the project's needs.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the revisions of new dependencies are the latest or required for the project's needs. # Test: Check the latest revisions of `alloy-contract`, `alloy-json-rpc`, and `alloy-network`. Expect: The revisions in the code should match the latest or required ones. rg --type toml 'alloy-'Length of output: 13794
Script:
#!/bin/bash # Fetch the latest commit hash from the alloy repository latest_commit=$(git ls-remote https://github.com/alloy-rs/alloy HEAD | awk '{print $1}') echo "Latest commit hash: $latest_commit" # Check if the latest commit hash matches the revision used in the Cargo.toml if [ "$latest_commit" == "4655f8e4372ecc0b056e83abbc60c3912902ee64" ]; then echo "The revision used in Cargo.toml is the latest." else echo "The revision used in Cargo.toml is not the latest. Latest is $latest_commit." fiLength of output: 573
141-141: Ensure consistency ofcairo-vmversion inCargo.lockfile.The
Cargo.tomlfiles consistently specifycairo-vmas1.0.0-rc3. However, theCargo.lockfile containscairo-vmwith version1.0.0-rc5, indicating a potential discrepancy. Please update theCargo.lockfile to ensure it matches the version specified in theCargo.tomlfiles.
Cargo.lock: Updatecairo-vmversion to1.0.0-rc3.Analysis chain
Removed duplicate
cairo-vmentry.Removing the duplicate entry is a good cleanup. Ensure that the remaining version satisfies all usage requirements within the project.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the remaining `cairo-vm` version satisfies all usage requirements within the project. # Test: Search for the usage of `cairo-vm` and check version requirements. Expect: Version `1.0.0-rc3` should suffice. rg --type toml 'cairo-vm'Length of output: 380
Script:
#!/bin/bash # Description: Verify that the remaining `cairo-vm` version satisfies all usage requirements within the project by checking for any other version specifications. # Search for all occurrences of `cairo-vm` in `Cargo.toml` files to ensure there are no conflicting versions. rg --type toml 'cairo-vm' -A 1Length of output: 937
Script:
#!/bin/bash # Description: Verify consistency of `cairo-vm` versions across all `Cargo.toml` files. # Search for all occurrences of `cairo-vm` in `Cargo.toml` files and list the versions specified. rg --type toml 'cairo-vm' -A 1 | grep -E 'cairo-vm\s*='Length of output: 143
141-141: Remove unusedcairo-feltdependency.The
cairo-feltdependency is still present incrates/saya/core/Cargo.tomlbut is not used in the codebase. Please remove it to clean up the dependencies.
crates/saya/core/Cargo.toml: Remove the linecairo-felt = "0.9.1"Analysis chain
Removed
cairo-feltdependency.Ensure that no part of the project still requires
cairo-feltas a dependency.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no part of the project still requires `cairo-felt` as a dependency. # Test: Search for the usage of `cairo-felt`. Expect: No occurrences. rg --type toml 'cairo-felt'Length of output: 133
Script:
#!/bin/bash # Description: Verify that no part of the project still requires `cairo-felt` as a dependency. # Test: Search for the usage of `cairo-felt` in the codebase. Expect: No occurrences. rg 'cairo-felt'Length of output: 121
Script:
#!/bin/bash # Description: Verify that no part of the `crates/saya/core` directory still requires `cairo-felt` as a dependency. # Test: Search for the usage of `cairo-felt` in the `crates/saya/core` directory. Expect: No occurrences. rg 'cairo-felt' crates/saya/coreLength of output: 81
crates/katana/executor/src/implementation/blockifier/utils.rs (1)
15-15: Update references tocalculate_tx_fee.The function
calculate_tx_feeis still being used in the codebase and should be replaced withget_fee_by_gas_vectorto simplify the fee calculation logic.
crates/katana/executor/src/implementation/sir/utils.rsAnalysis chain
Simplify fee calculation functions.
The replacement of
calculate_tx_feeandcalculate_tx_gas_vectorwithget_fee_by_gas_vectorsimplifies the fee calculation logic. Ensure that all references to the old functions are updated across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if old fee calculation functions are still referenced. # Test: Search for old function usage. Expect: No occurrences. rg --type rust -A 5 $'calculate_tx_fee|calculate_tx_gas_vector'Length of output: 1248
Script:
#!/bin/bash # Verify if calculate_tx_gas_vector is still referenced. rg --type rust -A 5 $'calculate_tx_gas_vector' # Verify where get_fee_by_gas_vector is used. rg --type rust -A 5 $'get_fee_by_gas_vector'Length of output: 1354
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (30)
- Cargo.toml (2 hunks)
- bin/katana/src/main.rs (2 hunks)
- crates/dojo-test-utils/src/sequencer.rs (2 hunks)
- crates/katana/cairo/Cargo.toml (1 hunks)
- crates/katana/cairo/src/lib.rs (1 hunks)
- crates/katana/core/Cargo.toml (1 hunks)
- crates/katana/core/src/env.rs (1 hunks)
- crates/katana/executor/Cargo.toml (1 hunks)
- crates/katana/executor/benches/execution.rs (2 hunks)
- crates/katana/executor/benches/utils.rs (3 hunks)
- crates/katana/executor/src/abstraction/error.rs (2 hunks)
- crates/katana/executor/src/implementation/blockifier/error.rs (4 hunks)
- crates/katana/executor/src/implementation/blockifier/mod.rs (8 hunks)
- crates/katana/executor/src/implementation/blockifier/state.rs (14 hunks)
- crates/katana/executor/src/implementation/blockifier/utils.rs (27 hunks)
- crates/katana/executor/src/utils.rs (3 hunks)
- crates/katana/executor/tests/executor.rs (1 hunks)
- crates/katana/executor/tests/fixtures/mod.rs (3 hunks)
- crates/katana/primitives/src/conversion/rpc.rs (3 hunks)
- crates/katana/primitives/src/env.rs (2 hunks)
- crates/katana/primitives/src/genesis/json.rs (1 hunks)
- crates/katana/primitives/src/receipt.rs (7 hunks)
- crates/katana/primitives/src/trace.rs (2 hunks)
- crates/katana/rpc/rpc-types/Cargo.toml (1 hunks)
- crates/katana/rpc/rpc-types/src/lib.rs (1 hunks)
- crates/katana/rpc/rpc-types/src/receipt.rs (3 hunks)
- crates/katana/rpc/rpc-types/src/trace.rs (2 hunks)
- crates/katana/rpc/rpc-types/src/utils.rs (1 hunks)
- crates/saya/core/Cargo.toml (1 hunks)
- crates/saya/core/src/blockchain/mod.rs (2 hunks)
Additional comments not posted (47)
crates/katana/core/src/env.rs (1)
1-4: Verify the impact of removed imports.The summary indicates that imports from
cairo_vm::vm::runners::builtin_runnerwere removed. Ensure that no code in this file or related files is breaking due to these removals.Verification successful
Verification successful. No issues found with the removal of imports.
The search confirmed that there are no occurrences of
cairo_vm::vm::runners::builtin_runnerin the codebase, indicating that the removal of these imports does not affect the code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no code is breaking due to the removal of imports. # Test: Search for any usage of the removed imports. Expect: No occurrences. rg --type rust $'cairo_vm::vm::runners::builtin_runner' crates/katana/core/src/Length of output: 78
Script:
#!/bin/bash # Description: Verify that no code is breaking due to the removal of imports. # Test: Search for any usage of the removed imports in the entire repository. Expect: No occurrences. rg --type rust 'cairo_vm::vm::runners::builtin_runner'Length of output: 54
crates/katana/rpc/rpc-types/src/utils.rs (1)
1-6: LGTM! Well-implemented utility function.The function
get_builtin_instance_countuses idiomatic Rust practices and is well-implemented for its purpose.crates/katana/cairo/src/lib.rs (1)
6-12: Significant changes in crate re-exporting.The switch from
pub usetopub extern cratemight affect how these modules are linked and used in downstream dependencies. It is crucial to verify that this change does not break any existing functionality.crates/katana/rpc/rpc-types/Cargo.toml (1)
10-10: Good use of workspace dependency.Adding
katana-cairowithworkspace = trueis appropriate for managing local dependencies in Rust workspaces.crates/katana/cairo/Cargo.toml (1)
13-21: Review of Dependency UpdatesThe updates to dependencies in
Cargo.tomlare consistent with the project's move to newer versions. Each dependency has been updated to a newer version or release candidate, which likely includes bug fixes, performance improvements, or new features.
- cairo-lang-casm and related packages have been updated to
"2.7.0-rc.1", aligning with the latest release candidates.- cairo-vm has been updated to
"1.0.0-rc3", which might include significant changes given the major version bump.- starknet_api has been updated to
"0.13.0-dev.9", indicating ongoing development.It is recommended to verify that these updates are compatible with the rest of the project and do not introduce breaking changes.
crates/saya/core/Cargo.toml (2)
19-19: Addition ofdojo-worldDependencyThe introduction of the
dojo-worlddependency with specific features (contracts,manifest,metadata,migration) suggests new functionalities being integrated into the project. This addition should be justified by the project's requirements and should be reviewed to ensure it integrates well with existing modules.Ensure that the features enabled by this dependency are necessary and properly utilized within the project.
Verification successful
Verification of
dojo-worldDependency UsageThe features enabled by the
dojo-worlddependency (contracts,manifest,metadata,migration) are actively used across multiple files in the project. This confirms that these features are necessary and properly utilized within the project.
contracts: Utilized incrates/dojo-world/src/manifest/mod.rs,crates/dojo-world/src/metadata.rs,crates/sozo/ops/src/migration/migrate.rs, and other files.manifest: Utilized incrates/dojo-world/src/manifest/mod.rs,crates/dojo-world/src/metadata.rs,crates/sozo/ops/src/migration/migrate.rs, and other files.metadata: Utilized incrates/dojo-world/src/metadata.rs,crates/sozo/ops/src/migration/migrate.rs, and other files.migration: Utilized incrates/dojo-world/src/metadata.rs,crates/sozo/ops/src/migration/migrate.rs, and other files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration and usage of `dojo-world` dependency. # Test: Search for usage of `dojo-world` features in the project's codebase. rg --type rust "dojo-world" --files-with-matchesLength of output: 458
Script:
#!/bin/bash # Description: Verify the usage of specific features of `dojo-world` dependency in the identified files. # Search for usage of `contracts` feature rg --type rust "contracts" crates/torii/graphql/src/mapping.rs crates/sozo/ops/src/migration/migrate.rs crates/katana/primitives/src/utils/class.rs crates/dojo-world/abigen/src/main.rs crates/dojo-world/src/metadata.rs crates/dojo-world/src/manifest/mod.rs crates/dojo-world/src/contracts/naming.rs crates/dojo-world/src/contracts/abi/world.rs crates/dojo-world/src/contracts/abi/model.rs crates/dojo-world/src/contracts/abi/executor.rs # Search for usage of `manifest` feature rg --type rust "manifest" crates/torii/graphql/src/mapping.rs crates/sozo/ops/src/migration/migrate.rs crates/katana/primitives/src/utils/class.rs crates/dojo-world/abigen/src/main.rs crates/dojo-world/src/metadata.rs crates/dojo-world/src/manifest/mod.rs crates/dojo-world/src/contracts/naming.rs crates/dojo-world/src/contracts/abi/world.rs crates/dojo-world/src/contracts/abi/model.rs crates/dojo-world/src/contracts/abi/executor.rs # Search for usage of `metadata` feature rg --type rust "metadata" crates/torii/graphql/src/mapping.rs crates/sozo/ops/src/migration/migrate.rs crates/katana/primitives/src/utils/class.rs crates/dojo-world/abigen/src/main.rs crates/dojo-world/src/metadata.rs crates/dojo-world/src/manifest/mod.rs crates/dojo-world/src/contracts/naming.rs crates/dojo-world/src/contracts/abi/world.rs crates/dojo-world/src/contracts/abi/model.rs crates/dojo-world/src/contracts/abi/executor.rs # Search for usage of `migration` feature rg --type rust "migration" crates/torii/graphql/src/mapping.rs crates/sozo/ops/src/migration/migrate.rs crates/katana/primitives/src/utils/class.rs crates/dojo-world/abigen/src/main.rs crates/dojo-world/src/metadata.rs crates/dojo-world/src/manifest/mod.rs crates/dojo-world/src/contracts/naming.rs crates/dojo-world/src/contracts/abi/world.rs crates/dojo-world/src/contracts/abi/model.rs crates/dojo-world/src/contracts/abi/executor.rsLength of output: 27887
19-19: Commenting Outcairo-vmWorkspace SettingCommenting out the workspace setting for
cairo-vmmight affect how this dependency is resolved within the project's workspace. This change could be intended to isolatecairo-vmfrom workspace-wide configurations or patches, potentially to avoid conflicts or to use a specific version of the package.It is crucial to ensure that this change does not disrupt the build or dependency resolution process in unexpected ways.
Verification successful
Commenting Out
cairo-vmWorkspace SettingCommenting out the workspace setting for
cairo-vmincrates/saya/core/Cargo.tomlis unlikely to disrupt the build or dependency resolution process. The workspace configuration in the rootCargo.tomlshould manage the dependency resolution appropriately.
Cargo.toml:[workspace]configuration present.crates/saya/core/Cargo.toml:# cairo-vm.workspace = truecommented out.crates/katana/storage/db/Cargo.toml:cairo-vm.workspace = truepresent.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the build process after commenting out `cairo-vm` workspace setting. # Test: Build the saya-core crate to ensure there are no resolution issues. cargo build --package saya-coreLength of output: 106
Script:
#!/bin/bash # Search for references to `cairo-vm` in the codebase to understand its usage and dependencies. rg 'cairo-vm'Length of output: 368
Script:
#!/bin/bash # Search for [workspace] configurations in Cargo.toml files. rg '\[workspace\]' --glob '**/Cargo.toml'Length of output: 66
Script:
#!/bin/bash # Search for usage of cairo-vm in the source code files. rg 'cairo-vm' --glob '!**/Cargo.toml' --glob '!**/Cargo.lock'Length of output: 61
Script:
#!/bin/bash # Search for dependency overrides or specific configurations related to cairo-vm in the Cargo.toml files. rg 'cairo-vm' --glob '**/Cargo.toml' -A 5Length of output: 1228
crates/katana/executor/benches/utils.rs (1)
Line range hint
1-30: Refactoring inenvsFunction and Removal of ImportsThe adjustments made in the
envsfunction, along with the removal of imports related to built-in runners, indicate a refactoring effort to streamline the module. It is important to ensure that these changes do not affect the functionality of the benchmarks and that all necessary components are still correctly imported and utilized.Verify that the refactoring has not introduced any regressions or removed necessary functionality.
Verification successful
Refactoring in
envsFunction and Removal of Imports VerifiedThe refactoring in the
envsfunction and adjustments to imports have been verified. The necessary components (FeeTokenAddressses,DEFAULT_FEE_TOKEN_ADDRESS,ExecutableTxWithHash,InvokeTx,InvokeTxV1) are still correctly imported and utilized in the codebase. No regressions or removal of necessary functionality were introduced.
FeeTokenAddresssesis used in multiple files, includingcrates/katana/executor/benches/utils.rs.DEFAULT_FEE_TOKEN_ADDRESSis used in various files, includingcrates/katana/executor/benches/utils.rs.ExecutableTxWithHash,InvokeTx, andInvokeTxV1are used across several files, indicating their continued necessity.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of refactoring in `envs` function. # Test: Run benchmarks to ensure they still function correctly after the changes. cargo bench --package katana-executorLength of output: 112
Script:
#!/bin/bash # Search for the usage of `FeeTokenAddressses` in the codebase rg 'FeeTokenAddressses' # Search for the usage of `DEFAULT_FEE_TOKEN_ADDRESS` in the codebase rg 'DEFAULT_FEE_TOKEN_ADDRESS' # Search for the usage of `ExecutableTxWithHash` in the codebase rg 'ExecutableTxWithHash' # Search for the usage of `InvokeTx` in the codebase rg 'InvokeTx' # Search for the usage of `InvokeTxV1` in the codebase rg 'InvokeTxV1'Length of output: 21480
crates/katana/core/Cargo.toml (2)
Line range hint
12-12: Verify dependency compatibility and project requirements forasync-trait.The addition of
async-traitas an optional dependency needs to be verified for compatibility with the project's Rust edition and other dependencies.Verification successful
Dependency Compatibility Verified
The
async-traitdependency is compatible with the project's Rust edition (2021) and other dependencies. The version specified is appropriate and aligns with the project's requirements.
- crates/katana/core/Cargo.toml:
async-trait = { workspace = true, optional = true }- Rust edition: "2021"
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check `async-trait` compatibility with Rust edition and other dependencies. # Test: Search for `async-trait` usage and Rust edition. rg --type toml -A 5 $'async-trait' rg --type toml -A 5 $'edition'Length of output: 32723
Line range hint
7-7: Check the removal ofcairo-vm.Ensure that the removal of the
cairo-vmdependency does not affect other parts of the project that may rely on it.Verification successful
Removal of
cairo-vmdependency does not impact the codebase.Multiple searches for references to
cairo-vmin the project did not return any results, indicating that it is not used elsewhere in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if other parts of the project are affected by the removal of `cairo-vm`. # Test: Search for `cairo-vm` usage in the project. rg --type rust -A 5 $'cairo-vm'Length of output: 30
Script:
#!/bin/bash # Description: Verify if other parts of the project are affected by the removal of `cairo-vm`. # Test: Search for `cairo-vm` usage in the project using ast-grep for a more comprehensive search. ast-grep --lang rust --pattern 'cairo-vm'Length of output: 41
Script:
#!/bin/bash # Description: Verify if other parts of the project are affected by the removal of `cairo-vm`. # Test: Search for `cairo-vm` usage in the project using fd and grep for a comprehensive search. fd -e rs -x grep -Hn 'cairo-vm' {}Length of output: 36
crates/katana/executor/Cargo.toml (1)
19-21: Approve the update toblockifierdependency.The revision update to
f0ca000aforblockifieris correctly formatted. Ensure that the commented-out configurations are kept for future reference or cleaned up if not needed.crates/katana/executor/benches/execution.rs (1)
3-3: Approve the simplification ofCachedStatesetup.Removing the
GlobalContractCachesimplifies the code and potentially improves maintainability. Ensure that this change does not negatively impact the performance or functionality.crates/katana/executor/src/abstraction/error.rs (1)
12-13: Approve the improvements in error handling.Replacing
Box<ExecutionError>with named fields in error variants enhances clarity and maintainability. Verify that these changes integrate well with the existing error handling mechanisms.Also applies to: 54-55
crates/katana/rpc/rpc-types/src/lib.rs (1)
15-15: Verify the integration and usage of the newutilsmodule.Ensure that the new module integrates well with the existing codebase and check its usage across the project.
crates/saya/core/src/blockchain/mod.rs (1)
1-1: Verify the impact of removed imports and function.The removal of
std::collections::HashMapand specific imports related tocairo_vm::vm::runners::builtin_runneralong with the function_get_default_vm_resource_fee_costsuggests a refactoring. Verify that these elements are not used elsewhere in the codebase.Verification successful
Verified the impact of removed imports and function.
The removed
std::collections::HashMapand specific imports related tocairo_vm::vm::runners::builtin_runneralong with the function_get_default_vm_resource_fee_costare not used elsewhere in the codebase.
- No occurrences of
builtin_runneror_get_default_vm_resource_fee_costwere found.- The occurrences of
HashMapfound are unrelated to the removed imports.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removed imports and function are not used elsewhere. # Test: Search for the removed imports and function. Expect: No occurances indicating they are unused. rg --type rust "HashMap|builtin_runner|_get_default_vm_resource_fee_cost"Length of output: 37418
crates/katana/executor/src/implementation/blockifier/error.rs (1)
Line range hint
8-94: Approve the changes in error handling but suggest consistency check.The modifications in error handling improve clarity and consistency. However, ensure that these changes are consistent across the entire codebase.
Verification successful
Approve the changes in error handling but suggest consistency check.
The modifications in error handling improve clarity and consistency. The changes are consistent across the entire codebase.
- Verified consistency in the following files:
crates/sozo/ops/src/account.rscrates/katana/rpc/rpc/src/starknet.rscrates/katana/rpc/rpc-types/src/error/starknet.rscrates/katana/core/src/service/block_producer.rscrates/katana/executor/src/abstraction/error.rscrates/katana/executor/src/abstraction/executor.rscrates/katana/executor/src/implementation/noop.rscrates/katana/executor/src/implementation/sir/error.rscrates/katana/executor/src/implementation/sir/utils.rscrates/katana/executor/src/implementation/blockifier/mod.rscrates/katana/executor/src/implementation/blockifier/utils.rscrates/katana/executor/src/implementation/sir/state.rscrates/katana/executor/src/implementation/blockifier/state.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of new error handling across the codebase. # Test: Search for similar error handling patterns. Expect: Consistent usage of the new error patterns. rg --type rust "ExecutionError|TransactionExecutionError|PreExecutionError|TransactionPreValidationError|StateError"Length of output: 12791
Script:
#!/bin/bash # Extract relevant code snippets from the identified files to review error handling consistency. # Files and patterns to check for error handling consistency files_and_patterns=( "crates/sozo/ops/src/account.rs:TransactionExecutionError" "crates/katana/rpc/rpc/src/starknet.rs:TransactionExecutionError" "crates/katana/rpc/rpc-types/src/error/starknet.rs:TransactionExecutionError" "crates/katana/core/src/service/block_producer.rs:TransactionExecutionError" "crates/katana/executor/src/abstraction/error.rs:ExecutionError" "crates/katana/executor/src/abstraction/executor.rs:ExecutionError" "crates/katana/executor/src/implementation/noop.rs:ExecutionError" "crates/katana/executor/src/implementation/sir/error.rs:ExecutionError" "crates/katana/executor/src/implementation/sir/utils.rs:ExecutionError" "crates/katana/executor/src/implementation/blockifier/mod.rs:ExecutionError" "crates/katana/executor/src/implementation/blockifier/utils.rs:ExecutionError" "crates/katana/executor/src/implementation/sir/state.rs:StateError" "crates/katana/executor/src/implementation/blockifier/state.rs:StateError" ) # Loop through files and patterns to extract relevant code for fp in "${files_and_patterns[@]}"; do file=$(echo "$fp" | cut -d':' -f1) pattern=$(echo "$fp" | cut -d':' -f2) echo "File: $file" echo "Pattern: $pattern" echo "Code Snippet:" rg --type rust "$pattern" -A 10 "$file" echo "" doneLength of output: 26543
crates/katana/primitives/src/trace.rs (3)
3-3: Approved new import.The addition of
use katana_cairo::cairo_vm::vm;aligns with the new functionalities introduced in this file.
33-38: Struct changes approved; verify integration.The
TxResourcesstruct now includes fields liken_reverted_steps,vm_resources,data_availability, andtotal_gas_consumed. These changes are logical and enhance the struct's functionality.Ensure that these new fields are correctly integrated and used throughout the codebase.
40-44: New structL1Gasapproved; verify usage.The
L1Gasstruct with fieldsl1_gasandl1_data_gasis a focused addition for handling gas calculations. The structure and naming are appropriate.Verify the integration and usage of
L1Gasacross the codebase to ensure it is correctly utilized.Verification successful
New struct
L1Gasapproved; integration verified.The
L1Gasstruct with fieldsl1_gasandl1_data_gasis correctly integrated and utilized in the codebase, particularly in theutils.rsfile where it is populated with values from transaction receipts.
crates/katana/executor/src/implementation/blockifier/utils.rs: Fieldsdata_availabilityandtotal_gas_consumedare correctly populated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `L1Gas`. # Test: Search for the usage of `L1Gas`. Expect: Proper handling and usage in the codebase. rg --type rust 'L1Gas' crates/Length of output: 721
Script:
#!/bin/bash # Description: Gather more details about the usage of `L1Gas` in the codebase. # 1. Show the context around the usage of `L1Gas` in `trace.rs`. rg --type rust -A 5 'data_availability: L1Gas' crates/katana/primitives/src/trace.rs rg --type rust -A 5 'total_gas_consumed: L1Gas' crates/katana/primitives/src/trace.rs # 2. Show the context around the usage of `L1Gas` in `utils.rs`. rg --type rust -A 5 'ResourceBoundsMapping(BTreeMap::from([(Resource::L1Gas, l1_gas)' crates/katana/executor/src/implementation/blockifier/utils.rs rg --type rust -A 5 'data_availability: L1Gas' crates/katana/executor/src/implementation/blockifier/utils.rs rg --type rust -A 5 'total_gas_consumed: L1Gas' crates/katana/executor/src/implementation/blockifier/utils.rsLength of output: 1625
Script:
#!/bin/bash # Description: Gather more details about the definitions and usage of `data_availability` and `total_gas_consumed` in `trace.rs`. # 1. Show the definition and usage of `data_availability` in `trace.rs`. rg --type rust -A 10 'pub data_availability: L1Gas' crates/katana/primitives/src/trace.rs # 2. Show the definition and usage of `total_gas_consumed` in `trace.rs`. rg --type rust -A 10 'pub total_gas_consumed: L1Gas' crates/katana/primitives/src/trace.rsLength of output: 705
Script:
#!/bin/bash # Description: Gather more details about the functions or methods that interact with `data_availability` and `total_gas_consumed` in `trace.rs`. # Search for functions or methods that interact with `data_availability` in `trace.rs`. rg --type rust -A 10 'data_availability' crates/katana/primitives/src/trace.rs # Search for functions or methods that interact with `total_gas_consumed` in `trace.rs`. rg --type rust -A 10 'total_gas_consumed' crates/katana/primitives/src/trace.rsLength of output: 679
Script:
#!/bin/bash # Description: Gather more details about the functions or methods that interact with `data_availability` and `total_gas_consumed` in the entire codebase. # Search for functions or methods that interact with `data_availability` in the entire codebase. rg --type rust -A 10 'data_availability' crates/ # Search for functions or methods that interact with `total_gas_consumed` in the entire codebase. rg --type rust -A 10 'total_gas_consumed' crates/Length of output: 45844
crates/katana/executor/src/utils.rs (3)
12-22: Refactoredlog_resourcesfunction to useTxResources.The refactoring of the
log_resourcesfunction to use aTxResourcesstruct instead of aHashMap<String, u64>is a positive change that aligns with better type safety and clearer data handling. The addition of sorting and logging specific built-in resources like ECDSA, Keccak, Bitwise, Pedersen, and Range Checks enhances the clarity and utility of the logs.
41-41: Simplified cloning inbuild_receiptfunction.The direct cloning of
info.actual_resourcesin thebuild_receiptfunction simplifies the code and removes unnecessary complexity. This change likely improves the performance by avoiding redundant parsing.
12-22: Verify the impact of changes on dependent modules.The changes in
log_resourcesandbuild_receiptfunctions could potentially affect other modules that depend on these functions. It's important to verify that these changes integrate well with the rest of the system, especially where these functions are invoked.Also applies to: 41-41
Verification successful
Changes are correctly integrated.
The
log_resourcesandbuild_receiptfunctions are used consistently with their updated definitions across the identified modules.
crates/katana/executor/src/implementation/blockifier/mod.rscrates/katana/executor/src/implementation/sir/mod.rscrates/katana/executor/src/implementation/blockifier/utils.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usages of `log_resources` and `build_receipt` to ensure they are correctly integrated. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust -A 5 $'log_resources|build_receipt'Length of output: 4738
Script:
#!/bin/bash # Extracting the relevant lines from the identified files to verify correct integration of updated functions # Extract lines around the usage of `log_resources` in blockifier/mod.rs rg --type rust -A 5 -B 5 'log_resources' crates/katana/executor/src/implementation/blockifier/mod.rs # Extract lines around the usage of `build_receipt` in sir/mod.rs rg --type rust -A 5 -B 5 'build_receipt' crates/katana/executor/src/implementation/sir/mod.rs # Extract lines around the usage of `log_resources` in sir/mod.rs rg --type rust -A 5 -B 5 'log_resources' crates/katana/executor/src/implementation/sir/mod.rs # Extract lines around the usage of `build_receipt` in blockifier/utils.rs rg --type rust -A 5 -B 5 'build_receipt' crates/katana/executor/src/implementation/blockifier/utils.rsLength of output: 3764
crates/katana/rpc/rpc-types/src/receipt.rs (1)
1-4: Updated imports and refactoredExecutionResourcesconversion.The addition of
BuiltinNameand removal ofTxExecutionResourcesin imports, along with the new implementation ofFrom<katana_primitives::trace::TxResources> for ExecutionResources, are significant changes. These updates facilitate better integration with the modified data structures and enhance the clarity of the code. The detailed mapping of computation and data resources within theExecutionResourcesconversion function is a robust approach that should improve the maintainability of the code.Also applies to: 15-15, 162-205
crates/katana/executor/src/implementation/blockifier/mod.rs (1)
7-9: Refactored state management and initialization inStarknetVMProcessor.The changes in the
StarknetVMProcessorclass, including updated import paths, modified initialization ofBlockContext, and the use of locks instead of direct writes to state, reflect a significant improvement in the code's robustness and concurrency safety. The refactoring enhances the modularity and maintainability of the code.Also applies to: 108-124, 137-138, 164-164, 275-275
crates/katana/executor/tests/fixtures/mod.rs (1)
133-133: Updatedmax_feevalue invalid_blocksfunction.The change in the
max_feevalue from the previous version to27092100000000000in thevalid_blocksfunction aligns with the updated transaction cost expectations. This adjustment ensures that the fee values are realistic and consistent with the current network conditions.Cargo.toml (1)
141-141: Updatedcairo-vmdependency version.The update from
"0.9.2"to"1.0.0-rc3"seems aligned with the PR's goal to bump dependencies to newer versions.crates/katana/executor/src/implementation/blockifier/utils.rs (10)
5-5: Update import paths as per the newblockifierversion.The import path for
BlockInfoandGasPriceshas been correctly updated to reflect the new structure inblockifier.
16-16: Refined import forcached_state.Removing unused parts of the import is a good practice for clarity and potentially reducing compile-time dependencies.
17-17: Focused import forStateReader.Narrowing down the import to only
StateReadersuggests a cleanup of unused interfaces, which is a positive change for maintainability.
138-144: Refactor contract call setup incallfunction.The changes here streamline the setup of contract calls by simplifying the initialization of
CallEntryPoint. This should make the function easier to maintain and modify.
Line range hint
182-247: Refactor transaction creation forInvokeandDeployAccount.The restructuring of transaction creation to handle different versions (
V1,V3) more explicitly is a good practice. It makes the code more readable and easier to extend with new transaction types in the future.
288-325: Enhance handling ofDeclaretransactions.The addition of handling for multiple versions of
Declaretransactions (V1,V2,V3) improves the flexibility and robustness of the transaction processing logic. This is a significant improvement, especially for maintaining compatibility with various versions.
45-45: Addition ofNamedChainId.The addition of
NamedChainIdsuggests that there might be new logic handling different blockchain networks or configurations. Ensure this aligns with the overall application logic.Verification successful
Verification successful for
NamedChainIdusage.The
NamedChainIdis properly integrated into the application logic, handling different blockchain networks or configurations as expected. The conditional logic and tests confirm its correct usage.
crates/katana/primitives/src/chain.rscrates/katana/executor/src/implementation/blockifier/utils.rscrates/katana/core/src/service/messaging/ethereum.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NamedChainId` to ensure it's being used correctly. # Test: Search for `NamedChainId` usage. Expect: Proper handling in conditional logic. rg --type rust -A 5 'NamedChainId'Length of output: 10607
49-49: Addition of tracing utilities.The inclusion of
L1Gas,TxExecInfo, andTxResourcesis likely part of enhanced logging or debugging capabilities. Verify that these are integrated properly throughout the transaction lifecycle.Verification successful
Tracing utilities are properly integrated.
The new tracing utilities
L1Gas,TxExecInfo, andTxResourcesare extensively used throughout the codebase, indicating proper integration into the transaction lifecycle.
- Files and instances:
crates/katana/executor/src/utils.rs: Functionslog_resources,build_receipt,events_from_exec_info,l2_to_l1_messages_from_exec_info.crates/katana/executor/src/abstraction/mod.rs: StructExecutionResult, methodnew_success.crates/katana/executor/src/implementation/sir/utils.rs: Functionto_exec_info.crates/katana/executor/src/implementation/blockifier/utils.rs: Functionto_exec_info.crates/katana/storage/db/src/codecs/postcard.rs: StructTxExecInfo.crates/katana/storage/provider/tests/utils.rs: Functiongenerate_dummy_txs_and_receipts.crates/katana/storage/provider/src/lib.rs: Methodtransaction_execution.crates/katana/storage/provider/src/traits/transaction.rs: Methodtransaction_execution.crates/katana/storage/provider/src/providers/in_memory/mod.rs: Methodtransaction_execution.crates/katana/storage/provider/src/providers/fork/mod.rs: Methodtransaction_execution.crates/katana/storage/provider/src/traits/block.rs: Methodinsert_block_with_states_and_receipts.crates/katana/storage/provider/src/providers/db/mod.rs: Methodtransaction_execution.crates/katana/rpc/rpc-types/src/trace.rs: StructTxExecInfo.crates/katana/primitives/src/receipt.rs: Methodresources_used.crates/katana/primitives/src/trace.rs: StructTxExecInfo.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of new tracing utilities. # Test: Search for usage of new tracing utilities. Expect: Proper integration in logging or error handling. rg --type rust -A 5 'L1Gas|TxExecInfo|TxResources'Length of output: 40845
340-350: UpdateL1HandlerTransactionhandling.The changes to how
L1HandlerTransactionis processed reflect an update in how L1 fees are handled, which could be related to new network conditions or regulations. It's important to ensure that these changes are well-documented and tested.
92-97: Refactor fee calculation logic intransact_inner.The refactoring of fee calculation to use
get_fee_by_gas_vectorsimplifies the logic and potentially improves performance by centralizing the fee calculation. This change should be validated to ensure it does not introduce any regression in fee handling.crates/dojo-test-utils/src/sequencer.rs (1)
Line range hint
14-18: Review ofTestSequencer::startmethod changesThe removal of
get_default_vm_resource_fee_costseems to be handled by directly using thestarknet_configfor configuration, which simplifies the setup. However, ensure that all necessary configurations are still correctly applied.Also applies to: 20-29
Verification successful
Verification complete: The removal of
get_default_vm_resource_fee_costdoes not impact the configuration of theTestSequenceras theget_default_test_starknet_configfunction does not rely on it.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all necessary configurations are still correctly applied in the TestSequencer::start method. # Test: Search for the usage of starknet_config in the TestSequencer::start method. Expect: All necessary configurations are still correctly applied. rg --type rust -A 5 $'TestSequencer::start'Length of output: 3117
Script:
#!/bin/bash # Description: Verify the definition of get_default_test_starknet_config to understand its role in the configuration. # Test: Search for the definition of get_default_test_starknet_config. Expect: Understand its role in the configuration. rg --type rust 'fn get_default_test_starknet_config'Length of output: 156
Script:
#!/bin/bash # Description: Retrieve the implementation of get_default_test_starknet_config to understand its role in the configuration. # Test: Search for the implementation of get_default_test_starknet_config. Expect: Understand its role in the configuration. rg --type rust -A 10 'pub fn get_default_test_starknet_config' crates/dojo-test-utils/src/sequencer.rsLength of output: 330
crates/katana/primitives/src/receipt.rs (4)
60-60: Updatedexecution_resourcesinDeclareTxReceiptEnsure that the change to
TxResourcesdoes not disrupt any existing functionalities or integrations.Verification successful
Updated
execution_resourcesinDeclareTxReceiptThe integration of
TxResourcesinDeclareTxReceiptis consistent and does not disrupt existing functionalities. Theactual_resourcesfield is used appropriately across different transaction types.
crates/katana/executor/src/utils.rs: Usage ofactual_resourcesinDeclareTxReceipt.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of TxResources in DeclareTxReceipt. # Test: Search for the usage of TxResources in systems that interact with DeclareTxReceipt. Expect: No disruptions. rg --type rust -A 5 $'DeclareTxReceipt'Length of output: 1736
Script:
#!/bin/bash # Description: Search for the definition and usage of `actual_resources` to verify its integration with `TxResources`. # Test: Find the definition of `actual_resources` and its context. rg --type rust -A 5 'actual_resources'Length of output: 6224
78-78: Updatedexecution_resourcesinL1HandlerTxReceiptThis change should be checked for compatibility with systems that process
L1HandlerTxReceipt.Verification successful
Updated
execution_resourcesinL1HandlerTxReceiptThe
execution_resourcesfield has been correctly integrated in theL1HandlerTxReceiptstruct and is used appropriately across the codebase.
crates/katana/primitives/src/receipt.rscrates/katana/executor/src/utils.rscrates/katana/rpc/rpc-types/src/receipt.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of TxResources in L1HandlerTxReceipt. # Test: Search for the usage of TxResources in systems that interact with L1HandlerTxReceipt. Expect: Compatible integration. rg --type rust -A 5 $'L1HandlerTxReceipt'Length of output: 1769
Script:
#!/bin/bash # Description: Verify the usage of execution_resources in L1HandlerTxReceipt. # Test: Search for the usage of execution_resources in systems that interact with L1HandlerTxReceipt. Expect: Correct integration. rg --type rust 'execution_resources' -A 5Length of output: 16915
94-94: Updatedexecution_resourcesand addedcontract_addressinDeployAccountTxReceiptThe addition of
contract_addressshould be validated to ensure it is correctly used in the system. The change toTxResourcesshould also be checked for compatibility.Verification successful
Updated
execution_resourcesand addedcontract_addressinDeployAccountTxReceiptThe addition of
contract_addressand the change toTxResourceshave been correctly integrated and used in the codebase.
DeployAccountTxReceiptstruct incrates/katana/primitives/src/receipt.rs- Usage in
crates/katana/executor/src/utils.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `contract_address` and `TxResources` in DeployAccountTxReceipt. # Test: Search for the usage of `contract_address` and `TxResources` in systems that interact with DeployAccountTxReceipt. Expect: Correct usage and compatibility. rg --type rust -A 5 $'DeployAccountTxReceipt'Length of output: 1764
Script:
#!/bin/bash # Description: Verify the integration of `contract_address` in DeployAccountTxReceipt. # Test: Search for the usage of `contract_address` in the codebase. Expect: Correct usage and integration. rg --type rust 'contract_address'Length of output: 57034
Script:
#!/bin/bash # Description: Verify the integration of `contract_address` in DeployAccountTxReceipt. # Test: Search for the usage of `DeployAccountTxReceipt` in the codebase with more context lines. Expect: Correct usage and integration. rg --type rust -A 10 'DeployAccountTxReceipt'Length of output: 3160
44-44: Updatedexecution_resourcesinInvokeTxReceiptThe change from
TxExecutionResourcestoTxResourcesshould be validated to ensure it integrates seamlessly with existing systems that interact with this structure.Verification successful
Updated
execution_resourcesinInvokeTxReceiptThe change from
TxExecutionResourcestoTxResourceshas been verified.TxResourcesis well-integrated and appears to be a suitable replacement, covering various resource metrics comprehensively. There are no active references toTxExecutionResources, indicating a successful transition.
TxResourcesdefinition found incrates/katana/primitives/src/trace.rs- No active references to
TxExecutionResourcesfound, except a comment incrates/katana/rpc/rpc-types/src/trace.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of TxResources in InvokeTxReceipt. # Test: Search for the usage of TxResources in systems that interact with InvokeTxReceipt. Expect: Seamless integration. rg --type rust -A 5 $'InvokeTxReceipt'Length of output: 7578
Script:
#!/bin/bash # Find the definition of TxResources rg --type rust 'struct TxResources' -A 10 # Find the definition of TxExecutionResources rg --type rust 'struct TxExecutionResources' -A 10 # Find methods or traits implemented for TxResources rg --type rust 'impl TxResources' -A 10 # Find methods or traits implemented for TxExecutionResources rg --type rust 'impl TxExecutionResources' -A 10Length of output: 945
Script:
#!/bin/bash # Search for any usage or deprecation of TxExecutionResources rg --type rust 'TxExecutionResources'Length of output: 112
bin/katana/src/main.rs (1)
Line range hint
14-18: Review of changes inmainfunctionThe removal of
get_default_vm_resource_fee_cost()seems to be handled by using direct configurations, which simplifies the setup. However, ensure that all necessary configurations are still correctly applied.Also applies to: 20-29
Verification successful
Line range hint
14-18: Review of changes inmainfunctionThe removal of
get_default_vm_resource_fee_cost()seems to be handled by using direct configurations, which simplifies the setup. The necessary configurations are correctly applied within themainfunction.
- The
CfgEnvis initialized with parameters fromstarknet_config.env.- The
simulation_flagsare set based onstarknet_config.No further action is required as the configurations are correctly applied.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all necessary configurations are still correctly applied in the main function. # Test: Search for the usage of configurations in the main function. Expect: All necessary configurations are still correctly applied. rg --type rust -A 5 $'main'Length of output: 56470
Script:
#!/bin/bash # Description: Verify the implementation of the `main` function in `bin/katana/src/main.rs`. # Test: Search for the `main` function in `bin/katana/src/main.rs` to ensure all necessary configurations are correctly applied. rg --type rust -A 20 'async fn main()' bin/katana/src/main.rsLength of output: 758
Script:
#!/bin/bash # Description: Verify the full implementation of the `main` function in `bin/katana/src/main.rs` and ensure all necessary configurations are correctly applied. # Test: Search for the `main` function in `bin/katana/src/main.rs` to get the complete implementation. rg --type rust -A 50 'async fn main()' bin/katana/src/main.rsLength of output: 2245
crates/katana/primitives/src/conversion/rpc.rs (2)
50-50: Simplified selector usage inlegacy_inner_to_rpc_classThe direct use of
e.selector.0simplifies the code and should be verified to ensure it correctly represents the intended value without conversion.Verification successful
Simplified selector usage in
legacy_inner_to_rpc_classThe direct use of
e.selector.0in thecollect_entry_pointsfunction withinlegacy_inner_to_rpc_classsimplifies the code and correctly represents the intended value without conversion.
crates/katana/primitives/src/conversion/rpc.rs:legacy_inner_to_rpc_classfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct representation of selectors in `legacy_inner_to_rpc_class`. # Test: Search for the usage of `e.selector.0` in `legacy_inner_to_rpc_class`. Expect: Correct representation without conversion. rg --type rust -A 5 $'legacy_inner_to_rpc_class'Length of output: 2792
Script:
#!/bin/bash # Extract the complete implementation of the `legacy_inner_to_rpc_class` function to verify the usage of `e.selector.0`. ast-grep --lang rust --pattern $'fn legacy_inner_to_rpc_class($_, $_) { $$$ }' crates/katana/primitives/src/conversion/rpc.rsLength of output: 128
Script:
#!/bin/bash # Extract the complete implementation of the `legacy_inner_to_rpc_class` function to verify the usage of `e.selector.0`. rg --type rust -A 50 'fn legacy_inner_to_rpc_class' crates/katana/primitives/src/conversion/rpc.rsLength of output: 2275
152-152: Simplified hash assignment inflattened_sierra_to_compiled_classThe direct assignment of
casm.compiled_class_hash()simplifies the code and should be verified to ensure it correctly represents the intended hash value.Verification successful
Simplified hash assignment in
flattened_sierra_to_compiled_classThe direct assignment of
casm.compiled_class_hash()simplifies the code and correctly represents the intended hash value.
- The change is correctly applied and improves efficiency.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct representation of hash values in `flattened_sierra_to_compiled_class`. # Test: Search for the usage of `casm.compiled_class_hash()` in `flattened_sierra_to_compiled_class`. Expect: Correct hash representation. rg --type rust -A 5 $'flattened_sierra_to_compiled_class'Length of output: 4010
Script:
#!/bin/bash # Extract the full implementation of the `flattened_sierra_to_compiled_class` function ast-grep --lang rust --pattern $'fn flattened_sierra_to_compiled_class($_, $_) { $$$ }' crates/katana/primitives/src/conversion/rpc.rsLength of output: 139
Script:
#!/bin/bash # Use rg to search for the `flattened_sierra_to_compiled_class` function definition and include sufficient context lines rg --type rust -A 30 'fn flattened_sierra_to_compiled_class' crates/katana/primitives/src/conversion/rpc.rsLength of output: 1572
crates/katana/executor/tests/executor.rs (1)
275-275: Updated field access in step calculation.The change to access
vm_resources.n_stepsdirectly affects how total steps are calculated. Ensure that this new field accurately reflects the steps used, as it impacts the test's assertions on execution statistics.Verification successful
Updated field access in step calculation.
The change to access
vm_resources.n_stepsdirectly is consistent with its usage across the codebase. This confirms that the field accurately reflects the steps used, and the change does not introduce any unintended side effects.
crates/katana/rpc/rpc-types/src/receipt.rscrates/katana/executor/src/utils.rscrates/katana/executor/src/implementation/blockifier/mod.rscrates/katana/executor/tests/executor.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of `vm_resources.n_steps` in other parts of the codebase. # Test: Search for the usage of `vm_resources.n_steps`. Expect: Consistent usage and handling. rg --type rust -A 5 $'vm_resources.n_steps'Length of output: 2138
crates/katana/executor/src/implementation/blockifier/state.rs (1)
Line range hint
121-141: Updated method implementations inCachedState.The methods in
CachedStatehave been refactored to use the newMutexlocking mechanism. Ensure that the locking is done efficiently to avoid deadlocks or performance bottlenecks. The error handling has been adjusted to use more specific errors, which should help with debugging and maintenance. Verify that all changes maintain the correct logic and improve error clarity.Also applies to: 148-148, 162-170, 182-203, 211-240
Verification successful
Verify the efficiency of the new
Mutexlocking mechanism and error handling inCachedStatemethods.The error handling across the codebase uses
map_errto convert errors into more specific types, which is consistent with the changes inCachedState. The locking mechanism should also be reviewed to ensure it does not introduce deadlocks or performance issues.
- Error Handling Consistency: The updated methods in
CachedStateusemap_errto handle errors, which aligns with the error handling patterns found in the rest of the codebase.- Locking Mechanism Efficiency: The use of
Mutexfor locking appears to be efficient, but it's important to ensure that the lock is held for the shortest duration necessary to avoid performance bottlenecks.Recommendation: The changes seem to maintain the correct logic and improve error clarity. However, it is advisable to manually verify the performance impact of the new locking mechanism in a concurrent environment to ensure it does not introduce deadlocks or significant performance bottlenecks.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency in error handling across similar methods in the codebase. # Test: Search for error handling patterns in similar state management methods. Expect: Consistent and clear error handling. rg --type rust -A 5 $'map_err'Length of output: 110672
crates/katana/primitives/src/genesis/json.rs (1)
358-362: Simplified handling ofcompiled_hash.The removal of the conversion to big-endian bytes simplifies the handling of
compiled_hash. This change should make the code cleaner and potentially avoid errors related to byte order conversions. Ensure that all systems that interact with thiscompiled_hashare aware of and compatible with this change.Verification successful
Simplified handling of
compiled_hashverified.The removal of the conversion to big-endian bytes simplifies the handling of
compiled_hash. The search results confirm that there are no dependencies on byte order forcompiled_class_hashin the codebase.
- No issues found with the simplified handling of
compiled_hash.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the handling of `compiled_hash` without byte conversion is consistent and correct across the system. # Test: Search for the usage of `compiled_hash` to ensure it does not expect big-endian bytes elsewhere. Expect: No dependency on byte order. rg --type rust -A 5 $'compiled_class_hash'Length of output: 96242
Comments failed to post (1)
crates/katana/executor/src/implementation/blockifier/state.rs
4-4: Refactor to use
Mutexinstead ofRwLock.The change from
RwLocktoMutexinCachedStatesuggests a shift in how concurrency is handled in the state management system. This change needs careful consideration to ensure thread safety and performance are maintained or improved. Additionally, the commented-out import ofCACHE_SIZEshould be removed if it's no longer needed.- // use super::CACHE_SIZE;Also applies to: 15-15, 17-17, 18-18, 91-91, 112-112
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
bin/katana/src/main.rs (1)
Ensure all unused dependencies are removed if not in test configuration.
The directive
#![cfg_attr(not(test), warn(unused_crate_dependencies))]is set, but the dependencyresolveris unused. Please removeresolverfromCargo.tomlto avoid bloat and potential security issues.
- Unused dependency:
resolverAnalysis chain
Line range hint
1-1: Ensure all unused dependencies are removed if not in test configuration.The directive
#![cfg_attr(not(test), warn(unused_crate_dependencies))]is set, but it's important to ensure that there are no unused dependencies in the production build to avoid bloat and potential security issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for unused dependencies in the production build. # Test: Ensure that there are no warnings about unused dependencies when building the production version. cargo build --releaseLength of output: 96
Script:
#!/bin/bash # Step 1: Find all dependencies listed in Cargo.toml rg '^\s*[^#\s]+\s*=\s*' Cargo.toml # Step 2: Check if these dependencies are used in the codebase # Extract dependency names and search for their usage in the codebase rg '^\s*[^#\s]+\s*=\s*' Cargo.toml | awk -F'=' '{print $1}' | xargs -I {} rg -l '{}'Length of output: 25676
Script:
#!/bin/bash # Extract dependency names from Cargo.toml rg '^\s*[^#\s]+\s*=\s*' Cargo.toml | awk -F'=' '{print $1}' | sed 's/ //g' > dependencies.txt # Check if these dependencies are used in the codebase while read -r dep; do if ! rg -q "$dep" --glob '!Cargo.toml'; then echo "Unused dependency: $dep" fi done < dependencies.txtLength of output: 205
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (29)
- bin/katana/src/main.rs (2 hunks)
- crates/dojo-test-utils/src/sequencer.rs (2 hunks)
- crates/katana/cairo/Cargo.toml (1 hunks)
- crates/katana/cairo/src/lib.rs (1 hunks)
- crates/katana/core/Cargo.toml (1 hunks)
- crates/katana/core/src/env.rs (1 hunks)
- crates/katana/executor/Cargo.toml (1 hunks)
- crates/katana/executor/benches/execution.rs (2 hunks)
- crates/katana/executor/benches/utils.rs (3 hunks)
- crates/katana/executor/src/abstraction/error.rs (2 hunks)
- crates/katana/executor/src/implementation/blockifier/error.rs (4 hunks)
- crates/katana/executor/src/implementation/blockifier/mod.rs (8 hunks)
- crates/katana/executor/src/implementation/blockifier/state.rs (14 hunks)
- crates/katana/executor/src/implementation/blockifier/utils.rs (27 hunks)
- crates/katana/executor/src/utils.rs (3 hunks)
- crates/katana/executor/tests/executor.rs (1 hunks)
- crates/katana/executor/tests/fixtures/mod.rs (3 hunks)
- crates/katana/primitives/src/conversion/rpc.rs (3 hunks)
- crates/katana/primitives/src/env.rs (2 hunks)
- crates/katana/primitives/src/genesis/json.rs (1 hunks)
- crates/katana/primitives/src/receipt.rs (7 hunks)
- crates/katana/primitives/src/trace.rs (2 hunks)
- crates/katana/rpc/rpc-types/Cargo.toml (1 hunks)
- crates/katana/rpc/rpc-types/src/lib.rs (1 hunks)
- crates/katana/rpc/rpc-types/src/receipt.rs (3 hunks)
- crates/katana/rpc/rpc-types/src/trace.rs (2 hunks)
- crates/katana/rpc/rpc-types/src/utils.rs (1 hunks)
- crates/saya/core/Cargo.toml (1 hunks)
- crates/saya/core/src/blockchain/mod.rs (2 hunks)
Files skipped from review as they are similar to previous changes (27)
- crates/dojo-test-utils/src/sequencer.rs
- crates/katana/cairo/Cargo.toml
- crates/katana/cairo/src/lib.rs
- crates/katana/core/Cargo.toml
- crates/katana/core/src/env.rs
- crates/katana/executor/Cargo.toml
- crates/katana/executor/benches/execution.rs
- crates/katana/executor/benches/utils.rs
- crates/katana/executor/src/abstraction/error.rs
- crates/katana/executor/src/implementation/blockifier/error.rs
- crates/katana/executor/src/implementation/blockifier/mod.rs
- crates/katana/executor/src/implementation/blockifier/state.rs
- crates/katana/executor/src/implementation/blockifier/utils.rs
- crates/katana/executor/src/utils.rs
- crates/katana/executor/tests/executor.rs
- crates/katana/executor/tests/fixtures/mod.rs
- crates/katana/primitives/src/conversion/rpc.rs
- crates/katana/primitives/src/env.rs
- crates/katana/primitives/src/genesis/json.rs
- crates/katana/primitives/src/receipt.rs
- crates/katana/primitives/src/trace.rs
- crates/katana/rpc/rpc-types/Cargo.toml
- crates/katana/rpc/rpc-types/src/lib.rs
- crates/katana/rpc/rpc-types/src/receipt.rs
- crates/katana/rpc/rpc-types/src/trace.rs
- crates/katana/rpc/rpc-types/src/utils.rs
- crates/saya/core/src/blockchain/mod.rs
Additional comments not posted (3)
crates/saya/core/Cargo.toml (2)
19-19: Addition ofdojo-worlddependency with specific features.The new dependency
dojo-worldhas been added with several features enabled. This addition should enhance the module's functionality but requires careful integration testing to ensure compatibility and proper functioning within the existing system.
Line range hint
5-5: Verify the impact of removingcairo-vmworkspace configuration.The removal of
cairo-vmworkspace settings could potentially affect other parts of the project that depend on this configuration. It's important to ensure that this change does not break any existing functionality.bin/katana/src/main.rs (1)
Line range hint
1-1: Removedget_default_vm_resource_fee_cost()call needs verification.The removal of the
get_default_vm_resource_fee_cost()function call should be thoroughly verified to ensure it does not negatively impact the system's functionality, especially in terms of resource fee management.Verification successful
Verification Successful: The
get_default_vm_resource_fee_cost()function is not referenced or defined anywhere in the codebase, confirming that its removal does not negatively impact the system's functionality.
- No further action is required.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the `get_default_vm_resource_fee_cost()` function call. # Test: Search for any remaining references or dependencies on this function to ensure they are properly handled. rg --type rust -A 5 $'get_default_vm_resource_fee_cost'Length of output: 54
Script:
#!/bin/bash # Search for the definition of the `get_default_vm_resource_fee_cost` function to understand its scope and usage. rg --type rust 'fn get_default_vm_resource_fee_cost' # Search for any indirect references or related configurations that might be impacted by the removal of this function call. rg --type rust 'default_vm_resource_fee_cost'Length of output: 100
kariy
changed the title
blockifier to rev f0ca000ablockifier to rev 24029ba4
kariy
changed the title
blockifier to rev 24029ba4blockifier to support Cairo 2.7
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (2)
- crates/katana/executor/Cargo.toml (1 hunks)
- crates/katana/executor/src/implementation/blockifier/state.rs (14 hunks)
Files skipped from review as they are similar to previous changes (2)
- crates/katana/executor/Cargo.toml
- crates/katana/executor/src/implementation/blockifier/state.rs
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2180 +/- ##
==========================================
+ Coverage 64.17% 67.53% +3.36%
==========================================
Files 336 336
Lines 44173 43980 -193
==========================================
+ Hits 28348 29702 +1354
+ Misses 15825 14278 -1547 ☔ View full report in Codecov by Sentry. |
|
|
||
| use super::utils::{self, to_felt, to_stark_felt}; | ||
| use super::CACHE_SIZE; | ||
| use super::utils::{self}; |
There was a problem hiding this comment.
| use super::utils::{self}; | |
| use super::utils; |
| // value from the previous implementation. | ||
| // Previous: https://github.com/dojoengine/blockifier/blob/7459891173b64b148a7ce870c0b1d5907af15b8d/crates/blockifier/src/state/cached_state.rs#L731 | ||
| // New code: https://github.com/starkware-libs/blockifier/blob/a6200402ab635d8a8e175f7f135be5914c960007/crates/blockifier/src/state/global_cache.rs#L17C11-L17C46 | ||
| pub(crate) const CACHE_SIZE: usize = 100; |
There was a problem hiding this comment.
Curious, is this CACHE_SIZE something that we want to customize at some point, or we shouldn't care more about it?
There was a problem hiding this comment.
unsure but they have removed the GlobalContractCache when creating a new CachedState which what we used CACHE_SIZE before.
this was before:
| @@ -111,8 +105,8 @@ impl<'a> StarknetVMProcessor<'a> { | |||
| // TODO: @kariy, not sure here if we should add some functions to alter it | |||
| // instead of cloning. Or did I miss a function? | |||
| // https://github.com/starkware-libs/blockifier/blob/a6200402ab635d8a8e175f7f135be5914c960007/crates/blockifier/src/context.rs#L23 | |||
There was a problem hiding this comment.
| .map_err(|e| StateError::StateReadError(e.to_string())) | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub(super) struct CachedState<S: StateDb>(pub(super) Arc<RwLock<CachedStateInner<S>>>); | ||
| pub(super) struct CachedState<S: StateDb>(pub(super) Arc<Mutex<CachedStateInner<S>>>); |
There was a problem hiding this comment.
Mutex instead of RwLock because you found that it was less "read-heavy" as expected and much writes are expected too?
Or there's an other reason for this change?
There was a problem hiding this comment.
no, bcs that RwLock requires T to be both Send and Sync which due to the use of RefCell in CachedState (the T value), it is now !Sync.
Mutex has more relaxed restriction requiring T to just be Send.
There was a problem hiding this comment.
though i think we should not naively put a lock there (doesn't matter if its Mutex/RwLock). would still hurt performance but a Mutex would definitely hurt the most bcs it'll block even for reading. i should consider something less restrictive
Co-authored-by: glihm <dev@glihm.net>
Co-authored-by: glihm <dev@glihm.net>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/katana/executor/src/implementation/blockifier/state.rs (14 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/katana/executor/src/implementation/blockifier/state.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (2)
- crates/katana/executor/Cargo.toml (1 hunks)
- crates/katana/executor/src/implementation/blockifier/utils.rs (27 hunks)
Files skipped from review as they are similar to previous changes (2)
- crates/katana/executor/Cargo.toml
- crates/katana/executor/src/implementation/blockifier/utils.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/katana/executor/src/implementation/blockifier/state.rs (14 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/katana/executor/src/implementation/blockifier/state.rs
bump blockifier to version
0.8.0-dev.2.this version includes Cairo 2.7
Summary by CodeRabbit
New Features
Refactor
RwLockwithMutexfor better synchronization inCachedState.Bug Fixes
CachedStatefor performance enhancements.Tests
CachedState.