Merge #9: Introduce the commands module

2609061a69c99e5b5f05b2a105751e5509055d54 commands: implement getnewaddress (Antoine Poinsot)
8309c85e303ece0110ed1f84a5748ecc82f25566 commands: a module for the implementation of the API (Antoine Poinsot)
4b659bc35a0fdf0a853f80bcf836c9b9d0193c5b lib: introduce a VERSION constant, the daemon version (Antoine Poinsot)
fd86cfccee0c43306890829e4be7bd3666380147 lib: introduce a daemon controller for the API (Antoine Poinsot)

Pull request description:

  This module contains the implementation of our external API. Like the others, it's very inspired (or grossly copied from) `revaultd`.

  This PR implements the `getinfo` and `getnewaddress` commands to showcase the usage of the new `DaemonControl` command.

ACKs for top commit:
  darosior:
    ACK 2609061a69c99e5b5f05b2a105751e5509055d54, adapted from already reviewed `revaultd` code, and largely tested through follow-up #11

Tree-SHA512: 5e30a29807ec854b6d2e8ca0eb340ca45146cd726810875435f4d5959cd5d6f44ceed19439edaf6477e8bf747af3b551edc8d424c4462475a5924f1b8d13c648

src/bin/daemon.rs

@@ -58,10 +58,11 @@ fn main() {
         process::exit(1);
     });
 
-    let _ = DaemonHandle::start(config).unwrap_or_else(|e| {
+    let daemon = DaemonHandle::start(config).unwrap_or_else(|e| {
         // The panic hook will log::error
         panic!("Starting Minisafe daemon: {}", e);
     });
+    daemon.shutdown();
 
     // We are always logging to stdout, should it be then piped to the log file (if self) or
     // not. So just make sure that all messages were actually written.

src/commands/mod.rs

@@ -0,0 +1,58 @@
+//! # Minisafe commands
+//!
+//! External interface to the Minisafe daemon.
+
+use crate::{DaemonControl, VERSION};
+
+use miniscript::{
+    bitcoin,
+    descriptor::{self, DescriptorTrait},
+    TranslatePk2,
+};
+
+impl DaemonControl {
+    /// Get information about the current state of the daemon
+    pub fn get_info(&self) -> GetInfoResult {
+        GetInfoResult {
+            version: VERSION.to_string(),
+            network: self.config.bitcoind_config.network,
+            blockheight: self.bitcoin.chain_tip().height,
+            sync: self.bitcoin.sync_progress(),
+            descriptors: GetInfoDescriptors {
+                main: self.config.main_descriptor.clone(),
+            },
+        }
+    }
+
+    /// Get a new deposit address. This will always generate a new deposit address, regardless of
+    /// whether it was actually used.
+    pub fn get_new_address(&self) -> bitcoin::Address {
+        let mut db_conn = self.db.connection();
+        let index = db_conn.derivation_index();
+        // TODO: handle should we wrap around instead of failing?
+        db_conn.update_derivation_index(index.increment().expect("TODO: handle wraparound"));
+        self.config
+            .main_descriptor
+            // TODO: have a descriptor newtype along with a derived descriptor one.
+            .derive(index.into())
+            .translate_pk2(|xpk| xpk.derive_public_key(&self.secp))
+            .expect("All pubkeys were derived, no wildcard.")
+            .address(self.config.bitcoind_config.network)
+            .expect("It's a wsh() descriptor")
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct GetInfoDescriptors {
+    pub main: descriptor::Descriptor<descriptor::DescriptorPublicKey>,
+}
+
+/// Information about the daemon
+#[derive(Debug, Clone)]
+pub struct GetInfoResult {
+    pub version: String,
+    pub network: bitcoin::Network,
+    pub blockheight: i32,
+    pub sync: f64,
+    pub descriptors: GetInfoDescriptors,
+}

src/database/mod.rs

@@ -8,6 +8,8 @@ use crate::{
     database::sqlite::{schema::DbTip, SqliteConn, SqliteDb},
 };
 
+use miniscript::bitcoin::util::bip32;
+
 pub trait DatabaseInterface: Send {
     fn connection(&self) -> Box<dyn DatabaseConnection>;
 }
@@ -24,6 +26,10 @@ pub trait DatabaseConnection {
 
     /// Update our best chain seen.
     fn update_tip(&mut self, tip: &BlockChainTip);
+
+    fn derivation_index(&mut self) -> bip32::ChildNumber;
+
+    fn update_derivation_index(&mut self, index: bip32::ChildNumber);
 }
 
 impl DatabaseConnection for SqliteConn {
@@ -41,4 +47,12 @@ impl DatabaseConnection for SqliteConn {
     fn update_tip(&mut self, tip: &BlockChainTip) {
         self.update_tip(&tip)
     }
+
+    fn derivation_index(&mut self) -> bip32::ChildNumber {
+        self.db_wallet().deposit_derivation_index
+    }
+
+    fn update_derivation_index(&mut self, index: bip32::ChildNumber) {
+        self.update_derivation_index(index)
+    }
 }

src/database/sqlite/mod.rs

@@ -19,7 +19,10 @@ use crate::{
 
 use std::{convert::TryInto, fmt, io, path};
 
-use miniscript::{bitcoin, Descriptor, DescriptorPublicKey};
+use miniscript::{
+    bitcoin::{self, util::bip32},
+    Descriptor, DescriptorPublicKey,
+};
 
 const DB_VERSION: i64 = 0;
 
@@ -196,6 +199,21 @@ impl SqliteConn {
         })
         .expect("Database must be available")
     }
+
+    /// Update the deposit derivation index.
+    pub fn update_derivation_index(&mut self, index: bip32::ChildNumber) {
+        let new_index: u32 = index.into();
+        db_exec(&mut self.conn, |db_tx| {
+            // NOTE: should be updated if we ever have multi-wallet support
+            db_tx
+                .execute(
+                    "UPDATE wallets SET deposit_derivation_index = (?1)",
+                    rusqlite::params![new_index],
+                )
+                .map(|_| ())
+        })
+        .expect("Database must be available")
+    }
 }
 
 #[cfg(test)]

src/lib.rs

@@ -1,4 +1,5 @@
 mod bitcoin;
+mod commands;
 pub mod config;
 #[cfg(unix)]
 mod daemonize;
@@ -10,14 +11,19 @@ pub use miniscript;
 use crate::{
     bitcoin::{
         d::{BitcoinD, BitcoindError},
-        poller,
+        poller, BitcoinInterface,
     },
     config::{config_folder_path, Config},
-    database::sqlite::{FreshDbOptions, SqliteDb, SqliteDbError},
+    database::{
+        sqlite::{FreshDbOptions, SqliteDb, SqliteDbError},
+        DatabaseInterface,
+    },
 };
 
 use std::{error, fmt, fs, io, path, sync};
 
+use miniscript::bitcoin::secp256k1;
+
 #[cfg(not(test))]
 use std::{panic, process};
 // A panic in any thread should stop the main thread, and print the panic.
@@ -51,6 +57,20 @@ fn setup_panic_hook() {
     }));
 }
 
+#[derive(Debug, Clone)]
+pub struct Version {
+    pub major: u32,
+    pub minor: u32,
+}
+
+impl fmt::Display for Version {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}.{}", self.major, self.minor)
+    }
+}
+
+pub const VERSION: Version = Version { major: 0, minor: 1 };
+
 #[derive(Debug)]
 pub enum StartupError {
     Io(io::Error),
@@ -124,7 +144,33 @@ fn create_datadir(datadir_path: &path::Path) -> Result<(), StartupError> {
     };
 }
 
-pub struct DaemonHandle {}
+pub struct DaemonControl {
+    config: Config,
+    bitcoin: Box<dyn BitcoinInterface>,
+    db: Box<dyn DatabaseInterface>,
+    secp: secp256k1::Secp256k1<secp256k1::VerifyOnly>,
+}
+
+impl DaemonControl {
+    pub fn new(
+        config: Config,
+        bitcoin: Box<dyn BitcoinInterface>,
+        db: Box<dyn DatabaseInterface>,
+    ) -> DaemonControl {
+        let secp = secp256k1::Secp256k1::verification_only();
+        DaemonControl {
+            config,
+            bitcoin,
+            db,
+            secp,
+        }
+    }
+}
+
+pub struct DaemonHandle {
+    pub control: DaemonControl,
+    bitcoin_poller: poller::Poller,
+}
 
 impl DaemonHandle {
     /// This starts the Minisafe daemon. Call `shutdown` to shut it down.
@@ -138,6 +184,7 @@ impl DaemonHandle {
         // First, check the data directory
         let mut data_dir = config
             .data_dir
+            .clone()
             .unwrap_or(config_folder_path().ok_or(StartupError::DefaultDataDirNotFound)?);
         data_dir.push(config.bitcoind_config.network.to_string());
         let fresh_data_dir = !data_dir.as_path().exists();
@@ -158,8 +205,8 @@ impl DaemonHandle {
         } else {
             None
         };
-        let db = SqliteDb::new(db_path, options)?;
-        db.sanity_check(config.bitcoind_config.network, &config.main_descriptor)?;
+        let sqlite = SqliteDb::new(db_path, options)?;
+        sqlite.sanity_check(config.bitcoind_config.network, &config.main_descriptor)?;
         log::info!("Database initialized and checked.");
 
         // Now set up the bitcoind interface
@@ -197,19 +244,26 @@ 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 bit_poller = poller::Poller::start(
+        let bitcoin_poller = poller::Poller::start(
             bitcoind.clone(),
-            db,
+            sqlite.clone(),
             config.bitcoind_config.poll_interval_secs,
         );
-        bit_poller.stop();
 
-        Ok(Self {})
+        // Finally, set up the API.
+        let control = DaemonControl::new(config, Box::from(bitcoind), Box::from(sqlite));
+
+        Ok(Self {
+            control,
+            bitcoin_poller,
+        })
     }
 
     // NOTE: this moves out the data as it should not be reused after shutdown
     /// Shut down the Minisafe daemon.
-    pub fn shutdown(self) {}
+    pub fn shutdown(self) {
+        self.bitcoin_poller.stop();
+    }
 }
 
 #[cfg(all(test, unix))]
@@ -410,7 +464,7 @@ mod tests {
         };
 
         // Create a dummy config with this bitcoind
-        let desc_str = "wsh(andor(pk(03b506a1dbe57b4bf48c95e0c7d417b87dd3b4349d290d2e7e9ba72c912652d80a),older(10000),pk(0295e7f5d12a2061f1fd2286cefec592dff656a19f55f4f01305d6aa56630880ce)))#39x77spy";
+        let desc_str = "wsh(andor(pk(xpub68JJTXc1MWK8KLW4HGLXZBJknja7kDUJuFHnM424LbziEXsfkh1WQCiEjjHw4zLqSUm4rvhgyGkkuRowE9tCJSgt3TQB5J3SKAbZ2SdcKST/*),older(10000),pk(xpub68JJTXc1MWK8PEQozKsRatrUHXKFNkD1Cb1BuQU9Xr5moCv87anqGyXLyUd4KpnDyZgo3gz4aN1r3NiaoweFW8UutBsBbgKHzaD5HkTkifK/*)))#tk6wzexy";
         let desc = Descriptor::<DescriptorPublicKey>::from_str(desc_str).unwrap();
         let config = Config {
             bitcoind_config,
@@ -426,7 +480,21 @@ mod tests {
             let config = config.clone();
             move || {
                 let handle = DaemonHandle::start(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.
+                let addr = handle.control.get_new_address();
+                let addr2 = handle.control.get_new_address();
+                assert_eq!(
+                    addr,
+                    bitcoin::Address::from_str(
+                        "bc1qdu9dama0pwc6fd9lj4sqzq4f728y5q2ucqyj55mfzfvuxr268zks7yajm3"
+                    )
+                    .unwrap()
+                );
+                assert_ne!(addr, addr2);
                 handle.shutdown();
+                addr
             }
         });
         complete_sanity_check(&server);
@@ -437,11 +505,13 @@ mod tests {
         complete_wallet_check(&server, &wo_path);
         complete_desc_check(&server, desc_str);
         complete_sync_check(&server);
-        daemon_thread.join().unwrap();
+        let addr = daemon_thread.join().unwrap();
 
         // 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();
+            // TODO: avoid scope creep. See above comment.
+            assert_ne!(handle.control.get_new_address(), addr);
             handle.shutdown();
         });
         complete_sanity_check(&server);