Skip to content

Commit

Permalink
bugfixes, testfixes, cleanup, refactored select_utxos()
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrei Gubarev <[email protected]>
  • Loading branch information
@agubarev
agubarev committed Jun 28, 2022
1 parent 1bec403 commit 2b7b5c0
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 94 deletions.
6 changes: 0 additions & 6 deletions applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,12 +599,6 @@ impl wallet_server::Wallet for WalletGrpcServer {
async fn coin_split(&self, request: Request<CoinSplitRequest>) -> Result<Response<CoinSplitResponse>, 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
Expand Down
7 changes: 7 additions & 0 deletions base_layer/wallet/src/output_manager_service/input_selection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8>, parent_public_key: Option<PublicKey>) -> Self {
Self {
filter: UtxoSelectionFilter::TokenOutput {
Expand Down
146 changes: 84 additions & 62 deletions base_layer/wallet/src/output_manager_service/service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1443,89 +1443,97 @@ 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,
selection_criteria: UtxoSelectionCriteria,
) -> Result<UtxoSelection, OutputManagerError> {
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,
})
Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/wallet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 18 additions & 14 deletions base_layer/wallet/tests/output_manager_service_tests/service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
}

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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]
Expand All @@ -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;
Expand All @@ -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);
Expand Down
Loading

0 comments on commit 2b7b5c0

Please sign in to comment.