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

Merge #930: gui: add warnings for descendant & conflicting transactions that will be dropped

Browse Source 
fde28bf571e4ba791617e62f0c124138cd58e1fe gui(transactions): get prev feerate from tx instead of param (jp1ac4)
dc817dee3be64bba5e8bee7a0ec646bb90e643d4 gui(transactions): check for direct descendants before rbf (jp1ac4)
881d9a74a29eef21aafd75f9851f588142e0091f gui(psbt): check for conflicting txs before broadcast (jp1ac4)
beabf08cbfe55e12a191d02c0a3370dc207fac86 gui: match on message for PsbtState (jp1ac4)
9e5872c56f7877227d5116207f611c2e44618325 gui(transactions): reword rbf modal message (jp1ac4)
c50954e9105fd81231fcb17527941d650e9c9f42 gui(transactions): split strings across multiple lines (jp1ac4)

Pull request description:

  This is for #903.

  It adds two warnings to the GUI:
  - when creating a new RBF if there are any descendant transactions of the transaction to be replaced:
  ![image](https://github.com/wizardsardine/liana/assets/121959000/b3d91253-fa5f-4c4c-b3f7-1e22d56ff0ba)

  - when broadcasting a transaction if there are any conflicting transactions
  ![image](https://github.com/wizardsardine/liana/assets/121959000/c3c8d932-e2d9-4982-9f5d-c7cf15d53370)

  These warnings are generated in the GUI using coins data from the DB and so will not appear if the DB coins have not yet been updated. Would resolving https://github.com/wizardsardine/liana/issues/887 help ensure the coins are updated before running these checks?

ACKs for top commit:
  edouardparis:
    ACK fde28bf571e4ba791617e62f0c124138cd58e1fe

Tree-SHA512: 3c428439926448a1d1fd0deb161b2b4016fcd6c4e2c0792ceb36e2c8bfc545ee5849ee76a6a89573a4389c6033bfe8ebd9f68555eca8a4f8f0e5fe1ac780be6d
This commit is contained in:
edouardparis 2024-02-02 10:35:29 +01:00
commit 445a8c1334
No known key found for this signature in database
GPG Key ID: E65F7A089C20DC8F
5 changed files with 266 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
gui/src/app/message.rs
View File

@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use liana::{
@ -42,4 +42,6 @@ pub enum Message {
HistoryTransactions(Result<Vec<HistoryTransaction>, Error>),
PendingTransactions(Result<Vec<HistoryTransaction>, Error>),
LabelsUpdated(Result<HashMap<String, Option<String>>, Error>),
BroadcastModal(Result<HashSet<Txid>, Error>),
RbfModal(HistoryTransaction, bool, Result<HashSet<Txid>, Error>),
}

132
gui/src/app/state/psbt.rs
View File

@ -8,7 +8,7 @@ use iced::Subscription;
use iced::Command;
use liana::{
descriptors::LianaPolicy,
miniscript::bitcoin::{bip32::Fingerprint, psbt::Psbt, Network},
miniscript::bitcoin::{bip32::Fingerprint, psbt::Psbt, Network, Txid},
};
use liana_ui::component::toast;
@ -131,60 +131,69 @@ impl PsbtState {
cache: &Cache,
message: Message,
) -> Command<Message> {
match &message {
Message::View(view::Message::Spend(msg)) => match msg {
view::SpendTxMessage::Cancel => {
if let Some(PsbtAction::Sign(SignAction { display_modal, .. })) =
&mut self.action
{
*display_modal = false;
return Command::none();
}
match message {
Message::View(view::Message::Spend(view::SpendTxMessage::Cancel)) => {
if let Some(PsbtAction::Sign(SignAction { display_modal, .. })) = &mut self.action {
*display_modal = false;
return Command::none();
}
self.action = None;
self.action = None;
}
Message::View(view::Message::Spend(view::SpendTxMessage::Delete)) => {
self.action = Some(PsbtAction::Delete(DeleteAction::default()));
}
Message::View(view::Message::Spend(view::SpendTxMessage::Sign)) => {
if let Some(PsbtAction::Sign(SignAction { display_modal, .. })) = &mut self.action {
*display_modal = true;
return Command::none();
}
view::SpendTxMessage::Delete => {
self.action = Some(PsbtAction::Delete(DeleteAction::default()));
}
view::SpendTxMessage::Sign => {
if let Some(PsbtAction::Sign(SignAction { display_modal, .. })) =
&mut self.action
{
*display_modal = true;
return Command::none();
}
let action = SignAction::new(
self.tx.signers(),
self.wallet.clone(),
cache.datadir_path.clone(),
cache.network,
self.saved,
);
let cmd = action.load(daemon);
self.action = Some(PsbtAction::Sign(action));
return cmd;
}
view::SpendTxMessage::EditPsbt => {
let action = UpdateAction::new(self.wallet.clone(), self.tx.psbt.to_string());
let cmd = action.load(daemon);
self.action = Some(PsbtAction::Update(action));
return cmd;
}
view::SpendTxMessage::Broadcast => {
self.action = Some(PsbtAction::Broadcast(BroadcastAction::default()));
}
view::SpendTxMessage::Save => {
self.action = Some(PsbtAction::Save(SaveAction::default()));
}
_ => {
if let Some(action) = self.action.as_mut() {
return action
.as_mut()
.update(daemon.clone(), message, &mut self.tx);
}
}
},
let action = SignAction::new(
self.tx.signers(),
self.wallet.clone(),
cache.datadir_path.clone(),
cache.network,
self.saved,
);
let cmd = action.load(daemon);
self.action = Some(PsbtAction::Sign(action));
return cmd;
}
Message::View(view::Message::Spend(view::SpendTxMessage::EditPsbt)) => {
let action = UpdateAction::new(self.wallet.clone(), self.tx.psbt.to_string());
let cmd = action.load(daemon);
self.action = Some(PsbtAction::Update(action));
return cmd;
}
Message::View(view::Message::Spend(view::SpendTxMessage::Broadcast)) => {
let outpoints: HashSet<_> = self.tx.coins.keys().cloned().collect();
return Command::perform(
async move {
daemon
// TODO: filter for the outpoints in `tx.coins` when this is possible:
// https://github.com/wizardsardine/liana/issues/677
.list_coins()
.map(|res| {
res.coins
.iter()
.filter_map(|c| {
if outpoints.contains(&c.outpoint) {
c.spend_info.map(|info| info.txid)
} else {
None
}
})
.collect()
})
.map_err(|e| e.into())
},
Message::BroadcastModal,
);
}
Message::View(view::Message::Spend(view::SpendTxMessage::Save)) => {
self.action = Some(PsbtAction::Save(SaveAction::default()));
}
Message::View(view::Message::Label(_, _)) | Message::LabelsUpdated(_) => {
match self.labels_edited.update(
daemon,
@ -207,6 +216,17 @@ impl PsbtState {
.update(daemon.clone(), message, &mut self.tx);
}
}
Message::BroadcastModal(res) => match res {
Ok(conflicting_txids) => {
self.action = Some(PsbtAction::Broadcast(BroadcastAction {
conflicting_txids,
..Default::default()
}));
}
Err(e) => {
self.warning = Some(e);
}
},
_ => {
if let Some(action) = self.action.as_mut() {
return action
@ -290,6 +310,8 @@ impl Action for SaveAction {
pub struct BroadcastAction {
broadcast: bool,
error: Option<Error>,
/// IDs of any directly conflicting transactions.
conflicting_txids: HashSet<Txid>,
}
impl Action for BroadcastAction {
@ -327,7 +349,11 @@ impl Action for BroadcastAction {
fn view<'a>(&'a self, content: Element<'a, view::Message>) -> Element<'a, view::Message> {
modal::Modal::new(
content,
view::psbt::broadcast_action(self.error.as_ref(), self.broadcast),
view::psbt::broadcast_action(
&self.conflicting_txids,
self.error.as_ref(),
self.broadcast,
),
)
.on_blur(Some(view::Message::Spend(view::SpendTxMessage::Cancel)))
.into()

60
gui/src/app/state/transactions.rs
View File

@ -1,5 +1,5 @@
use std::{
collections::HashMap,
collections::{HashMap, HashSet},
convert::TryInto,
sync::Arc,
time::{SystemTime, UNIX_EPOCH},
@ -112,6 +112,15 @@ impl State for TransactionsPanel {
}
}
},
Message::RbfModal(tx, is_cancel, res) => match res {
Ok(descendant_txids) => {
let modal = CreateRbfModal::new(tx, is_cancel, descendant_txids);
self.create_rbf_modal = Some(modal);
}
Err(e) => {
self.warning = e.into();
}
},
Message::View(view::Message::Close) => {
self.selected_tx = None;
}
@ -124,13 +133,31 @@ impl State for TransactionsPanel {
Message::View(view::Message::CreateRbf(view::CreateRbfMessage::New(is_cancel))) => {
if let Some(idx) = self.selected_tx {
if let Some(tx) = self.pending_txs.get(idx) {
if let Some(fee_amount) = tx.fee_amount {
let prev_feerate_vb = fee_amount
.to_sat()
.checked_div(tx.tx.vsize().try_into().unwrap())
.unwrap();
let modal = CreateRbfModal::new(tx.clone(), is_cancel, prev_feerate_vb);
self.create_rbf_modal = Some(modal);
if tx.fee_amount.is_some() {
let tx = tx.clone();
let txid = tx.tx.txid();
return Command::perform(
async move {
daemon
// TODO: filter for spending coins when this is possible:
// https://github.com/wizardsardine/liana/issues/677
.list_coins()
.map(|res| {
res.coins
.iter()
.filter_map(|c| {
if c.outpoint.txid == txid {
c.spend_info.map(|info| info.txid)
} else {
None
}
})
.collect()
})
.map_err(|e| e.into())
},
move |res| Message::RbfModal(tx, is_cancel, res),
);
}
}
}
@ -247,6 +274,9 @@ pub struct CreateRbfModal {
is_cancel: bool,
/// Min feerate required for RBF.
min_feerate_vb: u64,
/// IDs of any transactions from this wallet that are direct descendants of
/// the transaction to be replaced.
descendant_txids: HashSet<Txid>,
/// Feerate form value.
feerate_val: form::Value<String>,
/// Parsed feerate.
@ -259,12 +289,23 @@ pub struct CreateRbfModal {
}
impl CreateRbfModal {
fn new(tx: model::HistoryTransaction, is_cancel: bool, prev_feerate_vb: u64) -> Self {
fn new(
tx: model::HistoryTransaction,
is_cancel: bool,
descendant_txids: HashSet<Txid>,
) -> Self {
let prev_feerate_vb = tx
.fee_amount
.expect("rbf should only be used on a transaction with fee amount set")
.to_sat()
.checked_div(tx.tx.vsize().try_into().expect("vsize must fit in u64"))
.expect("transaction vsize must be positive");
let min_feerate_vb = prev_feerate_vb.checked_add(1).unwrap();
Self {
tx,
is_cancel,
min_feerate_vb,
descendant_txids,
feerate_val: form::Value {
valid: true,
value: min_feerate_vb.to_string(),
@ -329,6 +370,7 @@ impl CreateRbfModal {
content,
view::transactions::create_rbf_modal(
self.is_cancel,
&self.descendant_txids,
&self.feerate_val,
self.replacement_txid,
self.warning.as_ref(),

70
gui/src/app/view/psbt.rs
View File

@ -140,7 +140,15 @@ pub fn save_action<'a>(warning: Option<&Error>, saved: bool) -> Element<'a, Mess
}
}
pub fn broadcast_action<'a>(warning: Option<&Error>, saved: bool) -> Element<'a, Message> {
/// Return the modal view to broadcast a transaction.
///
/// `conflicting_txids` contains the IDs of any directly conflicting transactions
/// of the transaction to be broadcast.
pub fn broadcast_action<'a>(
conflicting_txids: &HashSet<Txid>,
warning: Option<&Error>,
saved: bool,
) -> Element<'a, Message> {
if saved {
card::simple(text("Transaction is broadcast"))
.width(Length::Fixed(400.0))
@ -151,7 +159,59 @@ pub fn broadcast_action<'a>(warning: Option<&Error>, saved: bool) -> Element<'a,
Column::new()
.spacing(10)
.push_maybe(warning.map(|w| warn(Some(w))))
.push(text("Broadcast the transaction"))
.push(Container::new(h4_bold("Broadcast the transaction")).width(Length::Fill))
.push_maybe(if conflicting_txids.is_empty() {
None
} else {
Some(
conflicting_txids.iter().fold(
Column::new()
.spacing(5)
.push(Row::new().spacing(10).push(icon::warning_icon()).push(text(
if conflicting_txids.len() > 1 {
"WARNING: Broadcasting this transaction \
will invalidate some pending payments."
} else {
"WARNING: Broadcasting this transaction \
will invalidate a pending payment."
},
)))
.push(Row::new().padding([0, 30]).push(text(
if conflicting_txids.len() > 1 {
"The following transactions are \
spending one or more inputs \
from the transaction to be \
broadcast and will be \
dropped, along with any other \
transactions that depend on them:"
} else {
"The following transaction is \
spending one or more inputs \
from the transaction to be \
broadcast and will be \
dropped, along with any other \
transactions that depend on it:"
},
))),
|col, txid| {
col.push(
Row::new()
.padding([0, 30])
.spacing(5)
.align_items(Alignment::Center)
.push(text(txid.to_string()))
.push(
Button::new(
icon::clipboard_icon().style(color::GREY_3),
)
.on_press(Message::Clipboard(txid.to_string()))
.style(theme::Button::TransparentBorder),
),
)
},
),
)
})
.push(
Row::new().push(Column::new().width(Length::Fill)).push(
button::primary(None, "Broadcast")
@ -159,7 +219,11 @@ pub fn broadcast_action<'a>(warning: Option<&Error>, saved: bool) -> Element<'a,
),
),
)
.width(Length::Fixed(400.0))
.width(Length::Fixed(if conflicting_txids.is_empty() {
400.0
} else {
800.0
}))
.into()
}
}

71
gui/src/app/view/transactions.rs
View File

@ -1,5 +1,5 @@
use chrono::NaiveDateTime;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use iced::{alignment, widget::tooltip, Alignment, Length};
@ -157,8 +157,13 @@ fn tx_list_view(i: usize, tx: &HistoryTransaction) -> Element<'_, Message> {
.into()
}
/// Return the modal view for a new RBF transaction.
///
/// `descendant_txids` contains the IDs of any transactions from this wallet that are
/// direct descendants of the transaction to be replaced.
pub fn create_rbf_modal<'a>(
is_cancel: bool,
descendant_txids: &HashSet<Txid>,
feerate: &form::Value<String>,
replacement_txid: Option<Txid>,
warning: Option<&'a Error>,
@ -169,15 +174,70 @@ pub fn create_rbf_modal<'a>(
confirm_button.on_press(Message::CreateRbf(super::CreateRbfMessage::Confirm));
}
let help_text = if is_cancel {
"Replace the transaction with one paying a higher feerate that sends the coins back to us. There is no guarantee the original transaction won't get mined first. New inputs may be used for the replacement transaction."
"Replace the transaction with one paying a higher feerate \
that sends the coins back to your wallet. There is no guarantee \
the original transaction won't get mined first. New inputs may \
be used for the replacement transaction."
} else {
"Replace the transaction with one paying a higher feerate to incentivize faster confirmation. New inputs may be used for the replacement transaction."
"Replace the transaction with one paying a higher feerate \
to incentivize faster confirmation. New inputs may be used \
for the replacement transaction."
};
card::simple(
Column::new()
.spacing(10)
.push(Container::new(h4_bold("Transaction replacement")).width(Length::Fill))
.push(Row::new().push(text(help_text)))
.push_maybe(if descendant_txids.is_empty() {
None
} else {
Some(
descendant_txids.iter().fold(
Column::new()
.spacing(5)
.push(Row::new().spacing(10).push(icon::warning_icon()).push(text(
if descendant_txids.len() > 1 {
"WARNING: Replacing this transaction \
will invalidate some later payments."
} else {
"WARNING: Replacing this transaction \
will invalidate a later payment."
},
)))
.push(Row::new().padding([0, 30]).push(text(
if descendant_txids.len() > 1 {
"The following transactions are \
spending one or more outputs \
from the transaction to be replaced \
and will be dropped when the replacement \
is broadcast, along with any other \
transactions that depend on them:"
} else {
"The following transaction is \
spending one or more outputs \
from the transaction to be replaced \
and will be dropped when the replacement \
is broadcast, along with any other \
transactions that depend on it:"
},
))),
|col, txid| {
col.push(
Row::new()
.padding([0, 30])
.spacing(5)
.align_items(Alignment::Center)
.push(text(txid.to_string()))
.push(
Button::new(icon::clipboard_icon().style(color::GREY_3))
.on_press(Message::Clipboard(txid.to_string()))
.style(theme::Button::TransparentBorder),
),
)
},
),
)
})
.push_maybe(if !is_cancel {
Some(
Row::new()
@ -188,7 +248,8 @@ pub fn create_rbf_modal<'a>(
Message::CreateRbf(CreateRbfMessage::FeerateEdited(msg))
})
.warning(
"Feerate must be greater than previous value and less than or equal to 1000 sats/vbyte",
"Feerate must be greater than previous value and \
less than or equal to 1000 sats/vbyte",
)
.size(20)
.padding(10),
@ -219,7 +280,7 @@ pub fn create_rbf_modal<'a>(
)
})),
)
.width(Length::Fixed(600.0))
.width(Length::Fixed(800.0))
.into()
}