Repository: wizardsardine/liana
Pull request #1025 (2024-03-22 12:58:19)

fix wrong index in receive qrcode


Patch:
diff --git a/gui/src/app/state/receive.rs b/gui/src/app/state/receive.rs
index 0a710de4..7c7e431a 100644
--- a/gui/src/app/state/receive.rs
+++ b/gui/src/app/state/receive.rs
@@ -174,8 +174,11 @@ impl State for ReceivePanel {
                 )
             }
             Message::View(view::Message::ShowQrCode(i)) => {
-                if let Some(address) = self.addresses.list.get(i) {
-                    if let Some(modal) = ShowQrCodeModal::new(address, i) {
+                if let (Some(address), Some(index)) = (
+                    self.addresses.list.get(i),
+                    self.addresses.derivation_indexes.get(i),
+                ) {
+                    if let Some(modal) = ShowQrCodeModal::new(address, *index) {
                         self.modal = Modal::ShowQrCode(modal);
                     }
                 }
@@ -312,8 +315,8 @@ pub struct ShowQrCodeModal {
 }
 
 impl ShowQrCodeModal {
-    pub fn new(address: &Address, i: usize) -> Option<Self> {
-        qr_code::State::new(format!("bitcoin:{}?index={}", address, i))
+    pub fn new(address: &Address, index: ChildNumber) -> Option<Self> {
+        qr_code::State::new(format!("bitcoin:{}?index={}", address, index))
             .ok()
             .map(|qr_code| Self {
                 qr_code,
diff --git a/src/bin/daemon.rs b/src/bin/daemon.rs
index 3917dc11..51e3c134 100644
--- a/src/bin/daemon.rs
+++ b/src/bin/daemon.rs
@@ -70,16 +70,13 @@ fn main() {
         process::exit(1);
     });
 
-    let handle = DaemonHandle::start_default(config, true).unwrap_or_else(|e| {
+    let daemon = DaemonHandle::start_default(config).unwrap_or_else(|e| {
         log::error!("Error starting Liana daemon: {}", e);
         process::exit(1);
     });
-    while handle.is_alive() {
-        thread::sleep(time::Duration::from_millis(500));
-    }
-    if let Err(e) = handle.stop() {
-        log::error!("Error stopping Liana daemon: {}", e);
-    }
+    daemon
+        .rpc_server()
+        .expect("JSONRPC server must terminate cleanly");
 
     // 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.
diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs
index 93c85749..cdff3920 100644
--- a/src/bitcoin/poller/looper.rs
+++ b/src/bitcoin/poller/looper.rs
@@ -4,7 +4,11 @@ use crate::{
     descriptors,
 };
 
-use std::{collections::HashSet, sync, time};
+use std::{
+    collections::HashSet,
+    sync::{self, atomic},
+    thread, time,
+};
 
 use miniscript::bitcoin::{self, secp256k1};
 
@@ -204,11 +208,13 @@ fn new_tip(bit: &impl BitcoinInterface, current_tip: &BlockChainTip) -> TipUpdat
 }
 
 fn updates(
-    db_conn: &mut Box<dyn DatabaseConnection>,
     bit: &impl BitcoinInterface,
+    db: &impl DatabaseInterface,
     descs: &[descriptors::SinglePathLianaDesc],
     secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
 ) {
+    let mut db_conn = db.connection();
+
     // Check if there was a new block before updating ourselves.
     let current_tip = db_conn.chain_tip().expect("Always set at first startup");
     let latest_tip = match new_tip(bit, &current_tip) {
@@ -219,18 +225,18 @@ fn updates(
             // between our former chain and the new one, then restart fresh.
             db_conn.rollback_tip(&new_tip);
             log::info!("Tip was rolled back to '{}'.", new_tip);
-            return updates(db_conn, bit, descs, secp);
+            return updates(bit, db, descs, secp);
         }
     };
 
     // Then check the state of our coins. Do it even if the tip did not change since last poll, as
     // we may have unconfirmed transactions.
-    let updated_coins = update_coins(bit, db_conn, &current_tip, descs, secp);
+    let updated_coins = update_coins(bit, &mut db_conn, &current_tip, descs, secp);
 
     // If the tip changed while we were polling our Bitcoin interface, start over.
     if bit.chain_tip() != latest_tip {
         log::info!("Chain tip changed while we were updating our state. Starting over.");
-        return updates(db_conn, bit, descs, secp);
+        return updates(bit, db, descs, secp);
     }
 
     // The chain tip did not change since we started our updates. Record them and the latest tip.
@@ -252,12 +258,13 @@ fn updates(
 
 // Check if there is any rescan of the backend ongoing or one that just finished.
 fn rescan_check(
-    db_conn: &mut Box<dyn DatabaseConnection>,
     bit: &impl BitcoinInterface,
+    db: &impl DatabaseInterface,
     descs: &[descriptors::SinglePathLianaDesc],
     secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
 ) {
     log::debug!("Checking the state of an ongoing rescan if there is any");
+    let mut db_conn = db.connection();
 
     // Check if there is an ongoing rescan. If there isn't and we previously asked for a rescan of
     // the backend, we treat it as completed.
@@ -292,14 +299,14 @@ fn rescan_check(
             "Rolling back our internal tip to '{}' to update our internal state with past transactions.",
             rescan_tip
         );
-        updates(db_conn, bit, descs, secp)
+        updates(bit, db, descs, secp)
     } else {
         log::debug!("No ongoing rescan.");
     }
 }
 
-/// If the database chain tip is NULL (first startup), initialize it.
-pub fn maybe_initialize_tip(bit: &impl BitcoinInterface, db: &impl DatabaseInterface) {
+// If the database chain tip is NULL (first startup), initialize it.
+fn maybe_initialize_tip(bit: &impl BitcoinInterface, db: &impl DatabaseInterface) {
     let mut db_conn = db.connection();
 
     if db_conn.chain_tip().is_none() {
@@ -308,7 +315,7 @@ pub fn maybe_initialize_tip(bit: &impl BitcoinInterface, db: &impl DatabaseInter
     }
 }
 
-pub fn sync_poll_interval() -> time::Duration {
+fn sync_poll_interval() -> time::Duration {
     // TODO: be smarter, like in revaultd, but more generic too.
     #[cfg(not(test))]
     {
@@ -318,14 +325,60 @@ pub fn sync_poll_interval() -> time::Duration {
     time::Duration::from_secs(0)
 }
 
-/// Update our state from the Bitcoin backend.
-pub fn poll(
-    bit: &sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
-    db: &sync::Arc<sync::Mutex<dyn DatabaseInterface>>,
-    secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
-    descs: &[descriptors::SinglePathLianaDesc],
+/// Main event loop. Repeatedly polls the Bitcoin interface until told to stop through the
+/// `shutdown` atomic.
+pub fn looper(
+    bit: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
+    db: sync::Arc<sync::Mutex<dyn DatabaseInterface>>,
+    shutdown: sync::Arc<atomic::AtomicBool>,
+    poll_interval: time::Duration,
+    desc: descriptors::LianaDescriptor,
 ) {
-    let mut db_conn = db.connection();
-    updates(&mut db_conn, bit, descs, secp);
-    rescan_check(&mut db_conn, bit, descs, secp);
+    let mut last_poll = None;
+    let mut synced = false;
+    let descs = [
+        desc.receive_descriptor().clone(),
+        desc.change_descriptor().clone(),
+    ];
+    let secp = secp256k1::Secp256k1::verification_only();
+
+    maybe_initialize_tip(&bit, &db);
+
+    while !shutdown.load(atomic::Ordering::Relaxed) || last_poll.is_none() {
+        let now = time::Instant::now();
+
+        if let Some(last_poll) = last_poll {
+            let time_since_poll = now.duration_since(last_poll);
+            let poll_interval = if synced {
+                poll_interval
+            } else {
+                // Until we are synced we poll less often to avoid harassing bitcoind and impeding
+                // the sync. As a function since it's mocked for the tests.
+                sync_poll_interval()
+            };
+            if time_since_poll < poll_interval {
+                thread::sleep(time::Duration::from_millis(500));
+                continue;
+            }
+        }
+        last_poll = Some(now);
+
+        // Don't poll until the Bitcoin backend is fully synced.
+        if !synced {
+            let progress = bit.sync_progress();
+            log::info!(
+                "Block chain synchronization progress: {:.2}% ({} blocks / {} headers)",
+                progress.rounded_up_progress() * 100.0,
+                progress.blocks,
+                progress.headers
+            );
+            synced = progress.is_complete();
+            if !synced {
+                continue;
+            }
+        }
+
+        updates(&bit, &db, &descs, &secp);
+        rescan_check(&bit, &db, &descs, &secp);
+    }
 }
diff --git a/src/bitcoin/poller/mod.rs b/src/bitcoin/poller/mod.rs
index 8e9874e1..7731af2e 100644
--- a/src/bitcoin/poller/mod.rs
+++ b/src/bitcoin/poller/mod.rs
@@ -1,128 +1,60 @@
 mod looper;
 
-use crate::{bitcoin::BitcoinInterface, database::DatabaseInterface, descriptors};
+use crate::{
+    bitcoin::{poller::looper::looper, BitcoinInterface},
+    database::DatabaseInterface,
+    descriptors,
+};
 
 use std::{
-    sync::{self, mpsc},
-    time,
+    sync::{self, atomic},
+    thread, time,
 };
 
-use miniscript::bitcoin::secp256k1;
-
-#[derive(Debug, Clone)]
-pub enum PollerMessage {
-    Shutdown,
-    /// Ask the Bitcoin poller to poll immediately, get notified through the passed channel once
-    /// it's done.
-    PollNow(mpsc::SyncSender<()>),
-}
-
 /// The Bitcoin poller handler.
 pub struct Poller {
-    bit: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
-    db: sync::Arc<sync::Mutex<dyn DatabaseInterface>>,
-    secp: secp256k1::Secp256k1<secp256k1::VerifyOnly>,
-    // The receive and change descriptors (in this order).
-    descs: [descriptors::SinglePathLianaDesc; 2],
+    handle: thread::JoinHandle<()>,
+    shutdown: sync::Arc<atomic::AtomicBool>,
 }
 
 impl Poller {
-    pub fn new(
+    pub fn start(
         bit: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
         db: sync::Arc<sync::Mutex<dyn DatabaseInterface>>,
+        poll_interval: time::Duration,
         desc: descriptors::LianaDescriptor,
     ) -> Poller {
-        let secp = secp256k1::Secp256k1::verification_only();
-        let descs = [
-            desc.receive_descriptor().clone(),
-            desc.change_descriptor().clone(),
-        ];
-
-        // On first startup the tip may be NULL. Make sure it's set as the poller relies on it.
-        looper::maybe_initialize_tip(&bit, &db);
-
-        Poller {
-            bit,
-            db,
-            secp,
-            descs,
-        }
+        let shutdown = sync::Arc::from(atomic::AtomicBool::from(false));
+        let handle = thread::Builder::new()
+            .name("Bitcoin poller".to_string())
+            .spawn({
+                let shutdown = shutdown.clone();
+                move || looper(bit, db, shutdown, poll_interval, desc)
+            })
+            .expect("Must not fail");
+
+        Poller { shutdown, handle }
     }
 
-    /// Continuously update our state from the Bitcoin backend.
-    /// - `poll_interval`: how frequently to perform an update.
-    /// - `shutdown`: set to true to stop continuously updating and make this function return.
-    ///
-    /// Typically this would run for the whole duration of the program in a thread, and the main
-    /// thread would set the `shutdown` atomic to `true` when shutting down.
-    pub fn poll_forever(
-        &self,
-        poll_interval: time::Duration,
-        receiver: mpsc::Receiver<PollerMessage>,
-    ) {
-        let mut last_poll = None;
-        let mut synced = false;
-
-        loop {
-            // How long to wait before the next poll.
-            let time_before_poll = if let Some(last_poll) = last_poll {
-                let time_since_poll = time::Instant::now().duration_since(last_poll);
-                // Until we are synced we poll less often to avoid harassing bitcoind and impeding
-                // the sync. As a function since it's mocked for the tests.
-                let poll_interval = if synced {
-                    poll_interval
-                } else {
-                    looper::sync_poll_interval()
-                };
-                poll_interval.saturating_sub(time_since_poll)
-            } else {
-                // Don't wait before doing the first poll.
-                time::Duration::ZERO
-            };
+    pub fn trigger_stop(&self) {
+        self.shutdown.store(true, atomic::Ordering::Relaxed);
+    }
 
-            // Wait for the duration of the interval between polls, but listen to messages in the
-            // meantime.
-            match receiver.recv_timeout(time_before_poll) {
-                Ok(PollerMessage::Shutdown) => {
-                    log::info!("Bitcoin poller was told to shut down.");
-                    return;
-                }
-                Ok(PollerMessage::PollNow(sender)) => {
-                    // We've been asked to poll, don't wait any further and signal completion to
-                    // the caller.
-                    last_poll = Some(time::Instant::now());
-                    looper::poll(&self.bit, &self.db, &self.secp, &self.descs);
-                    if let Err(e) = sender.send(()) {
-                        log::error!("Error sending immediate poll completion signal: {}.", e);
-                    }
-                    continue;
-                }
-                Err(mpsc::RecvTimeoutError::Timeout) => {
-                    // It's been long enough since the last poll.
-                }
-                Err(mpsc::RecvTimeoutError::Disconnected) => {
-                    log::error!("Bitcoin poller communication channel got disconnected. Exiting.");
-                    return;
-                }
-            }
-            last_poll = Some(time::Instant::now());
+    pub fn stop(self) {
+        self.trigger_stop();
+        self.handle.join().expect("The poller loop must not fail");
+    }
 
-            // Don't poll until the Bitcoin backend is fully synced.
-            if !synced {
-                let progress = self.bit.sync_progress();
-                log::info!(
-                    "Block chain synchronization progress: {:.2}% ({} blocks / {} headers)",
-                    progress.rounded_up_progress() * 100.0,
-                    progress.blocks,
-                    progress.headers
-                );
-                synced = progress.is_complete();
-                if !synced {
-                    continue;
-                }
-            }
+    #[cfg(feature = "nonblocking_shutdown")]
+    pub fn is_stopped(&self) -> bool {
+        // Doc says "This might return true for a brief moment after the thread’s main function has
+        // returned, but before the thread itself has stopped running.". But it's not an issue for
+        // us, as long as the main poller function has returned we are good.
+        self.handle.is_finished()
+    }
 
-            looper::poll(&self.bit, &self.db, &self.secp, &self.descs);
-        }
+    #[cfg(test)]
+    pub fn test_stop(&mut self) {
+        self.shutdown.store(true, atomic::Ordering::Relaxed);
     }
 }
diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index a77fb8d2..6cb372d7 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -8,7 +8,6 @@ use crate::{
     bitcoin::BitcoinInterface,
     database::{Coin, DatabaseConnection, DatabaseInterface},
     descriptors,
-    poller::PollerMessage,
     spend::{
         create_spend, AddrInfo, AncestorInfo, CandidateCoin, CreateSpendRes, SpendCreationError,
         SpendOutputAddress, SpendTxFees, TxGetter,
@@ -25,8 +24,7 @@ use utils::{
 
 use std::{
     collections::{hash_map, HashMap, HashSet},
-    fmt,
-    sync::{self, mpsc},
+    fmt, sync,
 };
 
 use miniscript::{
@@ -690,18 +688,7 @@ impl DaemonControl {
         let final_tx = spend_psbt.extract_tx_unchecked_fee_rate();
         self.bitcoin
             .broadcast_tx(&final_tx)
-            .map_err(CommandError::TxBroadcast)?;
-
-        // Finally, update our state with the changes from this transaction.
-        let (tx, rx) = mpsc::sync_channel(0);
-        if let Err(e) = self.poller_sender.send(PollerMessage::PollNow(tx)) {
-            log::error!("Error requesting update from poller: {}", e);
-        }
-        if let Err(e) = rx.recv() {
-            log::error!("Error receiving completion signal from poller: {}", e);
-        }
-
-        Ok(())
+            .map_err(CommandError::TxBroadcast)
     }
 
     /// Create PSBT to replace the given transaction using RBF.
@@ -1264,7 +1251,7 @@ mod tests {
     fn getinfo() {
         let ms = DummyLiana::new(DummyBitcoind::new(), DummyDatabase::new());
         // We can query getinfo
-        ms.control().get_info();
+        ms.handle.control.get_info();
         ms.shutdown();
     }
 
@@ -1272,7 +1259,7 @@ mod tests {
     fn getnewaddress() {
         let ms = DummyLiana::new(DummyBitcoind::new(), DummyDatabase::new());
 
-        let control = &ms.control();
+        let control = &ms.handle.control;
         // We can get an address
         let addr = control.get_new_address().address;
         assert_eq!(
@@ -1294,7 +1281,7 @@ mod tests {
     fn listaddresses() {
         let ms = DummyLiana::new(DummyBitcoind::new(), DummyDatabase::new());
 
-        let control = &ms.control();
+        let control = &ms.handle.control;
 
         let list = control.list_addresses(Some(2), Some(5)).unwrap();
 
@@ -1425,7 +1412,7 @@ mod tests {
             ),
         );
         let ms = DummyLiana::new(dummy_bitcoind, DummyDatabase::new());
-        let control = &ms.control();
+        let control = &ms.handle.control;
 
         // Arguments sanity checking
         let dummy_addr =
@@ -1913,7 +1900,7 @@ mod tests {
             .insert(dummy_op_a.txid, (dummy_tx.clone(), None));
         dummy_bitcoind.txs.insert(dummy_op_b.txid, (dummy_tx, None));
         let ms = DummyLiana::new(dummy_bitcoind, DummyDatabase::new());
-        let control = &ms.control();
+        let control = &ms.handle.control;
         let mut db_conn = control.db().lock().unwrap().connection();
 
         // Add two (unconfirmed) coins in DB
@@ -2059,7 +2046,7 @@ mod tests {
         let dummy_txid_a = dummy_psbt_a.unsigned_tx.txid();
         dummy_bitcoind.txs.insert(dummy_txid_a, (dummy_tx_a, None));
         let ms = DummyLiana::new(dummy_bitcoind, DummyDatabase::new());
-        let control = &ms.control();
+        let control = &ms.handle.control;
         let mut db_conn = control.db().lock().unwrap().connection();
         // The spend needs to be in DB before using RBF.
         assert_eq!(
@@ -2297,7 +2284,7 @@ mod tests {
 
         let ms = DummyLiana::new(btc, db);
 
-        let control = &ms.control();
+        let control = &ms.handle.control;
 
         let transactions = control.list_confirmed_transactions(0, 4, 10).transactions;
         assert_eq!(transactions.len(), 4);
@@ -2429,7 +2416,7 @@ mod tests {
 
         let ms = DummyLiana::new(btc, DummyDatabase::new());
 
-        let control = &ms.control();
+        let control = &ms.handle.control;
 
         let transactions = control.list_transactions(&[tx1.txid()]).transactions;
         assert_eq!(transactions.len(), 1);
diff --git a/src/jsonrpc/server.rs b/src/jsonrpc/server.rs
index 424adc2a..dc6f7ca1 100644
--- a/src/jsonrpc/server.rs
+++ b/src/jsonrpc/server.rs
@@ -122,11 +122,11 @@ fn connection_handler(
 pub fn rpcserver_loop(
     listener: net::UnixListener,
     daemon_control: DaemonControl,
-    shutdown: sync::Arc<atomic::AtomicBool>,
 ) -> Result<(), io::Error> {
     // Keep it simple. We don't need great performances so just treat each connection in
     // its thread, with a given maximum number of connections.
     let connections_counter = sync::Arc::from(atomic::AtomicU32::new(0));
+    let shutdown = sync::Arc::from(atomic::AtomicBool::new(false));
 
     listener.set_nonblocking(true)?;
     while !shutdown.load(atomic::Ordering::Relaxed) {
@@ -400,7 +400,7 @@ mod tests {
     #[cfg(not(target_os = "macos"))]
     #[test]
     fn server_sanity_check() {
-        let ms = DummyLiana::new_server(DummyBitcoind::new(), DummyDatabase::new());
+        let ms = DummyLiana::new(DummyBitcoind::new(), DummyDatabase::new());
         let socket_path: path::PathBuf = [
             ms.tmp_dir.as_path(),
             path::Path::new("d"),
@@ -410,6 +410,7 @@ mod tests {
         .iter()
         .collect();
 
+        let t = thread::spawn(move || ms.rpc_server().unwrap());
         while !socket_path.exists() {
             thread::sleep(time::Duration::from_millis(100));
         }
@@ -425,6 +426,6 @@ mod tests {
             &[&serde_json::to_vec(&stop_req).unwrap(), b"\n"],
         );
 
-        ms.shutdown();
+        t.join().unwrap();
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
index 19f02a63..b608b24e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -28,16 +28,12 @@ use crate::{
     },
 };
 
-use std::{
-    error, fmt, fs, io, path,
-    sync::{self, mpsc},
-    thread,
-};
+use std::{error, fmt, fs, io, path, sync};
 
 use miniscript::bitcoin::secp256k1;
 
 #[cfg(not(test))]
-use std::panic;
+use std::{panic, process};
 // A panic in any thread should stop the main thread, and print the panic.
 #[cfg(not(test))]
 fn setup_panic_hook() {
@@ -64,6 +60,8 @@ fn setup_panic_hook() {
             info,
             bt
         );
+
+        process::exit(1);
     }));
 }
 
@@ -257,24 +255,21 @@ fn setup_bitcoind(
 pub struct DaemonControl {
     config: Config,
     bitcoin: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
-    poller_sender: mpsc::SyncSender<poller::PollerMessage>,
     // FIXME: Should we require Sync on DatabaseInterface rather than using a Mutex?
     db: sync::Arc<sync::Mutex<dyn DatabaseInterface>>,
     secp: secp256k1::Secp256k1<secp256k1::VerifyOnly>,
 }
 
 impl DaemonControl {
-    pub(crate) fn new(
+    pub fn new(
         config: Config,
         bitcoin: sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
-        poller_sender: mpsc::SyncSender<poller::PollerMessage>,
         db: sync::Arc<sync::Mutex<dyn DatabaseInterface>>,
         secp: secp256k1::Secp256k1<secp256k1::VerifyOnly>,
     ) -> DaemonControl {
         DaemonControl {
             config,
             bitcoin,
-            poller_sender,
             db,
             secp,
         }
@@ -287,39 +282,25 @@ impl DaemonControl {
     }
 }
 
-/// The handle to a Liana daemon. It might either be the handle for a daemon which exposes a
-/// JSONRPC server or one which exposes its API through a `DaemonControl`.
-pub enum DaemonHandle {
-    Controller {
-        poller_sender: mpsc::SyncSender<poller::PollerMessage>,
-        poller_handle: thread::JoinHandle<()>,
-        control: DaemonControl,
-    },
-    #[cfg(feature = "daemon")]
-    Server {
-        poller_sender: mpsc::SyncSender<poller::PollerMessage>,
-        poller_handle: thread::JoinHandle<()>,
-        rpcserver_shutdown: sync::Arc<sync::atomic::AtomicBool>,
-        rpcserver_handle: thread::JoinHandle<Result<(), io::Error>>,
-    },
+pub struct DaemonHandle {
+    pub control: DaemonControl,
+    bitcoin_poller: poller::Poller,
 }
 
 impl DaemonHandle {
-    /// This starts the Liana daemon. A user of this interface should regularly poll the `is_alive`
-    /// method to check for internal errors. To shut down the daemon use the `stop` method.
-    ///
-    /// The `with_rpc_server` controls whether we should start a JSONRPC server to receive queries
-    /// or instead return a `DaemonControl` object for a caller to access the daemon's API.
+    /// This starts the Liana 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.
     /// You may specify a custom Database interface through the `db` parameter. If `None`, the
     /// default Database interface (SQLite) 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,
         bitcoin: Option<impl BitcoinInterface + 'static>,
         db: Option<impl DatabaseInterface + 'static>,
-        #[cfg(feature = "daemon")] with_rpc_server: bool,
     ) -> Result<Self, StartupError> {
         #[cfg(not(test))]
         setup_panic_hook();
@@ -372,126 +353,82 @@ impl DaemonHandle {
             }
         }
 
-        // Start the poller thread. Keep the thread handle to be able to check if it crashed. Store
-        // an atomic to be able to stop it.
-        let bitcoin_poller =
-            poller::Poller::new(bit.clone(), db.clone(), config.main_descriptor.clone());
-        let (poller_sender, poller_receiver) = mpsc::sync_channel(0);
-        let poller_handle = thread::Builder::new()
-            .name("Bitcoin Network poller".to_string())
-            .spawn({
-                let poll_interval = config.bitcoin_config.poll_interval_secs;
-                move || {
-                    log::info!("Bitcoin poller started.");
-                    bitcoin_poller.poll_forever(poll_interval, poller_receiver);
-                    log::info!("Bitcoin poller stopped.");
-                }
-            })
-            .expect("Spawning the poller thread must never fail.");
-
-        // Create the API the external world will use to talk to us, either directly through the Rust
-        // structure or through the JSONRPC server we may setup below.
-        let control = DaemonControl::new(config, bit, poller_sender.clone(), db, secp);
-
-        #[cfg(feature = "daemon")]
-        if with_rpc_server {
-            let rpcserver_shutdown = sync::Arc::from(sync::atomic::AtomicBool::from(false));
-            let rpcserver_handle = thread::Builder::new()
-                .name("Bitcoin Network poller".to_string())
-                .spawn({
-                    let shutdown = rpcserver_shutdown.clone();
-                    move || {
-                        let mut rpc_socket = data_dir;
-                        rpc_socket.push("lianad_rpc");
-                        let listener = rpcserver_setup(&rpc_socket)?;
-                        log::info!("JSONRPC server started.");
-
-                        rpcserver_loop(listener, control, shutdown)?;
-                        log::info!("JSONRPC server stopped.");
-                        Ok(())
-                    }
-                })
-                .expect("Spawning the RPC server thread should never fail.");
-
-            return Ok(DaemonHandle::Server {
-                poller_sender,
-                poller_handle,
-                rpcserver_shutdown,
-                rpcserver_handle,
-            });
-        }
+        // Spawn the bitcoind poller with a retry limit high enough that we'd fail after that.
+        let bitcoin_poller = poller::Poller::start(
+            bit.clone(),
+            db.clone(),
+            config.bitcoin_config.poll_interval_secs,
+            config.main_descriptor.clone(),
+        );
+
+        // Finally, set up the API.
+        let control = DaemonControl::new(config, bit, db, secp);
 
-        Ok(DaemonHandle::Controller {
-            poller_sender,
-            poller_handle,
+        Ok(Self {
             control,
+            bitcoin_poller,
         })
     }
 
     /// Start the Liana daemon with the default Bitcoin and database interfaces (`bitcoind` RPC
     /// and SQLite).
-    pub fn start_default(
-        config: Config,
-        #[cfg(feature = "daemon")] with_rpc_server: bool,
-    ) -> Result<DaemonHandle, StartupError> {
-        Self::start(
-            config,
-            Option::<BitcoinD>::None,
-            Option::<SqliteDb>::None,
-            #[cfg(feature = "daemon")]
-            with_rpc_server,
-        )
+    pub fn start_default(config: Config) -> Result<DaemonHandle, StartupError> {
+        DaemonHandle::start(config, Option::<BitcoinD>::None, Option::<SqliteDb>::None)
     }
 
-    /// Check whether the daemon is still up and running. This needs to be regularly polled to
-    /// check for internal errors. If this returns `false`, collect the error using the `stop`
-    /// method.
-    pub fn is_alive(&self) -> bool {
-        match self {
-            Self::Controller {
-                ref poller_handle, ..
-            } => !poller_handle.is_finished(),
-            #[cfg(feature = "daemon")]
-            Self::Server {
-                ref poller_handle,
-                ref rpcserver_handle,
-                ..
-            } => !poller_handle.is_finished() && !rpcserver_handle.is_finished(),
-        }
+    /// Start the JSONRPC server and listen for incoming commands until we die.
+    /// Like DaemonHandle::shutdown(), this stops the Bitcoin poller at teardown.
+    #[cfg(feature = "daemon")]
+    pub fn rpc_server(self) -> Result<(), io::Error> {
+        let DaemonHandle {
+            control,
+            bitcoin_poller: poller,
+        } = self;
+
+        let rpc_socket: path::PathBuf = [
+            control
+                .config
+                .data_dir()
+                .expect("Didn't fail at startup, must not now")
+                .as_path(),
+            path::Path::new(&control.config.bitcoin_config.network.to_string()),
+            path::Path::new("lianad_rpc"),
+        ]
+        .iter()
+        .collect();
+        let listener = rpcserver_setup(&rpc_socket)?;
+        log::info!("JSONRPC server started.");
+
+        rpcserver_loop(listener, control)?;
+        log::info!("JSONRPC server stopped.");
+
+        poller.stop();
+
+        Ok(())
     }
 
-    /// Stop the Liana daemon. This returns any error which may have occurred.
-    pub fn stop(self) -> Result<(), Box<dyn error::Error>> {
-        match self {
-            Self::Controller {
-                poller_sender,
-                poller_handle,
-                ..
-            } => {
-                poller_sender
-                    .send(poller::PollerMessage::Shutdown)
-                    .expect("The other end should never have hung up before this.");
-                poller_handle.join().expect("Poller thread must not panic");
-                Ok(())
-            }
-            #[cfg(feature = "daemon")]
-            Self::Server {
-                poller_sender,
-                poller_handle,
-                rpcserver_shutdown,
-                rpcserver_handle,
-            } => {
-                poller_sender
-                    .send(poller::PollerMessage::Shutdown)
-                    .expect("The other end should never have hung up before this.");
-                rpcserver_shutdown.store(true, sync::atomic::Ordering::Relaxed);
-                rpcserver_handle
-                    .join()
-                    .expect("Poller thread must not panic")?;
-                poller_handle.join().expect("Poller thread must not panic");
-                Ok(())
-            }
-        }
+    /// Shut down the Liana daemon.
+    pub fn shutdown(self) {
+        self.bitcoin_poller.stop();
+    }
+
+    /// Tell the daemon to shut down. This will return before the shutdown completes. The structure
+    /// must not be reused after triggering shutdown.
+    #[cfg(feature = "nonblocking_shutdown")]
+    pub fn trigger_shutdown(&self) {
+        self.bitcoin_poller.trigger_stop()
+    }
+
+    /// Whether the daemon has finished shutting down.
+    #[cfg(feature = "nonblocking_shutdown")]
+    pub fn shutdown_complete(&self) -> bool {
+        self.bitcoin_poller.is_stopped()
+    }
+
+    // We need a shutdown utility that does not move for implementing Drop for the DummyLiana
+    #[cfg(test)]
+    pub fn test_shutdown(&mut self) {
+        self.bitcoin_poller.test_stop();
     }
 }
 
@@ -671,6 +608,18 @@ mod tests {
         stream.flush().unwrap();
     }
 
+    // Send them a response to 'getblockchaininfo' saying we are far from being synced
+    fn complete_sync_check(server: &net::TcpListener) {
+        let net_resp = [
+            "HTTP/1.1 200\n\r\n{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"verificationprogress\":0.1,\"headers\":1000,\"blocks\":100}}\n".as_bytes(),
+        ]
+        .concat();
+        let (mut stream, _) = server.accept().unwrap();
+        read_til_json_end(&mut stream);
+        stream.write_all(&net_resp).unwrap();
+        stream.flush().unwrap();
+    }
+
     // TODO: we could move the dummy bitcoind thread stuff to the bitcoind module to test the
     // bitcoind interface, and use the DummyLiana from testutils to sanity check the startup.
     // Note that startup as checked by this unit test is also tested in the functional test
@@ -732,16 +681,11 @@ mod tests {
         };
 
         // Start the daemon in a new thread so the current one acts as the bitcoind server.
-        let t = thread::spawn({
+        let daemon_thread = thread::spawn({
             let config = config.clone();
             move || {
-                let handle = DaemonHandle::start_default(
-                    config,
-                    #[cfg(feature = "daemon")]
-                    false,
-                )
-                .unwrap();
-                handle.stop().unwrap();
+                let handle = DaemonHandle::start_default(config).unwrap();
+                handle.shutdown();
             }
         });
         complete_sanity_check(&server);
@@ -752,22 +696,13 @@ mod tests {
         complete_wallet_check(&server, &wo_path);
         complete_desc_check(&server, &receive_desc.to_string(), &change_desc.to_string());
         complete_tip_init(&server);
-        // We don't have to complete the sync check as the poller checks whether it needs to stop
-        // before checking the bitcoind sync status.
-        t.join().unwrap();
+        complete_sync_check(&server);
+        daemon_thread.join().unwrap();
 
         // The datadir is created now, so if we restart it it won't create the wo wallet.
-        let t = thread::spawn({
-            let config = config.clone();
-            move || {
-                let handle = DaemonHandle::start_default(
-                    config,
-                    #[cfg(feature = "daemon")]
-                    false,
-                )
-                .unwrap();
-                handle.stop().unwrap();
-            }
+        let daemon_thread = thread::spawn(move || {
+            let handle = DaemonHandle::start_default(config).unwrap();
+            handle.shutdown();
         });
         complete_sanity_check(&server);
         complete_version_check(&server);
@@ -775,9 +710,8 @@ mod tests {
         complete_wallet_loading(&server);
         complete_wallet_check(&server, &wo_path);
         complete_desc_check(&server, &receive_desc.to_string(), &change_desc.to_string());
-        // We don't have to complete the sync check as the poller checks whether it needs to stop
-        // before checking the bitcoind sync status.
-        t.join().unwrap();
+        complete_sync_check(&server);
+        daemon_thread.join().unwrap();
 
         fs::remove_dir_all(&tmp_dir).unwrap();
     }


Test patch:
diff --git a/src/testutils.rs b/src/testutils.rs
index 712d3906..9e2adfc0 100644
--- a/src/testutils.rs
+++ b/src/testutils.rs
@@ -2,13 +2,13 @@ use crate::{
     bitcoin::{BitcoinInterface, Block, BlockChainTip, MempoolEntry, SyncProgress, UTxO},
     config::{BitcoinConfig, Config},
     database::{BlockInfo, Coin, CoinStatus, DatabaseConnection, DatabaseInterface, LabelItem},
-    descriptors, DaemonControl, DaemonHandle,
+    descriptors, DaemonHandle,
 };
 
 use std::convert::TryInto;
 use std::{
     collections::{HashMap, HashSet},
-    env, fs, path, process,
+    env, fs, io, path, process,
     str::FromStr,
     sync, thread, time,
     time::{SystemTime, UNIX_EPOCH},
@@ -464,10 +464,9 @@ pub fn tmp_dir() -> path::PathBuf {
 
 impl DummyLiana {
     /// Creates a new DummyLiana interface
-    pub fn _new(
+    pub fn new(
         bitcoin_interface: impl BitcoinInterface + 'static,
         database: impl DatabaseInterface + 'static,
-        rpc_server: bool,
     ) -> DummyLiana {
         let tmp_dir = tmp_dir();
         fs::create_dir_all(&tmp_dir).unwrap();
@@ -498,44 +497,19 @@ impl DummyLiana {
             main_descriptor: desc,
         };
 
-        let handle = DaemonHandle::start(
-            config,
-            Some(bitcoin_interface),
-            Some(database),
-            #[cfg(feature = "daemon")]
-            rpc_server,
-        )
-        .unwrap();
+        let handle = DaemonHandle::start(config, Some(bitcoin_interface), Some(database)).unwrap();
         DummyLiana { tmp_dir, handle }
     }
 
-    /// Creates a new DummyLiana interface
-    pub fn new(
-        bitcoin_interface: impl BitcoinInterface + 'static,
-        database: impl DatabaseInterface + 'static,
-    ) -> DummyLiana {
-        Self::_new(bitcoin_interface, database, false)
-    }
-
-    /// Creates a new DummyLiana interface which also spins up an RPC server.
     #[cfg(feature = "daemon")]
-    pub fn new_server(
-        bitcoin_interface: impl BitcoinInterface + 'static,
-        database: impl DatabaseInterface + 'static,
-    ) -> DummyLiana {
-        Self::_new(bitcoin_interface, database, true)
-    }
-
-    pub fn control(&self) -> &DaemonControl {
-        match self.handle {
-            DaemonHandle::Controller { ref control, .. } => control,
-            #[cfg(feature = "daemon")]
-            DaemonHandle::Server { .. } => unreachable!(),
-        }
+    pub fn rpc_server(self) -> Result<(), io::Error> {
+        self.handle.rpc_server()?;
+        fs::remove_dir_all(&self.tmp_dir)?;
+        Ok(())
     }
 
     pub fn shutdown(self) {
-        self.handle.stop().unwrap();
-        fs::remove_dir_all(self.tmp_dir).unwrap();
+        self.handle.shutdown();
+        fs::remove_dir_all(&self.tmp_dir).unwrap();
     }
 }