diff --git a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs index d4738507161..8df5ee26bb3 100644 --- a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs +++ b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs @@ -599,12 +599,6 @@ impl wallet_server::Wallet for WalletGrpcServer { async fn coin_split(&self, request: Request) -> Result, Status> { let message = request.into_inner(); - // let lock_height = if message.lock_height == 0 { - // None - // } else { - // Some(message.lock_height) - // }; - let mut wallet = self.wallet.clone(); let tx_id = wallet diff --git a/base_layer/wallet/src/output_manager_service/input_selection.rs b/base_layer/wallet/src/output_manager_service/input_selection.rs index f1f25a1d7ae..2264e38d8d8 100644 --- a/base_layer/wallet/src/output_manager_service/input_selection.rs +++ b/base_layer/wallet/src/output_manager_service/input_selection.rs @@ -41,6 +41,13 @@ impl UtxoSelectionCriteria { } } + pub fn smallest_first() -> Self { + Self { + filter: UtxoSelectionFilter::Standard, + ordering: UtxoSelectionOrdering::SmallestFirst, + } + } + pub fn for_token(unique_id: Vec, parent_public_key: Option) -> Self { Self { filter: UtxoSelectionFilter::TokenOutput { diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 61137d58387..a146a5b95dc 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -1443,7 +1443,7 @@ where /// selection strategy to choose the outputs. It also determines if a change output is required. async fn select_utxos( &mut self, - amount: MicroTari, + target_amount: MicroTari, fee_per_gram: MicroTari, num_outputs: usize, output_metadata_byte_size: usize, @@ -1451,81 +1451,89 @@ where ) -> Result { debug!( target: LOG_TARGET, - "select_utxos amount: {}, fee_per_gram: {}, num_outputs: {}, output_metadata_byte_size: {}, \ + "select_utxos target_amount: {}, fee_per_gram: {}, num_outputs: {}, output_metadata_byte_size: {}, \ selection_criteria: {:?}", - amount, + target_amount, fee_per_gram, num_outputs, output_metadata_byte_size, selection_criteria ); - let mut utxos = Vec::new(); - let mut utxos_total_value = MicroTari::from(0); - let mut fee_without_change = MicroTari::from(0); - let mut fee_with_change = MicroTari::from(0); - let fee_calc = self.get_fee_calc(); + let tip_height = self + .base_node_service + .get_chain_metadata() + .await? + .as_ref() + .map(|m| m.height_of_longest_chain()); + let balance = self.get_balance(tip_height)?; + + // collecting UTXOs sufficient to cover the target amount + let utxos = self.resources.db.fetch_unspent_outputs_for_spending( + selection_criteria.clone(), + target_amount, + tip_height, + )?; - // Attempt to get the chain tip height - let chain_metadata = self.base_node_service.get_chain_metadata().await?; + let accumulated_amount = utxos + .iter() + .fold(MicroTari::zero(), |acc, x| acc + x.unblinded_output.value); - warn!( - target: LOG_TARGET, - "select_utxos selection criteria: {}", selection_criteria - ); - let tip_height = chain_metadata.as_ref().map(|m| m.height_of_longest_chain()); - let uo = self - .resources - .db - .fetch_unspent_outputs_for_spending(selection_criteria, amount, tip_height)?; - trace!(target: LOG_TARGET, "We found {} UTXOs to select from", uo.len()); + if accumulated_amount <= target_amount { + return Err(OutputManagerError::NotEnoughFunds); + } - // Assumes that default Outputfeatures are used for change utxo - let output_features_estimate = OutputFeatures::default(); - let default_metadata_size = fee_calc.weighting().round_up_metadata_size( - output_features_estimate.consensus_encode_exact_size() + script![Nop].consensus_encode_exact_size(), - ); - let mut requires_change_output = false; - for o in uo { - utxos_total_value += o.unblinded_output.value; - utxos.push(o); - // The assumption here is that the only output will be the payment output and change if required - fee_without_change = - fee_calc.calculate(fee_per_gram, 1, utxos.len(), num_outputs, output_metadata_byte_size); - if utxos_total_value == amount + fee_without_change { - break; - } - fee_with_change = fee_calc.calculate( + let fee_without_change = + self.get_fee_calc() + .calculate(fee_per_gram, 1, utxos.len(), num_outputs, output_metadata_byte_size); + + // checking whether the total output value is enough + if accumulated_amount < (target_amount + fee_without_change) { + return Err(OutputManagerError::NotEnoughFunds); + } + + let fee_with_change = match accumulated_amount + .saturating_sub(target_amount + fee_without_change) + .as_u64() + { + 0 => fee_without_change, + _ => self.get_fee_calc().calculate( fee_per_gram, 1, utxos.len(), num_outputs + 1, - output_metadata_byte_size + default_metadata_size, - ); - if utxos_total_value > amount + fee_with_change { - requires_change_output = true; - break; - } - } + output_metadata_byte_size + self.default_metadata_size(), + ), + }; - let perfect_utxo_selection = utxos_total_value == amount + fee_without_change; - let enough_spendable = utxos_total_value > amount + fee_with_change; + // this is how much it would require in the end + let target_amount_with_fee = target_amount + fee_with_change; - if !perfect_utxo_selection && !enough_spendable { - let current_tip_for_time_lock_calculation = chain_metadata.map(|cm| cm.height_of_longest_chain()); - let balance = self.get_balance(current_tip_for_time_lock_calculation)?; - let pending_incoming = balance.pending_incoming_balance; - if utxos_total_value + pending_incoming >= amount + fee_with_change { - return Err(OutputManagerError::FundsPending); + // checking, again, whether a total output value is enough + if accumulated_amount < target_amount_with_fee { + return Err(OutputManagerError::NotEnoughFunds); + } + + // balance check + if balance.available_balance < target_amount_with_fee { + return if accumulated_amount + balance.pending_incoming_balance >= target_amount_with_fee { + Err(OutputManagerError::FundsPending) } else { - return Err(OutputManagerError::NotEnoughFunds); - } + Err(OutputManagerError::NotEnoughFunds) + }; } + trace!( + target: LOG_TARGET, + "select_utxos selection criteria: {}\noutputs found: {}", + selection_criteria, + utxos.len() + ); + Ok(UtxoSelection { utxos, - requires_change_output, - total_value: utxos_total_value, + requires_change_output: accumulated_amount.saturating_sub(target_amount_with_fee) > MicroTari::zero(), + total_value: accumulated_amount, fee_without_change, fee_with_change, }) @@ -1591,13 +1599,18 @@ where fee_per_gram: MicroTari, ) -> Result<(TxId, Transaction, MicroTari), OutputManagerError> { let total_split_amount = MicroTari::from(amount_per_split.as_u64() * number_of_splits as u64); + + eprintln!("* total_split_amount = {:#?}", total_split_amount); + eprintln!("* amount_per_split = {:#?}", amount_per_split); + eprintln!("* fee_per_gram = {:#?}", fee_per_gram); + let src_outputs = self .select_utxos( total_split_amount, fee_per_gram, number_of_splits, - self.default_metadata_size(), - UtxoSelectionCriteria::default(), + self.default_metadata_size() * number_of_splits, + UtxoSelectionCriteria::largest_first(), ) .await?; @@ -1638,7 +1651,7 @@ where 1, src_outputs.len(), number_of_splits, - default_metadata_size, + default_metadata_size * number_of_splits, ); // checking whether a total output value is enough @@ -1765,8 +1778,10 @@ where dest_outputs.push(output); } + let has_leftover_change = leftover_change > MicroTari::zero(); + // extending transaction if there is some `change` left over - if leftover_change > MicroTari::zero() { + if has_leftover_change { let (spending_key, script_private_key) = self.get_spend_and_script_keys().await?; tx_builder.with_change_secret(spending_key); tx_builder.with_rewindable_outputs(self.resources.rewind_data.clone()); @@ -1796,7 +1811,7 @@ where ); // again, to obtain output for leftover change - if leftover_change > MicroTari::zero() { + if has_leftover_change { // obtaining output for the `change` let unblinded_output_for_change = stp.get_change_unblinded_output()?.ok_or_else(|| { OutputManagerError::BuildError( @@ -1834,7 +1849,13 @@ where self.last_seen_tip_height.unwrap_or(u64::MAX), )?; - Ok((tx_id, stp.take_transaction()?, total_split_amount)) + let value = if has_leftover_change { + total_split_amount + } else { + total_split_amount + final_fee + }; + + Ok((tx_id, stp.take_transaction()?, value)) } #[allow(clippy::too_many_lines)] @@ -1989,7 +2010,7 @@ where self.last_seen_tip_height.unwrap_or(u64::MAX), )?; - Ok((tx_id, stp.take_transaction()?, accumulated_amount)) + Ok((tx_id, stp.take_transaction()?, aftertax_amount + fee)) } async fn fetch_outputs_from_node( @@ -2368,6 +2389,7 @@ struct UtxoSelection { fee_with_change: MicroTari, } +#[allow(dead_code)] impl UtxoSelection { pub fn as_final_fee(&self) -> MicroTari { if self.requires_change_output { diff --git a/base_layer/wallet/src/wallet.rs b/base_layer/wallet/src/wallet.rs index e19cd909c83..59e2e3234ed 100644 --- a/base_layer/wallet/src/wallet.rs +++ b/base_layer/wallet/src/wallet.rs @@ -580,7 +580,7 @@ where Ok((tx_id, tx, output_value)) => { let coin_tx = self .transaction_service - .submit_transaction(tx_id, tx, output_value, msg.unwrap_or(String::new())) + .submit_transaction(tx_id, tx, output_value, msg.unwrap_or_default()) .await; match coin_tx { diff --git a/base_layer/wallet/tests/output_manager_service_tests/service.rs b/base_layer/wallet/tests/output_manager_service_tests/service.rs index fe4e8d085dc..d80e56c555a 100644 --- a/base_layer/wallet/tests/output_manager_service_tests/service.rs +++ b/base_layer/wallet/tests/output_manager_service_tests/service.rs @@ -105,9 +105,10 @@ use crate::support::{ }; fn default_metadata_byte_size() -> usize { - let output_features = OutputFeatures { ..Default::default() }; TransactionWeight::latest().round_up_metadata_size( - output_features.consensus_encode_exact_size() + script![Nop].consensus_encode_exact_size(), + Covenant::default().consensus_encode_exact_size() + + OutputFeatures::default().consensus_encode_exact_size() + + script![Nop].consensus_encode_exact_size(), ) } @@ -519,7 +520,7 @@ async fn test_utxo_selection_no_chain_metadata() { assert!(matches!(err, OutputManagerError::NotEnoughFunds)); // coin split uses the "Largest" selection strategy - let (_, tx, utxos_total_value) = oms.create_coin_split(amount, 5, fee_per_gram, None).await.unwrap(); + let (_, tx, utxos_total_value) = oms.create_coin_split(vec![], amount, 5, fee_per_gram).await.unwrap(); let expected_fee = fee_calc.calculate(fee_per_gram, 1, 1, 6, default_metadata_byte_size() * 6); assert_eq!(tx.body.get_total_fee(), expected_fee); assert_eq!(utxos_total_value, MicroTari::from(10_000)); @@ -604,7 +605,7 @@ async fn test_utxo_selection_with_chain_metadata() { assert!(matches!(err, OutputManagerError::NotEnoughFunds)); // test coin split is maturity aware - let (_, tx, utxos_total_value) = oms.create_coin_split(amount, 5, fee_per_gram, None).await.unwrap(); + let (_, tx, utxos_total_value) = oms.create_coin_split(vec![], amount, 5, fee_per_gram).await.unwrap(); assert_eq!(utxos_total_value, MicroTari::from(6_000)); let expected_fee = fee_calc.calculate(fee_per_gram, 1, 1, 6, default_metadata_byte_size() * 6); assert_eq!(tx.body.get_total_fee(), expected_fee); @@ -1154,23 +1155,25 @@ async fn coin_split_with_change() { let fee_per_gram = MicroTari::from(5); let split_count = 8; - let (_tx_id, coin_split_tx, amount) = oms - .output_manager_handle - .create_coin_split(1000.into(), split_count, fee_per_gram, None) - .await - .unwrap(); - assert_eq!(coin_split_tx.body.inputs().len(), 2); - assert_eq!(coin_split_tx.body.outputs().len(), split_count + 1); let fee_calc = Fee::new(*create_consensus_constants(0).transaction_weight()); let expected_fee = fee_calc.calculate( fee_per_gram, 1, - 2, + 3, split_count + 1, (split_count + 1) * default_metadata_byte_size(), ); + + let (_tx_id, coin_split_tx, amount) = oms + .output_manager_handle + .create_coin_split(vec![], 1000.into(), split_count, fee_per_gram) + .await + .unwrap(); + + assert_eq!(coin_split_tx.body.inputs().len(), 3); + assert_eq!(coin_split_tx.body.outputs().len(), split_count + 1); assert_eq!(coin_split_tx.body.get_total_fee(), expected_fee); - assert_eq!(amount, val2 + val3); + assert_eq!(amount, val3); } #[tokio::test] @@ -1192,6 +1195,7 @@ async fn coin_split_no_change() { split_count, split_count * default_metadata_byte_size(), ); + let val1 = 4_000 * uT; let val2 = 5_000 * uT; let val3 = 6_000 * uT + expected_fee; @@ -1204,7 +1208,7 @@ async fn coin_split_no_change() { let (_tx_id, coin_split_tx, amount) = oms .output_manager_handle - .create_coin_split(1000.into(), split_count, fee_per_gram, None) + .create_coin_split(vec![], 1000.into(), split_count, fee_per_gram) .await .unwrap(); assert_eq!(coin_split_tx.body.inputs().len(), 3); diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index f9b10f8ffe7..9759cc48551 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -290,6 +290,7 @@ pub struct TariVector { pub ptr: *mut c_void, } +#[allow(dead_code)] impl TariVector { fn from_string_vec(v: Vec) -> Result { let mut strings = ManuallyDrop::new( @@ -306,7 +307,6 @@ impl TariVector { }) } - #[allow(dead_code)] fn from_commitment_vec(v: &mut Vec) -> Result { Ok(Self { tag: TariTypeTag::Commitment, @@ -333,20 +333,19 @@ impl TariVector { Ok(unsafe { Vec::from_raw_parts(self.ptr as *mut *mut c_char, self.len, self.cap) .into_iter() - .map(|x| String::from(CString::from_raw(x).into_string().unwrap())) + .map(|x| CString::from_raw(x).into_string().unwrap()) .collect() }) } fn to_commitment_vec(&self) -> Result, InterfaceError> { - Ok(self - .to_string_vec()? + self.to_string_vec()? .into_iter() .map(|x| { Commitment::from_hex(x.as_str()) .map_err(|e| InterfaceError::PointerError(format!("failed to convert hex to commitment: {:?}", e))) }) - .try_collect::, InterfaceError>()?) + .try_collect::, InterfaceError>() } #[allow(dead_code)] @@ -4400,7 +4399,7 @@ pub unsafe extern "C" fn destroy_tari_vector(x: *mut TariVector) { /// `c_ulonglong` - Returns the transaction id. /// /// # Safety -/// None +/// `TariVector` must be freed after use with `destroy_tari_vector()` #[no_mangle] pub unsafe extern "C" fn wallet_coin_split( wallet: *mut TariWallet, @@ -4457,6 +4456,23 @@ pub unsafe extern "C" fn wallet_coin_split( } } +/// This function will tell the wallet to do a coin join, resulting in a new coin worth a sum of the joined coins minus +/// the fee. +/// +/// ## Arguments +/// * `wallet` - The TariWallet pointer +/// * `commitments` - A `TariVector` of "strings", tagged as `TariTypeTag::String`, containing commitment's hex values +/// (see `Commitment::to_hex()`) +/// * `fee_per_gram` - The transaction fee +/// * `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` - Returns the transaction id. +/// +/// # Safety +/// `TariVector` must be freed after use with `destroy_tari_vector()` #[no_mangle] pub unsafe extern "C" fn wallet_coin_join( wallet: *mut TariWallet, @@ -9156,7 +9172,7 @@ mod test { let payload = utxos[0..3] .iter() - .map(|x| String::from(CString::from_raw(x.commitment).into_string().unwrap().clone())) + .map(|x| CString::from_raw(x.commitment).into_string().unwrap()) .collect::>(); let commitments = Box::into_raw(Box::new(TariVector::from_string_vec(payload).unwrap())) as *mut TariVector; @@ -9175,7 +9191,7 @@ mod test { .unwrap() .into_iter() .map(|x| x.unblinded_output.value) - .collect::>(); + .collect::>(); let new_pending_outputs = (*alice_wallet) .wallet @@ -9187,7 +9203,7 @@ mod test { .unwrap() .into_iter() .map(|x| x.unblinded_output.value) - .collect::>(); + .collect::>(); let outputs = wallet_get_utxos(alice_wallet, 0, 20, TariUtxoSort::ValueAsc, 0, error_ptr); let utxos: &[TariUtxo] = slice::from_raw_parts_mut((*outputs).ptr, (*outputs).len); @@ -9286,7 +9302,7 @@ mod test { let payload = utxos[0..3] .iter() - .map(|x| String::from(CString::from_raw(x.commitment).into_string().unwrap().clone())) + .map(|x| CString::from_raw(x.commitment).into_string().unwrap()) .collect::>(); let commitments = Box::into_raw(Box::new(TariVector::from_string_vec(payload).unwrap())) as *mut TariVector; diff --git a/base_layer/wallet_ffi/tari_wallet_ffi.h b/base_layer/wallet_ffi/tari_wallet_ffi.h index c76c79f0305..09f19acf2fc 100644 --- a/base_layer/wallet_ffi/tari_wallet_ffi.h +++ b/base_layer/wallet_ffi/tari_wallet_ffi.h @@ -2252,7 +2252,7 @@ void destroy_tari_vector(struct TariVector *x); * `c_ulonglong` - Returns the transaction id. * * # Safety - * None + * `TariVector` must be freed after use with `destroy_tari_vector()` */ uint64_t wallet_coin_split(struct TariWallet *wallet, struct TariVector *commitments, @@ -2261,6 +2261,25 @@ uint64_t wallet_coin_split(struct TariWallet *wallet, uint64_t fee_per_gram, int32_t *error_ptr); +/** + * This function will tell the wallet to do a coin join, resulting in a new coin worth a sum of the joined coins minus + * the fee. + * + * ## Arguments + * * `wallet` - The TariWallet pointer + * * `commitments` - A `TariVector` of "strings", tagged as `TariTypeTag::String`, containing commitment's hex values + * (see `Commitment::to_hex()`) + * * `fee_per_gram` - The transaction fee + * * `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` - Returns the transaction id. + * + * # Safety + * `TariVector` must be freed after use with `destroy_tari_vector()` + */ uint64_t wallet_coin_join(struct TariWallet *wallet, struct TariVector *commitments, uint64_t fee_per_gram,