Add Wallet Integration Tests and Fix Descriptor Persistence Issues

.github/workflows/rust.yml

@@ -27,6 +27,8 @@ jobs:
 
       - name: Spell check
         uses: crate-ci/typos@v1
+        with:
+          config: ./_typos.toml
 
       - name: Run cargo fmt
         run: cargo +nightly fmt --all --check
@@ -140,8 +142,6 @@ jobs:
         include:
           - crate: floresta-common
             feature_args: ""
-          - crate: floresta-common
-            feature_args: "--features descriptors-no-std"
 
     steps:
       - uses: actions/checkout@v4

_typos.toml

@@ -0,0 +1,9 @@
+[default]
+extend-ignore-re = [
+    # This regex is used to prevent https://github.com/crate-ci/typos from analyzing words
+    # with lengths between 32 and 150 characters. These lengths correspond to hashes,
+    # public keys, private keys, XPUBs, descriptors, and Bitcoin addresses.
+    # These items are usually randomly generated for testing and may contain patterns
+    # that would be flagged as errors but should not be.
+    "[0-9A-Za-z]{32,150}+",
+]

bin/florestad/src/cli.rs

@@ -63,14 +63,13 @@ pub struct Cli {
     pub wallet_xpub: Option<Vec<String>>,
 
     #[arg(long, value_name = "DESCRIPTOR")]
-    /// Add an output descriptor to our wallet
+    /// Add an output descriptor to our wallet.
     ///
-    /// This option can be passed many times, and will accept any valid output descriptor.
-    /// You only need to pass this once, but there's no harm in passing it more than once.
-    /// After you start florestad at least once, passing some xpub, florestad
-    /// will follow the first 100 addresses derived from this xpub on each keychain  and
-    /// cache any transactions where those addresses appear. You can use either the integrated
-    /// json-rpc or electrum server to fetch an address's history, balance and utxos.
+    /// This option can be passed multiple times, as long as each descriptor is valid.
+    /// For each valid descriptor, the node will derive the first 100 addresses and cache any
+    /// transactions related to those addresses.
+    /// You can use the integrated JSON-RPC or Electrum server to fetch the transaction history,
+    /// balance, and UTXOs for these addresses.
     pub wallet_descriptor: Option<Vec<String>>,
 
     #[arg(long, value_name = "BLOCK_HASH|0", default_value = "hardcoded", value_parser = parse_assume_valid)]

crates/floresta-common/Cargo.toml

@@ -11,16 +11,12 @@ readme.workspace = true # Floresta/README.md
 [dependencies]
 bitcoin = { workspace = true }
 hashbrown = { version = "0.16" }
-miniscript = { workspace = true, optional = true, default-features = false }
 sha2 = { workspace = true }
 spin = { workspace = true }
 
 [features]
-default = ["std", "descriptors-std"]
+default = ["std"]
 std = ["bitcoin/std", "sha2/std"]
-# Both features can be set at the same time, but for `no_std` only the latter should be used
-descriptors-std = ["miniscript/std"]
-descriptors-no-std = ["miniscript/no-std"]
 
 [lints]
 workspace = true

crates/floresta-common/src/lib.rs

@@ -18,10 +18,6 @@ use bitcoin::hashes::sha256;
 use bitcoin::hashes::Hash;
 use bitcoin::ScriptBuf;
 use bitcoin::VarInt;
-#[cfg(any(feature = "descriptors-std", feature = "descriptors-no-std"))]
-use miniscript::Descriptor;
-#[cfg(any(feature = "descriptors-std", feature = "descriptors-no-std"))]
-use miniscript::DescriptorPublicKey;
 use sha2::Digest;
 
 #[cfg(feature = "std")]
@@ -33,8 +29,6 @@ pub mod spsc;
 
 #[cfg(feature = "std")]
 pub use ema::Ema;
-#[cfg(any(feature = "descriptors-std", feature = "descriptors-no-std"))]
-use prelude::*;
 pub use spsc::Channel;
 
 /// Computes the SHA-256 digest of the byte slice data and returns a [Hash] from `bitcoin_hashes`.
@@ -86,24 +80,41 @@ pub mod service_flags {
     pub const UTREEXO_FILTER: u64 = 1 << 25;
 }
 
-#[cfg(any(feature = "descriptors-std", feature = "descriptors-no-std"))]
-/// Takes an array of descriptors as `String`, performs sanity checks on each one
-/// and returns list of parsed descriptors.
-pub fn parse_descriptors(
-    descriptors: &[String],
-) -> Result<Vec<Descriptor<DescriptorPublicKey>>, miniscript::Error> {
-    let descriptors = descriptors
-        .iter()
-        .map(|descriptor| {
-            let descriptor = Descriptor::<DescriptorPublicKey>::from_str(descriptor.as_str())?;
-            descriptor.sanity_check()?;
-            descriptor.into_single_descriptors()
-        })
-        .collect::<Result<Vec<Vec<_>>, _>>()?
-        .into_iter()
-        .flatten()
-        .collect::<Vec<_>>();
-    Ok(descriptors)
+#[derive(Debug, Clone)]
+/// A simple fraction struct that allows adding numbers to the numerator and denominator
+///
+/// If we want compute a rolling-average, we would naively hold all elements in a list and
+/// compute the average from it. This is not efficient, as it requires O(n) memory and O(n)
+/// time to compute the average. Instead, we can use a fraction to compute the average in O(1)
+/// time and O(1) memory, by keeping track of the sum of all elements and the number of elements.
+pub struct FractionAvg {
+    numerator: u64,
+    denominator: u64,
+}
+
+impl FractionAvg {
+    /// Creates a new fraction with the given numerator and denominator
+    pub fn new(numerator: u64, denominator: u64) -> Self {
+        Self {
+            numerator,
+            denominator,
+        }
+    }
+
+    /// Adds a number to the numerator and increments the denominator
+    pub fn add(&mut self, other: u64) {
+        self.numerator += other;
+        self.denominator += 1;
+    }
+
+    /// Returns the average of the fraction
+    pub fn value(&self) -> f64 {
+        if self.denominator == 0 {
+            return 0.0;
+        }
+
+        self.numerator as f64 / self.denominator as f64
+    }
 }
 
 #[cfg(not(feature = "std"))]

crates/floresta-node/src/error.rs

@@ -6,11 +6,11 @@ use floresta_chain::BlockchainError;
 use floresta_chain::FlatChainstoreError;
 #[cfg(feature = "compact-filters")]
 use floresta_compact_filters::IterableFilterStoreError;
+use floresta_watch_only::descriptor::DescriptorError;
 use floresta_watch_only::kv_database::KvDatabaseError;
 use floresta_watch_only::WatchOnlyError;
 use tokio_rustls::rustls::pki_types;
 
-use crate::slip132;
 #[derive(Debug)]
 pub enum FlorestadError {
     /// Encoding/decoding error.
@@ -41,7 +41,7 @@ pub enum FlorestadError {
     TomlParsing(toml::de::Error),
 
     /// Parsing registered HD version bytes from slip132.
-    WalletInput(slip132::Error),
+    WalletInput(DescriptorError),
 
     /// Parsing a bitcoin address.
     AddressParsing(bitcoin::address::ParseError),
@@ -98,9 +98,6 @@ pub enum FlorestadError {
     /// Failed to create the TLS data directory.
     CouldNotCreateTLSDataDir(String, std::io::Error),
 
-    /// Failed to provide a valid xpub.
-    InvalidProvidedXpub(String, slip132::Error),
-
     /// Failed to obtain the wallet cache.
     CouldNotObtainWalletCache(WatchOnlyError<KvDatabaseError>),
 
@@ -198,9 +195,6 @@ impl std::fmt::Display for FlorestadError {
             FlorestadError::CouldNotCreateTLSDataDir(path, err) => {
                 write!(f, "Could not create TLS data directory {path}: {err}")
             }
-            FlorestadError::InvalidProvidedXpub(xpub, err) => {
-                write!(f, "Invalid provided xpub {xpub}: {err:?}")
-            }
             FlorestadError::CouldNotObtainWalletCache(err) => {
                 write!(f, "Could not obtain wallet cache: {err}")
             }
@@ -248,7 +242,7 @@ impl_from_error!(Io, std::io::Error);
 impl_from_error!(ScriptValidation, bitcoin::blockdata::script::Error);
 impl_from_error!(Blockchain, BlockchainError);
 impl_from_error!(SerdeJson, serde_json::Error);
-impl_from_error!(WalletInput, slip132::Error);
+impl_from_error!(WalletInput, DescriptorError);
 impl_from_error!(TomlParsing, toml::de::Error);
 impl_from_error!(BlockValidation, BlockValidationErrors);
 impl_from_error!(AddressParsing, bitcoin::address::ParseError);

crates/floresta-node/src/florestad.rs

@@ -4,12 +4,15 @@ use std::net::Ipv4Addr;
 use std::net::SocketAddr;
 use std::path::Path;
 use std::path::PathBuf;
+use std::str::FromStr;
 use std::sync::Arc;
 use std::sync::Mutex;
 #[cfg(feature = "json-rpc")]
 use std::sync::OnceLock;
 
+use bitcoin::Address;
 pub use bitcoin::Network;
+use bitcoin::ScriptBuf;
 #[cfg(feature = "zmq-server")]
 use floresta_chain::pruned_utreexo::BlockchainInterface;
 pub use floresta_chain::AssumeUtreexoValue;
@@ -27,6 +30,7 @@ use floresta_electrum::electrum_protocol::ElectrumServer;
 use floresta_mempool::Mempool;
 use floresta_watch_only::kv_database::KvDatabase;
 use floresta_watch_only::AddressCache;
+use floresta_watch_only::WatchOnlyError;
 use floresta_wire::address_man::AddressMan;
 use floresta_wire::node::running_ctx::RunningNode;
 use floresta_wire::node::UtreexoNode;
@@ -57,7 +61,6 @@ use crate::error::FlorestadError;
 use crate::florestad::fs::OpenOptions;
 #[cfg(feature = "json-rpc")]
 use crate::json_rpc;
-use crate::wallet_input::InitialWalletSetup;
 #[cfg(feature = "zmq-server")]
 use crate::zmq::ZMQServer;
 
@@ -741,45 +744,71 @@ impl Florestad {
                 Self::get_config_file(&default_path)
             }
         };
-        let setup = self.prepare_wallet_setup(config_file)?;
 
         // Add the configured descriptors and addresses to the wallet
-        for descriptor in setup.descriptors {
-            let descriptor = descriptor.to_string();
-            let is_cached = wallet.is_cached(&descriptor)?;
+        for descriptor in self.get_descriptors(&config_file) {
+            if let Err(e) = wallet.push_descriptor(&descriptor) {
+                if let WatchOnlyError::DescriptorDuplicate = e {
+                    warn!("Descriptor already exists in wallet, skipping: {descriptor}");
+                } else {
+                    return Err(FlorestadError::from(e));
+                }
+            }
+        }
 
-            if !is_cached {
-                wallet.push_descriptor(&descriptor)?;
+        for xpub in self.get_xpubs(&config_file) {
+            if let Err(e) = wallet.push_xpub(&xpub, self.config.network) {
+                if let WatchOnlyError::DescriptorDuplicate = e {
+                    warn!("Descriptor for the provided XPUB already exists in the wallet. Skipping: {xpub}");
+                } else {
+                    return Err(FlorestadError::from(e));
+                }
             }
         }
-        for addresses in setup.addresses {
-            wallet.cache_address(addresses.script_pubkey());
+
+        for address in self.get_addresses(&config_file)? {
+            wallet.cache_address(address);
         }
 
         info!("Wallet setup completed!");
         Ok(())
     }
 
-    /// Parses the configured list of xpubs, output descriptors and addresses to watch for, and
-    /// returns the constructed `InitialWalletSetup`.
-    fn prepare_wallet_setup(
-        &self,
-        config_file: ConfigFile,
-    ) -> Result<InitialWalletSetup, FlorestadError> {
-        let config = &self.config;
+    /// Get the wallet descriptors from the config file and the environment.
+    fn get_descriptors(&self, config_file: &ConfigFile) -> Vec<String> {
+        let mut descriptors = Vec::new();
+        descriptors.extend(self.config.wallet_descriptor.clone().unwrap_or_default());
+        descriptors.extend(config_file.wallet.descriptors.clone().unwrap_or_default());
+
+        descriptors
+    }
 
+    /// Get the wallet xpubs from the config file and the environment
+    fn get_xpubs(&self, config_file: &ConfigFile) -> Vec<String> {
         let mut xpubs = Vec::new();
-        xpubs.extend(config.wallet_xpub.clone().unwrap_or_default());
-        xpubs.extend(config_file.wallet.xpubs.unwrap_or_default());
+        xpubs.extend(self.config.wallet_xpub.clone().unwrap_or_default());
+        xpubs.extend(config_file.wallet.xpubs.clone().unwrap_or_default());
         xpubs.extend(Self::get_key_from_env());
 
-        let mut descriptors = Vec::new();
-        descriptors.extend(config.wallet_descriptor.clone().unwrap_or_default());
-        descriptors.extend(config_file.wallet.descriptors.unwrap_or_default());
+        xpubs
+    }
 
-        let addresses = config_file.wallet.addresses.unwrap_or_default();
+    /// Get the wallet addresses from the config file
+    fn get_addresses(&self, config_file: &ConfigFile) -> Result<Vec<ScriptBuf>, FlorestadError> {
+        let addresses_string = config_file.wallet.addresses.clone().unwrap_or_default();
+
+        let addresses = addresses_string
+            .iter()
+            .map(|address| match Address::from_str(address) {
+                Ok(address) => Ok(address.assume_checked().script_pubkey()),
+                Err(e) => {
+                    error!("Invalid address provided: {address} \nReason: {e:?}");
+                    Err(e)
+                }
+            })
+            .collect::<Result<Vec<_>, _>>()?;
 
-        InitialWalletSetup::build(&xpubs, &descriptors, &addresses, config.network, 100)
+        Ok(addresses)
     }
 
     /// Get the default Electrum port for the Network and TLS combination.

crates/floresta-node/src/json_rpc/res.rs

@@ -5,6 +5,7 @@ use corepc_types::v30::GetBlockVerboseOne;
 use floresta_chain::extensions::HeaderExtError;
 use floresta_common::impl_error_from;
 use floresta_mempool::mempool::AcceptToMempoolError;
+use floresta_watch_only::descriptor::DescriptorError;
 use serde::Deserialize;
 use serde::Serialize;
 
@@ -163,7 +164,7 @@ pub enum JsonRpcError {
     InvalidScript,
 
     /// The provided descriptor is invalid, e.g., if it does not match the expected format
-    InvalidDescriptor(miniscript::Error),
+    InvalidDescriptor(DescriptorError),
 
     /// The requested block is not found in the blockchain
     BlockNotFound,
@@ -283,7 +284,7 @@ impl From<HeaderExtError> for JsonRpcError {
     }
 }
 
-impl_error_from!(JsonRpcError, miniscript::Error, InvalidDescriptor);
+impl_error_from!(JsonRpcError, DescriptorError, InvalidDescriptor);
 
 impl<T: std::fmt::Debug> From<floresta_watch_only::WatchOnlyError<T>> for JsonRpcError {
     fn from(e: floresta_watch_only::WatchOnlyError<T>) -> Self {