tests: add tests for migration #2614
Conversation
WalkthroughOhayo, sensei! This pull request introduces several changes across multiple files, primarily focusing on the migration process and UI enhancements. Key modifications include updating 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: 4
🧹 Outside diff range and nitpick comments (5)
crates/sozo/ops/src/tests/mod.rs (1)
1-5: Ohayo sensei! Consider tracking these TODOs as GitHub issues.These TODO comments indicate several test modules that need to be implemented. To better track and prioritize this work, it would be helpful to create GitHub issues for each pending module.
Would you like me to help create GitHub issues for tracking these pending test modules (auth, call, model, utils)?
examples/spawn-and-move/src/others.cairo (1)
Line range hint
8-12: Ohayo sensei! Consider a more descriptive event name.While the implementation is correct, the name
MyInitis quite generic. Consider a more descriptive name that reflects its purpose, such asOthersModuleInitializedorOthersContractInitialized.crates/sozo/ops/src/migration_ui.rs (1)
24-32: Clean implementation with thoughtful defaults!The implementation elegantly handles both cases:
- When text is provided: Animated frames with shorter duration (500ms) for visual feedback
- When None: Single empty frame with longer duration (5000ms) for minimal overhead
However, consider extracting the magic numbers (500, 5000) into named constants for better maintainability.
Apply this improvement:
pub struct MigrationUi { spinner: Spinner, default_frames: SpinnerFrames, silent: bool, } +const ANIMATED_FRAME_DURATION_MS: u64 = 500; +const EMPTY_FRAME_DURATION_MS: u64 = 5000; + impl MigrationUi { pub fn new(text: Option<&'static str>) -> Self { if let Some(text) = text { - let frames = spinner!(["⛩️ ", "🎃", "👻", "🧟", "💀"], 500); + let frames = spinner!(["⛩️ ", "🎃", "👻", "🧟", "💀"], ANIMATED_FRAME_DURATION_MS); let spinner = Spinner::new(frames.clone(), text, None); Self { spinner, default_frames: frames, silent: false } } else { - let frames = spinner!([""], 5000); + let frames = spinner!([""], EMPTY_FRAME_DURATION_MS); let spinner = Spinner::new(frames.clone(), "", None); Self { spinner, default_frames: frames, silent: false } } }crates/dojo/world/src/remote/events_to_remote.rs (1)
Line range hint
189-199: Ohayo sensei! The logging improvements look great!The added trace logging for contract initialization events enhances observability. The implementation is sound with proper error handling and safe unwraps.
Consider using the debug format specifier
{:?}for the selector as well, to maintain consistency with the init_calldata formatting:- selector = format!("{:#066x}", e.selector), + selector = format!("{:?}", e.selector),crates/sozo/ops/src/migrate/mod.rs (1)
214-216: Consider simplifying the string formatting, sensei!The multi-line string formatting could be more concise.
- let ui_text = - format!("Initializing {} contracts (sequentially)...", invoker.calls.len()); + let ui_text = format!("Initializing {} contracts (sequentially)...", invoker.calls.len());
📜 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 (25)
bin/sozo/src/commands/execute.rs(1 hunks)bin/sozo/src/commands/migrate.rs(1 hunks)crates/dojo/world/src/remote/events_to_remote.rs(1 hunks)crates/sozo/ops/Cargo.toml(1 hunks)crates/sozo/ops/src/migrate/mod.rs(1 hunks)crates/sozo/ops/src/migration_ui.rs(1 hunks)crates/sozo/ops/src/tests/migration.rs(1 hunks)crates/sozo/ops/src/tests/mod.rs(1 hunks)crates/torii/types-test/overlays/dev/records.toml(0 hunks)examples/game-lib/armory/manifests/dev/base/abis/dojo-base.json(0 hunks)examples/game-lib/armory/manifests/dev/base/abis/dojo-world.json(0 hunks)examples/game-lib/armory/manifests/dev/base/abis/models/armory-ModelA-19290fb7.json(0 hunks)examples/game-lib/armory/manifests/dev/base/dojo-base.toml(0 hunks)examples/game-lib/armory/manifests/dev/base/dojo-world.toml(0 hunks)examples/game-lib/armory/manifests/dev/base/models/armory-ModelA-19290fb7.toml(0 hunks)examples/spawn-and-move/manifest_dev.json(6 hunks)examples/spawn-and-move/manifests/dev/deployment/manifest.toml(0 hunks)examples/spawn-and-move/overlays/dev/actions.toml(0 hunks)examples/spawn-and-move/overlays/dev/mock_token.toml(0 hunks)examples/spawn-and-move/overlays/dev/others.toml(0 hunks)examples/spawn-and-move/overlays/saya/actions.toml(0 hunks)examples/spawn-and-move/overlays/saya/others.toml(0 hunks)examples/spawn-and-move/src/others.cairo(2 hunks)scripts/rebuild_test_artifacts.sh(1 hunks)xtask/generate-test-db/src/main.rs(2 hunks)
💤 Files with no reviewable changes (13)
- crates/torii/types-test/overlays/dev/records.toml
- examples/game-lib/armory/manifests/dev/base/abis/dojo-base.json
- examples/game-lib/armory/manifests/dev/base/abis/dojo-world.json
- examples/game-lib/armory/manifests/dev/base/abis/models/armory-ModelA-19290fb7.json
- examples/game-lib/armory/manifests/dev/base/dojo-base.toml
- examples/game-lib/armory/manifests/dev/base/dojo-world.toml
- examples/game-lib/armory/manifests/dev/base/models/armory-ModelA-19290fb7.toml
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml
- examples/spawn-and-move/overlays/dev/actions.toml
- examples/spawn-and-move/overlays/dev/mock_token.toml
- examples/spawn-and-move/overlays/dev/others.toml
- examples/spawn-and-move/overlays/saya/actions.toml
- examples/spawn-and-move/overlays/saya/others.toml
🔇 Additional comments (19)
crates/sozo/ops/src/tests/mod.rs (1)
7-7: LGTM! The migration module inclusion aligns with the PR objective.
The uncommented migration module makes perfect sense as it supports the PR's goal of adding migration tests.
Let's verify the migration test file exists:
✅ Verification successful
Migration test file exists as expected
The verification confirms the presence of migration.rs at the expected location: crates/sozo/ops/src/tests/migration.rs, which aligns perfectly with the module inclusion.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the migration test file exists
fd --type f "migration.rs$" "crates/sozo/ops/src/tests"
Length of output: 94
examples/spawn-and-move/src/others.cairo (1)
17-17: Verify event consumers are updated.
The event rename from ContractInitialized to MyInit looks good, but we should ensure all consumers are updated.
crates/sozo/ops/Cargo.toml (1)
39-41: Ohayo! These dependency additions look perfect, sensei! ٩(◕‿◕｡)۶
The new workspace dependencies align perfectly with the migration testing objectives:
tokiofor handling async operationsscarbfor package managementsozo-scarbextfor Scarb extensions
scripts/rebuild_test_artifacts.sh (1)
Line range hint 1-43: Verify script execution directory assumptions, sensei!
The script uses several relative paths and assumes it's being run from the repository root.
Let's verify if there are any other scripts that might call this one from a different directory:
✅ Verification successful
Ohayo! The script's directory assumptions are safe, sensei!
The script is only referenced in the DEVELOPMENT.md documentation, which explicitly instructs users to run it with bash scripts/rebuild_test_artifacts.sh from the repository root. No other scripts or workflows call this script, making its relative path assumptions safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any references to rebuild_test_artifacts.sh
rg -l "rebuild_test_artifacts.sh"
# Check if this script is referenced in any GitHub Actions workflows
fd -e yml -e yaml . .github/workflows/ --exec grep -l "rebuild_test_artifacts.sh" {} \;
Length of output: 139
Script:
#!/bin/bash
# Let's check the content of DEVELOPMENT.md to see how this script is referenced
cat DEVELOPMENT.md
# Let's also check if there are any other shell scripts that might be sourcing this one
fd -e sh . --exec grep -l "source.*rebuild_test_artifacts.sh" {} \;
fd -e sh . --exec grep -l "\. .*rebuild_test_artifacts.sh" {} \;
# Check for any shell scripts that might be executing this script
fd -e sh . --exec grep -l "bash.*rebuild_test_artifacts.sh" {} \;
fd -e sh . --exec grep -l "./scripts/rebuild_test_artifacts.sh" {} \;
Length of output: 3078
crates/sozo/ops/src/migration_ui.rs (1)
23-23: Ohayo! Nice API improvement, sensei!
The change to Option<&'static str> makes the API more explicit and flexible by allowing callers to clearly indicate when no text is needed.
bin/sozo/src/commands/migrate.rs (2)
51-51: Ohayo sensei! The change to Option looks good, but let's verify consistency.
The modification to use Some("Evaluating world diff...") aligns well with making the UI messages optional. This change improves flexibility by allowing the UI to handle cases where no message is needed.
Let's check if this pattern is consistently applied across the codebase:
✅ Verification successful
Ohayo sensei! The Option change is consistently implemented across the codebase!
All instances of MigrationUi::new are using the Option parameter correctly:
xtask/generate-test-db/src/main.rs: UsesNonewith silent modecrates/sozo/ops/src/tests/migration.rs: UsesNonefor testsbin/sozo/src/commands/migrate.rs: UsesSome("Evaluating world diff...")bin/sozo/src/commands/execute.rs: UsesNonewith silent mode
The change is properly aligned across all usages, with each context appropriately choosing between Some and None based on its UI needs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify MigrationUi::new usage pattern across the codebase
# Expected: All instances should use Option<String> parameter
# Search for all MigrationUi::new usages
ast-grep --pattern 'MigrationUi::new($_)'
Length of output: 1143
Script:
#!/bin/bash
# Let's try a different approach using ripgrep to find MigrationUi::new usages
rg "MigrationUi::new" -A 2 -B 2
Length of output: 1679
51-51: Consider adding tests for the new Option behavior.
Given that this PR's objective is to add tests for migration, we should ensure the new optional message behavior is properly tested.
Let's check if there are any existing tests for this functionality:
Would you like me to help generate test cases for the optional message behavior in MigrationUi?
bin/sozo/src/commands/execute.rs (1)
83-83: Ohayo! The MigrationUi initialization looks good, sensei!
The change from empty string to None improves type safety and makes the intent clearer.
xtask/generate-test-db/src/main.rs (2)
59-59: Ohayo! The migration UI change looks good, sensei!
The transition from an empty string to None aligns well with the new optional message approach in MigrationUi. The with_silent() chaining maintains the existing behavior while using the new API.
106-106: The same clean migration UI update here too, sensei!
Consistent with the previous change, maintaining uniformity across both test migrations.
crates/sozo/ops/src/migrate/mod.rs (2)
207-209: Ohayo! The UI feedback for multicall initialization looks good, sensei!
The code provides clear feedback about the number of contracts being initialized in multicall mode.
207-218: Ohayo! The initialization feedback improvements look solid overall, sensei!
The changes effectively handle both multicall and sequential initialization scenarios while providing clear user feedback. The error handling is robust, and the code follows Rust best practices.
examples/spawn-and-move/manifest_dev.json (6)
6-7: LGTM! World configuration looks good.
The world name and seed are properly configured with clear, descriptive values.
8-1120: Ohayo sensei! Comprehensive world ABI implementation looks solid.
The ABI definitions are well-structured and include all necessary components for the Dojo world:
- Core types (ByteArray, Resource, etc.)
- World interfaces (IWorld, IUpgradeableWorld)
- Event definitions
- Access control functions
Line range hint 1124-1441: Contract configuration for actions system is well-defined.
The actions contract is properly configured with:
- Correct address and class hash
- Complete ABI including implementations, interfaces, and events
- Proper namespace prefix in tag (ns-actions)
Line range hint 1626-1785: Mock token contract configuration follows best practices.
The mock token contract configuration maintains consistency with other contracts:
- Follows the same ABI structure
- Uses consistent namespace prefix (ns-mock_token)
- Properly defined events and interfaces
1994-2000: Event configuration update looks good.
The events have been properly updated:
- ContractInitialized renamed to Moved
- New MyInit event added
- Class hashes are properly configured
#!/bin/bash
# Verify the event name changes in the codebase
echo "Checking for old event name usage..."
rg "ContractInitialized" --type cairo
echo "Checking for new event names..."
rg "MyInit|Moved" --type cairoLine range hint 1789-1955: Verify initialization value for others contract.
The others contract has an initialization value of 0xff. Please confirm if this is the intended value for production.
crates/sozo/ops/src/tests/migration.rs (1)
74-78: Great job on the migrate_no_change test
Ohayo sensei! The migrate_no_change test correctly verifies that no changes occur when the state is unchanged. This is a valuable addition to ensure stability in migrations.
| @@ -36,7 +36,7 @@ cargo +nightly-2024-08-28 fmt --all -- "$@" | |||
| ./target/release/sozo build --manifest-path crates/torii/types-test/Scarb.toml | |||
|
|
|||
| # Generates the database for testing by migrating the spawn and move example. | |||
| cargo generate-test-db | |||
| KATANA_RUNNER_BIN=./target/release/katana cargo generate-test-db | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo! Consider using an absolute path for KATANA_RUNNER_BIN.
The change to use the local katana binary is good, but using a relative path could be fragile if the script is executed from a different directory.
Consider using:
-KATANA_RUNNER_BIN=./target/release/katana cargo generate-test-db
+KATANA_RUNNER_BIN="$(pwd)/target/release/katana" cargo generate-test-db📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| KATANA_RUNNER_BIN=./target/release/katana cargo generate-test-db | |
| KATANA_RUNNER_BIN="$(pwd)/target/release/katana" cargo generate-test-db |
| @@ -56,7 +56,7 @@ async fn migrate_spawn_and_move(db_path: &Path) -> Result<Manifest> { | |||
| profile_config, | |||
| runner.url().to_string(), | |||
| ) | |||
| .migrate(&mut MigrationUi::new("").with_silent()) | |||
| .migrate(&mut MigrationUi::new(None).with_silent()) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting the common migration UI configuration, sensei!
Both test functions use identical MigrationUi configuration. We could improve maintainability by extracting this into a helper function.
Here's a suggested implementation:
fn create_silent_migration_ui() -> MigrationUi {
MigrationUi::new(None).with_silent()
}Then use it in both functions:
- .migrate(&mut MigrationUi::new(None).with_silent())
+ .migrate(&mut create_silent_migration_ui())Also applies to: 106-106
| let ws = scarb::ops::read_workspace(config.manifest_path(), &config).unwrap(); | ||
|
|
||
| #[tokio::test(flavor = "multi_thread")] | ||
| #[katana_runner::test(accounts = 10)] | ||
| async fn migrate_with_auto_mine(sequencer: &RunnerCtx) { | ||
| let config = setup::load_config(); | ||
| let ws = setup::setup_ws(&config); | ||
| let world_local = ws.load_world_local().unwrap(); | ||
| let world_address = world_local.deterministic_world_address().unwrap(); |
There was a problem hiding this comment.
Consider handling errors instead of using unwrap()
Ohayo sensei! Using unwrap() can cause the program to panic if an error occurs. It's better to handle possible errors gracefully using the ? operator to propagate them, enhancing the robustness of the code.
Apply this diff to improve error handling:
-let ws = scarb::ops::read_workspace(config.manifest_path(), &config).unwrap();
+let ws = scarb::ops::read_workspace(config.manifest_path(), &config)?;
-let world_local = ws.load_world_local().unwrap();
+let world_local = ws.load_world_local()?;
-let world_address = world_local.deterministic_world_address().unwrap();
+let world_address = world_local.deterministic_world_address()?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let ws = scarb::ops::read_workspace(config.manifest_path(), &config).unwrap(); | |
| #[tokio::test(flavor = "multi_thread")] | |
| #[katana_runner::test(accounts = 10)] | |
| async fn migrate_with_auto_mine(sequencer: &RunnerCtx) { | |
| let config = setup::load_config(); | |
| let ws = setup::setup_ws(&config); | |
| let world_local = ws.load_world_local().unwrap(); | |
| let world_address = world_local.deterministic_world_address().unwrap(); | |
| let ws = scarb::ops::read_workspace(config.manifest_path(), &config)?; | |
| let world_local = ws.load_world_local()?; | |
| let world_address = world_local.deterministic_world_address()?; |
|
|
||
| assert_eq!(local_manifest.world.inner.class_hash, remote_manifest.world.inner.class_hash); | ||
| assert_eq!(local_manifest.models.len(), remote_manifest.models.len()); | ||
| migration.migrate(&mut ui).await.expect("Migration spawn-and-move failed.") |
There was a problem hiding this comment.
Handle migration errors without panicking
Ohayo sensei! Using expect() will cause a panic if the migration fails. Consider handling the error gracefully to improve robustness and provide better error reporting.
Apply this diff to handle the error:
-migration.migrate(&mut ui).await.expect("Migration spawn-and-move failed.")
+if let Err(e) = migration.migrate(&mut ui).await {
+ eprintln!("Migration failed: {:?}", e);
+ return Err(e.into());
+}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 (1)
bin/sozo/tests/test_data/policies.json (1)
Line range hint
1-999: Consider organizing test policies by functionality, sensei.The file contains a mix of different policy types (spawn, move, upgrade, etc.). Consider organizing these policies into logical groups within the test data, perhaps using separate files or adding comments to clearly indicate the purpose of each policy group.
Consider structuring the test data like this:
{ "core_policies": [ // Core system policies ], "game_policies": [ // Game-specific policies ], "upgrade_policies": [ // All upgrade-related policies ] }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
bin/sozo/tests/test_data/policies.json(1 hunks)
🔇 Additional comments (1)
bin/sozo/tests/test_data/policies.json (1)
35-37: Ohayo! Verify duplicate upgrade policies, sensei.
I notice we have multiple entries for the "upgrade" method with different target addresses. While this might be intentional for testing purposes, we should verify that this doesn't cause any conflicts in the test scenarios.
Let's check for all upgrade method entries:
✅ Verification successful
Ohayo! Multiple upgrade policies are expected in test data, sensei!
The presence of multiple upgrade policies with different target addresses in the test data file is intentional and valid. This is common in test scenarios to verify different upgrade paths and configurations. Each target address is unique, and there are no duplicates that could cause conflicts:
- 0x7bc340...34d4
- 0x200cd8...43d1 (newly added)
- 0x2e7d50...4a8
- 0x41ceb7...9db
- 0x234d35...c32
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all upgrade method entries and their targets
echo "Checking all upgrade method policies..."
jq '.[] | select(.method == "upgrade") | .target' bin/sozo/tests/test_data/policies.json
Length of output: 519
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2614 +/- ##
==========================================
+ Coverage 55.65% 57.94% +2.28%
==========================================
Files 393 393
Lines 48566 48567 +1
==========================================
+ Hits 27029 28141 +1112
+ Misses 21537 20426 -1111 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (3)
bin/sozo/src/commands/migrate.rs(1 hunks)crates/sozo/ops/src/migration_ui.rs(1 hunks)examples/spawn-and-move/manifest_dev.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- bin/sozo/src/commands/migrate.rs
- crates/sozo/ops/src/migration_ui.rs
🧰 Additional context used
🪛 Biome
examples/spawn-and-move/manifest_dev.json
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: expected , but instead found HEAD
Remove HEAD
(parse)
[error] 1790-1790: expected , but instead found "address"
Remove "address"
(parse)
[error] 1790-1790: expected , but instead found :
Remove :
(parse)
[error] 1790-1790: expected , but instead found "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1"
Remove "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1"
(parse)
[error] 1791-1791: expected , but instead found :
Remove :
(parse)
[error] 1791-1791: expected , but instead found "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e"
Remove "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e"
(parse)
[error] 1791-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1793-1793: expected , but instead found "address"
Remove "address"
(parse)
[error] 1793-1793: expected , but instead found :
Remove :
(parse)
[error] 1793-1793: expected , but instead found "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792"
Remove "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792"
(parse)
[error] 1794-1794: expected , but instead found :
Remove :
(parse)
[error] 1794-1794: expected , but instead found "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88"
Remove "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88"
(parse)
[error] 1794-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: expected , but instead found dojo
Remove dojo
(parse)
[error] 1795-1795: unexpected character /
(parse)
[error] 1795-1795: expected , but instead found main
Remove main
(parse)
🔇 Additional comments (2)
examples/spawn-and-move/manifest_dev.json (2)
Line range hint 1789-2005: Verify dojo_init parameter change impact.
The dojo_init function in the "others" contract now accepts a value parameter of type u8. This change is reflected in the init_calldata which now contains ["0xff"].
Let's verify this change is properly implemented across the codebase:
#!/bin/bash
# Description: Verify dojo_init implementation consistency
# Search for dojo_init implementations and their parameter lists
echo "Searching for dojo_init implementations..."
ast-grep --pattern 'fn dojo_init($$$) {
$$$
}'🧰 Tools
🪛 Biome
[error] 1788-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: expected , but instead found HEAD
Remove HEAD
(parse)
[error] 1790-1790: expected , but instead found "address"
Remove "address"
(parse)
[error] 1790-1790: expected , but instead found :
Remove :
(parse)
[error] 1790-1790: expected , but instead found "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1"
Remove "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1"
(parse)
[error] 1791-1791: expected , but instead found :
Remove :
(parse)
[error] 1791-1791: expected , but instead found "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e"
Remove "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e"
(parse)
[error] 1791-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1793-1793: expected , but instead found "address"
Remove "address"
(parse)
[error] 1793-1793: expected , but instead found :
Remove :
(parse)
[error] 1793-1793: expected , but instead found "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792"
Remove "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792"
(parse)
[error] 1794-1794: expected , but instead found :
Remove :
(parse)
[error] 1794-1794: expected , but instead found "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88"
Remove "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88"
(parse)
[error] 1794-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: expected , but instead found dojo
Remove dojo
(parse)
[error] 1795-1795: unexpected character /
(parse)
[error] 1795-1795: expected , but instead found main
Remove main
(parse)
[error] 1796-1796: expected , but instead found "abi"
Remove "abi"
(parse)
[error] 1796-1796: expected , but instead found :
Remove :
(parse)
[error] 1796-1796: expected , but instead found [
Remove [
(parse)
1999-2005: Event changes need verification.
The manifest shows two significant event changes:
- Event with class hash
0x22c33f3c7e525a21cd3651326f74c61deff6d75e26e06725f53ac2dfd4646fais now tagged as "Moved" (previously "ContractInitialized") - New event with class hash
0x2c78ad5ec05d05be24a1cbace308ad15bff9375b56be8a8fa892ff5fc79eb23is tagged as "MyInit"
Let's verify these event changes are consistent with the codebase:
✅ Verification successful
Ohayo sensei! Based on the verification results, I can now provide a conclusive response:
Event changes are valid and properly implemented
The changes in the manifest are consistent with the codebase:
- The new "MyInit" event is properly implemented in
others.cairowith appropriate structure and emitted during initialization - The "Moved" tag replacing "ContractInitialized" is valid as ContractInitialized is a core system event (found in
world_contract.cairo), not a custom game event
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the event changes in the codebase
# Look for references to these events in the code
# Search for Moved event usage
echo "Searching for Moved event usage..."
rg -l "Moved"
# Search for MyInit event usage
echo "Searching for MyInit event usage..."
rg -l "MyInit"
# Search for any remaining references to ContractInitialized
echo "Checking for any remaining ContractInitialized references..."
rg -l "ContractInitialized"
Length of output: 817
Script:
#!/bin/bash
# Let's check the implementation details of these events
# Check the implementation of MyInit in others.cairo
echo "Checking MyInit implementation in others.cairo..."
rg -A 10 "MyInit" "examples/spawn-and-move/src/others.cairo"
# Check for Moved event implementation
echo "Checking Moved event implementation..."
rg -A 10 "Moved" "examples/spawn-and-move/src/others.cairo"
# Check the context around ContractInitialized in world_contract.cairo
echo "Checking ContractInitialized context in world contract..."
rg -A 10 "ContractInitialized" "crates/dojo/core/src/world/world_contract.cairo"
Length of output: 1817
| <<<<<<< HEAD | ||
| "address": "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1", | ||
| "class_hash": "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e", | ||
| ======= | ||
| "address": "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792", | ||
| "class_hash": "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88", | ||
| >>>>>>> dojo/main |
There was a problem hiding this comment.
Ohayo sensei! Please resolve the merge conflict.
There's an unresolved merge conflict in the "others" contract configuration. The conflict shows different values for both the address and class_hash:
HEAD version:
- address: 0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1
- class_hash: 0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e
dojo/main version:
- address: 0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792
- class_hash: 0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88
Please resolve this conflict by choosing the appropriate version based on your migration requirements.
🧰 Tools
🪛 Biome
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: unexpected character <
(parse)
[error] 1789-1789: expected , but instead found HEAD
Remove HEAD
(parse)
[error] 1790-1790: expected , but instead found "address"
Remove "address"
(parse)
[error] 1790-1790: expected , but instead found :
Remove :
(parse)
[error] 1790-1790: expected , but instead found "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1"
Remove "0x200cd8070a7dbe0894c74426d2f4c9d778b13042ce955203cb189d85cfb43d1"
(parse)
[error] 1791-1791: expected , but instead found :
Remove :
(parse)
[error] 1791-1791: expected , but instead found "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e"
Remove "0x4f290c42a0a2e342bf8da74103a31c6e3f2ebca2c72054da9d6d24754c3cc7e"
(parse)
[error] 1791-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1792-1792: unexpected character =
(parse)
[error] 1793-1793: expected , but instead found "address"
Remove "address"
(parse)
[error] 1793-1793: expected , but instead found :
Remove :
(parse)
[error] 1793-1793: expected , but instead found "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792"
Remove "0x79e0653fbebdbdb864ca69d1470b263f2efdfce9cf355cfe9c7719627eff792"
(parse)
[error] 1794-1794: expected , but instead found :
Remove :
(parse)
[error] 1794-1794: expected , but instead found "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88"
Remove "0x3356ff99134a997be6f9366c47ad18993aac3e29803819cc80309a57fa23f88"
(parse)
[error] 1794-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: unexpected character >
(parse)
[error] 1795-1795: expected , but instead found dojo
Remove dojo
(parse)
[error] 1795-1795: unexpected character /
(parse)
[error] 1795-1795: expected , but instead found main
Remove main
(parse)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores