From 7c9fa61a13587e3ac014e942c1f068e0d9c8cfc7 Mon Sep 17 00:00:00 2001 From: edouardparis Date: Tue, 6 May 2025 17:04:20 +0200 Subject: [PATCH] Encapsulate update of settings file --- liana-gui/src/app/settings.rs | 121 ++++++++++++++------- liana-gui/src/app/state/settings/wallet.rs | 67 ++++++------ liana-gui/src/app/wallet.rs | 53 ++------- liana-gui/src/backup.rs | 17 ++- liana-gui/src/export.rs | 67 +++++++----- liana-gui/src/installer/mod.rs | 6 +- liana-gui/src/launcher.rs | 14 +-- liana-gui/src/loader.rs | 6 +- liana-gui/src/main.rs | 22 ++-- 9 files changed, 204 insertions(+), 169 deletions(-) diff --git a/liana-gui/src/app/settings.rs b/liana-gui/src/app/settings.rs index 261c983c..eca160d8 100644 --- a/liana-gui/src/app/settings.rs +++ b/liana-gui/src/app/settings.rs @@ -1,8 +1,12 @@ //! Settings is the module to handle the GUI settings file. //! The settings file is used by the GUI to store useful information. use std::collections::HashMap; -use std::fs::OpenOptions; -use std::io::Write; + +use async_fd_lock::LockWrite; +use std::io::SeekFrom; +use tokio::fs::OpenOptions; +use tokio::io::AsyncSeekExt; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; use liana::miniscript::bitcoin::bip32::Fingerprint; use liana_ui::component::form; @@ -17,49 +21,65 @@ use crate::{ pub const DEFAULT_FILE_NAME: &str = "settings.json"; -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Default, Deserialize, Serialize)] pub struct Settings { - pub wallets: Vec, + pub wallets: Vec, } -impl Settings { - pub fn from_file(network_dir: &NetworkDirectory) -> Result { - let mut path = network_dir.path().to_path_buf(); - path.push(DEFAULT_FILE_NAME); +pub async fn update_settings_file( + network_dir: &NetworkDirectory, + updater: F, +) -> Result<(), SettingsError> +where + F: FnOnce(Settings) -> Settings, +{ + let path = network_dir.path().join(DEFAULT_FILE_NAME); + let file_exists = tokio::fs::try_exists(&path).await.unwrap_or(false); - let config = std::fs::read(path) - .map_err(|e| match e.kind() { - std::io::ErrorKind::NotFound => SettingsError::NotFound, - _ => SettingsError::ReadingFile(format!("Reading settings file: {}", e)), - }) - .and_then(|file_content| { - serde_json::from_slice::(&file_content).map_err(|e| { - SettingsError::ReadingFile(format!("Parsing settings file: {}", e)) - }) - })?; - Ok(config) - } + let mut file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&path) + .await + .map_err(|e| SettingsError::ReadingFile(format!("Opening file: {}", e)))? + .lock_write() + .await + .map_err(|e| SettingsError::ReadingFile(format!("Locking file: {:?}", e)))?; - pub fn to_file(&self, network_dir: &NetworkDirectory) -> Result<(), SettingsError> { - let mut path = network_dir.path().to_path_buf(); - path.push(DEFAULT_FILE_NAME); + let settings = if file_exists { + let mut file_content = Vec::new(); + file.read_to_end(&mut file_content) + .await + .map_err(|e| SettingsError::ReadingFile(format!("Reading file content: {}", e)))?; - let content = serde_json::to_string_pretty(&self).map_err(|e| { - SettingsError::WritingFile(format!("Failed to serialize settings: {}", e)) - })?; + serde_json::from_slice::(&file_content) + .map_err(|e| SettingsError::ReadingFile(e.to_string()))? + } else { + Settings::default() + }; - let mut settings_file = OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .open(path) - .map_err(|e| SettingsError::WritingFile(e.to_string()))?; + let settings = updater(settings); - settings_file.write_all(content.as_bytes()).map_err(|e| { - tracing::warn!("failed to write to file: {:?}", e); - SettingsError::WritingFile(e.to_string()) - }) - } + let content = serde_json::to_vec_pretty(&settings) + .map_err(|e| SettingsError::WritingFile(format!("Failed to serialize settings: {}", e)))?; + + file.seek(SeekFrom::Start(0)).await.map_err(|e| { + SettingsError::WritingFile(format!("Failed to seek to start of file: {}", e)) + })?; + + file.write_all(&content).await.map_err(|e| { + tracing::warn!("failed to write to file: {:?}", e); + SettingsError::WritingFile(e.to_string()) + })?; + + file.inner_mut() + .set_len(content.len() as u64) + .await + .map_err(|e| SettingsError::WritingFile(format!("Failed to truncate file: {}", e)))?; + + Ok(()) } #[derive(Debug, Clone, Deserialize, Serialize)] @@ -84,7 +104,7 @@ impl AuthConfig { } #[derive(Debug, Clone, Deserialize, Serialize)] -pub struct WalletSetting { +pub struct WalletSettings { pub name: String, pub descriptor_checksum: String, // if wallet is using remote backend, then this information is stored on the remote backend @@ -101,7 +121,30 @@ pub struct WalletSetting { pub start_internal_bitcoind: Option, } -impl WalletSetting { +impl WalletSettings { + pub fn from_file( + network_dir: &NetworkDirectory, + selecter: F, + ) -> Result, SettingsError> + where + F: FnMut(&WalletSettings) -> bool, + { + let mut path = network_dir.path().to_path_buf(); + path.push(DEFAULT_FILE_NAME); + + std::fs::read(path) + .map_err(|e| match e.kind() { + std::io::ErrorKind::NotFound => SettingsError::NotFound, + _ => SettingsError::ReadingFile(format!("Reading settings file: {}", e)), + }) + .and_then(|file_content| { + serde_json::from_slice::(&file_content).map_err(|e| { + SettingsError::ReadingFile(format!("Parsing settings file: {}", e)) + }) + }) + .map(|cache| cache.wallets.into_iter().find(selecter)) + } + pub fn keys_aliases(&self) -> HashMap { let mut map = HashMap::new(); for key in self.keys.iter().filter(|k| !k.name.is_empty()) { diff --git a/liana-gui/src/app/state/settings/wallet.rs b/liana-gui/src/app/state/settings/wallet.rs index e7c59629..92f0f230 100644 --- a/liana-gui/src/app/state/settings/wallet.rs +++ b/liana-gui/src/app/state/settings/wallet.rs @@ -19,7 +19,7 @@ use crate::{ cache::Cache, error::Error, message::Message, - settings, + settings::{self, update_settings_file}, state::{export::ExportModal, State}, view, wallet::Wallet, @@ -428,26 +428,27 @@ async fn register_wallet( if daemon.backend() != DaemonBackend::RemoteBackend { let network_dir = data_dir.network_directory(network); - let mut settings = settings::Settings::from_file(&network_dir)?; let checksum = wallet.descriptor_checksum(); - - if let Some(wallet_setting) = settings - .wallets - .iter_mut() - .find(|w| w.descriptor_checksum == checksum) - { - if let Some(hw_config) = wallet_setting - .hardware_wallets + update_settings_file(&network_dir, |mut settings| { + if let Some(wallet_setting) = settings + .wallets .iter_mut() - .find(|cfg| cfg.kind == kind && cfg.fingerprint == fingerprint) + .find(|w| w.descriptor_checksum == checksum) { - *hw_config = hw_cfg.clone(); - } else { - wallet_setting.hardware_wallets.push(hw_cfg.clone()) + if let Some(hw_config) = wallet_setting + .hardware_wallets + .iter_mut() + .find(|cfg| cfg.kind == kind && cfg.fingerprint == fingerprint) + { + *hw_config = hw_cfg.clone(); + } else { + wallet_setting.hardware_wallets.push(hw_cfg.clone()) + } } - } - settings.to_file(&network_dir)?; + settings + }) + .await?; } let mut wallet = wallet.as_ref().clone(); @@ -478,24 +479,26 @@ pub async fn update_keys_aliases( ) -> Result, Error> { if daemon.backend() != DaemonBackend::RemoteBackend { let network_dir = data_dir.network_directory(network); - let mut settings = settings::Settings::from_file(&network_dir)?; let checksum = wallet.descriptor_checksum(); - if let Some(wallet_setting) = settings - .wallets - .iter_mut() - .find(|w| w.descriptor_checksum == checksum) - { - wallet_setting.keys = keys_aliases - .iter() - .map(|(master_fingerprint, name)| settings::KeySetting { - master_fingerprint: *master_fingerprint, - name: name.clone(), - provider_key: wallet.provider_keys.get(master_fingerprint).cloned(), - }) - .collect(); - } + update_settings_file(&network_dir, |mut settings| { + if let Some(wallet_setting) = settings + .wallets + .iter_mut() + .find(|w| w.descriptor_checksum == checksum) + { + wallet_setting.keys = keys_aliases + .iter() + .map(|(master_fingerprint, name)| settings::KeySetting { + master_fingerprint: *master_fingerprint, + name: name.clone(), + provider_key: wallet.provider_keys.get(master_fingerprint).cloned(), + }) + .collect(); + } - settings.to_file(&network_dir)?; + settings + }) + .await?; } let mut wallet = wallet.as_ref().clone(); diff --git a/liana-gui/src/app/wallet.rs b/liana-gui/src/app/wallet.rs index 30064663..1cfdcf10 100644 --- a/liana-gui/src/app/wallet.rs +++ b/liana-gui/src/app/wallet.rs @@ -102,48 +102,17 @@ impl Wallet { } pub fn load_from_settings(self, dir: &NetworkDirectory) -> Result { - let wallet = match settings::Settings::from_file(dir) { - Ok(settings) => { - if let Some(wallet_setting) = settings.wallets.first() { - self.with_name(wallet_setting.name.clone()) - .with_hardware_wallets(wallet_setting.hardware_wallets.clone()) - .with_key_aliases(wallet_setting.keys_aliases()) - .with_provider_keys(wallet_setting.provider_keys()) - } else { - self - } - } - Err(settings::SettingsError::NotFound) => { - let s = settings::Settings { - wallets: vec![settings::WalletSetting { - name: self.name.clone(), - hardware_wallets: self.hardware_wallets.clone(), - keys: self - .keys_aliases - .clone() - .into_iter() - .map(|(master_fingerprint, name)| settings::KeySetting { - name, - master_fingerprint, - provider_key: self.provider_keys.get(&master_fingerprint).cloned(), - }) - .collect(), - descriptor_checksum: self.descriptor_checksum(), - // Only local wallet from previous version of Liana GUI may not have a - // settings.json file - remote_backend_auth: None, - start_internal_bitcoind: None, - }], - }; - - tracing::info!("Settings file not found, creating one"); - s.to_file(dir)?; - self - } - Err(e) => return Err(e.into()), - }; - - Ok(wallet) + if let Some(wallet_settings) = settings::WalletSettings::from_file(dir, |w| { + w.descriptor_checksum == self.descriptor_checksum() + })? { + Ok(self + .with_key_aliases(wallet_settings.keys_aliases()) + .with_provider_keys(wallet_settings.provider_keys()) + .with_name(wallet_settings.name) + .with_hardware_wallets(wallet_settings.hardware_wallets)) + } else { + Ok(self) + } } pub fn load_hotsigners( diff --git a/liana-gui/src/backup.rs b/liana-gui/src/backup.rs index 0dc4ae64..11a9aa61 100644 --- a/liana-gui/src/backup.rs +++ b/liana-gui/src/backup.rs @@ -18,7 +18,11 @@ use std::{ use tokio::sync::mpsc::UnboundedSender; use crate::{ - app::{settings::Settings, wallet::Wallet, Config}, + app::{ + settings::{Settings, WalletSettings}, + wallet::Wallet, + Config, + }, daemon::{model::HistoryTransaction, Daemon, DaemonBackend, DaemonError}, dir::LianaDirectory, export::Progress, @@ -195,12 +199,15 @@ impl Backup { let keys = wallet.keys(); let network_dir = datadir.network_directory(network); - let settings = Settings::from_file(&network_dir).map_err(|_| Error::SettingsFromFile)?; - if settings.wallets.len() == 1 { - if let Ok(settings) = serde_json::to_value(settings.wallets[0].clone()) { + if let Some(settings) = WalletSettings::from_file(&network_dir, |settings| { + wallet.descriptor_checksum() == settings.descriptor_checksum + }) + .map_err(|_| Error::SettingsFromFile)? + { + if let Ok(settings) = serde_json::to_value(settings) { proprietary.insert(SETTINGS_KEY.to_string(), settings); } - } + }; if let Ok(config) = serde_json::to_value((*config).clone()) { proprietary.insert(CONFIG_KEY.to_string(), config); diff --git a/liana-gui/src/export.rs b/liana-gui/src/export.rs index 0c52209a..46c59505 100644 --- a/liana-gui/src/export.rs +++ b/liana-gui/src/export.rs @@ -34,7 +34,7 @@ use iced::futures::{SinkExt, Stream}; use crate::{ app::{ cache::Cache, - settings::{self, KeySetting, Settings}, + settings::{self, update_settings_file, KeySetting, WalletSettings}, view, wallet::Wallet, Config, @@ -815,35 +815,34 @@ pub async fn import_backup( .and_then(|c| c.data_directory()) .ok_or(Error::BackupImport("Failed to get Daemon config".into()))?; + let descriptor_checksum = descriptor + .to_string() + .split_once('#') + .map(|(_, checksum)| checksum) + .unwrap() + .to_string(); + // check if key aliases can be imported w/o conflict let mut write_aliases = true; let settings = if !account.keys.is_empty() { // TODO: change lianad_datadir is common to gui datadir only for legacy wallet before // multiple wallet let network_dir = NetworkDirectory::new(lianad_datadir.path().to_path_buf()); - let settings = match Settings::from_file(&network_dir) { - Ok(s) => s, - Err(_) => { + let wallet_settings = match WalletSettings::from_file(&network_dir, |w| { + w.descriptor_checksum == descriptor_checksum + }) { + Ok(Some(s)) => s, + _ => { return Err(Error::BackupImport("Failed to get App Settings".into())); } }; - let settings_aliases: HashMap<_, _> = match settings.wallets.len() { - 1 => settings - .wallets - .first() - .expect("already checked") - .keys - .clone() - .into_iter() - .map(|s| (s.master_fingerprint, s)) - .collect(), - _ => { - return Err(Error::BackupImport( - "Settings.wallets.len() is not 1".into(), - )); - } - }; + let settings_aliases: HashMap<_, _> = wallet_settings + .keys + .clone() + .into_iter() + .map(|s| (s.master_fingerprint, s)) + .collect(); let (ack_sender, mut ack_receiver) = channel(1); let mut conflict = false; @@ -867,7 +866,7 @@ pub async fn import_backup( }; } - Some((settings, settings_aliases)) + Some(settings_aliases) } else { None }; @@ -925,7 +924,7 @@ pub async fn import_backup( } // update aliases if no conflict or user ACK - if let (true, Some((mut settings, mut settings_aliases))) = (write_aliases, settings) { + if let (true, Some(mut settings_aliases)) = (write_aliases, settings) { for (k, v) in &account.keys { if let Some(ks) = KeySetting::from_backup( v.alias.clone().unwrap_or("".into()), @@ -938,11 +937,25 @@ pub async fn import_backup( } } - settings.wallets.get_mut(0).expect("already checked").keys = - settings_aliases.clone().into_values().collect(); - let network_dir = NetworkDirectory::new(lianad_datadir.path().to_path_buf()); - if settings.to_file(&network_dir).is_err() { - return Err(Error::BackupImport("Failed to import keys aliases".into())); + if let Err(e) = update_settings_file( + &NetworkDirectory::new(lianad_datadir.path().to_path_buf()), + |mut settings| { + if let Some(wallet) = settings + .wallets + .iter_mut() + .find(|w| w.descriptor_checksum == descriptor_checksum) + { + wallet.keys = settings_aliases.clone().into_values().collect(); + } + settings + }, + ) + .await + { + return Err(Error::BackupImport(format!( + "Failed to import keys aliases: {}", + e + ))); } else { // Update wallet state send_progress!(sender, UpdateAliases(settings_aliases)); diff --git a/liana-gui/src/installer/mod.rs b/liana-gui/src/installer/mod.rs index 768d2402..2ebce40b 100644 --- a/liana-gui/src/installer/mod.rs +++ b/liana-gui/src/installer/mod.rs @@ -23,7 +23,7 @@ use std::sync::{Arc, Mutex}; use crate::{ app::{ config as gui_config, - settings::{self as gui_settings, AuthConfig, Settings, SettingsError, WalletSetting}, + settings::{self as gui_settings, AuthConfig, Settings, SettingsError, WalletSettings}, wallet::wallet_name, }, backup, @@ -665,7 +665,7 @@ pub async fn extract_remote_gui_settings(ctx: &Context, backend: &BackendWalletC .to_string(); Settings { - wallets: vec![WalletSetting { + wallets: vec![WalletSettings { name: wallet_name(descriptor), descriptor_checksum, keys: Vec::new(), @@ -702,7 +702,7 @@ pub fn extract_local_gui_settings(ctx: &Context) -> Settings { }) .collect(); Settings { - wallets: vec![WalletSetting { + wallets: vec![WalletSettings { name: wallet_name(descriptor), descriptor_checksum, keys: ctx.keys.values().cloned().collect(), diff --git a/liana-gui/src/launcher.rs b/liana-gui/src/launcher.rs index 42e56b95..0fa95e16 100644 --- a/liana-gui/src/launcher.rs +++ b/liana-gui/src/launcher.rs @@ -513,14 +513,12 @@ async fn check_network_datadir(path: NetworkDirectory) -> Result })?; } - if let Ok(settings) = app::settings::Settings::from_file(&path) { - if let Some(wallet) = settings.wallets.first().cloned() { - return Ok(State::Wallet { - name: Some(wallet.name), - checksum: Some(wallet.descriptor_checksum), - email: wallet.remote_backend_auth.map(|auth| auth.email), - }); - } + if let Ok(Some(wallet)) = app::settings::WalletSettings::from_file(&path, |_w| true) { + return Ok(State::Wallet { + name: Some(wallet.name), + checksum: Some(wallet.descriptor_checksum), + email: wallet.remote_backend_auth.map(|auth| auth.email), + }); } Ok(State::Wallet { name: None, diff --git a/liana-gui/src/loader.rs b/liana-gui/src/loader.rs index 2631c81e..c34c5dbe 100644 --- a/liana-gui/src/loader.rs +++ b/liana-gui/src/loader.rs @@ -23,7 +23,7 @@ use lianad::{ }; use crate::app; -use crate::app::settings::WalletSetting; +use crate::app::settings::WalletSettings; use crate::backup::Backup; use crate::dir::LianaDirectory; use crate::export::RestoreBackupError; @@ -59,7 +59,7 @@ pub struct Loader { pub internal_bitcoind: Option, pub waiting_daemon_bitcoind: bool, pub backup: Option, - pub wallet_setting: Option, + pub wallet_setting: Option, step: Step, } @@ -119,7 +119,7 @@ impl Loader { network: bitcoin::Network, internal_bitcoind: Option, backup: Option, - wallet_setting: Option, + wallet_setting: Option, ) -> (Self, Task) { let path = socket_path(&datadir_path, network); ( diff --git a/liana-gui/src/main.rs b/liana-gui/src/main.rs index 28397bfb..42dc9cc8 100644 --- a/liana-gui/src/main.rs +++ b/liana-gui/src/main.rs @@ -208,10 +208,12 @@ impl GUI { .unwrap_or_else(|| cfg.log_level().unwrap_or(LevelFilter::INFO)), ); let network_dir = datadir_path.network_directory(network); - if let Ok(settings) = app::settings::Settings::from_file(&network_dir) { - let setting = settings.wallets.into_iter().next(); - if let Some(setting) = - setting.as_ref().and_then(|w| w.remote_backend_auth.clone()) + if let Ok(settings) = + app::settings::WalletSettings::from_file(&network_dir, |_w| true) + { + if let Some(setting) = settings + .as_ref() + .and_then(|w| w.remote_backend_auth.clone()) { let (login, command) = login::LianaLiteLogin::new(datadir_path, network, setting); @@ -219,7 +221,7 @@ impl GUI { command.map(|msg| Message::Login(Box::new(msg))) } else { let (loader, command) = - Loader::new(datadir_path, cfg, network, None, None, setting); + Loader::new(datadir_path, cfg, network, None, None, settings); self.state = State::Loader(Box::new(loader)); command.map(|msg| Message::Load(Box::new(msg))) } @@ -279,11 +281,11 @@ impl GUI { (State::Installer(i), Message::Install(msg)) => { if let installer::Message::Exit(path, internal_bitcoind, remove_log) = *msg { let network_dir = i.datadir.network_directory(i.network); - let settings = app::settings::Settings::from_file(&network_dir) + let settings = app::settings::WalletSettings::from_file(&network_dir, |_| true) .expect("A settings file was created"); - let setting = settings.wallets.into_iter().next(); - if let Some(setting) = - setting.as_ref().and_then(|w| w.remote_backend_auth.clone()) + if let Some(setting) = settings + .as_ref() + .and_then(|w| w.remote_backend_auth.clone()) { let (login, command) = login::LianaLiteLogin::new(i.datadir.clone(), i.network, setting); @@ -308,7 +310,7 @@ impl GUI { i.network, internal_bitcoind, i.context.backup.take(), - setting, + settings, ); self.state = State::Loader(Box::new(loader)); command.map(|msg| Message::Load(Box::new(msg)))