From 946e499b5a1f0f340169ace8228bddb451a8a7a2 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 | 156 +++++++++++++++++++++++--------------- 3 files changed, 136 insertions(+), 93 deletions(-) diff --git a/gui/src/app/view/psbt.rs b/gui/src/app/view/psbt.rs index a1ace3f0..d76b8844 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 6e25a2bc..66252f9c 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 b62c06f2..06ab8144 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::::with_capacity(coins.len()); + 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 mut inputs_amount = Amount::from_sat(0); - for coin in &coins { - inputs_amount += coin.amount; - } - - let fee_amount = if inputs_amount > outgoing_amount + incoming_amount { - Some(inputs_amount - outgoing_amount - incoming_amount) + 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 { - None + 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); + let mut coins_map = HashMap::::with_capacity(coins.len()); + for coin in coins { + inputs_amount += coin.amount; + coins_map.insert(coin.outpoint, coin); + } + 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)); }