From f365effacd12d0d3fce105dbc59e7ccf5f85488c Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sun, 7 Aug 2022 12:41:45 +0200 Subject: [PATCH] Accept a custom Bitcoin interface when starting the daemon The Bitcoin interface was thought of as being generic, but a caller couldn't use one different from bitcoind. Make it so they can, and fix our trait and generics implementations. --- src/bin/daemon.rs | 2 +- src/bitcoin/mod.rs | 25 ++++++++--- src/bitcoin/poller/looper.rs | 2 +- src/bitcoin/poller/mod.rs | 2 +- src/commands/mod.rs | 2 +- src/config.rs | 1 + src/lib.rs | 80 ++++++++++++++++++++++++------------ 7 files changed, 78 insertions(+), 36 deletions(-) diff --git a/src/bin/daemon.rs b/src/bin/daemon.rs index 734cf40d..13e7b366 100644 --- a/src/bin/daemon.rs +++ b/src/bin/daemon.rs @@ -59,7 +59,7 @@ fn main() { process::exit(1); }); - let daemon = DaemonHandle::start(config).unwrap_or_else(|e| { + let daemon = DaemonHandle::start_default(config).unwrap_or_else(|e| { // The panic hook will log::error panic!("Starting Minisafe daemon: {}", e); }); diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index fee4f427..0bf4170d 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -28,20 +28,33 @@ pub trait BitcoinInterface: Send { fn is_in_chain(&self, tip: &BlockChainTip) -> bool; } -impl BitcoinInterface for sync::Arc> { +impl BitcoinInterface for d::BitcoinD { fn sync_progress(&self) -> f64 { - self.read().unwrap().sync_progress() + self.sync_progress() } fn chain_tip(&self) -> BlockChainTip { - self.read().unwrap().chain_tip() + self.chain_tip() } fn is_in_chain(&self, tip: &BlockChainTip) -> bool { - self.read() - .unwrap() - .get_block_hash(tip.height) + self.get_block_hash(tip.height) .map(|bh| bh == tip.hash) .unwrap_or(false) } } + +// FIXME: do we need to repeat the entire trait implemenation? Isn't there a nicer way? +impl BitcoinInterface for sync::Arc> { + fn sync_progress(&self) -> f64 { + self.lock().unwrap().sync_progress() + } + + fn chain_tip(&self) -> BlockChainTip { + self.lock().unwrap().chain_tip() + } + + fn is_in_chain(&self, tip: &BlockChainTip) -> bool { + self.lock().unwrap().is_in_chain(tip) + } +} diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 0a212932..4bdb7350 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -39,7 +39,7 @@ fn update_tip(bit: &impl BitcoinInterface, db_conn: &mut Box>, db: impl DatabaseInterface, shutdown: sync::Arc, poll_interval: time::Duration, diff --git a/src/bitcoin/poller/mod.rs b/src/bitcoin/poller/mod.rs index 7ee7b5f0..23dd0b57 100644 --- a/src/bitcoin/poller/mod.rs +++ b/src/bitcoin/poller/mod.rs @@ -18,7 +18,7 @@ pub struct Poller { impl Poller { pub fn start( - bit: impl BitcoinInterface + 'static, + bit: sync::Arc>, db: impl DatabaseInterface + 'static, poll_interval: time::Duration, ) -> Poller { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 767e19ee..733d2b56 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -2,7 +2,7 @@ //! //! External interface to the Minisafe daemon. -use crate::{DaemonControl, VERSION}; +use crate::{bitcoin::BitcoinInterface, DaemonControl, VERSION}; use miniscript::{ bitcoin, diff --git a/src/config.rs b/src/config.rs index ec41369c..42085b74 100644 --- a/src/config.rs +++ b/src/config.rs @@ -50,6 +50,7 @@ fn default_daemon() -> bool { false } +// TODO: separate Bitcoin config and bitcoind-specific config. /// Everything we need to know for talking to bitcoind serenely #[derive(Debug, Clone, Deserialize, Serialize)] pub struct BitcoindConfig { diff --git a/src/lib.rs b/src/lib.rs index 526ffeb2..a50dcdb9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,9 +148,35 @@ fn create_datadir(datadir_path: &path::Path) -> Result<(), StartupError> { }; } +// Connect to bitcoind. Setup the watchonly wallet, and do some sanity checks. +// If all went well, returns the interface to bitcoind. +fn setup_bitcoind( + config: &Config, + data_dir: &path::Path, + fresh_data_dir: bool, +) -> Result { + // Now set up the bitcoind interface + let wo_path: path::PathBuf = [data_dir, path::Path::new("minisafed_watchonly_wallet")] + .iter() + .collect(); + let bitcoind = BitcoinD::new( + &config.bitcoind_config, + wo_path.to_str().expect("Must be valid unicode").to_string(), + )?; + if fresh_data_dir { + bitcoind.create_watchonly_wallet(&config.main_descriptor)?; + log::info!("Created a new watchonly wallet on bitcoind."); + } + bitcoind.try_load_watchonly_wallet(); + bitcoind.sanity_check(&config.main_descriptor, config.bitcoind_config.network)?; + log::info!("Connection to bitcoind established and checked."); + + Ok(bitcoind.with_retry_limit(None)) +} + pub struct DaemonControl { config: Config, - bitcoin: Box, + bitcoin: sync::Arc>, db: Box, secp: secp256k1::Secp256k1, } @@ -158,7 +184,7 @@ pub struct DaemonControl { impl DaemonControl { pub fn new( config: Config, - bitcoin: Box, + bitcoin: sync::Arc>, db: Box, ) -> DaemonControl { let secp = secp256k1::Secp256k1::verification_only(); @@ -179,9 +205,15 @@ pub struct DaemonHandle { impl DaemonHandle { /// This starts the Minisafe daemon. Call `shutdown` to shut it down. /// + /// You may specify a custom Bitcoin interface through the `bitcoin` parameter. If `None`, the + /// default Bitcoin interface (`bitcoind` JSONRPC) will be used. + /// /// **Note**: we internally use threads, and set a panic hook. A downstream application must /// not overwrite this panic hook. - pub fn start(config: Config) -> Result { + pub fn start( + config: Config, + bitcoin: Option, + ) -> Result { #[cfg(not(test))] setup_panic_hook(); @@ -212,24 +244,15 @@ impl DaemonHandle { sqlite.sanity_check(config.bitcoind_config.network, &config.main_descriptor)?; log::info!("Database initialized and checked."); - // Now set up the bitcoind interface - let wo_path: path::PathBuf = [ - data_dir.as_path(), - path::Path::new("minisafed_watchonly_wallet"), - ] - .iter() - .collect(); - let bitcoind = BitcoinD::new( - &config.bitcoind_config, - wo_path.to_str().expect("Must be valid unicode").to_string(), - )?; - if fresh_data_dir { - bitcoind.create_watchonly_wallet(&config.main_descriptor)?; - log::info!("Created a new watchonly wallet on bitcoind."); - } - bitcoind.try_load_watchonly_wallet(); - bitcoind.sanity_check(&config.main_descriptor, config.bitcoind_config.network)?; - log::info!("Connection to bitcoind established and checked."); + // Now, set up the Bitcoin interface. + let bit = match bitcoin { + Some(bit) => sync::Arc::from(sync::Mutex::from(bit)), + None => sync::Arc::from(sync::Mutex::from(setup_bitcoind( + &config, + &data_dir, + fresh_data_dir, + )?)) as sync::Arc>, + }; // If we are on a UNIX system and they told us to daemonize, do it now. // NOTE: it's safe to daemonize now, as we don't carry any open DB connection @@ -246,15 +269,14 @@ impl DaemonHandle { } // Spawn the bitcoind poller with a retry limit high enough that we'd fail after that. - let bitcoind = sync::Arc::from(sync::RwLock::from(bitcoind.with_retry_limit(None))); let bitcoin_poller = poller::Poller::start( - bitcoind.clone(), + bit.clone(), sqlite.clone(), config.bitcoind_config.poll_interval_secs, ); // Finally, set up the API. - let control = DaemonControl::new(config, Box::from(bitcoind), Box::from(sqlite)); + let control = DaemonControl::new(config, bit, Box::from(sqlite)); Ok(Self { control, @@ -262,6 +284,12 @@ impl DaemonHandle { }) } + /// Start the Minisafe daemon with the default Bitcoin and database interfaces (`bitcoind` RPC + /// and SQLite). + pub fn start_default(config: Config) -> Result { + DaemonHandle::start(config, Option::::None) + } + /// Start the JSONRPC server and listen for incoming commands until we die. /// Like DaemonHandle::shutdown(), this stops the Bitcoin poller at teardown. #[cfg(feature = "jsonrpc_server")] @@ -515,7 +543,7 @@ mod tests { let daemon_thread = thread::spawn({ let config = config.clone(); move || { - let handle = DaemonHandle::start(config).unwrap(); + let handle = DaemonHandle::start_default(config).unwrap(); // TODO: avoid scope creep. We should move the bitcoind-specific checks to the // bitcoind module, test the startup with a mocked bitcoind interface, and not test // commands here but in the commands module. @@ -545,7 +573,7 @@ mod tests { // The datadir is created now, so if we restart it it won't create the wo wallet. let daemon_thread = thread::spawn(move || { - let handle = DaemonHandle::start(config).unwrap(); + let handle = DaemonHandle::start_default(config).unwrap(); // TODO: avoid scope creep. See above comment. assert_ne!(handle.control.get_new_address().address, addr); handle.shutdown();