From 6de5016c0886d297350b47267e3a6614bae38df3 Mon Sep 17 00:00:00 2001 From: edouardparis Date: Wed, 30 Oct 2024 19:46:36 +0100 Subject: [PATCH 1/2] Refac pagination: sort list in async command --- gui/src/app/message.rs | 1 - gui/src/app/state/mod.rs | 61 +++++++++++------------ gui/src/app/state/transactions.rs | 80 +++++++++++++------------------ gui/src/app/view/home.rs | 13 +---- gui/src/app/view/transactions.rs | 15 +----- gui/src/daemon/model.rs | 2 + 6 files changed, 64 insertions(+), 108 deletions(-) diff --git a/gui/src/app/message.rs b/gui/src/app/message.rs index dd466946..ea5e0ef6 100644 --- a/gui/src/app/message.rs +++ b/gui/src/app/message.rs @@ -42,7 +42,6 @@ pub enum Message { HardwareWallets(HardwareWalletMessage), HistoryTransactionsExtension(Result, Error>), HistoryTransactions(Result, Error>), - PendingTransactions(Result, Error>), LabelsUpdated(Result>, Error>), BroadcastModal(Result, Error>), RbfModal(Box, bool, Result, Error>), diff --git a/gui/src/app/state/mod.rs b/gui/src/app/state/mod.rs index 682864a0..a71a968f 100644 --- a/gui/src/app/state/mod.rs +++ b/gui/src/app/state/mod.rs @@ -80,7 +80,6 @@ pub struct Home { unconfirmed_balance: Amount, remaining_sequence: Option, expiring_coins: Vec, - pending_events: Vec, events: Vec, is_last_page: bool, processing: bool, @@ -113,7 +112,6 @@ impl Home { expiring_coins: Vec::new(), selected_event: None, events: Vec::new(), - pending_events: Vec::new(), labels_edited: LabelsEdited::default(), warning: None, is_last_page: false, @@ -125,14 +123,9 @@ impl Home { impl State for Home { fn view<'a>(&'a self, cache: &'a Cache) -> Element<'a, view::Message> { if let Some((i, output_index)) = self.selected_event { - let event = if i < self.pending_events.len() { - &self.pending_events[i] - } else { - &self.events[i - self.pending_events.len()] - }; view::home::payment_view( cache, - event, + &self.events[i], output_index, self.labels_edited.cache(), self.warning.as_ref(), @@ -147,7 +140,6 @@ impl State for Home { &self.unconfirmed_balance, &self.remaining_sequence, &self.expiring_coins, - &self.pending_events, &self.events, self.is_last_page, self.processing, @@ -203,7 +195,6 @@ impl State for Home { Ok(events) => { self.warning = None; self.events = events; - self.events.sort_by(|a, b| b.time.cmp(&a.time)); self.is_last_page = (self.events.len() as u64) < HISTORY_EVENT_PAGE_SIZE; } }, @@ -213,19 +204,25 @@ impl State for Home { self.processing = false; self.warning = None; self.is_last_page = (events.len() as u64) < HISTORY_EVENT_PAGE_SIZE; - for event in events { - if !self.events.iter().any(|other| other.tx == event.tx) { - self.events.push(event); + if let Some(event) = events.first() { + if let Some(position) = self + .events + .iter() + .position(|event2| event2.txid == event.txid) + { + let len = self.events.len(); + for event in events { + if !self.events[position..len] + .iter() + .any(|event2| event2.txid == event.txid) + { + self.events.push(event); + } + } + } else { + self.events.extend(events); } } - self.events.sort_by(|a, b| b.time.cmp(&a.time)); - } - }, - Message::PendingTransactions(res) => match res { - Err(e) => self.warning = Some(e), - Ok(events) => { - self.warning = None; - self.pending_events = events; } }, Message::UpdatePanelCache(is_current, Ok(cache)) => { @@ -246,10 +243,7 @@ impl State for Home { match self.labels_edited.update( daemon, message, - self.pending_events - .iter_mut() - .map(|tx| tx as &mut dyn Labelled) - .chain(self.events.iter_mut().map(|tx| tx as &mut dyn Labelled)), + self.events.iter_mut().map(|tx| tx as &mut dyn Labelled), ) { Ok(cmd) => { return cmd; @@ -302,6 +296,7 @@ impl State for Home { limit += HISTORY_EVENT_PAGE_SIZE; events = daemon.list_history_txs(0, last_event_date, limit).await?; } + events.sort_by(|a, b| b.time.cmp(&a.time)); Ok(events) }, Message::HistoryTransactionsExtension, @@ -324,9 +319,7 @@ impl State for Home { } self.selected_event = None; self.wallet = wallet; - let daemon1 = daemon.clone(); let daemon2 = daemon.clone(); - let daemon3 = daemon.clone(); let now: u32 = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() @@ -334,16 +327,16 @@ impl State for Home { .try_into() .unwrap(); Command::batch(vec![ - Command::perform( - async move { daemon3.list_pending_txs().await.map_err(|e| e.into()) }, - Message::PendingTransactions, - ), Command::perform( async move { - daemon1 + let mut txs = daemon .list_history_txs(0, now, HISTORY_EVENT_PAGE_SIZE) - .await - .map_err(|e| e.into()) + .await?; + txs.sort_by(|a, b| b.time.cmp(&a.time)); + + let mut pending_txs = daemon.list_pending_txs().await?; + pending_txs.extend(txs); + Ok(pending_txs) }, Message::HistoryTransactions, ), diff --git a/gui/src/app/state/transactions.rs b/gui/src/app/state/transactions.rs index e0e15e6f..fbe9edbe 100644 --- a/gui/src/app/state/transactions.rs +++ b/gui/src/app/state/transactions.rs @@ -37,7 +37,6 @@ use crate::daemon::{ pub struct TransactionsPanel { wallet: Arc, - pending_txs: Vec, txs: Vec, labels_edited: LabelsEdited, selected_tx: Option, @@ -53,7 +52,6 @@ impl TransactionsPanel { wallet, selected_tx: None, txs: Vec::new(), - pending_txs: Vec::new(), labels_edited: LabelsEdited::default(), warning: None, create_rbf_modal: None, @@ -86,7 +84,6 @@ impl State for TransactionsPanel { } else { view::transactions::transactions_view( cache, - &self.pending_txs, &self.txs, self.warning.as_ref(), self.is_last_page, @@ -108,7 +105,6 @@ impl State for TransactionsPanel { self.warning = None; self.txs = txs; self.is_last_page = (self.txs.len() as u64) < HISTORY_EVENT_PAGE_SIZE; - self.txs.sort_by(|a, b| b.time.cmp(&a.time)); } }, Message::HistoryTransactionsExtension(res) => match res { @@ -117,21 +113,22 @@ impl State for TransactionsPanel { self.processing = false; self.warning = None; self.is_last_page = (txs.len() as u64) < HISTORY_EVENT_PAGE_SIZE; - for tx in txs { - if let Some(t) = self.txs.iter_mut().find(|other| other.tx == tx.tx) { - t.labels = tx.labels; + if let Some(tx) = txs.first() { + if let Some(position) = self.txs.iter().position(|tx2| tx2.txid == tx.txid) + { + let len = self.txs.len(); + for tx in txs { + if !self.txs[position..len] + .iter() + .any(|tx2| tx2.txid == tx.txid) + { + self.txs.push(tx); + } + } } else { - self.txs.push(tx); + self.txs.extend(txs); } } - self.txs.sort_by(|a, b| b.time.cmp(&a.time)); - } - }, - Message::PendingTransactions(res) => match res { - Err(e) => self.warning = Some(e), - Ok(txs) => { - self.warning = None; - self.pending_txs = txs; } }, Message::RbfModal(tx, is_cancel, res) => match res { @@ -147,11 +144,7 @@ impl State for TransactionsPanel { return self.reload(daemon, self.wallet.clone()); } Message::View(view::Message::Select(i)) => { - self.selected_tx = if i < self.pending_txs.len() { - self.pending_txs.get(i).cloned() - } else { - self.txs.get(i - self.pending_txs.len()).cloned() - }; + self.selected_tx = self.txs.get(i).cloned(); // Clear modal if it's for a different tx. if let Some(modal) = &self.create_rbf_modal { if Some(modal.tx.tx.txid()) @@ -199,15 +192,11 @@ impl State for TransactionsPanel { match self.labels_edited.update( daemon, message, - self.pending_txs - .iter_mut() - .map(|tx| tx as &mut dyn Labelled) - .chain(self.txs.iter_mut().map(|tx| tx as &mut dyn Labelled)) - .chain( - self.selected_tx - .iter_mut() - .map(|tx| tx as &mut dyn Labelled), - ), + self.txs.iter_mut().map(|tx| tx as &mut dyn Labelled).chain( + self.selected_tx + .iter_mut() + .map(|tx| tx as &mut dyn Labelled), + ), ) { Ok(cmd) => { return cmd; @@ -250,6 +239,7 @@ impl State for TransactionsPanel { limit += HISTORY_EVENT_PAGE_SIZE; txs = daemon.list_history_txs(0, last_tx_date, limit).await?; } + txs.sort_by(|a, b| b.time.cmp(&a.time)); Ok(txs) }, Message::HistoryTransactionsExtension, @@ -271,29 +261,25 @@ impl State for TransactionsPanel { _wallet: Arc, ) -> Command { self.selected_tx = None; - let daemon1 = daemon.clone(); - let daemon2 = daemon.clone(); let now: u32 = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() .as_secs() .try_into() .unwrap(); - Command::batch(vec![ - Command::perform( - async move { daemon2.list_pending_txs().await.map_err(|e| e.into()) }, - Message::PendingTransactions, - ), - Command::perform( - async move { - daemon1 - .list_history_txs(0, now, HISTORY_EVENT_PAGE_SIZE) - .await - .map_err(|e| e.into()) - }, - Message::HistoryTransactions, - ), - ]) + Command::batch(vec![Command::perform( + async move { + let mut txs = daemon + .list_history_txs(0, now, HISTORY_EVENT_PAGE_SIZE) + .await?; + txs.sort_by(|a, b| b.time.cmp(&a.time)); + + let mut pending_txs = daemon.list_pending_txs().await?; + pending_txs.extend(txs); + Ok(pending_txs) + }, + Message::HistoryTransactions, + )]) } } diff --git a/gui/src/app/view/home.rs b/gui/src/app/view/home.rs index c4bcdead..007cb3b4 100644 --- a/gui/src/app/view/home.rs +++ b/gui/src/app/view/home.rs @@ -32,7 +32,6 @@ pub fn home_view<'a>( unconfirmed_balance: &'a bitcoin::Amount, remaining_sequence: &Option, expiring_coins: &[bitcoin::OutPoint], - pending_events: &'a [HistoryTransaction], events: &'a [HistoryTransaction], is_last_page: bool, processing: bool, @@ -148,21 +147,11 @@ pub fn home_view<'a>( Column::new() .spacing(10) .push(h4_bold("Last payments")) - .push(pending_events.iter().enumerate().fold( - Column::new().spacing(10), - |col, (i, event)| { - if !event.is_send_to_self() { - col.push(event_list_view(i, event)) - } else { - col - } - }, - )) .push(events.iter().enumerate().fold( Column::new().spacing(10), |col, (i, event)| { if !event.is_send_to_self() { - col.push(event_list_view(i + pending_events.len(), event)) + col.push(event_list_view(i, event)) } else { col } diff --git a/gui/src/app/view/transactions.rs b/gui/src/app/view/transactions.rs index c035c45a..dc51f919 100644 --- a/gui/src/app/view/transactions.rs +++ b/gui/src/app/view/transactions.rs @@ -22,7 +22,6 @@ use crate::{ pub fn transactions_view<'a>( cache: &'a Cache, - pending_txs: &'a [HistoryTransaction], txs: &'a [HistoryTransaction], warning: Option<&'a Error>, is_last_page: bool, @@ -37,23 +36,11 @@ pub fn transactions_view<'a>( .push( Column::new() .spacing(10) - .push_maybe(if !pending_txs.is_empty() { - Some( - pending_txs - .iter() - .enumerate() - .fold(Column::new().spacing(10), |col, (i, tx)| { - col.push(tx_list_view(i, tx)) - }), - ) - } else { - None - }) .push( txs.iter() .enumerate() .fold(Column::new().spacing(10), |col, (i, tx)| { - col.push(tx_list_view(i + pending_txs.len(), tx)) + col.push(tx_list_view(i, tx)) }), ) .push_maybe(if !is_last_page && !txs.is_empty() { diff --git a/gui/src/daemon/model.rs b/gui/src/daemon/model.rs index bc1d315b..2a7e05df 100644 --- a/gui/src/daemon/model.rs +++ b/gui/src/daemon/model.rs @@ -275,6 +275,7 @@ pub struct HistoryTransaction { pub coins: HashMap, pub change_indexes: Vec, pub tx: Transaction, + pub txid: Txid, pub outgoing_amount: Amount, pub incoming_amount: Amount, pub fee_amount: Option, @@ -355,6 +356,7 @@ impl HistoryTransaction { Self { labels: HashMap::new(), kind, + txid: tx.txid(), tx, coins: coins_map, change_indexes, From c620c8278ffb8a051a5613be7620b2589c6a19f7 Mon Sep 17 00:00:00 2001 From: edouardparis Date: Mon, 4 Nov 2024 16:44:27 +0100 Subject: [PATCH 2/2] Add specific comparaison order for HistoryTransaction --- gui/src/app/state/mod.rs | 4 ++-- gui/src/app/state/transactions.rs | 4 ++-- gui/src/daemon/model.rs | 13 +++++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/gui/src/app/state/mod.rs b/gui/src/app/state/mod.rs index a71a968f..070d9143 100644 --- a/gui/src/app/state/mod.rs +++ b/gui/src/app/state/mod.rs @@ -296,7 +296,7 @@ impl State for Home { limit += HISTORY_EVENT_PAGE_SIZE; events = daemon.list_history_txs(0, last_event_date, limit).await?; } - events.sort_by(|a, b| b.time.cmp(&a.time)); + events.sort_by(|a, b| a.compare(b)); Ok(events) }, Message::HistoryTransactionsExtension, @@ -332,7 +332,7 @@ impl State for Home { let mut txs = daemon .list_history_txs(0, now, HISTORY_EVENT_PAGE_SIZE) .await?; - txs.sort_by(|a, b| b.time.cmp(&a.time)); + txs.sort_by(|a, b| a.compare(b)); let mut pending_txs = daemon.list_pending_txs().await?; pending_txs.extend(txs); diff --git a/gui/src/app/state/transactions.rs b/gui/src/app/state/transactions.rs index fbe9edbe..c7285109 100644 --- a/gui/src/app/state/transactions.rs +++ b/gui/src/app/state/transactions.rs @@ -239,7 +239,7 @@ impl State for TransactionsPanel { limit += HISTORY_EVENT_PAGE_SIZE; txs = daemon.list_history_txs(0, last_tx_date, limit).await?; } - txs.sort_by(|a, b| b.time.cmp(&a.time)); + txs.sort_by(|a, b| a.compare(b)); Ok(txs) }, Message::HistoryTransactionsExtension, @@ -272,7 +272,7 @@ impl State for TransactionsPanel { let mut txs = daemon .list_history_txs(0, now, HISTORY_EVENT_PAGE_SIZE) .await?; - txs.sort_by(|a, b| b.time.cmp(&a.time)); + txs.sort_by(|a, b| a.compare(b)); let mut pending_txs = daemon.list_pending_txs().await?; pending_txs.extend(txs); diff --git a/gui/src/daemon/model.rs b/gui/src/daemon/model.rs index 2a7e05df..072f4775 100644 --- a/gui/src/daemon/model.rs +++ b/gui/src/daemon/model.rs @@ -1,3 +1,4 @@ +use std::cmp::Ordering; use std::collections::{HashMap, HashSet}; use liana::descriptors::LianaDescriptor; @@ -369,6 +370,18 @@ impl HistoryTransaction { } } + pub fn compare(&self, other: &Self) -> Ordering { + match (&self.time, &other.time) { + // `None` values come first + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + // Both are `None`, so we consider them equal + (None, None) => self.txid.cmp(&other.txid), + // Both are `Some`, compare by descending time, then by txid + (Some(time1), Some(time2)) => time2.cmp(time1).then_with(|| self.txid.cmp(&other.txid)), + } + } + pub fn is_external(&self) -> bool { matches!( self.kind,