diff --git a/CHANGELOG.md b/CHANGELOG.md index 7753f2037..7d47df411 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **Automatic Platform node refresh during upgrades**: migrating a pre-1.0 + installation now triggers a best-effort Mainnet or Testnet node refresh. + Failed attempts retry on later launches until fresh addresses are saved and + the app reconnects, so upgrading users do not need to find the manual action. + - **Search tags in the "Send to" field**: type `type:core`, `type:platform`, `type:shielded`, or `wallet:` to narrow the address suggestions instead of scrolling through everything; plain words still search like diff --git a/docs/user-stories.md b/docs/user-stories.md index c2fccf397..d731a6ea9 100644 --- a/docs/user-stories.md +++ b/docs/user-stories.md @@ -1261,6 +1261,9 @@ As a user, I want to fetch a fresh list of Platform (DAPI) node addresses from D - "Refresh DAPI endpoints" action available on Mainnet and Testnet. - Confirmation prompt before replacing an existing configured address set. - New addresses are persisted to config and the SDK reinitialized without an app restart. +- A pre-1.0 migration triggers a silent, best-effort Mainnet or Testnet address + refresh; failures retry on later launches until addresses are saved, while + the manual action keeps its existing success message. ### NET-017: View live connection status (indicator and Platform endpoints) [Implemented] **Persona:** Alex, Priya, Jordan diff --git a/src/backend_task/dapi_discovery.rs b/src/backend_task/dapi_discovery.rs index 7a0b0c81a..3100e87a6 100644 --- a/src/backend_task/dapi_discovery.rs +++ b/src/backend_task/dapi_discovery.rs @@ -27,6 +27,10 @@ use std::num::NonZeroUsize; use std::str::FromStr; use std::time::Duration; +use crate::backend_task::error::TaskError; +use crate::config::{CONFIG_PERSISTENCE_LOCK, Config}; +use crate::context::AppContext; + /// Errors from DAPI address resolution and discovery. #[derive(Debug, thiserror::Error)] pub enum DapiDiscoveryError { @@ -133,3 +137,38 @@ pub async fn discover_and_format( let csv = urls.join(","); Ok((count, csv)) } + +/// Persist DAPI addresses and publish them to the active network configuration. +pub(crate) fn persist_dapi_addresses( + app_context: &AppContext, + addresses_csv: String, +) -> Result<(), TaskError> { + persist_dapi_addresses_inner(app_context, addresses_csv, || {}, || {}) +} + +fn persist_dapi_addresses_inner( + app_context: &AppContext, + addresses_csv: String, + before_save: impl FnOnce(), + after_save: impl FnOnce(), +) -> Result<(), TaskError> { + let _persistence_guard = CONFIG_PERSISTENCE_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let network = app_context.network(); + before_save(); + Config::save_dapi_addresses(app_context.data_dir(), network, &addresses_csv)?; + app_context.config.write()?.dapi_addresses = Some(addresses_csv); + after_save(); + Ok(()) +} + +#[cfg(test)] +pub(crate) fn persist_dapi_addresses_with_hook( + app_context: &AppContext, + addresses_csv: String, + before_save: impl FnOnce(), + after_save: impl FnOnce(), +) -> Result<(), TaskError> { + persist_dapi_addresses_inner(app_context, addresses_csv, before_save, after_save) +} diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 50c343d57..99416d1f4 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -2125,13 +2125,17 @@ pub enum TaskError { #[error("Could not connect to {network}. Check your network configuration and retry.")] NetworkContextCreationFailed { network: Network }, + /// A DAPI refresh completed after its network context was removed. + #[error( + "The node addresses could not be applied because the selected network changed. Select the network and retry." + )] + DapiConfigContextUnavailable { network: Network }, + // ────────────────────────────────────────────────────────────────────────── // Migration errors // ────────────────────────────────────────────────────────────────────────── - /// Surfaced when wallet/identity/DashPay storage is being upgraded - /// from the legacy `data.db` and a task tried to touch it before - /// the migration finished. The user can retry once the migration - /// banner clears. + /// Surfaced while the legacy-data upgrade or its best-effort DAPI refresh + /// still owns the migration guard. The user can retry after a short wait. #[error("The storage update is still running. Please wait a moment and try again.")] WalletStorageNotReady, diff --git a/src/backend_task/migration/finish_unwire.rs b/src/backend_task/migration/finish_unwire.rs index db7582f82..1a9ef72a9 100644 --- a/src/backend_task/migration/finish_unwire.rs +++ b/src/backend_task/migration/finish_unwire.rs @@ -7,6 +7,7 @@ //! launches **on the same network**. use std::collections::BTreeSet; +use std::future::Future; use std::sync::Arc; use dash_sdk::dpp::dashcore::Network; @@ -15,6 +16,7 @@ use dash_sdk::platform::Identifier; use rusqlite::Connection; use serde::{Deserialize, Serialize}; +use crate::backend_task::dapi_discovery::persist_dapi_addresses; use crate::backend_task::error::TaskError; use crate::context::AppContext; use crate::context::migration_status::{MigrationState, MigrationStep}; @@ -43,6 +45,15 @@ pub fn sentinel_key_for(network: Network) -> String { ) } +/// Per-network completion key for automatic DAPI refresh during an upgrade. +/// +/// This pass cannot share the wallet-drain sentinel: a prior launch may finish +/// moving wallets while node discovery is temporarily unavailable. Keeping a +/// separate key lets discovery retry without rerunning or gating fund recovery. +pub fn dapi_refresh_sentinel_key_for(network: Network) -> String { + format!("det:migration:dapi_refresh:{}:v1", network_prefix(network)) +} + /// Tables sniffed during detection. Any non-empty row count flips the /// migration into the `Running` state. Ordered so the cheapest check /// (the single-row `wallet` table) runs first. @@ -373,6 +384,145 @@ fn validate_saved_data_for_migration(app_context: &AppContext) -> Result<(), Mig validate_legacy_database_version(version) } +#[cfg(not(test))] +async fn refresh_dapi_nodes_once(app_context: &Arc) { + refresh_dapi_nodes_once_with(app_context, |network| async move { + crate::backend_task::dapi_discovery::discover_and_format(network, None).await + }) + .await; +} + +#[cfg(test)] +async fn refresh_dapi_nodes_once(app_context: &Arc) { + refresh_dapi_nodes_once_with(app_context, |_| async { + Err(crate::backend_task::dapi_discovery::DapiDiscoveryError::Timeout) + }) + .await; +} + +/// Runs one best-effort refresh with an injected discovery operation. +/// +/// Run-triggered passes still hold their per-context `migration_run`, while migration and manual refresh +/// share a process-wide guard across whole-file config persistence, including different network contexts. +async fn refresh_dapi_nodes_once_with(app_context: &Arc, discover: D) +where + D: FnOnce(Network) -> F, + F: Future< + Output = Result<(usize, String), crate::backend_task::dapi_discovery::DapiDiscoveryError>, + >, +{ + refresh_dapi_nodes_once_with_legacy_check(app_context, discover, detect_legacy_rows).await; +} + +async fn refresh_dapi_nodes_once_with_legacy_check( + app_context: &Arc, + discover: D, + detect_legacy: L, +) where + D: FnOnce(Network) -> F, + F: Future< + Output = Result<(usize, String), crate::backend_task::dapi_discovery::DapiDiscoveryError>, + >, + L: FnOnce(&AppContext) -> Result, +{ + let network = app_context.network; + let app_kv = app_context.app_kv(); + let sentinel_key = dapi_refresh_sentinel_key_for(network); + + match app_kv.get::(DetScope::Global, &sentinel_key) { + Ok(Some(_)) => return, + Ok(None) => {} + Err(error) => { + tracing::warn!( + target = "migration::finish_unwire", + ?network, + ?error, + "Could not read the automatic DAPI refresh sentinel; node discovery will retry on the next launch", + ); + return; + } + } + + if !matches!(network, Network::Mainnet | Network::Testnet) { + write_dapi_refresh_completion(&app_kv, &sentinel_key, network, 0); + return; + } + + match detect_legacy(app_context) { + Ok(true) => {} + Ok(false) => { + write_dapi_refresh_completion(&app_kv, &sentinel_key, network, 0); + return; + } + Err(error) => { + tracing::warn!( + target = "migration::finish_unwire", + ?network, + ?error, + "Could not inspect the previous version's data for automatic DAPI refresh; node discovery will retry on the next launch", + ); + return; + } + } + + let (count, addresses_csv) = match discover(network).await { + Ok(result) => result, + Err(error) => { + tracing::warn!( + target = "migration::finish_unwire", + ?network, + ?error, + "Automatic DAPI node discovery failed during migration; it will retry on the next launch", + ); + return; + } + }; + + if let Err(error) = persist_dapi_addresses(app_context, addresses_csv) { + tracing::warn!( + target = "migration::finish_unwire", + ?network, + ?error, + "Could not persist automatically discovered DAPI nodes; the refresh will retry on the next launch", + ); + return; + } + + if let Err(error) = Arc::clone(app_context).reinit_core_client_and_sdk() { + tracing::warn!( + target = "migration::finish_unwire", + ?network, + ?error, + "Could not reinitialize network clients after automatic DAPI refresh; the saved addresses will be used on the next launch", + ); + return; + } + + tracing::info!( + target = "migration::finish_unwire", + ?network, + count, + "Automatically refreshed DAPI nodes during migration", + ); + write_dapi_refresh_completion(&app_kv, &sentinel_key, network, 1); +} + +fn write_dapi_refresh_completion( + app_kv: &crate::wallet_backend::DetKv, + sentinel_key: &str, + network: Network, + network_count: u32, +) { + if let Err(error) = write_completion_sentinel(app_kv, sentinel_key, network_count) { + tracing::warn!( + target = "migration::finish_unwire", + ?network, + ?error, + "Could not write the automatic DAPI refresh sentinel; the pass may retry on the next launch", + ); + } +} + /// Run the FinishUnwire migration. Idempotent — completes a no-op when /// the sentinels are already present. /// @@ -382,7 +532,12 @@ fn validate_saved_data_for_migration(app_context: &AppContext) -> Result<(), Mig /// decide whether to surface a "storage update complete" banner — a no-op /// launch must not show one. /// -/// Three independent passes, each under its own sentinel, in this order: +/// A best-effort DAPI refresh is queued before the migration passes. After the +/// run publishes terminal status and releases its guard, refresh acquires and +/// holds that guard until it finishes. Operations that claim the same guard, +/// including local identity deletion, remain unavailable during that interval. +/// +/// Three independent recovery passes, each under its own sentinel, in this order: /// /// 1. **App data** (scheduled votes, top-up history) — DET-owned rows the /// wallet drain never touched. @@ -478,9 +633,39 @@ pub async fn run(app_context: &Arc) -> Result { } } +/// Waits for the launching pass, then holds `migration_run` through refresh. +/// Operations that claim this guard stay gated until the detached work completes. +fn spawn_dapi_refresh(app_context: &Arc, refresh: F) -> tokio::task::JoinHandle<()> +where + F: Future + Send + 'static, +{ + let ctx = Arc::clone(app_context); + tokio::spawn(async move { + let _refresh_guard = ctx.migration_run.lock().await; + refresh.await; + }) +} + +/// Queues refresh before migration so its waiter acquires the guard at completion. async fn run_under_guard(app_context: &Arc) -> Result { + let ctx = Arc::clone(app_context); + std::mem::drop(spawn_dapi_refresh(app_context, async move { + refresh_dapi_nodes_once(&ctx).await; + })); + run_under_guard_with_dapi_refresh(app_context, std::future::ready(())).await +} + +async fn run_under_guard_with_dapi_refresh( + app_context: &Arc, + dapi_refresh: F, +) -> Result +where + F: Future, +{ validate_saved_data_for_migration(app_context)?; + dapi_refresh.await; + let status = app_context.migration_status(); // Scheduled votes and top-up history carry their own sentinel and run @@ -1240,8 +1425,9 @@ fn migrate_app_data(app_context: &Arc) -> Result for TaskError { #[cfg(test)] mod tests { use super::*; + use crate::backend_task::dapi_discovery::persist_dapi_addresses_with_hook; + use crate::config::{CONFIG_ENV_LOCK, Config, NetworkConfig}; use crate::wallet_backend::DetKv; use crate::wallet_backend::kv_test_support::InMemoryKv; + use std::sync::atomic::{AtomicUsize, Ordering}; fn kv() -> DetKv { DetKv::from_store(Arc::new(InMemoryKv::default())) } + fn dapi_refresh_is_complete(app_context: &AppContext) -> bool { + app_context + .app_kv() + .get::( + DetScope::Global, + &dapi_refresh_sentinel_key_for(app_context.network), + ) + .expect("read DAPI refresh sentinel") + .is_some() + } + + async fn wait_for_dapi_refresh(app_context: &Arc) { + // `run` deliberately detaches this work; yield so it queues for the guard, + // then acquire the same guard after the refresh releases it. + tokio::task::yield_now().await; + let guard = app_context.migration_run.lock().await; + drop(guard); + } + + fn seed_legacy_single_key(app_context: &AppContext) { + let path = app_context.db.db_file_path().expect("file-backed database"); + let conn = Connection::open(path).expect("open legacy database"); + seed_legacy_row( + &conn, + &[7u8; 32], + &[1u8; 32], + &[], + &[], + "addr", + None, + false, + app_context.network, + ); + } + + #[tokio::test] + async fn dapi_refresh_fresh_install_skips_discovery_and_preserves_config() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + let original_env = std::fs::read(tmp.path().join(".env")).expect("read original config"); + let original_live_addresses = ctx + .config + .read() + .expect("read original in-memory config") + .dapi_addresses + .clone(); + let calls = AtomicUsize::new(0); + + refresh_dapi_nodes_once_with(&ctx, |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Ok((1, "https://unused.example:443".to_string())) + }) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 0); + assert!(dapi_refresh_is_complete(&ctx)); + assert_eq!( + std::fs::read(tmp.path().join(".env")).expect("read final config"), + original_env, + ); + assert_eq!( + ctx.config + .read() + .expect("read in-memory config") + .dapi_addresses + .as_ref(), + original_live_addresses.as_ref(), + ); + } + + #[tokio::test] + async fn dapi_refresh_legacy_install_updates_disk_and_live_config() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + seed_legacy_single_key(&ctx); + let calls = AtomicUsize::new(0); + let calls_ref = &calls; + let addresses = "https://one.example:443,https://two.example:443"; + + refresh_dapi_nodes_once_with(&ctx, |network| async move { + assert_eq!(network, Network::Testnet); + calls_ref.fetch_add(1, Ordering::SeqCst); + Ok((2, addresses.to_string())) + }) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(dapi_refresh_is_complete(&ctx)); + let saved = crate::config::Config::load_from(tmp.path()).expect("reload saved config"); + assert_eq!( + saved + .config_for_network(Network::Testnet) + .as_ref() + .and_then(|config| config.dapi_addresses.as_deref()), + Some(addresses), + ); + assert_eq!( + ctx.config + .read() + .expect("read in-memory config") + .dapi_addresses + .as_deref(), + Some(addresses), + ); + } + + #[test] + fn dapi_config_persistence_updates_only_selected_key() { + const CHILD_MARKER: &str = "DET_DAPI_PERSISTENCE_TEST_CHILD"; + if std::env::var_os(CHILD_MARKER).is_none() { + let status = std::process::Command::new( + std::env::current_exe().expect("test executable"), + ) + .arg( + "backend_task::migration::finish_unwire::tests::dapi_config_persistence_updates_only_selected_key", + ) + .arg("--exact") + .env(CHILD_MARKER, "1") + .env_remove("MAINNET_core_rpc_password") + .env_remove("TESTNET_wallet_private_key") + .status() + .expect("run isolated persistence test"); + assert!(status.success(), "isolated persistence test failed"); + return; + } + + let tmp = tempfile::tempdir().expect("tempdir"); + let env_path = tmp.path().join(".env"); + let app_context = app_context_for_network(tmp.path(), Network::Mainnet); + let mainnet_password_before = std::env::var_os("MAINNET_core_rpc_password"); + let testnet_key_before = std::env::var_os("TESTNET_wallet_private_key"); + std::fs::write( + &env_path, + "# Preserve operator formatting\n\ + MAINNET_dapi_addresses=https://mainnet-old.example:443\n\ + MAINNET_core_rpc_password='[REDACTED]' # unchanged\n\ + TESTNET_wallet_private_key='[NOT_A_KEY]' # unchanged\n", + ) + .expect("write initial config"); + + persist_dapi_addresses_with_hook( + &app_context, + "https://mainnet-new.example:443".to_string(), + || { + app_context + .config + .write() + .expect("update live config during persistence") + .core_rpc_password = Some("[LIVE_UPDATE]".to_string()); + }, + || {}, + ) + .expect("persist selected DAPI addresses"); + + let saved = std::fs::read_to_string(env_path).expect("read updated config"); + assert!(saved.contains("MAINNET_dapi_addresses=https://mainnet-new.example:443\n")); + assert!(saved.contains("MAINNET_core_rpc_password='[REDACTED]' # unchanged\n")); + assert!(saved.contains("TESTNET_wallet_private_key='[NOT_A_KEY]' # unchanged\n")); + assert_eq!( + std::env::var_os("MAINNET_core_rpc_password"), + mainnet_password_before, + ); + assert_eq!( + std::env::var_os("TESTNET_wallet_private_key"), + testnet_key_before, + ); + assert_eq!( + app_context + .config + .read() + .expect("read live config after persistence") + .core_rpc_password + .as_deref(), + Some("[LIVE_UPDATE]"), + ); + } + + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum ConfigPersistenceEvent { + FirstEntered, + FirstPausedBeforeSave, + SecondAttempting, + SecondEntered, + SecondSaved, + FirstSaved, + } + + #[tokio::test] + async fn dapi_config_persistence_serializes_across_networks() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + Config { + mainnet_config: Some(NetworkConfig { + dapi_addresses: Some("https://mainnet-old.example:443".to_string()), + ..Default::default() + }), + testnet_config: Some(NetworkConfig { + dapi_addresses: Some("https://testnet-old.example:443".to_string()), + ..Default::default() + }), + devnet_config: None, + local_config: None, + } + .save(tmp.path()) + .expect("write initial config"); + let mainnet_context = app_context_for_network(tmp.path(), Network::Mainnet); + let testnet_context = app_context_for_network_with_shared_storage( + tmp.path(), + Network::Testnet, + &mainnet_context, + ); + + let (event_tx, event_rx) = std::sync::mpsc::channel(); + let (release_first_tx, release_first_rx) = std::sync::mpsc::channel(); + let first_context = Arc::clone(&mainnet_context); + let first_before_save_tx = event_tx.clone(); + let first_after_save_tx = event_tx.clone(); + let first = std::thread::spawn(move || { + persist_dapi_addresses_with_hook( + &first_context, + "https://mainnet-new.example:443".to_string(), + || { + first_before_save_tx + .send(ConfigPersistenceEvent::FirstEntered) + .expect("report first entry"); + first_before_save_tx + .send(ConfigPersistenceEvent::FirstPausedBeforeSave) + .expect("report first pause"); + release_first_rx.recv().expect("release first save"); + }, + || { + first_after_save_tx + .send(ConfigPersistenceEvent::FirstSaved) + .expect("report first save"); + }, + ) + .expect("first persistence"); + }); + + assert_eq!( + event_rx.recv().expect("first entry event"), + ConfigPersistenceEvent::FirstEntered, + ); + assert_eq!( + event_rx.recv().expect("first pause event"), + ConfigPersistenceEvent::FirstPausedBeforeSave, + ); + + let second_context = Arc::clone(&testnet_context); + let second_attempt_tx = event_tx.clone(); + let second_before_save_tx = event_tx.clone(); + let second_after_save_tx = event_tx.clone(); + let second = std::thread::spawn(move || { + second_attempt_tx + .send(ConfigPersistenceEvent::SecondAttempting) + .expect("report second attempt"); + persist_dapi_addresses_with_hook( + &second_context, + "https://testnet-new.example:443".to_string(), + || { + second_before_save_tx + .send(ConfigPersistenceEvent::SecondEntered) + .expect("report second entry"); + }, + || { + second_after_save_tx + .send(ConfigPersistenceEvent::SecondSaved) + .expect("report second save"); + }, + ) + .expect("second persistence"); + }); + + assert_eq!( + event_rx.recv().expect("second attempt event"), + ConfigPersistenceEvent::SecondAttempting, + ); + let premature_entry = event_rx.recv_timeout(std::time::Duration::from_millis(250)); + let premature_save = if premature_entry.is_ok() { + Some( + event_rx + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("second save before first release"), + ) + } else { + None + }; + + release_first_tx.send(()).expect("release first"); + first.join().expect("join first persistence section"); + second.join().expect("join second persistence section"); + + assert_eq!( + premature_entry, + Err(std::sync::mpsc::RecvTimeoutError::Timeout), + "the second persistence section entered before the first released", + ); + assert!( + premature_save.is_none(), + "the second persistence section saved before the first released: {premature_save:?}", + ); + assert_eq!( + event_rx.try_iter().collect::>(), + [ + ConfigPersistenceEvent::FirstSaved, + ConfigPersistenceEvent::SecondEntered, + ConfigPersistenceEvent::SecondSaved, + ], + ); + + let saved = Config::load_from(tmp.path()).expect("load final config"); + assert_eq!( + saved + .config_for_network(Network::Mainnet) + .as_ref() + .and_then(|config| config.dapi_addresses.as_deref()), + Some("https://mainnet-new.example:443"), + ); + assert_eq!( + saved + .config_for_network(Network::Testnet) + .as_ref() + .and_then(|config| config.dapi_addresses.as_deref()), + Some("https://testnet-new.example:443"), + ); + assert_eq!( + mainnet_context + .config + .read() + .expect("read mainnet live config") + .dapi_addresses + .as_deref(), + Some("https://mainnet-new.example:443"), + ); + assert_eq!( + testnet_context + .config + .read() + .expect("read testnet live config") + .dapi_addresses + .as_deref(), + Some("https://testnet-new.example:443"), + ); + } + + #[tokio::test] + async fn dapi_config_persistence_reports_save_failure() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + Config { + mainnet_config: Some(NetworkConfig { + dapi_addresses: Some("https://mainnet-old.example:443".to_string()), + ..Default::default() + }), + testnet_config: None, + devnet_config: None, + local_config: None, + } + .save(tmp.path()) + .expect("write initial config"); + let app_context = app_context_for_network(tmp.path(), Network::Mainnet); + let env_path = tmp.path().join(".env"); + + let error = persist_dapi_addresses_with_hook( + &app_context, + "https://mainnet-new.example:443".to_string(), + || { + std::fs::remove_file(&env_path).expect("remove config file"); + std::fs::create_dir(&env_path).expect("replace config file with directory"); + }, + || {}, + ) + .expect_err("persistence must report the save failure"); + + assert!(matches!( + error, + TaskError::Config(crate::config::ConfigError::SaveError { .. }) + )); + } + + #[tokio::test] + async fn dapi_refresh_reinit_failure_withholds_completion_sentinel() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + seed_legacy_single_key(&ctx); + + refresh_dapi_nodes_once_with(&ctx, |_| async { Ok((1, String::new())) }).await; + + assert!(!dapi_refresh_is_complete(&ctx)); + } + + #[tokio::test] + async fn dapi_refresh_legacy_detection_failure_retries_then_completes() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + seed_legacy_single_key(&ctx); + let calls = AtomicUsize::new(0); + + refresh_dapi_nodes_once_with_legacy_check( + &ctx, + |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Ok((1, "https://unused.example:443".to_string())) + }, + |_| { + Err(MigrationError::LegacyDbRead { + table: "wallet", + source: rusqlite::Error::InvalidQuery, + }) + }, + ) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 0); + assert!(!dapi_refresh_is_complete(&ctx)); + + refresh_dapi_nodes_once_with(&ctx, |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Ok((1, "https://retry.example:443".to_string())) + }) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(dapi_refresh_is_complete(&ctx)); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn dapi_refresh_discovery_failure_retries_without_failing_migration() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + seed_legacy_wallet(&ctx, &[0x81u8; 64], "funds", ctx.network); + wire_backend(&ctx).await; + let backend = ctx.wallet_backend().expect("backend wired"); + let calls = AtomicUsize::new(0); + + let refresh = refresh_dapi_nodes_once_with(&ctx, |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Err(crate::backend_task::dapi_discovery::DapiDiscoveryError::Timeout) + }); + let did_work = run_under_guard_with_dapi_refresh(&ctx, refresh) + .await + .expect("DAPI discovery must not fail the migration"); + + assert!(did_work, "the independent wallet pass still moved data"); + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(!dapi_refresh_is_complete(&ctx)); + + refresh_dapi_nodes_once_with(&ctx, |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Err(crate::backend_task::dapi_discovery::DapiDiscoveryError::Timeout) + }) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 2, "the next launch retries"); + assert!(!dapi_refresh_is_complete(&ctx)); + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn dapi_refresh_launches_are_serialized() { + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + let attempts = Arc::new(AtomicUsize::new(0)); + let first_started = Arc::new(tokio::sync::Notify::new()); + let release_first = Arc::new(tokio::sync::Notify::new()); + + let first_attempts = Arc::clone(&attempts); + let first_started_task = Arc::clone(&first_started); + let release_first_task = Arc::clone(&release_first); + let first = spawn_dapi_refresh(&ctx, async move { + first_attempts.fetch_add(1, Ordering::SeqCst); + first_started_task.notify_one(); + release_first_task.notified().await; + }); + first_started.notified().await; + + let second_attempts = Arc::clone(&attempts); + let second_started = Arc::new(tokio::sync::Notify::new()); + let second_started_task = Arc::clone(&second_started); + let second = spawn_dapi_refresh(&ctx, async move { + second_attempts.fetch_add(1, Ordering::SeqCst); + second_started_task.notify_one(); + }); + + assert!( + tokio::time::timeout( + std::time::Duration::from_millis(50), + second_started.notified(), + ) + .await + .is_err(), + "a second refresh must wait for the first refresh's migration guard", + ); + assert_eq!(attempts.load(Ordering::SeqCst), 1); + + release_first.notify_one(); + first.await.expect("join first refresh"); + second.await.expect("join second refresh"); + assert_eq!(attempts.load(Ordering::SeqCst), 2); + } + + #[tokio::test] + async fn dapi_refresh_devnet_detection_failure_completes_without_discovery() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = app_context_for_network(tmp.path(), Network::Devnet); + let calls = AtomicUsize::new(0); + + refresh_dapi_nodes_once_with_legacy_check( + &ctx, + |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Ok((1, "https://unused.example:443".to_string())) + }, + |_| { + Err(MigrationError::LegacyDbRead { + table: "wallet", + source: rusqlite::Error::InvalidQuery, + }) + }, + ) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 0); + assert!(dapi_refresh_is_complete(&ctx)); + } + + #[tokio::test] + async fn dapi_refresh_existing_sentinel_skips_discovery() { + let _env_guard = CONFIG_ENV_LOCK.lock().await; + let tmp = tempfile::tempdir().expect("tempdir"); + let ctx = fresh_app_context(tmp.path()); + seed_legacy_single_key(&ctx); + let calls = AtomicUsize::new(0); + + refresh_dapi_nodes_once_with(&ctx, |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Ok((1, "https://first.example:443".to_string())) + }) + .await; + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(dapi_refresh_is_complete(&ctx)); + + refresh_dapi_nodes_once_with(&ctx, |_| async { + calls.fetch_add(1, Ordering::SeqCst); + Ok((1, "https://unused.example:443".to_string())) + }) + .await; + + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(dapi_refresh_is_complete(&ctx)); + } + #[test] fn unreadable_top_up_warning_is_durable_and_acknowledgeable() { let app_kv = kv(); @@ -4381,8 +5128,10 @@ mod tests { /// two `run()` no-op paths, which return before touching the wallet /// backend. fn fresh_app_context(dir: &std::path::Path) -> Arc { - use dash_sdk::dpp::dashcore::Network; + app_context_for_network(dir, Network::Testnet) + } + fn app_context_for_network(dir: &std::path::Path, network: Network) -> Arc { crate::app_dir::ensure_env_file(dir); let db_file = dir.join("data.db"); let db = Arc::new(crate::database::Database::new(&db_file).expect("db")); @@ -4393,7 +5142,7 @@ mod tests { let secret_store = AppContext::open_secret_store(dir).expect("open secret store"); AppContext::new( dir.to_path_buf(), - Network::Testnet, + network, db, Default::default(), Default::default(), @@ -4405,6 +5154,25 @@ mod tests { .expect("AppContext") } + fn app_context_for_network_with_shared_storage( + dir: &std::path::Path, + network: Network, + shared: &AppContext, + ) -> Arc { + AppContext::new( + dir.to_path_buf(), + network, + Arc::clone(&shared.db), + Default::default(), + Default::default(), + egui::Context::default(), + shared.app_kv(), + shared.secret_store(), + crate::model::user_role::UserRoleCell::default(), + ) + .expect("AppContext") + } + #[test] fn identity_deletion_is_rejected_while_migration_is_running() { let tmp = tempfile::tempdir().expect("tempdir"); @@ -5297,6 +6065,7 @@ mod tests { run(&ctx).await.expect("first run"), "the first launch moves legacy data", ); + wait_for_dapi_refresh(&ctx).await; assert!(backend.is_wallet_registered(&seed_hash)); assert_eq!(ctx.get_scheduled_votes().expect("read votes").len(), 1); let sentinel_after_first = read_sentinel(&ctx.app_kv(), network) @@ -5750,6 +6519,7 @@ mod tests { let backend = ctx.wallet_backend().expect("backend wired"); run(&ctx).await.expect("first launch"); + wait_for_dapi_refresh(&ctx).await; let deleted_id = Identifier::from(deleted); assert!( diff --git a/src/config.rs b/src/config.rs index b1f2c4178..1042f4d6f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; +use std::fs; use std::io::{self, Write}; use std::path::Path; use std::str::FromStr; @@ -8,6 +10,22 @@ use dash_sdk::dpp::dashcore::Network; use serde::Deserialize; use tempfile::NamedTempFile; +pub(crate) static CONFIG_PERSISTENCE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); +#[cfg(test)] +pub(crate) static CONFIG_ENV_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + +const NETWORK_PREFIXES: [&str; 4] = ["MAINNET_", "TESTNET_", "DEVNET_", "LOCAL_"]; +const NETWORK_CONFIG_FIELDS: [&str; 8] = [ + "dapi_addresses", + "core_host", + "core_rpc_port", + "core_rpc_user", + "core_rpc_password", + "core_zmq_endpoint", + "devnet_name", + "wallet_private_key", +]; + #[derive(Debug, Deserialize, Clone)] pub struct Config { pub mainnet_config: Option, @@ -19,8 +37,28 @@ pub struct Config { #[derive(Debug, thiserror::Error)] pub enum ConfigError { /// Failed to load configuration from disk or environment. - #[error("{0}")] - LoadError(String), + #[error("Could not load settings. Check that the application folder is readable and retry.")] + LoadError { + #[source] + source: std::io::Error, + }, + + /// The existing `.env` file could not be parsed safely. + #[error("The saved settings contain an invalid entry. Correct the settings file and retry.")] + InvalidEnvFile { + #[source] + source: dotenvy::Error, + }, + + /// A network-specific value could not be parsed into its modeled type. + #[error( + "The saved {network} settings contain an invalid value. Correct the settings file and retry." + )] + InvalidNetworkConfig { + network: &'static str, + #[source] + source: envy::Error, + }, /// Failed to save configuration to disk. #[error("Could not save settings. Check that the application folder is writable and retry.")] @@ -101,137 +139,67 @@ impl Config { /// Write the current configuration back to the `.env` file so that /// subsequent calls to `Config::load()` will reflect changes. /// - /// Uses atomic write (write to temp file, then rename) to prevent - /// config corruption if a write fails partway through. + /// Preserves lines not modeled by `Config` and replaces the modeled fields. + /// Uses a temporary file and atomic rename to prevent partial writes. pub fn save(&self, data_dir: &Path) -> Result<(), ConfigError> { let env_file_path = data_file_path(data_dir, ".env").map_err(|e| ConfigError::SaveError { source: e })?; - - // Write to a temporary file in the same directory first, then - // atomically replace. This prevents corruption if the write fails - // partway through. NamedTempFile::persist() closes the handle before - // renaming and uses MoveFileEx with MOVEFILE_REPLACE_EXISTING on - // Windows for atomic replacement. - let parent_dir = env_file_path - .parent() - .ok_or_else(|| ConfigError::SaveError { - source: io::Error::new( - io::ErrorKind::NotFound, - "config file path has no parent directory", - ), - })?; - let mut env_file = - NamedTempFile::new_in(parent_dir).map_err(|e| ConfigError::SaveError { source: e })?; - - // Helper function to write a single network config to the `.env` file - let mut write_network_config = |prefix: &str, config: &NetworkConfig| { - // Each line becomes e.g. MAINNET_dapi_addresses=... - // For "local" (regtest), you'll see LOCAL_dapi_addresses=... - // - // Use the environment variable scheme you prefer. Make sure it - // matches what `load()` expects (i.e. `envy::prefixed("MAINNET_")`, - // etc.). - - if let Some(ref addrs) = config.dapi_addresses - && !addrs.is_empty() - { - writeln!(env_file, "{}dapi_addresses={}", prefix, addrs) - .map_err(|e| ConfigError::SaveError { source: e })?; - } - if let Some(ref host) = config.core_host { - writeln!(env_file, "{}core_host={}", prefix, host) - .map_err(|e| ConfigError::SaveError { source: e })?; - } - if let Some(port) = config.core_rpc_port { - writeln!(env_file, "{}core_rpc_port={}", prefix, port) - .map_err(|e| ConfigError::SaveError { source: e })?; - } - if let Some(ref user) = config.core_rpc_user { - writeln!(env_file, "{}core_rpc_user={}", prefix, user) - .map_err(|e| ConfigError::SaveError { source: e })?; - } - if let Some(ref password) = config.core_rpc_password { - writeln!(env_file, "{}core_rpc_password={}", prefix, password) - .map_err(|e| ConfigError::SaveError { source: e })?; - } - if let Some(core_zmq_endpoint) = &config.core_zmq_endpoint { - writeln!( - env_file, - "{}core_zmq_endpoint={}", - prefix, core_zmq_endpoint - ) - .map_err(|e| ConfigError::SaveError { source: e })?; + let existing_contents = read_env_contents(&env_file_path)?; + validate_env_contents(&existing_contents)?; + let modeled_entries = self.modeled_entries(); + atomic_replace_env_file(&env_file_path, |env_file| { + let mut written_keys = HashSet::new(); + for line in existing_contents.lines() { + if let Some(key) = owned_config_key(line) { + if let Some((_, value)) = modeled_entries + .iter() + .find(|(candidate, _)| candidate == &key) + && written_keys.insert(key.clone()) + { + let assignment = + line.split_once('=').map_or(key.as_str(), |(left, _)| left); + writeln!(env_file, "{assignment}={value}") + .map_err(|source| ConfigError::SaveError { source })?; + } + } else { + writeln!(env_file, "{line}") + .map_err(|source| ConfigError::SaveError { source })?; + } } - - if let Some(devnet_name) = &config.devnet_name { - // Only write devnet name if it exists - writeln!(env_file, "{}devnet_name={}", prefix, devnet_name) - .map_err(|e| ConfigError::SaveError { source: e })?; - } - if let Some(wallet_private_key) = &config.wallet_private_key { - writeln!( - env_file, - "{}wallet_private_key={}", - prefix, wallet_private_key - ) - .map_err(|e| ConfigError::SaveError { source: e })?; + for (key, value) in modeled_entries { + if written_keys.insert(key.clone()) { + writeln!(env_file, "{key}={value}") + .map_err(|source| ConfigError::SaveError { source })?; + } } - - // Add a blank line after each config block - writeln!(env_file).map_err(|e| ConfigError::SaveError { source: e })?; - Ok(()) - }; - - // Mainnet - if let Some(ref mainnet_config) = self.mainnet_config { - // `envy::prefixed("MAINNET_")` expects these lines to start with "MAINNET_" - write_network_config("MAINNET_", mainnet_config)?; - } - - // Testnet - if let Some(ref testnet_config) = self.testnet_config { - write_network_config("TESTNET_", testnet_config)?; - } - - // Devnet - if let Some(ref devnet_config) = self.devnet_config { - write_network_config("DEVNET_", devnet_config)?; - } - - // Local (Regtest) - if let Some(ref local_config) = self.local_config { - // `envy::prefixed("LOCAL_")` expects "LOCAL_..." - write_network_config("LOCAL_", local_config)?; - } - - // Sync all data to disk before renaming to ensure crash-safety - env_file - .as_file() - .sync_all() - .map_err(|e| ConfigError::SaveError { source: e })?; - - // Atomically replace the old config with the new one. - // persist() closes the file handle and uses platform-safe rename - // (MoveFileEx with MOVEFILE_REPLACE_EXISTING on Windows). - env_file - .persist(&env_file_path) - .map_err(|e| ConfigError::SaveError { source: e.error })?; - - // Restrict file permissions on Unix (config contains RPC credentials). - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let perms = std::fs::Permissions::from_mode(0o600); - if let Err(e) = std::fs::set_permissions(&env_file_path, perms) { - tracing::warn!("Could not set config file permissions to 0600: {e}"); - } - } + })?; tracing::info!("Successfully saved configuration to {:?}", env_file_path); Ok(()) } + /// Persist one network's DAPI addresses without parsing or reserializing other values. + pub(crate) fn save_dapi_addresses( + data_dir: &Path, + network: Network, + addresses: &str, + ) -> Result<(), ConfigError> { + let env_file_path = + data_file_path(data_dir, ".env").map_err(|source| ConfigError::SaveError { source })?; + let existing_contents = read_env_contents(&env_file_path)?; + let key = format!("{}dapi_addresses", dotenv_network_prefix(network)); + atomic_replace_env_file(&env_file_path, |env_file| { + write_single_env_update(env_file, &existing_contents, &key, addresses) + })?; + tracing::info!( + ?network, + path = ?env_file_path, + "Saved DAPI addresses for one network" + ); + Ok(()) + } + /// Loads the configuration for all networks from environment variables and `.env` file /// located in the default app data directory. pub fn load() -> Result { @@ -243,38 +211,43 @@ impl Config { /// located in the given data directory. pub fn load_from(data_dir: &Path) -> Result { let env_file_path = - data_file_path(data_dir, ".env").map_err(|e| ConfigError::LoadError(e.to_string()))?; + data_file_path(data_dir, ".env").map_err(|source| ConfigError::LoadError { source })?; Self::load_from_env_path(env_file_path) } fn load_from_env_path(env_file_path: std::path::PathBuf) -> Result { - if let Err(err) = dotenvy::from_path_override(env_file_path) { - tracing::warn!( - ?err, - "Failed to load .env file. Continuing with environment variables." - ); - } else { - tracing::info!("Successfully loaded .env file"); + let file_entries = match fs::read_to_string(&env_file_path) { + Ok(contents) => dotenvy::Iter::new( + contents + .strip_prefix('\u{feff}') + .unwrap_or(&contents) + .as_bytes(), + ) + .collect::, _>>() + .map_err(|source| ConfigError::InvalidEnvFile { source })?, + Err(error) if error.kind() == io::ErrorKind::NotFound => Vec::new(), + Err(source) => return Err(ConfigError::LoadError { source }), + }; + match dotenvy::from_path_override(&env_file_path) { + Ok(()) => tracing::info!("Successfully loaded .env file"), + Err(error) if error.not_found() => { + tracing::warn!( + ?error, + "No .env file was found. Continuing with environment variables." + ); + } + Err(source) => return Err(ConfigError::InvalidEnvFile { source }), } - // Load each network config. Missing configs are normal — not every - // user configures all networks. Only fail if nothing is configured at all. - let mainnet_config = envy::prefixed("MAINNET_") - .from_env::() - .inspect_err(|e| tracing::debug!("Failed to parse mainnet config: {e}")) - .ok(); - let testnet_config = envy::prefixed("TESTNET_") - .from_env::() - .inspect_err(|e| tracing::debug!("Failed to parse testnet config: {e}")) - .ok(); - let devnet_config = envy::prefixed("DEVNET_") - .from_env::() - .inspect_err(|e| tracing::debug!("Failed to parse devnet config: {e}")) - .ok(); - let local_config = envy::prefixed("LOCAL_") - .from_env::() - .inspect_err(|e| tracing::debug!("Failed to parse local config: {e}")) - .ok(); + let process_entries = std::env::vars().collect::>(); + let mainnet_config = + load_network_config_or_none("MAINNET_", "Mainnet", &process_entries, &file_entries); + let testnet_config = + load_network_config_or_none("TESTNET_", "Testnet", &process_entries, &file_entries); + let devnet_config = + load_network_config_or_none("DEVNET_", "Devnet", &process_entries, &file_entries); + let local_config = + load_network_config_or_none("LOCAL_", "local network", &process_entries, &file_entries); if mainnet_config.is_none() && testnet_config.is_none() @@ -301,6 +274,246 @@ impl Config { Network::Regtest => self.local_config = Some(new_config), } } + + fn modeled_entries(&self) -> Vec<(String, String)> { + let mut entries = Vec::new(); + for (prefix, config) in [ + ("MAINNET_", self.mainnet_config.as_ref()), + ("TESTNET_", self.testnet_config.as_ref()), + ("DEVNET_", self.devnet_config.as_ref()), + ("LOCAL_", self.local_config.as_ref()), + ] { + if let Some(config) = config { + append_network_entries(&mut entries, prefix, config); + } + } + entries + } +} + +fn read_env_contents(env_file_path: &Path) -> Result { + match fs::read_to_string(env_file_path) { + Ok(contents) => Ok(contents), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(String::new()), + Err(source) => Err(ConfigError::SaveError { source }), + } +} + +fn atomic_replace_env_file( + env_file_path: &Path, + write_contents: impl FnOnce(&mut NamedTempFile) -> Result<(), ConfigError>, +) -> Result<(), ConfigError> { + let parent_dir = env_file_path + .parent() + .ok_or_else(|| ConfigError::SaveError { + source: io::Error::new( + io::ErrorKind::NotFound, + "config file path has no parent directory", + ), + })?; + let mut env_file = + NamedTempFile::new_in(parent_dir).map_err(|source| ConfigError::SaveError { source })?; + write_contents(&mut env_file)?; + env_file + .as_file() + .sync_all() + .map_err(|source| ConfigError::SaveError { source })?; + env_file + .persist(env_file_path) + .map_err(|error| ConfigError::SaveError { + source: error.error, + })?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let permissions = std::fs::Permissions::from_mode(0o600); + if let Err(error) = std::fs::set_permissions(env_file_path, permissions) { + tracing::warn!( + ?error, + path = ?env_file_path, + "Could not restrict config file permissions to 0600" + ); + } + } + Ok(()) +} + +fn write_single_env_update( + env_file: &mut NamedTempFile, + existing_contents: &str, + target_key: &str, + value: &str, +) -> Result<(), ConfigError> { + let mut updated = false; + for line in existing_contents.split_inclusive('\n') { + let (body, line_ending) = if let Some(body) = line.strip_suffix("\r\n") { + (body, "\r\n") + } else if let Some(body) = line.strip_suffix('\n') { + (body, "\n") + } else { + (line, "") + }; + if owned_config_key(body).as_deref() == Some(target_key) { + if updated { + continue; + } + let assignment = body.split_once('=').map_or(target_key, |(left, _)| left); + write!(env_file, "{assignment}={value}{line_ending}") + .map_err(|source| ConfigError::SaveError { source })?; + updated = true; + } else { + write!(env_file, "{line}").map_err(|source| ConfigError::SaveError { source })?; + } + } + if !updated { + if !existing_contents.is_empty() && !existing_contents.ends_with('\n') { + writeln!(env_file).map_err(|source| ConfigError::SaveError { source })?; + } + writeln!(env_file, "{target_key}={value}") + .map_err(|source| ConfigError::SaveError { source })?; + } + Ok(()) +} + +fn dotenv_network_prefix(network: Network) -> &'static str { + match network { + Network::Mainnet => "MAINNET_", + Network::Testnet => "TESTNET_", + Network::Devnet => "DEVNET_", + Network::Regtest => "LOCAL_", + } +} + +fn load_network_config_or_none( + prefix: &'static str, + network: &'static str, + process_entries: &[(String, String)], + file_entries: &[(String, String)], +) -> Option { + match load_network_config(prefix, network, process_entries, file_entries) { + Ok(config) => config, + Err(_) => { + tracing::warn!( + network, + "Ignoring invalid network settings while loading other networks" + ); + None + } + } +} + +fn load_network_config( + prefix: &'static str, + network: &'static str, + process_entries: &[(String, String)], + file_entries: &[(String, String)], +) -> Result, ConfigError> { + let mut entries = std::collections::HashMap::new(); + for (key, value) in process_entries.iter().chain(file_entries) { + if let Some(field) = key.strip_prefix(prefix) { + entries.insert( + format!("{prefix}{}", field.to_ascii_lowercase()), + value.clone(), + ); + } + } + if entries.is_empty() { + return Ok(None); + } + envy::prefixed(prefix) + .from_iter::<_, NetworkConfig>(entries) + .map(Some) + .map_err(|source| ConfigError::InvalidNetworkConfig { network, source }) +} + +fn validate_env_contents(contents: &str) -> Result<(), ConfigError> { + let contents = contents.strip_prefix('\u{feff}').unwrap_or(contents); + let entries = dotenvy::Iter::new(contents.as_bytes()) + .collect::, _>>() + .map_err(|source| ConfigError::InvalidEnvFile { source })?; + for (prefix, network) in [ + ("MAINNET_", "Mainnet"), + ("TESTNET_", "Testnet"), + ("DEVNET_", "Devnet"), + ("LOCAL_", "local network"), + ] { + if entries.iter().any(|(key, _)| key.starts_with(prefix)) { + envy::prefixed(prefix) + .from_iter::<_, NetworkConfig>(entries.iter().cloned()) + .map_err(|source| ConfigError::InvalidNetworkConfig { network, source })?; + } + } + Ok(()) +} + +fn append_network_entries( + entries: &mut Vec<(String, String)>, + prefix: &str, + config: &NetworkConfig, +) { + let mut push = |field: &str, value: String| { + entries.push((format!("{prefix}{field}"), value)); + }; + if let Some(addresses) = &config.dapi_addresses + && !addresses.is_empty() + { + push("dapi_addresses", addresses.clone()); + } + if let Some(host) = &config.core_host { + push("core_host", host.clone()); + } + if let Some(port) = config.core_rpc_port { + push("core_rpc_port", port.to_string()); + } + if let Some(user) = &config.core_rpc_user { + push("core_rpc_user", user.clone()); + } + if let Some(password) = &config.core_rpc_password { + push("core_rpc_password", password.clone()); + } + if let Some(endpoint) = &config.core_zmq_endpoint { + push("core_zmq_endpoint", endpoint.clone()); + } + if let Some(name) = &config.devnet_name { + push("devnet_name", name.clone()); + } + if let Some(key) = &config.wallet_private_key { + push("wallet_private_key", key.clone()); + } +} + +fn owned_config_key(line: &str) -> Option { + let line = line.strip_prefix('\u{feff}').unwrap_or(line).trim_start(); + let (mut key, mut rest) = take_env_key(line)?; + rest = rest.trim_start(); + if key == "export" && !rest.starts_with('=') { + (key, rest) = take_env_key(rest)?; + rest = rest.trim_start(); + } + if !rest.starts_with('=') { + return None; + } + NETWORK_PREFIXES.iter().find_map(|prefix| { + let field = key.strip_prefix(prefix)?; + NETWORK_CONFIG_FIELDS + .iter() + .find(|owned| field.eq_ignore_ascii_case(owned)) + .map(|owned| format!("{prefix}{owned}")) + }) +} + +fn take_env_key(input: &str) -> Option<(&str, &str)> { + let first = *input.as_bytes().first()?; + if !first.is_ascii_alphabetic() && first != b'_' { + return None; + } + let end = input + .as_bytes() + .iter() + .position(|byte| !byte.is_ascii_alphanumeric() && *byte != b'_' && *byte != b'.') + .unwrap_or(input.len()); + Some((&input[..end], &input[end..])) } impl NetworkConfig { @@ -331,6 +544,24 @@ impl NetworkConfig { mod tests { use super::*; + fn run_config_test_in_subprocess(test_name: &str) -> bool { + const CHILD_MARKER: &str = "DET_CONFIG_TEST_CHILD"; + if std::env::var_os(CHILD_MARKER).is_some() { + return false; + } + let mut command = + std::process::Command::new(std::env::current_exe().expect("test executable")); + command.arg(test_name).arg("--exact").env(CHILD_MARKER, "1"); + for prefix in NETWORK_PREFIXES { + for field in NETWORK_CONFIG_FIELDS { + command.env_remove(format!("{prefix}{field}")); + } + } + let status = command.status().expect("run isolated config test"); + assert!(status.success(), "isolated config test failed"); + true + } + /// Helper to create a minimal valid NetworkConfig for testing fn make_network_config(dapi_addresses: &str, port: u16) -> NetworkConfig { let dapi = if dapi_addresses.is_empty() { @@ -557,6 +788,180 @@ mod tests { assert!(output.contains("MAINNET_core_zmq_endpoint=tcp://127.0.0.1:23708")); } + #[test] + fn save_preserves_unmodeled_env_keys() { + if run_config_test_in_subprocess("config::tests::save_preserves_unmodeled_env_keys") { + return; + } + let tmp = tempfile::tempdir().expect("tempdir"); + let env_path = tmp.path().join(".env"); + std::fs::write( + &env_path, + "# Operator settings\nMCP_API_KEY=some-fake-test-value\nMAINNET_dapi_addresses=https://old.example:443\nRUST_LOG=info\n", + ) + .expect("write test config"); + + let mut config = Config::load_from(tmp.path()).expect("load test config"); + config + .mainnet_config + .as_mut() + .expect("mainnet config") + .dapi_addresses = Some("https://new.example:443".to_string()); + config.save(tmp.path()).expect("save test config"); + + let saved = std::fs::read_to_string(env_path).expect("read saved config"); + assert!(saved.contains("# Operator settings\n")); + assert!(saved.contains("MCP_API_KEY=some-fake-test-value\n")); + assert!(saved.contains("RUST_LOG=info\n")); + assert!(saved.contains("MAINNET_dapi_addresses=https://new.example:443\n")); + assert!(!saved.contains("MAINNET_dapi_addresses=https://old.example:443\n")); + } + + #[test] + fn save_replaces_modeled_keys_case_insensitively() { + if run_config_test_in_subprocess( + "config::tests::save_replaces_modeled_keys_case_insensitively", + ) { + return; + } + let tmp = tempfile::tempdir().expect("tempdir"); + let env_path = tmp.path().join(".env"); + std::fs::write( + &env_path, + "\u{feff}export\tMAINNET_DAPI_ADDRESSES=https://old.example:443\nmainnet_dapi_addresses=keep-this-line\n", + ) + .expect("write test config"); + + let mut config = Config::load_from(tmp.path()).expect("load test config"); + config + .mainnet_config + .as_mut() + .expect("mainnet config") + .dapi_addresses = Some("https://new.example:443".to_string()); + config.save(tmp.path()).expect("save test config"); + + let saved = std::fs::read_to_string(&env_path).expect("read saved config"); + assert!(!saved.contains("MAINNET_DAPI_ADDRESSES=https://old.example:443\n")); + assert!(saved.contains("mainnet_dapi_addresses=keep-this-line\n")); + let reloaded = Config::load_from(tmp.path()).expect("reload test config"); + assert_eq!( + reloaded + .mainnet_config + .as_ref() + .and_then(|network| network.dapi_addresses.as_deref()), + Some("https://new.example:443"), + ); + } + + #[test] + fn save_preserves_unmodeled_line_order() { + if run_config_test_in_subprocess("config::tests::save_preserves_unmodeled_line_order") { + return; + } + let tmp = tempfile::tempdir().expect("tempdir"); + let env_path = tmp.path().join(".env"); + std::fs::write( + &env_path, + "MAINNET_core_host=127.0.0.1\nEXTRA_HOST=${MAINNET_core_host}\nMAINNET_dapi_addresses=https://old.example:443\n", + ) + .expect("write test config"); + + let mut config = Config::load_from(tmp.path()).expect("load test config"); + config + .mainnet_config + .as_mut() + .expect("mainnet config") + .dapi_addresses = Some("https://new.example:443".to_string()); + config.save(tmp.path()).expect("save test config"); + + let saved = std::fs::read_to_string(env_path).expect("read saved config"); + let host_position = saved.find("MAINNET_core_host=").expect("modeled host"); + let extra_position = saved.find("EXTRA_HOST=").expect("unmodeled reference"); + let dapi_position = saved + .find("MAINNET_dapi_addresses=") + .expect("modeled addresses"); + assert!(host_position < extra_position); + assert!(extra_position < dapi_position); + } + + #[test] + fn save_rejects_invalid_modeled_values_without_rewriting() { + let tmp = tempfile::tempdir().expect("tempdir"); + let env_path = tmp.path().join(".env"); + let original = "MAINNET_dapi_addresses=https://mainnet.example:443\nTESTNET_core_rpc_port=not-a-number\nTESTNET_core_rpc_password=fake-test-password\n"; + std::fs::write(&env_path, original).expect("write test config"); + let config = Config { + mainnet_config: Some(make_network_config("https://mainnet-new.example:443", 9998)), + testnet_config: None, + devnet_config: None, + local_config: None, + }; + + let error = config + .save(tmp.path()) + .expect_err("invalid modeled value must fail closed"); + + assert!(matches!( + error, + ConfigError::InvalidNetworkConfig { + network: "Testnet", + .. + } + )); + assert_eq!( + std::fs::read_to_string(env_path).expect("read unchanged config"), + original, + ); + } + + #[test] + fn load_ignores_invalid_network_when_another_network_is_valid() { + if run_config_test_in_subprocess( + "config::tests::load_ignores_invalid_network_when_another_network_is_valid", + ) { + return; + } + let tmp = tempfile::tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(".env"), + "MAINNET_core_rpc_port=invalid\nTESTNET_core_rpc_port=19998\n", + ) + .expect("write test config"); + + let config = Config::load_from(tmp.path()).expect("load the valid network"); + + assert!(config.mainnet_config.is_none()); + assert_eq!( + config + .testnet_config + .as_ref() + .and_then(|network| network.core_rpc_port), + Some(19998), + ); + } + + #[test] + fn load_returns_no_valid_configs_when_all_networks_are_invalid() { + if run_config_test_in_subprocess( + "config::tests::load_returns_no_valid_configs_when_all_networks_are_invalid", + ) { + return; + } + let tmp = tempfile::tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(".env"), + "MAINNET_core_rpc_port=invalid\n\ + TESTNET_core_rpc_port=invalid\n\ + DEVNET_core_rpc_port=invalid\n\ + LOCAL_core_rpc_port=invalid\n", + ) + .expect("write test config"); + + let error = Config::load_from(tmp.path()).expect_err("all networks are invalid"); + + assert!(matches!(error, ConfigError::NoValidConfigs)); + } + // ── envy parsing roundtrip ────────────────────────────────────── #[test] diff --git a/src/context/mod.rs b/src/context/mod.rs index 70e215fd4..8078bb45b 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -149,10 +149,9 @@ pub struct AppContext { /// frame from the UI. Always present and idle on fresh installs; /// driven by [`MigrationTask::FinishUnwire`](crate::backend_task::migration::MigrationTask). pub(crate) migration_status: Arc, - /// Serializes complete storage-update runs. This prevents a GUI dispatch - /// and a shared MCP request from creating two password waiters for the same - /// wallet; a follower waits here and returns the leader's terminal result - /// without rerunning the update. + /// Serializes complete storage-update runs, including the detached automatic + /// DAPI refresh that continues after migration publishes terminal status. + /// This also prevents duplicate password waiters for the same wallet. pub(crate) migration_run: tokio::sync::Mutex<()>, /// Process-local claim shared by every UI surface before a paid DashPay /// request action enters its backend flow. diff --git a/src/ui/network_chooser_screen.rs b/src/ui/network_chooser_screen.rs index 822b45d88..c2c07cdaf 100644 --- a/src/ui/network_chooser_screen.rs +++ b/src/ui/network_chooser_screen.rs @@ -1,9 +1,9 @@ use crate::app::AppAction; use crate::backend_task::core::CoreTask; +use crate::backend_task::dapi_discovery::persist_dapi_addresses; use crate::backend_task::error::TaskError; use crate::backend_task::system_task::SystemTask; use crate::backend_task::{BackendTask, BackendTaskContext, BackendTaskSuccessResult}; -use crate::config::Config; use crate::context::AppContext; use crate::context::connection_status::OverallConnectionState; use crate::model::spv_status::{SpvStatus, SpvStatusSnapshot}; @@ -1473,33 +1473,29 @@ impl ScreenLike for NetworkChooserScreen { { self.discovery_in_progress = false; - // Update config with new addresses - let data_dir = &self.current_app_context().data_dir; - if let Ok(mut config) = Config::load_from(data_dir) { - let mut network_cfg = config - .config_for_network(network) - .clone() - .unwrap_or_default(); - network_cfg.dapi_addresses = Some(addresses_csv); - config.update_config_for_network(network, network_cfg.clone()); - - if let Err(e) = config.save(data_dir) { - tracing::error!("Failed to save config after DAPI discovery: {e}"); - } + let persistence_result = self + .context_for_network(network) + .cloned() + .ok_or(TaskError::DapiConfigContextUnavailable { network }) + .and_then(|app_context| persist_dapi_addresses(&app_context, addresses_csv)); - // Update in-memory config and schedule async SDK reinit - if let Some(app_context) = self.context_for_network(network) { - if let Ok(mut cfg_lock) = app_context.config.write() { - *cfg_lock = network_cfg; - } + match persistence_result { + Ok(()) => { self.pending_reinit_after_discovery = true; + MessageBanner::set_global( + self.current_app_context().egui_ctx(), + format!("Updated to {count} node addresses."), + MessageType::Success, + ); + } + Err(error) => { + MessageBanner::set_global( + self.current_app_context().egui_ctx(), + error.to_string(), + MessageType::Error, + ) + .with_details(error); } - - MessageBanner::set_global( - self.current_app_context().egui_ctx(), - format!("Updated to {count} node addresses."), - MessageType::Success, - ); } } }