From b08f5a36a78e226bc1ecf7251b00b8da8d4214a9 Mon Sep 17 00:00:00 2001 From: JSwambo Date: Mon, 7 Feb 2022 13:54:08 +0000 Subject: [PATCH] Compute checksums for wallet descriptors without bitcoind The desc_checksum function and poly_mod were taken from rust-miniscript: https://github.com/rust-bitcoin/rust-miniscript/blob/master/src/descriptor/checksum.rs Hopefully the desc_checksum function will be publicly exposed in the future and we can remove it. --- src/bitcoind/interface.rs | 16 ----- src/bitcoind/mod.rs | 12 +++- src/bitcoind/poller.rs | 23 ++----- src/revaultd.rs | 135 ++++++++++++++++++++++++++++++++++---- tests/servers/cosignerd | 2 +- 5 files changed, 143 insertions(+), 45 deletions(-) diff --git a/src/bitcoind/interface.rs b/src/bitcoind/interface.rs index f57916e6..0602d06a 100644 --- a/src/bitcoind/interface.rs +++ b/src/bitcoind/interface.rs @@ -381,22 +381,6 @@ impl BitcoinD { } } - /// Constructs an `addr()` descriptor out of an address - pub fn addr_descriptor(&self, address: &str) -> Result { - let desc_wo_checksum = format!("addr({})", address); - - Ok(self - .make_watchonly_request( - "getdescriptorinfo", - ¶ms!(Json::String(desc_wo_checksum)), - )? - .get("descriptor") - .expect("No 'descriptor' in 'getdescriptorinfo'") - .as_str() - .expect("'descriptor' in 'getdescriptorinfo' isn't a string anymore") - .to_string()) - } - fn bulk_import_descriptors( &self, client: &Client, diff --git a/src/bitcoind/mod.rs b/src/bitcoind/mod.rs index 6d2eb758..43206daf 100644 --- a/src/bitcoind/mod.rs +++ b/src/bitcoind/mod.rs @@ -3,7 +3,11 @@ pub mod poller; pub mod utils; use crate::config::BitcoindConfig; -use crate::{database::DatabaseError, revaultd::RevaultD, threadmessages::BitcoindMessageOut}; +use crate::{ + database::DatabaseError, + revaultd::{ChecksumError, RevaultD}, + threadmessages::BitcoindMessageOut, +}; use interface::{BitcoinD, WalletTransaction}; use poller::poller_main; use revault_tx::bitcoin::{Network, Txid}; @@ -80,6 +84,12 @@ impl From for BitcoindError { } } +impl From for BitcoindError { + fn from(e: ChecksumError) -> Self { + Self::Custom(e.to_string()) + } +} + fn check_bitcoind_network( bitcoind: &BitcoinD, config_network: &Network, diff --git a/src/bitcoind/poller.rs b/src/bitcoind/poller.rs index 91279bc5..18219c5d 100644 --- a/src/bitcoind/poller.rs +++ b/src/bitcoind/poller.rs @@ -1383,13 +1383,11 @@ fn handle_new_deposit( })?; db_update_deposit_index(&revaultd.read().unwrap().db_file(), new_index)?; revaultd.write().unwrap().current_unused_index = new_index; - let next_addr = bitcoind - .addr_descriptor(&revaultd.read().unwrap().last_deposit_address().to_string())?; + let next_addr = revaultd.read().unwrap().last_deposit_desc()?; bitcoind.import_fresh_deposit_descriptor(next_addr)?; - let next_addr = bitcoind - .addr_descriptor(&revaultd.read().unwrap().last_unvault_address().to_string())?; - bitcoind.import_fresh_unvault_descriptor(next_addr)?; + let next_addr = revaultd.read().unwrap().last_unvault_desc()?; + bitcoind.import_fresh_unvault_descriptor(next_addr)?; log::debug!( "Incremented deposit derivation index from {}", current_first_index @@ -1725,7 +1723,7 @@ fn maybe_create_wallet(revaultd: &mut RevaultD, bitcoind: &BitcoinD) -> Result<( bitcoind.createwallet_startup(bitcoind_wallet_path, true)?; log::info!("Importing descriptors to bitcoind watchonly wallet."); - // Now, import descriptors. + // Now, import deposit address descriptors. // In theory, we could just import the vault (deposit) descriptor expressed using xpubs, give a // range to bitcoind as the gap limit, and be fine. // Unfortunately we cannot just import descriptors as is, since bitcoind does not support @@ -1734,23 +1732,16 @@ fn maybe_create_wallet(revaultd: &mut RevaultD, bitcoind: &BitcoinD) -> Result<( // currently supported by bitcoind) if there are more than 15 stakeholders. // Therefore, we derive [max index] `addr()` descriptors to import into bitcoind, and handle // the derivation index mess ourselves :'( - let addresses: Vec<_> = revaultd - .all_deposit_addresses() - .into_iter() - .map(|a| bitcoind.addr_descriptor(&a)) - .collect::, _>>()?; + let addresses: Vec<_> = revaultd.all_deposit_descriptors(); log::trace!("Importing deposit descriptors '{:?}'", &addresses); bitcoind.startup_import_deposit_descriptors(addresses, wallet.timestamp, fresh_wallet)?; + // Now, import the unvault address descriptors. // As a consequence, we don't have enough information to opportunistically import a // descriptor at the reception of a deposit anymore. Thus we need to blindly import *both* // deposit and unvault descriptors.. // FIXME: maybe we actually have, with the derivation_index_map ? - let addresses: Vec<_> = revaultd - .all_unvault_addresses() - .into_iter() - .map(|a| bitcoind.addr_descriptor(&a)) - .collect::, _>>()?; + let addresses: Vec<_> = revaultd.all_unvault_descriptors(); log::trace!("Importing unvault descriptors '{:?}'", &addresses); bitcoind.startup_import_unvault_descriptors(addresses, wallet.timestamp, fresh_wallet)?; } diff --git a/src/revaultd.rs b/src/revaultd.rs index ea61b165..80da4567 100644 --- a/src/revaultd.rs +++ b/src/revaultd.rs @@ -8,6 +8,7 @@ use std::{ convert::TryFrom, fmt, fs, io::{self, Read, Write}, + iter::FromIterator, net::SocketAddr, path::{Path, PathBuf}, str::FromStr, @@ -384,6 +385,93 @@ impl fmt::Display for DatadirError { impl std::error::Error for DatadirError {} +#[derive(Debug)] +pub enum ChecksumError { + Checksum(String), +} + +impl fmt::Display for ChecksumError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::Checksum(e) => { + write!(f, "Error computing checksum: {}", e) + } + } + } +} + +impl std::error::Error for ChecksumError {} + +const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ "; +const CHECKSUM_CHARSET: &str = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; + +fn poly_mod(mut c: u64, val: u64) -> u64 { + let c0 = c >> 35; + + c = ((c & 0x7ffffffff) << 5) ^ val; + if c0 & 1 > 0 { + c ^= 0xf5dee51989 + }; + if c0 & 2 > 0 { + c ^= 0xa9fdca3312 + }; + if c0 & 4 > 0 { + c ^= 0x1bab10e32d + }; + if c0 & 8 > 0 { + c ^= 0x3706b1677a + }; + if c0 & 16 > 0 { + c ^= 0x644d626ffd + }; + + c +} + +/// Compute the checksum of a descriptor +/// Note that this function does not check if the +/// descriptor string is syntactically correct or not. +/// This only computes the checksum +pub fn desc_checksum(desc: &str) -> Result { + let mut c = 1; + let mut cls = 0; + let mut clscount = 0; + + for ch in desc.chars() { + let pos = INPUT_CHARSET + .find(ch) + .ok_or(ChecksumError::Checksum(format!( + "Invalid character in checksum: '{}'", + ch + )))? as u64; + c = poly_mod(c, pos & 31); + cls = cls * 3 + (pos >> 5); + clscount += 1; + if clscount == 3 { + c = poly_mod(c, cls); + cls = 0; + clscount = 0; + } + } + if clscount > 0 { + c = poly_mod(c, cls); + } + (0..8).for_each(|_| c = poly_mod(c, 0)); + c ^= 1; + + let mut chars = Vec::with_capacity(8); + for j in 0..8 { + chars.push( + CHECKSUM_CHARSET + .chars() + .nth(((c >> (5 * (7 - j))) & 31) as usize) + .unwrap(), + ); + } + + Ok(String::from_iter(chars)) +} + impl RevaultD { /// Creates our global state by consuming the static configuration pub fn from_config(config: Config) -> Result { @@ -517,6 +605,13 @@ impl RevaultD { NoisePubKey(curve25519::scalarmult_base(&scalar).0) } + /// vault (deposit) address descriptor with checksum in canonical form (e.g. + /// 'addr(ADDRESS)#CHECKSUM') for importing with bitcoind + pub fn vault_desc(&self, child_number: ChildNumber) -> Result { + let addr_desc = format!("addr({})", self.vault_address(child_number)); + Ok(format!("{}#{}", addr_desc, desc_checksum(&addr_desc)?)) + } + pub fn vault_address(&self, child_number: ChildNumber) -> Address { self.deposit_descriptor .derive(child_number, &self.secp_ctx) @@ -525,6 +620,13 @@ impl RevaultD { .expect("deposit_descriptor is a wsh") } + /// unvault address descriptor with checksum in canonical form (e.g. + /// 'addr(ADDRESS)#CHECKSUM') for importing with bitcoind + pub fn unvault_desc(&self, child_number: ChildNumber) -> Result { + let addr_desc = format!("addr({})", self.unvault_address(child_number)); + Ok(format!("{}#{}", addr_desc, desc_checksum(&addr_desc)?)) + } + pub fn unvault_address(&self, child_number: ChildNumber) -> Address { self.unvault_descriptor .derive(child_number, &self.secp_ctx) @@ -601,38 +703,49 @@ impl RevaultD { self.vault_address(self.current_unused_index) } + pub fn last_deposit_desc(&self) -> Result { + let raw_index: u32 = self.current_unused_index.into(); + // FIXME: this should fail instead of creating a hardened index + self.vault_desc(ChildNumber::from(raw_index + self.gap_limit())) + } + pub fn last_deposit_address(&self) -> Address { let raw_index: u32 = self.current_unused_index.into(); // FIXME: this should fail instead of creating a hardened index self.vault_address(ChildNumber::from(raw_index + self.gap_limit())) } + pub fn last_unvault_desc(&self) -> Result { + let raw_index: u32 = self.current_unused_index.into(); + // FIXME: this should fail instead of creating a hardened index + self.unvault_desc(ChildNumber::from(raw_index + self.gap_limit())) + } + pub fn last_unvault_address(&self) -> Address { let raw_index: u32 = self.current_unused_index.into(); // FIXME: this should fail instead of creating a hardened index self.unvault_address(ChildNumber::from(raw_index + self.gap_limit())) } - /// All deposit addresses as strings up to the gap limit (100) - pub fn all_deposit_addresses(&mut self) -> Vec { + /// All deposit address descriptors as strings up to the gap limit (100) + pub fn all_deposit_descriptors(&mut self) -> Vec { self.derivation_index_map - .keys() - .map(|s| { - Address::from_script(s, self.bitcoind_config.network) - .expect("Created from P2WSH address") - .to_string() + .values() + .map(|child_num| { + self.vault_desc(ChildNumber::from(*child_num)) + .expect("Failed checksum computation") }) .collect() } - /// All unvault addresses as strings up to the gap limit (100) - pub fn all_unvault_addresses(&mut self) -> Vec { + /// All unvault address descriptors as strings up to the gap limit (100) + pub fn all_unvault_descriptors(&mut self) -> Vec { let raw_index: u32 = self.current_unused_index.into(); (0..raw_index + self.gap_limit()) .map(|raw_index| { // FIXME: this should fail instead of creating a hardened index - self.unvault_address(ChildNumber::from(raw_index)) - .to_string() + self.unvault_desc(ChildNumber::from(raw_index)) + .expect("Failed to comput checksum") }) .collect() } diff --git a/tests/servers/cosignerd b/tests/servers/cosignerd index e41e5caf..507c26d5 160000 --- a/tests/servers/cosignerd +++ b/tests/servers/cosignerd @@ -1 +1 @@ -Subproject commit e41e5cafc33568f61076700ba360f831e3b072cc +Subproject commit 507c26d563b03f151649cd71ac7a62f6b9f3a4de