Address account domain review feedback

Author: integraledelebesgue

crates/sncast/src/accounts/error.rs

@@ -6,6 +6,9 @@ pub enum AccountsError {
     #[error("{kind} cannot be empty")]
     EmptyIdentifier { kind: &'static str },
 
+    #[error("invalid account type `{account_type}`")]
+    InvalidAccountType { account_type: String },
+
     #[error("account is missing required field `{field}` for {operation}")]
     MissingField {
         field: &'static str,

crates/sncast/src/accounts/model.rs

@@ -3,7 +3,6 @@ use std::collections::BTreeMap;
 use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 
-use anyhow::anyhow;
 use clap::ValueEnum;
 use serde::{Deserialize, Serialize};
 use starknet_types_core::felt::Felt;
@@ -16,20 +15,23 @@ use crate::signers::SignerSpec;
 pub enum AccountType {
     #[serde(rename = "open_zeppelin")]
     OpenZeppelin,
+    // Backwards compatibility with pre-rebranding account files.
     #[serde(alias = "argent")]
     Ready,
     Braavos,
 }
 
 impl FromStr for AccountType {
-    type Err = anyhow::Error;
+    type Err = AccountsError;
 
     fn from_str(value: &str) -> Result<Self, Self::Err> {
         match value {
             "open_zeppelin" | "open-zeppelin" | "oz" => Ok(Self::OpenZeppelin),
             "ready" => Ok(Self::Ready),
             "braavos" => Ok(Self::Braavos),
-            account_type => Err(anyhow!("Invalid account type = {account_type}")),
+            account_type => Err(AccountsError::InvalidAccountType {
+                account_type: account_type.to_owned(),
+            }),
         }
     }
 }
@@ -48,7 +50,7 @@ macro_rules! identifier {
         impl $name {
             pub fn new(value: impl Into<String>) -> Result<Self, AccountsError> {
                 let value = value.into();
-                if value.is_empty() {
+                if value.trim().is_empty() {
                     return Err(AccountsError::EmptyIdentifier { kind: $kind });
                 }
                 Ok(Self(value))
@@ -208,7 +210,7 @@ impl<'a> DeployableAccountRecord<'a> {
     }
 }
 
-fn required<T: Copy>(
+fn required<T>(
     value: Option<T>,
     field: &'static str,
     operation: &'static str,
@@ -240,6 +242,8 @@ mod tests {
     fn identifiers_reject_empty_values() {
         assert!(AccountName::new("").is_err());
         assert!(NetworkName::new("").is_err());
+        assert!(AccountName::new("  \t").is_err());
+        assert!(NetworkName::new("\n").is_err());
     }
 
     #[test]
@@ -252,6 +256,7 @@ mod tests {
 
         let registry = AccountRegistry::new(networks);
         assert!(registry.account("alpha-sepolia", "alice").is_some());
+        assert!(registry.account("alpha-sepolia", "bob").is_none());
     }
 
     #[test]

crates/sncast/src/signers/spec.rs

@@ -1,4 +1,4 @@
-use camino::Utf8PathBuf;
+use camino::{Utf8Path, Utf8PathBuf};
 use starknet_rust::signers::DerivationPath;
 use starknet_types_core::felt::Felt;
 
@@ -57,8 +57,8 @@ impl KeystoreSpec {
     }
 
     #[must_use]
-    pub fn path(&self) -> &Utf8PathBuf {
-        &self.path
+    pub fn path(&self) -> &Utf8Path {
+        self.path.as_path()
     }
 
     #[must_use]