From 36ba2c5522859b9abfb48b7fe1778226b4e4ba43 Mon Sep 17 00:00:00 2001 From: glihm Date: Fri, 12 Jan 2024 12:56:10 -0600 Subject: [PATCH] fix: ensure local manifest and remote manifest models are comparable (#1424) * fix: ensure local manifest and remote manifest are compared with the same models name * add test --------- Co-authored-by: Kariy --- crates/dojo-world/Cargo.toml | 2 +- crates/dojo-world/src/manifest_test.rs | 27 ++++++++++++------ crates/dojo-world/src/migration/world.rs | 16 ++++++++++- crates/dojo-world/src/migration/world_test.rs | 28 +++++++++++++++++-- 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/crates/dojo-world/Cargo.toml b/crates/dojo-world/Cargo.toml index 5f7a534c4e..57d203176a 100644 --- a/crates/dojo-world/Cargo.toml +++ b/crates/dojo-world/Cargo.toml @@ -24,7 +24,7 @@ starknet.workspace = true thiserror.workspace = true tracing.workspace = true -cainome = { git = "https://github.com/cartridge-gg/cainome", rev = "950e487", features = ["abigen-rs"] } +cainome = { git = "https://github.com/cartridge-gg/cainome", rev = "950e487", features = [ "abigen-rs" ] } dojo-types = { path = "../dojo-types", optional = true } http = { version = "0.2.9", optional = true } ipfs-api-backend-hyper = { git = "https://github.com/ferristseng/rust-ipfs-api", rev = "af2c17f7b19ef5b9898f458d97a90055c3605633", features = [ "with-hyper-rustls" ], optional = true } diff --git a/crates/dojo-world/src/manifest_test.rs b/crates/dojo-world/src/manifest_test.rs index a53b262950..1ad5b0d696 100644 --- a/crates/dojo-world/src/manifest_test.rs +++ b/crates/dojo-world/src/manifest_test.rs @@ -12,6 +12,7 @@ use starknet::providers::jsonrpc::{JsonRpcClient, JsonRpcMethod}; use super::{parse_contracts_events, Contract, Manifest, Model}; use crate::contracts::world::test::deploy_world; use crate::manifest::{parse_models_events, ManifestError}; +use crate::migration::world::WorldDiff; #[tokio::test] async fn manifest_from_remote_throw_error_on_not_deployed() { @@ -231,14 +232,24 @@ async fn fetch_remote_manifest() { let account = sequencer.account(); let provider = account.provider(); - let (world_address, _) = deploy_world( - &sequencer, - Utf8PathBuf::from_path_buf("../../examples/spawn-and-move/target/dev".into()).unwrap(), - ) - .await; + let artifacts_path = + Utf8PathBuf::from_path_buf("../../examples/spawn-and-move/target/dev".into()).unwrap(); + let manifest_path = artifacts_path.join("manifest.json"); - let manifest = Manifest::load_from_remote(provider, world_address).await.unwrap(); + let (world_address, _) = deploy_world(&sequencer, artifacts_path).await; - assert_eq!(manifest.models.len(), 2); - assert_eq!(manifest.contracts.len(), 1); + let local_manifest = Manifest::load_from_path(manifest_path).unwrap(); + let remote_manifest = Manifest::load_from_remote(provider, world_address).await.unwrap(); + + assert_eq!(local_manifest.models.len(), 2); + assert_eq!(local_manifest.contracts.len(), 1); + + assert_eq!(remote_manifest.models.len(), 2); + assert_eq!(remote_manifest.contracts.len(), 1); + + // compute diff from local and remote manifest + + let diff = WorldDiff::compute(local_manifest, Some(remote_manifest)); + + assert_eq!(diff.count_diffs(), 0, "there should not be any diff"); } diff --git a/crates/dojo-world/src/migration/world.rs b/crates/dojo-world/src/migration/world.rs index 60aa94c68a..a7d8a9cb25 100644 --- a/crates/dojo-world/src/migration/world.rs +++ b/crates/dojo-world/src/migration/world.rs @@ -1,5 +1,7 @@ use std::fmt::Display; +use convert_case::{Case, Casing}; + use super::class::ClassDiff; use super::contract::ContractDiff; use super::StateDiff; @@ -28,7 +30,19 @@ impl WorldDiff { name: model.name.to_string(), local: model.class_hash, remote: remote.as_ref().and_then(|m| { - m.models.iter().find(|e| e.name == model.name).map(|s| s.class_hash) + // Remote models are detected from events, where only the struct + // name (pascal case) is emitted. + // Local models uses the fully qualified name of the model, + // always in snake_case from cairo compiler. + let model_name = model + .name + .split("::") + .last() + .unwrap_or(&model.name) + .from_case(Case::Snake) + .to_case(Case::Pascal); + + m.models.iter().find(|e| e.name == model_name).map(|s| s.class_hash) }), }) .collect::>(); diff --git a/crates/dojo-world/src/migration/world_test.rs b/crates/dojo-world/src/migration/world_test.rs index 28dc999e8e..698500f401 100644 --- a/crates/dojo-world/src/migration/world_test.rs +++ b/crates/dojo-world/src/migration/world_test.rs @@ -26,13 +26,22 @@ fn no_diff_when_local_and_remote_are_equal() { ..Default::default() }]; + let remote_models = vec![Model { + members: vec![], + name: "Model".into(), + class_hash: 11_u32.into(), + ..Default::default() + }]; + let local = Manifest { models, world: world_contract, executor: executor_contract, ..Default::default() }; - let remote = local.clone(); + + let mut remote = local.clone(); + remote.models = remote_models; let diff = WorldDiff::compute(local, Some(remote)); @@ -68,6 +77,21 @@ fn diff_when_local_and_remote_are_different() { }, ]; + let remote_models = vec![ + Model { + members: vec![], + name: "Model".into(), + class_hash: felt!("0x11"), + ..Default::default() + }, + Model { + members: vec![], + name: "Model2".into(), + class_hash: felt!("0x33"), + ..Default::default() + }, + ]; + let contracts = vec![ Contract { name: "dojo_mock::contracts::my_contract".into(), @@ -92,9 +116,9 @@ fn diff_when_local_and_remote_are_different() { }; let mut remote = local.clone(); + remote.models = remote_models; remote.world.class_hash = 44_u32.into(); remote.executor.class_hash = 55_u32.into(); - remote.models[1].class_hash = 33_u32.into(); remote.contracts[0].class_hash = felt!("0x1112"); let diff = WorldDiff::compute(local, Some(remote));