feat: bump Cairo 2.9.1 #2793
feat: bump Cairo 2.9.1 #2793
Conversation
WalkthroughOhayo, sensei! This pull request introduces significant updates to the CI/CD pipeline and dependency management for a Rust project. Key changes include upgrading the Changes
Possibly related PRs
Suggested reviewers
🪧 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (22)
crates/dojo/core-cairo-test/src/world.cairo (2)
164-176: Clean resource registration implementation, sensei!The restructured match arms for resource registration are well-organized and properly handle all TestResource variants. However, consider adding error handling for the
try_intoconversions.Consider wrapping the conversions in Result handling:
match r { TestResource::Event(ch) => { - world.register_event(namespace.clone(), (*ch).try_into().unwrap()); + let class_hash = (*ch).try_into().map_err(|_| 'Invalid class hash')?; + world.register_event(namespace.clone(), class_hash); }, // Similar changes for Model and Contract variants }
Line range hint
190-218: Clean permission and initialization handling, sensei!The formatting improvements in pattern matching enhance readability. However, consider splitting the permission syncing and initialization logic into separate private functions for better maintainability.
Consider refactoring into separate functions:
fn sync_permissions(self: @WorldStorage, contracts: Span<ContractDef>) { // Permission syncing logic } fn init_contracts(self: @WorldStorage, contracts: Span<ContractDef>) { // Contract initialization logic }crates/dojo/core-cairo-test/src/tests/contract.cairo (3)
Line range hint
13-16: Missingdojo_nameImplementation incontract_invalid_upgradeThe module
contract_invalid_upgradeis expected to implement thedojo_namemethod as per theIDeployedResourcetrait. Without this method, the contract may not integrate properly with systems relying on it.Please implement the
dojo_namemethod to ensure compatibility.+ #[abi(embed_v0)] + pub impl ContractImpl of dojo::meta::interface::IDeployedResource<ContractState> { + fn dojo_name(self: @ContractState) -> ByteArray { + "contract_invalid_upgrade" + } + }
Line range hint
29-31: Ensure Correct Trait Implementation intest_contract_upgradeThe
ContractImplintest_contract_upgradeshould implementIContractfromdojo::contract, notdojo::meta::interface::IDeployedResource.Adjust the trait implementation to the correct interface.
- #[abi(embed_v0)] - pub impl ContractImpl of dojo::meta::interface::IDeployedResource<ContractState> { + #[abi(embed_v0)] + pub impl ContractImpl of dojo::contract::IContract<ContractState> {
Line range hint
52-54: Unnecessary Trailing Comma in AttributeIn the
#[should_panic]attribute of thetest_register_namespace_empty_namefunction, there's an unnecessary trailing comma after the expected message tuple.Remove the trailing comma for cleaner code.
#[test] #[available_gas(6000000)] #[should_panic( expected: ( "Namespace `` is invalid according to Dojo naming rules: ^[a-zA-Z0-9_]+$", 'ENTRYPOINT_FAILED', - ), + ) )] fn test_register_namespace_empty_name() {crates/katana/pool/src/validation/stateful.rs (1)
169-173: Simplify Execution Flags InitializationThe initialization of
flagscan be simplified by directly passing the negatedskip_validateandskip_fee_checkvariables.Consider simplifying the code for better readability.
let flags = - ExecutionFlags::new().with_account_validation(!skip_validate).with_fee(!skip_fee_check); + ExecutionFlags::new() + .with_account_validation(!skip_validate) + .with_fee(!skip_fee_check);crates/katana/executor/src/implementation/blockifier/mod.rs (3)
104-110: Use Consistent Gas Price InitializationThere's an inconsistency in how
NonzeroGasPriceis initialized foreth_l1_gas_priceandstrk_l1_gas_price. Ensure consistent initialization for clarity.Unify the initialization pattern for better readability.
let eth_l1_gas_price = NonzeroGasPrice::new(header.l1_gas_prices.eth.into()).unwrap_or(NonzeroGasPrice::MIN); let strk_l1_gas_price = - NonzeroGasPrice::new(header.l1_gas_prices.strk.into()).unwrap_or(NonzeroGasPrice::MIN); + NonzeroGasPrice::new(header.l1_gas_prices.strk.into()) + .unwrap_or(NonzeroGasPrice::MIN);
119-130: Update TODO Comments with Action ItemsThe TODO comments in the
GasPricesinitialization suggest updates are needed. Adding clear action items or opening issues can help track these updates.Would you like me to open a GitHub issue to address the TODOs and ensure the correct gas price values are used?
240-266: Simplify Access to Gas Prices inblock_envThe access to gas prices in the
block_envmethod is verbose. Consider simplifying or creating helper functions for cleaner code.Streamline the code for better maintainability.
l1_gas_prices: KatanaGasPrices { - eth: self.block_context.block_info().gas_prices.eth_gas_prices.l1_gas_price.get().0, + eth: self.block_context.block_info().gas_prices.eth_gas_prices.l1_gas_price.into(), strk: self.block_context.block_info().gas_prices.strk_gas_prices.l1_gas_price.into(), }, l1_data_gas_prices: KatanaGasPrices { - eth: self.block_context.block_info().gas_prices.eth_gas_prices.l1_data_gas_price.get().0, + eth: self.block_context.block_info().gas_prices.eth_gas_prices.l1_data_gas_price.into(), strk: self.block_context.block_info().gas_prices.strk_gas_prices.l1_data_gas_price.into(), },bin/sozo/src/commands/test.rs (2)
160-160: Avoid Unnecessary Cloning ofbuild_external_contractsCloning
props.build_external_contractsmay be unnecessary. You can borrow the data instead, which can improve performance by reducing memory allocations.Modify the code as follows:
-if let Some(external_contracts) = props.build_external_contracts.clone() { +if let Some(ref external_contracts) = props.build_external_contracts {This way,
external_contractsis borrowed rather than cloned.
296-324: Optimize Dependencies Collection incrates_config_for_compilation_unitThe current method of collecting dependencies involves nested iteration and searching, which may impact performance. Consider indexing components by their IDs to streamline access and reduce complexity.
crates/katana/executor/src/implementation/blockifier/utils.rs (2)
525-530: Extendto_api_resource_boundsto Support All Resource BoundsThe function
to_api_resource_boundscurrently supports onlyValidResourceBounds::L1Gas. To ensure compatibility with newer protocol versions, consider implementing support for additional variants.Update the function to handle other resource bounds as necessary.
562-575: Ohayo sensei! Enhance Error Handling into_runnable_classFunctionIn
to_runnable_class, improve error handling by providing explicit error messages when conversions fail. This will aid in debugging and make the code more robust.Adjust the code as follows to better handle errors:
match class { class::CompiledClass::Legacy(class) => { - Ok(RunnableCompiledClass::V0(CompiledClassV0::try_from(class)?)) + let compiled_class = CompiledClassV0::try_from(class) + .map_err(|e| ProgramError::Other(format!("Failed to convert Legacy class: {}", e)))?; + Ok(RunnableCompiledClass::V0(compiled_class)) } class::CompiledClass::Class(casm) => { - Ok(RunnableCompiledClass::V1(CompiledClassV1::try_from(casm)?)) + let compiled_class = CompiledClassV1::try_from(casm) + .map_err(|e| ProgramError::Other(format!("Failed to convert Class: {}", e)))?; + Ok(RunnableCompiledClass::V1(compiled_class)) } }examples/simple/Scarb.toml (1)
2-2: Sensei, consider pinning the starknet version exactly.While the versions are correctly updated to 2.9.1, the starknet dependency isn't using the exact version pin ("=") like in the core package. Consider making it consistent:
-starknet = "2.9.1" +starknet = "=2.9.1"Also applies to: 14-14
crates/dojo/core/src/contract/components/world_provider.cairo (1)
22-22: Ohayo! Type parameter changes align with the framework's evolution, sensei!The explicit
+HasComponent<TContractState>bound enhances type safety and makes the component requirements clearer. This change follows the same pattern as the event component updates, showing consistent application of Cairo 2.9.1's type system improvements across the codebase.Consider documenting these type parameter requirements in the module-level documentation to help other developers understand the component's constraints and requirements better.
Also applies to: 31-31
crates/katana/executor/Cargo.toml (1)
17-17: Consider using a stable tag instead of a commit hash, senseiWhile pinning to a specific commit (
92da150d) provides exact version control, it might make the build less reproducible if the commit becomes unavailable. Consider using a stable tag instead.crates/dojo/lang/src/inline_macros/spawn_test_world.rs (1)
101-101: Ohayo! Consistent implementation with potential for test diagnosticsThe addition of
diagnostics_note: Nonealigns with other macro implementations. Given this is a test world spawning macro, this field could be valuable for future test-specific diagnostics.Consider utilizing
diagnostics_notein the future for providing additional test setup information or validation messages that could help with debugging test world initialization issues..github/workflows/ci.yml (2)
Line range hint
192-206: Consider pinning hurl version in environment variablesSensei, consider moving the hurl version to the environment variables section at the top of the file, similar to how RUST_VERSION is handled. This would make future updates easier to manage.
env: CARGO_TERM_COLOR: always RUST_VERSION: 1.80.0 + HURL_VERSION: 3.0.0 ... - curl --location --remote-name https://github.com/Orange-OpenSource/hurl/releases/download/3.0.0/hurl_3.0.0_amd64.deb - sudo apt update && sudo apt install ./hurl_3.0.0_amd64.deb + curl --location --remote-name https://github.com/Orange-OpenSource/hurl/releases/download/${HURL_VERSION}/hurl_${HURL_VERSION}_amd64.deb + sudo apt update && sudo apt install ./hurl_${HURL_VERSION}_amd64.deb🧰 Tools
🪛 actionlint (1.7.4)
112-112: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Line range hint
204-206: Consider adding timeout for katana startupSensei, the current implementation might proceed with tests before katana is fully ready. Consider adding a health check.
chmod +x /tmp/bins/katana chmod +x /tmp/bins/sozo nohup /tmp/bins/katana --dev --dev.accounts 2 --dev.no-fee & + # Wait for katana to be ready (max 30 seconds) + for i in {1..30}; do + if curl -s http://localhost:5050 >/dev/null; then + break + fi + sleep 1 + if [ $i -eq 30 ]; then + echo "Timeout waiting for katana to start" + exit 1 + fi + done🧰 Tools
🪛 actionlint (1.7.4)
112-112: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
crates/dojo/lang/src/attribute_macros/event.rs (1)
161-161: Ohayo! The diagnostics enhancement looks promising, sensei!The addition of
diagnostics_notefield aligns with the codebase-wide enhancement of diagnostic capabilities.Consider implementing meaningful diagnostic notes in a future PR, particularly for common Cairo 2.9.1 compatibility issues.
crates/katana/executor/src/implementation/blockifier/state.rs (1)
381-386: Consider extracting common test setup logic.The repeated pattern of converting classes using
to_runnable_classcould be extracted into a test helper function to improve maintainability.fn to_runnable_test_class(class: CompiledClass) -> RunnableCompiledClass { utils::to_runnable_class(class).unwrap() }crates/katana/rpc/rpc/tests/starknet.rs (1)
Line range hint
929-950: Consider adding error path testing, sensei.While the happy path test is good, the removal of error handling reduces test coverage. Consider adding separate test cases for error scenarios to maintain comprehensive coverage.
#[tokio::test] async fn v3_transactions_error_cases() { // Test insufficient gas // Test invalid calldata // Test other error scenarios }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (9)
Cargo.lockis excluded by!**/*.lockcrates/benches/contracts/Scarb.lockis excluded by!**/*.lockcrates/dojo/core-cairo-test/Scarb.lockis excluded by!**/*.lockcrates/dojo/core/Scarb.lockis excluded by!**/*.lockcrates/dojo/lang/Scarb.lockis excluded by!**/*.lockcrates/torii/types-test/Scarb.lockis excluded by!**/*.lockexamples/game-lib/Scarb.lockis excluded by!**/*.lockexamples/simple/Scarb.lockis excluded by!**/*.lockexamples/spawn-and-move/Scarb.lockis excluded by!**/*.lock
📒 Files selected for processing (92)
.github/workflows/ci.yml(1 hunks).tool-versions(1 hunks)Cargo.toml(4 hunks)bin/sozo/benches/spawn-and-move-build.rs(1 hunks)bin/sozo/src/commands/build.rs(1 hunks)bin/sozo/src/commands/test.rs(6 hunks)crates/benches/contracts/Scarb.toml(2 hunks)crates/dojo/core-cairo-test/Scarb.toml(1 hunks)crates/dojo/core-cairo-test/src/lib.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/benchmarks.cairo(12 hunks)crates/dojo/core-cairo-test/src/tests/contract.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/event/event.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/helpers/event.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/helpers/helpers.cairo(6 hunks)crates/dojo/core-cairo-test/src/tests/helpers/model.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/meta/introspect.cairo(6 hunks)crates/dojo/core-cairo-test/src/tests/model/model.cairo(2 hunks)crates/dojo/core-cairo-test/src/tests/storage/packing.cairo(6 hunks)crates/dojo/core-cairo-test/src/tests/utils/hash.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/utils/key.cairo(1 hunks)crates/dojo/core-cairo-test/src/tests/utils/layout.cairo(4 hunks)crates/dojo/core-cairo-test/src/tests/utils/misc.cairo(2 hunks)crates/dojo/core-cairo-test/src/tests/world/acl.cairo(9 hunks)crates/dojo/core-cairo-test/src/tests/world/contract.cairo(7 hunks)crates/dojo/core-cairo-test/src/tests/world/event.cairo(16 hunks)crates/dojo/core-cairo-test/src/tests/world/metadata.cairo(6 hunks)crates/dojo/core-cairo-test/src/tests/world/model.cairo(17 hunks)crates/dojo/core-cairo-test/src/tests/world/namespace.cairo(3 hunks)crates/dojo/core-cairo-test/src/tests/world/storage.cairo(2 hunks)crates/dojo/core-cairo-test/src/tests/world/world.cairo(10 hunks)crates/dojo/core-cairo-test/src/utils.cairo(1 hunks)crates/dojo/core-cairo-test/src/world.cairo(7 hunks)crates/dojo/core/Scarb.toml(1 hunks)crates/dojo/core/src/contract/components/upgradeable.cairo(2 hunks)crates/dojo/core/src/contract/components/world_provider.cairo(2 hunks)crates/dojo/core/src/event/component.cairo(2 hunks)crates/dojo/core/src/event/event.cairo(2 hunks)crates/dojo/core/src/lib.cairo(3 hunks)crates/dojo/core/src/meta/introspect.cairo(2 hunks)crates/dojo/core/src/meta/layout.cairo(2 hunks)crates/dojo/core/src/model/component.cairo(2 hunks)crates/dojo/core/src/model/definition.cairo(1 hunks)crates/dojo/core/src/model/metadata.cairo(2 hunks)crates/dojo/core/src/model/model.cairo(2 hunks)crates/dojo/core/src/model/model_value.cairo(1 hunks)crates/dojo/core/src/model/storage.cairo(4 hunks)crates/dojo/core/src/storage/database.cairo(1 hunks)crates/dojo/core/src/storage/entity_model.cairo(6 hunks)crates/dojo/core/src/storage/layout.cairo(19 hunks)crates/dojo/core/src/storage/packing.cairo(7 hunks)crates/dojo/core/src/storage/storage.cairo(11 hunks)crates/dojo/core/src/utils/layout.cairo(2 hunks)crates/dojo/core/src/utils/misc.cairo(1 hunks)crates/dojo/core/src/utils/serde.cairo(1 hunks)crates/dojo/core/src/utils/snf_test.cairo(2 hunks)crates/dojo/core/src/world/errors.cairo(1 hunks)crates/dojo/core/src/world/iworld.cairo(8 hunks)crates/dojo/core/src/world/resource.cairo(1 hunks)crates/dojo/core/src/world/storage.cairo(24 hunks)crates/dojo/core/src/world/world_contract.cairo(43 hunks)crates/dojo/lang/Scarb.toml(1 hunks)crates/dojo/lang/src/attribute_macros/contract.rs(1 hunks)crates/dojo/lang/src/attribute_macros/event.rs(1 hunks)crates/dojo/lang/src/attribute_macros/interface.rs(1 hunks)crates/dojo/lang/src/attribute_macros/model.rs(1 hunks)crates/dojo/lang/src/derive_macros/mod.rs(1 hunks)crates/dojo/lang/src/inline_macros/delete.rs(1 hunks)crates/dojo/lang/src/inline_macros/emit.rs(1 hunks)crates/dojo/lang/src/inline_macros/get.rs(1 hunks)crates/dojo/lang/src/inline_macros/get_models_test_class_hashes.rs(1 hunks)crates/dojo/lang/src/inline_macros/selector_from_tag.rs(1 hunks)crates/dojo/lang/src/inline_macros/set.rs(1 hunks)crates/dojo/lang/src/inline_macros/spawn_test_world.rs(1 hunks)crates/dojo/test-utils/src/compiler.rs(2 hunks)crates/dojo/world/abigen/src/main.rs(1 hunks)crates/katana/cairo/Cargo.toml(1 hunks)crates/katana/executor/Cargo.toml(1 hunks)crates/katana/executor/src/implementation/blockifier/error.rs(1 hunks)crates/katana/executor/src/implementation/blockifier/mod.rs(4 hunks)crates/katana/executor/src/implementation/blockifier/state.rs(5 hunks)crates/katana/executor/src/implementation/blockifier/utils.rs(21 hunks)crates/katana/pool/src/validation/stateful.rs(3 hunks)crates/katana/primitives/src/conversion/rpc.rs(2 hunks)crates/katana/rpc/rpc-types/src/class.rs(2 hunks)crates/katana/rpc/rpc/tests/starknet.rs(2 hunks)crates/torii/types-test/Scarb.toml(1 hunks)examples/simple/Scarb.toml(2 hunks)examples/simple/src/lib.cairo(4 hunks)examples/spawn-and-move/Scarb.toml(2 hunks)examples/spawn-and-move/src/actions.cairo(6 hunks)examples/spawn-and-move/src/models.cairo(3 hunks)rust-toolchain.toml(1 hunks)
✅ Files skipped from review due to trivial changes (45)
- crates/dojo/core/src/utils/misc.cairo
- crates/dojo/core-cairo-test/src/tests/event/event.cairo
- crates/dojo/core/src/storage/database.cairo
- crates/dojo/core-cairo-test/src/tests/utils/key.cairo
- crates/dojo/core/src/utils/serde.cairo
- crates/dojo/lang/Scarb.toml
- .tool-versions
- crates/dojo/core-cairo-test/src/tests/world/metadata.cairo
- rust-toolchain.toml
- crates/dojo/core-cairo-test/src/tests/world/storage.cairo
- crates/dojo/core-cairo-test/src/tests/utils/layout.cairo
- crates/dojo/core-cairo-test/src/tests/helpers/helpers.cairo
- crates/dojo/core/src/model/model.cairo
- crates/dojo/core/src/meta/layout.cairo
- crates/dojo/core/src/event/event.cairo
- crates/dojo/core-cairo-test/src/tests/utils/misc.cairo
- crates/dojo/core/src/model/metadata.cairo
- crates/dojo/core-cairo-test/src/tests/utils/hash.cairo
- crates/dojo/core/src/world/errors.cairo
- crates/dojo/core-cairo-test/src/lib.cairo
- crates/dojo/core-cairo-test/src/tests/storage/packing.cairo
- crates/dojo/core/src/model/definition.cairo
- crates/dojo/core-cairo-test/Scarb.toml
- crates/benches/contracts/Scarb.toml
- crates/dojo/core/src/world/resource.cairo
- examples/spawn-and-move/src/actions.cairo
- crates/dojo/core-cairo-test/src/tests/world/model.cairo
- crates/dojo/core-cairo-test/src/tests/world/world.cairo
- crates/dojo/core/src/model/model_value.cairo
- crates/dojo/core/src/model/component.cairo
- crates/dojo/core-cairo-test/src/tests/model/model.cairo
- crates/dojo/core/src/utils/layout.cairo
- crates/dojo/core/src/contract/components/upgradeable.cairo
- crates/dojo/core/src/lib.cairo
- examples/simple/src/lib.cairo
- crates/dojo/core/src/storage/storage.cairo
- crates/dojo/core/src/storage/entity_model.cairo
- crates/dojo/core/src/meta/introspect.cairo
- crates/dojo/core/src/storage/packing.cairo
- crates/dojo/core/src/model/storage.cairo
- crates/dojo/core/src/world/iworld.cairo
- crates/dojo/core/src/world/storage.cairo
- crates/dojo/core-cairo-test/src/tests/meta/introspect.cairo
- crates/dojo/core/src/storage/layout.cairo
- crates/dojo/core/src/world/world_contract.cairo
🔇 Additional comments (51)
crates/dojo/core-cairo-test/src/world.cairo (2)
Line range hint 58-81: Ohayo! Nice formatting improvements, sensei!
The addition of trailing commas in struct initialization and improved error message formatting enhances code maintainability and follows idiomatic Cairo style.
109-111: Clean syscall formatting, sensei!
The trailing comma in deploy_syscall parameters improves readability while maintaining the correct functionality.
crates/dojo/core-cairo-test/src/tests/world/contract.cairo (2)
232-239: Handle Unregistered Namespace Appropriately
The test test_deploy_contract_with_unregistered_namespace should ensure that deploying a contract with an unregistered namespace fails as expected.
The test correctly verifies the expected behavior.
Line range hint 359-365: Verify Contract Deployment Before Upgrade
In test_upgrade_contract_through_malicious_contract, ensure that the contract is deployed before attempting an upgrade to avoid unexpected CONTRACT_NOT_DEPLOYED errors.
Let's confirm if the contract deployment is successful before the upgrade.
crates/dojo/lang/src/attribute_macros/contract.rs (1)
155-155: Ohayo sensei! Verify the Use of diagnostics_note
The PluginGeneratedFile now includes diagnostics_note: None. Please confirm if this is intentional or if additional diagnostic notes are needed to provide helpful context during plugin generation.
bin/sozo/src/commands/test.rs (3)
114-114: Ohayo sensei! Confirm ignore_cairo_version Flag Setting
The ignore_cairo_version flag is set to false in CompileOpts. Ensure this aligns with the intended behavior, especially if compatibility with different Cairo versions is a concern.
183-183: Verify Necessity of Cloning main_crate_ids
main_crate_ids is cloned when creating the TestCompiler. Ensure that cloning is required here, or consider borrowing if possible to avoid unnecessary memory usage.
277-279: Ohayo sensei! Handle discriminator Appropriately When Interning Crates
When interning crates, you're setting discriminator to None. Confirm that this is appropriate, or if a specific discriminator should be provided to uniquely identify crates.
crates/katana/executor/src/implementation/blockifier/utils.rs (2)
97-104: Ohayo sensei! Verify Fee Type and Gas Price Mapping
Ensure that the gas prices for FeeType::Eth and FeeType::Strk are correctly mapped and retrieved. Confirm that using l2_gas_price.get().0 aligns with the intended fee calculations for each fee type.
160-160:
Address Potential Precision Loss When Casting initial_gas
Casting initial_gas from u128 to u64 may cause truncation if initial_gas exceeds u64::MAX. To prevent overflow errors, consider adding a check or handling large values appropriately.
Apply this diff to handle large initial_gas values safely:
-let mut remaining_gas = initial_gas as u64;
+let mut remaining_gas = if initial_gas > u64::MAX as u128 {
+ u64::MAX
+} else {
+ initial_gas as u64
+};Likely invalid or redundant comment.
crates/torii/types-test/Scarb.toml (1)
2-2: Ohayo sensei! Confirm Compatibility with Updated Cairo Version
The cairo-version is updated to =2.9.1. Ensure that the codebase and all dependencies are compatible with this version, addressing any deprecations or breaking changes introduced.
crates/dojo/core/Scarb.toml (1)
2-2: Ohayo! Version updates look consistent, but let's verify the upgrade path.
The version updates from 2.8.4 to 2.9.1 are consistently applied across cairo-version, starknet, and cairo_test dependencies. However, we should verify if there are any breaking changes in Cairo 2.9.1 that need attention.
Also applies to: 9-9, 14-14
✅ Verification successful
Ohayo! The storage API usage is safe for the Cairo 2.9.1 upgrade, sensei!
The code inspection shows that storage-related types and functions are used in a way that's compatible with Cairo 2.9.1. The core storage implementation and benchmarks are using the standard Starknet storage access patterns that remain stable in this version:
- Using
storage_base_address_from_felt252for base address generation - Proper usage of
storage_address_from_baseandstorage_address_from_base_and_offset - Standard storage read/write syscalls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any Cairo contracts using deprecated features
ast-grep --pattern 'use deprecated::*'
# Look for potential breaking changes in Cairo contracts
fd -e cairo | xargs rg -l '(storage_address|StorageAddress|storage_base_address|StorageBaseAddress)'
Length of output: 241
Script:
#!/bin/bash
# Check the content of files that use storage-related types
echo "=== core/src/storage/storage.cairo ==="
rg -A 5 -B 5 '(storage_address|StorageAddress|storage_base_address|StorageBaseAddress)' crates/dojo/core/src/storage/storage.cairo
echo -e "\n=== core-cairo-test/src/tests/benchmarks.cairo ==="
rg -A 5 -B 5 '(storage_address|StorageAddress|storage_base_address|StorageBaseAddress)' crates/dojo/core-cairo-test/src/tests/benchmarks.cairo
# Check if there are any other storage-related types or patterns we should look for
echo -e "\n=== Checking for storage type imports ==="
rg -l 'use.*storage::'
Length of output: 7256
crates/dojo/core/src/event/component.cairo (1)
6-6: Ohayo! Type parameter changes look good, sensei!
The addition of +Event<E> bound makes the type relationship more explicit and aligns well with Cairo 2.9.1's type system improvements. This change enhances type safety without altering the functionality.
Let's verify if this change is consistently applied across the codebase:
Also applies to: 15-15
✅ Verification successful
The previous pattern might have been too specific. Let's try these broader searches to understand the Event trait usage across the codebase and verify the consistency of the changes.
Ohayo! The Event trait bounds are consistently applied, sensei!
After checking the implementation file, I can confirm that the +Event<E> bound is properly and consistently applied across all three trait implementations: IDeployedEventImpl, IStoredEventImpl, and IEventImpl. This maintains type safety throughout the component's event handling system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other Event trait implementations that might need similar updates
ast-grep --pattern 'impl $name<$_, E> of IEvent<$_>'
Length of output: 54
Script:
#!/bin/bash
# Let's try a broader search for Event-related implementations
ast-grep --pattern 'impl $_ of IEvent'
# Also search for Event trait usage in type parameters
ast-grep --pattern '$_<$_, +Event<$_>>'
# And let's see the actual file content for context
cat crates/dojo/core/src/event/component.cairo
Length of output: 981
crates/katana/cairo/Cargo.toml (1)
21-21: Ohayo! Let's verify the stability of this commit, sensei!
While using a specific commit (92da150d) makes builds more reproducible, we should verify its stability. This commit was chosen as part of the Cairo 2.9.1 update.
Let's check the commit's status:
crates/dojo/core-cairo-test/src/utils.cairo (1)
52-52: 🛠️ Refactor suggestion
Ohayo! Consider maintaining the conventional assertion message format
The parameter order in the assertion message appears to have been swapped. Testing frameworks typically follow the convention of showing "expected" before "got" in assertion messages for better readability and consistency.
Here's the suggested fix:
- *value.at(i),
+ *expected.at(i),
+ *value.at(i)Likely invalid or redundant comment.
bin/sozo/benches/spawn-and-move-build.rs (1)
26-26: Ohayo! Good defensive programming, sensei!
The explicit ignore_cairo_version: false setting ensures proper version checking, which is crucial for the Cairo 2.9.1 upgrade.
Let's verify the Cairo version compatibility:
✅ Verification successful
Ohayo! The Cairo version compatibility is well-aligned, sensei!
The verification shows that all active project components are consistently using Cairo version 2.9.1, with the only exception being in the test data directory (test_data/invalid_cairo_version) which is intentionally set to an invalid version for testing purposes. This confirms that the ignore_cairo_version: false setting in the benchmark file is appropriate and aligned with the project's Cairo version requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Cairo version compatibility across the project
# Check Scarb.toml files for cairo-version declarations
fd -e toml | xargs rg "cairo-version|cairo_version"
Length of output: 485
crates/katana/executor/Cargo.toml (1)
18-19: Clean workspace dependency management!
The reintroduction of parking_lot and starknet as optional dependencies with proper workspace alignment is well-structured.
crates/dojo/core-cairo-test/src/tests/helpers/model.cairo (1)
60-70: Ohayo! Clean formatting improvements, sensei!
The formatting changes improve readability by:
- Consistent indentation of array elements
- Moving
.span()to a new line for better visual separation
crates/dojo/core-cairo-test/src/tests/world/namespace.cairo (1)
34-34: Ohayo! Nice alignment with Cairo 2.9.1's error handling, sensei!
The #[should_panic] attribute formatting has been updated to match Cairo 2.9.1's expectations, making the test cases more robust and maintainable.
Also applies to: 48-48, 73-73
crates/dojo/lang/src/inline_macros/selector_from_tag.rs (1)
70-70: Ohayo! Essential adaptation for Cairo 2.9.1, sensei!
The addition of diagnostics_note: None is required to support Cairo 2.9.1's enhanced plugin diagnostic capabilities.
Let's verify this change is consistent across other plugin files:
✅ Verification successful
Ohayo! The diagnostics_note field is consistently implemented across all plugin files, sensei!
The verification shows that diagnostics_note: None is properly added to all plugin-related files in the codebase, including:
- Inline macros (get, delete, set, emit, etc.)
- Attribute macros (contract, interface, event, model)
- Derive macros
This confirms the change is part of a systematic adaptation for Cairo 2.9.1 across the entire plugin system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if diagnostics_note is consistently added across plugin files
rg -l "PluginGeneratedFile" | xargs rg "diagnostics_note"
Length of output: 1122
examples/spawn-and-move/src/models.cairo (1)
33-33: Ohayo! Nice work on the consistent trailing commas sensei!
The addition of trailing commas in struct field declarations improves git diff readability and makes future additions cleaner. This change aligns well with modern Cairo style guidelines.
Also applies to: 42-42, 56-56
crates/dojo/core-cairo-test/src/tests/helpers/event.cairo (1)
68-78: Ohayo! Clean formatting of namespace resources sensei!
The improved formatting with proper indentation and trailing commas makes the resource array more readable. However, let's verify the event contract deployment with Cairo 2.9.1.
✅ Verification successful
Let me analyze the event contracts and Cairo version compatibility with these commands to provide a more accurate verification.
Ohayo! Event contracts are compatible with Cairo 2.9.1 sensei!
The code is properly configured with Cairo 2.9.1 as specified in the Scarb.toml, and the event contracts are using the correct attribute macros and implementations. The formatting changes are safe and don't affect functionality.
- The event contracts correctly use
#[dojo::event]attribute - The storage implementation follows Cairo 2.9.1 patterns
- The world deployment for event upgrades is properly structured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if event contracts can be deployed with Cairo 2.9.1
# Check if any compilation issues exist in event contracts
ast-grep --pattern 'struct Storage {}' | while read -r file; do
echo "Checking event contract in $file"
scarb build
done
Length of output: 55
Script:
#!/bin/bash
# Let's check for event-related contracts and their structure
rg -l "Event" --type cairo
# Check for any event-related test files
rg -l "test.*event" --type cairo
# Look for event contract structures
ast-grep --pattern 'contract $_ {
$$$
#[event]
$$$
}'
# Check Cairo version in Scarb.toml
cat **/Scarb.toml | grep "cairo-lang"
Length of output: 271
Script:
#!/bin/bash
# Let's try finding Scarb.toml files first
fd Scarb.toml
# Find all Cairo files
fd -e cairo
# Look for event-related files
rg -l "Event"
# Check for test event files specifically
rg -l "test.*event"
Length of output: 13311
Script:
#!/bin/bash
# Let's check the Cairo version in the relevant Scarb.toml
cat crates/dojo/core-cairo-test/Scarb.toml
# Check the event test implementation
cat crates/dojo/core-cairo-test/src/tests/helpers/event.cairo
# Look for any event-related changes in recent commits
git log -n 5 --oneline -- crates/dojo/core-cairo-test/src/tests/helpers/event.cairo
Length of output: 3672
crates/dojo/core/src/utils/snf_test.cairo (2)
40-56: Ohayo! Clean match expression formatting sensei!
The improved formatting of the match expression and its arms makes the resource registration logic more readable.
65-65: Verify world deployment compatibility with Cairo 2.9.1
The function signatures have been updated with trailing commas. While this is a good style improvement, we should verify that the world deployment functionality works correctly with Cairo 2.9.1.
Also applies to: 68-68, 74-74, 81-81, 87-87, 94-94
✅ Verification successful
World deployment is compatible with Cairo 2.9.1, sensei! ✨
The verification shows that:
- The project explicitly specifies Cairo 2.9.1 in multiple Scarb.toml files
- The world deployment functionality is extensively used across test files and examples
- The trailing commas in function signatures are a supported feature in Cairo 2.9.1
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify world deployment functionality with Cairo 2.9.1
# Check for any breaking changes in world deployment
ast-grep --pattern 'fn spawn_test_world' | while read -r file; do
echo "Checking world deployment in $file"
# Look for any related test files
rg -l "spawn_test_world" --type cairo
done
# Check for any changes in panic message formatting
rg "panic_with_byte_array.*format!" --type cairo
Length of output: 138
Script:
#!/bin/bash
# Let's try a different approach to check world deployment and panic messages
# Check for world deployment related functions
rg -l "spawn_test_world"
# Look for any test files that might use world deployment
rg -l "test_world"
# Check for panic message patterns
rg -l "panic_with"
# Check Cairo version specification in any config or manifest files
fd "Scarb.toml" --exec cat {}
Length of output: 6465
crates/dojo/lang/src/inline_macros/get.rs (1)
102-102: Ohayo! Addition of diagnostics_note field aligns with Cairo 2.9.1 requirements
The addition of diagnostics_note: None appears to be part of the Cairo 2.9.1 upgrade. Let's verify this pattern across the codebase.
✅ Verification successful
Ohayo! The diagnostics_note field is consistently implemented across all macros
The verification shows that diagnostics_note: None is uniformly implemented across:
- All inline macros in
crates/dojo/lang/src/inline_macros/ - All attribute macros in
crates/dojo/lang/src/attribute_macros/ - The derive macros module
This consistent pattern confirms the change aligns with Cairo 2.9.1's plugin system requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent implementation of diagnostics_note field
# across all inline macro files
# Search for PluginGeneratedFile struct usage
rg -A 5 "PluginGeneratedFile\s*\{" crates/dojo/lang/src/inline_macros/
# Search for any existing usage of diagnostics_note field
rg "diagnostics_note\s*:" crates/dojo/lang/src/
Length of output: 4938
crates/dojo/lang/src/inline_macros/get_models_test_class_hashes.rs (1)
100-100: Consistent implementation of Cairo 2.9.1 plugin structure, sensei!
The addition of diagnostics_note: None maintains consistency with other inline macro implementations.
crates/dojo/lang/src/inline_macros/delete.rs (1)
108-108: Ohayo! LGTM - Addition of diagnostics_note field
The addition of the diagnostics_note field aligns with the Cairo 2.9.1 upgrade and enhances the diagnostic capabilities of the macro system.
Let's verify this change is consistent across other macro files:
✅ Verification successful
Ohayo! LGTM - diagnostics_note field consistently implemented
The verification confirms that the diagnostics_note field is consistently set to None across all macro files in the codebase:
- set.rs
- selector_from_tag.rs
- get_models_test_class_hashes.rs
- emit.rs
- get.rs
- delete.rs
- spawn_test_world.rs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent diagnostics_note implementation across macro files
rg -U "PluginGeneratedFile\s*\{[\s\S]*?diagnostics_note:" crates/dojo/lang/src/inline_macros/
Length of output: 3826
crates/dojo/lang/src/inline_macros/emit.rs (1)
112-112: Ohayo! LGTM - Consistent diagnostics_note implementation
The implementation matches the pattern seen in delete.rs, maintaining consistency across macro implementations.
crates/katana/executor/src/implementation/blockifier/error.rs (1)
34-35: Ohayo sensei! Updated error handling for Cairo 2.9.1
The change from error_data to error_trace simplifies error message construction and aligns with Cairo 2.9.1's error handling patterns.
Let's verify the error trace format:
✅ Verification successful
Ohayo! Error handling change is properly aligned with Cairo 2.9.1 sensei!
The change from error_data to error_trace is correctly implemented and consistently used across the codebase. The error handling structure shows that:
error_traceis the only implementation used for execution failures in the codebase- The error conversion is properly integrated into the error handling hierarchy
- The change aligns with the broader error handling patterns in the executor module
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent error trace handling across the codebase
rg -U "error_trace.*to_string\(\)" crates/katana/
Length of output: 187
Script:
#!/bin/bash
# Let's check for any other error trace related patterns and their usage
rg -U "error_trace" crates/katana/ -A 3 -B 3
# Also check for error_data to understand the previous pattern
rg -U "error_data" crates/katana/ -A 3 -B 3
Length of output: 1035
Script:
#!/bin/bash
# Let's check the git history for this file to see the previous implementation
git log -p crates/katana/executor/src/implementation/blockifier/error.rs | grep -A 10 -B 10 "error_data"
# Also check for any related test files that might verify error handling
fd "error.*rs" crates/katana/executor/src/
Length of output: 154
Script:
#!/bin/bash
# Let's check the content of both error files to understand the error handling structure
cat crates/katana/executor/src/abstraction/error.rs
cat crates/katana/executor/src/implementation/blockifier/error.rs
# Also check for any tests that might be using these error types
rg -U "ExecutionError|EntryPointExecutionError" crates/katana/ -A 3 -B 3
Length of output: 32400
crates/dojo/lang/src/inline_macros/set.rs (1)
124-124: Ohayo! LGTM - Addition of diagnostics_note field
The addition of diagnostics_note field aligns with the broader changes for Cairo 2.9.1 update.
crates/dojo/lang/src/attribute_macros/interface.rs (1)
89-89: Ohayo! LGTM - Consistent diagnostics_note implementation
The addition of diagnostics_note field maintains consistency with other macro implementations in the codebase.
.github/workflows/ci.yml (2)
115-115: Ohayo! LGTM - Cairo 2.9.1 version update
The scarb version update to 2.9.1 aligns perfectly with the PR objectives.
Line range hint 192-206: Verify hurl installation success
Ohayo! Let's ensure the hurl installation succeeds before proceeding with tests.
✅ Verification successful
Let me verify the installation process more thoroughly by checking the actual workflow file and installation commands.
Let me check the actual hurl test files to understand the test requirements better.
Installation and test setup for hurl looks good, sensei!
The verification shows:
- Hurl 3.0.0 installation is properly configured in the workflow
- All reported issues for hurl 3.0.0 are closed
- The test files contain valid hurl syntax for RPC endpoint testing
- The workflow correctly sets up the test environment with katana before running hurl tests
- The test execution command matches the documented approach in the README
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if hurl version 3.0.0 has any reported issues or vulnerabilities
# Check for any GitHub issues mentioning installation problems with hurl 3.0.0
gh api graphql -f query='
{
search(query: "repo:Orange-OpenSource/hurl is:issue label:bug version:3.0.0", type: ISSUE, first: 5) {
nodes {
... on Issue {
title
state
url
}
}
}
}'
Length of output: 929
Script:
#!/bin/bash
# Check the workflow file for hurl installation and usage
cat .github/workflows/ci.yml
# Verify if there are any other hurl-related configurations
rg -l "hurl" .
Length of output: 6117
Script:
#!/bin/bash
# Check the hurl test files
fd -e hurl -x cat {} \;
# Also check the README for any specific hurl test requirements
cat examples/rpc/starknet/README.md
Length of output: 8153
🧰 Tools
🪛 actionlint (1.7.4)
112-112: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
crates/dojo/lang/src/derive_macros/mod.rs (1)
58-58: Ohayo! Consistent diagnostic enhancement, sensei!
The addition of diagnostics_note field maintains consistency with other macro implementations.
Let's verify the consistency across all plugin files:
✅ Verification successful
Ohayo! Diagnostic consistency verified, sensei!
The diagnostics_note field is consistently set to None across all plugin-generated files in the codebase, including:
- All inline macros (spawn_test_world, set, selector_from_tag, get_models_test_class_hashes, get, emit, delete)
- All attribute macros (model, interface, event, contract)
- The derive macros implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for PluginGeneratedFile struct usage
ast-grep --pattern 'PluginGeneratedFile {
$$$
diagnostics_note: $_,
$$$
}'
Length of output: 7819
Cargo.toml (2)
147-168: Ohayo! Cairo dependencies consistently updated to 2.9.1
The update of all cairo-lang-* dependencies from 2.8.4 to 2.9.1 is consistent and well-structured.
222-223: Verify stability of starknet_api git reference
The change from a stable version (0.11.0) to a git reference could introduce instability. Please ensure that commit 92da150d is stable and well-tested.
crates/dojo/core-cairo-test/src/tests/world/event.cairo (1)
25-25: Ohayo! Style improvements with trailing commas
The addition of trailing commas improves code maintainability and aligns with Cairo 2.9.1 style guidelines.
Also applies to: 35-35, 45-45, 71-71, 75-75, 139-139, 157-157, 161-161, 165-165, 177-177, 189-189, 201-201, 216-216, 228-228
crates/dojo/lang/src/attribute_macros/model.rs (1)
204-204: Ohayo! Enhanced diagnostic capabilities
The addition of diagnostics_note field aligns with the codebase-wide enhancement of diagnostic capabilities.
bin/sozo/src/commands/build.rs (1)
106-106: LGTM! Version check enforcement looks good.
Ohayo! The addition of ignore_cairo_version: false aligns well with the Cairo 2.9.1 upgrade, ensuring proper version compatibility checks.
Let's verify this setting's usage across the codebase:
✅ Verification successful
LGTM! Version check enforcement is consistently applied across the codebase
Ohayo! After verification, the ignore_cairo_version: false setting is consistently used across the main build paths (sozo build, test, and world abigen). The only exception is in test-utils where it's intentionally set to true for testing purposes, which is the expected pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other occurrences of ignore_cairo_version
rg -A 2 "ignore_cairo_version"
Length of output: 972
crates/dojo/core-cairo-test/src/tests/world/acl.cairo (2)
5-5: LGTM! Import formatting looks clean.
Ohayo! The import line is properly formatted and includes all necessary components.
67-68: Error message formatting improvements.
Ohayo sensei! The error message formatting has been standardized across all test cases, making the expected panic messages more consistent and readable. The changes include:
- Proper comma placement after error messages
- Consistent 'ENTRYPOINT_FAILED' formatting
- Clearer error message structure
Also applies to: 106-107, 170-171, 209-210, 232-233, 254-254, 265-265, 297-297
crates/katana/rpc/rpc-types/src/class.rs (2)
9-11: Clean import organization.
Ohayo! The imports have been properly reorganized to use the new starknet_api path structure.
130-130: Type update aligns with Cairo 2.9.1.
Ohayo sensei! The change from EntryPoint to EntryPointV0 in the entry_points_by_type field reflects the API updates in Cairo 2.9.1.
Let's verify the consistency of this change:
✅ Verification successful
Let me verify the usage of EntryPoint types further to ensure consistency.
EntryPointV0 change is consistent with the codebase
Ohayo sensei! After verifying the codebase, I can confirm that the change from EntryPoint to EntryPointV0 in entry_points_by_type is correct and consistent. The EntryPointV0 type is properly imported from the Cairo package and used consistently in the relevant conversion and type definitions across the codebase. The old EntryPoint references in the codebase are different types used for other purposes (like enums and execution contexts) and don't conflict with this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining old EntryPoint usages
rg "EntryPoint[^V]" --type rust
Length of output: 8974
Script:
#!/bin/bash
# Check the specific EntryPointV0 usage
rg "EntryPointV0" --type rust -A 2
Length of output: 1522
crates/katana/primitives/src/conversion/rpc.rs (1)
7-7: Ohayo! Import changes look good for the new Cairo version.
The import changes align well with the Cairo 2.9.1 upgrade, particularly the addition of storage_address_from_base_and_offset and reorganization of EntryPointType imports.
Also applies to: 9-9
crates/dojo/test-utils/src/compiler.rs (2)
174-174: Ohayo! New compilation parameter added for Cairo version handling.
The addition of ignore_cairo_version: true aligns with the PR objective of upgrading to Cairo 2.9.1, allowing for smoother version transitions during compilation.
323-324: Parameter propagation looks good, sensei!
The ignore_cairo_version parameter is correctly propagated to the generate_compilation_units function.
crates/dojo/core-cairo-test/src/tests/benchmarks.cairo (2)
47-53: Ohayo! Array formatting improvements look clean.
The formatting changes in the deploy_world function improve readability with proper indentation and trailing commas.
189-189: Nice struct formatting improvements, sensei!
The consistent addition of trailing commas and proper alignment in struct initializations across multiple test functions improves code readability and maintainability. This includes improvements in:
- Foo struct initialization
- Case struct initialization
- Complex struct member formatting
- World entity setup formatting
Also applies to: 290-290, 417-417, 421-421, 431-435, 453-453, 464-464, 474-474, 484-484, 494-494
crates/katana/executor/src/implementation/blockifier/state.rs (3)
Line range hint 49-64: Ohayo! Clean implementation of the new compiled class handling.
The refactoring from get_compiled_contract_class to get_compiled_class with RunnableCompiledClass return type improves the abstraction by making it more generic and execution-focused.
218-222: Consistent trait implementation for StateReader.
The changes maintain consistency with the base implementation while properly implementing the StateReader trait.
309-325: Test cases properly updated for the new class handling.
The test assertions have been correctly updated to use to_runnable_class, maintaining test coverage for the new implementation.
| TransactionExecutionError::PanicInValidate { panic_reason } => { | ||
| // TODO: maybe can remove the address and class hash? | ||
| Ok(InvalidTransactionError::ValidationFailure { | ||
| address: Default::default(), | ||
| class_hash: Default::default(), | ||
| error: panic_reason.to_string(), | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle All Variants of TransactionExecutionError
The map_invalid_tx_err function should handle all possible variants of TransactionExecutionError to ensure comprehensive error mapping.
Consider adding match arms for unhandled variants or use a wildcard arm to capture all other errors.
_ => Err(Box::new(err)),
+ // Add handling for other variants if necessary
+ // _ => Ok(InvalidTransactionError::OtherError { error: e.to_string() }),Committable suggestion skipped: line range outside the PR's diff.
| expected: ("Account `2827` does NOT have OWNER role on namespace `dojo`", 'ENTRYPOINT_FAILED'), | ||
| )] | ||
| fn test_deploy_contract_for_namespace_writer() { | ||
| let world = deploy_world(); |
There was a problem hiding this comment.
Correct the Error Message in should_panic Attribute
In the test_deploy_contract_for_namespace_writer function, the expected panic message has inconsistent backticks around the account address.
Ensure consistent formatting for accurate error matching.
#[should_panic(
expected: (
- "Account `2827` does NOT have OWNER role on namespace `dojo`",
+ "Account `0xb0b` does NOT have OWNER role on namespace `dojo`",
'ENTRYPOINT_FAILED'
)
)]Committable suggestion skipped: line range outside the PR's diff.
| let compilation_units = | ||
| ops::generate_compilation_units(&resolve, &opts.features, false, &ws)? | ||
| .into_iter() | ||
| .filter(|cu| { | ||
| let is_excluded = | ||
| opts.exclude_target_kinds.contains(&cu.main_component().target_kind()); | ||
| let is_included = opts.include_target_kinds.is_empty() | ||
| || opts.include_target_kinds.contains(&cu.main_component().target_kind()); | ||
| let is_included = is_included | ||
| && (opts.include_target_names.is_empty() | ||
| || cu | ||
| .main_component() | ||
| .targets | ||
| .iter() | ||
| .any(|t| opts.include_target_names.contains(&t.name))); | ||
|
|
||
| let is_selected = packages.iter().any(|p| p.id == cu.main_package_id()); | ||
|
|
||
| let is_cairo_plugin = matches!(cu, CompilationUnit::ProcMacro(_)); | ||
| is_cairo_plugin || (is_included && is_selected && !is_excluded) | ||
| }) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify Logic for Filtering Compilation Units
The reassignment of is_included at line 125 may lead to confusion. Consider renaming variables or restructuring the logic for better clarity.
Apply this refactor for improved readability:
-let is_included = is_included
+let is_target_included = is_includedThen, use is_target_included in subsequent conditions to enhance code clarity.
Committable suggestion skipped: line range outside the PR's diff.
| @@ -1,5 +1,5 @@ | |||
| [package] | |||
| cairo-version = "=2.8.4" | |||
| cairo-version = "=2.9.1" | |||
There was a problem hiding this comment.
💡 Codebase verification
Ohayo! Version pinning needs attention, but contract implementations look safe, sensei!
- All Scarb.toml files need exact version pinning (
=) for consistency:- examples/spawn-and-move/Scarb.toml
- examples/simple/Scarb.toml
- crates/katana/contracts/Scarb.toml (also needs upgrade from 2.8.2)
The contract implementations review shows they're using standard Cairo patterns (syscalls, trait implementations) that are stable across the 2.8.x to 2.9.x upgrade path.
🔗 Analysis chain
Ohayo! Same version pinning inconsistency here, sensei.
The starknet dependency should use exact version pinning for consistency with core:
-starknet = "2.9.1"
+starknet = "=2.9.1"Additionally, since this completes our review of all Scarb.toml files, let's verify the overall upgrade impact:
Also applies to: 19-19
Ohayo! Found some version inconsistencies across the codebase, sensei!
The verification revealed multiple concerns that need attention:
-
Unpinned starknet versions in multiple Scarb.toml files:
- examples/spawn-and-move/Scarb.toml:
starknet = "2.9.1" - examples/simple/Scarb.toml:
starknet = "2.9.1" - crates/katana/contracts/Scarb.toml:
starknet = "2.8.2"
- examples/spawn-and-move/Scarb.toml:
-
The katana contracts are still using an older version (2.8.2) which should be updated for consistency
-
Several contract implementations were found that might be affected by the blockifier changes:
- crates/katana/contracts/messaging/cairo/src/contract_msg_starknet.cairo
- crates/katana/contracts/messaging/cairo/src/contract_msg_l1.cairo
- crates/dojo/core-cairo-test/src/world.cairo
- And more in core-cairo-test
Let's verify these contract implementations to ensure compatibility:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any unpinned versions across all Scarb.toml files
fd Scarb.toml | xargs rg "starknet = \"[^=]"
# Look for any remaining old versions
fd Scarb.toml | xargs rg "2\.8\.4"
# Check for any custom implementations that might be affected by the new blockifier
fd -e cairo | xargs rg -l "impl Contract"
Length of output: 574
Script:
#!/bin/bash
# Check the contract implementations for potential compatibility issues
for file in crates/katana/contracts/messaging/cairo/src/contract_msg_starknet.cairo \
crates/katana/contracts/messaging/cairo/src/contract_msg_l1.cairo \
crates/dojo/core-cairo-test/src/world.cairo \
crates/dojo/core-cairo-test/src/tests/world/contract.cairo \
crates/dojo/core-cairo-test/src/tests/contract.cairo; do
echo "=== $file ==="
cat "$file" | grep -A 10 "impl Contract"
done
Length of output: 5037
| @@ -170,6 +170,7 @@ fn compile_dojo_core() -> Result<()> { | |||
| include_target_kinds: vec![], | |||
| exclude_target_kinds: vec![TargetKind::TEST], | |||
| features: features_opts, | |||
| ignore_cairo_version: false, | |||
There was a problem hiding this comment.
💡 Codebase verification
Ohayo! Found an inconsistency in version checking across the codebase, sensei!
I noticed that ignore_cairo_version is set to true in test-utils/src/compiler.rs while it's set to false everywhere else. This could lead to version compatibility issues during testing.
crates/dojo/test-utils/src/compiler.rs: Setignore_cairo_version: true- All other locations correctly set
ignore_cairo_version: false
🔗 Analysis chain
Ohayo! The version check addition looks good, sensei!
Setting ignore_cairo_version: false is appropriate for ensuring compatibility with Cairo 2.9.1.
Let's verify other compilation options across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other CompileOpts configurations
rg "CompileOpts\s*\{" -A 5
Length of output: 2400
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
examples/game-lib/bestiary/Scarb.toml (1)
Line range hint
1-11: Ohayo! Consider adding workspace documentation, sensei!Since we're using workspace inheritance in the example code, it would be helpful to add a README.md in the examples directory explaining the workspace structure and how these configurations relate to the Cairo 2.9.1 upgrade.
Would you like me to help draft the workspace documentation?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockspawn-and-move-db.tar.gzis excluded by!**/*.gztypes-test-db.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (8)
Cargo.toml(5 hunks)bin/sozo/src/commands/events.rs(1 hunks)crates/dojo/world/abigen/src/main.rs(2 hunks)crates/dojo/world/src/contracts/abigen/model.rs(7 hunks)examples/game-lib/Scarb.toml(1 hunks)examples/game-lib/armory/Scarb.toml(1 hunks)examples/game-lib/bestiary/Scarb.toml(1 hunks)examples/spawn-and-move/dojo_dev.toml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/game-lib/Scarb.toml
- examples/spawn-and-move/dojo_dev.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/dojo/world/abigen/src/main.rs
🔇 Additional comments (8)
examples/game-lib/armory/Scarb.toml (1)
5-5: Ohayo! Verify workspace root configuration, sensei!
The workspace inheritance looks good, but we should ensure these values are properly defined in the workspace root's Scarb.toml.
Also applies to: 8-8
✅ Verification successful
Ohayo! The workspace configuration is properly set, sensei!
I found the workspace root's Scarb.toml in examples/game-lib/Scarb.toml which correctly defines the edition = "2024_07" in the [workspace.package] section. The inheritance in armory/Scarb.toml is working as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check workspace root configuration
# Look for workspace root Scarb.toml and verify edition and dependencies
# Find workspace root Scarb.toml
fd -t f "Scarb.toml" --max-depth 1
# Check its contents if found
if [ $? -eq 0 ]; then
rg -A 10 "^\[workspace\]" Scarb.toml
fi
Length of output: 142
Script:
#!/bin/bash
# Let's search for Scarb.toml files in the entire repository
fd -t f "Scarb.toml"
# If we find any, let's check their contents for workspace configuration
fd -t f "Scarb.toml" -x rg -A 10 "^\[workspace\]" {}
Length of output: 1017
examples/game-lib/bestiary/Scarb.toml (1)
5-5: Configuration consistency looks perfect, sensei!
The workspace configuration matches the pattern used in armory, which is exactly what we want to see for maintainable example code.
Also applies to: 8-8
Cargo.toml (3)
76-77: LGTM! Dependency source updates for cainome.
The change from tag-based to revision-based versioning provides better control over the dependency version.
147-168: Ohayo! Cairo-lang suite update looks good, sensei!
The comprehensive update of all cairo-lang dependencies to version 2.9.1 maintains consistency across the ecosystem.
222-223: Verify the sequencer compatibility.
The switch from version-based to git-based dependency for starknet_api warrants verification of compatibility with the sequencer.
✅ Verification successful
Let me check the starknet_api version in the sequencer repository and any potential dependency conflicts in our codebase.
Dependency change is compatible with the codebase
The switch to git-based dependency is consistent across the project. The same commit 92da150d is used in both the root Cargo.toml and crates/katana/cairo/Cargo.toml, and all imports of starknet_api throughout the codebase are properly aligned with the API surface provided by the sequencer repository.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the sequencer commit exists and is stable
gh api repos/dojoengine/sequencer/commits/92da150d --jq '.sha'
Length of output: 103
Script:
#!/bin/bash
# Check the Cargo.toml in the sequencer repository at the specified commit
gh api repos/dojoengine/sequencer/contents/Cargo.toml?ref=92da150d --jq '.content' | base64 -d | grep -A 5 "starknet_api"
# Check if there are any other dependencies in our project using starknet_api
rg "starknet_api" -A 3 -B 3
Length of output: 24949
bin/sozo/src/commands/events.rs (1)
89-97: LGTM! Clean refactoring of event handling.
The introduction of local variables tx_hash and block_number improves code readability by reducing property access chains.
crates/dojo/world/src/contracts/abigen/model.rs (2)
224-224: Ohayo! New metadata_hash field looks good, sensei!
The addition of metadata_hash to both ResourceMetadata and ResourceMetadataValue structs is properly implemented with the correct type.
Also applies to: 261-261
597-606: LGTM! Consistent implementation of ensure_unique.
The ensure_unique method is properly implemented for both ModelContract and ModelContractReader with consistent behavior.
Also applies to: 714-723
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
crates/katana/executor/src/implementation/blockifier/utils.rs (2)
566-575: Ohayo, sensei! Address the TODO into_runnable_classfunctionThe TODO comment suggests uncertainty about the correct variant to use and whether the return type should include an error case for contract class conversions. Resolving this will ensure proper handling of different compiled class types.
Shall I assist in determining the appropriate variant and updating the return type to accurately reflect possible errors during class conversion?
579-598: Ohayo, sensei! Clarify implementation details into_class_infofunctionThere are TODO comments questioning the correct variant usage and how to obtain the ABI length for the class. It's crucial to address these uncertainties to ensure accurate construction of
ClassInfo.Would you like help in identifying the correct variant and methods to retrieve the ABI length? I can assist in refining the implementation or open a GitHub issue if needed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
crates/katana/executor/src/implementation/blockifier/utils.rs(21 hunks)crates/katana/rpc/rpc/tests/starknet.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/katana/rpc/rpc/tests/starknet.rs
🔇 Additional comments (2)
crates/katana/executor/src/implementation/blockifier/utils.rs (2)
156-158: Ohayo, sensei! Confirm the assignment of vm_run_resources
You're assigning a new RunResources to ctx.vm_run_resources based on initial_gas. Ensure that this aligns with the latest Blockifier implementation, as changes in gas computation might affect execution.
Would you like me to check the Blockifier documentation or source code to confirm that this assignment is appropriate?
95-104: Ohayo, sensei! Verify gas price retrieval for accurate fee calculation
In the fee calculation, the gas price is obtained using l2_gas_price.get().0. Ensure that this retrieves the correct gas price for fee computation. Any mismatch could lead to incorrect fee assessments.
Please run the following script to confirm the gas prices are correctly retrieved:
This will list all instances where eth_gas_prices and strk_gas_prices are used, allowing you to verify their correctness.
✅ Verification successful
Let's verify how the gas price is structured and used in the codebase to ensure we're accessing it correctly with .get().0.
Ohayo! The gas price retrieval looks correct, sensei!
The code correctly accesses the gas prices through the proper struct hierarchy. The .get().0 pattern is consistently used for both ETH and STRK gas prices, and the l2_gas_price field is properly initialized in both GasPriceVector instances. The implementation aligns with the expected behavior for fee calculation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the gas prices for ETH and STRK are correctly defined.
# Check ETH gas prices
rg 'eth_gas_prices' crates/katana/executor/src/implementation/blockifier/utils.rs
# Check STRK gas prices
rg 'strk_gas_prices' crates/katana/executor/src/implementation/blockifier/utils.rs
Length of output: 430
Script:
#!/bin/bash
# Let's check the GasPriceVector struct definition and its usage
ast-grep --pattern 'struct GasPriceVector {
$$$
}'
# Also check for l2_gas_price usage and definition
rg 'l2_gas_price' -A 2
Length of output: 1819
| // TODO: i dont know if this is correct | ||
| let (unit, gas_price) = match fee_type { | ||
| FeeType::Eth => { | ||
| (PriceUnit::Wei, block_context.block_info().gas_prices.eth_l1_gas_price) | ||
| } | ||
| FeeType::Strk => { | ||
| (PriceUnit::Fri, block_context.block_info().gas_prices.strk_l1_gas_price) | ||
| } | ||
| FeeType::Eth => ( | ||
| PriceUnit::Wei, | ||
| block_context.block_info().gas_prices.eth_gas_prices.l2_gas_price.get().0, | ||
| ), | ||
| FeeType::Strk => ( | ||
| PriceUnit::Fri, | ||
| block_context.block_info().gas_prices.strk_gas_prices.l2_gas_price.get().0, | ||
| ), |
There was a problem hiding this comment.
Ohayo, sensei! Clarify the fee unit and gas price mapping
The TODO comment indicates uncertainty about the correctness of the unit and gas_price mapping for FeeType::Eth and FeeType::Strk. It's important to ensure that the fee calculation aligns with the expected units and gas prices for each fee type.
Would you like assistance in verifying and correcting the fee unit and gas price mappings for FeeType::Eth and FeeType::Strk? I can help update the code or open a GitHub issue to track this task.
| )?; | ||
| let tx_context = Arc::new(TransactionContext { | ||
| block_context: block_context.clone(), | ||
| tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo, sensei! Update usage of deprecated TransactionInfo
You're using TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()) in the tx_context. Since this is deprecated, it would be prudent to update to the current TransactionInfo variant to maintain compatibility with the latest standards.
I suggest updating the code to use the latest TransactionInfo variant. Here's a possible refactor:
- tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()),
+ tx_info: TransactionInfo::default(),This assumes that TransactionInfo implements Default. Please adjust accordingly based on the actual implementation.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
crates/katana/rpc/rpc/tests/starknet.rs (1)
1148-1175: Ohayo sensei! Consider enhancing the contract call assertions.While the test structure and implementation look good, we could improve the test coverage by validating the returned values from the contract calls instead of just checking if they don't panic.
Here's a suggested enhancement:
- let _ = contract.name().call().await.unwrap(); - let _ = contract.balanceOf(&account).call().await.unwrap(); + let name = contract.name().call().await.unwrap(); + assert_eq!(name, "Ether"); // or expected token name + + let balance = contract.balanceOf(&account).call().await.unwrap(); + assert!(balance > 0, "Account should have non-zero balance"); - let _ = contract.get_public_key().call().await.unwrap(); + let public_key = contract.get_public_key().call().await.unwrap(); + assert_ne!(public_key, Felt::ZERO, "Public key should not be zero");crates/katana/executor/src/implementation/blockifier/utils.rs (3)
568-570: Ohayo, sensei! Address the TODO regarding contract class conversionThe TODO comment indicates uncertainty about the correct variant to use for contract class conversions in
to_runnable_class. Resolving this will ensure accurate execution of compiled classes and prevent potential runtime errors.Would you like assistance in determining the appropriate variant and updating the return type if necessary? I can help clarify the conversion process or open a GitHub issue to track this task.
582-584: Ohayo, sensei! Address the TODO regarding class information conversionThe TODO comment in
to_class_infosuggests uncertainty about the correct variant for contract class conversions. Clarifying this is important for providing accurate class information and preventing potential errors during execution.Would you like assistance in determining the appropriate variant and updating the function accordingly? I can help with the implementation or open a GitHub issue to track this task.
723-723: Ohayo, sensei! Consider makingonly_queryconfigurableIn the
From<ExecutionFlags>implementation, theonly_queryfield is hardcoded tofalse. If there's a possibility of supporting query-only executions in the future, consider derivingonly_queryfrom theExecutionFlagsor introducing a new configuration parameter to enhance flexibility.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
crates/katana/executor/src/implementation/blockifier/utils.rs(21 hunks)crates/katana/rpc/rpc/tests/starknet.rs(4 hunks)
🔇 Additional comments (4)
crates/katana/rpc/rpc/tests/starknet.rs (1)
Line range hint 929-951: Ohayo sensei! The v3 transaction test implementation looks good!
The changes align well with the testing patterns in the codebase. The unwrap calls are appropriate for test code, and the test properly verifies both transaction submission and execution status.
crates/katana/executor/src/implementation/blockifier/utils.rs (3)
95-104: Ohayo, sensei! Clarify the fee unit and gas price mapping
The TODO comment indicates uncertainty about the correctness of the unit and gas_price mapping for FeeType::Eth and FeeType::Strk. It's crucial to ensure that the fee calculation aligns with the expected units and gas prices for each fee type to maintain accurate fee assessments.
Would you like assistance in verifying and correcting the fee unit and gas price mappings for FeeType::Eth and FeeType::Strk? I can help update the code or open a GitHub issue to track this task.
144-144: 🛠️ Refactor suggestion
Ohayo, sensei! Update usage of deprecated TransactionInfo
You're using TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()) in the tx_context. Since this is deprecated, it's advisable to update to the current TransactionInfo variant to maintain compatibility with the latest standards.
I suggest updating the code to use the latest TransactionInfo variant. Here's a possible refactor:
- tx_info: TransactionInfo::Deprecated(DeprecatedTransactionInfo::default()),
+ tx_info: TransactionInfo::default(),This assumes that TransactionInfo implements Default. Please adjust accordingly based on the actual implementation.
69-69: Ohayo, sensei! Ensure all calls to transact include the new flags parameter
The transact function signature now includes flags: &ExecutionFlags. Please verify that all invocations of this function have been updated to include the new parameter to prevent any compilation errors or unexpected behavior.
Run the following script to identify any calls to transact missing the flags parameter:
✅ Verification successful
The previous search wasn't conclusive enough. Let's look at the actual implementation and all usages to verify if the flags parameter is properly handled.
Ohayo! All transact calls are properly updated with the flags parameter, sensei!
After checking all usages of the transact function in the codebase, I can confirm that all calls include the required flags parameter:
- In
crates/katana/executor/benches/execution.rs:transact(&mut state, block_context, flags, tx) - In
crates/katana/executor/src/implementation/blockifier/mod.rs:utils::transact(&mut state, block_context, flags, exec_tx)utils::transact(&mut state.inner, block_context, flags, exec_tx)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find calls to `transact` that may not include the new `flags` parameter.
# Expected: No matches indicating all calls are updated.
rg -A2 "transact\(" | grep -B2 -A2 -E "transact\([^,]+,[^,]+,[^)]+\)" | grep -v "flags"
Length of output: 654
Script:
#!/bin/bash
# Let's search for the actual implementation and usage of transact function
ast-grep --pattern 'fn transact($$$)'
# Also search for all transact calls with more context
rg "transact\(" -B 2 -A 2
Length of output: 1942
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
bin/sozo/src/commands/test.rs (2)
160-170: Optimize clone operation placementConsider moving the
clone()operation closer to whereprops.build_external_contractsis actually used to make the code's intent clearer.-if let Some(external_contracts) = props.build_external_contracts.clone() { +let external_contracts = props.build_external_contracts.clone(); +if let Some(external_contracts) = external_contracts {
304-324: Consider extracting dependency mapping logicThe nested mapping operations for collecting dependencies could be made more maintainable by extracting them into a separate function.
+fn collect_component_dependencies( + component_id: &CompilationUnitComponentId, + unit: &CairoCompilationUnit, +) -> Vec<(String, DependencySettings)> { + unit.components + .iter() + .find(|component| component.id == *component_id) + .expect("dependency of a component is guaranteed to exist") + .map(|compilation_unit_component| ( + compilation_unit_component.cairo_package_name().to_string(), + DependencySettings { + discriminator: compilation_unit_component.id.to_discriminator(), + }, + )) + .collect() +} let dependencies = component .dependencies .iter() - .map(|compilation_unit_component_id| { - let compilation_unit_component = unit - .components - .iter() - .find(|component| component.id == *compilation_unit_component_id) - .expect( - "dependency of a component is guaranteed to exist in compilation unit components", - ); - ( - compilation_unit_component.cairo_package_name().to_string(), - DependencySettings { - discriminator: compilation_unit_component.id.to_discriminator(), - }, - ) - }) - .collect(); + .flat_map(|id| collect_component_dependencies(id, unit)) + .collect();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
crates/dojo/core-cairo-test/Scarb.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
bin/sozo/src/commands/test.rs(6 hunks)crates/benches/contracts/src/models/character.cairo(1 hunks)crates/dojo/core-cairo-test/Scarb.toml(1 hunks)crates/dojo/core-cairo-test/src/tests/helpers/helpers.cairo(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/benches/contracts/src/models/character.cairo
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/dojo/core-cairo-test/src/tests/helpers/helpers.cairo
- crates/dojo/core-cairo-test/Scarb.toml
🔇 Additional comments (4)
bin/sozo/src/commands/test.rs (4)
117-138: Simplify Logic for Filtering Compilation Units
The reassignment of is_included at line 125 may lead to confusion. Consider renaming variables or restructuring the logic for better clarity.
183-192: LGTM! Test compiler configuration aligns with Cairo 2.9.1
The configuration properly separates contract and executable crate IDs, which is required for the new Cairo version.
277-279: Ohayo! Clean implementation of discriminator support
The addition of discriminator support in CrateLongId handling improves crate identification and aligns with Cairo 2.9.1's requirements.
Also applies to: 286-291
114-114: Ohayo sensei! Verify Cairo version compatibility
The ignore_cairo_version: false setting ensures strict version checking, which aligns with the Cairo 2.9.1 upgrade. However, we should verify that all dependent packages are compatible with this version.
✅ Verification successful
Ohayo sensei! Cairo version compatibility is verified
All active project packages are correctly set to Cairo version 2.9.1, which aligns with the ignore_cairo_version: false setting. The only exception is in the test data directory (test_data/invalid_cairo_version), which appears to be intentionally set to an older version for testing purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for Cairo version specifications in workspace
rg -g 'Scarb.toml' 'cairo-lang|cairo-version'
Length of output: 477
New
blockifierandstarknet_apirev:92da150dSummary by CodeRabbit
New Features
ignore_cairo_versionparameter to compilation options, allowing for more flexible handling of Cairo versioning during builds.ensure_uniquemethod in bothModelContractandModelContractReaderimplementations for enhanced contract interaction.call_contractto validate contract interactions.Bug Fixes
Dependency Updates
2.9.1, includingcairo,starknet, and others across various packages.world_addressin configuration files to reflect the new address.Documentation
Scarb.tomlfiles to reflect new editions and versions, ensuring compatibility with the latest features.Style Changes