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

@@ -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);
     });

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<sync::RwLock<d::BitcoinD>> {
+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()
+        self.get_block_hash(tip.height)
-            .unwrap()
-            .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<sync::Mutex<dyn BitcoinInterface + 'static>> {
+    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)
+    }
+}

src/bitcoin/poller/looper.rs

@@ -39,7 +39,7 @@ fn update_tip(bit: &impl BitcoinInterface, db_conn: &mut Box<dyn DatabaseConnect
 /// Main event loop. Repeatedly polls the Bitcoin interface until told to stop through the
 /// `shutdown` atomic.
 pub fn looper(
-    bit: impl BitcoinInterface,
+    bit: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
     db: impl DatabaseInterface,
     shutdown: sync::Arc<atomic::AtomicBool>,
     poll_interval: time::Duration,

src/bitcoin/poller/mod.rs

@@ -18,7 +18,7 @@ pub struct Poller {
 
 impl Poller {
     pub fn start(
-        bit: impl BitcoinInterface + 'static,
+        bit: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
         db: impl DatabaseInterface + 'static,
         poll_interval: time::Duration,
     ) -> Poller {

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,

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 {

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<BitcoinD, StartupError> {
+    // 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<dyn BitcoinInterface>,
+    bitcoin: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
     db: Box<dyn DatabaseInterface>,
     secp: secp256k1::Secp256k1<secp256k1::VerifyOnly>,
 }
@@ -158,7 +184,7 @@ pub struct DaemonControl {
 impl DaemonControl {
     pub fn new(
         config: Config,
-        bitcoin: Box<dyn BitcoinInterface>,
+        bitcoin: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
         db: Box<dyn DatabaseInterface>,
     ) -> 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<Self, StartupError> {
+    pub fn start(
+        config: Config,
+        bitcoin: Option<impl BitcoinInterface + 'static>,
+    ) -> Result<Self, StartupError> {
         #[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
+        // Now, set up the Bitcoin interface.
-        let wo_path: path::PathBuf = [
+        let bit = match bitcoin {
-            data_dir.as_path(),
+            Some(bit) => sync::Arc::from(sync::Mutex::from(bit)),
-            path::Path::new("minisafed_watchonly_wallet"),
+            None => sync::Arc::from(sync::Mutex::from(setup_bitcoind(
-        ]
+                &config,
-        .iter()
+                &data_dir,
-        .collect();
+                fresh_data_dir,
-        let bitcoind = BitcoinD::new(
+            )?)) as sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
-            &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.");
 
         // 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, StartupError> {
+        DaemonHandle::start(config, Option::<BitcoinD>::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();