From e462cf3a77c631aaebdb774d807105d76087cc93 Mon Sep 17 00:00:00 2001 From: edouardparis Date: Mon, 11 Dec 2023 15:00:40 +0100 Subject: [PATCH] Use create_spend to calculate amount left to select close #822 --- gui/Cargo.lock | 2 +- gui/src/app/error.rs | 10 +- gui/src/app/state/coins.rs | 8 ++ gui/src/app/state/spend/mod.rs | 4 +- gui/src/app/state/spend/step.rs | 225 +++++++++++++++++--------------- gui/src/app/view/warning.rs | 1 + gui/src/daemon/embedded.rs | 7 +- 7 files changed, 143 insertions(+), 114 deletions(-) diff --git a/gui/Cargo.lock b/gui/Cargo.lock index b150df96d..dd21e6c57 100644 --- a/gui/Cargo.lock +++ b/gui/Cargo.lock @@ -2431,7 +2431,7 @@ dependencies = [ [[package]] name = "liana" version = "2.0.0" -source = "git+https://github.com/wizardsardine/liana?branch=master#6151c57af492dacc8502b0ea1ec1cd04580e08dc" +source = "git+https://github.com/wizardsardine/liana?branch=master#3e0f82a71e031604fcf8bb0fd757ae95908e8ca6" dependencies = [ "backtrace", "bdk_coin_select", diff --git a/gui/src/app/error.rs b/gui/src/app/error.rs index 94263e628..0ddbf8966 100644 --- a/gui/src/app/error.rs +++ b/gui/src/app/error.rs @@ -1,7 +1,7 @@ use std::convert::From; use std::io::ErrorKind; -use liana::{config::ConfigError, descriptors::LianaDescError}; +use liana::{config::ConfigError, descriptors::LianaDescError, spend::SpendCreationError}; use crate::{ app::{settings::SettingsError, wallet::WalletError}, @@ -16,6 +16,7 @@ pub enum Error { Unexpected(String), HardwareWallet(async_hwi::Error), Desc(LianaDescError), + Spend(SpendCreationError), } impl std::fmt::Display for Error { @@ -23,6 +24,7 @@ impl std::fmt::Display for Error { match self { Self::Config(e) => write!(f, "{}", e), Self::Wallet(e) => write!(f, "{}", e), + Self::Spend(e) => write!(f, "{}", e), Self::Daemon(e) => match e { DaemonError::Unexpected(e) => write!(f, "{}", e), DaemonError::NoAnswer => write!(f, "Daemon did not answer"), @@ -84,3 +86,9 @@ impl From for Error { Error::HardwareWallet(error) } } + +impl From for Error { + fn from(error: SpendCreationError) -> Self { + Error::Spend(error) + } +} diff --git a/gui/src/app/state/coins.rs b/gui/src/app/state/coins.rs index 3474d37d0..12342fb17 100644 --- a/gui/src/app/state/coins.rs +++ b/gui/src/app/state/coins.rs @@ -216,6 +216,8 @@ mod tests { spend_info: None, is_immature: false, address: dummy_address.clone(), + derivation_index: 0.into(), + is_change: false, }, Coin { outpoint: bitcoin::OutPoint { txid, vout: 3 }, @@ -224,6 +226,8 @@ mod tests { spend_info: None, is_immature: false, address: dummy_address.clone(), + derivation_index: 1.into(), + is_change: false, }, Coin { outpoint: bitcoin::OutPoint { txid, vout: 0 }, @@ -232,6 +236,8 @@ mod tests { spend_info: None, is_immature: false, address: dummy_address.clone(), + derivation_index: 2.into(), + is_change: false, }, Coin { outpoint: bitcoin::OutPoint { txid, vout: 1 }, @@ -240,6 +246,8 @@ mod tests { spend_info: None, is_immature: false, address: dummy_address, + derivation_index: 3.into(), + is_change: false, }, ]); diff --git a/gui/src/app/state/spend/mod.rs b/gui/src/app/state/spend/mod.rs index eab826bf6..4c39ef348 100644 --- a/gui/src/app/state/spend/mod.rs +++ b/gui/src/app/state/spend/mod.rs @@ -32,7 +32,7 @@ impl CreateSpendPanel { current: 0, steps: vec![ Box::new( - step::DefineSpend::new(descriptor, coins, timelock) + step::DefineSpend::new(network, descriptor, coins, timelock) .with_coins_sorted(blockheight), ), Box::new(step::SaveSpend::new(wallet)), @@ -54,7 +54,7 @@ impl CreateSpendPanel { current: 0, steps: vec![ Box::new( - step::DefineSpend::new(descriptor, coins, timelock) + step::DefineSpend::new(network, descriptor, coins, timelock) .with_preselected_coins(preselected_coins) .with_coins_sorted(blockheight) .self_send(), diff --git a/gui/src/app/state/spend/step.rs b/gui/src/app/state/spend/step.rs index 877ca8182..1fc77eea6 100644 --- a/gui/src/app/state/spend/step.rs +++ b/gui/src/app/state/spend/step.rs @@ -6,7 +6,10 @@ use iced::{Command, Subscription}; use liana::{ descriptors::LianaDescriptor, miniscript::bitcoin::{ - self, address, psbt::Psbt, Address, Amount, Denomination, Network, OutPoint, + self, address, psbt::Psbt, secp256k1, Address, Amount, Denomination, Network, OutPoint, + }, + spend::{ + create_spend, CandidateCoin, SpendCreationError, SpendOutputAddress, SpendTxFees, TxGetter, }, }; @@ -19,7 +22,7 @@ use crate::{ app::{cache::Cache, error::Error, message::Message, state::psbt, view, wallet::Wallet}, daemon::{ model::{remaining_sequence, Coin, SpendTx}, - Daemon, DaemonError, + Daemon, }, }; @@ -73,7 +76,9 @@ pub struct DefineSpend { is_valid: bool, is_duplicate: bool, + network: Network, descriptor: LianaDescriptor, + curve: secp256k1::Secp256k1, timelock: u16, coins: Vec<(Coin, bool)>, coins_labels: HashMap, @@ -85,7 +90,12 @@ pub struct DefineSpend { } impl DefineSpend { - pub fn new(descriptor: LianaDescriptor, coins: &[Coin], timelock: u16) -> Self { + pub fn new( + network: Network, + descriptor: LianaDescriptor, + coins: &[Coin], + timelock: u16, + ) -> Self { let balance_available = coins .iter() .filter_map(|coin| { @@ -99,7 +109,7 @@ impl DefineSpend { let coins: Vec<(Coin, bool)> = coins .iter() .filter_map(|c| { - if c.spend_info.is_none() { + if c.spend_info.is_none() && !c.is_immature { Some((c.clone(), false)) } else { None @@ -109,7 +119,9 @@ impl DefineSpend { Self { balance_available, + network, descriptor, + curve: secp256k1::Secp256k1::verification_only(), timelock, generated: None, coins, @@ -175,110 +187,122 @@ impl DefineSpend { } } } - fn auto_select_coins(&mut self, daemon: Arc) { - // Set non-input values in the same way as for user selection. - let mut outputs: HashMap, u64> = HashMap::new(); - for recipient in &self.recipients { - outputs.insert( - Address::from_str(&recipient.address.value).expect("Checked before"), - recipient.amount().expect("Checked before"), - ); + /// redraft calcul amount left to select and auto select coin is the user did not select a coin + /// manually + fn redraft(&mut self, daemon: Arc) { + if !self.form_values_are_valid() || self.recipients.is_empty() { + return; } - let feerate_vb = self.feerate.value.parse::().unwrap_or(0); + + let destinations: Vec<(SpendOutputAddress, Amount)> = self + .recipients + .iter() + .map(|recipient| { + ( + SpendOutputAddress { + addr: Address::from_str(&recipient.address.value) + .expect("Checked before") + .assume_checked(), + info: None, + }, + Amount::from_sat(recipient.amount().expect("Checked before")), + ) + }) + .collect(); + + let coins: Vec = if self.is_user_coin_selection { + self.coins + .iter() + .filter_map(|(c, selected)| { + if *selected { + Some(CandidateCoin { + amount: c.amount, + outpoint: c.outpoint, + deriv_index: c.derivation_index, + is_change: c.is_change, + sequence: None, + must_select: *selected, + }) + } else { + None + } + }) + .collect() + } else { + self.coins + .iter() + .map(|(c, _)| CandidateCoin { + amount: c.amount, + outpoint: c.outpoint, + deriv_index: c.derivation_index, + is_change: c.is_change, + sequence: None, + must_select: false, + }) + .collect() + }; + + let dummy_address = self + .descriptor + .change_descriptor() + .derive(0.into(), &self.curve) + .address(self.network); + + let feerate_vb = self.feerate.value.parse::().expect("Checked before"); // Create a spend with empty inputs in order to use auto-selection. - match daemon.create_spend_tx(&[], &outputs, feerate_vb) { + match create_spend( + &self.descriptor, + &self.curve, + &mut DaemonTxGetter(&daemon), + &destinations, + &coins, + SpendTxFees::Regular(feerate_vb), + // we enter a dummy address to calculate + SpendOutputAddress { + addr: dummy_address, + info: None, + }, + ) { Ok(spend) => { self.warning = None; - let selected_coins: Vec = spend - .psbt - .unsigned_tx - .input - .iter() - .map(|c| c.previous_output) - .collect(); - // Mark coins as selected. - for (coin, selected) in &mut self.coins { - *selected = selected_coins.contains(&coin.outpoint); + if !self.is_user_coin_selection { + let selected_coins: Vec = spend + .psbt + .unsigned_tx + .input + .iter() + .map(|c| c.previous_output) + .collect(); + // Mark coins as selected. + for (coin, selected) in &mut self.coins { + *selected = selected_coins.contains(&coin.outpoint); + } } // As coin selection was successful, we can assume there is nothing left to select. self.amount_left_to_select = Some(Amount::from_sat(0)); } + // For coin selection error (insufficient funds), do not make any changes to + // selected coins on screen and just show user how much is left to select. + // User can then either: + // - modify recipient amounts and/or feerate and let coin selection run again, or + // - select coins manually. + Err(SpendCreationError::CoinSelection(amount)) => { + self.amount_left_to_select = Some(Amount::from_sat(amount.missing)); + } Err(e) => { - if let DaemonError::CoinSelectionError = e { - // For coin selection error (insufficient funds), do not make any changes to - // selected coins on screen and just show user how much is left to select. - // User can then either: - // - modify recipient amounts and/or feerate and let coin selection run again, or - // - select coins manually. - self.amount_left_to_select(); - } else { - self.warning = Some(e.into()); - } + self.warning = Some(e.into()); } } } - fn amount_left_to_select(&mut self) { - // We need the feerate in order to compute the required amount of BTC to - // select. Return early if we don't to not do unnecessary computation. - let feerate = match self.feerate.value.parse::() { - Ok(f) => f, - Err(_) => { - self.amount_left_to_select = None; - return; - } - }; - - // The coins to be included in this transaction. - let selected_coins: Vec<_> = self - .coins - .iter() - .filter_map(|(c, selected)| if *selected { Some(c) } else { None }) - .collect(); +} - // A dummy representation of the transaction that will be computed, for - // the purpose of computing its size in order to anticipate the fees needed. - // NOTE: we make the conservative estimation a change output will always be - // needed. - let tx_template = bitcoin::Transaction { - version: 2, - lock_time: bitcoin::blockdata::locktime::absolute::LockTime::ZERO, - input: selected_coins - .iter() - .map(|_| bitcoin::TxIn::default()) - .collect(), - output: self - .recipients - .iter() - .filter_map(|recipient| { - if recipient.valid() { - Some(bitcoin::TxOut { - script_pubkey: Address::from_str(&recipient.address.value) - .unwrap() - .payload - .script_pubkey(), - value: recipient.amount().unwrap(), - }) - } else { - None - } - }) - .collect(), - }; - // nValue size + scriptPubKey CompactSize + OP_0 + PUSH32 + - const CHANGE_TXO_SIZE: usize = 8 + 1 + 1 + 1 + 32; - let satisfaction_vsize = self.descriptor.max_sat_weight() / 4; - let transaction_size = - tx_template.vsize() + satisfaction_vsize * tx_template.input.len() + CHANGE_TXO_SIZE; - - // Now the calculation of the amount left to be selected by the user is a simple - // substraction between the value needed by the transaction to be created and the - // value that was selected already. - let selected_amount = selected_coins.iter().map(|c| c.amount.to_sat()).sum(); - let output_sum: u64 = tx_template.output.iter().map(|o| o.value).sum(); - let needed_amount: u64 = transaction_size as u64 * feerate + output_sum; - self.amount_left_to_select = Some(Amount::from_sat( - needed_amount.saturating_sub(selected_amount), - )); +pub struct DaemonTxGetter<'a>(&'a Arc); +impl<'a> TxGetter for DaemonTxGetter<'a> { + fn get_tx(&mut self, txid: &bitcoin::Txid) -> Option { + self.0 + .list_txs(&[*txid]) + .ok() + .and_then(|mut txs| txs.transactions.pop().map(|tx| tx.tx)) } } @@ -317,14 +341,11 @@ impl Step for DefineSpend { if let Ok(value) = s.parse::() { self.feerate.value = s; self.feerate.valid = value != 0; - self.amount_left_to_select(); } else if s.is_empty() { self.feerate.value = "".to_string(); self.feerate.valid = true; - self.amount_left_to_select = None; } else { self.feerate.valid = false; - self.amount_left_to_select = None; } self.warning = None; } @@ -368,7 +389,6 @@ impl Step for DefineSpend { coin.1 = !coin.1; // Once user edits selection, auto-selection can no longer be used. self.is_user_coin_selection = true; - self.amount_left_to_select(); } } _ => {} @@ -378,12 +398,7 @@ impl Step for DefineSpend { // - all form values have been added and validated // - not a self-send // - user has not yet selected coins manually - if self.form_values_are_valid() - && !self.recipients.is_empty() - && !self.is_user_coin_selection - { - self.auto_select_coins(daemon); - } + self.redraft(daemon); self.check_valid(); } Message::Psbt(res) => match res { diff --git a/gui/src/app/view/warning.rs b/gui/src/app/view/warning.rs index 9f03312ff..1ec4dfd27 100644 --- a/gui/src/app/view/warning.rs +++ b/gui/src/app/view/warning.rs @@ -41,6 +41,7 @@ impl From<&Error> for WarningMessage { Error::Unexpected(_) => WarningMessage("Unknown error".to_string()), Error::HardwareWallet(_) => WarningMessage("Hardware wallet error".to_string()), Error::Desc(e) => WarningMessage(format!("Descriptor analysis error: '{}'.", e)), + Error::Spend(e) => WarningMessage(format!("Spend creation error: '{}'.", e)), } } } diff --git a/gui/src/daemon/embedded.rs b/gui/src/daemon/embedded.rs index a07d0bbe4..4b1708447 100644 --- a/gui/src/daemon/embedded.rs +++ b/gui/src/daemon/embedded.rs @@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet}; use super::{model::*, Daemon, DaemonError}; use liana::{ - commands::{CommandError, LabelItem}, + commands::LabelItem, config::Config, miniscript::bitcoin::{address, psbt::Psbt, Address, OutPoint, Txid}, DaemonControl, DaemonHandle, @@ -90,10 +90,7 @@ impl Daemon for EmbeddedDaemon { ) -> Result { self.control()? .create_spend(destinations, coins_outpoints, feerate_vb, None) - .map_err(|e| match e { - CommandError::CoinSelectionError(_) => DaemonError::CoinSelectionError, - e => DaemonError::Unexpected(e.to_string()), - }) + .map_err(|e| DaemonError::Unexpected(e.to_string())) } fn rbf_psbt(