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 <[email protected]>
dojoengine · Jan 12, 2024 · 36ba2c5 · 36ba2c5
1 parent 39d0ba4
commit 36ba2c5
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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::<Vec<_>>();

diff --git 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));