after review

bin/sozo/src/commands/auth.rs

@@ -7,6 +7,7 @@ use clap::{Args, Subcommand};
 use colored::Colorize;
 use dojo_utils::Invoker;
 use dojo_world::config::ProfileConfig;
+use dojo_world::constants::WORLD;
 use dojo_world::contracts::{ContractInfo, WorldContract};
 use dojo_world::diff::{DiffPermissions, WorldDiff};
 use scarb::core::{Config, Workspace};
@@ -305,12 +306,12 @@ async fn clone_permissions(
     writer_of.extend(
         external_writer_of
             .iter()
-            .map(|r| if r != &Felt::ZERO { format!("{:#066x}", r) } else { "World".to_string() }),
+            .map(|r| if r != &WORLD { format!("{:#066x}", r) } else { "World".to_string() }),
     );
     owner_of.extend(
         external_owner_of
             .iter()
-            .map(|r| if r != &Felt::ZERO { format!("{:#066x}", r) } else { "World".to_string() }),
+            .map(|r| if r != &WORLD { format!("{:#066x}", r) } else { "World".to_string() }),
     );
 
     // Sort the tags to have a deterministic output.
@@ -417,13 +418,13 @@ async fn list_permissions(
 
     let mut world_writers = world_diff
         .external_writers
-        .get(&Felt::ZERO)
+        .get(&WORLD)
         .map(|writers| writers.iter().cloned().collect::<Vec<_>>())
         .unwrap_or_default();
 
     let mut world_owners = world_diff
         .external_owners
-        .get(&Felt::ZERO)
+        .get(&WORLD)
         .map(|owners| owners.iter().cloned().collect::<Vec<_>>())
         .unwrap_or_default();
 
@@ -677,7 +678,7 @@ impl PermissionPair {
         contracts: &HashMap<String, ContractInfo>,
     ) -> Result<(Felt, Felt)> {
         let selector = if self.resource_tag == "world" {
-            Felt::ZERO
+            WORLD
         } else if self.resource_tag.starts_with("0x") {
             Felt::from_str(&self.resource_tag)
                 .map_err(|_| anyhow!("Invalid resource selector: {}", self.resource_tag))?
@@ -788,7 +789,7 @@ mod tests {
             grantee_tag_or_address: "0x123".to_string(),
         };
         let (selector, address) = pair.to_selector_and_address(&contracts).unwrap();
-        assert_eq!(selector, Felt::ZERO);
+        assert_eq!(selector, WORLD);
         assert_eq!(address, Felt::from_str("0x123").unwrap());
 
         let pair = PermissionPair {

bin/sozo/src/commands/dev.rs

@@ -83,12 +83,16 @@ impl DevArgs {
         build_args.clone().run(config)?;
         info!("Initial build completed.");
 
+        // As this `dev` command is for development purpose only,
+        // allowing to watch for changes, compile and migrate them,
+        // there is no need for metadata uploading. That's why,
+        // `ipfs` is set to its default value meaning it is disabled.
         let migrate_args = MigrateArgs {
             world: self.world,
             starknet: self.starknet,
             account: self.account,
             transaction: self.transaction,
-            ipfs: IpfsOptions::default(), // no need for IPFS metadata upload here.
+            ipfs: IpfsOptions::default(),
         };
 
         let _ = migrate_args.clone().run(config);

bin/sozo/src/commands/migrate.rs

@@ -3,7 +3,7 @@ use clap::Args;
 use colored::Colorize;
 use dojo_utils::{self, TxnConfig};
 use dojo_world::contracts::WorldContract;
-use dojo_world::metadata::IpfsMetadataService;
+use dojo_world::services::IpfsService;
 use scarb::core::{Config, Workspace};
 use sozo_ops::migrate::{Migration, MigrationResult};
 use sozo_ops::migration_ui::MigrationUi;
@@ -81,18 +81,12 @@ impl MigrateArgs {
             let MigrationResult { manifest, has_changes } =
                 migration.migrate(&mut spinner).await.context("Migration failed.")?;
 
-            let ipfs_url = ipfs.url(profile_config.env.as_ref());
-            let ipfs_username = ipfs.username(profile_config.env.as_ref());
-            let ipfs_password = ipfs.password(profile_config.env.as_ref());
-
+            let ipfs_config =
+                ipfs.config().or(profile_config.env.map(|env| env.ipfs_config).unwrap_or(None));
             let mut metadata_upload_text = String::new();
 
-            if ipfs_url.is_some() && ipfs_username.is_some() && ipfs_password.is_some() {
-                let mut metadata_service = IpfsMetadataService::new(
-                    &ipfs_url.unwrap(),
-                    &ipfs_username.unwrap(),
-                    &ipfs_password.unwrap(),
-                )?;
+            if let Some(config) = ipfs_config {
+                let mut metadata_service = IpfsService::new(config)?;
 
                 migration
                     .upload_metadata(&mut spinner, &mut metadata_service)

bin/sozo/src/commands/options/ipfs.rs

@@ -1,6 +1,6 @@
 use clap::Args;
 use dojo_utils::env::{IPFS_PASSWORD_ENV_VAR, IPFS_URL_ENV_VAR, IPFS_USERNAME_ENV_VAR};
-use dojo_world::config::Environment;
+use dojo_world::config::IpfsConfig;
 use tracing::trace;
 use url::Url;
 
@@ -27,47 +27,16 @@ pub struct IpfsOptions {
 }
 
 impl IpfsOptions {
-    pub fn url(&self, env_metadata: Option<&Environment>) -> Option<String> {
-        trace!("Retrieving URL for IpfsOptions.");
-
-        if let Some(url) = self.ipfs_url.as_ref() {
-            trace!(?url, "Using IPFS URL from command line.");
-            Some(url.to_string())
-        } else if let Some(url) = env_metadata.and_then(|env| env.ipfs_url()) {
-            trace!(url, "Using IPFS URL from environment metadata.");
-            Some(url.to_string())
-        } else {
-            trace!("No default IPFS URL.");
-            None
-        }
-    }
-
-    pub fn username(&self, env_metadata: Option<&Environment>) -> Option<String> {
-        trace!("Retrieving username for IpfsOptions.");
-
-        if let Some(username) = self.ipfs_username.as_ref() {
-            trace!(?username, "Using IPFS username from command line.");
-            Some(username.clone())
-        } else if let Some(username) = env_metadata.and_then(|env| env.ipfs_username()) {
-            trace!(username, "Using IPFS username from environment metadata.");
-            Some(username.to_string())
-        } else {
-            trace!("No default IPFS username.");
-            None
-        }
-    }
+    pub fn config(&self) -> Option<IpfsConfig> {
+        trace!("Retrieving IPFS config for IpfsOptions.");
 
-    pub fn password(&self, env_metadata: Option<&Environment>) -> Option<String> {
-        trace!("Retrieving password for IpfsOptions.");
+        let url = self.ipfs_url.as_ref().map(|url| url.to_string());
+        let username = self.ipfs_username.clone();
+        let password = self.ipfs_password.clone();
 
-        if let Some(password) = self.ipfs_password.as_ref() {
-            trace!(?password, "Using IPFS password from command line.");
-            Some(password.clone())
-        } else if let Some(password) = env_metadata.and_then(|env| env.ipfs_password()) {
-            trace!(password, "Using IPFS password from environment metadata.");
-            Some(password.to_string())
+        if let (Some(url), Some(username), Some(password)) = (url, username, password) {
+            Some(IpfsConfig { url, username, password })
         } else {
-            trace!("No default IPFS password.");
             None
         }
     }
@@ -97,74 +66,51 @@ mod tests {
         std::env::set_var(IPFS_PASSWORD_ENV_VAR, ENV_IPFS_PASSWORD);
 
         let cmd = Command::parse_from([""]);
-        assert_eq!(cmd.options.url(None).unwrap().as_str(), ENV_IPFS_URL);
-        assert_eq!(cmd.options.username(None).unwrap(), ENV_IPFS_USERNAME);
-        assert_eq!(cmd.options.password(None).unwrap(), ENV_IPFS_PASSWORD);
+        let config = cmd.options.config().unwrap();
+        assert_eq!(config.url, ENV_IPFS_URL.to_string());
+        assert_eq!(config.username, ENV_IPFS_USERNAME.to_string());
+        assert_eq!(config.password, ENV_IPFS_PASSWORD.to_string());
     }
 
     #[test]
-    fn options_exist_in_env_but_not_in_args() {
-        let env_metadata = dojo_world::config::Environment {
-            ipfs_url: Some(ENV_IPFS_URL.into()),
-            ipfs_username: Some(ENV_IPFS_USERNAME.into()),
-            ipfs_password: Some(ENV_IPFS_PASSWORD.into()),
-            ..Default::default()
-        };
-
-        let cmd = Command::parse_from([""]);
-        assert_eq!(cmd.options.url(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_URL);
-        assert_eq!(cmd.options.username(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_USERNAME);
-        assert_eq!(cmd.options.password(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_PASSWORD);
-    }
+    fn cli_args_override_env_variables() {
+        std::env::set_var(IPFS_URL_ENV_VAR, ENV_IPFS_URL);
+        let url = "http://different.url/";
+        let username = "bobsmith";
+        let password = "654321";
 
-    #[test]
-    fn options_doesnt_exist_in_env_but_exist_in_args() {
-        let env_metadata = dojo_world::config::Environment::default();
         let cmd = Command::parse_from([
             "sozo",
             "--ipfs-url",
-            ENV_IPFS_URL,
+            url,
             "--ipfs-username",
-            ENV_IPFS_USERNAME,
+            username,
             "--ipfs-password",
-            ENV_IPFS_PASSWORD,
+            password,
         ]);
-
-        assert_eq!(cmd.options.url(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_URL);
-        assert_eq!(cmd.options.username(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_USERNAME);
-        assert_eq!(cmd.options.password(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_PASSWORD);
+        let config = cmd.options.config().unwrap();
+        assert_eq!(config.url, url);
+        assert_eq!(config.username, username);
+        assert_eq!(config.password, password);
     }
 
     #[test]
-    fn options_exists_in_both() {
-        let env_metadata = dojo_world::config::Environment {
-            ipfs_url: Some(ENV_IPFS_URL.into()),
-            ipfs_username: Some(ENV_IPFS_USERNAME.into()),
-            ipfs_password: Some(ENV_IPFS_PASSWORD.into()),
-            ..Default::default()
-        };
-
-        let cmd = Command::parse_from([
+    fn invalid_url_format() {
+        let cmd = Command::try_parse_from([
             "sozo",
             "--ipfs-url",
-            ENV_IPFS_URL,
+            "invalid-url",
             "--ipfs-username",
-            ENV_IPFS_USERNAME,
+            "bobsmith",
             "--ipfs-password",
-            ENV_IPFS_PASSWORD,
+            "654321",
         ]);
-
-        assert_eq!(cmd.options.url(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_URL);
-        assert_eq!(cmd.options.username(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_USERNAME);
-        assert_eq!(cmd.options.password(Some(&env_metadata)).unwrap().as_str(), ENV_IPFS_PASSWORD);
+        assert!(cmd.is_err());
     }
 
     #[test]
-    fn url_exists_in_neither() {
-        let env_metadata = dojo_world::config::Environment::default();
-        let cmd = Command::parse_from([""]);
-        assert_eq!(cmd.options.url(Some(&env_metadata)), None);
-        assert_eq!(cmd.options.username(Some(&env_metadata)), None);
-        assert_eq!(cmd.options.password(Some(&env_metadata)), None);
+    fn options_not_provided_in_env_variable() {
+        let cmd = Command::parse_from(["sozo"]);
+        assert!(cmd.options.config().is_none());
     }
 }

crates/dojo-world/Cargo.toml

@@ -51,5 +51,5 @@ tokio.workspace = true
 [features]
 contracts = [ "dep:dojo-types", "dep:http", "dep:num-traits" ]
 manifest = [ "contracts", "dep:dojo-types", "dep:scarb", "dep:url" ]
-metadata = [ "dep:ipfs-api-backend-hyper", "dep:scarb", "dep:url" ]
+ipfs = [ "dep:ipfs-api-backend-hyper", "dep:scarb", "dep:url" ]
 migration = [ "dep:dojo-utils", "dep:scarb", "dep:tokio", "manifest" ]

crates/dojo/world/Cargo.toml

@@ -33,4 +33,4 @@ tokio.workspace = true
 futures.workspace = true
 
 [features]
-metadata = [ "dep:ipfs-api-backend-hyper" ]
+ipfs = [ "dep:ipfs-api-backend-hyper" ]

crates/dojo/world/src/config/environment.rs

@@ -1,5 +1,7 @@
 use serde::Deserialize;
 
+use super::IpfsConfig;
+
 #[derive(Default, Deserialize, Clone, Debug)]
 pub struct Environment {
     pub rpc_url: Option<String>,
@@ -10,9 +12,7 @@ pub struct Environment {
     pub world_address: Option<String>,
     pub world_block: Option<u64>,
     pub http_headers: Option<Vec<HttpHeader>>,
-    pub ipfs_url: Option<String>,
-    pub ipfs_username: Option<String>,
-    pub ipfs_password: Option<String>,
+    pub ipfs_config: Option<IpfsConfig>,
 }
 
 #[derive(Debug, Clone, Deserialize)]
@@ -45,16 +45,4 @@ impl Environment {
     pub fn keystore_password(&self) -> Option<&str> {
         self.keystore_password.as_deref()
     }
-
-    pub fn ipfs_url(&self) -> Option<&str> {
-        self.ipfs_url.as_deref()
-    }
-
-    pub fn ipfs_username(&self) -> Option<&str> {
-        self.ipfs_username.as_deref()
-    }
-
-    pub fn ipfs_password(&self) -> Option<&str> {
-        self.ipfs_password.as_deref()
-    }
 }

crates/dojo/world/src/config/ipfs_config.rs

@@ -0,0 +1,22 @@
+use anyhow::Result;
+use serde::Deserialize;
+
+#[derive(Default, Deserialize, Clone, Debug)]
+pub struct IpfsConfig {
+    pub url: String,
+    pub username: String,
+    pub password: String,
+}
+
+impl IpfsConfig {
+    pub fn assert_valid(&self) -> Result<()> {
+        if self.url.is_empty() || self.username.is_empty() || self.password.is_empty() {
+            anyhow::bail!("Invalid IPFS credentials: empty values not allowed");
+        }
+        if !self.url.starts_with("http://") && !self.url.starts_with("https://") {
+            anyhow::bail!("Invalid IPFS URL: must start with http:// or https://");
+        }
+
+        Ok(())
+    }
+}

crates/dojo/world/src/config/mod.rs

@@ -1,5 +1,6 @@
 pub mod calldata_decoder;
 pub mod environment;
+pub mod ipfs_config;
 pub mod metadata_config;
 pub mod migration_config;
 pub mod namespace_config;
@@ -8,6 +9,7 @@ pub mod resource_config;
 pub mod world_config;
 
 pub use environment::Environment;
+pub use ipfs_config::IpfsConfig;
 pub use metadata_config::WorldMetadata;
 pub use namespace_config::NamespaceConfig;
 pub use profile_config::ProfileConfig;

crates/dojo/world/src/config/profile_config.rs

@@ -165,6 +165,11 @@ mod tests {
         keystore_password = "test"
         world_address = "test"
 
+        [env.ipfs_config]
+        url = "https://ipfs.service"
+        username = "johndoe"
+        password = "123456"
+
         [migration]
         skip_contracts = [ "module::my-contract" ]
 
@@ -191,6 +196,11 @@ mod tests {
         assert_eq!(env.keystore_password, Some("test".to_string()));
         assert_eq!(env.world_address, Some("test".to_string()));
 
+        let ipfs_config = env.ipfs_config.unwrap();
+        assert_eq!(ipfs_config.url, "https://ipfs.service".to_string());
+        assert_eq!(ipfs_config.username, "johndoe".to_string());
+        assert_eq!(ipfs_config.password, "123456".to_string());
+
         assert_eq!(config.world.description, Some("test".to_string()));
         assert_eq!(
             config.world.cover_uri,

crates/dojo/world/src/constants.rs

@@ -0,0 +1,4 @@
+use starknet_crypto::Felt;
+
+// the world selector
+pub const WORLD: Felt = Felt::ZERO;