diff --git a/.cirrus.yml b/.cirrus.yml index 6a6f8f6d..27d0f817 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -68,9 +68,9 @@ task: # Download the bitcoind binary ARCHIVE_NAME="$BITCOIND_DIR_NAME.tar.gz" - curl https://bitcoincore.org/bin/bitcoin-core-$BITCOIND_VERSION/bitcoin-$BITCOIND_VERSION-x86_64-linux-gnu.tar.gz -o $ARCHIVE_NAME + curl http://download.darosior.ninja/dump/bitcoind-patched-for-revaultd.tar.gz -o $ARCHIVE_NAME tar -xzf $ARCHIVE_NAME - export BITCOIND_PATH=$BITCOIND_DIR_NAME/bin/bitcoind + export BITCOIND_PATH=bitcoind-patched-for-revaultd/bin/bitcoind # Setup the postgres instance for the servers pg_ctlcluster 13 main start diff --git a/src/bitcoind/interface.rs b/src/bitcoind/interface.rs index 9d0290e5..b0c5782a 100644 --- a/src/bitcoind/interface.rs +++ b/src/bitcoind/interface.rs @@ -5,7 +5,12 @@ use revault_tx::bitcoin::{ Address, Amount, BlockHash, OutPoint, Script, Transaction, TxOut, Txid, }; -use std::{collections::HashMap, fs, str::FromStr, time::Duration}; +use std::{ + collections::{HashMap, HashSet}, + fs, + str::FromStr, + time::Duration, +}; use jsonrpc::{ arg, @@ -19,11 +24,6 @@ use serde_json::Value as Json; // If bitcoind takes more than 3 minutes to answer one of our queries, fail. const RPC_SOCKET_TIMEOUT: u64 = 180; -// Labels used to tag utxos in the watchonly wallet -const DEPOSIT_UTXOS_LABEL: &str = "revault-deposit"; -const UNVAULT_UTXOS_LABEL: &str = "revault-unvault"; -const CPFP_UTXOS_LABEL: &str = "revault-cpfp"; - pub struct BitcoinD { node_client: Client, watchonly_client: Client, @@ -345,32 +345,14 @@ 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( + fn import_descriptors( &self, client: &Client, descriptors: Vec, - timestamp: u32, - label: String, - fresh_wallet: bool, + timestamp: Option, active: bool, ) -> Result<(), BitcoindError> { - if !fresh_wallet { + if timestamp.is_some() { log::debug!("Not a fresh wallet, rescan *may* take some time."); } @@ -383,13 +365,11 @@ impl BitcoinD { // will rescan the last few blocks for each of them. desc_map.insert( "timestamp".to_string(), - if fresh_wallet { - Json::String("now".to_string()) - } else { - Json::Number(serde_json::Number::from(timestamp)) - }, + timestamp + .map(serde_json::Number::from) + .map(Json::Number) + .unwrap_or_else(|| Json::String("now".to_string())), ); - desc_map.insert("label".to_string(), Json::String(label.clone())); desc_map.insert("active".to_string(), Json::Bool(active)); Json::Object(desc_map) @@ -401,94 +381,44 @@ impl BitcoinD { "importdescriptors", ¶ms!(Json::Array(all_descriptors)), )?; - if res.get(0).map(|x| x.get("success")) == Some(Some(&Json::Bool(true))) { + let all_succeeded = res + .as_array() + .map(|results| { + results + .iter() + .all(|res| res.get("success") == Some(&Json::Bool(true))) + }) + .unwrap_or(false); + if all_succeeded { return Ok(()); } Err(BitcoindError::Custom(format!( "Error returned from 'importdescriptor': {:?}", - res.get(0).map(|r| r.get("error")) + res ))) } - pub fn startup_import_deposit_descriptors( - &self, - descriptors: Vec, - timestamp: u32, - fresh_wallet: bool, - ) -> Result<(), BitcoindError> { - self.bulk_import_descriptors( - &self.watchonly_client, - descriptors, - timestamp, - DEPOSIT_UTXOS_LABEL.to_string(), - fresh_wallet, - false, - ) - } - - pub fn startup_import_unvault_descriptors( + /// Import the deposit and Unvault descriptors, when at startup. + pub fn startup_import_descriptors( &self, - descriptors: Vec, - timestamp: u32, - fresh_wallet: bool, + descriptors: [String; 2], + timestamp: Option, ) -> Result<(), BitcoindError> { - self.bulk_import_descriptors( + self.import_descriptors( &self.watchonly_client, - descriptors, + descriptors.to_vec(), timestamp, - UNVAULT_UTXOS_LABEL.to_string(), - fresh_wallet, false, ) } - pub fn startup_import_cpfp_descriptor( + pub fn import_cpfp_descriptor( &self, descriptor: String, - timestamp: u32, - fresh_wallet: bool, + timestamp: Option, ) -> Result<(), BitcoindError> { - self.bulk_import_descriptors( - &self.cpfp_client, - vec![descriptor], - timestamp, - CPFP_UTXOS_LABEL.to_string(), - fresh_wallet, - true, - ) - } - - fn import_fresh_descriptor( - &self, - descriptor: String, - label: String, - ) -> Result<(), BitcoindError> { - let mut desc_map = serde_json::Map::with_capacity(3); - desc_map.insert("desc".to_string(), Json::String(descriptor)); - desc_map.insert("timestamp".to_string(), Json::String("now".to_string())); - desc_map.insert("label".to_string(), Json::String(label)); - - let res = self.make_watchonly_request( - "importdescriptors", - ¶ms!(Json::Array(vec![Json::Object(desc_map,)])), - )?; - if res.get(0).map(|x| x.get("success")).flatten() == Some(&Json::Bool(true)) { - return Ok(()); - } - - Err(BitcoindError::Custom(format!( - "In import_fresh descriptor, no success returned from 'importdescriptor': {:?}", - res - ))) - } - - pub fn import_fresh_deposit_descriptor(&self, descriptor: String) -> Result<(), BitcoindError> { - self.import_fresh_descriptor(descriptor, DEPOSIT_UTXOS_LABEL.to_string()) - } - - pub fn import_fresh_unvault_descriptor(&self, descriptor: String) -> Result<(), BitcoindError> { - self.import_fresh_descriptor(descriptor, UNVAULT_UTXOS_LABEL.to_string()) + self.import_descriptors(&self.cpfp_client, vec![descriptor], timestamp, true) } pub fn list_unspent_cpfp(&self) -> Result, BitcoindError> { @@ -546,7 +476,7 @@ impl BitcoinD { fn list_since_block( &self, tip: &BlockchainTip, - label: Option<&'static str>, + descriptor: Option, ) -> Result, BitcoindError> { let req = if tip.height == 0 { self.make_request(&self.watchonly_client, "listsinceblock", ¶ms!())? @@ -554,7 +484,13 @@ impl BitcoinD { self.make_request( &self.watchonly_client, "listsinceblock", - ¶ms!(Json::String(tip.hash.to_string())), + ¶ms!( + Json::String(tip.hash.to_string()), + Json::Number(1.into()), + Json::Bool(true), + Json::Bool(true), + Json::Bool(true) + ), )? }; Ok(req @@ -565,20 +501,26 @@ impl BitcoinD { .iter() .filter_map(|t| { let t = ListSinceBlockTransaction::from(t); - if label.or_else(|| t.label.as_deref()) == t.label.as_deref() { - Some(t) - } else { - None + match descriptor { + None => Some(t), + Some(ref desc) => { + if t.wallet_descs.contains(desc) { + Some(t) + } else { + None + } + } } }) .collect()) } - pub fn list_deposits_since_block( + fn list_deposits_since_block( &self, tip: &BlockchainTip, + deposit_desc: String, ) -> Result, BitcoindError> { - self.list_since_block(tip, Some(DEPOSIT_UTXOS_LABEL)) + self.list_since_block(tip, Some(deposit_desc)) } /// Repeatedly called by our main loop to stay in sync with bitcoind. @@ -588,6 +530,7 @@ impl BitcoinD { deposits_utxos: &HashMap, db_tip: &BlockchainTip, min_conf: u32, + deposit_desc: String, ) -> Result { let (mut new_unconf, mut new_conf, mut new_spent) = (HashMap::new(), HashMap::new(), HashMap::new()); @@ -632,7 +575,7 @@ impl BitcoinD { } // Second, we scan for new ones. - let utxos = self.list_deposits_since_block(db_tip)?; + let utxos = self.list_deposits_since_block(db_tip, deposit_desc)?; for utxo in utxos { if utxo.is_receive && deposits_utxos.get(&utxo.outpoint).is_none() { new_unconf.insert( @@ -827,14 +770,16 @@ impl BitcoinD { block_hash )); + // Get the spent txid to ignore the entries about this transaction + let spent_txid = spent_outpoint.txid.to_string(); + // We use a cache to avoid needless iterations, since listsinceblock returns an entry + // per transaction output, not per transaction. + let mut visited_txs = HashSet::new(); for transaction in transactions { if transaction.get("category").map(|c| c.as_str()).flatten() != Some("send") { continue; } - // TODO: i think we can also filter out the entries *with* a "revault-somthing" label, - // but we need to be sure. - let spending_txid = transaction .get("txid") .map(|t| t.as_str()) @@ -844,6 +789,12 @@ impl BitcoinD { block_hash )); + if visited_txs.contains(&spending_txid) || &spent_txid == spending_txid { + continue; + } else { + visited_txs.insert(spending_txid); + } + let gettx_res = self.make_watchonly_request( "gettransaction", ¶ms!( @@ -1145,7 +1096,7 @@ pub struct ListSinceBlockTransaction { pub outpoint: OutPoint, pub txo: TxOut, pub is_receive: bool, - pub label: Option, + pub wallet_descs: Vec, pub confirmations: i32, pub blockheight: Option, } @@ -1201,11 +1152,23 @@ impl From<&Json> for ListSinceBlockTransaction { .map(|a| a.as_u64()) .flatten() .map(|b| b as u32); - let label = j - .get("label") - .map(|l| l.as_str()) - .flatten() - .map(|l| l.to_string()); + // FIXME: allocs + let wallet_descs = j + .get("wallet_descs") + .map(|l| { + l.as_array() + .expect( + "API break, 'listsinceblock' entry didn't contain a valid 'wallet_descs'.", + ) + .iter() + .map(|desc| { + desc.as_str() + .expect("Invalid desc string in 'listsinceblock'.") + .to_string() + }) + .collect() + }) + .unwrap_or_else(|| Vec::new()); ListSinceBlockTransaction { outpoint: OutPoint { @@ -1213,7 +1176,7 @@ impl From<&Json> for ListSinceBlockTransaction { vout: vout as u32, // Bitcoin makes this safe }, is_receive, - label, + wallet_descs, txo: TxOut { value, script_pubkey, diff --git a/src/bitcoind/poller.rs b/src/bitcoind/poller.rs index 977f4378..aa9348ce 100644 --- a/src/bitcoind/poller.rs +++ b/src/bitcoind/poller.rs @@ -1286,7 +1286,6 @@ fn handle_spent_unvault( fn handle_new_deposit( revaultd: &mut Arc>, db_path: &Path, - bitcoind: &BitcoinD, deposits_cache: &mut HashMap, outpoint: OutPoint, utxo: UtxoInfo, @@ -1302,6 +1301,7 @@ fn handle_new_deposit( return Ok(()); } + // FIXME: get rid of this mapping, use bitcoind instead. let derivation_index = *revaultd .read() .unwrap() @@ -1333,8 +1333,6 @@ fn handle_new_deposit( ); deposits_cache.insert(outpoint, utxo); - // Mind the gap! https://www.youtube.com/watch?v=UOPyGKDQuRk - // FIXME: of course, that's rudimentary let current_first_index = revaultd.read().unwrap().current_unused_index; if derivation_index >= current_first_index { let new_index = revaultd @@ -1342,18 +1340,10 @@ fn handle_new_deposit( .unwrap() .current_unused_index .increment() - .map_err(|e| { - // FIXME: we should probably go back to 0 at this point. - BitcoindError::Custom(format!("Deriving next index: {}", e)) - })?; + // FIXME: we should probably go back to 0 at this point. + .expect("Incrementing derivation index"); 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())?; - 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)?; log::debug!( "Incremented deposit derivation index from {}", @@ -1555,10 +1545,11 @@ fn update_utxos( deposits_cache, previous_tip, revaultd.read().unwrap().min_conf, + revaultd.read().unwrap().deposit_descriptor.to_string(), )?; for (outpoint, utxo) in new_deposits { - handle_new_deposit(revaultd, &db_path, bitcoind, deposits_cache, outpoint, utxo)?; + handle_new_deposit(revaultd, &db_path, deposits_cache, outpoint, utxo)?; } for (outpoint, utxo) in conf_deposits { @@ -1711,6 +1702,11 @@ fn maybe_create_wallet(revaultd: &mut RevaultD, bitcoind: &BitcoinD) -> Result<( BitcoindError::Custom(format!("Computing time since epoch: {}", e.to_string())) })?; let fresh_wallet = (curr_timestamp - wallet.timestamp as u64) < 30; + let rescan_timestamp = if fresh_wallet { + None + } else { + Some(wallet.timestamp) + }; // TODO: sanity check descriptors are imported when migrating to 0.22 @@ -1727,34 +1723,13 @@ 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. - // 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 - // Miniscript ones yet. Worse, we actually need to derive them to pass them to bitcoind since - // the vault one (which we are interested about) won't be expressed with a `multi()` statement ( - // 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::, _>>()?; - log::trace!("Importing deposit descriptors '{:?}'", &addresses); - bitcoind.startup_import_deposit_descriptors(addresses, wallet.timestamp, fresh_wallet)?; - - // 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::, _>>()?; - log::trace!("Importing unvault descriptors '{:?}'", &addresses); - bitcoind.startup_import_unvault_descriptors(addresses, wallet.timestamp, fresh_wallet)?; + bitcoind.startup_import_descriptors( + [ + revaultd.deposit_descriptor.to_string(), + revaultd.unvault_descriptor.to_string(), + ], + rescan_timestamp, + )?; } if let Some(cpfp_key) = revaultd.cpfp_key { @@ -1794,7 +1769,7 @@ fn maybe_create_wallet(revaultd: &mut RevaultD, bitcoind: &BitcoinD) -> Result<( .inner() .to_string_with_secret(&keymap); - bitcoind.startup_import_cpfp_descriptor(cpfp_desc, wallet.timestamp, fresh_wallet)?; + bitcoind.import_cpfp_descriptor(cpfp_desc, rescan_timestamp)?; } } else { log::info!("Not creating the CPFP wallet, as we don't have a CPFP key");