diff --git a/dash-spv/src/test_utils/context.rs b/dash-spv/src/test_utils/context.rs index fbb66b4ed..24aec046b 100644 --- a/dash-spv/src/test_utils/context.rs +++ b/dash-spv/src/test_utils/context.rs @@ -9,7 +9,7 @@ use std::net::SocketAddr; use tempfile::TempDir; use tracing::info; -use super::fs_helpers::{copy_dir, retain_test_dir}; +use super::fs_helpers::{copy_dir, retain_test_dir, RetainOnPanic}; use super::node::TestChain; use super::{DashCoreConfig, DashCoreNode, WalletFile}; @@ -53,6 +53,8 @@ impl DashdTestContext { async fn create(mut config: DashCoreConfig) -> Self { let datadir = TempDir::new().expect("failed to create temp dir"); copy_dir(&config.datadir, datadir.path()).expect("failed to copy datadir"); + // Stale fixture locks are cleared in DashCoreNode::start (covers all + // callers, including masternode harnesses). config.datadir = datadir.path().to_path_buf(); config.wallet = "wallet".to_string(); @@ -62,14 +64,22 @@ impl DashdTestContext { wallet.wallet_name, wallet.transaction_count, wallet.utxo_count, wallet.balance ); + // retain_guard is declared before node so reverse-declaration drop order + // shuts dashd down (DashCoreNode::drop / stop_and_wait) before + // RetainOnPanic copies the datadir on post-start panics. + // start() failures retain via fail_startup instead, so the guard is + // installed only after start returns. + let retain_guard; let mut node = DashCoreNode::with_config(config); let addr = node.start().await; info!("DashCoreNode started at {}", addr); + retain_guard = RetainOnPanic::new(datadir.path(), "dashd-startup"); // Load a separate wallet for mining so coinbase rewards don't pollute // the test wallet's address space (the "wallet" wallet and SPV wallet - // share the same mnemonic). - node.ensure_wallet("default"); + // share the same mnemonic). The fixture already ships this wallet on + // disk — load only; never create. + node.load_wallet("default"); info!("Mining wallet 'default' ready"); let initial_height = node.get_block_count(); @@ -80,6 +90,7 @@ impl DashdTestContext { info!("RPC miner not available (tests requiring block generation will be skipped)"); } + retain_guard.defuse(); DashdTestContext { node, addr, @@ -94,6 +105,7 @@ impl DashdTestContext { impl Drop for DashdTestContext { fn drop(&mut self) { let label = format!("dashd-{}", self.addr.port()); + self.node.stop_and_wait(); retain_test_dir(self.datadir.path(), &label); } } diff --git a/dash-spv/src/test_utils/fs_helpers.rs b/dash-spv/src/test_utils/fs_helpers.rs index 258506cad..d8b1c9077 100644 --- a/dash-spv/src/test_utils/fs_helpers.rs +++ b/dash-spv/src/test_utils/fs_helpers.rs @@ -19,6 +19,58 @@ pub(super) fn copy_dir(src: &Path, dst: &Path) -> io::Result<()> { Ok(()) } +/// Remove runtime lock files that must not survive a datadir copy. +/// +/// The regtest fixtures are snapshots of a previously running node, so they +/// may contain `regtest/.lock` and per-wallet `.walletlock` files. A live +/// dashd refuses to start (or fails wallet load) when those are present. +pub(super) fn clear_stale_runtime_locks(datadir: &Path) -> io::Result<()> { + let regtest = datadir.join("regtest"); + remove_if_exists(®test.join(".lock"))?; + // Legacy single-wallet layout stores the lock at regtest/.walletlock. + remove_if_exists(®test.join(".walletlock"))?; + + // Named wallet directories may sit under regtest// or regtest/wallets//. + let wallets_root = regtest.join("wallets"); + for wallet_root in [®test, &wallets_root] { + let entries = match fs::read_dir(wallet_root) { + Ok(entries) => entries, + Err(e) if e.kind() == io::ErrorKind::NotFound => continue, + Err(e) => { + return Err(io::Error::new( + e.kind(), + format!("failed to read wallet root {}: {}", wallet_root.display(), e), + )); + } + }; + for entry in entries { + let entry = entry.map_err(|e| { + io::Error::new( + e.kind(), + format!("failed to read entry in {}: {}", wallet_root.display(), e), + ) + })?; + let path = entry.path(); + if entry.file_type()?.is_dir() { + remove_if_exists(&path.join(".walletlock"))?; + } + } + } + + Ok(()) +} + +fn remove_if_exists(path: &Path) -> io::Result<()> { + match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(io::Error::new( + e.kind(), + format!("failed to remove stale lock {}: {}", path.display(), e), + )), + } +} + /// When `DASHD_TEST_RETAIN_DIR` is set, copy `src` to a test-named /// subdirectory for post-mortem inspection. /// @@ -33,6 +85,14 @@ pub fn retain_test_dir(src: &Path, label: &str) { return; } + retain_test_dir_now(src, label); +} + +/// Unconditionally retain `src` when `DASHD_TEST_RETAIN_DIR` is set. +/// +/// Use this before panicking during setup that has not yet constructed a type +/// whose `Drop` impl calls [`retain_test_dir`]. +pub(super) fn retain_test_dir_now(src: &Path, label: &str) { let Ok(retain_dir) = std::env::var("DASHD_TEST_RETAIN_DIR") else { return; }; @@ -48,3 +108,75 @@ pub fn retain_test_dir(src: &Path, label: &str) { eprintln!("Test data retained at: {}", dest.display()); } } + +/// Retains `path` on panic drop when `DASHD_TEST_RETAIN_DIR` is set. +/// +/// Used while constructing [`super::DashdTestContext`] so startup failures +/// still leave dashd logs for CI artifacts. +pub(super) struct RetainOnPanic { + path: PathBuf, + label: String, +} + +impl RetainOnPanic { + pub(super) fn new(path: impl Into, label: impl Into) -> Self { + Self { + path: path.into(), + label: label.into(), + } + } + + pub(super) fn defuse(self) { + std::mem::forget(self); + } +} + +impl Drop for RetainOnPanic { + fn drop(&mut self) { + if std::thread::panicking() { + // Already know we are panicking; skip retain_test_dir's re-check. + retain_test_dir_now(&self.path, &self.label); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn remove_if_exists_treats_missing_file_as_success() { + let tmp = TempDir::new().unwrap(); + remove_if_exists(&tmp.path().join("missing.lock")).unwrap(); + } + + #[test] + fn remove_if_exists_propagates_removal_failures() { + let tmp = TempDir::new().unwrap(); + let lock_path = tmp.path().join(".lock"); + fs::create_dir(&lock_path).unwrap(); + + let err = remove_if_exists(&lock_path).unwrap_err(); + + assert_ne!(err.kind(), io::ErrorKind::NotFound); + } + + #[test] + fn clear_stale_runtime_locks_treats_missing_roots_as_success() { + let tmp = TempDir::new().unwrap(); + clear_stale_runtime_locks(tmp.path()).unwrap(); + } + + #[test] + fn clear_stale_runtime_locks_propagates_directory_read_failures() { + let tmp = TempDir::new().unwrap(); + let regtest = tmp.path().join("regtest"); + fs::create_dir(®test).unwrap(); + fs::write(regtest.join("wallets"), b"not a directory").unwrap(); + + let err = clear_stale_runtime_locks(tmp.path()).unwrap_err(); + + assert_ne!(err.kind(), io::ErrorKind::NotFound); + } +} diff --git a/dash-spv/src/test_utils/node.rs b/dash-spv/src/test_utils/node.rs index 41dcd9b71..de27171fc 100644 --- a/dash-spv/src/test_utils/node.rs +++ b/dash-spv/src/test_utils/node.rs @@ -9,13 +9,43 @@ use serde::Deserialize; use serde_json::{Map, Value}; use std::collections::HashMap; use std::fs; +use std::io::Read; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU16, Ordering}; use std::time::Duration; use tokio::process::Child; +use tokio::task; use tokio::time::{sleep, timeout}; +use super::fs_helpers::{clear_stale_runtime_locks, retain_test_dir_now}; + +/// Default readiness wait for dashd startup. +/// +/// Windows CI hosts frequently need longer than Unix runners when several +/// independent dashd processes start in parallel. Override with +/// `DASHD_STARTUP_TIMEOUT_SECS` when diagnosing slow environments. +fn readiness_timeout() -> Duration { + const DEFAULT_SECS: u64 = if cfg!(windows) { + 90 + } else { + 30 + }; + let secs = match std::env::var("DASHD_STARTUP_TIMEOUT_SECS") { + Ok(raw) => match raw.parse::() { + Ok(secs) if secs > 0 => secs, + _ => { + tracing::warn!( + "invalid DASHD_STARTUP_TIMEOUT_SECS={raw:?}; using default {DEFAULT_SECS}s" + ); + DEFAULT_SECS + } + }, + Err(_) => DEFAULT_SECS, + }; + Duration::from_secs(secs) +} + /// Atomic counter for unique port allocation across parallel tests. /// Starts below the standard Dash regtest ports (19898/19899) to avoid conflicts. static NEXT_PORT: AtomicU16 = AtomicU16::new(19400); @@ -133,6 +163,13 @@ impl DashCoreNode { tracing::info!(" RPC port: {}", self.config.rpc_port); fs::create_dir_all(&self.config.datadir).expect("failed to create datadir"); + // Fixture snapshots may include lock files from the process that built them. + clear_stale_runtime_locks(&self.config.datadir).unwrap_or_else(|e| { + panic!( + "failed to clear stale dashd runtime locks from {} before startup: {e}", + self.config.datadir.display() + ) + }); let mut args_vec = vec![ "-regtest".to_string(), @@ -170,22 +207,16 @@ impl DashCoreNode { self.process = Some(child); - tracing::info!("Waiting for dashd to be ready..."); + let ready_timeout = readiness_timeout(); + tracing::info!("Waiting for dashd to be ready (timeout {}s)...", ready_timeout.as_secs()); + // Brief yield so a process that dies on spawn is observed immediately. tokio::time::sleep(Duration::from_millis(500)).await; - if let Some(ref mut proc) = self.process { - if let Ok(Some(status)) = proc.try_wait() { - panic!("dashd exited immediately with status: {}", status); - } + if let Some(status) = self.process_exit_status() { + self.fail_startup(&format!("dashd exited immediately with status: {status}")).await; } - let ready = self.wait_for_ready().await; - if !ready { - if let Some(ref mut proc) = self.process { - if let Ok(Some(status)) = proc.try_wait() { - panic!("dashd exited with status: {}", status); - } - } - panic!("dashd failed to start within timeout"); + if let Err(reason) = self.wait_for_ready(ready_timeout).await { + self.fail_startup(&reason).await; } let addr = SocketAddr::from(([127, 0, 0, 1], self.config.p2p_port)); @@ -194,40 +225,130 @@ impl DashCoreNode { addr } - async fn wait_for_ready(&self) -> bool { - let max_wait = Duration::from_secs(30); + fn process_exit_status(&mut self) -> Option { + let proc = self.process.as_mut()?; + match proc.try_wait() { + Ok(status) => status, + Err(e) => { + tracing::warn!("failed to poll dashd process status: {e}"); + None + } + } + } + + async fn fail_startup(&mut self, reason: &str) -> ! { + // Kill dashd before reading/copying the datadir so Windows does not + // hit sharing violations on open LevelDB/wallet/debug.log handles. + let shutdown_status = self.terminate_process_for_startup().await; + + let debug_log = self.config.datadir.join("regtest/debug.log"); + let tail = read_log_tail(&debug_log, 40); + // Callers that need post-start retain (e.g. DashdTestContext) install + // RetainOnPanic only after start() returns, so this is the sole retain + // path for startup failures. + retain_test_dir_now(&self.config.datadir, &format!("dashd-{}", self.config.p2p_port)); + panic!( + "{reason}\n binary: {}\n datadir: {}\n p2p: {}\n rpc: {}\n {shutdown_status}\n debug.log tail:\n{tail}", + self.config.dashd_path.display(), + self.config.datadir.display(), + self.config.p2p_port, + self.config.rpc_port, + ); + } + + async fn terminate_process_for_startup(&mut self) -> String { + let Some(mut process) = self.process.take() else { + return "dashd shutdown: process was not running".to_string(); + }; + + let kill_result = process.start_kill(); + let wait_result = process.wait().await; + match (kill_result, wait_result) { + (Ok(()), Ok(status)) => format!("dashd shutdown: exited with {status}"), + (Err(kill_err), Ok(status)) => format!( + "dashd shutdown: kill request failed ({kill_err}); process exited with {status}" + ), + (Ok(()), Err(wait_err)) => { + format!("dashd shutdown: kill requested but wait failed: {wait_err}") + } + (Err(kill_err), Err(wait_err)) => { + format!("dashd shutdown: kill request failed ({kill_err}); wait failed: {wait_err}") + } + } + } + + async fn wait_for_ready(&mut self, max_wait: Duration) -> Result<(), String> { let check_interval = Duration::from_millis(500); + let mut last_rpc_error = String::from("no RPC attempt yet"); + let mut last_blockchain_error = String::from("no blockchain readiness attempt yet"); + let mut p2p_ready = false; + let mut cookie_seen = false; + let mut rpc_ready = false; let result = timeout(max_wait, async { - // Wait for the P2P port to accept connections loop { - let addr = SocketAddr::from(([127, 0, 0, 1], self.config.p2p_port)); - if tokio::net::TcpStream::connect(addr).await.is_ok() { - break; + if let Some(status) = self.process_exit_status() { + return Err(format!("dashd exited during startup with status: {status}")); } - sleep(check_interval).await; - } - // Wait for RPC to be fully responsive (not just "warming up") - loop { - let url = format!("http://127.0.0.1:{}", self.config.rpc_port); - let cookie_path = self.config.datadir.join("regtest/.cookie"); - if cookie_path.exists() { - if let Ok(client) = Client::new(&url, Auth::CookieFile(cookie_path)) { - match client.get_blockchain_info() { - Ok(_) => return true, - Err(e) => { - tracing::debug!("RPC not ready yet: {}", e); + let addr = SocketAddr::from(([127, 0, 0, 1], self.config.p2p_port)); + let current_p2p_ready = tokio::net::TcpStream::connect(addr).await.is_ok(); + if current_p2p_ready && !p2p_ready { + tracing::debug!("dashd P2P port accepting connections"); + } + p2p_ready = current_p2p_ready; + + let cookie_path = self.rpc_cookie_path(); + let current_cookie_seen = cookie_path.exists(); + cookie_seen |= current_cookie_seen; + if current_cookie_seen { + let rpc_port = self.config.rpc_port; + match task::spawn_blocking(move || { + probe_blockchain_ready(cookie_path, rpc_port) + }) + .await + { + Ok(Ok(())) => { + rpc_ready = true; + last_rpc_error = "RPC ready".to_string(); + last_blockchain_error = "blockchain ready".to_string(); + if p2p_ready { + return Ok(()); } } + Ok(Err(e)) => { + rpc_ready = false; + last_rpc_error = e.clone(); + last_blockchain_error = e; + tracing::debug!("RPC/blockchain not ready yet: {last_rpc_error}"); + } + Err(e) => { + rpc_ready = false; + last_rpc_error = format!("readiness task failed: {e}"); + last_blockchain_error = last_rpc_error.clone(); + tracing::debug!("RPC readiness task failed: {e}"); + } } + } else { + rpc_ready = false; + last_rpc_error = "RPC cookie file not created yet".to_string(); + tracing::debug!("RPC client not ready yet: {last_rpc_error}"); } sleep(check_interval).await; } }) .await; - result.unwrap_or(false) + match result { + Ok(Ok(())) => Ok(()), + Ok(Err(e)) => Err(e), + Err(_) => Err(format!( + "dashd failed to become ready within {}s \ + (p2p_ready={p2p_ready}, cookie_seen={cookie_seen}, rpc_ready={rpc_ready}, \ + last_rpc_error={last_rpc_error}, last_blockchain_error={last_blockchain_error})", + max_wait.as_secs() + )), + } } /// Get block count via RPC. @@ -243,29 +364,123 @@ impl DashCoreNode { /// Get an RPC client targeting a specific wallet. fn rpc_client_for_wallet(&self, wallet_name: &str) -> Client { - let url = format!("http://127.0.0.1:{}/wallet/{}", self.config.rpc_port, wallet_name); - let cookie_path = self.config.datadir.join("regtest/.cookie"); + self.rpc_client_at_path(&format!("/wallet/{wallet_name}")) + } + + /// Base (non-wallet) RPC client for node-global methods. + fn rpc_client_base(&self) -> Client { + self.rpc_client_at_path("") + } + + fn rpc_cookie_path(&self) -> PathBuf { + self.config.datadir.join("regtest/.cookie") + } + + fn rpc_client_at_path(&self, path: &str) -> Client { + let cookie_path = self.rpc_cookie_path(); assert!( cookie_path.exists(), "RPC cookie file not found at {}. Is dashd running with this datadir?", cookie_path.display() ); - let auth = Auth::CookieFile(cookie_path); - Client::new(&url, auth).expect("failed to create rpc client") + let url = format!("http://127.0.0.1:{}{path}", self.config.rpc_port); + Client::new(&url, Auth::CookieFile(cookie_path)).expect("failed to create rpc client") + } + + /// Soft base RPC client for readiness probes and best-effort RPCs. + /// + /// Returns a diagnostic string on failure so readiness timeouts can report + /// whether the cookie was missing or cookie auth itself failed. + fn try_rpc_client_base(&self) -> Result { + let cookie_path = self.rpc_cookie_path(); + if !cookie_path.exists() { + return Err("RPC cookie file not created yet".to_string()); + } + let url = format!("http://127.0.0.1:{}", self.config.rpc_port); + Client::new(&url, Auth::CookieFile(cookie_path)).map_err(|e| format!("cookie auth: {e}")) } - /// Load a wallet by name, creating it if it doesn't exist. + /// Ensure a wallet is loaded, creating it only when Core reports it is missing + /// and no on-disk database already exists. + /// + /// The regtest fixtures ship both a `wallet` and a `default` wallet on + /// disk. Treating every `loadwallet` failure as permission to call + /// `createwallet` races with those existing databases and panics with + /// "Database already exists" (especially under parallel Windows CI). + /// + /// Prefer [`Self::load_wallet`] when the wallet is known to ship in the + /// fixture (e.g. the mining `default` wallet). pub fn ensure_wallet(&self, wallet_name: &str) { - let client = self.rpc_client(); + // Wallet management RPCs are node-global; use the base endpoint so we + // are not coupled to whichever wallet was started with `-wallet=`. + let client = self.rpc_client_base(); match client.load_wallet(wallet_name) { - Ok(_) => tracing::info!("Loaded wallet: {}", wallet_name), - Err(_) => { - client - .create_wallet(wallet_name, None, None, None, None) - .unwrap_or_else(|e| panic!("failed to create wallet '{}': {}", wallet_name, e)); - tracing::info!("Created wallet: {}", wallet_name); + Ok(_) => { + tracing::info!("Loaded wallet: {wallet_name}"); + return; + } + Err(e) if wallet_already_loaded(&e) => { + tracing::info!("Wallet already loaded: {wallet_name}"); + return; + } + Err(e) if wallet_does_not_exist(&e) => { + if wallet_database_exists(self.config.datadir.as_path(), wallet_name) { + panic!( + "loadwallet reported wallet '{wallet_name}' missing, but a \ + database path already exists under the datadir: {e}" + ); + } + tracing::info!("Wallet {wallet_name} not found; creating"); + } + Err(e) => { + // Prefer loading an on-disk wallet over creating a new one when + // the error is ambiguous but the database path already exists. + if wallet_database_exists(self.config.datadir.as_path(), wallet_name) { + panic!( + "failed to load existing wallet '{wallet_name}' \ + (database present under datadir): {e}" + ); + } + panic!("failed to load wallet '{wallet_name}': {e}"); } } + + match client.create_wallet(wallet_name, None, None, None, None) { + Ok(_) => tracing::info!("Created wallet: {wallet_name}"), + Err(e) if wallet_already_loaded(&e) => { + tracing::info!("Wallet already loaded during create: {wallet_name}"); + } + Err(e) if wallet_already_exists(&e) => { + // Database appeared between load and create. Load it rather + // than treating the create error as success by name alone. + match client.load_wallet(wallet_name) { + Ok(_) => tracing::info!("Loaded wallet after create race: {wallet_name}"), + Err(load_err) if wallet_already_loaded(&load_err) => { + tracing::info!("Wallet already loaded after create race: {wallet_name}"); + } + Err(load_err) => panic!( + "failed to create wallet '{wallet_name}': {e}; \ + subsequent load also failed: {load_err}" + ), + } + } + Err(e) => panic!("failed to create wallet '{wallet_name}': {e}"), + } + } + + /// Load a wallet that is expected to already exist (fixture or prior create). + /// + /// Unlike [`Self::ensure_wallet`], this never calls `createwallet`, so a + /// shipped fixture wallet cannot race into "Database already exists". + pub fn load_wallet(&self, wallet_name: &str) { + let client = self.rpc_client_base(); + match client.load_wallet(wallet_name) { + Ok(_) => tracing::info!("Loaded wallet: {wallet_name}"), + Err(e) if wallet_already_loaded(&e) => { + tracing::info!("Wallet already loaded: {wallet_name}"); + } + Err(e) => panic!("failed to load expected wallet '{wallet_name}': {e}"), + } } pub fn get_new_address(&self) -> Address { @@ -308,12 +523,7 @@ impl DashCoreNode { /// Send DASH to an address from the primary wallet. pub fn send_to_address(&self, address: &Address, amount: Amount) -> Txid { - let client = self.rpc_client(); - let txid = client - .send_to_address(address, amount, None, None, None, None, None, None, None, None) - .expect("failed to send to address"); - tracing::info!("Sent {} to {}, txid: {}", amount, address, txid); - txid + self.send_to_address_from_wallet(&self.config.wallet, address, amount) } /// Send DASH to many addresses in a single transaction from the primary @@ -367,35 +577,20 @@ impl DashCoreNode { destination: &Address, fee: Amount, ) -> Txid { - let client = self.rpc_client_for_wallet(wallet_name); - - let inputs = vec![rpc_json::CreateRawTransactionInput { - txid: input_txid, - vout: input_vout, - sequence: None, - }]; - let send_amount = input_amount.checked_sub(fee).expect("fee exceeds input amount"); - let mut outputs = HashMap::new(); - outputs.insert(destination.to_string(), send_amount); - - let raw_tx: Transaction = client - .create_raw_transaction(&inputs, &outputs, None) - .expect("failed to create raw tx"); - - let signed = client - .sign_raw_transaction_with_wallet(&raw_tx, None, None) - .expect("failed to sign raw tx"); - assert!(signed.complete, "raw transaction signing incomplete"); - - let txid = client - .send_raw_transaction(&signed.transaction().expect("invalid signed tx")) - .expect("failed to send raw tx"); - tracing::info!( - "Sent raw tx from wallet '{}': {} -> {}, txid: {}", + let tx = self.create_signed_transaction( wallet_name, + input_txid, + input_vout, input_amount, destination, - txid + fee, + ); + let txid = self + .rpc_client_for_wallet(wallet_name) + .send_raw_transaction(&tx) + .expect("failed to send raw tx"); + tracing::info!( + "Sent raw tx from wallet '{wallet_name}': {input_amount} -> {destination}, txid: {txid}" ); txid } @@ -500,14 +695,7 @@ impl DashCoreNode { /// Uses the base URL (no wallet path) which works for all non-wallet RPCs. /// Useful during DKG orchestration where transient failures are expected. pub fn try_rpc_call(&self, method: &str, params: &[serde_json::Value]) -> Option { - let url = format!("http://127.0.0.1:{}", self.config.rpc_port); - let cookie_path = self.config.datadir.join("regtest/.cookie"); - if !cookie_path.exists() { - return None; - } - let auth = Auth::CookieFile(cookie_path); - let client = Client::new(&url, auth).ok()?; - client.call(method, params).ok() + self.try_rpc_client_base().ok()?.call(method, params).ok() } pub fn datadir(&self) -> &Path { @@ -521,19 +709,47 @@ impl DashCoreNode { pub fn rpc_port(&self) -> u16 { self.config.rpc_port } + + pub(super) fn stop_and_wait(&mut self) { + let Some(mut process) = self.process.take() else { + return; + }; + + tracing::info!("Stopping dashd process..."); + if let Err(e) = process.start_kill() { + tracing::warn!("Failed to request dashd shutdown: {}", e); + } + loop { + match process.try_wait() { + Ok(Some(status)) => { + tracing::info!("dashd process exited with {}", status); + break; + } + Ok(None) => { + std::thread::sleep(Duration::from_millis(50)); + } + Err(e) => { + tracing::warn!("Failed to wait for dashd process exit: {}", e); + break; + } + } + } + } } impl Drop for DashCoreNode { fn drop(&mut self) { - if let Some(mut process) = self.process.take() { - tracing::info!("Stopping dashd process in Drop..."); - if let Err(e) = process.start_kill() { - tracing::warn!("Failed to kill dashd process: {}", e); - } - } + self.stop_and_wait(); } } +fn probe_blockchain_ready(cookie_path: PathBuf, rpc_port: u16) -> Result<(), String> { + let url = format!("http://127.0.0.1:{rpc_port}"); + let client = Client::new(&url, Auth::CookieFile(cookie_path)) + .map_err(|e| format!("cookie auth: {e}"))?; + client.get_blockchain_info().map(|_| ()).map_err(|e| format!("getblockchaininfo: {e}")) +} + /// Wallet file structure for test wallets. #[derive(Debug, Deserialize)] pub struct WalletFile { @@ -565,3 +781,214 @@ impl WalletFile { serde_json::from_str(&contents).expect("Failed to deserialize wallet file") } } + +/// RPC error code used by Bitcoin/Dash Core when a wallet file is missing. +const RPC_WALLET_NOT_FOUND: i32 = -18; +/// RPC error code used when a wallet is already loaded. +const RPC_WALLET_ALREADY_LOADED: i32 = -35; +/// RPC error code used for generic wallet errors (e.g. database already exists). +const RPC_WALLET_ERROR: i32 = -4; + +fn rpc_error_parts(err: &dashcore_rpc::Error) -> Option<(i32, &str)> { + match err { + dashcore_rpc::Error::JsonRpc(dashcore_rpc::jsonrpc::Error::Rpc(rpc)) => { + Some((rpc.code, rpc.message.as_str())) + } + _ => None, + } +} + +/// True when `loadwallet`/`createwallet` reports the wallet is already loaded. +pub(crate) fn wallet_already_loaded(err: &dashcore_rpc::Error) -> bool { + match rpc_error_parts(err) { + Some((RPC_WALLET_ALREADY_LOADED, _)) => true, + Some((_, msg)) => { + let lower = msg.to_ascii_lowercase(); + lower.contains("already loaded") + } + None => false, + } +} + +/// True when `loadwallet` reports the wallet file does not exist. +pub(crate) fn wallet_does_not_exist(err: &dashcore_rpc::Error) -> bool { + match rpc_error_parts(err) { + Some((code, msg)) => { + let lower = msg.to_ascii_lowercase(); + // Require RPC_WALLET_NOT_FOUND (-18) and wallet-missing wording so + // unrelated nonzero codes (e.g. -32601 "Wallet method not found") + // never authorize createwallet. + let walletish = lower.contains("wallet"); + let missing = lower.contains("not found") || lower.contains("does not exist"); + code == RPC_WALLET_NOT_FOUND && walletish && missing + } + None => false, + } +} + +/// True when `createwallet` reports the wallet database already exists. +pub(crate) fn wallet_already_exists(err: &dashcore_rpc::Error) -> bool { + match rpc_error_parts(err) { + Some((code, msg)) => { + let lower = msg.to_ascii_lowercase(); + // Bitcoin/Dash Core uses -4 (RPC_WALLET_ERROR) for this; match the + // message so minor code drift does not reintroduce create-on-exists. + let message_match = lower.contains("database already exists") + || (lower.contains("already exists") + && (lower.contains("wallet") || lower.contains("database"))); + message_match || (code == RPC_WALLET_ERROR && lower.contains("already exists")) + } + None => false, + } +} + +/// Whether a wallet database directory already exists under the datadir. +pub(crate) fn wallet_database_exists(datadir: &Path, wallet_name: &str) -> bool { + let regtest = datadir.join("regtest"); + let candidates = [ + regtest.join(wallet_name).join("wallet.dat"), + regtest.join("wallets").join(wallet_name).join("wallet.dat"), + regtest.join(wallet_name), + regtest.join("wallets").join(wallet_name), + ]; + candidates.iter().any(|p| p.exists()) +} + +fn read_log_tail(path: &Path, max_lines: usize) -> String { + use std::io::{Seek, SeekFrom}; + + // Cap how much of debug.log we load: -debug=all against large fixtures can + // produce multi-hundred-MB logs that would OOM a full read on failure. + const MAX_TAIL_BYTES: u64 = 64 * 1024; + + let mut file = match fs::File::open(path) { + Ok(f) => f, + Err(e) => return format!(" ", path.display(), e), + }; + let len = match file.metadata() { + Ok(m) => m.len(), + Err(e) => return format!(" ", path.display(), e), + }; + if len > MAX_TAIL_BYTES { + if let Err(e) = file.seek(SeekFrom::End(-(MAX_TAIL_BYTES as i64))) { + return format!(" ", path.display(), e); + } + } + let mut contents = String::new(); + if let Err(e) = file.read_to_string(&mut contents) { + return format!(" ", path.display(), e); + } + // Drop a partial first line after a mid-file seek. + let body = if len > MAX_TAIL_BYTES { + contents.split_once('\n').map(|(_, rest)| rest).unwrap_or(&contents) + } else { + contents.as_str() + }; + let lines: Vec<&str> = body.lines().collect(); + let start = lines.len().saturating_sub(max_lines); + if lines.is_empty() { + return " ".to_string(); + } + lines[start..].iter().map(|l| format!(" {l}")).collect::>().join("\n") +} + +#[cfg(test)] +mod tests { + use super::*; + use dashcore_rpc::jsonrpc::error::RpcError; + use dashcore_rpc::Error as RpcErrorEnum; + use std::io::Write; + use tempfile::TempDir; + + fn rpc_err(code: i32, message: &str) -> RpcErrorEnum { + RpcErrorEnum::JsonRpc(dashcore_rpc::jsonrpc::Error::Rpc(RpcError { + code, + message: message.to_string(), + data: None, + })) + } + + #[test] + fn classifies_wallet_not_found() { + let err = rpc_err(RPC_WALLET_NOT_FOUND, "Wallet file not found."); + assert!(wallet_does_not_exist(&err)); + assert!(!wallet_already_loaded(&err)); + assert!(!wallet_already_exists(&err)); + + // Unrelated "not found" must not authorize createwallet. + let method = rpc_err(-32601, "Method not found"); + assert!(!wallet_does_not_exist(&method)); + } + + #[test] + fn wallet_not_found_requires_rpc_wallet_not_found_code() { + let err = rpc_err(RPC_WALLET_NOT_FOUND, "Wallet file does not exist."); + assert!(wallet_does_not_exist(&err)); + + let wrong_code = rpc_err(-32601, "Wallet file not found."); + assert!(!wallet_does_not_exist(&wrong_code)); + } + + #[test] + fn classifies_wallet_already_loaded() { + let err = rpc_err(RPC_WALLET_ALREADY_LOADED, "Wallet already loaded."); + assert!(wallet_already_loaded(&err)); + assert!(!wallet_does_not_exist(&err)); + } + + #[test] + fn classifies_database_already_exists() { + // Exact message observed on Windows CI for issue #903. + let err = rpc_err( + RPC_WALLET_ERROR, + "Wallet file verification failed. Failed to create database path \ + 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\.tmpwqBeK5\\regtest\\default'. \ + Database already exists.", + ); + assert!(wallet_already_exists(&err)); + // The old ensure_wallet treated this as create-permission; it must not + // be classified as a missing wallet. + assert!(!wallet_does_not_exist(&err)); + } + + #[test] + fn wallet_database_exists_detects_fixture_layout() { + let tmp = TempDir::new().unwrap(); + let wallet_dir = tmp.path().join("regtest").join("default"); + fs::create_dir_all(&wallet_dir).unwrap(); + fs::write(wallet_dir.join("wallet.dat"), b"dummy").unwrap(); + assert!(wallet_database_exists(tmp.path(), "default")); + assert!(!wallet_database_exists(tmp.path(), "missing")); + } + + #[test] + fn clear_stale_runtime_locks_removes_fixture_locks() { + let tmp = TempDir::new().unwrap(); + let regtest = tmp.path().join("regtest"); + let wallet = regtest.join("default"); + fs::create_dir_all(&wallet).unwrap(); + fs::write(regtest.join(".lock"), b"").unwrap(); + fs::write(regtest.join(".walletlock"), b"").unwrap(); + fs::write(wallet.join(".walletlock"), b"").unwrap(); + + clear_stale_runtime_locks(tmp.path()).unwrap(); + + assert!(!regtest.join(".lock").exists()); + assert!(!regtest.join(".walletlock").exists()); + assert!(!wallet.join(".walletlock").exists()); + } + + #[test] + fn read_log_tail_returns_last_lines() { + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("debug.log"); + let mut f = fs::File::create(&path).unwrap(); + for i in 0..10 { + writeln!(f, "line{i}").unwrap(); + } + let tail = read_log_tail(&path, 3); + assert!(tail.contains("line7")); + assert!(tail.contains("line9")); + assert!(!tail.contains("line0")); + } +} diff --git a/key-wallet/src/transaction_checking/transaction_router/mod.rs b/key-wallet/src/transaction_checking/transaction_router/mod.rs index 51f35acbe..0b2ebc0d9 100644 --- a/key-wallet/src/transaction_checking/transaction_router/mod.rs +++ b/key-wallet/src/transaction_checking/transaction_router/mod.rs @@ -88,7 +88,9 @@ impl TransactionRouter { /// label, never a precondition for discovery, so both shapes must consult the full set of /// fund-bearing accounts. An account only matches when a scriptPubKey or spent UTXO actually /// belongs to it, so checking extra accounts never produces false positives. - fn fund_bearing_account_types() -> Vec { + /// Visible to unit tests so routing assertions can compare against the + /// production list rather than re-listing the five types by hand. + pub(crate) fn fund_bearing_account_types() -> Vec { vec![ AccountTypeToCheck::StandardBIP44, AccountTypeToCheck::StandardBIP32, @@ -156,14 +158,17 @@ impl TransactionRouter { ]); accounts } - TransactionType::AssetUnlock => { - vec![AccountTypeToCheck::StandardBIP44, AccountTypeToCheck::StandardBIP32] + // Credit-side mirror of the AssetLock debit fix (#867 / #900): a coinbase + // (mining reward / masternode payout) or asset unlock (Platform credit + // withdrawal) can pay any user-chosen address, including CoinJoin and + // DashPay. Only the account types returned here are consulted for + // ownership, so omitting those accounts dropped the coin after the + // block was already downloaded. Discovery is membership-based like + // Dash Core's `IsMine`, so consulting the full fund-bearing set never + // yields false positives. + TransactionType::AssetUnlock | TransactionType::Coinbase => { + Self::fund_bearing_account_types() } - TransactionType::Coinbase => vec![ - // Check all account types for unknown special transactions - AccountTypeToCheck::StandardBIP44, - AccountTypeToCheck::StandardBIP32, - ], TransactionType::Ignored => vec![], } } diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs b/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs index 6371710b1..1ff4205c3 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs @@ -21,10 +21,13 @@ fn test_asset_unlock_routing() { let tx_type = TransactionType::AssetUnlock; let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - // Asset unlock only goes to standard accounts - assert_eq!(accounts.len(), 2); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + // Asset unlock withdrawals can pay any fund-bearing address (destination is + // user-chosen), so routing must cover the full fund-bearing set. + assert_eq!( + accounts, + TransactionRouter::fund_bearing_account_types(), + "AssetUnlock should route to all fund-bearing account types" + ); // Should NOT check identity accounts - those are for locks only assert!(!accounts.contains(&AccountTypeToCheck::IdentityRegistration)); @@ -62,9 +65,11 @@ fn test_asset_unlock_classification() { // Verify routing for AssetUnlock let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - assert_eq!(accounts.len(), 2, "AssetUnlock should route to exactly 2 account types"); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + assert_eq!( + accounts, + TransactionRouter::fund_bearing_account_types(), + "AssetUnlock should route to all fund-bearing account types" + ); } #[tokio::test] diff --git a/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs b/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs index 0ed906a49..0d59d675c 100644 --- a/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs +++ b/key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs @@ -273,13 +273,15 @@ fn test_coinbase_routing() { let tx_type = TransactionType::Coinbase; let accounts = TransactionRouter::get_relevant_account_types(&tx_type); - // Coinbase should route to standard accounts - assert_eq!(accounts.len(), 2, "Coinbase should route to exactly 2 account types"); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP44)); - assert!(accounts.contains(&AccountTypeToCheck::StandardBIP32)); + // Coinbase can pay any fund-bearing address (mining reward / masternode + // payout is user-chosen), so routing must cover the full fund-bearing set. + assert_eq!( + accounts, + TransactionRouter::fund_bearing_account_types(), + "Coinbase should route to all fund-bearing account types" + ); - // Should NOT route to special account types - assert!(!accounts.contains(&AccountTypeToCheck::CoinJoin)); + // Should NOT route to non-fund-bearing special account types assert!(!accounts.contains(&AccountTypeToCheck::IdentityRegistration)); assert!(!accounts.contains(&AccountTypeToCheck::ProviderOwnerKeys)); } diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index 7ea653924..15717d5b0 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -257,6 +257,32 @@ mod tests { use dashcore::{Address, BlockHash, TxIn, Txid}; use dashcore_hashes::Hash; + /// Wallet with a single CoinJoin account and one derived external address. + /// Shared fixture for credit/debit regressions that target CoinJoin ownership. + fn wallet_with_coinjoin_address() -> (Wallet, ManagedWalletInfo, Address) { + let network = Network::Testnet; + let mut wallet = Wallet::new_random(network, WalletAccountCreationOptions::None) + .expect("Should create wallet"); + wallet + .add_account( + AccountType::CoinJoin { + index: 0, + }, + None, + ) + .expect("Should add CoinJoin account"); + let mut managed_wallet = + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); + let coinjoin_xpub = + wallet.accounts.coinjoin_accounts.get(&0).expect("coinjoin account").account_xpub; + let coinjoin_address = managed_wallet + .first_coinjoin_managed_account_mut() + .expect("managed coinjoin") + .next_address(Some(&coinjoin_xpub), true) + .expect("coinjoin address"); + (wallet, managed_wallet, coinjoin_address) + } + /// Test wallet checker with unrelated transaction #[tokio::test] async fn test_wallet_checker_unrelated_transaction() { @@ -744,6 +770,180 @@ mod tests { ); } + /// Regression: a coinbase that pays a CoinJoin address must credit that account. + /// + /// `check_core_transaction` only consults the account types returned by + /// `TransactionRouter::get_relevant_account_types`. Before the fix, the + /// `Coinbase` arm returned only StandardBIP44/BIP32, so a mining reward or + /// masternode payout to a CoinJoin (or DashPay) address was never matched: + /// the block was still downloaded (filters query all scripts), but the + /// output was dropped purely by the account-type narrowing, undercounting + /// the balance (dashpay/rust-dashcore#900). Discovery is membership-based + /// like Dash Core's `IsMine`, so consulting the full fund-bearing set cannot + /// yield a false positive. + #[tokio::test] + async fn test_coinbase_paying_coinjoin_address_is_credited() { + use crate::transaction_checking::transaction_router::TransactionRouter; + + let (mut wallet, mut managed_wallet, coinjoin_address) = wallet_with_coinjoin_address(); + + let reward = 5_000_000_000u64; + let coinbase_tx = Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::all_zeros(), + vout: 0xffffffff, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: reward, + script_pubkey: coinjoin_address.script_pubkey(), + }], + special_transaction_payload: None, + }; + assert_eq!( + TransactionRouter::classify_transaction(&coinbase_tx), + TransactionType::Coinbase, + "tx must classify as Coinbase so it routes through the Coinbase arm" + ); + + let block_height = 100_000; + let context = TransactionContext::InBlock(BlockInfo::new( + block_height, + BlockHash::from_slice(&[9u8; 32]).expect("Should create block hash"), + 1_650_000_200, + )); + let result = managed_wallet + .check_core_transaction(&coinbase_tx, context, &mut wallet, true, true) + .await; + managed_wallet.update_last_processed_height(block_height); + + assert!(result.is_relevant, "coinbase paying a CoinJoin address must be relevant"); + assert_eq!(result.total_received, reward, "coinbase must credit the CoinJoin output value"); + + let coinjoin_account = + managed_wallet.first_coinjoin_managed_account().expect("coinjoin account"); + assert!( + coinjoin_account.transactions().contains_key(&coinbase_tx.txid()), + "coinbase must be recorded on the CoinJoin account" + ); + assert_eq!(coinjoin_account.utxos.len(), 1, "coinbase must create a CoinJoin UTXO"); + let utxo = coinjoin_account.utxos.values().next().expect("CoinJoin UTXO"); + assert!(utxo.is_coinbase, "credited UTXO must be marked coinbase"); + + // Aggregate wallet balance — the actual regression is a lost credit. + // Before the fix, routing never consulted CoinJoin, so the reward was + // dropped and immature balance stayed 0. + assert_eq!( + managed_wallet.balance.immature(), + reward, + "immature balance must credit the CoinJoin coinbase reward" + ); + assert_eq!( + managed_wallet.balance.total(), + reward, + "total balance must include the immature CoinJoin coinbase" + ); + } + + /// Sibling credit-side regression for AssetUnlock (Platform credit withdrawal). + /// + /// Same membership-based routing gap as the coinbase case above: before the + /// fix, `AssetUnlock` only consulted StandardBIP44/BIP32, so a withdrawal to + /// a CoinJoin address was never credited (dashpay/rust-dashcore#900). + #[tokio::test] + async fn test_asset_unlock_paying_coinjoin_address_is_credited() { + use crate::transaction_checking::transaction_router::TransactionRouter; + use dashcore::blockdata::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; + use dashcore::blockdata::transaction::special_transaction::asset_unlock::request_info::AssetUnlockRequestInfo; + use dashcore::blockdata::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBasePayload; + use dashcore::blockdata::transaction::special_transaction::TransactionPayload; + use dashcore::bls_sig_utils::BLSSignature; + + let (mut wallet, mut managed_wallet, coinjoin_address) = wallet_with_coinjoin_address(); + + let unlock_value = 100_000_000u64; + let asset_unlock_tx = Transaction { + version: 3, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::from_byte_array([1u8; 32]), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: unlock_value, + script_pubkey: coinjoin_address.script_pubkey(), + }], + special_transaction_payload: Some(TransactionPayload::AssetUnlockPayloadType( + AssetUnlockPayload { + base: AssetUnlockBasePayload { + version: 1, + index: 42, + fee: 1000, + }, + request_info: AssetUnlockRequestInfo { + request_height: 500_000, + quorum_hash: [5u8; 32].into(), + }, + quorum_sig: BLSSignature::from([6u8; 96]), + }, + )), + }; + assert_eq!( + TransactionRouter::classify_transaction(&asset_unlock_tx), + TransactionType::AssetUnlock, + "tx must classify as AssetUnlock so it routes through the AssetUnlock arm" + ); + + // Use InBlock (not chainlocked) so the full record is retained under the + // default `keep-finalized-transactions=OFF` feature; the load-bearing + // assertions are UTXO creation and confirmed balance credit. + let context = TransactionContext::InBlock(BlockInfo::new( + 500_100, + BlockHash::from_slice(&[10u8; 32]).expect("Should create block hash"), + 1_650_000_300, + )); + let result = managed_wallet + .check_core_transaction(&asset_unlock_tx, context, &mut wallet, true, true) + .await; + managed_wallet.update_last_processed_height(500_100); + + assert!(result.is_relevant, "asset unlock paying a CoinJoin address must be relevant"); + assert_eq!( + result.total_received, unlock_value, + "asset unlock must credit the CoinJoin output value" + ); + + let coinjoin_account = + managed_wallet.first_coinjoin_managed_account().expect("coinjoin account"); + assert!( + coinjoin_account.transactions().contains_key(&asset_unlock_tx.txid()), + "asset unlock must be recorded on the CoinJoin account" + ); + assert_eq!(coinjoin_account.utxos.len(), 1, "asset unlock must create a CoinJoin UTXO"); + + assert_eq!( + managed_wallet.balance.confirmed(), + unlock_value, + "confirmed balance must credit the CoinJoin asset-unlock withdrawal" + ); + assert_eq!( + managed_wallet.balance.total(), + unlock_value, + "total balance must include the CoinJoin asset-unlock credit" + ); + } + /// Test the full coinbase maturity flow - immature to mature transition #[tokio::test] async fn test_wallet_checker_immature_transaction_flow() {