From 413757bcea5474524b18a860a95df255cbe95d33 Mon Sep 17 00:00:00 2001 From: David Main <51991544+StriderDM@users.noreply.github.com> Date: Tue, 9 Nov 2021 12:30:00 +0200 Subject: [PATCH] refactor!: remove outdated wallet_ffi balance methods (#3528) Description Removed outdated balance methods in wallet_ffi Updated wallet.h Updated cucumber tests Motivation and Context Removal of outdated methods Merge https://github.com/tari-project/tari/pull/3547 first. How Has This Been Tested? nvm use 12.22.6 && node_modules/.bin/cucumber-js --profile "ci" --tags "not @long-running and not @broken and @wallet-ffi" --- base_layer/wallet/src/wallet.rs | 2 +- base_layer/wallet_ffi/src/error.rs | 6 + base_layer/wallet_ffi/src/lib.rs | 146 ++++-------------- base_layer/wallet_ffi/wallet.h | 12 +- integration_tests/features/WalletFFI.feature | 2 +- .../features/support/ffi_steps.js | 2 +- integration_tests/helpers/ffi/ffiInterface.js | 38 +---- integration_tests/helpers/ffi/wallet.js | 50 ++++-- integration_tests/helpers/walletFFIClient.js | 4 + 9 files changed, 91 insertions(+), 171 deletions(-) diff --git a/base_layer/wallet/src/wallet.rs b/base_layer/wallet/src/wallet.rs index 0f3ee66fda..f184cadc42 100644 --- a/base_layer/wallet/src/wallet.rs +++ b/base_layer/wallet/src/wallet.rs @@ -266,7 +266,7 @@ where /// This method consumes the wallet so that the handles are dropped which will result in the services async loops /// exiting. pub async fn wait_until_shutdown(self) { - self.comms.clone().wait_until_shutdown().await; + self.comms.to_owned().wait_until_shutdown().await; } /// This function will set the base node that the wallet uses to broadcast transactions, monitor outputs, and diff --git a/base_layer/wallet_ffi/src/error.rs b/base_layer/wallet_ffi/src/error.rs index fb9a8b09da..37349dd778 100644 --- a/base_layer/wallet_ffi/src/error.rs +++ b/base_layer/wallet_ffi/src/error.rs @@ -54,6 +54,8 @@ pub enum InterfaceError { InvalidEmojiId, #[error("An error has occurred due to an invalid argument: `{0}`")] InvalidArgument(String), + #[error("Balance Unavailable")] + BalanceError, } /// This struct is meant to hold an error for use by FFI client applications. The error has an integer code and string @@ -96,6 +98,10 @@ impl From for LibWalletError { code: 7, message: format!("{:?}", v), }, + InterfaceError::BalanceError => Self { + code: 8, + message: "Balance Unavailable".to_string(), + }, } } } diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index ea9753b0c9..6fb0da6528 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -3050,6 +3050,39 @@ pub unsafe extern "C" fn wallet_create( } } +/// Retrieves the balance from a wallet +/// +/// ## Arguments +/// `wallet` - The TariWallet pointer. +/// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions +/// as an out parameter. +/// ## Returns +/// `*mut Balance` - Returns the pointer to the TariBalance or null if error occurs +/// +/// # Safety +/// The ```balance_destroy``` method must be called when finished with a TariBalance to prevent a memory leak +#[no_mangle] +pub unsafe extern "C" fn wallet_get_balance(wallet: *mut TariWallet, error_out: *mut c_int) -> *mut TariBalance { + let mut error = 0; + ptr::swap(error_out, &mut error as *mut c_int); + if wallet.is_null() { + error = LibWalletError::from(InterfaceError::NullError("wallet".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + } + let balance = (*wallet) + .runtime + .block_on((*wallet).wallet.output_manager_service.get_balance()); + match balance { + Ok(balance) => Box::into_raw(Box::new(balance)), + Err(_) => { + error = LibWalletError::from(InterfaceError::BalanceError).code; + ptr::swap(error_out, &mut error as *mut c_int); + ptr::null_mut() + }, + } +} + /// Signs a message using the public key of the TariWallet /// /// ## Arguments @@ -3457,119 +3490,6 @@ pub unsafe extern "C" fn balance_destroy(balance: *mut TariBalance) { } } -/// Gets the available balance from a TariWallet. This is the balance the user can spend. -/// -/// ## Arguments -/// `wallet` - The TariWallet pointer -/// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions -/// as an out parameter. -/// -/// ## Returns -/// `c_ulonglong` - The available balance, 0 if wallet is null -/// -/// # Safety -/// None -#[no_mangle] -pub unsafe extern "C" fn wallet_get_available_balance(wallet: *mut TariWallet, error_out: *mut c_int) -> c_ulonglong { - let mut error = 0; - ptr::swap(error_out, &mut error as *mut c_int); - if wallet.is_null() { - error = LibWalletError::from(InterfaceError::NullError("wallet".to_string())).code; - ptr::swap(error_out, &mut error as *mut c_int); - return 0; - } - - match (*wallet) - .runtime - .block_on((*wallet).wallet.output_manager_service.get_balance()) - { - Ok(b) => c_ulonglong::from(b.available_balance), - Err(e) => { - error = LibWalletError::from(WalletError::OutputManagerError(e)).code; - ptr::swap(error_out, &mut error as *mut c_int); - 0 - }, - } -} - -/// Gets the incoming balance from a `TariWallet`. This is the uncleared balance of Tari that is -/// expected to come into the `TariWallet` but is not yet spendable. -/// -/// ## Arguments -/// `wallet` - The TariWallet pointer -/// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions -/// as an out parameter. -/// -/// ## Returns -/// `c_ulonglong` - The incoming balance, 0 if wallet is null -/// -/// # Safety -/// None -#[no_mangle] -pub unsafe extern "C" fn wallet_get_pending_incoming_balance( - wallet: *mut TariWallet, - error_out: *mut c_int, -) -> c_ulonglong { - let mut error = 0; - ptr::swap(error_out, &mut error as *mut c_int); - if wallet.is_null() { - error = LibWalletError::from(InterfaceError::NullError("wallet".to_string())).code; - ptr::swap(error_out, &mut error as *mut c_int); - return 0; - } - - match (*wallet) - .runtime - .block_on((*wallet).wallet.output_manager_service.get_balance()) - { - Ok(b) => c_ulonglong::from(b.pending_incoming_balance), - Err(e) => { - error = LibWalletError::from(WalletError::OutputManagerError(e)).code; - ptr::swap(error_out, &mut error as *mut c_int); - 0 - }, - } -} - -/// Gets the outgoing balance from a `TariWallet`. This is the uncleared balance of Tari that has -/// been spent -/// -/// ## Arguments -/// `wallet` - The TariWallet pointer -/// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions -/// as an out parameter. -/// -/// ## Returns -/// `c_ulonglong` - The outgoing balance, 0 if wallet is null -/// -/// # Safety -/// None -#[no_mangle] -pub unsafe extern "C" fn wallet_get_pending_outgoing_balance( - wallet: *mut TariWallet, - error_out: *mut c_int, -) -> c_ulonglong { - let mut error = 0; - ptr::swap(error_out, &mut error as *mut c_int); - if wallet.is_null() { - error = LibWalletError::from(InterfaceError::NullError("wallet".to_string())).code; - ptr::swap(error_out, &mut error as *mut c_int); - return 0; - } - - match (*wallet) - .runtime - .block_on((*wallet).wallet.output_manager_service.get_balance()) - { - Ok(b) => c_ulonglong::from(b.pending_outgoing_balance), - Err(e) => { - error = LibWalletError::from(WalletError::OutputManagerError(e)).code; - ptr::swap(error_out, &mut error as *mut c_int); - 0 - }, - } -} - /// Sends a TariPendingOutboundTransaction /// /// ## Arguments diff --git a/base_layer/wallet_ffi/wallet.h b/base_layer/wallet_ffi/wallet.h index 376462987f..7b0ee7ca16 100644 --- a/base_layer/wallet_ffi/wallet.h +++ b/base_layer/wallet_ffi/wallet.h @@ -477,6 +477,9 @@ struct TariWallet *wallet_create(struct TariCommsConfig *config, bool *recovery_in_progress, int *error_out); +// Gets the balance +struct TariBalance *wallet_get_balance(struct TariWallet *wallet, int *error_out); + // Signs a message char *wallet_sign_message(struct TariWallet *wallet, const char *msg, int *error_out); @@ -504,15 +507,6 @@ unsigned long long balance_get_pending_incoming(struct TariBalance *balance, int // Gets the available balance from a TariBalance unsigned long long balance_get_pending_outgoing(struct TariBalance *balance, int *error_out); -// Gets the available balance from a TariWallet -unsigned long long wallet_get_available_balance(struct TariWallet *wallet, int *error_out); - -// Gets the incoming balance from a TariWallet -unsigned long long wallet_get_pending_incoming_balance(struct TariWallet *wallet, int *error_out); - -// Gets the outgoing balance from a TariWallet -unsigned long long wallet_get_pending_outgoing_balance(struct TariWallet *wallet, int *error_out); - // Get a fee estimate from a TariWallet for a given amount unsigned long long wallet_get_fee_estimate(struct TariWallet *wallet, unsigned long long amount, unsigned long long fee_per_gram, unsigned long long num_kernels, unsigned long long num_outputs, int *error_out); diff --git a/integration_tests/features/WalletFFI.feature b/integration_tests/features/WalletFFI.feature index 79f66144ab..79b23f241d 100644 --- a/integration_tests/features/WalletFFI.feature +++ b/integration_tests/features/WalletFFI.feature @@ -156,7 +156,7 @@ Feature: Wallet FFI # Scenario: As a client I want feedback about my connection status to the specifed Base Node # Scenario: As a client I want feedback about the wallet restoration process - # As a client I want to be able to restore my wallet from seed words + # It's a subtest of "As a client I want to be able to restore my wallet from seed words" # Scenario: As a client I want feedback about TXO and TX validation processes # It's a subtest of "As a client I want to retrieve a list of transactions I have made and received" diff --git a/integration_tests/features/support/ffi_steps.js b/integration_tests/features/support/ffi_steps.js index c0f73e82cb..6c083b01d1 100644 --- a/integration_tests/features/support/ffi_steps.js +++ b/integration_tests/features/support/ffi_steps.js @@ -444,7 +444,7 @@ Then( 120 ); - let balance = wallet.getBalance().available; + let balance = wallet.pollBalance().available; if (!(balance >= amount)) { console.log("Balance not adequate!"); diff --git a/integration_tests/helpers/ffi/ffiInterface.js b/integration_tests/helpers/ffi/ffiInterface.js index 1d47cb7c83..78430ac90e 100644 --- a/integration_tests/helpers/ffi/ffiInterface.js +++ b/integration_tests/helpers/ffi/ffiInterface.js @@ -294,6 +294,7 @@ class InterfaceFFI { this.intPtr, ], ], + wallet_get_balance: [this.ptr, [this.ptr, this.intPtr]], wallet_sign_message: [ this.stringPtr, [this.ptr, this.string, this.intPtr], @@ -312,15 +313,6 @@ class InterfaceFFI { balance_get_time_locked: [this.ulonglong, [this.ptr, this.intPtr]], balance_get_pending_incoming: [this.ulonglong, [this.ptr, this.intPtr]], balance_get_pending_outgoing: [this.ulonglong, [this.ptr, this.intPtr]], - wallet_get_available_balance: [this.ulonglong, [this.ptr, this.intPtr]], - wallet_get_pending_incoming_balance: [ - this.ulonglong, - [this.ptr, this.intPtr], - ], - wallet_get_pending_outgoing_balance: [ - this.ulonglong, - [this.ptr, this.intPtr], - ], wallet_get_fee_estimate: [ this.ulonglong, [ @@ -1211,6 +1203,13 @@ class InterfaceFFI { return result; } + static walletGetBalance(ptr) { + let error = this.initError(); + let result = this.fn.wallet_get_balance(ptr, error); + this.checkErrorResult(error, `walletGetBalance`); + return result; + } + static walletGetPublicKey(ptr) { let error = this.initError(); let result = this.fn.wallet_get_public_key(ptr, error); @@ -1292,27 +1291,6 @@ class InterfaceFFI { return result; } - static walletGetAvailableBalance(ptr) { - let error = this.initError(); - let result = this.fn.wallet_get_available_balance(ptr, error); - this.checkErrorResult(error, `walletGetAvailableBalance`); - return result; - } - - static walletGetPendingIncomingBalance(ptr) { - let error = this.initError(); - let result = this.fn.wallet_get_pending_incoming_balance(ptr, error); - this.checkErrorResult(error, `walletGetPendingIncomingBalance`); - return result; - } - - static walletGetPendingOutgoingBalance(ptr) { - let error = this.initError(); - let result = this.fn.wallet_get_pending_outgoing_balance(ptr, error); - this.checkErrorResult(error, `walletGetPendingOutgoingBalance`); - return result; - } - static walletGetFeeEstimate( ptr, amount, diff --git a/integration_tests/helpers/ffi/wallet.js b/integration_tests/helpers/ffi/wallet.js index 2ce779e0b0..0797ad81d5 100644 --- a/integration_tests/helpers/ffi/wallet.js +++ b/integration_tests/helpers/ffi/wallet.js @@ -10,8 +10,17 @@ const Contacts = require("./contacts"); const Balance = require("./balance"); const utf8 = require("utf8"); + +class WalletBalance { + available = 0; + timeLocked = 0; + pendingIn = 0; + pendingOut = 0; +} + class Wallet { ptr; + balance = new WalletBalance(); log_path = ""; receivedTransaction = 0; receivedTransactionReply = 0; @@ -269,8 +278,16 @@ class Wallet { onBalanceUpdated = (ptr) => { let b = new Balance(); b.pointerAssign(ptr); + this.balance.available = b.getAvailable(); + this.balance.timeLocked = b.getTimeLocked(); + this.balance.pendingIn = b.getPendingIncoming(); + this.balance.pendingOut = b.getPendingOutgoing(); console.log( - `${new Date().toISOString()} callbackBalanceUpdated: available = ${b.getAvailable()}, time locked = ${b.getTimeLocked()} pending incoming = ${b.getPendingIncoming()} pending outgoing = ${b.getPendingOutgoing()}` + `${new Date().toISOString()} callbackBalanceUpdated: available = ${ + this.balance.available + }, time locked = ${this.balance.timeLocked} pending incoming = ${ + this.balance.pendingIn + } pending outgoing = ${this.balance.pendingOut}` ); b.destroy(); }; @@ -320,6 +337,22 @@ class Wallet { return result; } + getBalance() { + return this.balance; + } + + pollBalance() { + let b = new Balance(); + let ptr = InterfaceFFI.walletGetBalance(this.ptr); + b.pointerAssign(ptr); + this.balance.available = b.getAvailable(); + this.balance.timeLocked = b.getTimeLocked(); + this.balance.pendingIn = b.getPendingIncoming(); + this.balance.pendingOut = b.getPendingOutgoing(); + b.destroy(); + return this.balance; + } + getEmojiId() { let ptr = InterfaceFFI.walletGetPublicKey(this.ptr); let pk = new PublicKey(); @@ -329,21 +362,6 @@ class Wallet { return result; } - getBalance() { - let available = InterfaceFFI.walletGetAvailableBalance(this.ptr); - let pendingIncoming = InterfaceFFI.walletGetPendingIncomingBalance( - this.ptr - ); - let pendingOutgoing = InterfaceFFI.walletGetPendingOutgoingBalance( - this.ptr - ); - return { - pendingIn: pendingIncoming, - pendingOut: pendingOutgoing, - available: available, - }; - } - addBaseNodePeer(public_key_hex, address) { let public_key = PublicKey.fromHexString(utf8.encode(public_key_hex)); let result = InterfaceFFI.walletAddBaseNodePeer( diff --git a/integration_tests/helpers/walletFFIClient.js b/integration_tests/helpers/walletFFIClient.js index 0929f96aaf..592ddaff6c 100644 --- a/integration_tests/helpers/walletFFIClient.js +++ b/integration_tests/helpers/walletFFIClient.js @@ -74,6 +74,10 @@ class WalletFFIClient { return this.wallet.getBalance(); } + pollBalance() { + return this.wallet.pollBalance(); + } + addBaseNodePeer(public_key_hex, address) { return this.wallet.addBaseNodePeer(public_key_hex, address); }