From 746c1831c5180041d904bebde328e980d9f5f7e7 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 22 Nov 2022 12:04:17 +0000 Subject: [PATCH 01/11] add query pagination options struct --- libwallet/src/api_impl/owner.rs | 6 ++++-- libwallet/src/api_impl/types.rs | 28 ++++++++++++++++++++++++++++ libwallet/src/lib.rs | 2 +- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index 96cbfdc7..5e410ddc 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -33,8 +33,9 @@ use crate::types::{AcctPathMapping, NodeClient, TxLogEntry, WalletBackend, Walle use crate::Error; use crate::{ address, wallet_lock, BuiltOutput, InitTxArgs, IssueInvoiceTxArgs, NodeHeightResult, - OutputCommitMapping, PaymentProof, ScannedBlockInfo, Slatepack, SlatepackAddress, Slatepacker, - SlatepackerArgs, TxLogEntryType, ViewWallet, WalletInitStatus, WalletInst, WalletLCProvider, + OutputCommitMapping, PaymentProof, RetrieveTxQueryArgs, ScannedBlockInfo, Slatepack, + SlatepackAddress, Slatepacker, SlatepackerArgs, TxLogEntryType, ViewWallet, WalletInitStatus, + WalletInst, WalletLCProvider, }; use ed25519_dalek::PublicKey as DalekPublicKey; use ed25519_dalek::SecretKey as DalekSecretKey; @@ -313,6 +314,7 @@ pub fn retrieve_txs<'a, L, C, K>( refresh_from_node: bool, tx_id: Option, tx_slate_id: Option, + query_args: Option, ) -> Result<(bool, Vec), Error> where L: WalletLCProvider<'a, C, K>, diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index f54102d7..362685e4 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -148,6 +148,34 @@ impl Default for IssueInvoiceTxArgs { } } +/// Retrieve Transaction List Pagination Arguments +#[derive(Clone, Serialize, Deserialize)] +pub struct RetrieveTxQueryArgs { + /// Retrieve transactions with an id lower than the given, inclusive + /// If None, consider items from the latest transaction and earlier + pub before_id_inc: Option, + /// Retrieve tranactions with an id higher than the given, inclusive + /// If None, consider items from the first transaction and later + pub after_id_inc: Option, + /// The maximum number of transactions to return + /// if both `before_id_inc` and `after_id_inc` are supplied, this will apply + /// to the before and earlier set + pub limit: Option, + /// whether to include cancelled transactions in the returned set + pub include_cancelled: Option, +} + +impl Default for RetrieveTxQueryArgs { + fn default() -> Self { + Self { + before_id_inc: None, + after_id_inc: None, + limit: None, + include_cancelled: None, + } + } +} + /// Fees in block to use for coinbase amount calculation #[derive(Serialize, Deserialize, Debug, Clone)] pub struct BlockFees { diff --git a/libwallet/src/lib.rs b/libwallet/src/lib.rs index 9da67f64..ec17cd01 100644 --- a/libwallet/src/lib.rs +++ b/libwallet/src/lib.rs @@ -65,7 +65,7 @@ pub use crate::slatepack::{ pub use api_impl::owner_updater::StatusMessage; pub use api_impl::types::{ Amount, BlockFees, BuiltOutput, InitTxArgs, InitTxSendArgs, IssueInvoiceTxArgs, - NodeHeightResult, OutputCommitMapping, PaymentProof, VersionInfo, + NodeHeightResult, OutputCommitMapping, PaymentProof, RetrieveTxQueryArgs, VersionInfo, }; pub use internal::scan::scan; pub use slate_versions::ser as dalek_ser; From cd3203be23b31fe0cb8f1a61a123c916ab32e7ae Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 22 Nov 2022 13:25:44 +0000 Subject: [PATCH 02/11] update transaction api calls throughout --- api/src/owner.rs | 8 +++- api/src/owner_rpc.rs | 4 ++ controller/src/command.rs | 5 ++- controller/tests/accounts.rs | 14 +++--- controller/tests/check.rs | 4 +- controller/tests/invoice.rs | 4 +- controller/tests/no_change.rs | 8 ++-- controller/tests/payment_proofs.rs | 2 +- controller/tests/repost.rs | 4 +- controller/tests/revert.rs | 16 +++---- controller/tests/transaction.rs | 20 ++++----- controller/tests/ttl_cutoff.rs | 10 ++--- libwallet/src/api_impl/foreign.rs | 1 + libwallet/src/api_impl/owner.rs | 13 +++++- libwallet/src/api_impl/types.rs | 60 +++++++++++++++++++++++++- libwallet/src/internal/scan.rs | 1 + libwallet/src/internal/tx.rs | 20 +++++++-- libwallet/src/internal/updater.rs | 68 +++++++++++++++++------------- libwallet/src/lib.rs | 3 +- tests/cmd_line_basic.rs | 8 ++-- 20 files changed, 188 insertions(+), 85 deletions(-) diff --git a/api/src/owner.rs b/api/src/owner.rs index 12e54e04..01fbdd09 100644 --- a/api/src/owner.rs +++ b/api/src/owner.rs @@ -16,6 +16,7 @@ use chrono::prelude::*; use ed25519_dalek::SecretKey as DalekSecretKey; +use grin_wallet_libwallet::RetrieveTxQueryArgs; use uuid::Uuid; use crate::config::{TorConfig, WalletConfig}; @@ -447,6 +448,9 @@ where /// the transaction log entry of id `i`. /// * `tx_slate_id` - If `Some(uuid)`, only return transactions associated with /// the given [`Slate`](../grin_wallet_libwallet/slate/struct.Slate.html) uuid. + /// * `tx_query_args` - If provided, use advanced query arguments as documented in + /// (../grin_wallet_libwallet/types.struct.RetrieveTxQueryArgs.html). If either + /// `tx_id` or `tx_slate_id` is provided in the same call, this argument is ignored /// /// # Returns /// * `(bool, Vec, tx_slate_id: Option, + tx_query_args: Option, ) -> Result<(bool, Vec), Error> { let tx = { let t = self.status_tx.lock(); @@ -496,6 +501,7 @@ where refresh_from_node, tx_id, tx_slate_id, + tx_query_args, )?; if self.doctest_mode { res.1 = res diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index 2591577f..beeb6a66 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -13,6 +13,7 @@ // limitations under the License. //! JSON-RPC Stub generation for the Owner API +use grin_wallet_libwallet::RetrieveTxQueryArgs; use uuid::Uuid; use crate::config::{TorConfig, WalletConfig}; @@ -307,6 +308,7 @@ pub trait OwnerRpc { refresh_from_node: bool, tx_id: Option, tx_slate_id: Option, + tx_query_args: Option, ) -> Result<(bool, Vec), Error>; /** @@ -1912,6 +1914,7 @@ where refresh_from_node: bool, tx_id: Option, tx_slate_id: Option, + query_args: Option, ) -> Result<(bool, Vec), Error> { Owner::retrieve_txs( self, @@ -1919,6 +1922,7 @@ where refresh_from_node, tx_id, tx_slate_id, + query_args, ) } diff --git a/controller/src/command.rs b/controller/src/command.rs index 3c11bcd1..938eb829 100644 --- a/controller/src/command.rs +++ b/controller/src/command.rs @@ -1123,7 +1123,8 @@ where let updater_running = owner_api.updater_running.load(Ordering::Relaxed); controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| { let res = api.node_height(m)?; - let (validated, txs) = api.retrieve_txs(m, true, args.id, args.tx_slate_id)?; + // Note advanced query args not currently supported by command line client + let (validated, txs) = api.retrieve_txs(m, true, args.id, args.tx_slate_id, None)?; let include_status = !args.id.is_some() && !args.tx_slate_id.is_some(); // If view count is specified, restrict the TX list to `txs.len() - count` let first_tx = args @@ -1235,7 +1236,7 @@ where } Some(s) => s, }; - let (_, txs) = api.retrieve_txs(m, true, Some(args.id), None)?; + let (_, txs) = api.retrieve_txs(m, true, Some(args.id), None, None)?; match args.dump_file { None => { if txs[0].confirmed { diff --git a/controller/tests/accounts.rs b/controller/tests/accounts.rs index 88cbf576..744a5e14 100644 --- a/controller/tests/accounts.rs +++ b/controller/tests/accounts.rs @@ -135,7 +135,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(wallet1_info.total, 5 * reward); assert_eq!(wallet1_info.amount_currently_spendable, (5 - cm) * reward); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 5); Ok(()) })?; @@ -159,7 +159,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(wallet1_info.total, 7 * reward); assert_eq!(wallet1_info.amount_currently_spendable, 7 * reward); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 7); Ok(()) })?; @@ -178,7 +178,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(wallet1_info.total, 0,); assert_eq!(wallet1_info.amount_currently_spendable, 0,); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 0); Ok(()) })?; @@ -210,7 +210,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(m, true, 1)?; assert!(wallet1_refreshed); assert_eq!(wallet1_info.last_confirmed_height, 13); - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 9); Ok(()) })?; @@ -225,7 +225,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(wallet1_info.last_confirmed_height, 12); let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?; assert_eq!(wallet1_info.last_confirmed_height, 13); - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; println!("{:?}", txs); assert_eq!(txs.len(), 5); Ok(()) @@ -236,7 +236,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { let (wallet2_refreshed, wallet2_info) = api.retrieve_summary_info(m, true, 1)?; assert!(wallet2_refreshed); assert_eq!(wallet2_info.last_confirmed_height, 13); - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); Ok(()) })?; @@ -254,7 +254,7 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(wallet2_info.total, 0,); assert_eq!(wallet2_info.amount_currently_spendable, 0,); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 0); Ok(()) })?; diff --git a/controller/tests/check.rs b/controller/tests/check.rs index 2034acc3..634cdd22 100644 --- a/controller/tests/check.rs +++ b/controller/tests/check.rs @@ -119,7 +119,7 @@ fn scan_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(wallet1_info.total, bh * reward); assert_eq!(wallet1_info.amount_currently_spendable, (bh - cm) * reward); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; let (c, _) = libwallet::TxLogEntry::sum_confirmed(&txs); assert_eq!(wallet1_info.total, c); assert_eq!(txs.len(), bh as usize); @@ -150,7 +150,7 @@ fn scan_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // check we have a problem now wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?; - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; let (c, _) = libwallet::TxLogEntry::sum_confirmed(&txs); assert!(wallet1_info.total != c); Ok(()) diff --git a/controller/tests/invoice.rs b/controller/tests/invoice.rs index 8faf8603..70511d32 100644 --- a/controller/tests/invoice.rs +++ b/controller/tests/invoice.rs @@ -146,7 +146,7 @@ fn invoice_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // Check transaction log for wallet 2 wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, m| { let (_, wallet2_info) = api.retrieve_summary_info(m, true, 1)?; - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); assert!(txs.len() == 1); println!( @@ -161,7 +161,7 @@ fn invoice_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // exists wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?; - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); assert_eq!(txs.len() as u64, bh + 1); println!( diff --git a/controller/tests/no_change.rs b/controller/tests/no_change.rs index b18fb25e..cefba915 100644 --- a/controller/tests/no_change.rs +++ b/controller/tests/no_change.rs @@ -104,7 +104,7 @@ fn no_change_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // Refresh and check transaction log for wallet 1 wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { - let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id))?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; assert!(refreshed); let tx = txs[0].clone(); println!("SIMPLE SEND - SENDING WALLET"); @@ -117,7 +117,7 @@ fn no_change_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // Refresh and check transaction log for wallet 2 wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, m| { - let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id))?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; assert!(refreshed); let tx = txs[0].clone(); println!("SIMPLE SEND - RECEIVING WALLET"); @@ -170,7 +170,7 @@ fn no_change_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // check wallet 2's version wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, m| { - let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id))?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; assert!(refreshed); for tx in txs { stored_excess = tx.kernel_excess; @@ -184,7 +184,7 @@ fn no_change_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { // Refresh and check transaction log for wallet 1 wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { - let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id))?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; assert!(refreshed); for tx in txs { println!("Wallet 1: {:?}", tx); diff --git a/controller/tests/payment_proofs.rs b/controller/tests/payment_proofs.rs index 92f919e3..edb416f3 100644 --- a/controller/tests/payment_proofs.rs +++ b/controller/tests/payment_proofs.rs @@ -116,7 +116,7 @@ fn payment_proofs_test_impl(test_dir: &'static str) -> Result<(), libwallet::Err sender_api.tx_lock_outputs(m, &slate)?; // Ensure what's stored in TX log for payment proof is correct - let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id), None)?; assert!(txs[0].payment_proof.is_some()); let pp = txs[0].clone().payment_proof.unwrap(); assert_eq!( diff --git a/controller/tests/repost.rs b/controller/tests/repost.rs index b0aef3a8..283164cf 100644 --- a/controller/tests/repost.rs +++ b/controller/tests/repost.rs @@ -154,7 +154,7 @@ fn file_repost_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> // Now repost from cached wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { - let (_, txs) = api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; println!("TXS[0]: {:?}", txs[0]); let stored_tx = api.get_stored_tx(m, None, Some(&txs[0].tx_slate_id.unwrap()))?; println!("Stored tx: {:?}", stored_tx); @@ -224,7 +224,7 @@ fn file_repost_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> // Now repost from cached wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { - let (_, txs) = api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; let stored_tx_slate = api.get_stored_tx(m, Some(txs[0].id), None)?.unwrap(); api.post_tx(m, &stored_tx_slate, false)?; bh += 1; diff --git a/controller/tests/revert.rs b/controller/tests/revert.rs index 57e73ad1..ea6c0b2a 100644 --- a/controller/tests/revert.rs +++ b/controller/tests/revert.rs @@ -133,7 +133,7 @@ fn revert( assert_eq!(info.amount_currently_spendable, (bh - cm) * reward); assert_eq!(info.amount_reverted, 0); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; let (c, _) = libwallet::TxLogEntry::sum_confirmed(&txs); assert_eq!(info.total, c); assert_eq!(txs.len(), bh as usize); @@ -148,7 +148,7 @@ fn revert( assert_eq!(info.amount_currently_spendable, 0); assert_eq!(info.amount_reverted, 0); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 0); Ok(()) })?; @@ -188,7 +188,7 @@ fn revert( assert_eq!(info.amount_currently_spendable, 0); assert_eq!(info.amount_reverted, 0); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); let tx = &txs[0]; assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceived); @@ -230,7 +230,7 @@ fn revert( assert_eq!(info.amount_currently_spendable, sent); assert_eq!(info.amount_reverted, 0); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); let tx = &txs[0]; assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceived); @@ -266,7 +266,7 @@ fn revert( assert_eq!(info.amount_currently_spendable, 0); assert_eq!(info.amount_reverted, sent); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); let tx = &txs[0]; assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReverted); @@ -300,7 +300,7 @@ fn revert_reconfirm_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(info.amount_currently_spendable, sent); assert_eq!(info.amount_reverted, 0); // check tx log as well - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); let tx = &txs[0]; assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceived); @@ -329,7 +329,7 @@ fn revert_cancel_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(info.amount_currently_spendable, 0); assert_eq!(info.amount_reverted, sent); - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); let tx = &txs[0]; @@ -345,7 +345,7 @@ fn revert_cancel_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { assert_eq!(info.amount_reverted, 0); // Check updated tx log - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; assert_eq!(txs.len(), 1); let tx = &txs[0]; assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceivedCancelled); diff --git a/controller/tests/transaction.rs b/controller/tests/transaction.rs index 0293555b..fa34014b 100644 --- a/controller/tests/transaction.rs +++ b/controller/tests/transaction.rs @@ -145,7 +145,7 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error> // Check transaction log for wallet 1 wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?; - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); let fee = core::libtx::tx_fee( wallet1_info.last_confirmed_height as usize - cm as usize, @@ -166,7 +166,7 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error> // Check transaction log for wallet 2 wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, m| { - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); // we should have a transaction entry for this slate let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id)); @@ -211,7 +211,7 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(wallet1_info.amount_immature, cm * reward + fee); // check tx log entry is confirmed - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id)); assert!(tx.is_some()); @@ -247,7 +247,7 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(wallet2_info.amount_currently_spendable, amount); // check tx log entry is confirmed - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id)); assert!(tx.is_some()); @@ -315,7 +315,7 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error> wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| { let (refreshed, _wallet1_info) = sender_api.retrieve_summary_info(m, true, 1)?; assert!(refreshed); - let (_, txs) = sender_api.retrieve_txs(m, true, None, None)?; + let (_, txs) = sender_api.retrieve_txs(m, true, None, None, None)?; // find the transaction let tx = txs .iter() @@ -344,7 +344,7 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(wallet2_info.amount_currently_spendable, amount * 3); // check tx log entry is confirmed - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id)); assert!(tx.is_some()); @@ -474,7 +474,7 @@ fn tx_rollback(test_dir: &'static str) -> Result<(), libwallet::Error> { wallet1_info.last_confirmed_height ); assert!(refreshed); - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; // we should have a transaction entry for this slate let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id)); assert!(tx.is_some()); @@ -499,7 +499,7 @@ fn tx_rollback(test_dir: &'static str) -> Result<(), libwallet::Error> { // Check transaction log for wallet 2 wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, m| { - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); let mut unconfirmed_count = 0; let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id)); @@ -529,7 +529,7 @@ fn tx_rollback(test_dir: &'static str) -> Result<(), libwallet::Error> { // can't roll back coinbase let res = api.cancel_tx(m, Some(1), None); assert!(res.is_err()); - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; let tx = txs .iter() .find(|t| t.tx_slate_id == Some(slate.id)) @@ -556,7 +556,7 @@ fn tx_rollback(test_dir: &'static str) -> Result<(), libwallet::Error> { // Wallet 2 rolls back wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, m| { - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; let tx = txs .iter() .find(|t| t.tx_slate_id == Some(slate.id)) diff --git a/controller/tests/ttl_cutoff.rs b/controller/tests/ttl_cutoff.rs index 47c66e9c..ec4340bc 100644 --- a/controller/tests/ttl_cutoff.rs +++ b/controller/tests/ttl_cutoff.rs @@ -95,7 +95,7 @@ fn ttl_cutoff_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> slate = client1.send_tx_slate_direct("wallet2", &slate_i)?; sender_api.tx_lock_outputs(m, &slate)?; - let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id), None)?; let tx = txs[0].clone(); assert_eq!(tx.ttl_cutoff_height, Some(12)); @@ -106,7 +106,7 @@ fn ttl_cutoff_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 2, false); wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| { - let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id), None)?; let tx = txs[0].clone(); assert_eq!(tx.ttl_cutoff_height, Some(12)); @@ -116,7 +116,7 @@ fn ttl_cutoff_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> // Should also be gone in wallet 2, and output gone wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |sender_api, m| { - let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id), None)?; let tx = txs[0].clone(); let outputs = sender_api.retrieve_outputs(m, false, true, None)?.1; assert_eq!(outputs.len(), 0); @@ -144,7 +144,7 @@ fn ttl_cutoff_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> sender_api.tx_lock_outputs(m, &slate_i)?; slate = slate_i; - let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, txs) = sender_api.retrieve_txs(m, true, None, Some(slate.id), None)?; let tx = txs[0].clone(); assert_eq!(tx.ttl_cutoff_height, Some(14)); @@ -156,7 +156,7 @@ fn ttl_cutoff_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> // Wallet 2 will need to have updated past the TTL wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |sender_api, m| { - let (_, _) = sender_api.retrieve_txs(m, true, None, Some(slate.id))?; + let (_, _) = sender_api.retrieve_txs(m, true, None, Some(slate.id), None)?; Ok(()) })?; diff --git a/libwallet/src/api_impl/foreign.rs b/libwallet/src/api_impl/foreign.rs index 7a21075c..be08950f 100644 --- a/libwallet/src/api_impl/foreign.rs +++ b/libwallet/src/api_impl/foreign.rs @@ -82,6 +82,7 @@ where &mut *w, None, Some(ret_slate.id), + None, Some(&parent_key_id), use_test_rng, )?; diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index 5e410ddc..0ab87e47 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -334,7 +334,14 @@ where wallet_lock!(wallet_inst, w); let parent_key_id = w.parent_key_id(); - let txs = updater::retrieve_txs(&mut **w, tx_id, tx_slate_id, Some(&parent_key_id), false)?; + let txs = updater::retrieve_txs( + &mut **w, + tx_id, + tx_slate_id, + query_args, + Some(&parent_key_id), + false, + )?; Ok((validated, txs)) } @@ -405,6 +412,7 @@ where refresh_from_node, tx_id, tx_slate_id, + None, )?; if txs.1.len() != 1 { return Err(Error::PaymentProofRetrieval( @@ -663,6 +671,7 @@ where &mut *w, None, Some(ret_slate.id), + None, Some(&parent_key_id), use_test_rng, )?; @@ -1123,7 +1132,7 @@ where // Step 2: Update outstanding transactions with no change outputs by kernel let mut txs = { wallet_lock!(wallet_inst, w); - updater::retrieve_txs(&mut **w, None, None, Some(&parent_key_id), true)? + updater::retrieve_txs(&mut **w, None, None, None, Some(&parent_key_id), true)? }; result = update_txs_via_kernel(wallet_inst.clone(), keychain_mask, &mut txs)?; if !result { diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index 362685e4..2cc81867 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -23,6 +23,7 @@ use crate::slate_versions::SlateVersion; use crate::types::OutputData; use crate::SlatepackAddress; +use chrono::prelude::*; use ed25519_dalek::Signature as DalekSignature; /// Type for storing amounts (in nanogrins). @@ -148,6 +149,32 @@ impl Default for IssueInvoiceTxArgs { } } +/// Sort tx retrieval order +#[derive(Clone, Serialize, Deserialize)] +pub enum RetrieveTxQuerySortOrder { + /// Ascending + Asc, + /// Descending + Desc, +} + +/// Valid sort fields for a transaction list retrieval query +#[derive(Clone, Serialize, Deserialize)] +pub enum RetrieveTxQuerySortField { + /// Transaction Id + Id, + /// Creation Timestamp + CreationTimestamp, + /// Confirmation Timestamp + ConfirmationTimestamp, + /// TotalAmount (AmountCredited-AmountDebited) + TotalAmount, + /// Amount Credited + AmountCredited, + /// Amount Debited + AmountDebited, +} + /// Retrieve Transaction List Pagination Arguments #[derive(Clone, Serialize, Deserialize)] pub struct RetrieveTxQueryArgs { @@ -163,6 +190,27 @@ pub struct RetrieveTxQueryArgs { pub limit: Option, /// whether to include cancelled transactions in the returned set pub include_cancelled: Option, + /// whether to only consider non-cancelled, outstanding transactions + pub include_outstanding_only: Option, + /// whether to only consider confirmed-only transactions + pub include_confirmed_only: Option, + /// lower bound on the total amount (amount_credited - amount_debited), inclusive + pub min_amount_inc: Option, + /// higher bound on the total amount (amount_credited - amount_debited), inclusive + pub max_amount_inc: Option, + /// lower bound on the creation timestamp, inclusive + pub min_creation_timestamp_inc: Option>, + /// higher bound on on the creation timestamp, inclusive + pub max_creation_timestamp_inc: Option>, + /// lower bound on the confirmation timestamp, inclusive + pub min_confirmed_timestamp_inc: Option>, + /// higher bound on the confirmation timestamp, inclusive + pub max_confirmed_timestamp_inc: Option>, + /// Field within the tranasction list on which to sort + /// defaults to ID if not present + pub sort_field: Option, + /// Sort order, defaults to DESC if not present (most recent is first) + pub sort_order: Option, } impl Default for RetrieveTxQueryArgs { @@ -171,7 +219,17 @@ impl Default for RetrieveTxQueryArgs { before_id_inc: None, after_id_inc: None, limit: None, - include_cancelled: None, + include_cancelled: Some(true), + include_outstanding_only: Some(false), + include_confirmed_only: Some(false), + min_amount_inc: None, + max_amount_inc: None, + min_creation_timestamp_inc: None, + max_creation_timestamp_inc: None, + min_confirmed_timestamp_inc: None, + max_confirmed_timestamp_inc: None, + sort_field: Some(RetrieveTxQuerySortField::Id), + sort_order: Some(RetrieveTxQuerySortOrder::Desc), } } } diff --git a/libwallet/src/internal/scan.rs b/libwallet/src/internal/scan.rs index f8d9f7f1..129e99ca 100644 --- a/libwallet/src/internal/scan.rs +++ b/libwallet/src/internal/scan.rs @@ -378,6 +378,7 @@ where &mut **w, output.tx_log_entry, None, + None, Some(&parent_key_id), false, )?; diff --git a/libwallet/src/internal/tx.rs b/libwallet/src/internal/tx.rs index a4a40c3e..a5dde1aa 100644 --- a/libwallet/src/internal/tx.rs +++ b/libwallet/src/internal/tx.rs @@ -351,7 +351,14 @@ where } else if let Some(tx_slate_id) = tx_slate_id { tx_id_string = tx_slate_id.to_string(); } - let tx_vec = updater::retrieve_txs(wallet, tx_id, tx_slate_id, Some(&parent_key_id), false)?; + let tx_vec = updater::retrieve_txs( + wallet, + tx_id, + tx_slate_id, + None, + Some(&parent_key_id), + false, + )?; if tx_vec.len() != 1 { return Err(Error::TransactionDoesntExist(tx_id_string)); } @@ -390,7 +397,7 @@ where K: Keychain + 'a, { // finalize command - let tx_vec = updater::retrieve_txs(wallet, None, Some(slate.id), None, false)?; + let tx_vec = updater::retrieve_txs(wallet, None, Some(slate.id), None, None, false)?; let mut tx = None; // don't want to assume this is the right tx, in case of self-sending for t in tx_vec { @@ -511,7 +518,14 @@ where C: NodeClient + 'a, K: Keychain + 'a, { - let tx_vec = updater::retrieve_txs(wallet, None, Some(slate.id), Some(parent_key_id), false)?; + let tx_vec = updater::retrieve_txs( + wallet, + None, + Some(slate.id), + None, + Some(parent_key_id), + false, + )?; if tx_vec.is_empty() { return Err(Error::PaymentProof( "TxLogEntry with original proof info not found (is account correct?)".to_owned(), diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index da2209e3..eb93ee3a 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -33,7 +33,7 @@ use crate::internal::keys; use crate::types::{ NodeClient, OutputData, OutputStatus, TxLogEntry, TxLogEntryType, WalletBackend, WalletInfo, }; -use crate::{BlockFees, CbData, OutputCommitMapping}; +use crate::{BlockFees, CbData, OutputCommitMapping, RetrieveTxQueryArgs}; /// Retrieve all of the outputs (doesn't attempt to update from node) pub fn retrieve_outputs<'a, T: ?Sized, C, K>( @@ -94,6 +94,7 @@ pub fn retrieve_txs<'a, T: ?Sized, C, K>( wallet: &mut T, tx_id: Option, tx_slate_id: Option, + query_args: Option, parent_key_id: Option<&Identifier>, outstanding_only: bool, ) -> Result, Error> @@ -102,34 +103,41 @@ where C: NodeClient + 'a, K: Keychain + 'a, { - let mut txs: Vec = wallet - .tx_log_iter() - .filter(|tx_entry| { - let f_pk = match parent_key_id { - Some(k) => tx_entry.parent_key_id == *k, - None => true, - }; - let f_tx_id = match tx_id { - Some(i) => tx_entry.id == i, - None => true, - }; - let f_txs = match tx_slate_id { - Some(t) => tx_entry.tx_slate_id == Some(t), - None => true, - }; - let f_outstanding = match outstanding_only { - true => { - !tx_entry.confirmed - && (tx_entry.tx_type == TxLogEntryType::TxReceived - || tx_entry.tx_type == TxLogEntryType::TxSent - || tx_entry.tx_type == TxLogEntryType::TxReverted) - } - false => true, - }; - f_pk && f_tx_id && f_txs && f_outstanding - }) - .collect(); - txs.sort_by_key(|tx| tx.creation_ts); + let mut txs: Vec = vec![]; + // Adding in new tranasction list query logic. If `tx_id` or `tx_slate_id` + // is provided, then `query_args` is ignored and old logic is followed. + if tx_id.is_some() || tx_slate_id.is_some() { + txs = wallet + .tx_log_iter() + .filter(|tx_entry| { + let f_pk = match parent_key_id { + Some(k) => tx_entry.parent_key_id == *k, + None => true, + }; + let f_tx_id = match tx_id { + Some(i) => tx_entry.id == i, + None => true, + }; + let f_txs = match tx_slate_id { + Some(t) => tx_entry.tx_slate_id == Some(t), + None => true, + }; + let f_outstanding = match outstanding_only { + true => { + !tx_entry.confirmed + && (tx_entry.tx_type == TxLogEntryType::TxReceived + || tx_entry.tx_type == TxLogEntryType::TxSent + || tx_entry.tx_type == TxLogEntryType::TxReverted) + } + false => true, + }; + f_pk && f_tx_id && f_txs && f_outstanding + }) + .collect(); + txs.sort_by_key(|tx| tx.creation_ts); + } else { + // TODO: Call Query Filter Function + } Ok(txs) } @@ -171,7 +179,7 @@ where .filter(|x| x.root_key_id == *parent_key_id && x.status != OutputStatus::Spent) .collect(); - let tx_entries = retrieve_txs(wallet, None, None, Some(&parent_key_id), true)?; + let tx_entries = retrieve_txs(wallet, None, None, None, Some(&parent_key_id), true)?; // Only select outputs that are actually involved in an outstanding transaction let unspents = match update_all { diff --git a/libwallet/src/lib.rs b/libwallet/src/lib.rs index ec17cd01..3c663a9c 100644 --- a/libwallet/src/lib.rs +++ b/libwallet/src/lib.rs @@ -65,7 +65,8 @@ pub use crate::slatepack::{ pub use api_impl::owner_updater::StatusMessage; pub use api_impl::types::{ Amount, BlockFees, BuiltOutput, InitTxArgs, InitTxSendArgs, IssueInvoiceTxArgs, - NodeHeightResult, OutputCommitMapping, PaymentProof, RetrieveTxQueryArgs, VersionInfo, + NodeHeightResult, OutputCommitMapping, PaymentProof, RetrieveTxQueryArgs, + RetrieveTxQuerySortField, RetrieveTxQuerySortOrder, VersionInfo, }; pub use internal::scan::scan; pub use slate_versions::ser as dalek_ser; diff --git a/tests/cmd_line_basic.rs b/tests/cmd_line_basic.rs index 56ecf2d0..bce4a8d0 100644 --- a/tests/cmd_line_basic.rs +++ b/tests/cmd_line_basic.rs @@ -238,7 +238,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller:: None, |api, m| { api.set_active_account(m, "mining")?; - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); assert_eq!(txs.len(), bh as usize); for t in txs { @@ -436,7 +436,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller:: None, |api, m| { api.set_active_account(m, "mining")?; - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); assert_eq!(txs.len(), bh as usize); Ok(()) @@ -509,7 +509,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller:: None, |api, m| { api.set_active_account(m, "mining")?; - let (refreshed, txs) = api.retrieve_txs(m, true, None, None)?; + let (refreshed, txs) = api.retrieve_txs(m, true, None, None, None)?; assert!(refreshed); assert_eq!(txs.len(), bh as usize + 1); Ok(()) @@ -634,7 +634,7 @@ fn command_line_test_impl(test_dir: &str) -> Result<(), grin_wallet_controller:: None, |api, m| { api.set_active_account(m, "default")?; - let (_, txs) = api.retrieve_txs(m, true, None, None)?; + let (_, txs) = api.retrieve_txs(m, true, None, None, None)?; let some_tx_id = txs[0].tx_slate_id.clone(); assert!(some_tx_id.is_some()); tx_id = some_tx_id.unwrap().to_hyphenated().to_string().clone(); From 806aa985d72b24e6dda7f64808edf09b8ee0f37c Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Wed, 23 Nov 2022 15:36:42 +0000 Subject: [PATCH 03/11] added transaction filtering implementation, addition of internal libwallet test to be filled out --- controller/tests/tx_list.rs | 197 ++++++++++++++++++++++++++++++ libwallet/src/api_impl/types.rs | 26 ++-- libwallet/src/internal/updater.rs | 173 +++++++++++++++++++++++++- 3 files changed, 378 insertions(+), 18 deletions(-) create mode 100644 controller/tests/tx_list.rs diff --git a/controller/tests/tx_list.rs b/controller/tests/tx_list.rs new file mode 100644 index 00000000..a227bdaa --- /dev/null +++ b/controller/tests/tx_list.rs @@ -0,0 +1,197 @@ +// Copyright 2021 The Grin Developers +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! tests of advanced TX filtering + +#[macro_use] +extern crate log; +extern crate grin_wallet_controller as wallet; +extern crate grin_wallet_impls as impls; +extern crate grin_wallet_libwallet as libwallet; + +use grin_core as core; +use grin_keychain as keychain; +use grin_util as util; + +use self::libwallet::{InitTxArgs, Slate}; +use impls::test_framework::{self, LocalWalletClient}; +use std::sync::{atomic::Ordering, Arc}; +use std::thread; +use std::time::Duration; +use util::secp::key::SecretKey; +use util::Mutex; + +use self::keychain::ExtKeychain; +use self::libwallet::WalletInst; +use impls::DefaultLCProvider; + +mod common; +use common::{clean_output_dir, create_wallet_proxy, setup}; + +fn test_wallet_tx_filtering( + wallet: Arc< + Mutex< + Box< + dyn WalletInst< + 'static, + DefaultLCProvider<'static, LocalWalletClient, ExtKeychain>, + LocalWalletClient, + ExtKeychain, + >, + >, + >, + >, + mask: Option<&SecretKey>, +) -> Result<(), libwallet::Error> { + wallet::controller::owner_single_use(Some(wallet.clone()), mask, None, |api, _m| { + let tx_results = api.retrieve_txs(mask, true, None, None, None)?.1; + for entry in tx_results.iter() { + println!("{:?}", entry); + } + Ok(()) + })?; + Ok(()) +} + +/// Builds a wallet + chain with a few transactions, and return wallet for further testing +fn build_chain_for_tx_filtering( + test_dir: &'static str, + block_height: usize, +) -> Result<(), libwallet::Error> { + // Create a new proxy to simulate server and wallet responses + let mut wallet_proxy = create_wallet_proxy(test_dir); + let chain = wallet_proxy.chain.clone(); + let stopper = wallet_proxy.running.clone(); + + create_wallet_and_add!( + client1, + wallet1, + mask1_i, + test_dir, + "wallet1", + None, + &mut wallet_proxy, + true + ); + let mask1 = (&mask1_i).as_ref(); + debug!("Mask1: {:?}", mask1); + create_wallet_and_add!( + client2, + wallet2, + mask2_i, + test_dir, + "wallet2", + None, + &mut wallet_proxy, + false + ); + let mask2 = (&mask2_i).as_ref(); + debug!("Mask2: {:?}", mask2); + + // Set the wallet proxy listener running + thread::spawn(move || { + if let Err(e) = wallet_proxy.run() { + error!("Wallet Proxy error: {}", e); + } + }); + + // Stop the scanning updater threads because it extends the time needed to build the chain + // exponentially + wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, _m| { + api.stop_updater()?; + Ok(()) + })?; + + wallet::controller::owner_single_use(Some(wallet2.clone()), mask2, None, |api, _m| { + api.stop_updater()?; + Ok(()) + })?; + + // few values to keep things shorter + let reward = core::consensus::REWARD; + + // Start off with a few blocks + let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 3, false); + + for i in 0..block_height { + let mut wallet_1_has_funds = false; + + // Check wallet 1 contents + wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| { + let (_, wallet1_info) = api.retrieve_summary_info(m, true, 1)?; + debug!( + "Wallet 1 spendable - {}", + wallet1_info.amount_currently_spendable + ); + if wallet1_info.amount_currently_spendable > reward { + wallet_1_has_funds = true; + } + Ok(()) + })?; + + if !wallet_1_has_funds { + let _ = + test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 1, false); + continue; + } + + // send a random tx + let num_txs = 1; + for _ in 0..num_txs { + let amount: u64 = i as u64 * 1_000_000; + let mut slate = Slate::blank(1, false); + debug!("Creating TX for {}", amount); + wallet::controller::owner_single_use( + Some(wallet1.clone()), + mask1, + None, + |sender_api, m| { + // note this will increment the block count as part of the transaction "Posting" + let args = InitTxArgs { + src_acct_name: None, + amount: amount, + minimum_confirmations: 1, + max_outputs: 500, + num_change_outputs: 1, + selection_strategy_is_use_all: false, + ..Default::default() + }; + let slate_i = sender_api.init_send_tx(m, args)?; + slate = client1.send_tx_slate_direct("wallet2", &slate_i)?; + sender_api.tx_lock_outputs(m, &slate)?; + slate = sender_api.finalize_tx(m, &slate)?; + Ok(()) + }, + )?; + } + } + + // Perform actual testing + test_wallet_tx_filtering(wallet1, mask1)?; + + // let logging finish + stopper.store(false, Ordering::Relaxed); + thread::sleep(Duration::from_millis(200)); + Ok(()) +} + +#[test] +fn wallet_tx_filtering() { + let test_dir = "test_output/advanced_tx_filtering"; + clean_output_dir(test_dir); + setup(test_dir); + if let Err(e) = build_chain_for_tx_filtering(test_dir, 30) { + panic!("Libwallet Error: {}", e); + } + clean_output_dir(test_dir); +} diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index 2cc81867..ce7221af 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -178,19 +178,19 @@ pub enum RetrieveTxQuerySortField { /// Retrieve Transaction List Pagination Arguments #[derive(Clone, Serialize, Deserialize)] pub struct RetrieveTxQueryArgs { - /// Retrieve transactions with an id lower than the given, inclusive - /// If None, consider items from the latest transaction and earlier - pub before_id_inc: Option, - /// Retrieve tranactions with an id higher than the given, inclusive + /// Retrieve transactions with an id higher than or equal to the given /// If None, consider items from the first transaction and later - pub after_id_inc: Option, + pub min_id_inc: Option, + /// Retrieve tranactions with an id less than or equal to the given + /// If None, consider items from the last transaction and earlier + pub max_id_inc: Option, /// The maximum number of transactions to return /// if both `before_id_inc` and `after_id_inc` are supplied, this will apply /// to the before and earlier set pub limit: Option, - /// whether to include cancelled transactions in the returned set - pub include_cancelled: Option, - /// whether to only consider non-cancelled, outstanding transactions + /// whether to exclude cancelled transactions in the returned set + pub exclude_cancelled: Option, + /// whether to only consider outstanding transactions pub include_outstanding_only: Option, /// whether to only consider confirmed-only transactions pub include_confirmed_only: Option, @@ -209,17 +209,17 @@ pub struct RetrieveTxQueryArgs { /// Field within the tranasction list on which to sort /// defaults to ID if not present pub sort_field: Option, - /// Sort order, defaults to DESC if not present (most recent is first) + /// Sort order, defaults to ASC if not present (earliest is first) pub sort_order: Option, } impl Default for RetrieveTxQueryArgs { fn default() -> Self { Self { - before_id_inc: None, - after_id_inc: None, + min_id_inc: None, + max_id_inc: None, limit: None, - include_cancelled: Some(true), + exclude_cancelled: Some(false), include_outstanding_only: Some(false), include_confirmed_only: Some(false), min_amount_inc: None, @@ -229,7 +229,7 @@ impl Default for RetrieveTxQueryArgs { min_confirmed_timestamp_inc: None, max_confirmed_timestamp_inc: None, sort_field: Some(RetrieveTxQuerySortField::Id), - sort_order: Some(RetrieveTxQuerySortOrder::Desc), + sort_order: Some(RetrieveTxQuerySortOrder::Asc), } } } diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index eb93ee3a..ff8db662 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -25,7 +25,6 @@ use crate::grin_core::global; use crate::grin_core::libtx::proof::ProofBuilder; use crate::grin_core::libtx::reward; use crate::grin_keychain::{Identifier, Keychain, SwitchCommitmentType}; -use crate::grin_util as util; use crate::grin_util::secp::key::SecretKey; use crate::grin_util::secp::pedersen; use crate::grin_util::static_secp_instance; @@ -33,7 +32,11 @@ use crate::internal::keys; use crate::types::{ NodeClient, OutputData, OutputStatus, TxLogEntry, TxLogEntryType, WalletBackend, WalletInfo, }; -use crate::{BlockFees, CbData, OutputCommitMapping, RetrieveTxQueryArgs}; +use crate::{grin_util as util, Amount}; +use crate::{ + BlockFees, CbData, OutputCommitMapping, RetrieveTxQueryArgs, RetrieveTxQuerySortField, + RetrieveTxQuerySortOrder, +}; /// Retrieve all of the outputs (doesn't attempt to update from node) pub fn retrieve_outputs<'a, T: ?Sized, C, K>( @@ -88,6 +91,166 @@ where Ok(res) } +/// Apply advanced filtering to resultset from retrieve_txs below +pub fn apply_advanced_tx_list_filtering<'a, T: ?Sized, C, K>( + wallet: &mut T, + query_args: &RetrieveTxQueryArgs, +) -> Vec +where + T: WalletBackend<'a, C, K>, + C: NodeClient + 'a, + K: Keychain + 'a, +{ + // Apply simple bool, GTE or LTE fields + let mut txs_iter: Box> = Box::new( + wallet + .tx_log_iter() + .filter(|tx_entry| { + if let Some(v) = query_args.exclude_cancelled { + if v { + tx_entry.tx_type != TxLogEntryType::TxReceivedCancelled + && tx_entry.tx_type != TxLogEntryType::TxSentCancelled + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.include_outstanding_only { + if v { + !tx_entry.confirmed + && (tx_entry.tx_type == TxLogEntryType::TxReceived + || tx_entry.tx_type == TxLogEntryType::TxSent + || tx_entry.tx_type == TxLogEntryType::TxReverted) + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.include_confirmed_only { + if v { + tx_entry.confirmed + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.min_id_inc { + tx_entry.id >= v + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.max_id_inc { + tx_entry.id <= v + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.min_amount_inc { + v >= tx_entry.amount_credited - tx_entry.amount_debited + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.max_amount_inc { + v <= tx_entry.amount_credited - tx_entry.amount_debited + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.min_creation_timestamp_inc { + tx_entry.creation_ts >= v + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.min_confirmed_timestamp_inc { + tx_entry.creation_ts <= v + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.min_confirmed_timestamp_inc { + if let Some(t) = tx_entry.confirmation_ts { + t >= v + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.max_confirmed_timestamp_inc { + if let Some(t) = tx_entry.confirmation_ts { + t <= v + } else { + true + } + } else { + true + } + }), + ); + + // Apply limit if requested + if let Some(l) = query_args.limit { + txs_iter = Box::new(txs_iter.take(l as usize)); + } + + let mut return_txs: Vec = txs_iter.collect(); + + // Now apply requested sorting + if let Some(ref s) = query_args.sort_field { + match s { + RetrieveTxQuerySortField::Id => { + return_txs.sort_by_key(|tx| tx.id); + } + RetrieveTxQuerySortField::CreationTimestamp => { + return_txs.sort_by_key(|tx| tx.creation_ts); + } + RetrieveTxQuerySortField::ConfirmationTimestamp => { + return_txs.sort_by_key(|tx| tx.confirmation_ts); + } + RetrieveTxQuerySortField::TotalAmount => { + return_txs.sort_by_key(|tx| tx.amount_credited - tx.amount_debited); + } + RetrieveTxQuerySortField::AmountCredited => { + return_txs.sort_by_key(|tx| tx.amount_credited); + } + RetrieveTxQuerySortField::AmountDebited => { + return_txs.sort_by_key(|tx| tx.amount_debited); + } + } + } else { + return_txs.sort_by_key(|tx| tx.id); + } + + if let Some(ref s) = query_args.sort_order { + match s { + RetrieveTxQuerySortOrder::Desc => return_txs.reverse(), + _ => {} + } + } + + return_txs +} + /// Retrieve all of the transaction entries, or a particular entry /// if `parent_key_id` is set, only return entries from that key pub fn retrieve_txs<'a, T: ?Sized, C, K>( @@ -106,7 +269,9 @@ where let mut txs: Vec = vec![]; // Adding in new tranasction list query logic. If `tx_id` or `tx_slate_id` // is provided, then `query_args` is ignored and old logic is followed. - if tx_id.is_some() || tx_slate_id.is_some() { + if query_args.is_some() && tx_id.is_none() && tx_slate_id.is_none() { + txs = apply_advanced_tx_list_filtering(wallet, &query_args.unwrap()) + } else { txs = wallet .tx_log_iter() .filter(|tx_entry| { @@ -135,8 +300,6 @@ where }) .collect(); txs.sort_by_key(|tx| tx.creation_ts); - } else { - // TODO: Call Query Filter Function } Ok(txs) } From d29b64248a1c46702c32e5561a4f92469dd481b0 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Mon, 28 Nov 2022 09:10:37 +0000 Subject: [PATCH 04/11] rename filter test --- controller/tests/{tx_list.rs => tx_list_filter.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename controller/tests/{tx_list.rs => tx_list_filter.rs} (100%) diff --git a/controller/tests/tx_list.rs b/controller/tests/tx_list_filter.rs similarity index 100% rename from controller/tests/tx_list.rs rename to controller/tests/tx_list_filter.rs From 13b1fc8e0174763fac331c7cabc66e0271fe9a3a Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Mon, 28 Nov 2022 15:10:29 +0000 Subject: [PATCH 05/11] addition of tx statuses to sort fields, fixes to total amount queries, inclusion of bigint, addition of unit tests to exercise filtering --- Cargo.lock | 1 + controller/tests/tx_list_filter.rs | 169 ++++++++++++++++++++++++++++- libwallet/Cargo.toml | 1 + libwallet/src/api_impl/types.rs | 12 ++ libwallet/src/internal/updater.rs | 85 ++++++++++++++- 5 files changed, 259 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e00aab63..99f5740d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1599,6 +1599,7 @@ dependencies = [ "grin_wallet_util", "lazy_static", "log", + "num-bigint", "rand 0.6.5", "regex", "secrecy 0.6.0", diff --git a/controller/tests/tx_list_filter.rs b/controller/tests/tx_list_filter.rs index a227bdaa..4a8676f3 100644 --- a/controller/tests/tx_list_filter.rs +++ b/controller/tests/tx_list_filter.rs @@ -22,6 +22,7 @@ extern crate grin_wallet_libwallet as libwallet; use grin_core as core; use grin_keychain as keychain; use grin_util as util; +use libwallet::{RetrieveTxQueryArgs, RetrieveTxQuerySortField}; use self::libwallet::{InitTxArgs, Slate}; use impls::test_framework::{self, LocalWalletClient}; @@ -54,10 +55,150 @@ fn test_wallet_tx_filtering( mask: Option<&SecretKey>, ) -> Result<(), libwallet::Error> { wallet::controller::owner_single_use(Some(wallet.clone()), mask, None, |api, _m| { - let tx_results = api.retrieve_txs(mask, true, None, None, None)?.1; - for entry in tx_results.iter() { + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.min_id_inc = Some(5); + + // Min ID + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results[0].id, 5); + assert_eq!(tx_results[tx_results.len() - 1].id, 33); + + // Max ID + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.min_id_inc = Some(5); + tx_query_args.max_id_inc = Some(20); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results[0].id, 5); + assert_eq!(tx_results[tx_results.len() - 1].id, 20); + + // Exclude 1 cancelled + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.exclude_cancelled = Some(true); + tx_query_args.min_id_inc = Some(5); + tx_query_args.max_id_inc = Some(50); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 28); + + // Exclude 1 cancelled, show confirmed only + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.exclude_cancelled = Some(true); + tx_query_args.include_confirmed_only = Some(true); + tx_query_args.min_id_inc = Some(5); + tx_query_args.max_id_inc = Some(50); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 14); + + // show outstanding only (including cancelled) + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.exclude_cancelled = Some(false); + tx_query_args.include_outstanding_only = Some(true); + tx_query_args.min_id_inc = Some(5); + tx_query_args.max_id_inc = Some(50); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 15); + + // outstanding only and confirmed only should give empty set + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.exclude_cancelled = Some(false); + tx_query_args.include_outstanding_only = Some(true); + tx_query_args.include_confirmed_only = Some(true); + tx_query_args.min_id_inc = Some(5); + tx_query_args.max_id_inc = Some(50); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 0); + + // include sent only + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.include_sent_only = Some(true); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 15); + + // include received only (none in this set) + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.include_received_only = Some(true); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 0); + + // include reverted only (none in this set) + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.include_reverted_only = Some(true); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 0); + + // include coinbase only + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.include_coinbase_only = Some(true); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 19); + + // Amounts + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.min_amount_inc = Some(60_000_000_000 - 59_963_300_000); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 27); + + // amount, should see as above with coinbases excluded + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.min_amount_inc = Some(60_000_000_000 - 59_963_300_000); + tx_query_args.max_amount_inc = Some(60_000_000_000 - 1); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 8); + + // Amount - should only see coinbase (incoming) + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.min_amount_inc = Some(60_000_000_000); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results.len(), 19); + + // sort order + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.sort_order = Some(libwallet::RetrieveTxQuerySortOrder::Desc); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + + assert_eq!(tx_results[0].id, 33); + assert_eq!(tx_results[tx_results.len() - 1].id, 0); + + // change sort field to amount desc, should have coinbases first + let mut tx_query_args = RetrieveTxQueryArgs::default(); + tx_query_args.sort_order = Some(libwallet::RetrieveTxQuerySortOrder::Desc); + tx_query_args.sort_field = Some(RetrieveTxQuerySortField::TotalAmount); + let tx_results = api + .retrieve_txs(mask, true, None, None, Some(tx_query_args))? + .1; + assert_eq!(tx_results[0].amount_credited, 60_000_000_000); + + /*for entry in tx_results.iter() { println!("{:?}", entry); - } + }*/ + Ok(()) })?; Ok(()) @@ -176,6 +317,28 @@ fn build_chain_for_tx_filtering( } } + // Cancel a tx for filtering testing + let amount: u64 = 1_000_000; + let mut slate = Slate::blank(1, false); + debug!("Creating TX for {}", amount); + wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| { + // note this will increment the block count as part of the transaction "Posting" + let args = InitTxArgs { + src_acct_name: None, + amount: amount, + minimum_confirmations: 1, + max_outputs: 500, + num_change_outputs: 1, + selection_strategy_is_use_all: false, + ..Default::default() + }; + let slate_i = sender_api.init_send_tx(m, args)?; + slate = client1.send_tx_slate_direct("wallet2", &slate_i)?; + sender_api.tx_lock_outputs(m, &slate)?; + sender_api.cancel_tx(m, Some(33), None)?; + Ok(()) + })?; + // Perform actual testing test_wallet_tx_filtering(wallet1, mask1)?; diff --git a/libwallet/Cargo.toml b/libwallet/Cargo.toml index e4762260..05a16627 100644 --- a/libwallet/Cargo.toml +++ b/libwallet/Cargo.toml @@ -34,6 +34,7 @@ curve25519-dalek = "2.1" secrecy = "0.6" bech32 = "0.7" byteorder = "1.3" +num-bigint = "0.2" grin_wallet_util = { path = "../util", version = "5.2.0-alpha.1" } grin_wallet_config = { path = "../config", version = "5.2.0-alpha.1" } diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index ce7221af..213700e1 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -194,6 +194,14 @@ pub struct RetrieveTxQueryArgs { pub include_outstanding_only: Option, /// whether to only consider confirmed-only transactions pub include_confirmed_only: Option, + /// whether to only consider sent transactions + pub include_sent_only: Option, + /// whether to only consider received transactions + pub include_received_only: Option, + /// whether to only consider coinbase transactions + pub include_coinbase_only: Option, + /// whether to only consider reverted transactions + pub include_reverted_only: Option, /// lower bound on the total amount (amount_credited - amount_debited), inclusive pub min_amount_inc: Option, /// higher bound on the total amount (amount_credited - amount_debited), inclusive @@ -222,6 +230,10 @@ impl Default for RetrieveTxQueryArgs { exclude_cancelled: Some(false), include_outstanding_only: Some(false), include_confirmed_only: Some(false), + include_sent_only: Some(false), + include_received_only: Some(false), + include_coinbase_only: Some(false), + include_reverted_only: Some(false), min_amount_inc: None, max_amount_inc: None, min_creation_timestamp_inc: None, diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index ff8db662..e9aea59d 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -38,6 +38,8 @@ use crate::{ RetrieveTxQuerySortOrder, }; +use num_bigint::BigInt; + /// Retrieve all of the outputs (doesn't attempt to update from node) pub fn retrieve_outputs<'a, T: ?Sized, C, K>( wallet: &mut T, @@ -121,9 +123,6 @@ where if let Some(v) = query_args.include_outstanding_only { if v { !tx_entry.confirmed - && (tx_entry.tx_type == TxLogEntryType::TxReceived - || tx_entry.tx_type == TxLogEntryType::TxSent - || tx_entry.tx_type == TxLogEntryType::TxReverted) } else { true } @@ -142,6 +141,52 @@ where true } }) + .filter(|tx_entry| { + if let Some(v) = query_args.include_sent_only { + if v { + tx_entry.tx_type == TxLogEntryType::TxSent + || tx_entry.tx_type == TxLogEntryType::TxSentCancelled + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.include_received_only { + if v { + tx_entry.tx_type == TxLogEntryType::TxReceived + || tx_entry.tx_type == TxLogEntryType::TxReceivedCancelled + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.include_coinbase_only { + if v { + tx_entry.tx_type == TxLogEntryType::ConfirmedCoinbase + } else { + true + } + } else { + true + } + }) + .filter(|tx_entry| { + if let Some(v) = query_args.include_reverted_only { + if v { + tx_entry.tx_type == TxLogEntryType::TxReverted + } else { + true + } + } else { + true + } + }) .filter(|tx_entry| { if let Some(v) = query_args.min_id_inc { tx_entry.id >= v @@ -158,14 +203,34 @@ where }) .filter(|tx_entry| { if let Some(v) = query_args.min_amount_inc { - v >= tx_entry.amount_credited - tx_entry.amount_debited + if tx_entry.tx_type == TxLogEntryType::TxSent + || tx_entry.tx_type == TxLogEntryType::TxSentCancelled + { + BigInt::from(tx_entry.amount_debited) + - BigInt::from(tx_entry.amount_credited) + >= BigInt::from(v) + } else { + BigInt::from(tx_entry.amount_credited) + - BigInt::from(tx_entry.amount_debited) + >= BigInt::from(v) + } } else { true } }) .filter(|tx_entry| { if let Some(v) = query_args.max_amount_inc { - v <= tx_entry.amount_credited - tx_entry.amount_debited + if tx_entry.tx_type == TxLogEntryType::TxSent + || tx_entry.tx_type == TxLogEntryType::TxSentCancelled + { + BigInt::from(tx_entry.amount_debited) + - BigInt::from(tx_entry.amount_credited) + <= BigInt::from(v) + } else { + BigInt::from(tx_entry.amount_credited) + - BigInt::from(tx_entry.amount_debited) + <= BigInt::from(v) + } } else { true } @@ -228,7 +293,15 @@ where return_txs.sort_by_key(|tx| tx.confirmation_ts); } RetrieveTxQuerySortField::TotalAmount => { - return_txs.sort_by_key(|tx| tx.amount_credited - tx.amount_debited); + return_txs.sort_by_key(|tx| { + if tx.tx_type == TxLogEntryType::TxSent + || tx.tx_type == TxLogEntryType::TxSentCancelled + { + BigInt::from(tx.amount_debited) - BigInt::from(tx.amount_credited) + } else { + BigInt::from(tx.amount_credited) - BigInt::from(tx.amount_debited) + } + }); } RetrieveTxQuerySortField::AmountCredited => { return_txs.sort_by_key(|tx| tx.amount_credited); From 1188f3287d9bf2300a9a2a8af0df723da9199c65 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 29 Nov 2022 09:30:45 +0000 Subject: [PATCH 06/11] add doctest exercising advanced query JSON API --- api/src/owner.rs | 2 +- api/src/owner_rpc.rs | 83 +++++++++++++++++++++++++++++++ libwallet/src/api_impl/types.rs | 4 ++ libwallet/src/internal/updater.rs | 4 +- 4 files changed, 90 insertions(+), 3 deletions(-) diff --git a/api/src/owner.rs b/api/src/owner.rs index 01fbdd09..ac80d787 100644 --- a/api/src/owner.rs +++ b/api/src/owner.rs @@ -1167,7 +1167,7 @@ where /// let tx_slate_id = None; /// /// // Return all TxLogEntries - /// let result = api_owner.retrieve_txs(None, update_from_node, tx_id, tx_slate_id); + /// let result = api_owner.retrieve_txs(None, update_from_node, tx_id, tx_slate_id, None); /// /// if let Ok((was_updated, tx_log_entries)) = result { /// let stored_tx = api_owner.get_stored_tx(None, Some(tx_log_entries[0].id), None).unwrap(); diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index beeb6a66..8ce8a5f6 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -300,6 +300,89 @@ pub trait OwnerRpc { # "# # , 2, false, false, false, false); ``` + + # JSON-RPC Example, with advanced query arguments - See (../grin_wallet_libwallet/types.struct.RetrieveTxQueryArgs.html) + + ``` + # grin_wallet_api::doctest_helper_json_rpc_owner_assert_response!( + # r#" + { + "jsonrpc": "2.0", + "method": "retrieve_txs", + "params": { + "token": "d202964900000000d302964900000000d402964900000000d502964900000000", + "refresh_from_node": true, + "tx_id": null, + "tx_slate_id": null, + "tx_query_args": { + "min_id_inc": 0, + "max_id_inc": 100, + "min_amount_inc": "0", + "max_amount_inc": "6000000000", + "sort_field": "Id", + "sort_order": "Desc" + } + }, + "id": 1 + } + # "# + # , + # r#" + { + "id": 1, + "jsonrpc": "2.0", + "result": { + "Ok": [ + true, + [ + { + "amount_credited": "60000000000", + "amount_debited": "0", + "confirmation_ts": "2019-01-15T16:01:26Z", + "confirmed": true, + "creation_ts": "2019-01-15T16:01:26Z", + "fee": null, + "id": 0, + "kernel_excess": "0838e19c490038b10f051c9c190a9b1f96d59bbd242f5d3143f50630deb74342ed", + "kernel_lookup_min_height": 1, + "num_inputs": 0, + "num_outputs": 1, + "parent_key_id": "0200000000000000000000000000000000", + "stored_tx": null, + "ttl_cutoff_height": null, + "tx_slate_id": null, + "payment_proof": null, + "reverted_after": null, + "tx_type": "ConfirmedCoinbase" + }, + { + "amount_credited": "60000000000", + "amount_debited": "0", + "confirmation_ts": "2019-01-15T16:01:26Z", + "confirmed": true, + "creation_ts": "2019-01-15T16:01:26Z", + "fee": null, + "id": 1, + "kernel_excess": "08cd9d890c0b6a004f700aa5939a1ce0488fe2a11fa33cf096b50732ceab0be1df", + "kernel_lookup_min_height": 2, + "num_inputs": 0, + "num_outputs": 1, + "parent_key_id": "0200000000000000000000000000000000", + "stored_tx": null, + "ttl_cutoff_height": null, + "payment_proof": null, + "reverted_after": null, + "tx_slate_id": null, + "tx_type": "ConfirmedCoinbase" + } + ] + ] + } + } + # "# + # , 2, false, false, false, false); + ``` + */ fn retrieve_txs( diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index 213700e1..2c35cc43 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -203,8 +203,12 @@ pub struct RetrieveTxQueryArgs { /// whether to only consider reverted transactions pub include_reverted_only: Option, /// lower bound on the total amount (amount_credited - amount_debited), inclusive + #[serde(with = "secp_ser::opt_string_or_u64")] + #[serde(default)] pub min_amount_inc: Option, /// higher bound on the total amount (amount_credited - amount_debited), inclusive + #[serde(with = "secp_ser::opt_string_or_u64")] + #[serde(default)] pub max_amount_inc: Option, /// lower bound on the creation timestamp, inclusive pub min_creation_timestamp_inc: Option>, diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index e9aea59d..ad5ada1d 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -25,6 +25,7 @@ use crate::grin_core::global; use crate::grin_core::libtx::proof::ProofBuilder; use crate::grin_core::libtx::reward; use crate::grin_keychain::{Identifier, Keychain, SwitchCommitmentType}; +use crate::grin_util as util; use crate::grin_util::secp::key::SecretKey; use crate::grin_util::secp::pedersen; use crate::grin_util::static_secp_instance; @@ -32,7 +33,6 @@ use crate::internal::keys; use crate::types::{ NodeClient, OutputData, OutputStatus, TxLogEntry, TxLogEntryType, WalletBackend, WalletInfo, }; -use crate::{grin_util as util, Amount}; use crate::{ BlockFees, CbData, OutputCommitMapping, RetrieveTxQueryArgs, RetrieveTxQuerySortField, RetrieveTxQuerySortOrder, @@ -339,7 +339,7 @@ where C: NodeClient + 'a, K: Keychain + 'a, { - let mut txs: Vec = vec![]; + let mut txs; // Adding in new tranasction list query logic. If `tx_id` or `tx_slate_id` // is provided, then `query_args` is ignored and old logic is followed. if query_args.is_some() && tx_id.is_none() && tx_slate_id.is_none() { From 17edec25c8e479d284369e5eab6cbd1a7af61bd6 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 29 Nov 2022 10:43:10 +0000 Subject: [PATCH 07/11] split JSON api functions into existing retrieve_tx and new query_txs functions --- api/src/owner_rpc.rs | 51 ++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index 8ce8a5f6..343a5e32 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -216,6 +216,7 @@ pub trait OwnerRpc { # , 2, false, false, false, false); ``` */ + fn retrieve_outputs( &self, token: Token, @@ -300,27 +301,36 @@ pub trait OwnerRpc { # "# # , 2, false, false, false, false); ``` + */ - # JSON-RPC Example, with advanced query arguments - See (../grin_wallet_libwallet/types.struct.RetrieveTxQueryArgs.html) + fn retrieve_txs( + &self, + token: Token, + refresh_from_node: bool, + tx_id: Option, + tx_slate_id: Option, + ) -> Result<(bool, Vec), Error>; + + /** + Networked version of [Owner::retrieve_txs](struct.Owner.html#method.retrieve_txs), which passes only the `tx_query_args` + parameter. See (../grin_wallet_libwallet/types.struct.RetrieveTxQueryArgs.html) ``` # grin_wallet_api::doctest_helper_json_rpc_owner_assert_response!( # r#" { "jsonrpc": "2.0", - "method": "retrieve_txs", + "method": "query_txs", "params": { "token": "d202964900000000d302964900000000d402964900000000d502964900000000", "refresh_from_node": true, - "tx_id": null, - "tx_slate_id": null, - "tx_query_args": { + "query": { "min_id_inc": 0, "max_id_inc": 100, "min_amount_inc": "0", - "max_amount_inc": "6000000000", + "max_amount_inc": "60000000000", "sort_field": "Id", - "sort_order": "Desc" + "sort_order": "Asc" } }, "id": 1 @@ -385,13 +395,11 @@ pub trait OwnerRpc { */ - fn retrieve_txs( + fn query_txs( &self, token: Token, refresh_from_node: bool, - tx_id: Option, - tx_slate_id: Option, - tx_query_args: Option, + query: RetrieveTxQueryArgs, ) -> Result<(bool, Vec), Error>; /** @@ -1997,7 +2005,6 @@ where refresh_from_node: bool, tx_id: Option, tx_slate_id: Option, - query_args: Option, ) -> Result<(bool, Vec), Error> { Owner::retrieve_txs( self, @@ -2005,7 +2012,23 @@ where refresh_from_node, tx_id, tx_slate_id, - query_args, + None, + ) + } + + fn query_txs( + &self, + token: Token, + refresh_from_node: bool, + query: RetrieveTxQueryArgs, + ) -> Result<(bool, Vec), Error> { + Owner::retrieve_txs( + self, + (&token.keychain_mask).as_ref(), + refresh_from_node, + None, + None, + Some(query), ) } @@ -2565,7 +2588,7 @@ macro_rules! doctest_helper_json_rpc_owner_assert_response { // These cause LMDB to run out of disk space on CircleCI // disable for now on windows // TODO: Fix properly - #[cfg(not(target_os = "windows"))] + #[cfg(not(target_os = "linux"))] { use grin_wallet_api::run_doctest_owner; use serde_json; From ae7145029b5132ec3424bc2c5a2b176d101499a5 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 29 Nov 2022 11:28:49 +0000 Subject: [PATCH 08/11] revert test change --- api/src/owner_rpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index 343a5e32..e7e9c31c 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -2588,7 +2588,7 @@ macro_rules! doctest_helper_json_rpc_owner_assert_response { // These cause LMDB to run out of disk space on CircleCI // disable for now on windows // TODO: Fix properly - #[cfg(not(target_os = "linux"))] + #[cfg(not(target_os = "windows"))] { use grin_wallet_api::run_doctest_owner; use serde_json; From b1beca420662fb3d6ddb49bff5c4d13a6fde5f8c Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Thu, 1 Dec 2022 09:44:51 +0000 Subject: [PATCH 09/11] remove _inc from field names --- api/src/owner_rpc.rs | 8 ++++---- controller/tests/tx_list_filter.rs | 24 ++++++++++++------------ libwallet/src/api_impl/types.rs | 28 ++++++++++++++-------------- libwallet/src/internal/updater.rs | 14 +++++++------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index e7e9c31c..23fc6b63 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -325,10 +325,10 @@ pub trait OwnerRpc { "token": "d202964900000000d302964900000000d402964900000000d502964900000000", "refresh_from_node": true, "query": { - "min_id_inc": 0, - "max_id_inc": 100, - "min_amount_inc": "0", - "max_amount_inc": "60000000000", + "min_id": 0, + "max_id": 100, + "min_amount": "0", + "max_amount": "60000000000", "sort_field": "Id", "sort_order": "Asc" } diff --git a/controller/tests/tx_list_filter.rs b/controller/tests/tx_list_filter.rs index 4a8676f3..f105a281 100644 --- a/controller/tests/tx_list_filter.rs +++ b/controller/tests/tx_list_filter.rs @@ -56,7 +56,7 @@ fn test_wallet_tx_filtering( ) -> Result<(), libwallet::Error> { wallet::controller::owner_single_use(Some(wallet.clone()), mask, None, |api, _m| { let mut tx_query_args = RetrieveTxQueryArgs::default(); - tx_query_args.min_id_inc = Some(5); + tx_query_args.min_id = Some(5); // Min ID let tx_results = api @@ -67,8 +67,8 @@ fn test_wallet_tx_filtering( // Max ID let mut tx_query_args = RetrieveTxQueryArgs::default(); - tx_query_args.min_id_inc = Some(5); - tx_query_args.max_id_inc = Some(20); + tx_query_args.min_id = Some(5); + tx_query_args.max_id = Some(20); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; @@ -78,8 +78,8 @@ fn test_wallet_tx_filtering( // Exclude 1 cancelled let mut tx_query_args = RetrieveTxQueryArgs::default(); tx_query_args.exclude_cancelled = Some(true); - tx_query_args.min_id_inc = Some(5); - tx_query_args.max_id_inc = Some(50); + tx_query_args.min_id = Some(5); + tx_query_args.max_id = Some(50); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; @@ -89,8 +89,8 @@ fn test_wallet_tx_filtering( let mut tx_query_args = RetrieveTxQueryArgs::default(); tx_query_args.exclude_cancelled = Some(true); tx_query_args.include_confirmed_only = Some(true); - tx_query_args.min_id_inc = Some(5); - tx_query_args.max_id_inc = Some(50); + tx_query_args.min_id = Some(5); + tx_query_args.max_id = Some(50); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; @@ -100,8 +100,8 @@ fn test_wallet_tx_filtering( let mut tx_query_args = RetrieveTxQueryArgs::default(); tx_query_args.exclude_cancelled = Some(false); tx_query_args.include_outstanding_only = Some(true); - tx_query_args.min_id_inc = Some(5); - tx_query_args.max_id_inc = Some(50); + tx_query_args.min_id = Some(5); + tx_query_args.max_id = Some(50); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; @@ -112,8 +112,8 @@ fn test_wallet_tx_filtering( tx_query_args.exclude_cancelled = Some(false); tx_query_args.include_outstanding_only = Some(true); tx_query_args.include_confirmed_only = Some(true); - tx_query_args.min_id_inc = Some(5); - tx_query_args.max_id_inc = Some(50); + tx_query_args.min_id = Some(5); + tx_query_args.max_id = Some(50); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; @@ -162,7 +162,7 @@ fn test_wallet_tx_filtering( // amount, should see as above with coinbases excluded let mut tx_query_args = RetrieveTxQueryArgs::default(); tx_query_args.min_amount_inc = Some(60_000_000_000 - 59_963_300_000); - tx_query_args.max_amount_inc = Some(60_000_000_000 - 1); + tx_query_args.max_amount = Some(60_000_000_000 - 1); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index 2c35cc43..64e33ba1 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -180,10 +180,10 @@ pub enum RetrieveTxQuerySortField { pub struct RetrieveTxQueryArgs { /// Retrieve transactions with an id higher than or equal to the given /// If None, consider items from the first transaction and later - pub min_id_inc: Option, + pub min_id: Option, /// Retrieve tranactions with an id less than or equal to the given /// If None, consider items from the last transaction and earlier - pub max_id_inc: Option, + pub max_id: Option, /// The maximum number of transactions to return /// if both `before_id_inc` and `after_id_inc` are supplied, this will apply /// to the before and earlier set @@ -209,15 +209,15 @@ pub struct RetrieveTxQueryArgs { /// higher bound on the total amount (amount_credited - amount_debited), inclusive #[serde(with = "secp_ser::opt_string_or_u64")] #[serde(default)] - pub max_amount_inc: Option, + pub max_amount: Option, /// lower bound on the creation timestamp, inclusive - pub min_creation_timestamp_inc: Option>, + pub min_creation_timestamp: Option>, /// higher bound on on the creation timestamp, inclusive - pub max_creation_timestamp_inc: Option>, + pub max_creation_timestamp: Option>, /// lower bound on the confirmation timestamp, inclusive - pub min_confirmed_timestamp_inc: Option>, + pub min_confirmed_timestamp: Option>, /// higher bound on the confirmation timestamp, inclusive - pub max_confirmed_timestamp_inc: Option>, + pub max_confirmed_timestamp: Option>, /// Field within the tranasction list on which to sort /// defaults to ID if not present pub sort_field: Option, @@ -228,8 +228,8 @@ pub struct RetrieveTxQueryArgs { impl Default for RetrieveTxQueryArgs { fn default() -> Self { Self { - min_id_inc: None, - max_id_inc: None, + min_id: None, + max_id: None, limit: None, exclude_cancelled: Some(false), include_outstanding_only: Some(false), @@ -239,11 +239,11 @@ impl Default for RetrieveTxQueryArgs { include_coinbase_only: Some(false), include_reverted_only: Some(false), min_amount_inc: None, - max_amount_inc: None, - min_creation_timestamp_inc: None, - max_creation_timestamp_inc: None, - min_confirmed_timestamp_inc: None, - max_confirmed_timestamp_inc: None, + max_amount: None, + min_creation_timestamp: None, + max_creation_timestamp: None, + min_confirmed_timestamp: None, + max_confirmed_timestamp: None, sort_field: Some(RetrieveTxQuerySortField::Id), sort_order: Some(RetrieveTxQuerySortOrder::Asc), } diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index ad5ada1d..8462d37c 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -188,14 +188,14 @@ where } }) .filter(|tx_entry| { - if let Some(v) = query_args.min_id_inc { + if let Some(v) = query_args.min_id { tx_entry.id >= v } else { true } }) .filter(|tx_entry| { - if let Some(v) = query_args.max_id_inc { + if let Some(v) = query_args.max_id { tx_entry.id <= v } else { true @@ -219,7 +219,7 @@ where } }) .filter(|tx_entry| { - if let Some(v) = query_args.max_amount_inc { + if let Some(v) = query_args.max_amount { if tx_entry.tx_type == TxLogEntryType::TxSent || tx_entry.tx_type == TxLogEntryType::TxSentCancelled { @@ -236,21 +236,21 @@ where } }) .filter(|tx_entry| { - if let Some(v) = query_args.min_creation_timestamp_inc { + if let Some(v) = query_args.min_creation_timestamp { tx_entry.creation_ts >= v } else { true } }) .filter(|tx_entry| { - if let Some(v) = query_args.min_confirmed_timestamp_inc { + if let Some(v) = query_args.min_confirmed_timestamp { tx_entry.creation_ts <= v } else { true } }) .filter(|tx_entry| { - if let Some(v) = query_args.min_confirmed_timestamp_inc { + if let Some(v) = query_args.min_confirmed_timestamp { if let Some(t) = tx_entry.confirmation_ts { t >= v } else { @@ -261,7 +261,7 @@ where } }) .filter(|tx_entry| { - if let Some(v) = query_args.max_confirmed_timestamp_inc { + if let Some(v) = query_args.max_confirmed_timestamp { if let Some(t) = tx_entry.confirmation_ts { t <= v } else { From 2a599027f5a027a1c1862ea84e2e9ce8713bbb67 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Mon, 5 Dec 2022 11:14:46 +0000 Subject: [PATCH 10/11] more tweaks based on review --- controller/tests/tx_list_filter.rs | 6 +++--- libwallet/src/api_impl/types.rs | 4 ++-- libwallet/src/internal/updater.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/controller/tests/tx_list_filter.rs b/controller/tests/tx_list_filter.rs index f105a281..7a8dab03 100644 --- a/controller/tests/tx_list_filter.rs +++ b/controller/tests/tx_list_filter.rs @@ -153,7 +153,7 @@ fn test_wallet_tx_filtering( // Amounts let mut tx_query_args = RetrieveTxQueryArgs::default(); - tx_query_args.min_amount_inc = Some(60_000_000_000 - 59_963_300_000); + tx_query_args.min_amount = Some(60_000_000_000 - 59_963_300_000); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; @@ -161,7 +161,7 @@ fn test_wallet_tx_filtering( // amount, should see as above with coinbases excluded let mut tx_query_args = RetrieveTxQueryArgs::default(); - tx_query_args.min_amount_inc = Some(60_000_000_000 - 59_963_300_000); + tx_query_args.min_amount = Some(60_000_000_000 - 59_963_300_000); tx_query_args.max_amount = Some(60_000_000_000 - 1); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? @@ -170,7 +170,7 @@ fn test_wallet_tx_filtering( // Amount - should only see coinbase (incoming) let mut tx_query_args = RetrieveTxQueryArgs::default(); - tx_query_args.min_amount_inc = Some(60_000_000_000); + tx_query_args.min_amount = Some(60_000_000_000); let tx_results = api .retrieve_txs(mask, true, None, None, Some(tx_query_args))? .1; diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index 64e33ba1..f50bfc48 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -205,7 +205,7 @@ pub struct RetrieveTxQueryArgs { /// lower bound on the total amount (amount_credited - amount_debited), inclusive #[serde(with = "secp_ser::opt_string_or_u64")] #[serde(default)] - pub min_amount_inc: Option, + pub min_amount: Option, /// higher bound on the total amount (amount_credited - amount_debited), inclusive #[serde(with = "secp_ser::opt_string_or_u64")] #[serde(default)] @@ -238,7 +238,7 @@ impl Default for RetrieveTxQueryArgs { include_received_only: Some(false), include_coinbase_only: Some(false), include_reverted_only: Some(false), - min_amount_inc: None, + min_amount: None, max_amount: None, min_creation_timestamp: None, max_creation_timestamp: None, diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index 8462d37c..c750ac91 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -202,7 +202,7 @@ where } }) .filter(|tx_entry| { - if let Some(v) = query_args.min_amount_inc { + if let Some(v) = query_args.min_amount { if tx_entry.tx_type == TxLogEntryType::TxSent || tx_entry.tx_type == TxLogEntryType::TxSentCancelled { @@ -340,7 +340,7 @@ where K: Keychain + 'a, { let mut txs; - // Adding in new tranasction list query logic. If `tx_id` or `tx_slate_id` + // Adding in new transaction list query logic. If `tx_id` or `tx_slate_id` // is provided, then `query_args` is ignored and old logic is followed. if query_args.is_some() && tx_id.is_none() && tx_slate_id.is_none() { txs = apply_advanced_tx_list_filtering(wallet, &query_args.unwrap()) From 319b9ca7e0a6a3d886956ab8f851408786143364 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 6 Dec 2022 09:51:50 +0000 Subject: [PATCH 11/11] move result count limiting to after sorting --- libwallet/src/internal/updater.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index c750ac91..a9958dec 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -104,7 +104,7 @@ where K: Keychain + 'a, { // Apply simple bool, GTE or LTE fields - let mut txs_iter: Box> = Box::new( + let txs_iter: Box> = Box::new( wallet .tx_log_iter() .filter(|tx_entry| { @@ -273,11 +273,6 @@ where }), ); - // Apply limit if requested - if let Some(l) = query_args.limit { - txs_iter = Box::new(txs_iter.take(l as usize)); - } - let mut return_txs: Vec = txs_iter.collect(); // Now apply requested sorting @@ -321,6 +316,11 @@ where } } + // Apply limit if requested + if let Some(l) = query_args.limit { + return_txs = return_txs.into_iter().take(l as usize).collect() + } + return_txs }