refactor(sozo): change priority order for arguments (#1358)

* refactor(sozo): change priority order for arguments * fix: tests * fix: code coverage
dojoengine · Jan 8, 2024 · 36e5853 · 36e5853
1 parent 74c12c0
commit 36e5853
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 22 deletions.
diff --git a/crates/sozo/src/commands/options/account.rs b/crates/sozo/src/commands/options/account.rs
@@ -8,13 +8,17 @@ use starknet::core::types::FieldElement;
 use starknet::providers::Provider;
 use starknet::signers::{LocalWallet, SigningKey};
 
+use super::{
+    DOJO_ACCOUNT_ADDRESS_ENV_VAR, DOJO_KEYSTORE_PASSWORD_ENV_VAR, DOJO_PRIVATE_KEY_ENV_VAR,
+};
+
 #[derive(Debug, Args)]
 #[command(next_help_heading = "Account options")]
 pub struct AccountOptions {
-    #[arg(long)]
+    #[arg(long, env = DOJO_ACCOUNT_ADDRESS_ENV_VAR)]
     pub account_address: Option<FieldElement>,
 
-    #[arg(long)]
+    #[arg(long, env = DOJO_PRIVATE_KEY_ENV_VAR)]
     #[arg(requires = "account_address")]
     #[arg(conflicts_with = "keystore_path")]
     #[arg(help_heading = "Signer options - RAW")]
@@ -27,7 +31,7 @@ pub struct AccountOptions {
     #[arg(help = "Use the keystore in the given folder or file.")]
     pub keystore_path: Option<String>,
 
-    #[arg(long = "password")]
+    #[arg(long = "password", env = DOJO_KEYSTORE_PASSWORD_ENV_VAR)]
     #[arg(value_name = "PASSWORD")]
     #[arg(requires = "keystore_path")]
     #[arg(help_heading = "Signer options - KEYSTORE")]
@@ -60,11 +64,8 @@ impl AccountOptions {
     }
 
     fn signer(&self, env_metadata: Option<&Environment>) -> Result<LocalWallet> {
-        if let Some(private_key) = self
-            .private_key
-            .as_deref()
-            .or_else(|| env_metadata.and_then(|env| env.private_key()))
-            .or(std::env::var("DOJO_PRIVATE_KEY").ok().as_deref())
+        if let Some(private_key) =
+            self.private_key.as_deref().or_else(|| env_metadata.and_then(|env| env.private_key()))
         {
             return Ok(LocalWallet::from_signing_key(SigningKey::from_secret_scalar(
                 FieldElement::from_str(private_key)?,
@@ -76,7 +77,6 @@ impl AccountOptions {
                 .keystore_password
                 .as_deref()
                 .or_else(|| env_metadata.and_then(|env| env.keystore_password()))
-                .or(std::env::var("DOJO_KEYSTORE_PASSWORD").ok().as_deref())
             {
                 return Ok(LocalWallet::from_signing_key(SigningKey::from_keystore(
                     path, password,
@@ -95,10 +95,7 @@ impl AccountOptions {
     fn account_address(&self, env_metadata: Option<&Environment>) -> Result<FieldElement> {
         if let Some(address) = self.account_address {
             Ok(address)
-        } else if let Some(address) = env_metadata
-            .and_then(|env| env.account_address())
-            .or(std::env::var("DOJO_ACCOUNT_ADDRESS").ok().as_deref())
-        {
+        } else if let Some(address) = env_metadata.and_then(|env| env.account_address()) {
             Ok(FieldElement::from_str(address)?)
         } else {
             Err(anyhow!(

diff --git a/crates/sozo/src/commands/options/mod.rs b/crates/sozo/src/commands/options/mod.rs
@@ -2,3 +2,8 @@ pub mod account;
 pub mod starknet;
 pub mod transaction;
 pub mod world;
+
+const STARKNET_RPC_URL_ENV_VAR: &str = "STARKNET_RPC_URL";
+const DOJO_PRIVATE_KEY_ENV_VAR: &str = "DOJO_PRIVATE_KEY";
+const DOJO_KEYSTORE_PASSWORD_ENV_VAR: &str = "DOJO_KEYSTORE_PASSWORD";
+const DOJO_ACCOUNT_ADDRESS_ENV_VAR: &str = "DOJO_ACCOUNT_ADDRESS";
diff --git a/crates/sozo/src/commands/options/starknet.rs b/crates/sozo/src/commands/options/starknet.rs
@@ -5,15 +5,15 @@ use starknet::providers::jsonrpc::HttpTransport;
 use starknet::providers::JsonRpcClient;
 use url::Url;
 
-const STARKNET_RPC_URL_ENV_VAR: &str = "STARKNET_RPC_URL";
+use super::STARKNET_RPC_URL_ENV_VAR;
 
 #[derive(Debug, Args)]
 #[command(next_help_heading = "Starknet options")]
 pub struct StarknetOptions {
-    #[arg(long, env = STARKNET_RPC_URL_ENV_VAR, default_value = "http://localhost:5050")]
+    #[arg(long, env = STARKNET_RPC_URL_ENV_VAR)]
     #[arg(value_name = "URL")]
     #[arg(help = "The Starknet RPC endpoint.")]
-    pub rpc_url: Url,
+    pub rpc_url: Option<Url>,
 }
 
 impl StarknetOptions {
@@ -26,11 +26,13 @@ impl StarknetOptions {
 
     // we dont check the env var because that would be handled by `clap`
     fn url(&self, env_metadata: Option<&Environment>) -> Result<Url> {
-        Ok(if let Some(url) = env_metadata.and_then(|env| env.rpc_url()) {
-            Url::parse(url)?
+        if let Some(url) = self.rpc_url.as_ref() {
+            Ok(url.clone())
+        } else if let Some(url) = env_metadata.and_then(|env| env.rpc_url()) {
+            Ok(Url::parse(url)?)
         } else {
-            self.rpc_url.clone()
-        })
+            Ok(Url::parse("http://localhost:5050").unwrap())
+        }
     }
 }
 
@@ -39,10 +41,11 @@ mod tests {
     use clap::Parser;
 
     use super::StarknetOptions;
-    use crate::commands::options::starknet::STARKNET_RPC_URL_ENV_VAR;
+    use crate::commands::options::STARKNET_RPC_URL_ENV_VAR;
 
     const ENV_RPC: &str = "http://localhost:7474/";
     const METADATA_RPC: &str = "http://localhost:6060/";
+    const DEFAULT_RPC: &str = "http://localhost:5050/";
 
     #[derive(clap::Parser)]
     struct Command {
@@ -77,6 +80,13 @@ mod tests {
             ..Default::default()
         };
         let cmd = Command::parse_from([""]);
-        assert_eq!(cmd.options.url(Some(&env_metadata)).unwrap().as_str(), METADATA_RPC);
+        assert_eq!(cmd.options.url(Some(&env_metadata)).unwrap().as_str(), ENV_RPC);
+    }
+
+    #[test]
+    fn exists_in_neither() {
+        let env_metadata = dojo_world::metadata::Environment::default();
+        let cmd = Command::parse_from([""]);
+        assert_eq!(cmd.options.url(Some(&env_metadata)).unwrap().as_str(), DEFAULT_RPC);
     }
 }