refactor: write manifest file even if migration failed

bin/sozo/src/commands/dev.rs

@@ -10,7 +10,7 @@
 use clap::Args;
 use dojo_lang::compiler::{BASE_DIR, MANIFESTS_DIR};
 use dojo_lang::scarb_internal::build_scarb_root_database;
-use dojo_world::manifest::{BaseManifest, DeployedManifest};
+use dojo_world::manifest::{BaseManifest, DeploymentManifest};
 use dojo_world::metadata::dojo_metadata_from_workspace;
 use dojo_world::migration::world::WorldDiff;
 use notify_debouncer_mini::notify::RecursiveMode;
@@ -26,7 +26,7 @@
 use super::options::account::AccountOptions;
 use super::options::starknet::StarknetOptions;
 use super::options::world::WorldOptions;
-use crate::ops::migration;
+use crate::ops::migration::{self, prepare_migration};
 
 #[derive(Args)]
 pub struct DevArgs {
@@ -113,8 +113,8 @@
     account: &SingleOwnerAccount<P, S>,
     name: Option<String>,
     ws: &Workspace<'_>,
-    previous_manifest: Option<DeployedManifest>,
-) -> Result<(DeployedManifest, Option<FieldElement>)>
+    previous_manifest: Option<DeploymentManifest>,
+) -> Result<(DeploymentManifest, Option<FieldElement>)>
 where
     P: Provider + Sync + Send + 'static,
     S: Signer + Sync + Send + 'static,
@@ -140,9 +140,10 @@
         return Ok((new_manifest.into(), world_address));
     }
 
-    match migration::apply_diff(ws, &target_dir, diff, name.clone(), world_address, account, None)
-        .await
-    {
+    let ui = ws.config().ui();
+    let strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?;
+
+    match migration::apply_diff(ws, account, None, &strategy).await {
         Ok(migration_output) => {
             config.ui().print(format!(
                 "🎉 World at address {} updated!",
@@ -209,7 +210,7 @@
             RecursiveMode::Recursive,
         )?;
         let name = self.name.clone();
-        let mut previous_manifest: Option<DeployedManifest> = Option::None;
+        let mut previous_manifest: Option<DeploymentManifest> = Option::None;
         let result = build(&mut context);
 
         let Some((mut world_address, account, _)) = context

bin/sozo/src/ops/events.rs

@@ -6,7 +6,7 @@
 use cainome::parser::AbiParser;
 use camino::Utf8PathBuf;
 use dojo_lang::compiler::{DEPLOYMENTS_DIR, MANIFESTS_DIR};
-use dojo_world::manifest::{DeployedManifest, ManifestMethods};
+use dojo_world::manifest::{DeploymentManifest, ManifestMethods};
 use dojo_world::metadata::Environment;
 use starknet::core::types::{BlockId, EventFilter, FieldElement};
 use starknet::core::utils::{parse_cairo_short_string, starknet_keccak};
@@ -48,7 +48,10 @@
             return Err(anyhow!("Run scarb migrate before running this command"));
         }
 
-        Some(extract_events(&DeployedManifest::load_from_path(&deployed_manifest)?, manifest_dir)?)
+        Some(extract_events(
+            &DeploymentManifest::load_from_path(&deployed_manifest)?,
+            manifest_dir,
+        )?)
     } else {
         None
     };
@@ -80,7 +83,7 @@
 }
 
 fn extract_events(
-    manifest: &DeployedManifest,
+    manifest: &DeploymentManifest,
     manifest_dir: &Utf8PathBuf,
 ) -> Result<HashMap<String, Vec<Token>>> {
     fn process_abi(

bin/sozo/src/ops/migration/migration_test.rs

@@ -5,7 +5,7 @@
 use dojo_test_utils::sequencer::{
     get_default_test_starknet_config, SequencerConfig, StarknetConfig, TestSequencer,
 };
-use dojo_world::manifest::{BaseManifest, DeployedManifest};
+use dojo_world::manifest::{BaseManifest, DeploymentManifest};
 use dojo_world::migration::strategy::prepare_for_migration;
 use dojo_world::migration::world::WorldDiff;
 use scarb::ops;
@@ -162,7 +162,7 @@
         &Utf8Path::new(base).to_path_buf().join(MANIFESTS_DIR).join(BASE_DIR),
     )
     .unwrap();
-    let remote_manifest = DeployedManifest::load_from_remote(
+    let remote_manifest = DeploymentManifest::load_from_remote(
         JsonRpcClient::new(HttpTransport::new(sequencer.url())),
         migration.world_address().unwrap(),
     )

bin/sozo/src/ops/migration/mod.rs

@@ -5,8 +5,8 @@
 use dojo_world::contracts::cairo_utils;
 use dojo_world::contracts::world::WorldContract;
 use dojo_world::manifest::{
-    AbstractManifestError, BaseManifest, DeployedManifest, DojoContract, Manifest, ManifestMethods,
-    OverlayManifest,
+    AbstractManifestError, BaseManifest, DeploymentManifest, DojoContract, Manifest,
+    ManifestMethods, OverlayManifest,
 };
 use dojo_world::metadata::{dojo_metadata_from_workspace, Environment};
 use dojo_world::migration::contract::ContractMigration;
@@ -48,6 +48,7 @@
     pub world_address: FieldElement,
     pub world_tx_hash: Option<FieldElement>,
     pub world_block_number: Option<u64>,
+    pub success: bool,
 }
 
 pub async fn execute(
@@ -83,38 +84,52 @@
     ui.print_sub(format!("Total diffs found: {total_diffs}"));
 
     if total_diffs == 0 {
-        ui.print("\n✨ No changes to be made. Remote World is already up to date!")
-    } else {
-        // Migrate according to the diff.
-        let migration_output = apply_diff(
-            ws,
-            &target_dir,
-            diff,
-            name,
-            world_address,
-            &account,
-            Some(args.transaction),
-        )
-        .await?;
-
-        update_manifests_and_abis(
-            ws,
-            local_manifest,
-            remote_manifest,
-            &manifest_dir,
-            migration_output,
-            &chain_id,
-        )
-        .await?;
+        ui.print("\n✨ No changes to be made. Remote World is already up to date!");
+        return Ok(());
     }
 
+    let strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?;
+    let world_address = strategy.world_address().expect("world address must exist");
+
+    // Migrate according to the diff.
+    match apply_diff(ws, &account, None, &strategy).await {
+        Ok(migration_output) => {
+            update_manifests_and_abis(
+                ws,
+                local_manifest,
+                remote_manifest,
+                &manifest_dir,
+                migration_output,
+                &chain_id,
+            )
+            .await?;
+        }
+        Err(e) => {
+            update_manifests_and_abis(
+                ws,
+                local_manifest,
+                remote_manifest,
+                &manifest_dir,
+                MigrationOutput {
+                    world_address,
+                    world_tx_hash: None,
+                    world_block_number: None,
+                    success: false,
+                },
+                &chain_id,
+            )
+            .await?;
+            return Err(e)?;
+        }
+    };
+
     Ok(())
 }
 
 async fn update_manifests_and_abis(
     ws: &Workspace<'_>,
     local_manifest: BaseManifest,
-    remote_manifest: Option<DeployedManifest>,
+    remote_manifest: Option<DeploymentManifest>,
     manifest_dir: &Utf8PathBuf,
     migration_output: MigrationOutput,
     chain_id: &str,
@@ -128,10 +143,10 @@
         .join(chain_id)
         .with_extension("toml");
 
-    let mut local_manifest: DeployedManifest = local_manifest.into();
+    let mut local_manifest: DeploymentManifest = local_manifest.into();
 
     if deployed_path.exists() {
-        let previous_manifest = DeployedManifest::load_from_path(&deployed_path)?;
+        let previous_manifest = DeploymentManifest::load_from_path(&deployed_path)?;
         local_manifest.merge_from_previous(previous_manifest);
     };
 
@@ -166,7 +181,7 @@
 }
 
 async fn update_manifest_abis(
-    local_manifest: &mut DeployedManifest,
+    local_manifest: &mut DeploymentManifest,
     manifest_dir: &Utf8PathBuf,
     chain_id: &str,
 ) {
@@ -202,42 +217,47 @@
     }
 }
 
-#[allow(clippy::too_many_arguments)]
 pub(crate) async fn apply_diff<P, S>(
     ws: &Workspace<'_>,
-    target_dir: &Utf8PathBuf,
-    diff: WorldDiff,
-    name: Option<String>,
-    world_address: Option<FieldElement>,
     account: &SingleOwnerAccount<P, S>,
     txn_config: Option<TransactionOptions>,
+    strategy: &MigrationStrategy,
 ) -> Result<MigrationOutput>
 where
     P: Provider + Sync + Send + 'static,
     S: Signer + Sync + Send + 'static,
 {
     let ui = ws.config().ui();
-    let strategy = prepare_migration(target_dir, diff, name, world_address, &ui)?;
 
     println!("  ");
 
-    let migration_output = execute_strategy(ws, &strategy, account, txn_config)
+    let migration_output = execute_strategy(ws, strategy, account, txn_config)
         .await
         .map_err(|e| anyhow!(e))
         .with_context(|| "Problem trying to migrate.")?;
 
-    if let Some(block_number) = migration_output.world_block_number {
-        ui.print(format!(
-            "\n🎉 Successfully migrated World on block #{} at address {}",
-            block_number,
-            bold_message(format!(
-                "{:#x}",
-                strategy.world_address().expect("world address must exist")
-            ))
-        ));
+    if migration_output.success {
+        if let Some(block_number) = migration_output.world_block_number {
+            ui.print(format!(
+                "\n🎉 Successfully migrated World on block #{} at address {}",
+                block_number,
+                bold_message(format!(
+                    "{:#x}",
+                    strategy.world_address().expect("world address must exist")
+                ))
+            ));
+        } else {
+            ui.print(format!(
+                "\n🎉 Successfully migrated World at address {}",
+                bold_message(format!(
+                    "{:#x}",
+                    strategy.world_address().expect("world address must exist")
+                ))
+            ));
+        }
     } else {
         ui.print(format!(
-            "\n🎉 Successfully migrated World at address {}",
+            "\n🚨 Partially migrated World at address {}",
             bold_message(format!(
                 "{:#x}",
                 strategy.world_address().expect("world address must exist")
@@ -298,7 +318,7 @@
     account: &SingleOwnerAccount<P, S>,
     world_address: Option<FieldElement>,
     ui: &Ui,
-) -> Result<(BaseManifest, Option<DeployedManifest>)>
+) -> Result<(BaseManifest, Option<DeploymentManifest>)>
 where
     P: Provider + Sync + Send + 'static,
     S: Signer + Sync + Send + 'static,
@@ -318,7 +338,7 @@
     }
 
     let remote_manifest = if let Some(address) = world_address {
-        match DeployedManifest::load_from_remote(account.provider(), address).await {
+        match DeploymentManifest::load_from_remote(account.provider(), address).await {
             Ok(manifest) => {
                 ui.print_sub(format!("Found remote World: {address:#x}"));
                 Some(manifest)
@@ -340,7 +360,7 @@
     Ok((local_manifest, remote_manifest))
 }
 
-fn prepare_migration(
+pub fn prepare_migration(
     target_dir: &Utf8PathBuf,
     diff: WorldDiff,
     name: Option<String>,
@@ -476,17 +496,34 @@
         None => {}
     };
 
+    let mut migration_output = MigrationOutput {
+        world_address: strategy.world_address()?,
+        world_tx_hash,
+        world_block_number,
+        success: false,
+    };
+
     // Once Torii supports indexing arrays, we should declare and register the
     // ResourceMetadata model.
 
-    register_models(strategy, migrator, &ui, txn_config.clone()).await?;
-    deploy_contracts(strategy, migrator, &ui, txn_config).await?;
+    match register_models(strategy, migrator, &ui, txn_config.clone()).await {
+        Ok(_) => (),
+        Err(e) => {
+            ui.anyhow(&e);
+            return Ok(migration_output);
+        }
+    }
+    match deploy_contracts(strategy, migrator, &ui, txn_config).await {
+        Ok(_) => (),
+        Err(e) => {
+            ui.anyhow(&e);
+            return Ok(migration_output);
+        }
+    };
 
-    Ok(MigrationOutput {
-        world_address: strategy.world_address()?,
-        world_tx_hash,
-        world_block_number,
-    })
+    migration_output.success = true;
+
+    Ok(migration_output)
 }
 
 enum ContractDeploymentOutput {

bin/sozo/src/ops/register.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use anyhow::{Context, Result};
 use dojo_world::contracts::model::ModelReader;
 use dojo_world::contracts::{WorldContract, WorldContractReader};
-use dojo_world::manifest::DeployedManifest;
+use dojo_world::manifest::DeploymentManifest;
 use dojo_world::metadata::Environment;
 use scarb::core::Config;
 use starknet::accounts::Account;
@@ -25,7 +25,7 @@ pub async fn execute(
             let world_reader = WorldContractReader::new(world_address, &provider)
                 .with_block(BlockId::Tag(BlockTag::Pending));
             let manifest = {
-                match DeployedManifest::load_from_remote(&provider, world_address).await {
+                match DeploymentManifest::load_from_remote(&provider, world_address).await {
                     Ok(manifest) => manifest,
                     Err(e) => {
                         return Err(anyhow::anyhow!("Failed to build remote World state: {e}"));

crates/benches/src/deployer.rs

@@ -6,7 +6,7 @@ use clap::Parser;
 use dojo_lang::compiler::{DojoCompiler, DEPLOYMENTS_DIR, MANIFESTS_DIR};
 use dojo_lang::plugin::CairoPluginRepository;
 use dojo_lang::scarb_internal::compile_workspace;
-use dojo_world::manifest::DeployedManifest;
+use dojo_world::manifest::DeploymentManifest;
 use futures::executor::block_on;
 use katana_runner::KatanaRunner;
 use scarb::compiler::CompilerRepository;
@@ -117,7 +117,7 @@ async fn prepare_migration_args(args: SozoArgs) -> Result<FieldElement> {
 
     migration::execute(&ws, migrate, None).await?;
 
-    let manifest = DeployedManifest::load_from_path(
+    let manifest = DeploymentManifest::load_from_path(
         &manifest_dir
             .join(MANIFESTS_DIR)
             .join(DEPLOYMENTS_DIR)