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(address) => {
             config
                 .ui()
@@ -208,7 +209,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 cairo_lang_starknet::plugin::events::EventFieldKind;
 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};
 use starknet::core::utils::{parse_cairo_short_string, starknet_keccak};
@@ -47,7 +47,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
     };
@@ -189,7 +192,7 @@
 }
 
 fn extract_events(
-    manifest: &DeployedManifest,
+    manifest: &DeploymentManifest,
     manifest_dir: &Utf8PathBuf,
 ) -> Result<HashMap<String, Vec<Event>>, Error> {
     fn inner_helper(

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;
@@ -158,7 +158,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;
@@ -77,46 +77,41 @@
     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 {
-        // Mirate according to the diff.
-        let world_address = 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,
-            world_address,
-            &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");
+
+    update_manifests_and_abis(
+        ws,
+        local_manifest,
+        remote_manifest,
+        &manifest_dir,
+        world_address,
+        &chain_id,
+    )
+    .await?;
+
+    // Mirate according to the diff.
+    apply_diff(ws, &account, None, &strategy).await?;
+
     Ok(())
 }
 
 async fn update_manifests_and_abis(
     ws: &Workspace<'_>,
     local_manifest: BaseManifest,
-    remote_manifest: Option<DeployedManifest>,
+    remote_manifest: Option<DeploymentManifest>,
     manifest_dir: &Utf8PathBuf,
     world_address: FieldElement,
     chain_id: &str,
 ) -> Result<()> {
     let ui = ws.config().ui();
     ui.print("\n✨ Updating manifests...");
 
-    let mut local_manifest: DeployedManifest = local_manifest.into();
+    let mut local_manifest: DeploymentManifest = local_manifest.into();
     local_manifest.world.inner.address = Some(world_address);
 
     let base_class_hash = match remote_manifest {
@@ -146,7 +141,7 @@
 }
 
 async fn update_manifest_abis(
-    local_manifest: &mut DeployedManifest,
+    local_manifest: &mut DeploymentManifest,
     manifest_dir: &Utf8PathBuf,
     chain_id: &str,
 ) {
@@ -182,26 +177,21 @@
     }
 }
 
-#[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<FieldElement>
 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 block_height = execute_strategy(ws, &strategy, account, txn_config)
+    let block_height = execute_strategy(ws, strategy, account, txn_config)
         .await
         .map_err(|e| anyhow!(e))
         .with_context(|| "Problem trying to migrate.")?;
@@ -278,7 +268,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,
@@ -298,7 +288,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)
@@ -320,7 +310,7 @@
     Ok((local_manifest, remote_manifest))
 }
 
-fn prepare_migration(
+pub fn prepare_migration(
     target_dir: &Utf8PathBuf,
     diff: WorldDiff,
     name: Option<String>,

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)

crates/dojo-world/src/manifest.rs

@@ -180,9 +180,9 @@ impl From<Manifest<Class>> for Manifest<Contract> {
     }
 }
 
-impl From<BaseManifest> for DeployedManifest {
+impl From<BaseManifest> for DeploymentManifest {
     fn from(value: BaseManifest) -> Self {
-        DeployedManifest {
+        DeploymentManifest {
             world: value.world.into(),
             base: value.base,
             contracts: value.contracts,
@@ -192,7 +192,7 @@ impl From<BaseManifest> for DeployedManifest {
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
-pub struct DeployedManifest {
+pub struct DeploymentManifest {
     pub world: Manifest<Contract>,
     pub base: Manifest<Class>,
     pub contracts: Vec<Manifest<DojoContract>>,
@@ -279,7 +279,7 @@ impl OverlayManifest {
     }
 }
 
-impl DeployedManifest {
+impl DeploymentManifest {
     pub fn load_from_path(path: &Utf8PathBuf) -> Result<Self, AbstractManifestError> {
         let manifest: Self = toml::from_str(&fs::read_to_string(path)?).unwrap();
 
@@ -324,7 +324,7 @@ impl DeployedManifest {
         let (models, contracts) =
             get_remote_models_and_contracts(world_address, &world.provider()).await?;
 
-        Ok(DeployedManifest {
+        Ok(DeploymentManifest {
             models,
             contracts,
             world: Manifest::new(

crates/dojo-world/src/manifest_test.rs

@@ -12,7 +12,7 @@ use starknet::providers::jsonrpc::{JsonRpcClient, JsonRpcMethod};
 
 use super::{parse_contracts_events, BaseManifest, DojoContract, DojoModel};
 use crate::contracts::world::test::deploy_world;
-use crate::manifest::{parse_models_events, AbstractManifestError, DeployedManifest, Manifest};
+use crate::manifest::{parse_models_events, AbstractManifestError, DeploymentManifest, Manifest};
 use crate::migration::world::WorldDiff;
 
 #[tokio::test]
@@ -31,7 +31,7 @@ async fn manifest_from_remote_throw_error_on_not_deployed() {
     );
 
     let rpc = JsonRpcClient::new(mock_transport);
-    let err = DeployedManifest::load_from_remote(rpc, FieldElement::ONE).await.unwrap_err();
+    let err = DeploymentManifest::load_from_remote(rpc, FieldElement::ONE).await.unwrap_err();
 
     match err {
         AbstractManifestError::RemoteWorldNotFound => {
@@ -378,7 +378,7 @@ async fn fetch_remote_manifest() {
     let local_manifest =
         BaseManifest::load_from_path(&manifest_path.join(MANIFESTS_DIR).join(BASE_DIR)).unwrap();
     let remote_manifest =
-        DeployedManifest::load_from_remote(provider, world_address).await.unwrap();
+        DeploymentManifest::load_from_remote(provider, world_address).await.unwrap();
 
     assert_eq!(local_manifest.models.len(), 2);
     assert_eq!(local_manifest.contracts.len(), 1);

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

@@ -6,7 +6,7 @@ use super::class::ClassDiff;
 use super::contract::ContractDiff;
 use super::StateDiff;
 use crate::manifest::{
-    BaseManifest, DeployedManifest, ManifestMethods, BASE_CONTRACT_NAME, WORLD_CONTRACT_NAME,
+    BaseManifest, DeploymentManifest, ManifestMethods, BASE_CONTRACT_NAME, WORLD_CONTRACT_NAME,
 };
 
 #[cfg(test)]
@@ -23,7 +23,7 @@ pub struct WorldDiff {
 }
 
 impl WorldDiff {
-    pub fn compute(local: BaseManifest, remote: Option<DeployedManifest>) -> WorldDiff {
+    pub fn compute(local: BaseManifest, remote: Option<DeploymentManifest>) -> WorldDiff {
         let models = local
             .models
             .iter()

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

@@ -28,7 +28,7 @@ fn no_diff_when_local_and_remote_are_equal() {
     let local =
         BaseManifest { models, world: world_contract, base: base_contract, contracts: vec![] };
 
-    let mut remote: DeployedManifest = local.clone().into();
+    let mut remote: DeploymentManifest = local.clone().into();
     remote.models = remote_models;
 
     let diff = WorldDiff::compute(local, Some(remote));
@@ -91,7 +91,7 @@ fn diff_when_local_and_remote_are_different() {
 
     let local = BaseManifest { models, contracts, world: world_contract, base: base_contract };
 
-    let mut remote: DeployedManifest = local.clone().into();
+    let mut remote: DeploymentManifest = local.clone().into();
     remote.models = remote_models;
     remote.world.inner.class_hash = 44_u32.into();
     remote.models[1].inner.class_hash = 33_u32.into();

crates/torii/graphql/src/tests/mod.rs

@@ -10,7 +10,7 @@ use dojo_test_utils::sequencer::{
 use dojo_types::primitive::Primitive;
 use dojo_types::schema::{Enum, EnumOption, Member, Struct, Ty};
 use dojo_world::contracts::WorldContractReader;
-use dojo_world::manifest::DeployedManifest;
+use dojo_world::manifest::DeploymentManifest;
 use dojo_world::utils::TransactionWaiter;
 use scarb::ops;
 use serde::Deserialize;
@@ -280,7 +280,7 @@ pub async fn spinup_types_test() -> Result<SqlitePool> {
     execute_strategy(&ws, &migration, &account, None).await.unwrap();
 
     let manifest =
-        DeployedManifest::load_from_remote(&provider, migration.world_address().unwrap())
+        DeploymentManifest::load_from_remote(&provider, migration.world_address().unwrap())
             .await
             .unwrap();