From 19a1b2db879f630219cd59746f866ca10ed94174 Mon Sep 17 00:00:00 2001 From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com> Date: Fri, 24 Nov 2023 18:12:27 +0000 Subject: [PATCH] commands: call `create_spend_internal` from `create_spend` --- src/commands/mod.rs | 284 +++++++------------------------------------- 1 file changed, 46 insertions(+), 238 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 6489ec41d..55bc6e199 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -437,21 +437,6 @@ impl DaemonControl { feerate_vb: 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. Fetch the selected coins from database and add them as inputs to the transaction. - // 4. Finalize the PSBT and sanity check it before returning it. - let is_self_send = destinations.is_empty(); // For self-send, the coins must be specified. if is_self_send && coins_outpoints.is_empty() { @@ -462,240 +447,63 @@ impl DaemonControl { } 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(coins_outpoints.len()), // Will be zero capacity for coin selection. - output: Vec::with_capacity(destinations.len()), - }; - // Add the destinations outputs to the transaction and PSBT. At the same time + // Check the destination addresses are valid for the network and // sanity check each output's value. - let mut psbt_outs = Vec::with_capacity(destinations.len()); + let mut destinations_checked: HashMap = + HashMap::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)?; - - 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, + destinations_checked.insert(address, amount); } - let (change_addr, int_change_info) = if let Some(addr) = change_address { - let addr = self.validate_address(addr)?; - (addr, None) + // Check also the change address if one has been given. + let change_address = change_address + .map(|addr| self.validate_address(addr)) + .transpose()?; + // 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. + // Otherwise, only the specified coins will be used, all as mandatory candidates. + let candidate_coins: Vec = if coins_outpoints.is_empty() { + // We only select confirmed coins for now. Including unconfirmed ones as well would + // introduce a whole bunch of additional complexity. + db_conn + .coins(&[CoinStatus::Confirmed], &[]) + .into_values() + .map(|c| CandidateCoin { + coin: c, + must_select: false, // No coin is mandatory. + }) + .collect() } 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, either select the coins necessary or use the ones provided (verifying they do in - // fact exist and are still unspent) and determine whether there is any leftover to create a - // change output. - let (selected_coins, change_amount) = { - let candidate_coins: Vec = if coins_outpoints.is_empty() { - // We only select confirmed coins for now. Including unconfirmed ones as well would - // introduce a whole bunch of additional complexity. - db_conn - .coins(&[CoinStatus::Confirmed], &[]) - .into_values() - .map(|c| CandidateCoin { - coin: c, - must_select: false, // No coin is mandatory. - }) - .collect() - } else { - // Query from DB and sanity check the provided coins to spend. - let coins = db_conn.coins(&[], coins_outpoints); - for op in coins_outpoints { - let coin = coins.get(op).ok_or(CommandError::UnknownOutpoint(*op))?; - if coin.is_spent() { - return Err(CommandError::AlreadySpent(*op)); - } - if coin.is_immature { - return Err(CommandError::ImmatureCoinbase(*op)); - } + // Query from DB and sanity check the provided coins to spend. + let coins = db_conn.coins(&[], coins_outpoints); + for op in coins_outpoints { + let coin = coins.get(op).ok_or(CommandError::UnknownOutpoint(*op))?; + if coin.is_spent() { + return Err(CommandError::AlreadySpent(*op)); + } + if coin.is_immature { + return Err(CommandError::ImmatureCoinbase(*op)); } - coins - .into_values() - .map(|c| CandidateCoin { - coin: c, - must_select: true, // All coins must be selected. - }) - .collect() - }; - // 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, - 0, // We only constrain the feerate. - max_sat_wu, - is_self_send, // Must have change if 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, + coins + .into_values() + .map(|c| CandidateCoin { + coin: c, + must_select: true, // All coins must be selected. + }) + .collect() }; - sanity_check_psbt(&self.config.main_descriptor, &psbt)?; - // TODO: maybe check for common standardness rules (max size, ..)? - Ok(CreateSpendResult { psbt }) + self.create_spend_internal( + &destinations_checked, + &candidate_coins, + feerate_vb, + 0, // No min fee required. + change_address, + ) } pub fn update_spend(&self, mut psbt: Psbt) -> Result<(), CommandError> {