From e172ecd2f1e910a5ba16ea4debb0091b70c2cb17 Mon Sep 17 00:00:00 2001
From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com>
Date: Fri, 23 Feb 2024 17:18:33 +0000
Subject: [PATCH 1/6] gui: move internal bitcoind config to bitcoind module

As part of this change, the struct fields were made public.
---
 gui/src/bitcoind.rs                | 209 ++++++++++++++++++++++++++++
 gui/src/installer/context.rs       |   4 +-
 gui/src/installer/step/bitcoind.rs | 212 +----------------------------
 gui/src/installer/step/mod.rs      |   3 +-
 4 files changed, 215 insertions(+), 213 deletions(-)

diff --git a/gui/src/bitcoind.rs b/gui/src/bitcoind.rs
index 69aaa5dc..83653dba 100644
--- a/gui/src/bitcoind.rs
+++ b/gui/src/bitcoind.rs
@@ -3,6 +3,7 @@ use liana::{
     miniscript::bitcoin::{self, Network},
 };
 use liana_ui::component::form;
+use std::collections::BTreeMap;
 use std::fmt;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
@@ -120,6 +121,143 @@ pub fn bitcoind_network_dir(network: &Network) -> Option<String> {
     Some(dir.to_string())
 }
 
+/// Represents section for a single network in `bitcoin.conf` file.
+#[derive(PartialEq, Eq, Debug, Clone)]
+pub struct InternalBitcoindNetworkConfig {
+    pub rpc_port: u16,
+    pub p2p_port: u16,
+    pub prune: u32,
+}
+
+/// Represents the `bitcoin.conf` file to be used by internal bitcoind.
+#[derive(Debug, Clone)]
+pub struct InternalBitcoindConfig {
+    pub networks: BTreeMap<Network, InternalBitcoindNetworkConfig>,
+}
+
+#[derive(PartialEq, Eq, Debug, Clone)]
+pub enum InternalBitcoindConfigError {
+    KeyNotFound(String),
+    CouldNotParseValue(String),
+    UnexpectedSection(String),
+    TooManyElements(String),
+    FileNotFound,
+    ReadingFile(String),
+    WritingFile(String),
+    Unexpected(String),
+}
+
+impl std::fmt::Display for InternalBitcoindConfigError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match self {
+            Self::KeyNotFound(e) => write!(f, "Config file does not contain expected key: {}", e),
+            Self::CouldNotParseValue(e) => write!(f, "Value could not be parsed: {}", e),
+            Self::UnexpectedSection(e) => write!(f, "Unexpected section in file: {}", e),
+            Self::TooManyElements(section) => {
+                write!(f, "Section in file contains too many elements: {}", section)
+            }
+            Self::FileNotFound => write!(f, "File not found"),
+            Self::ReadingFile(e) => write!(f, "Error while reading file: {}", e),
+            Self::WritingFile(e) => write!(f, "Error while writing file: {}", e),
+            Self::Unexpected(e) => write!(f, "Unexpected error: {}", e),
+        }
+    }
+}
+
+impl Default for InternalBitcoindConfig {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl InternalBitcoindConfig {
+    pub fn new() -> Self {
+        Self {
+            networks: BTreeMap::new(),
+        }
+    }
+
+    pub fn from_ini(ini: &ini::Ini) -> Result<Self, InternalBitcoindConfigError> {
+        let mut networks = BTreeMap::new();
+        for (maybe_sec, prop) in ini {
+            if let Some(sec) = maybe_sec {
+                let network = Network::from_core_arg(sec)
+                    .map_err(|e| InternalBitcoindConfigError::UnexpectedSection(e.to_string()))?;
+                if prop.len() > 3 {
+                    return Err(InternalBitcoindConfigError::TooManyElements(
+                        sec.to_string(),
+                    ));
+                }
+                let rpc_port = prop
+                    .get("rpcport")
+                    .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("rpcport".to_string()))?
+                    .parse::<u16>()
+                    .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
+                let p2p_port = prop
+                    .get("port")
+                    .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("port".to_string()))?
+                    .parse::<u16>()
+                    .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
+                let prune = prop
+                    .get("prune")
+                    .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("prune".to_string()))?
+                    .parse::<u32>()
+                    .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
+                networks.insert(
+                    network,
+                    InternalBitcoindNetworkConfig {
+                        rpc_port,
+                        p2p_port,
+                        prune,
+                    },
+                );
+            } else if !prop.is_empty() {
+                return Err(InternalBitcoindConfigError::UnexpectedSection(
+                    "General section should be empty".to_string(),
+                ));
+            }
+        }
+        Ok(Self { networks })
+    }
+
+    pub fn from_file(path: &PathBuf) -> Result<Self, InternalBitcoindConfigError> {
+        if !path.exists() {
+            return Err(InternalBitcoindConfigError::FileNotFound);
+        }
+        let conf_ini = ini::Ini::load_from_file(path)
+            .map_err(|e| InternalBitcoindConfigError::ReadingFile(e.to_string()))?;
+
+        Self::from_ini(&conf_ini)
+    }
+
+    pub fn to_ini(&self) -> ini::Ini {
+        let mut conf_ini = ini::Ini::new();
+
+        for (network, network_conf) in &self.networks {
+            conf_ini
+                .with_section(Some(network.to_core_arg()))
+                .set("rpcport", network_conf.rpc_port.to_string())
+                .set("port", network_conf.p2p_port.to_string())
+                .set("prune", network_conf.prune.to_string());
+        }
+        conf_ini
+    }
+
+    pub fn to_file(&self, path: &PathBuf) -> Result<(), InternalBitcoindConfigError> {
+        std::fs::create_dir_all(
+            path.parent()
+                .ok_or_else(|| InternalBitcoindConfigError::Unexpected("No parent".to_string()))?,
+        )
+        .map_err(|e| InternalBitcoindConfigError::Unexpected(e.to_string()))?;
+        info!("Writing to file {}", path.to_string_lossy());
+        self.to_ini()
+            .write_to_file(path)
+            .map_err(|e| InternalBitcoindConfigError::WritingFile(e.to_string()))?;
+
+        Ok(())
+    }
+}
+
 /// Possible errors when starting bitcoind.
 #[derive(PartialEq, Eq, Debug, Clone)]
 pub enum StartInternalBitcoindError {
@@ -318,3 +456,74 @@ impl fmt::Display for ConfigField {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use ini::Ini;
+    use liana::miniscript::bitcoin::Network;
+
+    // Test the format of the internal bitcoind configuration file.
+    #[test]
+    fn internal_bitcoind_config() {
+        // A valid config
+        let mut conf_ini = Ini::new();
+        conf_ini
+            .with_section(Some("main"))
+            .set("rpcport", "43345")
+            .set("port", "42355")
+            .set("prune", "15246");
+        conf_ini
+            .with_section(Some("regtest"))
+            .set("rpcport", "34067")
+            .set("port", "45175")
+            .set("prune", "2043");
+        let conf = InternalBitcoindConfig::from_ini(&conf_ini).expect("Loading conf from ini");
+        let main_conf = InternalBitcoindNetworkConfig {
+            rpc_port: 43345,
+            p2p_port: 42355,
+            prune: 15246,
+        };
+        let regtest_conf = InternalBitcoindNetworkConfig {
+            rpc_port: 34067,
+            p2p_port: 45175,
+            prune: 2043,
+        };
+        assert_eq!(conf.networks.len(), 2);
+        assert_eq!(
+            conf.networks.get(&Network::Bitcoin).expect("Missing main"),
+            &main_conf
+        );
+        assert_eq!(
+            conf.networks
+                .get(&Network::Regtest)
+                .expect("Missing regtest"),
+            &regtest_conf
+        );
+
+        let mut conf = InternalBitcoindConfig::new();
+        conf.networks.insert(Network::Bitcoin, main_conf);
+        conf.networks.insert(Network::Regtest, regtest_conf);
+        for (sec, prop) in &conf.to_ini() {
+            if let Some(sec) = sec {
+                assert_eq!(prop.len(), 3);
+                let rpc_port = prop.get("rpcport").expect("rpcport");
+                let p2p_port = prop.get("port").expect("port");
+                let prune = prop.get("prune").expect("prune");
+                if sec == "main" {
+                    assert_eq!(rpc_port, "43345");
+                    assert_eq!(p2p_port, "42355");
+                    assert_eq!(prune, "15246");
+                } else if sec == "regtest" {
+                    assert_eq!(rpc_port, "34067");
+                    assert_eq!(p2p_port, "45175");
+                    assert_eq!(prune, "2043");
+                } else {
+                    panic!("Unexpected section");
+                }
+            } else {
+                assert!(prop.is_empty())
+            }
+        }
+    }
+}
diff --git a/gui/src/installer/context.rs b/gui/src/installer/context.rs
index 7acf49d3..6413626c 100644
--- a/gui/src/installer/context.rs
+++ b/gui/src/installer/context.rs
@@ -7,7 +7,7 @@ use crate::{
         settings::{KeySetting, Settings, WalletSetting},
         wallet::wallet_name,
     },
-    bitcoind::Bitcoind,
+    bitcoind::{Bitcoind, InternalBitcoindConfig},
     hw::HardwareWalletConfig,
     signer::Signer,
 };
@@ -19,8 +19,6 @@ use liana::{
     miniscript::bitcoin,
 };
 
-use super::step::InternalBitcoindConfig;
-
 #[derive(Clone)]
 pub struct Context {
     pub bitcoin_config: BitcoinConfig,
diff --git a/gui/src/installer/step/bitcoind.rs b/gui/src/installer/step/bitcoind.rs
index 25b70b50..00ec02ee 100644
--- a/gui/src/installer/step/bitcoind.rs
+++ b/gui/src/installer/step/bitcoind.rs
@@ -1,4 +1,3 @@
-use std::collections::BTreeMap;
 #[cfg(target_os = "windows")]
 use std::io::{self, Cursor};
 use std::net::{IpAddr, Ipv4Addr, SocketAddr, TcpListener};
@@ -24,7 +23,9 @@ use liana_ui::{component::form, widget::*};
 use crate::{
     bitcoind::{
         self, bitcoind_network_dir, internal_bitcoind_datadir, internal_bitcoind_directory,
-        Bitcoind, ConfigField, RpcAuthType, RpcAuthValues, StartInternalBitcoindError, VERSION,
+        Bitcoind, ConfigField, InternalBitcoindConfig, InternalBitcoindConfigError,
+        InternalBitcoindNetworkConfig, RpcAuthType, RpcAuthValues, StartInternalBitcoindError,
+        VERSION,
     },
     download,
     hw::HardwareWallets,
@@ -108,143 +109,6 @@ pub const PRUNE_DEFAULT: u32 = 15_000;
 /// Default ports used by bitcoind across all networks.
 pub const BITCOIND_DEFAULT_PORTS: [u16; 8] = [8332, 8333, 18332, 18333, 18443, 18444, 38332, 38333];
 
-/// Represents section for a single network in `bitcoin.conf` file.
-#[derive(PartialEq, Eq, Debug, Clone)]
-pub struct InternalBitcoindNetworkConfig {
-    rpc_port: u16,
-    p2p_port: u16,
-    prune: u32,
-}
-
-/// Represents the `bitcoin.conf` file to be used by internal bitcoind.
-#[derive(Debug, Clone)]
-pub struct InternalBitcoindConfig {
-    networks: BTreeMap<Network, InternalBitcoindNetworkConfig>,
-}
-
-#[derive(PartialEq, Eq, Debug, Clone)]
-pub enum InternalBitcoindConfigError {
-    KeyNotFound(String),
-    CouldNotParseValue(String),
-    UnexpectedSection(String),
-    TooManyElements(String),
-    FileNotFound,
-    ReadingFile(String),
-    WritingFile(String),
-    Unexpected(String),
-}
-
-impl std::fmt::Display for InternalBitcoindConfigError {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        match self {
-            Self::KeyNotFound(e) => write!(f, "Config file does not contain expected key: {}", e),
-            Self::CouldNotParseValue(e) => write!(f, "Value could not be parsed: {}", e),
-            Self::UnexpectedSection(e) => write!(f, "Unexpected section in file: {}", e),
-            Self::TooManyElements(section) => {
-                write!(f, "Section in file contains too many elements: {}", section)
-            }
-            Self::FileNotFound => write!(f, "File not found"),
-            Self::ReadingFile(e) => write!(f, "Error while reading file: {}", e),
-            Self::WritingFile(e) => write!(f, "Error while writing file: {}", e),
-            Self::Unexpected(e) => write!(f, "Unexpected error: {}", e),
-        }
-    }
-}
-
-impl Default for InternalBitcoindConfig {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-impl InternalBitcoindConfig {
-    pub fn new() -> Self {
-        Self {
-            networks: BTreeMap::new(),
-        }
-    }
-
-    pub fn from_ini(ini: &ini::Ini) -> Result<Self, InternalBitcoindConfigError> {
-        let mut networks = BTreeMap::new();
-        for (maybe_sec, prop) in ini {
-            if let Some(sec) = maybe_sec {
-                let network = Network::from_core_arg(sec)
-                    .map_err(|e| InternalBitcoindConfigError::UnexpectedSection(e.to_string()))?;
-                if prop.len() > 3 {
-                    return Err(InternalBitcoindConfigError::TooManyElements(
-                        sec.to_string(),
-                    ));
-                }
-                let rpc_port = prop
-                    .get("rpcport")
-                    .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("rpcport".to_string()))?
-                    .parse::<u16>()
-                    .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
-                let p2p_port = prop
-                    .get("port")
-                    .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("port".to_string()))?
-                    .parse::<u16>()
-                    .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
-                let prune = prop
-                    .get("prune")
-                    .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("prune".to_string()))?
-                    .parse::<u32>()
-                    .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
-                networks.insert(
-                    network,
-                    InternalBitcoindNetworkConfig {
-                        rpc_port,
-                        p2p_port,
-                        prune,
-                    },
-                );
-            } else if !prop.is_empty() {
-                return Err(InternalBitcoindConfigError::UnexpectedSection(
-                    "General section should be empty".to_string(),
-                ));
-            }
-        }
-        Ok(Self { networks })
-    }
-
-    pub fn from_file(path: &PathBuf) -> Result<Self, InternalBitcoindConfigError> {
-        if !path.exists() {
-            return Err(InternalBitcoindConfigError::FileNotFound);
-        }
-        let conf_ini = ini::Ini::load_from_file(path)
-            .map_err(|e| InternalBitcoindConfigError::ReadingFile(e.to_string()))?;
-
-        Self::from_ini(&conf_ini)
-    }
-
-    pub fn to_ini(&self) -> ini::Ini {
-        let mut conf_ini = ini::Ini::new();
-
-        for (network, network_conf) in &self.networks {
-            conf_ini
-                .with_section(Some(network.to_core_arg()))
-                .set("rpcport", network_conf.rpc_port.to_string())
-                .set("port", network_conf.p2p_port.to_string())
-                .set("prune", network_conf.prune.to_string());
-        }
-        conf_ini
-    }
-
-    pub fn to_file(&self, path: &PathBuf) -> Result<(), InternalBitcoindConfigError> {
-        std::fs::create_dir_all(
-            path.parent()
-                .ok_or_else(|| InternalBitcoindConfigError::Unexpected("No parent".to_string()))?,
-        )
-        .map_err(|e| InternalBitcoindConfigError::Unexpected(e.to_string()))?;
-        info!("Writing to file {}", path.to_string_lossy());
-        self.to_ini()
-            .write_to_file(path)
-            .map_err(|e| InternalBitcoindConfigError::WritingFile(e.to_string()))?;
-
-        Ok(())
-    }
-}
-
 #[derive(Debug)]
 pub enum InstallState {
     InProgress,
@@ -941,75 +805,7 @@ impl Step for InternalBitcoindStep {
 
 #[cfg(test)]
 mod tests {
-    use crate::installer::step::bitcoind::{
-        verify_hash, InternalBitcoindConfig, InternalBitcoindNetworkConfig,
-    };
-    use ini::Ini;
-    use liana::miniscript::bitcoin::Network;
-
-    // Test the format of the internal bitcoind configuration file.
-    #[test]
-    fn internal_bitcoind_config() {
-        // A valid config
-        let mut conf_ini = Ini::new();
-        conf_ini
-            .with_section(Some("main"))
-            .set("rpcport", "43345")
-            .set("port", "42355")
-            .set("prune", "15246");
-        conf_ini
-            .with_section(Some("regtest"))
-            .set("rpcport", "34067")
-            .set("port", "45175")
-            .set("prune", "2043");
-        let conf = InternalBitcoindConfig::from_ini(&conf_ini).expect("Loading conf from ini");
-        let main_conf = InternalBitcoindNetworkConfig {
-            rpc_port: 43345,
-            p2p_port: 42355,
-            prune: 15246,
-        };
-        let regtest_conf = InternalBitcoindNetworkConfig {
-            rpc_port: 34067,
-            p2p_port: 45175,
-            prune: 2043,
-        };
-        assert_eq!(conf.networks.len(), 2);
-        assert_eq!(
-            conf.networks.get(&Network::Bitcoin).expect("Missing main"),
-            &main_conf
-        );
-        assert_eq!(
-            conf.networks
-                .get(&Network::Regtest)
-                .expect("Missing regtest"),
-            &regtest_conf
-        );
-
-        let mut conf = InternalBitcoindConfig::new();
-        conf.networks.insert(Network::Bitcoin, main_conf);
-        conf.networks.insert(Network::Regtest, regtest_conf);
-        for (sec, prop) in &conf.to_ini() {
-            if let Some(sec) = sec {
-                assert_eq!(prop.len(), 3);
-                let rpc_port = prop.get("rpcport").expect("rpcport");
-                let p2p_port = prop.get("port").expect("port");
-                let prune = prop.get("prune").expect("prune");
-                if sec == "main" {
-                    assert_eq!(rpc_port, "43345");
-                    assert_eq!(p2p_port, "42355");
-                    assert_eq!(prune, "15246");
-                } else if sec == "regtest" {
-                    assert_eq!(rpc_port, "34067");
-                    assert_eq!(p2p_port, "45175");
-                    assert_eq!(prune, "2043");
-                } else {
-                    panic!("Unexpected section");
-                }
-            } else {
-                assert!(prop.is_empty())
-            }
-        }
-    }
+    use super::verify_hash;
 
     #[test]
     fn hash() {
diff --git a/gui/src/installer/step/mod.rs b/gui/src/installer/step/mod.rs
index 3583dbca..15529629 100644
--- a/gui/src/installer/step/mod.rs
+++ b/gui/src/installer/step/mod.rs
@@ -3,8 +3,7 @@ mod descriptor;
 mod mnemonic;
 
 pub use bitcoind::{
-    DefineBitcoind, DownloadState, InstallState, InternalBitcoindConfig, InternalBitcoindStep,
-    SelectBitcoindTypeStep,
+    DefineBitcoind, DownloadState, InstallState, InternalBitcoindStep, SelectBitcoindTypeStep,
 };
 
 pub use descriptor::{

From c3aad0f40c94ae86d1b14ff1b05178c0c043f732 Mon Sep 17 00:00:00 2001
From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com>
Date: Mon, 26 Feb 2024 15:38:43 +0000
Subject: [PATCH 2/6] gui: add optional rpcauth to internal bitcoind config

With the updated liana dependency, we also need to pass `None` to
`list_spend_txs`.
---
 gui/Cargo.lock                     |   3 +-
 gui/Cargo.toml                     |   1 +
 gui/src/bitcoind.rs                | 112 ++++++++++++++++++++++++++++-
 gui/src/daemon/embedded.rs         |   4 +-
 gui/src/installer/step/bitcoind.rs |   1 +
 5 files changed, 116 insertions(+), 5 deletions(-)

diff --git a/gui/Cargo.lock b/gui/Cargo.lock
index e33c7e61..ed047fa3 100644
--- a/gui/Cargo.lock
+++ b/gui/Cargo.lock
@@ -2668,7 +2668,7 @@ dependencies = [
 [[package]]
 name = "liana"
 version = "4.0.0"
-source = "git+https://github.com/wizardsardine/liana?branch=master#5a56fdb108351d0a6877a11a1dec7a27a3b0928f"
+source = "git+https://github.com/wizardsardine/liana?branch=master#16afa3e9925cd016db03dbe954403bfa348b89e7"
 dependencies = [
  "backtrace",
  "bdk_coin_select",
@@ -2692,6 +2692,7 @@ version = "4.0.0"
 dependencies = [
  "async-hwi",
  "backtrace",
+ "base64 0.21.6",
  "bitcoin_hashes 0.12.0",
  "chrono",
  "dirs 3.0.2",
diff --git a/gui/Cargo.toml b/gui/Cargo.toml
index 09d471c1..7278e79d 100644
--- a/gui/Cargo.toml
+++ b/gui/Cargo.toml
@@ -42,6 +42,7 @@ toml = "0.5"
 chrono = "0.4"
 
 # Used for managing internal bitcoind
+base64 = "0.21"
 bitcoin_hashes = "0.12"
 reqwest = { version = "0.11", default-features=false, features = ["rustls-tls"] }
 rust-ini = "0.19.0"
diff --git a/gui/src/bitcoind.rs b/gui/src/bitcoind.rs
index 83653dba..db165949 100644
--- a/gui/src/bitcoind.rs
+++ b/gui/src/bitcoind.rs
@@ -1,6 +1,9 @@
+use base64::Engine;
+use bitcoin_hashes::{sha256, Hash, HashEngine, Hmac, HmacEngine};
 use liana::{
     config::{BitcoindConfig, BitcoindRpcAuth},
     miniscript::bitcoin::{self, Network},
+    random::{random_bytes, RandomnessError},
 };
 use liana_ui::component::form;
 use std::collections::BTreeMap;
@@ -121,12 +124,89 @@ pub fn bitcoind_network_dir(network: &Network) -> Option<String> {
     Some(dir.to_string())
 }
 
+#[derive(PartialEq, Eq, Debug, Clone)]
+pub enum RpcAuthParseError {
+    MissingColon,
+    MissingDollarSign,
+}
+
+impl std::fmt::Display for RpcAuthParseError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match self {
+            Self::MissingColon => write!(
+                f,
+                "RPC auth string should contain colon between user and salt."
+            ),
+            Self::MissingDollarSign => write!(
+                f,
+                "RPC auth string should contain dollar sign between salt and password HMAC."
+            ),
+        }
+    }
+}
+
+/// Represents RPC auth credentials as stored in bitcoin.conf.
+#[derive(PartialEq, Eq, Debug, Clone)]
+pub struct RpcAuth {
+    pub user: String,
+    salt: String,
+    password_hmac: String,
+}
+
+impl RpcAuth {
+    /// Returns a new `RpcAuth` object for the given `user` with a random salt and password.
+    /// This random password is also returned.
+    pub fn new(user: &str) -> Result<(Self, String), RandomnessError> {
+        // RPC auth generation follows approach in
+        // https://github.com/bitcoin/bitcoin/blob/master/share/rpcauth/rpcauth.py
+        let password =
+            random_bytes().map(|bytes| base64::prelude::BASE64_URL_SAFE_NO_PAD.encode(bytes))?;
+        // As per the Python script, only use 16 bytes for the salt.
+        let salt = random_bytes().map(|bytes| hex::encode(&bytes[..16]))?;
+        let mut engine = HmacEngine::<sha256::Hash>::new(salt.as_bytes());
+        engine.input(password.as_bytes());
+        let password_hmac = Hmac::<sha256::Hash>::from_engine(engine);
+
+        Ok((
+            Self {
+                user: user.to_string(),
+                salt,
+                password_hmac: hex::encode(&password_hmac[..]),
+            },
+            password,
+        ))
+    }
+}
+
+impl std::fmt::Display for RpcAuth {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "{}:{}${}", self.user, self.salt, self.password_hmac)
+    }
+}
+
+impl std::str::FromStr for RpcAuth {
+    type Err = RpcAuthParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let (user, salt_pw) = s.split_once(':').ok_or(RpcAuthParseError::MissingColon)?;
+        let (salt, pw) = salt_pw
+            .split_once('$')
+            .ok_or(RpcAuthParseError::MissingDollarSign)?;
+        Ok(Self {
+            user: user.to_string(),
+            salt: salt.to_string(),
+            password_hmac: pw.to_string(),
+        })
+    }
+}
+
 /// Represents section for a single network in `bitcoin.conf` file.
 #[derive(PartialEq, Eq, Debug, Clone)]
 pub struct InternalBitcoindNetworkConfig {
     pub rpc_port: u16,
     pub p2p_port: u16,
     pub prune: u32,
+    pub rpc_auth: Option<RpcAuth>,
 }
 
 /// Represents the `bitcoin.conf` file to be used by internal bitcoind.
@@ -183,7 +263,7 @@ impl InternalBitcoindConfig {
             if let Some(sec) = maybe_sec {
                 let network = Network::from_core_arg(sec)
                     .map_err(|e| InternalBitcoindConfigError::UnexpectedSection(e.to_string()))?;
-                if prop.len() > 3 {
+                if prop.len() > 4 {
                     return Err(InternalBitcoindConfigError::TooManyElements(
                         sec.to_string(),
                     ));
@@ -203,12 +283,22 @@ impl InternalBitcoindConfig {
                     .ok_or_else(|| InternalBitcoindConfigError::KeyNotFound("prune".to_string()))?
                     .parse::<u32>()
                     .map_err(|e| InternalBitcoindConfigError::CouldNotParseValue(e.to_string()))?;
+                let rpc_auth = prop
+                    .get("rpcauth")
+                    .map(|v| {
+                        v.parse::<RpcAuth>().map_err(|e| {
+                            InternalBitcoindConfigError::CouldNotParseValue(e.to_string())
+                        })
+                    })
+                    .transpose()?;
+
                 networks.insert(
                     network,
                     InternalBitcoindNetworkConfig {
                         rpc_port,
                         p2p_port,
                         prune,
+                        rpc_auth,
                     },
                 );
             } else if !prop.is_empty() {
@@ -239,6 +329,11 @@ impl InternalBitcoindConfig {
                 .set("rpcport", network_conf.rpc_port.to_string())
                 .set("port", network_conf.p2p_port.to_string())
                 .set("prune", network_conf.prune.to_string());
+            if let Some(rpc_auth) = network_conf.rpc_auth.as_ref() {
+                conf_ini
+                    .with_section(Some(network.to_core_arg()))
+                    .set("rpcauth", rpc_auth.to_string());
+            }
         }
         conf_ini
     }
@@ -477,17 +572,24 @@ mod tests {
             .with_section(Some("regtest"))
             .set("rpcport", "34067")
             .set("port", "45175")
-            .set("prune", "2043");
+            .set("prune", "2043")
+            .set("rpcauth", "my_user:my_salt$my_pw_hmac");
         let conf = InternalBitcoindConfig::from_ini(&conf_ini).expect("Loading conf from ini");
         let main_conf = InternalBitcoindNetworkConfig {
             rpc_port: 43345,
             p2p_port: 42355,
             prune: 15246,
+            rpc_auth: None,
         };
         let regtest_conf = InternalBitcoindNetworkConfig {
             rpc_port: 34067,
             p2p_port: 45175,
             prune: 2043,
+            rpc_auth: Some(RpcAuth {
+                user: "my_user".to_string(),
+                salt: "my_salt".to_string(),
+                password_hmac: "my_pw_hmac".to_string(),
+            }),
         };
         assert_eq!(conf.networks.len(), 2);
         assert_eq!(
@@ -506,18 +608,22 @@ mod tests {
         conf.networks.insert(Network::Regtest, regtest_conf);
         for (sec, prop) in &conf.to_ini() {
             if let Some(sec) = sec {
-                assert_eq!(prop.len(), 3);
                 let rpc_port = prop.get("rpcport").expect("rpcport");
                 let p2p_port = prop.get("port").expect("port");
                 let prune = prop.get("prune").expect("prune");
+                let rpc_auth = prop.get("rpcauth");
                 if sec == "main" {
+                    assert_eq!(prop.len(), 3);
                     assert_eq!(rpc_port, "43345");
                     assert_eq!(p2p_port, "42355");
                     assert_eq!(prune, "15246");
+                    assert!(rpc_auth.is_none());
                 } else if sec == "regtest" {
+                    assert_eq!(prop.len(), 4);
                     assert_eq!(rpc_port, "34067");
                     assert_eq!(p2p_port, "45175");
                     assert_eq!(prune, "2043");
+                    assert_eq!(rpc_auth, Some("my_user:my_salt$my_pw_hmac"));
                 } else {
                     panic!("Unexpected section");
                 }
diff --git a/gui/src/daemon/embedded.rs b/gui/src/daemon/embedded.rs
index 2978c181..5091bb28 100644
--- a/gui/src/daemon/embedded.rs
+++ b/gui/src/daemon/embedded.rs
@@ -64,7 +64,9 @@ impl Daemon for EmbeddedDaemon {
     }
 
     fn list_spend_txs(&self) -> Result<ListSpendResult, DaemonError> {
-        Ok(self.control()?.list_spend())
+        self.control()?
+            .list_spend(None)
+            .map_err(|e| DaemonError::Unexpected(e.to_string()))
     }
 
     fn list_confirmed_txs(
diff --git a/gui/src/installer/step/bitcoind.rs b/gui/src/installer/step/bitcoind.rs
index 00ec02ee..2f1ad1f2 100644
--- a/gui/src/installer/step/bitcoind.rs
+++ b/gui/src/installer/step/bitcoind.rs
@@ -614,6 +614,7 @@ impl Step for InternalBitcoindStep {
                                     rpc_port,
                                     p2p_port,
                                     prune: PRUNE_DEFAULT,
+                                    rpc_auth: None,
                                 }
                             }
                             (Ok(_), Err(e)) | (Err(e), Ok(_)) => {

From b5980fbc4f6be006b083fbc8c7e4142e50598fb8 Mon Sep 17 00:00:00 2001
From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com>
Date: Tue, 27 Feb 2024 08:45:37 +0000
Subject: [PATCH 3/6] gui: add sections check to internal bitcoind config test

---
 gui/src/bitcoind.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gui/src/bitcoind.rs b/gui/src/bitcoind.rs
index db165949..eb486d28 100644
--- a/gui/src/bitcoind.rs
+++ b/gui/src/bitcoind.rs
@@ -606,7 +606,10 @@ mod tests {
         let mut conf = InternalBitcoindConfig::new();
         conf.networks.insert(Network::Bitcoin, main_conf);
         conf.networks.insert(Network::Regtest, regtest_conf);
-        for (sec, prop) in &conf.to_ini() {
+        conf_ini = conf.to_ini();
+        assert_eq!(conf_ini.len(), 3); // 2 network sections plus the empty general section
+        assert!(conf_ini.general_section().is_empty());
+        for (sec, prop) in &conf_ini {
             if let Some(sec) = sec {
                 let rpc_port = prop.get("rpcport").expect("rpcport");
                 let p2p_port = prop.get("port").expect("port");

From 7584b6347bf6525d57d09596a60b4bdd2bccfb7f Mon Sep 17 00:00:00 2001
From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com>
Date: Wed, 28 Feb 2024 12:22:15 +0000
Subject: [PATCH 4/6] gui: start internal bitcoind with given config

This will allow bitcoind to start also in case of using rpcauth.

The cookie file is canonicalized in the installer beforehand
instead.

There is now no need for the config parameter to be mutable.
---
 gui/src/bitcoind.rs                |  8 ++------
 gui/src/installer/step/bitcoind.rs | 29 ++++++++++++++++++-----------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/gui/src/bitcoind.rs b/gui/src/bitcoind.rs
index eb486d28..c39c9fb2 100644
--- a/gui/src/bitcoind.rs
+++ b/gui/src/bitcoind.rs
@@ -1,7 +1,7 @@
 use base64::Engine;
 use bitcoin_hashes::{sha256, Hash, HashEngine, Hmac, HmacEngine};
 use liana::{
-    config::{BitcoindConfig, BitcoindRpcAuth},
+    config::BitcoindConfig,
     miniscript::bitcoin::{self, Network},
     random::{random_bytes, RandomnessError},
 };
@@ -402,7 +402,7 @@ impl Bitcoind {
     /// Start internal bitcoind for the given network.
     pub fn start(
         network: &bitcoin::Network,
-        mut config: BitcoindConfig,
+        config: BitcoindConfig,
         liana_datadir: &PathBuf,
     ) -> Result<Self, StartInternalBitcoindError> {
         let bitcoind_datadir = internal_bitcoind_datadir(liana_datadir);
@@ -477,10 +477,6 @@ impl Bitcoind {
             thread::sleep(time::Duration::from_millis(500));
         }
 
-        config.rpc_auth = BitcoindRpcAuth::CookieFile(cookie_path.canonicalize().map_err(|e| {
-            StartInternalBitcoindError::CouldNotCanonicalizeCookiePath(e.to_string())
-        })?);
-
         liana::BitcoinD::new(&config, "internal_bitcoind_start".to_string())
             .map_err(|e| StartInternalBitcoindError::BitcoinDError(e.to_string()))?;
 
diff --git a/gui/src/installer/step/bitcoind.rs b/gui/src/installer/step/bitcoind.rs
index 2f1ad1f2..1a8874c4 100644
--- a/gui/src/installer/step/bitcoind.rs
+++ b/gui/src/installer/step/bitcoind.rs
@@ -693,17 +693,24 @@ impl Step for InternalBitcoindStep {
                     }
                 }
                 message::InternalBitcoindMsg::Start => {
-                    if let Err(e) = self.bitcoind_datadir.canonicalize() {
-                        self.started = Some(Err(
-                            StartInternalBitcoindError::CouldNotCanonicalizeDataDir(e.to_string()),
-                        ));
-                        return Command::none();
-                    }
-
-                    let cookie_path = bitcoind::internal_bitcoind_cookie_path(
-                        &self.bitcoind_datadir,
-                        &self.network,
-                    );
+                    let bitcoind_datadir = match self.bitcoind_datadir.canonicalize() {
+                        Ok(path) => path,
+                        Err(e) => {
+                            self.started = Some(Err(
+                                StartInternalBitcoindError::CouldNotCanonicalizeDataDir(
+                                    e.to_string(),
+                                ),
+                            ));
+                            return Command::none();
+                        }
+                    };
+                    // Pass the canonicalized `bitcoind_datadir` so that the cookie path returned
+                    // by `internal_bitcoind_cookie_path` is also canonicalized. This way, the
+                    // canonicalized path will later be saved to the config file.
+                    // We cannot use `cookie_path.canonicalize()` as we have not yet started
+                    // bitcoind and so the path does not exist.
+                    let cookie_path =
+                        bitcoind::internal_bitcoind_cookie_path(&bitcoind_datadir, &self.network);
 
                     let rpc_port = self
                         .internal_bitcoind_config

From 5e5c3330ee14159b14e43852d8db1d51a1b6bad8 Mon Sep 17 00:00:00 2001
From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com>
Date: Tue, 5 Mar 2024 11:58:15 +0000
Subject: [PATCH 5/6] gui: use rpcauth in installer for internal bitcoind

---
 gui/src/installer/step/bitcoind.rs | 93 ++++++++++++++----------------
 1 file changed, 42 insertions(+), 51 deletions(-)

diff --git a/gui/src/installer/step/bitcoind.rs b/gui/src/installer/step/bitcoind.rs
index 1a8874c4..60b24853 100644
--- a/gui/src/installer/step/bitcoind.rs
+++ b/gui/src/installer/step/bitcoind.rs
@@ -24,8 +24,8 @@ use crate::{
     bitcoind::{
         self, bitcoind_network_dir, internal_bitcoind_datadir, internal_bitcoind_directory,
         Bitcoind, ConfigField, InternalBitcoindConfig, InternalBitcoindConfigError,
-        InternalBitcoindNetworkConfig, RpcAuthType, RpcAuthValues, StartInternalBitcoindError,
-        VERSION,
+        InternalBitcoindNetworkConfig, RpcAuth, RpcAuthType, RpcAuthValues,
+        StartInternalBitcoindError, VERSION,
     },
     download,
     hw::HardwareWallets,
@@ -598,9 +598,12 @@ impl Step for InternalBitcoindStep {
                             return Command::none();
                         }
                     };
-                    // Insert entry for network if not present.
-                    if conf.networks.get(&self.network).is_none() {
-                        let network_conf = match (get_available_port(), get_available_port()) {
+                    let (rpc_port, p2p_port) = if let Some(network_conf) =
+                        conf.networks.get(&self.network)
+                    {
+                        (network_conf.rpc_port, network_conf.p2p_port)
+                    } else {
+                        match (get_available_port(), get_available_port()) {
                             (Ok(rpc_port), Ok(p2p_port)) => {
                                 // In case ports are the same, user will need to click button again for another attempt.
                                 if rpc_port == p2p_port {
@@ -610,12 +613,7 @@ impl Step for InternalBitcoindStep {
                                     );
                                     return Command::none();
                                 }
-                                InternalBitcoindNetworkConfig {
-                                    rpc_port,
-                                    p2p_port,
-                                    prune: PRUNE_DEFAULT,
-                                    rpc_auth: None,
-                                }
+                                (rpc_port, p2p_port)
                             }
                             (Ok(_), Err(e)) | (Err(e), Ok(_)) => {
                                 self.error = Some(format!("Could not get available port: {}.", e));
@@ -626,9 +624,27 @@ impl Step for InternalBitcoindStep {
                                     Some(format!("Could not get available ports: {}; {}.", e1, e2));
                                 return Command::none();
                             }
-                        };
-                        conf.networks.insert(self.network, network_conf);
-                    }
+                        }
+                    };
+                    let (rpc_auth, rpc_password) = match RpcAuth::new("liana") {
+                        Ok((rpc_auth, password)) => (rpc_auth, password),
+                        Err(e) => {
+                            self.error = Some(e.to_string());
+                            return Command::none();
+                        }
+                    };
+                    let bitcoind_config = BitcoindConfig {
+                        rpc_auth: BitcoindRpcAuth::UserPass(rpc_auth.user.clone(), rpc_password),
+                        addr: internal_bitcoind_address(rpc_port),
+                    };
+                    let network_conf = InternalBitcoindNetworkConfig {
+                        rpc_port,
+                        p2p_port,
+                        prune: PRUNE_DEFAULT,
+                        // Overwrite any previous entry for this network as we would no longer know the RPC password.
+                        rpc_auth: Some(rpc_auth),
+                    };
+                    conf.networks.insert(self.network, network_conf);
                     if let Err(e) = conf.to_file(&bitcoind::internal_bitcoind_config_path(
                         &self.bitcoind_datadir,
                     )) {
@@ -636,7 +652,8 @@ impl Step for InternalBitcoindStep {
                         return Command::none();
                     };
                     self.error = None;
-                    self.internal_bitcoind_config = Some(conf.clone());
+                    self.internal_bitcoind_config = Some(conf);
+                    self.bitcoind_config = Some(bitcoind_config);
                     return Command::perform(async {}, |_| {
                         Message::InternalBitcoind(message::InternalBitcoindMsg::Reload)
                     });
@@ -693,43 +710,18 @@ impl Step for InternalBitcoindStep {
                     }
                 }
                 message::InternalBitcoindMsg::Start => {
-                    let bitcoind_datadir = match self.bitcoind_datadir.canonicalize() {
-                        Ok(path) => path,
-                        Err(e) => {
-                            self.started = Some(Err(
-                                StartInternalBitcoindError::CouldNotCanonicalizeDataDir(
-                                    e.to_string(),
-                                ),
-                            ));
-                            return Command::none();
-                        }
+                    if let Err(e) = self.bitcoind_datadir.canonicalize() {
+                        self.started = Some(Err(
+                            StartInternalBitcoindError::CouldNotCanonicalizeDataDir(e.to_string()),
+                        ));
+                        return Command::none();
                     };
-                    // Pass the canonicalized `bitcoind_datadir` so that the cookie path returned
-                    // by `internal_bitcoind_cookie_path` is also canonicalized. This way, the
-                    // canonicalized path will later be saved to the config file.
-                    // We cannot use `cookie_path.canonicalize()` as we have not yet started
-                    // bitcoind and so the path does not exist.
-                    let cookie_path =
-                        bitcoind::internal_bitcoind_cookie_path(&bitcoind_datadir, &self.network);
-
-                    let rpc_port = self
-                        .internal_bitcoind_config
+                    let bitcoind_config = self
+                        .bitcoind_config
                         .as_ref()
-                        .expect("Already added")
-                        .clone()
-                        .networks
-                        .get(&self.network)
-                        .expect("Already added")
-                        .rpc_port;
-
-                    match Bitcoind::start(
-                        &self.network,
-                        BitcoindConfig {
-                            rpc_auth: BitcoindRpcAuth::CookieFile(cookie_path),
-                            addr: internal_bitcoind_address(rpc_port),
-                        },
-                        &self.liana_datadir,
-                    ) {
+                        .expect("already added")
+                        .clone();
+                    match Bitcoind::start(&self.network, bitcoind_config, &self.liana_datadir) {
                         Err(e) => {
                             self.started =
                                 Some(Err(StartInternalBitcoindError::CommandError(e.to_string())));
@@ -737,7 +729,6 @@ impl Step for InternalBitcoindStep {
                         }
                         Ok(bitcoind) => {
                             self.error = None;
-                            self.bitcoind_config = Some(bitcoind.config.clone());
                             self.started = Some(Ok(()));
                             self.internal_bitcoind = Some(bitcoind);
                         }

From a997a7bcffac6eecbe472e383b7cb97bd2af738d Mon Sep 17 00:00:00 2001
From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com>
Date: Wed, 6 Mar 2024 17:28:03 +0000
Subject: [PATCH 6/6] gui: don't rely on cookie to check successful start

---
 gui/src/bitcoind.rs | 62 +++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/gui/src/bitcoind.rs b/gui/src/bitcoind.rs
index c39c9fb2..73499cd9 100644
--- a/gui/src/bitcoind.rs
+++ b/gui/src/bitcoind.rs
@@ -357,12 +357,10 @@ impl InternalBitcoindConfig {
 #[derive(PartialEq, Eq, Debug, Clone)]
 pub enum StartInternalBitcoindError {
     CommandError(String),
-    CouldNotCanonicalizeExePath(String),
     CouldNotCanonicalizeDataDir(String),
-    CouldNotCanonicalizeCookiePath(String),
-    CookieFileNotFound(String),
     BitcoinDError(String),
     ExecutableNotFound,
+    ProcessExited(std::process::ExitStatus),
 }
 
 impl std::fmt::Display for StartInternalBitcoindError {
@@ -371,24 +369,14 @@ impl std::fmt::Display for StartInternalBitcoindError {
             Self::CommandError(e) => {
                 write!(f, "Command to start bitcoind returned an error: {}", e)
             }
-            Self::CouldNotCanonicalizeExePath(e) => {
-                write!(f, "Failed to canonicalize executable path: {}", e)
-            }
             Self::CouldNotCanonicalizeDataDir(e) => {
                 write!(f, "Failed to canonicalize datadir: {}", e)
             }
-            Self::CouldNotCanonicalizeCookiePath(e) => {
-                write!(f, "Failed to canonicalize cookie path: {}", e)
-            }
-            Self::CookieFileNotFound(path) => {
-                write!(
-                    f,
-                    "Cookie file was not found at the expected path: {}",
-                    path
-                )
-            }
             Self::BitcoinDError(e) => write!(f, "bitcoind connection check failed: {}", e),
             Self::ExecutableNotFound => write!(f, "bitcoind executable not found."),
+            Self::ProcessExited(status) => {
+                write!(f, "bitcoind process exited with status '{}'.", status)
+            }
         }
     }
 }
@@ -455,35 +443,43 @@ impl Bitcoind {
             .map_err(|e| StartInternalBitcoindError::CommandError(e.to_string()))?;
 
         // We've started bitcoind in the background, however it may fail to start for whatever
-        // reason. And we need its JSONRPC interface to be available to continue. Thus wait for it
-        // to have created the cookie file, regularly checking it did not fail to start.
-        let cookie_path = internal_bitcoind_cookie_path(&bitcoind_datadir, network);
+        // reason. And we need its JSONRPC interface to be available to continue. Thus wait for
+        // the interface to be created successfully, regularly checking it did not fail to start.
         loop {
             match process.try_wait() {
                 Ok(None) => {}
                 Err(e) => log::error!("Error while trying to wait for bitcoind: {}", e),
                 Ok(Some(status)) => {
                     log::error!("Bitcoind exited with status '{}'", status);
-                    return Err(StartInternalBitcoindError::CookieFileNotFound(
-                        cookie_path.to_string_lossy().into_owned(),
-                    ));
+                    return Err(StartInternalBitcoindError::ProcessExited(status));
                 }
             }
-            if cookie_path.exists() {
-                log::info!("Bitcoind seems to have successfully started.");
-                break;
+            match liana::BitcoinD::new(&config, "internal_bitcoind_start".to_string()) {
+                Ok(_) => {
+                    log::info!("Bitcoind seems to have successfully started.");
+                    return Ok(Self {
+                        config,
+                        _process: Arc::new(process),
+                    });
+                }
+                Err(liana::BitcoindError::CookieFile(_)) => {
+                    // This is only raised if we're using cookie authentication.
+                    // Assume cookie file has not been created yet and try again.
+                }
+                Err(e) => {
+                    if !e.is_transient() {
+                        // Non-transient error could happen, e.g., if RPC auth credentials are wrong.
+                        // Kill process now in case it's not possible to do via RPC command later.
+                        if let Err(e) = process.kill() {
+                            log::error!("Error trying to kill bitcoind process: '{}'", e);
+                        }
+                        return Err(StartInternalBitcoindError::BitcoinDError(e.to_string()));
+                    }
+                }
             }
             log::info!("Waiting for bitcoind to start.");
             thread::sleep(time::Duration::from_millis(500));
         }
-
-        liana::BitcoinD::new(&config, "internal_bitcoind_start".to_string())
-            .map_err(|e| StartInternalBitcoindError::BitcoinDError(e.to_string()))?;
-
-        Ok(Self {
-            config,
-            _process: Arc::new(process),
-        })
     }
 
     /// Stop (internal) bitcoind.