-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v0.2-dev into master #32
base: master
Are you sure you want to change the base?
Changes from all commits
2664c03
1c720c2
f1b193c
7163701
8c03946
aecdc8d
42c0e41
35d1b42
4c48948
dba59a7
529b679
f0eac76
08388e0
c92a177
9ae9a07
7b82260
2f571a9
0ffa920
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,56 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use directories::ProjectDirs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use std::fs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use std::path::{Path, PathBuf}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const QUALIFIER: &str = ""; // Typically empty on macOS and Linux | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const ORGANIZATION: &str = ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const APPLICATION: &str = "DashEvoTool"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub fn app_user_data_dir_path() -> Result<PathBuf, std::io::Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let proj_dirs = ProjectDirs::from(QUALIFIER, ORGANIZATION, APPLICATION) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.ok_or_else(|| std::io::Error::new( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::io::ErrorKind::NotFound, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Failed to determine project directories", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
))?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(proj_dirs.config_dir().to_path_buf()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub fn create_app_user_data_directory_if_not_exists() -> Result<(), std::io::Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let app_data_dir = app_user_data_dir_path()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fs::create_dir_all(&app_data_dir)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Verify directory permissions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let metadata = fs::metadata(&app_data_dir)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if !metadata.is_dir() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Err(std::io::Error::new( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::io::ErrorKind::Other, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Created path is not a directory", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+17
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding permission checks for Unix systems. While the function correctly creates and verifies the directory, it might be worth adding permission checks on Unix systems to ensure the directory is readable and writable by the current user. if !metadata.is_dir() {
return Err(std::io::Error::new(
std::io::ErrorKind::Other,
"Created path is not a directory",
));
}
+ #[cfg(unix)]
+ {
+ use std::os::unix::fs::PermissionsExt;
+ let perms = metadata.permissions();
+ if perms.mode() & 0o600 != 0o600 {
+ return Err(std::io::Error::new(
+ std::io::ErrorKind::PermissionDenied,
+ "Insufficient directory permissions",
+ ));
+ }
+ }
Ok(()) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub fn app_user_data_file_path(filename: &str) -> Result<PathBuf, std::io::Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if filename.is_empty() || filename.contains(std::path::is_separator) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Err(std::io::Error::new( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::io::ErrorKind::InvalidInput, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Invalid filename", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let app_data_dir = app_user_data_dir_path()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(app_data_dir.join(filename)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+32
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance filename validation. While the function checks for path separators, consider adding validation for:
+const MAX_FILENAME_LENGTH: usize = 255;
+
pub fn app_user_data_file_path(filename: &str) -> Result<PathBuf, std::io::Error> {
- if filename.is_empty() || filename.contains(std::path::is_separator) {
+ if filename.is_empty()
+ || filename.contains(std::path::is_separator)
+ || filename.len() > MAX_FILENAME_LENGTH
+ || filename.chars().any(|c| c.is_control())
+ || cfg!(windows) && is_windows_reserved_filename(filename)
+ {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
- "Invalid filename",
+ "Invalid filename: must not be empty, contain separators, exceed length limit, or be a reserved name",
));
}
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub fn copy_env_file_if_not_exists() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let app_data_dir = app_user_data_dir_path().expect("Failed to determine application data directory"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let env_file = app_data_dir.join(".env".to_string()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if env_file.exists() && env_file.is_file() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let env_example_file = PathBuf::from(".env.example"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let target_env_file_path = app_user_data_file_path(".env").expect("should create target env file path"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fs::copy( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
&env_example_file, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
target_env_file_path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.expect("Failed to copy main net env file"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+43
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Improve error handling and security in env file copying. Several issues need attention:
Here's a safer implementation: -pub fn copy_env_file_if_not_exists() {
+pub fn copy_env_file_if_not_exists() -> Result<(), std::io::Error> {
let app_data_dir = app_user_data_dir_path()
- .expect("Failed to determine application data directory");
+ ?;
let env_file = app_data_dir.join(".env");
if env_file.exists() && env_file.is_file() {
+ return Ok(());
} else {
let env_example_file = PathBuf::from(".env.example");
- let target_env_file_path = app_user_data_file_path(".env")
- .expect("should create target env file path");
+ if !env_example_file.exists() {
+ return Err(std::io::Error::new(
+ std::io::ErrorKind::NotFound,
+ "Template .env.example file not found",
+ ));
+ }
+ let target_env_file_path = app_user_data_file_path(".env")?;
- fs::copy(
+ fs::copy(
&env_example_file,
target_env_file_path,
- )
- .expect("Failed to copy main net env file");
+ )?;
}
+ Ok(())
} 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
use std::str::FromStr; | ||
|
||
use crate::app_dir::app_user_data_file_path; | ||
use dash_sdk::dapi_client::AddressList; | ||
use dash_sdk::dpp::dashcore::Network; | ||
use dash_sdk::sdk::Uri; | ||
|
@@ -59,7 +60,8 @@ impl Config { | |
/// Loads the configuration for all networks from environment variables and `.env` file. | ||
pub fn load() -> Result<Self, ConfigError> { | ||
// Load the .env file if available | ||
if let Err(err) = dotenvy::from_path(".env") { | ||
let env_file_path = app_user_data_file_path(".env").expect("should create .env file path"); | ||
if let Err(err) = dotenvy::from_path(env_file_path) { | ||
Comment on lines
+63
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid potential panic with The current implementation uses Consider propagating the error instead: - let env_file_path = app_user_data_file_path(".env").expect("should create .env file path");
- if let Err(err) = dotenvy::from_path(env_file_path) {
+ let env_file_path = app_user_data_file_path(".env").map_err(|e| {
+ ConfigError::LoadError(format!("Failed to create .env file path: {}", e))
+ })?;
+ if let Err(err) = dotenvy::from_path(&env_file_path) { This change:
|
||
tracing::warn!( | ||
?err, | ||
"Failed to load .env file. Continuing with environment variables." | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,16 +3,17 @@ use crate::context_provider::Provider; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::database::Database; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::model::contested_name::ContestedName; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::model::qualified_contract::QualifiedContract; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::model::qualified_identity::QualifiedIdentity; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::model::qualified_identity::{DPNSNameInfo, QualifiedIdentity}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::model::wallet::Wallet; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::sdk_wrapper::initialize_sdk; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::ui::RootScreenType; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::dashcore_rpc::{Auth, Client}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::dpp::dashcore::Network; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::dpp::identity::accessors::IdentityGettersV0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::dpp::identity::Identity; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::dpp::system_data_contracts::{load_system_data_contract, SystemDataContract}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::dpp::version::PlatformVersion; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::platform::DataContract; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::platform::{DataContract, Identifier}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use dash_sdk::Sdk; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use rusqlite::Result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use std::sync::atomic::AtomicBool; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -27,6 +28,7 @@ pub struct AppContext { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) sdk: Sdk, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) config: NetworkConfig, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) dpns_contract: Arc<DataContract>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) withdraws_contract: Arc<DataContract>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) core_client: Client, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) has_wallet: AtomicBool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) wallets: RwLock<Vec<Arc<RwLock<Wallet>>>>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -55,6 +57,10 @@ impl AppContext { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
load_system_data_contract(SystemDataContract::DPNS, PlatformVersion::latest()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.expect("expected to load dpns contract"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let withdrawal_contract = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
load_system_data_contract(SystemDataContract::Withdrawals, PlatformVersion::latest()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.expect("expected to get withdrawal contract"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let addr = format!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"http://{}:{}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
network_config.core_host, network_config.core_rpc_port | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -83,6 +89,7 @@ impl AppContext { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sdk, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
config: network_config, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dpns_contract: Arc::new(dpns_contract), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
withdraws_contract: Arc::new(withdrawal_contract), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
core_client, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
has_wallet: (!wallets.is_empty()).into(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
wallets: RwLock::new(wallets), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -130,6 +137,29 @@ impl AppContext { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.db.get_ongoing_contested_names(self) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Fetches the local identities from the database and then maps them to their DPNS names. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub fn local_dpns_names(&self) -> Result<Vec<(Identifier, DPNSNameInfo)>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let qualified_identities = self.db.get_local_qualified_identities(self)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Map each identity's DPNS names to (Identifier, DPNSNameInfo) tuples | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let dpns_names = qualified_identities | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.flat_map(|qualified_identity| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
qualified_identity.dpns_names.iter().map(|dpns_name_info| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
qualified_identity.identity.id(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DPNSNameInfo { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: dpns_name_info.name.clone(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
acquired_at: dpns_name_info.acquired_at, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.collect::<Vec<(Identifier, DPNSNameInfo)>>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ok(dpns_names) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+140
to
+161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider optimizing the implementation and improving documentation. While the implementation is functional, there are several potential improvements:
pub fn local_dpns_names(&self) -> Result<Vec<(Identifier, DPNSNameInfo)>> {
- let qualified_identities = self.db.get_local_qualified_identities(self)?;
-
- // Map each identity's DPNS names to (Identifier, DPNSNameInfo) tuples
- let dpns_names = qualified_identities
- .iter()
- .flat_map(|qualified_identity| {
- qualified_identity.dpns_names.iter().map(|dpns_name_info| {
- (
- qualified_identity.identity.id(),
- DPNSNameInfo {
- name: dpns_name_info.name.clone(),
- acquired_at: dpns_name_info.acquired_at,
- },
- )
- })
- })
- .collect::<Vec<(Identifier, DPNSNameInfo)>>();
-
- Ok(dpns_names)
+ /// Returns a vector of tuples containing identity identifiers and their associated DPNS names.
+ /// Each identity can have multiple DPNS names, so there might be multiple entries for the same identifier.
+ self.db
+ .get_local_qualified_identities(self)?
+ .into_iter()
+ .flat_map(|identity| {
+ identity.dpns_names.into_iter().map(move |name_info| {
+ (
+ identity.identity.id(),
+ name_info,
+ )
+ })
+ })
+ .collect() The suggested changes:
📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/// Updates the `start_root_screen` in the settings table | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub fn update_settings(&self, root_screen_type: RootScreenType) -> Result<()> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.db | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,6 +64,12 @@ impl From<Purpose> for EncryptedPrivateKeyTarget { | |
} | ||
} | ||
|
||
#[derive(Debug, Encode, Decode, Clone, PartialEq)] | ||
pub struct DPNSNameInfo { | ||
pub name: String, | ||
pub acquired_at: u64, | ||
} | ||
|
||
#[derive(Debug, Encode, Decode, Clone, PartialEq)] | ||
pub struct QualifiedIdentity { | ||
pub identity: Identity, | ||
|
@@ -74,6 +80,7 @@ pub struct QualifiedIdentity { | |
pub alias: Option<String>, | ||
pub encrypted_private_keys: | ||
BTreeMap<(EncryptedPrivateKeyTarget, KeyID), (IdentityPublicKey, [u8; 32])>, | ||
pub dpns_names: Vec<DPNSNameInfo>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider encapsulating DPNS names management. The public
Example implementation: // In QualifiedIdentity struct
- pub dpns_names: Vec<DPNSNameInfo>,
+ dpns_names: HashSet<DPNSNameInfo>,
// Add these methods
impl QualifiedIdentity {
pub fn add_dpns_name(&mut self, name: String) -> Result<(), &'static str> {
// Add validation logic here
self.dpns_names.insert(DPNSNameInfo {
name,
acquired_at: std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs(),
});
Ok(())
}
pub fn get_dpns_names(&self) -> &HashSet<DPNSNameInfo> {
&self.dpns_names
}
} |
||
} | ||
|
||
impl Signer for QualifiedIdentity { | ||
|
@@ -271,6 +278,7 @@ impl From<Identity> for QualifiedIdentity { | |
identity_type: IdentityType::User, | ||
alias: None, | ||
encrypted_private_keys: Default::default(), | ||
dpns_names: vec![], | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent error handling between function calls
In the
AppState::new()
method, the call tocreate_app_user_data_directory_if_not_exists()
uses.expect("Failed to create app user_data directory")
to handle potential errors by panicking with a message. However, the subsequent call tocopy_env_file_if_not_exists()
does not have any error handling. Ifcopy_env_file_if_not_exists()
can fail or return aResult
, it's important to handle the error consistently to prevent silent failures.Apply this diff to handle potential errors from
copy_env_file_if_not_exists()
: