fix(migrate): when init_calldata depends on contract that was alrea…

…dy deployed (#2058)

* fix(migrate): when `init_calldata` depends on contract that was already deployed

* add tests

crates/dojo-world/src/migration/strategy.rs

@@ -13,13 +13,19 @@ use super::contract::{ContractDiff, ContractMigration};
 use super::world::WorldDiff;
 use super::MigrationType;
 
+#[derive(Debug, Clone)]
+pub enum MigrationMetadata {
+    Contract(ContractDiff),
+}
+
 #[derive(Debug, Clone)]
 pub struct MigrationStrategy {
     pub world_address: Option<FieldElement>,
     pub world: Option<ContractMigration>,
     pub base: Option<ClassMigration>,
     pub contracts: Vec<ContractMigration>,
     pub models: Vec<ClassMigration>,
+    pub metadata: HashMap<String, MigrationMetadata>,
 }
 
 #[derive(Debug)]
@@ -61,23 +67,29 @@ impl MigrationStrategy {
     }
 
     pub fn resolve_variable(&mut self, world_address: FieldElement) -> Result<()> {
-        let contracts_clone = self.contracts.clone();
         for contract in self.contracts.iter_mut() {
             for field in contract.diff.init_calldata.iter_mut() {
                 if let Some(dependency) = field.strip_prefix("$contract_address:") {
-                    let dependency_contract =
-                        contracts_clone.iter().find(|c| c.diff.name == dependency).unwrap();
-                    let contract_address = get_contract_address(
-                        generate_salt(&dependency_contract.diff.name),
-                        dependency_contract.diff.base_class_hash,
-                        &[],
-                        world_address,
-                    );
-                    *field = contract_address.to_string();
+                    let dependency_contract = self.metadata.get(dependency).unwrap();
+
+                    match dependency_contract {
+                        MigrationMetadata::Contract(c) => {
+                            let contract_address = get_contract_address(
+                                generate_salt(&c.name),
+                                c.base_class_hash,
+                                &[],
+                                world_address,
+                            );
+                            *field = contract_address.to_string();
+                        }
+                    }
                 } else if let Some(dependency) = field.strip_prefix("$class_hash:") {
-                    let dependency_contract =
-                        contracts_clone.iter().find(|c| c.diff.name == dependency).unwrap();
-                    *field = dependency_contract.diff.local_class_hash.to_string();
+                    let dependency_contract = self.metadata.get(dependency).unwrap();
+                    match dependency_contract {
+                        MigrationMetadata::Contract(c) => {
+                            *field = c.local_class_hash.to_string();
+                        }
+                    }
                 }
             }
         }
@@ -94,6 +106,7 @@ pub fn prepare_for_migration(
     target_dir: &Utf8PathBuf,
     diff: WorldDiff,
 ) -> Result<MigrationStrategy> {
+    let mut metadata = HashMap::new();
     let entries = fs::read_dir(target_dir).with_context(|| {
         format!(
             "Failed trying to read target directory ({target_dir})\nNOTE: build files are profile \
@@ -122,8 +135,12 @@ pub fn prepare_for_migration(
     // else we need to evaluate which contracts need to be migrated.
     let mut world = evaluate_contract_to_migrate(&diff.world, &artifact_paths, false)?;
     let base = evaluate_class_to_migrate(&diff.base, &artifact_paths, world.is_some())?;
-    let contracts =
-        evaluate_contracts_to_migrate(&diff.contracts, &artifact_paths, world.is_some())?;
+    let contracts = evaluate_contracts_to_migrate(
+        &diff.contracts,
+        &artifact_paths,
+        &mut metadata,
+        world.is_some(),
+    )?;
     let models = evaluate_models_to_migrate(&diff.models, &artifact_paths, world.is_some())?;
 
     // If world needs to be migrated, then we expect the `seed` to be provided.
@@ -151,7 +168,7 @@ pub fn prepare_for_migration(
         world.contract_address = generated_world_address;
     }
 
-    Ok(MigrationStrategy { world_address, world, base, contracts, models })
+    Ok(MigrationStrategy { world_address, world, base, contracts, models, metadata })
 }
 
 fn evaluate_models_to_migrate(
@@ -191,11 +208,13 @@ fn evaluate_class_to_migrate(
 fn evaluate_contracts_to_migrate(
     contracts: &[ContractDiff],
     artifact_paths: &HashMap<String, PathBuf>,
+    metadata: &mut HashMap<String, MigrationMetadata>,
     world_contract_will_migrate: bool,
 ) -> Result<Vec<ContractMigration>> {
     let mut comps_to_migrate = vec![];
 
     for c in contracts {
+        metadata.insert(c.name.clone(), MigrationMetadata::Contract(c.clone()));
         match c.remote_class_hash {
             Some(remote) if remote == c.local_class_hash && !world_contract_will_migrate => {
                 continue;

crates/sozo/ops/src/tests/migration.rs

@@ -12,7 +12,7 @@ use dojo_world::metadata::{
     dojo_metadata_from_workspace, ArtifactMetadata, DojoMetadata, Uri, WorldMetadata,
     IPFS_CLIENT_URL, IPFS_PASSWORD, IPFS_USERNAME,
 };
-use dojo_world::migration::strategy::prepare_for_migration;
+use dojo_world::migration::strategy::{prepare_for_migration, MigrationMetadata};
 use dojo_world::migration::world::WorldDiff;
 use dojo_world::migration::TxnConfig;
 use futures::TryStreamExt;
@@ -91,6 +91,113 @@ async fn migrate_with_small_fee_multiplier_will_fail() {
     );
 }
 
+#[tokio::test]
+async fn metadata_calculated_properly() {
+    let config = setup::load_config();
+    let ws = setup::setup_ws(&config);
+
+    let base = config.manifest_path().parent().unwrap();
+    let target_dir = format!("{}/target/dev", base);
+
+    let profile_name = ws.current_profile().unwrap().to_string();
+
+    let mut manifest = BaseManifest::load_from_path(
+        &base.to_path_buf().join(MANIFESTS_DIR).join(profile_name).join(BASE_DIR),
+    )
+    .unwrap();
+
+    let overlay_manifest =
+        OverlayManifest::load_from_path(&base.join(MANIFESTS_DIR).join("dev").join(OVERLAYS_DIR))
+            .unwrap();
+
+    manifest.merge(overlay_manifest);
+
+    let world = WorldDiff::compute(manifest, None);
+
+    let migration = prepare_for_migration(
+        None,
+        felt!("0x12345"),
+        &Utf8Path::new(&target_dir).to_path_buf(),
+        world,
+    )
+    .unwrap();
+
+    // verifies that key name and actual item name are same
+    for (key, value) in migration.metadata.iter() {
+        match value {
+            MigrationMetadata::Contract(c) => {
+                assert_eq!(key, &c.name);
+            }
+        }
+    }
+}
+
+#[tokio::test]
+async fn migration_with_correct_calldata_second_time_work_as_expected() {
+    let config = setup::load_config();
+    let ws = setup::setup_ws(&config);
+
+    let base = config.manifest_path().parent().unwrap();
+    let target_dir = format!("{}/target/dev", base);
+
+    let sequencer = KatanaRunner::new().expect("Failed to start runner.");
+
+    let account = sequencer.account(0);
+
+    let profile_name = ws.current_profile().unwrap().to_string();
+
+    let mut manifest = BaseManifest::load_from_path(
+        &base.to_path_buf().join(MANIFESTS_DIR).join(&profile_name).join(BASE_DIR),
+    )
+    .unwrap();
+
+    let world = WorldDiff::compute(manifest.clone(), None);
+
+    let migration = prepare_for_migration(
+        None,
+        felt!("0x12345"),
+        &Utf8Path::new(&target_dir).to_path_buf(),
+        world,
+    )
+    .unwrap();
+
+    let migration_output =
+        execute_strategy(&ws, &migration, &account, TxnConfig::init_wait()).await.unwrap();
+
+    // first time others will fail due to calldata error
+    assert!(!migration_output.full);
+
+    let world_address = migration_output.world_address;
+
+    let remote_manifest = DeploymentManifest::load_from_remote(sequencer.provider(), world_address)
+        .await
+        .expect("Failed to load remote manifest");
+
+    let overlay = OverlayManifest::load_from_path(
+        &base.join(MANIFESTS_DIR).join(&profile_name).join(OVERLAYS_DIR),
+    )
+    .expect("Failed to load overlay");
+
+    // adding correct calldata
+    manifest.merge(overlay);
+
+    let mut world = WorldDiff::compute(manifest, Some(remote_manifest));
+    world.update_order().expect("Failed to update order");
+
+    let mut migration = prepare_for_migration(
+        Some(world_address),
+        felt!("0x12345"),
+        &Utf8Path::new(&target_dir).to_path_buf(),
+        world,
+    )
+    .unwrap();
+    migration.resolve_variable(migration.world_address().unwrap()).expect("Failed to resolve");
+
+    let migration_output =
+        execute_strategy(&ws, &migration, &account, TxnConfig::init_wait()).await.unwrap();
+    assert!(migration_output.full);
+}
+
 #[tokio::test]
 async fn migration_from_remote() {
     let config = setup::load_config();

crates/torii/types-test/Scarb.lock

@@ -15,7 +15,7 @@ source = "git+https://github.com/dojoengine/dojo?tag=v0.3.11#1e651b5d4d3b79b14a7
 
 [[package]]
 name = "types_test"
-version = "0.6.0"
+version = "0.7.0"
 dependencies = [
  "dojo",
 ]