nik/liana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge #656: lib: implement a superior workaround for the watchonly wallet on Windows

Browse Source 
5680ad27ecce8409d582ebf834bdecb3650b9233 lib: on Windows, migrate the watchonly wallet from bitcoind datadir (Antoine Poinsot)
c9d86f1c75be693e796591be9f1f468687728600 lib: implement a superior workaround for the watchonly wallet on Windows (Antoine Poinsot)

Pull request description:

  See the added comment for the details. No need to store the watchonly wallet under bitcoind's datadir anymore. 🎉 🎉

  I've noticed this while working on fixing #630 on Windows which failed for the same root reason as why the watchonly wallet path didn't work on Windows.

  Fixes #653.

ACKs for top commit:
  darosior:
    ACK 5680ad27ecce8409d582ebf834bdecb3650b9233 -- tested it on Windows
  edouardparis:
    utACK 5680ad27ecce8409d582ebf834bdecb3650b9233

Tree-SHA512: 52158340097e286d882e6503d8bc1fbd4653729c08055cd2609a120aabc409ed38cbd40c7cb0c3a6cb9c67f163f8084fd886daa979e56ccf6223eb51773500ef
This commit is contained in:
Antoine Poinsot 2023-08-30 18:35:50 +02:00
commit ddb530374e
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
3 changed files with 91 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/bitcoin/d/mod.rs
View File

@ -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()) {

126
src/lib.rs
View File

@ -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 "<datadir>/<network>/wallets/<wallet_name>/" and
// the cookie file in "<datadir>/<network>/".
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<BitcoinD, StartupError> {
// 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()?;

21
tests/test_misc.py
View File

@ -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.")