diff --git a/gui/Cargo.lock b/gui/Cargo.lock index edbd18950..88ba99c99 100644 --- a/gui/Cargo.lock +++ b/gui/Cargo.lock @@ -252,6 +252,11 @@ dependencies = [ "byteorder", ] +[[package]] +name = "bdk_coin_select" +version = "0.1.0" +source = "git+https://github.com/evanlinjin/bdk?branch=new_bdk_coin_select#7e0d4a5a8cc88e354a09c676efb3d80d33c261e8" + [[package]] name = "bech32" version = "0.9.1" @@ -2415,14 +2420,16 @@ dependencies = [ [[package]] name = "liana" version = "2.0.0" -source = "git+https://github.com/wizardsardine/liana?branch=master#42578609e2ed340d277d75c747c74449308acbb7" +source = "git+https://github.com/jp1ac4/liana?branch=auto-coin-selection#1fcb332f9894ba5fc9c7b4b6c9a10bf11c9f1d94" dependencies = [ "backtrace", + "bdk_coin_select", "bip39", "dirs 5.0.0", "fern", "getrandom 0.2.8", "jsonrpc 0.16.0", + "libc", "log", "miniscript", "rdrand", diff --git a/gui/Cargo.toml b/gui/Cargo.toml index 89b2c4fb2..7ff7c05a9 100644 --- a/gui/Cargo.toml +++ b/gui/Cargo.toml @@ -15,7 +15,7 @@ path = "src/main.rs" [dependencies] async-hwi = "0.0.12" -liana = { git = "https://github.com/wizardsardine/liana", branch = "master", default-features = false, features = ["nonblocking_shutdown"] } +liana = { git = "https://github.com/jp1ac4/liana", branch = "auto-coin-selection", features = ["nonblocking_shutdown"] } liana_ui = { path = "ui" } backtrace = "0.3" base64 = "0.13" diff --git a/gui/src/app/error.rs b/gui/src/app/error.rs index 89fad1e91..dbd16aba0 100644 --- a/gui/src/app/error.rs +++ b/gui/src/app/error.rs @@ -45,6 +45,7 @@ impl std::fmt::Display for Error { DaemonError::Rpc(code, e) => { write!(f, "[{:?}] {}", code, e) } + DaemonError::CoinSelectionError => write!(f, "{}", e), }, Self::Unexpected(e) => write!(f, "Unexpected error: {}", e), Self::HardwareWallet(e) => write!(f, "{}", e), diff --git a/gui/src/app/state/spend/step.rs b/gui/src/app/state/spend/step.rs index 0fecbb0dd..58a4a420e 100644 --- a/gui/src/app/state/spend/step.rs +++ b/gui/src/app/state/spend/step.rs @@ -19,7 +19,7 @@ use crate::{ app::{cache::Cache, error::Error, message::Message, state::psbt, view, wallet::Wallet}, daemon::{ model::{remaining_sequence, Coin, SpendTx}, - Daemon, + Daemon, DaemonError, }, }; @@ -67,6 +67,9 @@ pub trait Step { pub struct DefineSpend { balance_available: Amount, recipients: Vec, + /// Will be `true` if coins for spend were manually selected by user. + /// Otherwise, will be `false` (including for self-send). + is_user_coin_selection: bool, is_valid: bool, is_duplicate: bool, @@ -77,6 +80,8 @@ pub struct DefineSpend { batch_label: form::Value, amount_left_to_select: Option, feerate: form::Value, + /// PSBT generated by auto-selection. + auto_generated: Option, generated: Option, warning: Option, } @@ -108,11 +113,13 @@ impl DefineSpend { balance_available, descriptor, timelock, + auto_generated: None, generated: None, coins, coins_labels: HashMap::new(), batch_label: form::Value::default(), recipients: vec![Recipient::default()], + is_user_coin_selection: false, // Start with auto-selection until user edits selection. is_valid: false, is_duplicate: false, feerate: form::Value::default(), @@ -151,18 +158,19 @@ impl DefineSpend { self } - fn check_valid(&mut self) { - self.is_valid = self.feerate.valid + fn form_values_are_valid(&self) -> bool { + self.feerate.valid && !self.feerate.value.is_empty() - && (self.batch_label.valid || self.recipients.len() < 2); + && (self.batch_label.valid || self.recipients.len() < 2) + // Recipients will be empty for self-send. + && self.recipients.iter().all(|r| r.valid()) + } + + fn check_valid(&mut self) { + self.is_valid = + self.form_values_are_valid() && self.coins.iter().any(|(_, selected)| *selected); self.is_duplicate = false; - if !self.coins.iter().any(|(_, selected)| *selected) { - self.is_valid = false; - } for (i, recipient) in self.recipients.iter().enumerate() { - if !recipient.valid() { - self.is_valid = false; - } if !self.is_duplicate && !recipient.address.value.is_empty() { self.is_duplicate = self.recipients[..i] .iter() @@ -170,6 +178,50 @@ 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"), + ); + } + let feerate_vb = self.feerate.value.parse::().unwrap_or(0); + // Create a spend with empty inputs in order to use auto-selection. + match daemon.create_spend_tx(&[], &outputs, feerate_vb) { + 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); + } + self.auto_generated = Some(spend.psbt); + // As coin selection was successful, we can assume there is nothing left to select. + self.amount_left_to_select = Some(Amount::from_sat(0)); + } + Err(e) => { + self.auto_generated = None; + 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()); + } + } + } + } 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. @@ -282,48 +334,70 @@ impl Step for DefineSpend { self.warning = None; } view::CreateSpendMessage::Generate => { - let inputs: Vec = self - .coins - .iter() - .filter_map( - |(coin, selected)| { - if *selected { - Some(coin.outpoint) - } else { - None - } + // For auto-selection, re-use corresponding PSBT in order to keep same change amount. + if !self.is_user_coin_selection + && !self.recipients.is_empty() + && self.auto_generated.is_some() + { + let psbt = self.auto_generated.clone().expect("Checked is_some()"); + return Command::perform(async move { Ok(psbt) }, Message::Psbt); + } else { + let inputs: Vec = self + .coins + .iter() + .filter_map( + |(coin, selected)| { + if *selected { + Some(coin.outpoint) + } else { + None + } + }, + ) + .collect(); + 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"), + ); + } + let feerate_vb = self.feerate.value.parse::().unwrap_or(0); + self.warning = None; + return Command::perform( + async move { + daemon + .create_spend_tx(&inputs, &outputs, feerate_vb) + .map(|res| res.psbt) + .map_err(|e| e.into()) }, - ) - .collect(); - 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"), + Message::Psbt, ); } - let feerate_vb = self.feerate.value.parse::().unwrap_or(0); - self.warning = None; - return Command::perform( - async move { - daemon - .create_spend_tx(&inputs, &outputs, feerate_vb) - .map(|res| res.psbt) - .map_err(|e| e.into()) - }, - Message::Psbt, - ); } view::CreateSpendMessage::SelectCoin(i) => { if let Some(coin) = self.coins.get_mut(i) { 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(); } } _ => {} } + + // Attempt to select coins automatically if: + // - 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.check_valid(); } Message::Psbt(res) => match res { diff --git a/gui/src/app/view/warning.rs b/gui/src/app/view/warning.rs index a07b18aaa..86ab02717 100644 --- a/gui/src/app/view/warning.rs +++ b/gui/src/app/view/warning.rs @@ -34,6 +34,9 @@ impl From<&Error> for WarningMessage { WarningMessage("Communication with Daemon failed".to_string()) } DaemonError::DaemonStopped => WarningMessage("Daemon stopped".to_string()), + DaemonError::CoinSelectionError => { + WarningMessage("Error when selecting coins for spend".to_string()) + } }, Error::Unexpected(_) => WarningMessage("Unknown error".to_string()), Error::HardwareWallet(_) => WarningMessage("Hardware wallet error".to_string()), diff --git a/gui/src/daemon/embedded.rs b/gui/src/daemon/embedded.rs index f7c64e6b6..57367adc1 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::LabelItem, + commands::{CommandError, LabelItem}, config::Config, miniscript::bitcoin::{address, psbt::Psbt, Address, OutPoint, Txid}, DaemonControl, DaemonHandle, @@ -90,7 +90,10 @@ impl Daemon for EmbeddedDaemon { ) -> Result { self.control()? .create_spend(destinations, coins_outpoints, feerate_vb) - .map_err(|e| DaemonError::Unexpected(e.to_string())) + .map_err(|e| match e { + CommandError::CoinSelectionError(_) => DaemonError::CoinSelectionError, + e => DaemonError::Unexpected(e.to_string()), + }) } fn update_spend_tx(&self, psbt: &Psbt) -> Result<(), DaemonError> { diff --git a/gui/src/daemon/mod.rs b/gui/src/daemon/mod.rs index 12e46f3a0..9191b9a93 100644 --- a/gui/src/daemon/mod.rs +++ b/gui/src/daemon/mod.rs @@ -30,6 +30,8 @@ pub enum DaemonError { Start(StartupError), // Error if the client is not supported. ClientNotSupported, + /// Error when selecting coins for spend. + CoinSelectionError, } impl std::fmt::Display for DaemonError { @@ -42,6 +44,7 @@ impl std::fmt::Display for DaemonError { Self::Unexpected(e) => write!(f, "Daemon unexpected error: {}", e), Self::Start(e) => write!(f, "Daemon did not start: {}", e), Self::ClientNotSupported => write!(f, "Daemon communication is not supported"), + Self::CoinSelectionError => write!(f, "Coin selection error"), } } }