nik/liana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge #974: gui: add max button for spend recipients

Browse Source 
46cd0f4923c5fe59a5ff08134c850d9a9ceb345a gui: add max checkbox for spend recipients (jp1ac4)
83fa9a9c924bb1326b859f1d2d5aac58c291e960 gui: unset amount left to select if form invalid (jp1ac4)
fba77a30e2aa9984cbda0fdaf40736ff1b0e57c4 gui: check if feerate is already set (jp1ac4)
d9b8285fac1288e895d4b460646f73c664d1a2ff gui: fix cargo fmt (jp1ac4)

Pull request description:

  This adds the MAX button from #546.

  The max amount is calculated within the `redraft` function based on the change output. The recipient's amount is updated directly within this same function in order to avoid another call to `redraft` after updating the amount.

  The MAX button only takes effect once when the user clicks it. If further changes are made (e.g. to feerate or other recipients), the value will not update and the user will need to click MAX again.

  Currently, the MAX button is always clickable. It would be possible to determine for each recipient whether the MAX button should be clickable for that recipient (feerate is valid and all other recipients are valid and no duplicates), but it's not a trivial change so I'm not sure if it's worth doing as I think the behaviour is quite intuitive.

ACKs for top commit:
  edouardparis:
    ACK 46cd0f4923c5fe59a5ff08134c850d9a9ceb345a

Tree-SHA512: 16f22804d630686427da196fd1627287e2e20f2e122602c385157064323113b27b9ec9c56ada162acb55d11b8084667ee3f1a9f992a2f4eef70f25c6f7a5b20b
This commit is contained in:
edouardparis 2024-03-04 16:20:53 +01:00
commit 3420c54793
No known key found for this signature in database
GPG Key ID: E65F7A089C20DC8F
3 changed files with 236 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

200
gui/src/app/state/spend/step.rs
View File

@ -1,6 +1,4 @@
use std::collections::HashMap;
use std::str::FromStr;
use std::sync::Arc;
use std::{cmp::Ordering, collections::HashMap, str::FromStr, sync::Arc};
use iced::{Command, Subscription};
use liana::{
@ -65,6 +63,9 @@ pub trait Step {
pub struct DefineSpend {
balance_available: Amount,
recipients: Vec<Recipient>,
/// If set, this is the index of a recipient that should
/// receive the max amount.
send_max_to_recipient: Option<usize>,
/// 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,
@ -123,6 +124,7 @@ impl DefineSpend {
coins_labels: HashMap::new(),
batch_label: form::Value::default(),
recipients: vec![Recipient::default()],
send_max_to_recipient: None,
is_user_coin_selection: false, // Start with auto-selection until user edits selection.
is_valid: false,
is_duplicate: false,
@ -162,12 +164,16 @@ impl DefineSpend {
self
}
fn form_values_are_valid(&self) -> bool {
// If `is_redraft`, the validation of recipients will take into account
// whether any should receive the max amount. Otherwise, all recipients
// will be fully validated.
fn form_values_are_valid(&self, is_redraft: bool) -> bool {
self.feerate.valid
&& !self.feerate.value.is_empty()
&& (self.batch_label.valid || self.recipients.len() < 2)
// Recipients will be empty for self-send.
&& self.recipients.iter().all(|r| r.valid())
&& self.recipients.iter().enumerate().all(|(i, r)|
r.valid() || (is_redraft && self.send_max_to_recipient == Some(i) && r.address_valid()))
}
fn exists_duplicate(&self) -> bool {
@ -185,27 +191,68 @@ impl DefineSpend {
fn check_valid(&mut self) {
self.is_valid =
self.form_values_are_valid() && self.coins.iter().any(|(_, selected)| *selected);
self.form_values_are_valid(false) && self.coins.iter().any(|(_, selected)| *selected);
self.is_duplicate = self.exists_duplicate();
}
/// redraft calculates the amount left to select and auto selects coins
/// if the user did not select a coin manually
fn redraft(&mut self, daemon: Arc<dyn Daemon + Sync + Send>) {
if !self.form_values_are_valid() || self.exists_duplicate() || self.recipients.is_empty() {
if !self.form_values_are_valid(true)
|| self.exists_duplicate()
|| self.recipients.is_empty()
{
// The current form details are not valid to draft a spend, so remove any previously
// calculated amount as it will no longer be valid and could be misleading, e.g. if
// the user removes the amount from one of the recipients.
// We can leave any coins selected as they will either be automatically updated
// as soon as the form is valid or the user has selected these specific coins and
// so we should not touch them.
self.amount_left_to_select = None;
// Remove any max amount from a recipient as it could be misleading.
if let Some(i) = self.send_max_to_recipient {
self.recipients
.get_mut(i)
.expect("max has been requested for this recipient so it must exist")
.update(
self.network,
view::CreateSpendMessage::RecipientEdited(i, "amount", "".to_string()),
);
}
return;
}
let destinations: HashMap<Address<address::NetworkUnchecked>, u64> = self
.recipients
.iter()
.map(|recipient| {
(
Address::from_str(&recipient.address.value).expect("Checked before"),
recipient.amount().expect("Checked before"),
)
.enumerate()
.filter_map(|(i, recipient)| {
// A recipient that receives the max should be treated as change for coin selection.
// Note that we only give a change output if its value is above the dust
// threshold, but a user can only send payments above the same dust threshold,
// so using change output to determine the max amount for a recipient will
// not prevent a value that could otherwise be entered manually by the user.
if self.send_max_to_recipient == Some(i) {
None
} else {
Some((
Address::from_str(&recipient.address.value).expect("Checked before"),
recipient.amount().expect("Checked before"),
))
}
})
.collect();
let recipient_with_max = if let Some(i) = self.send_max_to_recipient {
Some((
i,
self.recipients
.get_mut(i)
.expect("max has been requested for this recipient so it must exist"),
))
} else {
None
};
let outpoints = if self.is_user_coin_selection {
let outpoints: Vec<_> = self
.coins
@ -221,30 +268,53 @@ impl DefineSpend {
)
.collect();
if outpoints.is_empty() {
// If the user has deselected all coins, simply set the amount left to select as the
// total destination value. Note this doesn't take account of the fee, but passing
// an empty list to `create_spend_tx` would use auto-selection and so we settle for
// this approximation.
// If the user has deselected all coins, set any recipient's max amount to 0.
if let Some((i, recipient)) = recipient_with_max {
recipient.update(
self.network,
view::CreateSpendMessage::RecipientEdited(i, "amount", "0".to_string()),
);
}
// Simply set the amount left to select as the total destination value. Note this
// doesn't take account of the fee, but passing an empty list to `create_spend_tx`
// would use auto-selection and so we settle for this approximation.
self.amount_left_to_select = Some(Amount::from_sat(destinations.values().sum()));
return;
}
outpoints
} else if self.send_max_to_recipient.is_some() {
// If user has not selected coins, send the max available from all coins.
self.coins.iter().map(|(c, _)| c.outpoint).collect()
} else {
Vec::new() // pass empty list for auto-selection
};
// Use a fixed change address so that we don't increment the change index.
let dummy_address = self
.descriptor
.change_descriptor()
.derive(0.into(), &self.curve)
.address(self.network)
.as_unchecked()
.clone();
// If sending the max to a recipient, use that recipient's address as the
// change address.
// Otherwise, use a fixed change address from the user's own wallet so that
// we don't increment the change index.
let change_address = if let Some((_, recipient)) = &recipient_with_max {
Address::from_str(&recipient.address.value)
.expect("Checked before")
.as_unchecked()
.clone()
} else {
self.descriptor
.change_descriptor()
.derive(0.into(), &self.curve)
.address(self.network)
.as_unchecked()
.clone()
};
let feerate_vb = self.feerate.value.parse::<u64>().expect("Checked before");
match daemon.create_spend_tx(&outpoints, &destinations, feerate_vb, Some(dummy_address)) {
match daemon.create_spend_tx(
&outpoints,
&destinations,
feerate_vb,
Some(change_address.clone()),
) {
Ok(CreateSpendResult::Success { psbt, .. }) => {
self.warning = None;
if !self.is_user_coin_selection {
@ -261,6 +331,25 @@ impl DefineSpend {
}
// As coin selection was successful, we can assume there is nothing left to select.
self.amount_left_to_select = Some(Amount::from_sat(0));
if let Some((i, recipient)) = recipient_with_max {
// If there's no change output, any excess must be below the dust threshold
// and so the max available for this recipient is 0.
let amount = psbt
.unsigned_tx
.output
.iter()
.find(|o| {
o.script_pubkey
== change_address.clone().assume_checked().script_pubkey()
})
.map(|change_output| change_output.value.to_btc())
.unwrap_or(0.0)
.to_string();
recipient.update(
self.network,
view::CreateSpendMessage::RecipientEdited(i, "amount", amount),
);
}
}
// 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.
@ -269,6 +358,25 @@ impl DefineSpend {
// - select coins manually.
Ok(CreateSpendResult::InsufficientFunds { missing }) => {
self.amount_left_to_select = Some(Amount::from_sat(missing));
if let Some((i, recipient)) = recipient_with_max {
let amount = Amount::from_sat(if destinations.is_empty() {
// If there are no other recipients, then the missing value will
// be the amount left to select in order to create an output at the dust
// threshold. Therefore, set this recipient's amount to this value so
// that the information shown is consistent.
// Otherwise, there are already insufficient funds for the other
// recipients and so the max available for this recipient is 0.
DUST_OUTPUT_SATS
} else {
0
})
.to_btc()
.to_string();
recipient.update(
self.network,
view::CreateSpendMessage::RecipientEdited(i, "amount", amount),
);
}
}
Err(e) => {
self.warning = Some(e.into());
@ -300,6 +408,20 @@ impl Step for DefineSpend {
self.batch_label.valid = true;
self.batch_label.value = "".to_string();
}
if let Some(j) = self.send_max_to_recipient {
match j.cmp(&i) {
Ordering::Equal => {
self.send_max_to_recipient = None;
}
Ordering::Greater => {
self.send_max_to_recipient = Some(
j.checked_sub(1)
.expect("j must be greater than 0 in this case"),
);
}
_ => {}
}
}
}
view::CreateSpendMessage::RecipientEdited(i, _, _) => {
self.recipients
@ -372,6 +494,17 @@ impl Step for DefineSpend {
self.is_user_coin_selection = true;
}
}
view::CreateSpendMessage::SendMaxToRecipient(i) => {
if self.recipients.get(i).is_some() {
if self.send_max_to_recipient == Some(i) {
// If already set to this recipient, then unset it.
self.send_max_to_recipient = None;
} else {
// Either it's set to some other recipient or not at all.
self.send_max_to_recipient = Some(i);
};
}
}
_ => {}
}
@ -448,7 +581,11 @@ impl Step for DefineSpend {
self.recipients
.iter()
.enumerate()
.map(|(i, recipient)| recipient.view(i).map(view::Message::CreateSpend))
.map(|(i, recipient)| {
recipient
.view(i, self.send_max_to_recipient == Some(i))
.map(view::Message::CreateSpend)
})
.collect(),
Amount::from_sat(
self.recipients
@ -504,9 +641,12 @@ impl Recipient {
Ok(amount.to_sat())
}
fn address_valid(&self) -> bool {
!self.address.value.is_empty() && self.address.valid
}
fn valid(&self) -> bool {
!self.address.value.is_empty()
&& self.address.valid
self.address_valid()
&& !self.amount.value.is_empty()
&& self.amount.valid
&& self.label.valid
@ -545,8 +685,8 @@ impl Recipient {
};
}
fn view(&self, i: usize) -> Element<view::CreateSpendMessage> {
view::spend::recipient_view(i, &self.address, &self.amount, &self.label)
fn view(&self, i: usize, is_max_selected: bool) -> Element<view::CreateSpendMessage> {
view::spend::recipient_view(i, &self.address, &self.amount, &self.label, is_max_selected)
}
}

1
gui/src/app/view/message.rs
View File

@ -37,6 +37,7 @@ pub enum CreateSpendMessage {
FeerateEdited(String),
SelectPath(usize),
Generate,
SendMaxToRecipient(usize),
}
#[derive(Debug, Clone)]

79
gui/src/app/view/spend/mod.rs
View File

@ -2,7 +2,7 @@ use std::collections::HashMap;
use iced::{
alignment,
widget::{checkbox, scrollable, Space},
widget::{checkbox, scrollable, tooltip, Space},
Alignment, Length,
};
@ -192,7 +192,10 @@ pub fn create_spend_tx<'a>(
Message::CreateSpend(CreateSpendMessage::FeerateEdited(msg))
},
)
.warning("Feerate must be an integer less than or equal to 1000 sats/vbyte")
.warning(
"Feerate must be an integer less than \
or equal to 1000 sats/vbyte",
)
.size(20)
.padding(10),
)
@ -228,13 +231,39 @@ pub fn create_spend_tx<'a>(
))
.push(p2_regular("selected").style(color::GREY_3))
} else if let Some(amount_left) = amount_left {
Row::new()
.spacing(5)
.push(amount_with_size(amount_left, P2_SIZE))
.push(p2_regular("left to select").style(color::GREY_3))
if amount_left.to_sat() == 0 && !is_valid {
// If amount left is set, the current configuration must be redraftable.
// If it's not valid, either no coins are selected or there's a recipient
// with max selected and invalid amount.
if coins.iter().all(|(_, selected)| !selected) {
// This can happen if we have a single recipient
// and it has the max selected.
Row::new().push(
text("Select at least one coin.")
.style(color::GREY_3),
)
} else {
// There must be a recipient with max selected and value 0.
Row::new().push(
text("Check max amount for recipient.")
.style(color::GREY_3),
)
}
} else {
Row::new()
.spacing(5)
.push(amount_with_size(amount_left, P2_SIZE))
.push(p2_regular("left to select").style(color::GREY_3))
}
} else {
Row::new()
.push(text("Feerate needs to be set.").style(color::GREY_3))
Row::new().push(
text(if feerate.value.is_empty() || !feerate.valid {
"Feerate needs to be set."
} else {
"Add recipient details."
})
.style(color::GREY_3),
)
})
.width(Length::Fill),
)
@ -269,7 +298,8 @@ pub fn create_spend_tx<'a>(
.width(Length::Fixed(100.0)),
)
.push(
if is_valid && !duplicate
if is_valid
&& !duplicate
&& (is_self_send
|| (total_amount < *balance_available
&& Some(&Amount::from_sat(0)) == amount_left))
@ -292,6 +322,7 @@ pub fn recipient_view<'a>(
address: &form::Value<String>,
amount: &form::Value<String>,
label: &form::Value<String>,
is_max_selected: bool,
) -> Element<'a, CreateSpendMessage> {
Container::new(
Column::new()
@ -344,7 +375,7 @@ pub fn recipient_view<'a>(
)
.push(
Row::new()
.align_items(Alignment::Start)
.align_items(Alignment::Center)
.spacing(10)
.push(
Container::new(p1_bold("Amount"))
@ -352,16 +383,36 @@ pub fn recipient_view<'a>(
.align_x(alignment::Horizontal::Right)
.width(Length::Fixed(110.0)),
)
.push(
form::Form::new_trimmed("0.001 (in BTC)", amount, move |msg| {
.push_maybe(if is_max_selected {
Some(
Container::new(
text(amount.value.clone()).size(20).style(color::GREY_2),
)
.padding(10)
.width(Length::Fill),
)
} else {
None
})
.push_maybe(if !is_max_selected {
Some(form::Form::new_trimmed("0.001 (in BTC)", amount, move |msg| {
CreateSpendMessage::RecipientEdited(index, "amount", msg)
})
.warning(
"Invalid amount. (Note amounts lower than 0.00005 BTC are invalid.)",
)
.size(20)
.padding(10),
)
.padding(10))
} else {
None
})
.push(tooltip::Tooltip::new(
checkbox("MAX", is_max_selected, move |_| {
CreateSpendMessage::SendMaxToRecipient(index)
}),
"Total amount remaining after paying fee and any other recipients",
tooltip::Position::Left,
))
.width(Length::Fill),
),
)