diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fa7d65cc..fe3a528a 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -6,47 +6,37 @@ mod utils; use crate::{ bitcoin::BitcoinInterface, - database::{Coin, DatabaseInterface}, - descriptors, DaemonControl, VERSION, + database::{Coin, DatabaseConnection, DatabaseInterface}, + descriptors, + spend::{ + create_spend, AddrInfo, CandidateCoin, CreateSpendRes, SpendCreationError, + SpendOutputAddress, SpendTxFees, TxGetter, + }, + DaemonControl, VERSION, }; pub use crate::database::{CoinStatus, LabelItem}; -use bdk_coin_select::InsufficientFunds; use utils::{ - deser_addr_assume_checked, deser_amount_from_sats, deser_fromstr, deser_hex, - select_coins_for_spend, ser_amount, ser_hex, ser_to_string, unsigned_tx_max_vbytes, + deser_addr_assume_checked, deser_amount_from_sats, deser_fromstr, deser_hex, ser_amount, + ser_hex, ser_to_string, }; use std::{ - collections::{hash_map, BTreeMap, HashMap, HashSet}, + collections::{hash_map, HashMap, HashSet}, convert::TryInto, - fmt, + fmt, sync, }; use miniscript::{ bitcoin::{ - self, address, bip32, - locktime::absolute, - psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt}, + self, address, bip32, constants::WITNESS_SCALE_FACTOR, + psbt::PartiallySignedTransaction as Psbt, }, psbt::PsbtExt, }; use serde::{Deserialize, Serialize}; -// We would never create a transaction with an output worth less than this. -// That's 1$ at 20_000$ per BTC. -const DUST_OUTPUT_SATS: u64 = 5_000; - -// Long-term feerate (sats/vb) used for coin selection considerations. -const LONG_TERM_FEERATE_VB: f32 = 10.0; - -// Assume that paying more than 1BTC in fee is a bug. -const MAX_FEE: u64 = bitcoin::blockdata::constants::COIN_VALUE; - -// Assume that paying more than 1000sat/vb in feerate is a bug. -const MAX_FEERATE: u64 = 1_000; - // Timestamp in the header of the genesis block. Used for sanity checks. const MAINNET_GENESIS_TIME: u32 = 1231006505; @@ -58,15 +48,12 @@ pub enum CommandError { AlreadySpent(bitcoin::OutPoint), ImmatureCoinbase(bitcoin::OutPoint), Address(bitcoin::address::Error), - InvalidOutputValue(bitcoin::Amount), + SpendCreation(SpendCreationError), InsufficientFunds( /* in value */ bitcoin::Amount, /* out value */ Option, /* target feerate */ u64, ), - InsaneFees(InsaneFeeInfo), - FetchingTransaction(bitcoin::OutPoint), - SanityCheckFailure(Psbt), UnknownSpend(bitcoin::Txid), // FIXME: when upgrading Miniscript put the actual error there SpendFinalization(String), @@ -78,57 +65,40 @@ pub enum CommandError { RecoveryNotAvailable, /// Overflowing or unhardened derivation index. InvalidDerivationIndex, - CoinSelectionError(InsufficientFunds), RbfError(RbfErrorInfo), } impl fmt::Display for CommandError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Self::NoOutpointForSelfSend => write!(f, "No provided outpoint for self-send. Need at least one."), + Self::NoOutpointForSelfSend => { + write!(f, "No provided outpoint for self-send. Need at least one.") + } Self::InvalidFeerate(sats_vb) => write!(f, "Invalid feerate: {} sats/vb.", sats_vb), Self::AlreadySpent(op) => write!(f, "Coin at '{}' is already spent.", op), - Self::ImmatureCoinbase(op) => write!(f, "Coin at '{}' is from an immature coinbase transaction.", op), - Self::UnknownOutpoint(op) => write!(f, "Unknown outpoint '{}'.", op), - Self::Address(e) => write!( + Self::ImmatureCoinbase(op) => write!( f, - "Address error: {}", e + "Coin at '{}' is from an immature coinbase transaction.", + op ), - Self::InvalidOutputValue(amount) => write!(f, "Invalid output value '{}'.", amount), - Self::InsufficientFunds(in_val, out_val, feerate) => if let Some(out_val) = out_val { - write!( + Self::UnknownOutpoint(op) => write!(f, "Unknown outpoint '{}'.", op), + Self::Address(e) => write!(f, "Address error: {}", e), + Self::SpendCreation(e) => write!(f, "Creating spend: {}", e), + Self::InsufficientFunds(in_val, out_val, feerate) => { + if let Some(out_val) = out_val { + write!( f, "Cannot create a {} sat/vb transaction with input value {} and output value {}", feerate, in_val, out_val ) - } else { - write!( - f, - "Not enough fund to create a {} sat/vb transaction with input value {}", - feerate, in_val - ) - }, - Self::InsaneFees(info) => write!( - f, - "We assume transactions with a fee larger than {} sats or a feerate larger than {} sats/vb are a mistake. \ - The created transaction {}.", - MAX_FEE, - MAX_FEERATE, - match info { - InsaneFeeInfo::NegativeFee => "would have a negative fee".to_string(), - InsaneFeeInfo::TooHighFee(f) => format!("{} sats in fees", f), - InsaneFeeInfo::InvalidFeerate => "would have an invalid feerate".to_string(), - InsaneFeeInfo::TooHighFeerate(r) => format!("has a feerate of {} sats/vb", r), - }, - ), - Self::FetchingTransaction(op) => { - write!(f, "Could not fetch transaction for coin {}", op) + } else { + write!( + f, + "Not enough fund to create a {} sat/vb transaction with input value {}", + feerate, in_val + ) + } } - Self::SanityCheckFailure(psbt) => write!( - f, - "BUG! Please report this. Failed sanity checks for PSBT '{}'.", - psbt - ), Self::UnknownSpend(txid) => write!(f, "Unknown spend transaction '{}'.", txid), Self::SpendFinalization(e) => { write!(f, "Failed to finalize the spend transaction PSBT: '{}'.", e) @@ -143,36 +113,23 @@ impl fmt::Display for CommandError { Self::RecoveryNotAvailable => write!( f, "No coin currently spendable through this timelocked recovery path." - ), - Self::InvalidDerivationIndex => write!(f, "Unhardened or overflowing BIP32 derivation index."), - Self::CoinSelectionError(e) => write!(f, "Coin selection error: '{}'", e), - Self::RbfError(e) => write!(f, "RBF error: '{}'.", e) + ), + Self::InvalidDerivationIndex => { + write!(f, "Unhardened or overflowing BIP32 derivation index.") + } + Self::RbfError(e) => write!(f, "RBF error: '{}'.", e), } } } impl std::error::Error for CommandError {} -// Sanity check the value of a transaction output. -fn check_output_value(value: bitcoin::Amount) -> Result<(), CommandError> { - // NOTE: the network parameter isn't used upstream - if value.to_sat() > bitcoin::blockdata::constants::MAX_MONEY - || value.to_sat() < DUST_OUTPUT_SATS - { - Err(CommandError::InvalidOutputValue(value)) - } else { - Ok(()) +impl From for CommandError { + fn from(e: SpendCreationError) -> Self { + CommandError::SpendCreation(e) } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum InsaneFeeInfo { - NegativeFee, - InvalidFeerate, - TooHighFee(u64), - TooHighFeerate(u64), -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RbfErrorInfo { MissingFeerate, @@ -196,80 +153,68 @@ impl fmt::Display for RbfErrorInfo { } } -/// A candidate for coin selection when creating a transaction. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct CandidateCoin { - /// The candidate coin. - coin: Coin, - /// Whether or not this coin must be selected by the coin selection algorithm. - must_select: bool, +/// A wallet transaction getter which fetches the transaction from our Bitcoin backend with a cache +/// to avoid needless redundant calls. Note the cache holds an Option<> so we also avoid redundant +/// calls when the txid isn't known by our Bitcoin backend. +struct BitcoindTxGetter<'a> { + bitcoind: &'a sync::Arc>, + cache: HashMap>, } -// Apply some sanity checks on a created transaction's PSBT. -// TODO: add more sanity checks from revault_tx -fn sanity_check_psbt( - spent_desc: &descriptors::LianaDescriptor, - psbt: &Psbt, -) -> Result<(), CommandError> { - let tx = &psbt.unsigned_tx; - - // Must have as many in/out in the PSBT and Bitcoin tx. - if psbt.inputs.len() != tx.input.len() - || psbt.outputs.len() != tx.output.len() - || tx.output.is_empty() - { - return Err(CommandError::SanityCheckFailure(psbt.clone())); - } - - // Compute the transaction input value, checking all PSBT inputs have the derivation - // index set for signing devices to recognize them as ours. - let mut value_in = 0; - for psbtin in psbt.inputs.iter() { - if psbtin.bip32_derivation.is_empty() { - return Err(CommandError::SanityCheckFailure(psbt.clone())); +impl<'a> BitcoindTxGetter<'a> { + pub fn new(bitcoind: &'a sync::Arc>) -> Self { + Self { + bitcoind, + cache: HashMap::new(), } - value_in += psbtin - .witness_utxo - .as_ref() - .ok_or_else(|| CommandError::SanityCheckFailure(psbt.clone()))? - .value; } +} - // Compute the output value and check the absolute fee isn't insane. - let value_out: u64 = tx.output.iter().map(|o| o.value).sum(); - let abs_fee = value_in - .checked_sub(value_out) - .ok_or(CommandError::InsaneFees(InsaneFeeInfo::NegativeFee))?; - if abs_fee > MAX_FEE { - return Err(CommandError::InsaneFees(InsaneFeeInfo::TooHighFee(abs_fee))); +impl<'a> TxGetter for BitcoindTxGetter<'a> { + fn get_tx(&mut self, txid: &bitcoin::Txid) -> Option { + if let hash_map::Entry::Vacant(entry) = self.cache.entry(*txid) { + entry.insert(self.bitcoind.wallet_transaction(txid).map(|wtx| wtx.0)); + } + self.cache.get(txid).cloned().flatten() } +} - // Check the feerate isn't insane. - // Add weights together before converting to vbytes to avoid rounding up multiple times - // and increasing the result, which could lead to the feerate in sats/vb falling below 1. - let tx_wu = tx.weight().to_wu() + (spent_desc.max_sat_weight() * tx.input.len()) as u64; - let tx_vb = tx_wu - .checked_add(descriptors::WITNESS_FACTOR as u64 - 1) - .unwrap() - .checked_div(descriptors::WITNESS_FACTOR as u64) +/// An unsigned transaction's maximum possible size in vbytes after satisfaction. +/// +/// This assumes all inputs are internal (or have the same `max_sat_weight` value). +/// +/// `tx` is the unsigned transaction. +/// +/// `max_sat_weight` is the maximum weight difference of an input in the +/// transaction before and after satisfaction. Must be in weight units. +fn unsigned_tx_max_vbytes(tx: &bitcoin::Transaction, max_sat_weight: u64) -> u64 { + let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap(); + let num_inputs: u64 = tx.input.len().try_into().unwrap(); + let tx_wu: u64 = tx + .weight() + .to_wu() + .checked_add(max_sat_weight.checked_mul(num_inputs).unwrap()) .unwrap(); - let feerate_sats_vb = abs_fee - .checked_div(tx_vb) - .ok_or(CommandError::InsaneFees(InsaneFeeInfo::InvalidFeerate))?; - if !(1..=MAX_FEERATE).contains(&feerate_sats_vb) { - return Err(CommandError::InsaneFees(InsaneFeeInfo::TooHighFeerate( - feerate_sats_vb, - ))); - } + tx_wu + .checked_add(witness_factor.checked_sub(1).unwrap()) + .unwrap() + .checked_div(witness_factor) + .unwrap() +} - // Check for dust outputs - for txo in psbt.unsigned_tx.output.iter() { - if txo.value < txo.script_pubkey.dust_value().to_sat() { - return Err(CommandError::SanityCheckFailure(psbt.clone())); - } +fn coin_to_candidate( + coin: &Coin, + must_select: bool, + sequence: Option, +) -> CandidateCoin { + CandidateCoin { + outpoint: coin.outpoint, + amount: coin.amount, + deriv_index: coin.derivation_index, + is_change: coin.is_change, + must_select, + sequence, } - - Ok(()) } impl DaemonControl { @@ -293,6 +238,69 @@ impl DaemonControl { addr.require_network(self.config.bitcoin_config.network) .map_err(CommandError::Address) } + + // Get details about this address, if we know about it. + fn addr_info( + &self, + db_conn: &mut Box, + addr: &bitcoin::Address, + ) -> Option { + db_conn + .derivation_index_by_address(addr) + .map(|(index, is_change)| AddrInfo { index, is_change }) + } + + // Create an address to be used in an output of a spend transaction. + fn spend_addr( + &self, + db_conn: &mut Box, + addr: bitcoin::Address, + ) -> SpendOutputAddress { + SpendOutputAddress { + info: self.addr_info(db_conn, &addr), + addr, + } + } + + // Get the change address for the next derivation index. + fn next_change_addr(&self, db_conn: &mut Box) -> SpendOutputAddress { + let index = db_conn.change_index(); + let desc = self + .config + .main_descriptor + .change_descriptor() + .derive(index, &self.secp); + let addr = desc.address(self.config.bitcoin_config.network); + SpendOutputAddress { + addr, + info: Some(AddrInfo { + index, + is_change: true, + }), + } + } + + // If we detect the given address as ours, and it has a higher derivation index than our next + // derivation index, update our next derivation index to the one after the address'. + fn maybe_increase_next_deriv_index( + &self, + db_conn: &mut Box, + addr_info: &Option, + ) { + if let Some(AddrInfo { index, is_change }) = addr_info { + if *is_change && db_conn.change_index() < *index { + let next_index = index + .increment() + .expect("Must not get into hardened territory"); + db_conn.set_change_index(next_index, &self.secp); + } else if !is_change && db_conn.receive_index() < *index { + let next_index = index + .increment() + .expect("Must not get into hardened territory"); + db_conn.set_receive_index(next_index, &self.secp); + } + } + } } impl DaemonControl { @@ -362,7 +370,7 @@ impl DaemonControl { }; // Derive all receive and change addresses for the queried range. - let addresses: Result, _> = (start_index_u32..end_index) + let addresses: Result, CommandError> = (start_index_u32..end_index) .map(|index| { let child = bip32::ChildNumber::from_normal_idx(index) .map_err(|_| CommandError::InvalidDerivationIndex)?; @@ -448,20 +456,27 @@ impl DaemonControl { return Err(CommandError::InvalidFeerate(feerate_vb)); } let mut db_conn = self.db.connection(); + let mut tx_getter = BitcoindTxGetter::new(&self.bitcoin); - // Check the destination addresses are valid for the network and - // sanity check each output's value. - let mut destinations_checked = HashMap::with_capacity(destinations.len()); + // Prepare the destination addresses. + let mut destinations_checked = Vec::with_capacity(destinations.len()); for (address, value_sat) in destinations { let address = self.validate_address(address.clone())?; let amount = bitcoin::Amount::from_sat(*value_sat); - check_output_value(amount)?; - destinations_checked.insert(address, amount); + let address = self.spend_addr(&mut db_conn, address); + destinations_checked.push((address, amount)); } - // Check also the change address if one has been given. + + // The change address to be used if a change output needs to be created. It may be + // specified by the caller (for instance for the purpose of a sweep, or to avoid us + // creating a new change address on every call). let change_address = change_address - .map(|addr| self.validate_address(addr)) - .transpose()?; + .map(|addr| { + Ok::<_, CommandError>(self.spend_addr(&mut db_conn, self.validate_address(addr)?)) + }) + .transpose()? + .unwrap_or_else(|| self.next_change_addr(&mut db_conn)); + // The candidate coins will be either all optional or all mandatory. // If no coins have been specified, then coins will be selected automatically for // the spend from a set of optional candidates. @@ -472,9 +487,8 @@ impl DaemonControl { db_conn .coins(&[CoinStatus::Confirmed], &[]) .into_values() - .map(|c| CandidateCoin { - coin: c, - must_select: false, // No coin is mandatory. + .map(|c| { + coin_to_candidate(&c, /*must_select=*/ false, /*sequence=*/ None) }) .collect() } else { @@ -491,20 +505,31 @@ impl DaemonControl { } coins .into_values() - .map(|c| CandidateCoin { - coin: c, - must_select: true, // All coins must be selected. - }) + .map(|c| coin_to_candidate(&c, /*must_select=*/ true, /*sequence=*/ None)) .collect() }; - self.create_spend_internal( + // Create the PSBT. If there was no error in doing so make sure to update our next + // derivation index in case any address in the transaction outputs was ours and from the + // future. + let change_info = change_address.info; + let CreateSpendRes { psbt, has_change } = create_spend( + &self.config.main_descriptor, + &self.secp, + &mut tx_getter, &destinations_checked, &candidate_coins, - feerate_vb, - 0, // No min fee required. + SpendTxFees::Regular(feerate_vb), change_address, - ) + )?; + for (addr, _) in destinations_checked { + self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info); + } + if has_change { + self.maybe_increase_next_deriv_index(&mut db_conn, &change_info); + } + + Ok(CreateSpendResult { psbt }) } pub fn update_spend(&self, mut psbt: Psbt) -> Result<(), CommandError> { @@ -559,235 +584,6 @@ impl DaemonControl { Ok(()) } - fn create_spend_internal( - &self, - destinations: &HashMap, - candidate_coins: &[CandidateCoin], - feerate_vb: u64, - min_fee: u64, - change_address: Option, - ) -> Result { - // This method is a bit convoluted, but it's the nature of creating a Bitcoin transaction - // with a target feerate and outputs. In addition, we support different modes (coin control - // vs automated coin selection, self-spend, sweep, etc..) which make the logic a bit more - // intricate. Here is a brief overview of what we're doing here: - // 1. Create a transaction with all the target outputs (if this is a self-send, none are - // added at this step the only output will be added as a change output). - // 2. Automatically select the coins if necessary and determine whether a change output - // will be necessary for this transaction from the set of (automatically or manually) - // selected coins. The output for a self-send is added there. - // The change output is also (ab)used to implement a "sweep" functionality. We allow to - // set it to an external address to send all the inputs' value minus the fee and the - // other output's value to a specific, external, address. - // 3. Add the selected coins as inputs to the transaction. - // 4. Finalize the PSBT and sanity check it before returning it. - - let is_self_send = destinations.is_empty(); - if feerate_vb < 1 { - return Err(CommandError::InvalidFeerate(feerate_vb)); - } - let mut db_conn = self.db.connection(); - - // Create transaction with no inputs and no outputs. - let mut tx = bitcoin::Transaction { - version: 2, - lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), // TODO: randomized anti fee sniping - input: Vec::with_capacity(candidate_coins.iter().filter(|c| c.must_select).count()), - output: Vec::with_capacity(destinations.len()), - }; - // Add the destinations outputs to the transaction and PSBT. At the same time - // sanity check each output's value. - let mut psbt_outs = Vec::with_capacity(destinations.len()); - for (address, &amount) in destinations { - check_output_value(amount)?; - - tx.output.push(bitcoin::TxOut { - value: amount.to_sat(), - script_pubkey: address.script_pubkey(), - }); - // If it's an address of ours, signal it as change to signing devices by adding the - // BIP32 derivation path to the PSBT output. - let bip32_derivation = - if let Some((index, is_change)) = db_conn.derivation_index_by_address(address) { - let desc = if is_change { - self.config.main_descriptor.change_descriptor() - } else { - self.config.main_descriptor.receive_descriptor() - }; - desc.derive(index, &self.secp).bip32_derivations() - } else { - Default::default() - }; - psbt_outs.push(PsbtOut { - bip32_derivation, - ..PsbtOut::default() - }); - } - assert_eq!(tx.output.is_empty(), is_self_send); - - // Now compute whether we'll need a change output while automatically selecting coins to be - // used as input if necessary. - // We need to get the size of a potential change output to select coins / determine whether - // we should include one, so get the change address and create a dummy txo for this purpose. - // The change address may be externally specified for the purpose of a "sweep": the user - // would set the value of some outputs (or none) and fill-in an address to be used for "all - // the rest". This is the same logic as for a change output, except it's external. - struct InternalChangeInfo { - pub desc: descriptors::DerivedSinglePathLianaDesc, - pub index: bip32::ChildNumber, - } - let (change_addr, int_change_info) = if let Some(addr) = change_address { - (addr, None) - } else { - let index = db_conn.change_index(); - let desc = self - .config - .main_descriptor - .change_descriptor() - .derive(index, &self.secp); - ( - desc.address(self.config.bitcoin_config.network), - Some(InternalChangeInfo { desc, index }), - ) - }; - let mut change_txo = bitcoin::TxOut { - value: std::u64::MAX, - script_pubkey: change_addr.script_pubkey(), - }; - // Now select the coins necessary using the provided candidates and determine whether - // there is any leftover to create a change output. - let (selected_coins, change_amount) = { - // At this point the transaction still has no input and no change output, as expected - // by the coins selection helper function. - assert!(tx.input.is_empty()); - assert_eq!(tx.output.len(), destinations.len()); - // TODO: Introduce general conversion error type. - let feerate_vb: f32 = { - let fr: u16 = feerate_vb.try_into().map_err(|_| { - CommandError::InsaneFees(InsaneFeeInfo::TooHighFeerate(feerate_vb)) - })?; - fr - } - .try_into() - .expect("u16 must fit in f32"); - let max_sat_wu = self - .config - .main_descriptor - .max_sat_weight() - .try_into() - .expect("Weight must fit in a u32"); - select_coins_for_spend( - candidate_coins, - tx.clone(), - change_txo.clone(), - feerate_vb, - min_fee, - max_sat_wu, - is_self_send, - ) - .map_err(CommandError::CoinSelectionError)? - }; - // If necessary, add a change output. - // For a self-send, coin selection will only find solutions with change and will otherwise - // return an error. In any case, the PSBT sanity check will catch a transaction with no outputs. - if change_amount.to_sat() > 0 { - check_output_value(change_amount)?; - - // If we generated a change address internally, set the BIP32 derivations in the PSBT - // output to tell the signers it's an internal address and make sure to update our next - // change index. Otherwise it's a sweep, so no need to set anything. - // If the change address was set by the caller, check whether it's one of ours. If it - // is, set the BIP32 derivations accordingly. In addition, if it's a change address for - // a later index than we currently have set as next change derivation index, update it. - let bip32_derivation = if let Some(InternalChangeInfo { desc, index }) = int_change_info - { - let next_index = index - .increment() - .expect("Must not get into hardened territory"); - db_conn.set_change_index(next_index, &self.secp); - desc.bip32_derivations() - } else if let Some((index, is_change)) = - db_conn.derivation_index_by_address(&change_addr) - { - let desc = if is_change { - if db_conn.change_index() < index { - let next_index = index - .increment() - .expect("Must not get into hardened territory"); - db_conn.set_change_index(next_index, &self.secp); - } - self.config.main_descriptor.change_descriptor() - } else { - self.config.main_descriptor.receive_descriptor() - }; - desc.derive(index, &self.secp).bip32_derivations() - } else { - Default::default() - }; - - // TODO: shuffle once we have Taproot - change_txo.value = change_amount.to_sat(); - tx.output.push(change_txo); - psbt_outs.push(PsbtOut { - bip32_derivation, - ..PsbtOut::default() - }); - } - - // Iterate through selected coins and add necessary information to the PSBT inputs. - let mut psbt_ins = Vec::with_capacity(selected_coins.len()); - let mut spent_txs = HashMap::with_capacity(selected_coins.len()); - for coin in &selected_coins { - // Fetch the transaction that created it if necessary - if let hash_map::Entry::Vacant(e) = spent_txs.entry(coin.outpoint) { - let tx = self - .bitcoin - .wallet_transaction(&coin.outpoint.txid) - .ok_or(CommandError::FetchingTransaction(coin.outpoint))?; - e.insert(tx.0); - } - - tx.input.push(bitcoin::TxIn { - previous_output: coin.outpoint, - sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, - // TODO: once we move to Taproot, anti-fee-sniping using nSequence - ..bitcoin::TxIn::default() - }); - - // Populate the PSBT input with the information needed by signers. - let coin_desc = self.derived_desc(coin); - let witness_script = Some(coin_desc.witness_script()); - let witness_utxo = Some(bitcoin::TxOut { - value: coin.amount.to_sat(), - script_pubkey: coin_desc.script_pubkey(), - }); - let non_witness_utxo = spent_txs.get(&coin.outpoint).cloned(); - let bip32_derivation = coin_desc.bip32_derivations(); - psbt_ins.push(PsbtIn { - witness_script, - witness_utxo, - bip32_derivation, - non_witness_utxo, - ..PsbtIn::default() - }); - } - - // Finally, create the PSBT with all inputs and outputs, sanity check it and return it. - let psbt = Psbt { - unsigned_tx: tx, - version: 0, - xpub: BTreeMap::new(), - proprietary: BTreeMap::new(), - unknown: BTreeMap::new(), - inputs: psbt_ins, - outputs: psbt_outs, - }; - sanity_check_psbt(&self.config.main_descriptor, &psbt)?; - // TODO: maybe check for common standardness rules (max size, ..)? - - Ok(CreateSpendResult { psbt }) - } - pub fn update_labels(&self, items: &HashMap>) { let mut db_conn = self.db.connection(); db_conn.update_labels(items); @@ -869,6 +665,7 @@ impl DaemonControl { feerate_vb: Option, ) -> Result { let mut db_conn = self.db.connection(); + let mut tx_getter = BitcoindTxGetter::new(&self.bitcoin); if is_cancel && feerate_vb.is_some() { return Err(CommandError::RbfError(RbfErrorInfo::SuperfluousFeerate)); @@ -981,37 +778,30 @@ impl DaemonControl { .into_iter() .filter_map(|(addr, amt, _)| { if prev_change_address.as_ref() != Some(&addr) { - Some((addr, amt)) + Some((self.spend_addr(&mut db_conn, addr), amt)) } else { None } }) .collect() } else { - HashMap::new() + Vec::new() }; // If there was no previous change address, we set the change address for the replacement // to our next change address. This way, we won't increment the change index with each attempt // at creating the replacement PSBT below. - let change_address = prev_change_address.unwrap_or_else(|| { - let index = db_conn.change_index(); - let desc = self - .config - .main_descriptor - .change_descriptor() - .derive(index, &self.secp); - desc.address(self.config.bitcoin_config.network) - }); + let change_address = prev_change_address + .map(|addr| self.spend_addr(&mut db_conn, addr)) + .unwrap_or_else(|| self.next_change_addr(&mut db_conn)); // If `!is_cancel`, we take the previous coins as mandatory candidates and add confirmed coins as optional. // Otherwise, we take the previous coins as optional candidates and let coin selection find the // best solution that includes at least one of these. If there are insufficient funds to create the replacement // transaction in this way, then we set candidates in the same way as for the `!is_cancel` case. let mut candidate_coins: Vec = prev_coins .values() - .map(|c| CandidateCoin { - coin: *c, - must_select: !is_cancel, + .map(|c| { + coin_to_candidate(c, /*must_select=*/ !is_cancel, /*sequence=*/ None) }) .collect(); let confirmed_cands: Vec = db_conn @@ -1021,10 +811,9 @@ impl DaemonControl { // Make sure we don't have duplicate candidates in case any of the coins are not // currently set as spending in the DB (and are therefore still confirmed). if !prev_coins.contains_key(&c.outpoint) { - Some(CandidateCoin { - coin: c, - must_select: false, - }) + Some(coin_to_candidate( + &c, /*must_select=*/ false, /*sequence=*/ None, + )) } else { None } @@ -1046,18 +835,23 @@ impl DaemonControl { let mut replacement_vsize = 0; for incremental_feerate in 0.. { let min_fee = descendant_fees.to_sat() + replacement_vsize * incremental_feerate; - let rbf_psbt = match self.create_spend_internal( + let CreateSpendRes { + psbt: rbf_psbt, + has_change, + } = match create_spend( + &self.config.main_descriptor, + &self.secp, + &mut tx_getter, &destinations, &candidate_coins, - feerate_vb, - min_fee, - Some(change_address.clone()), + SpendTxFees::Rbf(feerate_vb, min_fee), + change_address.clone(), ) { Ok(psbt) => psbt, // If we get a coin selection error due to insufficient funds and we want to cancel the // transaction, then set all previous coins as mandatory and add confirmed coins as // optional, unless we have already done this. - Err(CommandError::CoinSelectionError(_)) + Err(SpendCreationError::CoinSelection(_)) if is_cancel && candidate_coins.iter().all(|c| !c.must_select) => { for cand in candidate_coins.iter_mut() { @@ -1067,19 +861,25 @@ impl DaemonControl { continue; } Err(e) => { - return Err(e); + return Err(e.into()); } }; - replacement_vsize = unsigned_tx_max_vbytes(&rbf_psbt.psbt.unsigned_tx, max_sat_weight); + replacement_vsize = unsigned_tx_max_vbytes(&rbf_psbt.unsigned_tx, max_sat_weight); // Make sure it satisfies RBF rule 4. - if rbf_psbt - .psbt - .fee() - .expect("has already been sanity checked") + if rbf_psbt.fee().expect("has already been sanity checked") >= descendant_fees + bitcoin::Amount::from_sat(replacement_vsize) { - return Ok(rbf_psbt); + // In case of success, make sure to update our next derivation index if any address + // used in the transaction outputs was from the future. + for (addr, _) in destinations { + self.maybe_increase_next_deriv_index(&mut db_conn, &addr.info); + } + if has_change { + self.maybe_increase_next_deriv_index(&mut db_conn, &change_address.info); + } + + return Ok(CreateSpendResult { psbt: rbf_psbt }); } } @@ -1176,27 +976,9 @@ impl DaemonControl { if feerate_vb < 1 { return Err(CommandError::InvalidFeerate(feerate_vb)); } - let address = self.validate_address(address)?; + let mut tx_getter = BitcoindTxGetter::new(&self.bitcoin); let mut db_conn = self.db.connection(); - - // The transaction template. We'll fill-in the inputs afterward. - let mut psbt = Psbt { - unsigned_tx: bitcoin::Transaction { - version: 2, - lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), // TODO: anti-fee sniping - input: Vec::new(), - output: vec![bitcoin::TxOut { - script_pubkey: address.script_pubkey(), - value: 0xFF_FF_FF_FF, - }], - }, - version: 0, - xpub: BTreeMap::new(), - proprietary: BTreeMap::new(), - unknown: BTreeMap::new(), - inputs: Vec::new(), - outputs: vec![PsbtOut::default()], - }; + let sweep_addr = self.spend_addr(&mut db_conn, self.validate_address(address)?); // Query the coins that we can spend through the specified recovery path (if no recovery // path specified, use the first available one) from the database. @@ -1204,72 +986,42 @@ impl DaemonControl { let timelock = timelock.unwrap_or_else(|| self.config.main_descriptor.first_timelock_value()); let height_delta: i32 = timelock.try_into().expect("Must fit, it's a u16"); - let sweepable_coins = db_conn - .coins(&[CoinStatus::Unconfirmed, CoinStatus::Confirmed], &[]) + let sweepable_coins: Vec<_> = db_conn + .coins(&[CoinStatus::Confirmed], &[]) .into_values() - .filter(|c| { + .filter_map(|c| { // We are interested in coins available at the *next* block - c.block_info + if c.block_info .map(|b| current_height + 1 >= b.height + height_delta) .unwrap_or(false) - }); - - // Fill-in the transaction inputs and PSBT inputs information. Record the value - // that is fed to the transaction while doing so, to compute the fees afterward. - let mut in_value = bitcoin::Amount::from_sat(0); - let txin_sat_vb = self.config.main_descriptor.max_sat_vbytes(); - let mut sat_vb = 1; // Start at 1 for the segwit marker size, rounded up. - let mut spent_txs = HashMap::new(); - for coin in sweepable_coins { - in_value += coin.amount; - psbt.unsigned_tx.input.push(bitcoin::TxIn { - previous_output: coin.outpoint, - sequence: bitcoin::Sequence::from_height(timelock), - // TODO: once we move to Taproot, anti-fee-sniping using nSequence - ..bitcoin::TxIn::default() - }); - - // Fetch the transaction that created this coin if necessary - if let hash_map::Entry::Vacant(e) = spent_txs.entry(coin.outpoint) { - let tx = self - .bitcoin - .wallet_transaction(&coin.outpoint.txid) - .ok_or(CommandError::FetchingTransaction(coin.outpoint))?; - e.insert(tx.0); - } - - let coin_desc = self.derived_desc(&coin); - sat_vb += txin_sat_vb; - let witness_script = Some(coin_desc.witness_script()); - let witness_utxo = Some(bitcoin::TxOut { - value: coin.amount.to_sat(), - script_pubkey: coin_desc.script_pubkey(), - }); - let non_witness_utxo = spent_txs.get(&coin.outpoint).cloned(); - let bip32_derivation = coin_desc.bip32_derivations(); - psbt.inputs.push(PsbtIn { - witness_script, - witness_utxo, - non_witness_utxo, - bip32_derivation, - ..PsbtIn::default() - }); - } - - // The sweepable_coins iterator may have been empty. - if psbt.unsigned_tx.input.is_empty() { + { + Some(coin_to_candidate( + &c, + /*must_select=*/ true, + /*sequence=*/ Some(bitcoin::Sequence::from_height(timelock)), + )) + } else { + None + } + }) + .collect(); + if sweepable_coins.is_empty() { return Err(CommandError::RecoveryNotAvailable); } - // Compute the value of the single output based on the requested feerate. - let tx_vbytes = (psbt.unsigned_tx.vsize() + sat_vb) as u64; - let absolute_fee = bitcoin::Amount::from_sat(tx_vbytes.checked_mul(feerate_vb).unwrap()); - let output_value = in_value - .checked_sub(absolute_fee) - .ok_or(CommandError::InsufficientFunds(in_value, None, feerate_vb))?; - psbt.unsigned_tx.output[0].value = output_value.to_sat(); - - sanity_check_psbt(&self.config.main_descriptor, &psbt)?; + let sweep_addr_info = sweep_addr.info; + let CreateSpendRes { psbt, has_change } = create_spend( + &self.config.main_descriptor, + &self.secp, + &mut tx_getter, + &[], // No destination, only the change address. + &sweepable_coins, + SpendTxFees::Regular(feerate_vb), + sweep_addr, + )?; + if has_change { + self.maybe_increase_next_deriv_index(&mut db_conn, &sweep_addr_info); + } Ok(CreateRecoveryResult { psbt }) } @@ -1403,7 +1155,7 @@ pub struct CreateRecoveryResult { #[cfg(test)] mod tests { use super::*; - use crate::{bitcoin::Block, database::BlockInfo, testutils::*}; + use crate::{bitcoin::Block, database::BlockInfo, spend::InsaneFeeInfo, testutils::*}; use bitcoin::{ bip32::{self, ChildNumber}, @@ -1411,7 +1163,7 @@ mod tests { locktime::absolute, OutPoint, ScriptBuf, Sequence, Transaction, Txid, Witness, }; - use std::str::FromStr; + use std::{collections::BTreeMap, str::FromStr}; #[test] fn getinfo() { @@ -1596,7 +1348,9 @@ mod tests { // Insufficient funds for coin selection. assert!(matches!( control.create_spend(&destinations, &[], 1, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); assert_eq!( control.create_spend(&destinations, &[dummy_op], 0, None), @@ -1624,7 +1378,9 @@ mod tests { // and so we get a coin selection error due to insufficient funds. assert!(matches!( control.create_spend(&destinations, &[], 1, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); let res = control .create_spend(&destinations, &[dummy_op], 1, None) @@ -1658,19 +1414,23 @@ mod tests { // If we ask for a too high feerate, or a too large/too small output, it'll fail. assert!(matches!( control.create_spend(&destinations, &[dummy_op], 10_000, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); *destinations.get_mut(&dummy_addr).unwrap() = 100_001; assert!(matches!( control.create_spend(&destinations, &[dummy_op], 1, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); *destinations.get_mut(&dummy_addr).unwrap() = 4_500; assert_eq!( control.create_spend(&destinations, &[dummy_op], 1, None), - Err(CommandError::InvalidOutputValue(bitcoin::Amount::from_sat( - 4_500 - ))) + Err(CommandError::SpendCreation( + SpendCreationError::InvalidOutputValue(bitcoin::Amount::from_sat(4_500)) + )) ); // If we ask to create an output for an address from another network, it will fail. @@ -1718,7 +1478,9 @@ mod tests { // and so we get a coin selection error due to insufficient funds. assert!(matches!( control.create_spend(&destinations, &[], 1, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); // We'd bail out if they tried to create a transaction with a too high feerate. @@ -1742,8 +1504,8 @@ mod tests { // the sats/vb feerate being lower than `feerate_vb`. assert_eq!( control.create_spend(&destinations, &[dummy_op_dup], 1_003, None), - Err(CommandError::InsaneFees(InsaneFeeInfo::TooHighFeerate( - 1_001 + Err(CommandError::SpendCreation(SpendCreationError::InsaneFees( + InsaneFeeInfo::TooHighFeerate(1_001) ))) ); @@ -1768,14 +1530,18 @@ mod tests { // Coin selection error due to insufficient funds. assert!(matches!( control.create_spend(&destinations, &[], 1, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); // Set destination amount equal to value of confirmed coins. *destinations.get_mut(&dummy_addr).unwrap() = 80_000; // Coin selection error occurs due to insufficient funds to pay fee. assert!(matches!( control.create_spend(&destinations, &[], 1, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); let confirmed_op_2 = bitcoin::OutPoint { txid: confirmed_op_1.txid, @@ -1850,7 +1616,9 @@ mod tests { let empty_dest = &HashMap::, u64>::new(); assert!(matches!( control.create_spend(empty_dest, &[confirmed_op_3], 5, None), - Err(CommandError::CoinSelectionError(..)) + Err(CommandError::SpendCreation( + SpendCreationError::CoinSelection(..) + )) )); // If we use a lower fee, the self-send will succeed. let res = control diff --git a/src/commands/utils.rs b/src/commands/utils.rs index d5f5e561..ce0e2644 100644 --- a/src/commands/utils.rs +++ b/src/commands/utils.rs @@ -1,17 +1,8 @@ -use bdk_coin_select::{ - change_policy, metrics::LowestFee, Candidate, CoinSelector, DrainWeights, FeeRate, - InsufficientFunds, Target, TXIN_BASE_WEIGHT, -}; -use log::warn; -use std::{convert::TryInto, str::FromStr}; +use std::str::FromStr; -use miniscript::bitcoin::{self, consensus, constants::WITNESS_SCALE_FACTOR, hashes::hex::FromHex}; +use miniscript::bitcoin::{self, consensus, hashes::hex::FromHex}; use serde::{de, Deserialize, Deserializer, Serializer}; -use crate::database::Coin; - -use super::{CandidateCoin, DUST_OUTPUT_SATS, LONG_TERM_FEERATE_VB}; - pub fn deser_fromstr<'de, D, T>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -71,199 +62,3 @@ where let s = Vec::from_hex(&s).map_err(de::Error::custom)?; consensus::deserialize(&s).map_err(de::Error::custom) } - -/// Metric based on [`LowestFee`] that aims to minimize transaction fees -/// with the additional option to only find solutions with a change output. -/// -/// Using this metric with `must_have_change: false` is equivalent to using -/// [`LowestFee`]. -pub struct LowestFeeChangeCondition<'c, C> { - /// The underlying [`LowestFee`] metric to use. - pub lowest_fee: LowestFee<'c, C>, - /// If `true`, only solutions with change will be found. - pub must_have_change: bool, -} - -impl<'c, C> bdk_coin_select::BnbMetric for LowestFeeChangeCondition<'c, C> -where - for<'a, 'b> C: Fn(&'b CoinSelector<'a>, Target) -> bdk_coin_select::Drain, -{ - fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - let drain = (self.lowest_fee.change_policy)(cs, self.lowest_fee.target); - if drain.is_none() && self.must_have_change { - None - } else { - self.lowest_fee.score(cs) - } - } - - fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { - self.lowest_fee.bound(cs) - } - - fn requires_ordering_by_descending_value_pwu(&self) -> bool { - self.lowest_fee.requires_ordering_by_descending_value_pwu() - } -} - -/// Select coins for spend. -/// -/// Returns the selected coins and the change amount, which could be zero. -/// -/// `candidate_coins` are the coins to consider for selection. -/// -/// `base_tx` is the transaction to select coins for. It should be without any inputs -/// and without a change output, but with all non-change outputs added. -/// -/// `change_txo` is the change output to add if needed (with any value). -/// -/// `feerate_vb` is the minimum feerate (in sats/vb). Note that the selected coins -/// and change may result in a slightly lower feerate than this as the underlying -/// function instead uses a minimum feerate of `feerate_vb / 4.0` sats/wu. -/// -/// `min_fee` is the minimum fee (in sats) that the selection must have. -/// -/// `max_sat_weight` is the maximum weight difference of an input in the -/// transaction before and after satisfaction. -/// -/// `must_have_change` indicates whether the transaction must have a change output. -/// If `true`, the returned change amount will be positive. -pub fn select_coins_for_spend( - candidate_coins: &[CandidateCoin], - base_tx: bitcoin::Transaction, - change_txo: bitcoin::TxOut, - feerate_vb: f32, - min_fee: u64, - max_sat_weight: u32, - must_have_change: bool, -) -> Result<(Vec, bitcoin::Amount), InsufficientFunds> { - let out_value_nochange = base_tx.output.iter().map(|o| o.value).sum(); - - // Create the coin selector from the given candidates. NOTE: the coin selector keeps track - // of the original ordering of candidates so we can select any mandatory candidates using their - // original indices. - let base_weight: u32 = base_tx - .weight() - .to_wu() - .try_into() - .expect("Transaction weight must fit in u32"); - let max_input_weight = TXIN_BASE_WEIGHT + max_sat_weight; - let candidates: Vec = candidate_coins - .iter() - .map(|cand| Candidate { - input_count: 1, - value: cand.coin.amount.to_sat(), - weight: max_input_weight, - is_segwit: true, // We only support receiving on Segwit scripts. - }) - .collect(); - let mut selector = CoinSelector::new(&candidates, base_weight); - for (i, cand) in candidate_coins.iter().enumerate() { - if cand.must_select { - // It's fine because the index passed to `select` refers to the original candidates ordering - // (and in any case the ordering of candidates is still the same in the coin selector). - selector.select(i); - } - } - - // Now set the change policy. We use a policy which ensures no change output is created with a - // lower value than our custom dust limit. NOTE: the change output weight must account for a - // potential difference in the size of the outputs count varint. This is why we take the whole - // change txo as argument and compute the weight difference below. - let long_term_feerate = FeeRate::from_sat_per_vb(LONG_TERM_FEERATE_VB); - let drain_weights = DrainWeights { - output_weight: { - let mut tx_with_change = base_tx; - tx_with_change.output.push(change_txo); - tx_with_change - .weight() - .to_wu() - .checked_sub(base_weight.into()) - .expect("base_weight can't be larger") - .try_into() - .expect("tx size must always fit in u32") - }, - spend_weight: max_input_weight, - }; - let change_policy = - change_policy::min_value_and_waste(drain_weights, DUST_OUTPUT_SATS, long_term_feerate); - - // Finally, run the coin selection algorithm. We use a BnB with 100k iterations and if it - // couldn't find any solution we fall back to selecting coins by descending value. - let target = Target { - value: out_value_nochange, - feerate: FeeRate::from_sat_per_vb(feerate_vb), - min_fee, - }; - let lowest_fee = LowestFee { - target, - long_term_feerate, - change_policy: &change_policy, - }; - let lowest_fee_change_cond = LowestFeeChangeCondition { - lowest_fee, - must_have_change, - }; - if let Err(e) = selector.run_bnb(lowest_fee_change_cond, 100_000) { - warn!( - "Coin selection error: '{}'. Selecting coins by descending value per weight unit...", - e.to_string() - ); - selector.sort_candidates_by_descending_value_pwu(); - // Select more coins until target is met and change condition satisfied. - loop { - let drain = change_policy(&selector, target); - if selector.is_target_met(target, drain) && (drain.is_some() || !must_have_change) { - break; - } - if !selector.select_next() { - // If the solution must have change, we calculate how much is missing from the current - // selection in order for there to be a change output with the smallest possible value. - let drain = if must_have_change { - bdk_coin_select::Drain { - weights: drain_weights, - value: DUST_OUTPUT_SATS, - } - } else { - drain - }; - let missing = selector.excess(target, drain).unsigned_abs(); - return Err(InsufficientFunds { missing }); - } - } - } - // By now, selection is complete and we can check how much change to give according to our policy. - let drain = change_policy(&selector, target); - let change_amount = bitcoin::Amount::from_sat(drain.value); - Ok(( - selector - .selected_indices() - .iter() - .map(|i| candidate_coins[*i].coin) - .collect(), - change_amount, - )) -} - -/// An unsigned transaction's maximum possible size in vbytes after satisfaction. -/// -/// This assumes all inputs are internal (or have the same `max_sat_weight` value). -/// -/// `tx` is the unsigned transaction. -/// -/// `max_sat_weight` is the maximum weight difference of an input in the -/// transaction before and after satisfaction. Must be in weight units. -pub fn unsigned_tx_max_vbytes(tx: &bitcoin::Transaction, max_sat_weight: u64) -> u64 { - let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap(); - let num_inputs: u64 = tx.input.len().try_into().unwrap(); - let tx_wu: u64 = tx - .weight() - .to_wu() - .checked_add(max_sat_weight.checked_mul(num_inputs).unwrap()) - .unwrap(); - tx_wu - .checked_add(witness_factor.checked_sub(1).unwrap()) - .unwrap() - .checked_div(witness_factor) - .unwrap() -} diff --git a/src/jsonrpc/mod.rs b/src/jsonrpc/mod.rs index bf0b9220..e21ac0e7 100644 --- a/src/jsonrpc/mod.rs +++ b/src/jsonrpc/mod.rs @@ -157,9 +157,8 @@ impl From for Error { | commands::CommandError::AlreadySpent(..) | commands::CommandError::ImmatureCoinbase(..) | commands::CommandError::Address(..) - | commands::CommandError::InvalidOutputValue(..) + | commands::CommandError::SpendCreation(..) | commands::CommandError::InsufficientFunds(..) - | commands::CommandError::InsaneFees(..) | commands::CommandError::UnknownSpend(..) | commands::CommandError::SpendFinalization(..) | commands::CommandError::InsaneRescanTimestamp(..) @@ -169,10 +168,7 @@ impl From for Error { | commands::CommandError::RecoveryNotAvailable => { Error::new(ErrorCode::InvalidParams, e.to_string()) } - commands::CommandError::FetchingTransaction(..) - | commands::CommandError::SanityCheckFailure(_) - | commands::CommandError::CoinSelectionError(..) - | commands::CommandError::RescanTrigger(..) => { + commands::CommandError::RescanTrigger(..) => { Error::new(ErrorCode::InternalError, e.to_string()) } commands::CommandError::TxBroadcast(_) => { diff --git a/src/lib.rs b/src/lib.rs index 5f58e04f..8d4068f3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,7 @@ pub mod descriptors; mod jsonrpc; mod random; pub mod signer; +mod spend; #[cfg(test)] mod testutils; diff --git a/src/spend.rs b/src/spend.rs new file mode 100644 index 00000000..1b89167a --- /dev/null +++ b/src/spend.rs @@ -0,0 +1,610 @@ +use crate::descriptors; + +use std::{collections::BTreeMap, convert::TryInto, fmt}; + +pub use bdk_coin_select::InsufficientFunds; +use bdk_coin_select::{ + change_policy, metrics::LowestFee, Candidate, CoinSelector, DrainWeights, FeeRate, Target, + TXIN_BASE_WEIGHT, +}; +use miniscript::bitcoin::{ + self, + absolute::{Height, LockTime}, + bip32, + constants::WITNESS_SCALE_FACTOR, + psbt::{Input as PsbtIn, Output as PsbtOut, Psbt}, + secp256k1, +}; + +/// We would never create a transaction with an output worth less than this. +/// That's 1$ at 20_000$ per BTC. +pub const DUST_OUTPUT_SATS: u64 = 5_000; + +/// Long-term feerate (sats/vb) used for coin selection considerations. +pub const LONG_TERM_FEERATE_VB: f32 = 10.0; + +/// Assume that paying more than 1BTC in fee is a bug. +pub const MAX_FEE: u64 = bitcoin::blockdata::constants::COIN_VALUE; + +/// Assume that paying more than 1000sat/vb in feerate is a bug. +pub const MAX_FEERATE: u64 = 1_000; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum InsaneFeeInfo { + NegativeFee, + InvalidFeerate, + TooHighFee(u64), + TooHighFeerate(u64), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SpendCreationError { + InvalidFeerate(/* sats/vb */ u64), + InvalidOutputValue(bitcoin::Amount), + InsaneFees(InsaneFeeInfo), + SanityCheckFailure(Psbt), + FetchingTransaction(bitcoin::OutPoint), + CoinSelection(InsufficientFunds), +} + +impl fmt::Display for SpendCreationError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::InvalidFeerate(sats_vb) => write!(f, "Invalid feerate: {} sats/vb.", sats_vb), + Self::InvalidOutputValue(amount) => write!(f, "Invalid output value '{}'.", amount), + Self::InsaneFees(info) => write!( + f, + "We assume transactions with a fee larger than {} sats or a feerate larger than {} sats/vb are a mistake. \ + The created transaction {}.", + MAX_FEE, + MAX_FEERATE, + match info { + InsaneFeeInfo::NegativeFee => "would have a negative fee".to_string(), + InsaneFeeInfo::TooHighFee(f) => format!("{} sats in fees", f), + InsaneFeeInfo::InvalidFeerate => "would have an invalid feerate".to_string(), + InsaneFeeInfo::TooHighFeerate(r) => format!("has a feerate of {} sats/vb", r), + }, + ), + Self::FetchingTransaction(op) => { + write!(f, "Could not fetch transaction for coin {}", op) + } + Self::CoinSelection(e) => write!(f, "Coin selection error: '{}'", e), + Self::SanityCheckFailure(psbt) => write!( + f, + "BUG! Please report this. Failed sanity checks for PSBT '{}'.", + psbt + ), + } + } +} + +impl std::error::Error for SpendCreationError {} + +// Sanity check the value of a transaction output. +fn check_output_value(value: bitcoin::Amount) -> Result<(), SpendCreationError> { + // NOTE: the network parameter isn't used upstream + if value.to_sat() > bitcoin::blockdata::constants::MAX_MONEY + || value.to_sat() < DUST_OUTPUT_SATS + { + Err(SpendCreationError::InvalidOutputValue(value)) + } else { + Ok(()) + } +} + +// Apply some sanity checks on a created transaction's PSBT. +// TODO: add more sanity checks from revault_tx +fn sanity_check_psbt( + spent_desc: &descriptors::LianaDescriptor, + psbt: &Psbt, +) -> Result<(), SpendCreationError> { + let tx = &psbt.unsigned_tx; + + // Must have as many in/out in the PSBT and Bitcoin tx. + if psbt.inputs.len() != tx.input.len() + || psbt.outputs.len() != tx.output.len() + || tx.output.is_empty() + { + return Err(SpendCreationError::SanityCheckFailure(psbt.clone())); + } + + // Compute the transaction input value, checking all PSBT inputs have the derivation + // index set for signing devices to recognize them as ours. + let mut value_in = 0; + for psbtin in psbt.inputs.iter() { + if psbtin.bip32_derivation.is_empty() { + return Err(SpendCreationError::SanityCheckFailure(psbt.clone())); + } + value_in += psbtin + .witness_utxo + .as_ref() + .ok_or_else(|| SpendCreationError::SanityCheckFailure(psbt.clone()))? + .value; + } + + // Compute the output value and check the absolute fee isn't insane. + let value_out: u64 = tx.output.iter().map(|o| o.value).sum(); + let abs_fee = value_in + .checked_sub(value_out) + .ok_or(SpendCreationError::InsaneFees(InsaneFeeInfo::NegativeFee))?; + if abs_fee > MAX_FEE { + return Err(SpendCreationError::InsaneFees(InsaneFeeInfo::TooHighFee( + abs_fee, + ))); + } + + // Check the feerate isn't insane. + // Add weights together before converting to vbytes to avoid rounding up multiple times + // and increasing the result, which could lead to the feerate in sats/vb falling below 1. + let tx_wu = tx.weight().to_wu() + (spent_desc.max_sat_weight() * tx.input.len()) as u64; + let tx_vb = tx_wu + .checked_add(WITNESS_SCALE_FACTOR as u64 - 1) + .unwrap() + .checked_div(WITNESS_SCALE_FACTOR as u64) + .unwrap(); + let feerate_sats_vb = abs_fee + .checked_div(tx_vb) + .ok_or(SpendCreationError::InsaneFees( + InsaneFeeInfo::InvalidFeerate, + ))?; + if !(1..=MAX_FEERATE).contains(&feerate_sats_vb) { + return Err(SpendCreationError::InsaneFees( + InsaneFeeInfo::TooHighFeerate(feerate_sats_vb), + )); + } + + // Check for dust outputs + for txo in psbt.unsigned_tx.output.iter() { + if txo.value < txo.script_pubkey.dust_value().to_sat() { + return Err(SpendCreationError::SanityCheckFailure(psbt.clone())); + } + } + + Ok(()) +} + +/// A candidate for coin selection when creating a transaction. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct CandidateCoin { + /// Unique identifier of this coin. + pub outpoint: bitcoin::OutPoint, + /// The value of this coin. + pub amount: bitcoin::Amount, + /// The derivation index used to generate the scriptpubkey of this coin. + pub deriv_index: bip32::ChildNumber, + /// Whether this coin pays to a scriptpubkey derived from the internal keychain. + pub is_change: bool, + /// Whether or not this coin must be selected by the coin selection algorithm. + pub must_select: bool, + /// The nSequence field to set for an input spending this coin. + pub sequence: Option, +} + +/// Metric based on [`LowestFee`] that aims to minimize transaction fees +/// with the additional option to only find solutions with a change output. +/// +/// Using this metric with `must_have_change: false` is equivalent to using +/// [`LowestFee`]. +struct LowestFeeChangeCondition<'c, C> { + /// The underlying [`LowestFee`] metric to use. + pub lowest_fee: LowestFee<'c, C>, + /// If `true`, only solutions with change will be found. + pub must_have_change: bool, +} + +impl<'c, C> bdk_coin_select::BnbMetric for LowestFeeChangeCondition<'c, C> +where + for<'a, 'b> C: Fn(&'b CoinSelector<'a>, Target) -> bdk_coin_select::Drain, +{ + fn score(&mut self, cs: &CoinSelector<'_>) -> Option { + let drain = (self.lowest_fee.change_policy)(cs, self.lowest_fee.target); + if drain.is_none() && self.must_have_change { + None + } else { + self.lowest_fee.score(cs) + } + } + + fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { + self.lowest_fee.bound(cs) + } + + fn requires_ordering_by_descending_value_pwu(&self) -> bool { + self.lowest_fee.requires_ordering_by_descending_value_pwu() + } +} + +/// Select coins for spend. +/// +/// Returns the selected coins and the change amount, which could be zero. +/// +/// `candidate_coins` are the coins to consider for selection. +/// +/// `base_tx` is the transaction to select coins for. It should be without any inputs +/// and without a change output, but with all non-change outputs added. +/// +/// `change_txo` is the change output to add if needed (with any value). +/// +/// `feerate_vb` is the minimum feerate (in sats/vb). Note that the selected coins +/// and change may result in a slightly lower feerate than this as the underlying +/// function instead uses a minimum feerate of `feerate_vb / 4.0` sats/wu. +/// +/// `min_fee` is the minimum fee (in sats) that the selection must have. +/// +/// `max_sat_weight` is the maximum weight difference of an input in the +/// transaction before and after satisfaction. +/// +/// `must_have_change` indicates whether the transaction must have a change output. +/// If `true`, the returned change amount will be positive. +fn select_coins_for_spend( + candidate_coins: &[CandidateCoin], + base_tx: bitcoin::Transaction, + change_txo: bitcoin::TxOut, + feerate_vb: f32, + min_fee: u64, + max_sat_weight: u32, + must_have_change: bool, +) -> Result<(Vec, bitcoin::Amount), InsufficientFunds> { + let out_value_nochange = base_tx.output.iter().map(|o| o.value).sum(); + + // Create the coin selector from the given candidates. NOTE: the coin selector keeps track + // of the original ordering of candidates so we can select any mandatory candidates using their + // original indices. + let base_weight: u32 = base_tx + .weight() + .to_wu() + .try_into() + .expect("Transaction weight must fit in u32"); + let max_input_weight = TXIN_BASE_WEIGHT + max_sat_weight; + let candidates: Vec = candidate_coins + .iter() + .map(|cand| Candidate { + input_count: 1, + value: cand.amount.to_sat(), + weight: max_input_weight, + is_segwit: true, // We only support receiving on Segwit scripts. + }) + .collect(); + let mut selector = CoinSelector::new(&candidates, base_weight); + for (i, cand) in candidate_coins.iter().enumerate() { + if cand.must_select { + // It's fine because the index passed to `select` refers to the original candidates ordering + // (and in any case the ordering of candidates is still the same in the coin selector). + selector.select(i); + } + } + + // Now set the change policy. We use a policy which ensures no change output is created with a + // lower value than our custom dust limit. NOTE: the change output weight must account for a + // potential difference in the size of the outputs count varint. This is why we take the whole + // change txo as argument and compute the weight difference below. + let long_term_feerate = FeeRate::from_sat_per_vb(LONG_TERM_FEERATE_VB); + let drain_weights = DrainWeights { + output_weight: { + let mut tx_with_change = base_tx; + tx_with_change.output.push(change_txo); + tx_with_change + .weight() + .to_wu() + .checked_sub(base_weight.into()) + .expect("base_weight can't be larger") + .try_into() + .expect("tx size must always fit in u32") + }, + spend_weight: max_input_weight, + }; + let change_policy = + change_policy::min_value_and_waste(drain_weights, DUST_OUTPUT_SATS, long_term_feerate); + + // Finally, run the coin selection algorithm. We use a BnB with 100k iterations and if it + // couldn't find any solution we fall back to selecting coins by descending value. + let target = Target { + value: out_value_nochange, + feerate: FeeRate::from_sat_per_vb(feerate_vb), + min_fee, + }; + let lowest_fee = LowestFee { + target, + long_term_feerate, + change_policy: &change_policy, + }; + let lowest_fee_change_cond = LowestFeeChangeCondition { + lowest_fee, + must_have_change, + }; + if let Err(e) = selector.run_bnb(lowest_fee_change_cond, 100_000) { + log::warn!( + "Coin selection error: '{}'. Selecting coins by descending value per weight unit...", + e.to_string() + ); + selector.sort_candidates_by_descending_value_pwu(); + // Select more coins until target is met and change condition satisfied. + loop { + let drain = change_policy(&selector, target); + if selector.is_target_met(target, drain) && (drain.is_some() || !must_have_change) { + break; + } + if !selector.select_next() { + // If the solution must have change, we calculate how much is missing from the current + // selection in order for there to be a change output with the smallest possible value. + let drain = if must_have_change { + bdk_coin_select::Drain { + weights: drain_weights, + value: DUST_OUTPUT_SATS, + } + } else { + drain + }; + let missing = selector.excess(target, drain).unsigned_abs(); + return Err(InsufficientFunds { missing }); + } + } + } + // By now, selection is complete and we can check how much change to give according to our policy. + let drain = change_policy(&selector, target); + let change_amount = bitcoin::Amount::from_sat(drain.value); + Ok(( + selector + .selected_indices() + .iter() + .map(|i| candidate_coins[*i]) + .collect(), + change_amount, + )) +} + +// Get the derived descriptor for this coin +fn derived_desc( + secp: &secp256k1::Secp256k1, + desc: &descriptors::LianaDescriptor, + coin: &CandidateCoin, +) -> descriptors::DerivedSinglePathLianaDesc { + let desc = if coin.is_change { + desc.change_descriptor() + } else { + desc.receive_descriptor() + }; + desc.derive(coin.deriv_index, secp) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AddrInfo { + pub index: bip32::ChildNumber, + pub is_change: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SpendOutputAddress { + pub addr: bitcoin::Address, + pub info: Option, +} + +/// A trait for getting a wallet transaction by its txid. +pub trait TxGetter { + /// Get a wallet transaction. Allows for a cache by making the access mutable. + fn get_tx(&mut self, txid: &bitcoin::Txid) -> Option; +} + +/// Specify the fee requirements for a transaction. In both cases set a target feerate in satoshi +/// per virtual byte. For RBF also set a minimum fee in satoshis for this transaction. See +/// https://github.com/bitcoin/bitcoin/blob/master/doc/policy/mempool-replacements.md for more +/// information about how it should be set. +pub enum SpendTxFees { + /// The target feerate in sats/vb for this transaction. + Regular(u64), + /// The (target feerate, minimum absolute fees) for this transactions. Both in sats. + Rbf(u64, u64), +} + +pub struct CreateSpendRes { + /// The created PSBT. + pub psbt: Psbt, + /// Whether the created PSBT has a change output. + pub has_change: bool, +} + +/// Create a PSBT for a transaction spending some, or all, of `candidate_coins` to `destinations`. +/// Important information for signers will be populated. Will refuse to create outputs worth less +/// than `DUST_OUTPUT_SATS`. Will refuse to create a transaction paying more than `MAX_FEE` +/// satoshis in fees or whose feerate is larger than `MAX_FEERATE` sats/vb. +/// +/// More about the parameters: +/// * `main_descriptor`: the multipath Liana descriptor, used to derive the addresses of the +/// candidate coins. +/// * `secp`: necessary to derive data from the descriptor. +/// * `tx_getter`: an interface to get the wallet transaction for the prevouts of the transaction. +/// Wouldn't be necessary if we only spent Taproot coins. +/// * `destinations`: a list of addresses and amounts, one per recipient i.e. per output in the +/// transaction created. If empty all the `candidate_coins` get spent and a single change output +/// is created to the provided `change_addr`. Can be used to sweep all, or some, coins from the +/// wallet. +/// * `candidate_coins`: a list of coins to consider including as input of the transaction. If +/// `destinations` is empty, they will all be included as inputs of the transaction. Otherwise, a +/// coin selection algorithm will be run to spend the most efficient subset of them to meet the +/// `destinations` requirements. +/// * `fees`: the target feerate (in sats/vb) and, if necessary, minimum absolute fee for this tx. +/// * `change_addr`: the address to use for a change output if we need to create one. Can be set to +/// an external address (if combined with an empty list of `destinations` it's useful to sweep some +/// or all coins of a wallet to an external address). +pub fn create_spend( + main_descriptor: &descriptors::LianaDescriptor, + secp: &secp256k1::Secp256k1, + tx_getter: &mut impl TxGetter, + destinations: &[(SpendOutputAddress, bitcoin::Amount)], + candidate_coins: &[CandidateCoin], + fees: SpendTxFees, + change_addr: SpendOutputAddress, +) -> Result { + // This method does quite a few things. In addition, we support different modes (coin control + // vs automated coin selection, self-spend, sweep, etc..) which make the logic a bit more + // intricate. Here is a brief overview of what we're doing here: + // 1. Create a transaction with all the target outputs (if this is a self-send, none are added + // at this step the only output will be added as a change output). + // 2. Automatically select the coins if necessary and determine whether a change output will be + // necessary for this transaction from the set of (automatically or manually) selected + // coins. The output for a self-send is added there. The change output is also (ab)used to + // implement a "sweep" functionality. We allow to set it to an external address to send all + // the inputs' value minus the fee and the + // other output's value to a specific, external, address. + // 3. Add the selected coins as inputs to the transaction. + // 4. Finalize the PSBT and sanity check it before returning it. + + let (feerate_vb, min_fee) = match fees { + SpendTxFees::Regular(feerate) => (feerate, 0), + SpendTxFees::Rbf(feerate, fee) => (feerate, fee), + }; + let is_self_send = destinations.is_empty(); + if feerate_vb < 1 { + return Err(SpendCreationError::InvalidFeerate(feerate_vb)); + } + + // Create transaction with no inputs and no outputs. + let mut tx = bitcoin::Transaction { + version: 2, + lock_time: LockTime::Blocks(Height::ZERO), // TODO: randomized anti fee sniping + input: Vec::with_capacity(candidate_coins.iter().filter(|c| c.must_select).count()), + output: Vec::with_capacity(destinations.len()), + }; + // Add the destinations outputs to the transaction and PSBT. At the same time + // sanity check each output's value. + let mut psbt_outs = Vec::with_capacity(destinations.len()); + for (address, amount) in destinations { + check_output_value(*amount)?; + + tx.output.push(bitcoin::TxOut { + value: amount.to_sat(), + script_pubkey: address.addr.script_pubkey(), + }); + // If it's an address of ours, signal it as change to signing devices by adding the + // BIP32 derivation path to the PSBT output. + let bip32_derivation = if let Some(AddrInfo { index, is_change }) = address.info { + let desc = if is_change { + main_descriptor.change_descriptor() + } else { + main_descriptor.receive_descriptor() + }; + desc.derive(index, secp).bip32_derivations() + } else { + Default::default() + }; + psbt_outs.push(PsbtOut { + bip32_derivation, + ..PsbtOut::default() + }); + } + assert_eq!(tx.output.is_empty(), is_self_send); + + // Now compute whether we'll need a change output while automatically selecting coins to be + // used as input if necessary. + // We need to get the size of a potential change output to select coins / determine whether + // we should include one, so get the change address and create a dummy txo for this purpose. + let mut change_txo = bitcoin::TxOut { + value: std::u64::MAX, + script_pubkey: change_addr.addr.script_pubkey(), + }; + // Now select the coins necessary using the provided candidates and determine whether + // there is any leftover to create a change output. + let (selected_coins, change_amount) = { + // At this point the transaction still has no input and no change output, as expected + // by the coins selection helper function. + assert!(tx.input.is_empty()); + assert_eq!(tx.output.len(), destinations.len()); + // TODO: Introduce general conversion error type. + let feerate_vb: f32 = { + let fr: u16 = feerate_vb.try_into().map_err(|_| { + SpendCreationError::InsaneFees(InsaneFeeInfo::TooHighFeerate(feerate_vb)) + })?; + fr + } + .try_into() + .expect("u16 must fit in f32"); + let max_sat_wu = main_descriptor + .max_sat_weight() + .try_into() + .expect("Weight must fit in a u32"); + select_coins_for_spend( + candidate_coins, + tx.clone(), + change_txo.clone(), + feerate_vb, + min_fee, + max_sat_wu, + is_self_send, + ) + .map_err(SpendCreationError::CoinSelection)? + }; + // If necessary, add a change output. + // For a self-send, coin selection will only find solutions with change and will otherwise + // return an error. In any case, the PSBT sanity check will catch a transaction with no outputs. + let has_change = change_amount.to_sat() > 0; + if has_change { + check_output_value(change_amount)?; + + // If the change address is ours, tell the signers by setting the BIP32 derivations in the + // PSBT output. + let bip32_derivation = if let Some(AddrInfo { index, is_change }) = change_addr.info { + let desc = if is_change { + main_descriptor.change_descriptor() + } else { + main_descriptor.receive_descriptor() + }; + desc.derive(index, secp).bip32_derivations() + } else { + Default::default() + }; + + // TODO: shuffle once we have Taproot + change_txo.value = change_amount.to_sat(); + tx.output.push(change_txo); + psbt_outs.push(PsbtOut { + bip32_derivation, + ..PsbtOut::default() + }); + } + + // Iterate through selected coins and add necessary information to the PSBT inputs. + let mut psbt_ins = Vec::with_capacity(selected_coins.len()); + for cand in &selected_coins { + let sequence = cand + .sequence + .unwrap_or(bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME); + tx.input.push(bitcoin::TxIn { + previous_output: cand.outpoint, + sequence, + // TODO: once we move to Taproot, anti-fee-sniping using nSequence + ..bitcoin::TxIn::default() + }); + + // Populate the PSBT input with the information needed by signers. + let coin_desc = derived_desc(secp, main_descriptor, cand); + let witness_script = Some(coin_desc.witness_script()); + let witness_utxo = Some(bitcoin::TxOut { + value: cand.amount.to_sat(), + script_pubkey: coin_desc.script_pubkey(), + }); + let non_witness_utxo = tx_getter.get_tx(&cand.outpoint.txid); + let bip32_derivation = coin_desc.bip32_derivations(); + psbt_ins.push(PsbtIn { + witness_script, + witness_utxo, + bip32_derivation, + non_witness_utxo, + ..PsbtIn::default() + }); + } + + // Finally, create the PSBT with all inputs and outputs, sanity check it and return it. + let psbt = Psbt { + unsigned_tx: tx, + version: 0, + xpub: BTreeMap::new(), + proprietary: BTreeMap::new(), + unknown: BTreeMap::new(), + inputs: psbt_ins, + outputs: psbt_outs, + }; + sanity_check_psbt(main_descriptor, &psbt)?; + // TODO: maybe check for common standardness rules (max size, ..)? + + Ok(CreateSpendRes { psbt, has_change }) +}