Skip to content

Commit

Permalink
after review
Browse files Browse the repository at this point in the history
  • Loading branch information
remybar committed Nov 20, 2024
1 parent bb6067b commit cdf4b4a
Show file tree
Hide file tree
Showing 30 changed files with 184 additions and 250 deletions.
13 changes: 7 additions & 6 deletions bin/sozo/src/commands/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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() }),

Check warning on line 309 in bin/sozo/src/commands/auth.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/auth.rs#L309

Added line #L309 was not covered by tests
);
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() }),

Check warning on line 314 in bin/sozo/src/commands/auth.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/auth.rs#L314

Added line #L314 was not covered by tests
);

// Sort the tags to have a deterministic output.
Expand Down Expand Up @@ -417,13 +418,13 @@ async fn list_permissions(

let mut world_writers = world_diff
.external_writers
.get(&Felt::ZERO)
.get(&WORLD)

Check warning on line 421 in bin/sozo/src/commands/auth.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/auth.rs#L421

Added line #L421 was not covered by tests
.map(|writers| writers.iter().cloned().collect::<Vec<_>>())
.unwrap_or_default();

let mut world_owners = world_diff
.external_owners
.get(&Felt::ZERO)
.get(&WORLD)

Check warning on line 427 in bin/sozo/src/commands/auth.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/auth.rs#L427

Added line #L427 was not covered by tests
.map(|owners| owners.iter().cloned().collect::<Vec<_>>())
.unwrap_or_default();

Expand Down Expand Up @@ -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))?
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion bin/sozo/src/commands/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),

Check warning on line 95 in bin/sozo/src/commands/dev.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/dev.rs#L95

Added line #L95 was not covered by tests
};

let _ = migrate_args.clone().run(config);
Expand Down
16 changes: 5 additions & 11 deletions bin/sozo/src/commands/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Check warning on line 86 in bin/sozo/src/commands/migrate.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/migrate.rs#L84-L86

Added lines #L84 - L86 were not covered by tests

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)?;

Check warning on line 89 in bin/sozo/src/commands/migrate.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/migrate.rs#L88-L89

Added lines #L88 - L89 were not covered by tests

migration
.upload_metadata(&mut spinner, &mut metadata_service)
Expand Down
118 changes: 14 additions & 104 deletions bin/sozo/src/commands/options/ipfs.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -27,47 +27,16 @@ pub struct IpfsOptions {
}

impl IpfsOptions {
pub fn url(&self, env_metadata: Option<&Environment>) -> Option<String> {
trace!("Retrieving URL for IpfsOptions.");
pub fn config(&self) -> Option<IpfsConfig> {
trace!("Retrieving IPFS config 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
}
}
let url = self.ipfs_url.as_ref().map(|url| url.to_string());
let username = self.ipfs_username.clone();
let password = self.ipfs_password.clone();

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())
if let (Some(url), Some(username), Some(password)) = (url, username, password) {
Some(IpfsConfig { url, username, password })
} else {
trace!("No default IPFS username.");
None
}
}

pub fn password(&self, env_metadata: Option<&Environment>) -> Option<String> {
trace!("Retrieving password for IpfsOptions.");

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())
} else {
trace!("No default IPFS password.");
None
}
}
Expand Down Expand Up @@ -97,74 +66,15 @@ 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);
}

#[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);
}

#[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,
"--ipfs-username",
ENV_IPFS_USERNAME,
"--ipfs-password",
ENV_IPFS_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);
}

#[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([
"sozo",
"--ipfs-url",
ENV_IPFS_URL,
"--ipfs-username",
ENV_IPFS_USERNAME,
"--ipfs-password",
ENV_IPFS_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, 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 url_exists_in_neither() {
let env_metadata = dojo_world::config::Environment::default();
fn options_not_provided_in_env_variable() {
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);
assert!(cmd.options.config().is_none());
}
}
2 changes: 1 addition & 1 deletion crates/dojo-world/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
2 changes: 1 addition & 1 deletion crates/dojo/world/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ tokio.workspace = true
futures.workspace = true

[features]
metadata = [ "dep:ipfs-api-backend-hyper" ]
ipfs = [ "dep:ipfs-api-backend-hyper" ]
18 changes: 3 additions & 15 deletions crates/dojo/world/src/config/environment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use serde::Deserialize;

use super::IpfsConfig;

#[derive(Default, Deserialize, Clone, Debug)]
pub struct Environment {
pub rpc_url: Option<String>,
Expand All @@ -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)]
Expand Down Expand Up @@ -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()
}
}
22 changes: 22 additions & 0 deletions crates/dojo/world/src/config/ipfs_config.rs
Original file line number Diff line number Diff line change
@@ -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(())
}

Check warning on line 21 in crates/dojo/world/src/config/ipfs_config.rs

View check run for this annotation

Codecov / codecov/patch

crates/dojo/world/src/config/ipfs_config.rs#L12-L21

Added lines #L12 - L21 were not covered by tests
}
2 changes: 2 additions & 0 deletions crates/dojo/world/src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
10 changes: 10 additions & 0 deletions crates/dojo/world/src/config/profile_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions crates/dojo/world/src/constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
use starknet_crypto::Felt;

// the world selector
pub const WORLD: Felt = Felt::ZERO;
3 changes: 2 additions & 1 deletion crates/dojo/world/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
#![warn(unused_crate_dependencies)]

#[cfg(feature = "metadata")]
pub mod metadata;

pub mod config;
pub mod constants;
pub mod contracts;
pub mod diff;
pub mod local;
pub mod remote;
pub mod services;
pub mod uri;
pub mod utils;

Expand Down
Loading

0 comments on commit cdf4b4a

Please sign in to comment.