From dcef77b4dd01edcbf922aeb21002047ce26ca83f Mon Sep 17 00:00:00 2001 From: edouardparis Date: Mon, 27 Nov 2023 16:53:38 +0100 Subject: [PATCH] Handle missing coin inputs in tx and psbt As coins deposited by an unconfirmed transactions can be removed from the database, GUI should not rely on them to calculate fees. The fees must be passed as Option. If a input coin is missing for a psbt, we use the witness_utxo amount and settle the total fee to None if both are absents. --- gui/src/app/view/psbt.rs | 71 +++++++++++------- gui/src/app/view/psbts.rs | 2 +- gui/src/daemon/model.rs | 154 +++++++++++++++++++++++--------------- 3 files changed, 135 insertions(+), 92 deletions(-) diff --git a/gui/src/app/view/psbt.rs b/gui/src/app/view/psbt.rs index a1ace3f00..d76b88443 100644 --- a/gui/src/app/view/psbt.rs +++ b/gui/src/app/view/psbt.rs @@ -225,13 +225,18 @@ pub fn spend_header<'a>( Row::new() .align_items(Alignment::Center) .push(h3("Miner fee: ").style(color::GREY_3)) - .push(amount_with_size(&tx.fee_amount, H3_SIZE)) + .push_maybe(if tx.fee_amount.is_none() { + Some(text("Missing information about transaction inputs")) + } else { + None + }) + .push_maybe(tx.fee_amount.map(|fee| amount_with_size(&fee, H3_SIZE))) .push(text(" ").size(H3_SIZE)) - .push( - text(format!("(~{} sats/vbyte)", &tx.min_feerate_vb())) + .push_maybe(tx.min_feerate_vb().map(|rate| { + text(format!("(~{} sats/vbyte)", &rate)) .size(H4_SIZE) - .style(color::GREY_3), - ), + .style(color::GREY_3) + })), ), ) .into() @@ -521,7 +526,7 @@ pub fn path_view<'a>( } pub fn inputs_and_outputs_view<'a>( - coins: &'a [Coin], + coins: &'a HashMap, tx: &'a Transaction, network: Network, change_indexes: Option>, @@ -572,12 +577,17 @@ pub fn inputs_and_outputs_view<'a>( .style(theme::Button::TransparentBorder) }, move || { - coins + tx.input .iter() .fold( Column::new().spacing(10).padding(20), - |col: Column<'a, Message>, coin| { - col.push(input_view(coin, labels, labels_editing)) + |col: Column<'a, Message>, input| { + col.push(input_view( + &input.previous_output, + coins.get(&input.previous_output), + labels, + labels_editing, + )) }, ) .into() @@ -729,12 +739,12 @@ pub fn inputs_and_outputs_view<'a>( } fn input_view<'a>( - coin: &'a Coin, + outpoint: &'a OutPoint, + coin: Option<&'a Coin>, labels: &'a HashMap, labels_editing: &'a HashMap>, ) -> Element<'a, Message> { - let outpoint = coin.outpoint.to_string(); - let addr = coin.address.to_string(); + let outpoint = outpoint.to_string(); Column::new() .width(Length::Fill) .push( @@ -753,7 +763,7 @@ fn input_view<'a>( }) .width(Length::Fill), ) - .push(amount(&coin.amount)), + .push_maybe(coin.map(|c| amount(&c.amount))), ) .push( Column::new() @@ -765,11 +775,12 @@ fn input_view<'a>( .push(p2_regular(outpoint.clone()).style(color::GREY_3)) .push( Button::new(icon::clipboard_icon().style(color::GREY_3)) - .on_press(Message::Clipboard(coin.outpoint.to_string())) + .on_press(Message::Clipboard(outpoint.clone())) .style(theme::Button::TransparentBorder), ), ) - .push( + .push_maybe(coin.map(|c| { + let addr = c.address.to_string(); Row::new() .align_items(Alignment::Center) .width(Length::Fill) @@ -782,23 +793,25 @@ fn input_view<'a>( .push(p2_regular(addr.clone()).style(color::GREY_3)) .push( Button::new(icon::clipboard_icon().style(color::GREY_3)) - .on_press(Message::Clipboard(addr.clone())) + .on_press(Message::Clipboard(addr)) .style(theme::Button::TransparentBorder), ), - ), - ) - .push_maybe(labels.get(&addr).map(|label| { - Row::new() - .align_items(Alignment::Center) - .width(Length::Fill) - .push( - Row::new() - .align_items(Alignment::Center) - .width(Length::Fill) - .spacing(5) - .push(p1_bold("Address label:").style(color::GREY_3)) - .push(p2_regular(label).style(color::GREY_3)), ) + })) + .push_maybe(coin.and_then(|c| { + labels.get(&c.address.to_string()).map(|label| { + Row::new() + .align_items(Alignment::Center) + .width(Length::Fill) + .push( + Row::new() + .align_items(Alignment::Center) + .width(Length::Fill) + .spacing(5) + .push(p1_bold("Address label:").style(color::GREY_3)) + .push(p2_regular(label).style(color::GREY_3)), + ) + }) })), ) .spacing(5) diff --git a/gui/src/app/view/psbts.rs b/gui/src/app/view/psbts.rs index 6e25a2bcd..66252f9c9 100644 --- a/gui/src/app/view/psbts.rs +++ b/gui/src/app/view/psbts.rs @@ -152,7 +152,7 @@ fn spend_tx_list_view(i: usize, tx: &SpendTx) -> Element<'_, Message> { } else { Container::new(p1_regular("Self-transfer")) }) - .push(amount_with_size(&tx.fee_amount, P2_SIZE)) + .push_maybe(tx.fee_amount.map(|fee| amount_with_size(&fee, P2_SIZE))) .width(Length::Shrink), ) .align_items(Alignment::Center) diff --git a/gui/src/daemon/model.rs b/gui/src/daemon/model.rs index b62c06f23..344a19a1e 100644 --- a/gui/src/daemon/model.rs +++ b/gui/src/daemon/model.rs @@ -32,12 +32,12 @@ pub fn remaining_sequence(coin: &Coin, blockheight: u32, timelock: u16) -> u32 { #[derive(Debug, Clone)] pub struct SpendTx { pub network: Network, - pub coins: Vec, + pub coins: HashMap, pub labels: HashMap, pub psbt: Psbt, pub change_indexes: Vec, pub spend_amount: Amount, - pub fee_amount: Amount, + pub fee_amount: Option, /// The maximum size difference (in virtual bytes) of /// an input in this transaction before and after satisfaction. pub max_sat_vbytes: usize, @@ -77,10 +77,9 @@ impl SpendTx { }, ); - let mut inputs_amount = Amount::from_sat(0); let mut status = SpendStatus::Pending; - for coin in &coins { - inputs_amount += coin.amount; + let mut coins_map = HashMap::::new(); + for coin in coins { if let Some(info) = coin.spend_info { if info.txid == psbt.unsigned_tx.txid() { if info.height.is_some() { @@ -92,7 +91,40 @@ impl SpendTx { status = SpendStatus::Deprecated } } + coins_map.insert(coin.outpoint, coin); + } + + let inputs_amount = { + let mut inputs_amount = Amount::from_sat(0); + for (i, input) in psbt.inputs.iter().enumerate() { + if let Some(utxo) = &input.witness_utxo { + inputs_amount += Amount::from_sat(utxo.value); + // we try to have it from the coin + } else if let Some(coin) = psbt + .unsigned_tx + .input + .get(i) + .and_then(|inpt| coins_map.get(&inpt.previous_output)) + { + inputs_amount += coin.amount; + // Information is missing, it is better to set inputs_amount to None. + } else { + inputs_amount = Amount::from_sat(0); + break; + } + } + if inputs_amount.to_sat() == 0 { + None + } else { + Some(inputs_amount) + } + }; + + // One input coin is missing, the psbt is deprecated for now. + if coins_map.len() != psbt.inputs.len() { + status = SpendStatus::Deprecated } + let sigs = desc .partial_spend_info(&psbt) .expect("PSBT must be generated by Liana"); @@ -125,11 +157,11 @@ impl SpendTx { } }, updated_at, - coins, + coins: coins_map, psbt, change_indexes, spend_amount, - fee_amount: inputs_amount - spend_amount - change_amount, + fee_amount: inputs_amount.and_then(|a| a.checked_sub(spend_amount + change_amount)), max_sat_vbytes, status, sigs, @@ -165,11 +197,11 @@ impl SpendTx { } /// Feerate obtained if all transaction inputs have the maximum satisfaction size. - pub fn min_feerate_vb(&self) -> u64 { + pub fn min_feerate_vb(&self) -> Option { // This assumes all inputs are internal (have same max satisfaction size). let max_tx_vbytes = self.psbt.unsigned_tx.vsize() + (self.max_sat_vbytes * self.psbt.inputs.len()); - self.fee_amount.to_sat() / max_tx_vbytes as u64 + self.fee_amount.map(|a| a.to_sat() / max_tx_vbytes as u64) } pub fn is_send_to_self(&self) -> bool { @@ -200,7 +232,7 @@ impl Labelled for SpendTx { let mut items = Vec::new(); let txid = self.psbt.unsigned_tx.txid(); items.push(LabelItem::Txid(txid)); - for coin in &self.coins { + for coin in self.coins.values() { items.push(LabelItem::Address(coin.address.clone())); items.push(LabelItem::OutPoint(coin.outpoint)); } @@ -221,7 +253,7 @@ impl Labelled for SpendTx { pub struct HistoryTransaction { pub network: Network, pub labels: HashMap, - pub coins: Vec, + pub coins: HashMap, pub change_indexes: Vec, pub tx: Transaction, pub outgoing_amount: Amount, @@ -252,66 +284,64 @@ impl HistoryTransaction { }, ); + let kind = if coins.is_empty() { + if change_indexes.len() == 1 { + TransactionKind::IncomingSinglePayment(OutPoint { + txid: tx.txid(), + vout: change_indexes[0] as u32, + }) + } else { + TransactionKind::IncomingPaymentBatch( + change_indexes + .iter() + .map(|i| OutPoint { + txid: tx.txid(), + vout: *i as u32, + }) + .collect(), + ) + } + } else if outgoing_amount == Amount::from_sat(0) { + TransactionKind::SendToSelf + } else { + let outpoints: Vec = tx + .output + .iter() + .enumerate() + .filter_map(|(i, _)| { + if !change_indexes.contains(&i) { + Some(OutPoint { + txid: tx.txid(), + vout: i as u32, + }) + } else { + None + } + }) + .collect(); + if outpoints.len() == 1 { + TransactionKind::OutgoingSinglePayment(outpoints[0]) + } else { + TransactionKind::OutgoingPaymentBatch(outpoints) + } + }; + let mut inputs_amount = Amount::from_sat(0); - for coin in &coins { + let mut coins_map = HashMap::::with_capacity(coins.len()); + for coin in coins { inputs_amount += coin.amount; + coins_map.insert(coin.outpoint, coin); } - let fee_amount = if inputs_amount > outgoing_amount + incoming_amount { - Some(inputs_amount - outgoing_amount - incoming_amount) - } else { - None - }; - Self { labels: HashMap::new(), - kind: if coins.is_empty() { - if change_indexes.len() == 1 { - TransactionKind::IncomingSinglePayment(OutPoint { - txid: tx.txid(), - vout: change_indexes[0] as u32, - }) - } else { - TransactionKind::IncomingPaymentBatch( - change_indexes - .iter() - .map(|i| OutPoint { - txid: tx.txid(), - vout: *i as u32, - }) - .collect(), - ) - } - } else if outgoing_amount == Amount::from_sat(0) { - TransactionKind::SendToSelf - } else { - let outpoints: Vec = tx - .output - .iter() - .enumerate() - .filter_map(|(i, _)| { - if !change_indexes.contains(&i) { - Some(OutPoint { - txid: tx.txid(), - vout: i as u32, - }) - } else { - None - } - }) - .collect(); - if outpoints.len() == 1 { - TransactionKind::OutgoingSinglePayment(outpoints[0]) - } else { - TransactionKind::OutgoingPaymentBatch(outpoints) - } - }, + kind, tx, - coins, + coins: coins_map, change_indexes, outgoing_amount, incoming_amount, - fee_amount, + fee_amount: inputs_amount.checked_sub(outgoing_amount + incoming_amount), height, time, network, @@ -362,7 +392,7 @@ impl Labelled for HistoryTransaction { let mut items = Vec::new(); let txid = self.tx.txid(); items.push(LabelItem::Txid(txid)); - for coin in &self.coins { + for coin in self.coins.values() { items.push(LabelItem::Address(coin.address.clone())); items.push(LabelItem::OutPoint(coin.outpoint)); }