diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 395dfdee..3efbaa25 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -607,7 +607,7 @@ impl BitcoinD { .collect() } - pub fn maybe_unload_watchonly_wallet(&self, watchonly_wallet_path: String) { + fn maybe_unload_watchonly_wallet(&self, watchonly_wallet_path: String) { while self.list_wallets().contains(&watchonly_wallet_path) { log::info!("Found a leftover watchonly wallet loaded on bitcoind. Removing it."); if let Some(e) = self.unload_wallet(watchonly_wallet_path.clone()) { diff --git a/src/lib.rs b/src/lib.rs index 7ed4f670..b0a577e9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,8 +89,6 @@ pub enum StartupError { DefaultDataDirNotFound, DatadirCreation(path::PathBuf, io::Error), MissingBitcoindConfig, - WindowsCantGuessBitcoindDatadir(path::PathBuf), - WindowsBitcoindWatchonlyDeletion(path::PathBuf, io::Error), Database(SqliteDbError), Bitcoind(BitcoindError), #[cfg(unix)] @@ -113,15 +111,6 @@ impl fmt::Display for StartupError { f, "Our Bitcoin interface is bitcoind but we have no 'bitcoind_config' entry in the configuration." ), - Self::WindowsCantGuessBitcoindDatadir(cookie_path) => write!( - f, - "Cannot guess the path to the bitcoind data directory from the cookie file whose path is '{}'.", - cookie_path.as_path().to_string_lossy() - ), - Self::WindowsBitcoindWatchonlyDeletion(path, e) => write!( - f, - "Error deleting bitcoind watchonly wallet at '{}': {}", path.as_path().to_string_lossy(), e - ), Self::Database(e) => write!(f, "Error initializing database: '{}'.", e), Self::Bitcoind(e) => write!(f, "Error setting up bitcoind interface: '{}'.", e), #[cfg(unix)] @@ -198,30 +187,31 @@ fn setup_sqlite( Ok(sqlite) } -// Windows-specific utility to remove a leftover watchonly wallet within bitcoind's datadir. -#[cfg(windows)] -fn maybe_delete_watchonly_wallet( - bitcoind: &BitcoinD, +// Try to copy the watchonly wallet from its former location on Windows to its new location. +fn copy_watchonly_wallet( bitcoind_cookie_path: &path::Path, bitcoin_net: miniscript::bitcoin::Network, wallet_name: &str, -) -> Result<(), StartupError> { - log::info!( - "Trying to guess where the watchonly wallet would be stored in bitcoind's data directory from the cookie path. \ - This might not work if you are using a custom path for the cookie file (very unlikely). In this case please delete the \ - leftover watchonly wallet in bitcoind's datadir by hand if there is any." - ); + new_wallet_path: &path::Path, +) -> bool { // For the main network both the wallet and the cookie file are stored at the root of the // datadir. For test networks the wallet is in "//wallets//" and // the cookie file in "//". - let parent_dir = bitcoind_cookie_path.parent().ok_or_else(|| { - StartupError::WindowsCantGuessBitcoindDatadir(bitcoind_cookie_path.to_path_buf()) - })?; + let parent_dir = match bitcoind_cookie_path.parent() { + Some(d) => d, + None => { + log::error!("Could not find the parent directory of the bitcoind cookie file."); + return false; + } + }; let wallet_path = match bitcoin_net { miniscript::bitcoin::Network::Bitcoin => parent_dir.join(wallet_name), miniscript::bitcoin::Network::Testnet | miniscript::bitcoin::Network::Signet - | miniscript::bitcoin::Network::Regtest => parent_dir.join("wallets").join(wallet_name), + | miniscript::bitcoin::Network::Regtest => parent_dir + .join("wallets") + .join(wallet_name) + .join("wallet.dat"), net => panic!( "Unsupported network '{}', unknown at the time of writing.", net @@ -229,21 +219,36 @@ fn maybe_delete_watchonly_wallet( }; if wallet_path.exists() { - bitcoind.maybe_unload_watchonly_wallet(wallet_path.to_string_lossy().to_string()); log::info!( - "Deleting the leftover watchonly wallet at '{}'.", + "Found the watchonly wallet file at the former location: '{}'. Copying it to our own datadir.", wallet_path.as_path().to_string_lossy() ); - fs::remove_dir_all(&wallet_path) - .map_err(|e| StartupError::WindowsBitcoindWatchonlyDeletion(wallet_path, e))?; + if let Err(e) = fs::create_dir(new_wallet_path) { + log::error!( + "Error while creating the watchonly wallet directory at {}: {}", + wallet_path.to_string_lossy(), + e + ); + return false; + } + let new_wallet_dat_path = new_wallet_path.join("wallet.dat"); + if let Err(e) = fs::copy(wallet_path, new_wallet_dat_path) { + log::error!( + "Error while copying the watchonly wallet to our own datadir: {}", + e + ); + false + } else { + log::info!("Successfully copied the watchonly wallet file."); + true + } } else { - log::info!( - "No leftover watchonly wallet found at '{}'.", + log::error!( + "No watchonly wallet found at '{}'.", wallet_path.as_path().to_string_lossy() ); + false } - - Ok(()) } // Connect to bitcoind. Setup the watchonly wallet, and do some sanity checks. @@ -253,43 +258,50 @@ fn setup_bitcoind( data_dir: &path::Path, fresh_data_dir: bool, ) -> Result { - // NOTE: this is a hack! We normally store the watchonly wallet within our data directory. - // But on windows bitcoind would prefix the wallet path with "C:\\\\?" when calling - // 'loadwallet'. Therefore instead on Windows store the wallet.dat in bitcoind's data directory - // instead by not providing an absolute path but the name of a wallet. - #[cfg(not(windows))] let wo_path: path::PathBuf = [data_dir, path::Path::new("lianad_watchonly_wallet")] .iter() .collect(); - #[cfg(windows)] - let wo_name = "lianad_watchonly_wallet"; - #[cfg(windows)] - let wo_path = path::Path::new(wo_name); + let wo_path_str = wo_path.to_str().expect("Must be valid unicode").to_string(); + // NOTE: On Windows, paths are canonicalized with a "\\?\" prefix to tell Windows to interpret + // the string "as is" and to ignore the maximum size of a path. HOWEVER this is not properly + // handled by most implementations of the C++ STL's std::filesystem. Therefore bitcoind would + // fail to find the wallet if we didn't strip this prefix. It's not ideal, but a lesser evil + // than other workarounds i could think about. + // See https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces + // about the prefix. + // See https://stackoverflow.com/questions/71590689/how-to-properly-handle-windows-paths-with-the-long-path-prefix-with-stdfilesys + // for a discussion of how one C++ STL implementation handles this. + #[cfg(target_os = "windows")] + let wo_path_str = wo_path_str.replace("\\\\?\\", "").replace("\\\\?", ""); let bitcoind_config = config .bitcoind_config .as_ref() .ok_or(StartupError::MissingBitcoindConfig)?; - let bitcoind = BitcoinD::new( - bitcoind_config, - wo_path.to_str().expect("Must be valid unicode").to_string(), - )?; + let bitcoind = BitcoinD::new(bitcoind_config, wo_path_str)?; bitcoind.node_sanity_checks(config.bitcoin_config.network)?; if fresh_data_dir { - // Because of the hack above, the assumption that whenever the data directory is fresh a - // watchonly wallet doesn't exist doesn't hold for Windows. Make sure it does by removing - // any leftover Liana watchonly wallet from bitcoind's data dir. - #[cfg(windows)] - maybe_delete_watchonly_wallet( - &bitcoind, - &bitcoind_config.cookie_path, - config.bitcoin_config.network, - wo_name, - )?; - log::info!("Creating a new watchonly wallet on bitcoind."); bitcoind.create_watchonly_wallet(&config.main_descriptor)?; log::info!("Watchonly wallet created."); + } else if !wo_path.exists() && !cfg!(test) { + // TODO: remove this hack. + // NOTE: this is Windows-specific but we don't gate it upon a windows target to be able to + // test this codepath in a functional test. + log::info!( + "A data directory exists with no watchonly wallet. This is most likely due to \ + having been created to an older version of Liana on Windows, where the watchonly \ + wallet would live under bitcoind's datadir. Trying to find the older wallet and copy it in our own datadir." + ); + let wo_name = "lianad_watchonly_wallet"; + if !copy_watchonly_wallet( + &bitcoind_config.cookie_path, + config.bitcoin_config.network, + wo_name, + wo_path.as_path(), + ) { + panic!("Cannot continue without a watchonly wallet. Please contact support."); + } } log::info!("Loading our watchonly wallet on bitcoind."); bitcoind.maybe_load_watchonly_wallet()?; diff --git a/tests/test_misc.py b/tests/test_misc.py index 6156a865..55841759 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1,5 +1,6 @@ import logging import pytest +import shutil import time from fixtures import * @@ -337,3 +338,23 @@ def test_retry_on_workqueue_exceeded(lianad, bitcoind, executor): # We should have retried the request to bitcoind, which should now succeed along with the call. # This just checks the response we get is sane, nothing particular with this field. assert "block_height" in f_liana.result(TIMEOUT) + + +def test_wo_wallet_copy_from_bitcoind_datadir(lianad, bitcoind): + """Simulate an old install on Windows and make sure the watchonly wallet gets migrated.""" + wo_name = "lianad_watchonly_wallet" + old_wo_path = os.path.join(bitcoind.bitcoin_dir, "regtest", "wallets", wo_name) + new_wo_path = os.path.join(lianad.datadir, "regtest", wo_name) + + # Start lianad. It'll have created the watchonly within our datadir. Move it where it would + # have been created by Liana v1 on Windows. + lianad.stop() + shutil.copytree(new_wo_path, old_wo_path) + shutil.rmtree(new_wo_path) + assert not os.path.isdir(new_wo_path) + + # Now restart lianad. It should detect it within the bitcoind datadir and copy it to its own. + lianad.start() + assert os.path.isdir(new_wo_path) + assert lianad.is_in_log("A data directory exists with no watchonly wallet. This is most likely due to.*") + assert lianad.is_in_log("Successfully copied the watchonly wallet file.")