Skip to content

Commit

Permalink
feat: hide Coinbases that are in the process of being mined (#4602)
Browse files Browse the repository at this point in the history
Description
---
This PR hides coinbases by default that are in the process of being mined and not yet included in a mined block. 
It also removes them from the pending incoming balance. 

Motivation and Context
---
We changed how the coinbases are handled in the wallet. Previously the wallet only had a single pending coinbase utxo. This was then shown as pending incoming and that single coinbase utxo was shown on the completed transactions screen on the UI. It was removed after it was not mined for the height or changed to MInedUnconfirmed if actually mined. 

Wallets can now keep multiple coinbases for a specific height and keep them there till they can either confirm or deny they have been mined. When running multiple miners, this can mean that you have a coinbase utxo per miner (will only happen if the actual fees change per utxo). This floods the transaction screen with many many false transactions and makes the pending incoming balance useless as all of these will count towards that amount. 

How Has This Been Tested?
---
Manual


Fixes #4584
Fixes #4583
  • Loading branch information
SWvheerden authored Sep 6, 2022
1 parent a81228c commit c6c47fc
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ impl<B: Backend> Component<B> for TransactionsTab {
span_vec.push(Span::styled("(C)", Style::default().add_modifier(Modifier::BOLD)));
span_vec.push(Span::raw(" cancel selected pending Txs "));
span_vec.push(Span::styled("(A)", Style::default().add_modifier(Modifier::BOLD)));
span_vec.push(Span::raw(" show/hide abandoned coinbases "));
span_vec.push(Span::raw(" show/hide mining "));
span_vec.push(Span::styled("(R)", Style::default().add_modifier(Modifier::BOLD)));
span_vec.push(Span::raw(" rebroadcast Txs "));
span_vec.push(Span::styled("(Esc)", Style::default().add_modifier(Modifier::BOLD)));
Expand Down
1 change: 1 addition & 0 deletions applications/tari_console_wallet/src/ui/state/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ impl AppState {
.completed_txs
.iter()
.filter(|tx| !matches!(tx.cancelled, Some(TxCancellationReason::AbandonedCoinbase)))
.filter(|tx| !matches!(tx.status, TransactionStatus::Coinbase))
.collect()
} else {
self.cached_data.completed_txs.iter().collect()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl OutputSql {
FROM outputs WHERE status = ? AND maturity > ? OR script_lock_height > ? \
UNION ALL \
SELECT coalesce(sum(value), 0) as amount, 'pending_incoming_balance' as category \
FROM outputs WHERE status = ? OR status = ? OR status = ? \
FROM outputs WHERE source != ? AND status = ? OR status = ? OR status = ? \
UNION ALL \
SELECT coalesce(sum(value), 0) as amount, 'pending_outgoing_balance' as category \
FROM outputs WHERE status = ? OR status = ? OR status = ?",
Expand All @@ -403,6 +403,7 @@ impl OutputSql {
.bind::<diesel::sql_types::BigInt, _>(current_tip as i64)
.bind::<diesel::sql_types::BigInt, _>(current_tip as i64)
// pending_incoming_balance
.bind::<diesel::sql_types::Integer, _>(OutputSource::Coinbase as i32)
.bind::<diesel::sql_types::Integer, _>(OutputStatus::EncumberedToBeReceived as i32)
.bind::<diesel::sql_types::Integer, _>(OutputStatus::ShortTermEncumberedToBeReceived as i32)
.bind::<diesel::sql_types::Integer, _>(OutputStatus::UnspentMinedUnconfirmed as i32)
Expand All @@ -417,14 +418,15 @@ impl OutputSql {
FROM outputs WHERE status = ? \
UNION ALL \
SELECT coalesce(sum(value), 0) as amount, 'pending_incoming_balance' as category \
FROM outputs WHERE status = ? OR status = ? OR status = ? \
FROM outputs WHERE source != ? AND status = ? OR status = ? OR status = ? \
UNION ALL \
SELECT coalesce(sum(value), 0) as amount, 'pending_outgoing_balance' as category \
FROM outputs WHERE status = ? OR status = ? OR status = ?",
)
// available_balance
.bind::<diesel::sql_types::Integer, _>(OutputStatus::Unspent as i32)
// pending_incoming_balance
.bind::<diesel::sql_types::Integer, _>(OutputSource::Coinbase as i32)
.bind::<diesel::sql_types::Integer, _>(OutputStatus::EncumberedToBeReceived as i32)
.bind::<diesel::sql_types::Integer, _>(OutputStatus::ShortTermEncumberedToBeReceived as i32)
.bind::<diesel::sql_types::Integer, _>(OutputStatus::UnspentMinedUnconfirmed as i32)
Expand Down
12 changes: 5 additions & 7 deletions base_layer/wallet/tests/output_manager_service_tests/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1227,10 +1227,8 @@ async fn handle_coinbase_with_bulletproofs_rewinding() {

let reward1 = MicroTari::from(1000);
let fees1 = MicroTari::from(500);
let value1 = reward1 + fees1;
let reward2 = MicroTari::from(2000);
let fees2 = MicroTari::from(500);
let value2 = reward2 + fees2;
let reward3 = MicroTari::from(3000);
let fees3 = MicroTari::from(500);
let value3 = reward3 + fees3;
Expand All @@ -1241,13 +1239,14 @@ async fn handle_coinbase_with_bulletproofs_rewinding() {
.await
.unwrap();
assert_eq!(oms.output_manager_handle.get_unspent_outputs().await.unwrap().len(), 0);
// pending coinbases should not show up as pending incoming
assert_eq!(
oms.output_manager_handle
.get_balance()
.await
.unwrap()
.pending_incoming_balance,
value1
MicroTari::from(0)
);

let _tx2 = oms
Expand All @@ -1262,7 +1261,7 @@ async fn handle_coinbase_with_bulletproofs_rewinding() {
.await
.unwrap()
.pending_incoming_balance,
value1 + value2
MicroTari::from(0)
);
let tx3 = oms
.output_manager_handle
Expand All @@ -1276,7 +1275,7 @@ async fn handle_coinbase_with_bulletproofs_rewinding() {
.await
.unwrap()
.pending_incoming_balance,
value1 + value2 + value3
MicroTari::from(0)
);

let output = tx3.body.outputs()[0].clone();
Expand Down Expand Up @@ -1482,8 +1481,7 @@ async fn test_txo_validation() {
MicroTari::from(output1_value) -
MicroTari::from(900_000) -
MicroTari::from(1260) + //Output4 = output 1 -900_000 and 1260 for fees
MicroTari::from(8_000_000) +
MicroTari::from(16_000_000)
MicroTari::from(8_000_000)
);

// Output 1: Spent in Block 5 - Unconfirmed
Expand Down
36 changes: 16 additions & 20 deletions base_layer/wallet/tests/transaction_service_tests/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3126,7 +3126,7 @@ async fn test_coinbase_transactions_rejection_same_hash_but_accept_on_same_heigh
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1
MicroTari::from(0)
);

// Create a second coinbase txn at the first block height, with same output hash as the previous one
Expand Down Expand Up @@ -3154,7 +3154,7 @@ async fn test_coinbase_transactions_rejection_same_hash_but_accept_on_same_heigh
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1
MicroTari::from(0)
);

// Create another coinbase Txn at the same block height; the previous one should not be cancelled
Expand All @@ -3181,7 +3181,7 @@ async fn test_coinbase_transactions_rejection_same_hash_but_accept_on_same_heigh
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1 + fees2 + reward2
MicroTari::from(0)
);

// Create a third coinbase Txn at the second block height; all the three should be valid
Expand All @@ -3208,7 +3208,7 @@ async fn test_coinbase_transactions_rejection_same_hash_but_accept_on_same_heigh
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1 + fees2 + reward2 + fees3 + reward3
MicroTari::from(0)
);

assert!(transactions.values().any(|tx| tx.amount == fees1 + reward1));
Expand Down Expand Up @@ -3263,7 +3263,7 @@ async fn test_coinbase_generation_and_monitoring() {
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1
MicroTari::from(0)
);

// Create another coinbase Txn at the next block height
Expand All @@ -3290,7 +3290,7 @@ async fn test_coinbase_generation_and_monitoring() {
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1 + fees2 + reward2
MicroTari::from(0)
);

// Take out a second one at the second height which should not overwrite the initial one
Expand All @@ -3317,7 +3317,7 @@ async fn test_coinbase_generation_and_monitoring() {
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1 + fees2b + reward2 + fees2 + reward2
MicroTari::from(0)
);

assert!(transactions.values().any(|tx| tx.amount == fees1 + reward1));
Expand Down Expand Up @@ -3515,7 +3515,7 @@ async fn test_coinbase_abandoned() {
.await
.unwrap()
.pending_incoming_balance,
fees1 + reward1
MicroTari::from(0)
);

let transaction_query_batch_responses = vec![TxQueryBatchResponseProto {
Expand Down Expand Up @@ -3549,7 +3549,7 @@ async fn test_coinbase_abandoned() {
.get_balance()
.await
.unwrap();
assert_eq!(balance.pending_incoming_balance, fees1 + reward1);
assert_eq!(balance.pending_incoming_balance, MicroTari::from(0));

let validation_id = alice_ts_interface
.transaction_service_handle
Expand Down Expand Up @@ -3644,7 +3644,7 @@ async fn test_coinbase_abandoned() {
.await
.unwrap()
.pending_incoming_balance,
fees2 + reward2
MicroTari::from(0)
);

let transaction_query_batch_responses = vec![
Expand Down Expand Up @@ -3963,22 +3963,20 @@ async fn test_coinbase_transaction_reused_for_same_height() {
.await
.unwrap();

let expected_pending_incoming_balance = fees1 + reward1;
assert_eq!(transactions.len(), 1);
let mut amount = MicroTari::zero();
for tx in transactions.values() {
amount += tx.amount;
}
assert_eq!(amount, expected_pending_incoming_balance);
// balance should be fees1 + reward1, not double
assert_eq!(amount, fees1 + reward1);
assert_eq!(
ts_interface
.output_manager_service_handle
.get_balance()
.await
.unwrap()
.pending_incoming_balance,
expected_pending_incoming_balance
MicroTari::from(0)
);

// a requested coinbase transaction for the same height but new amount should be different
Expand All @@ -3994,21 +3992,20 @@ async fn test_coinbase_transaction_reused_for_same_height() {
.get_completed_transactions()
.await
.unwrap();
let expected_pending_incoming_balance = fees1 + reward1 + fees2 + reward2;
assert_eq!(transactions.len(), 2);
let mut amount = MicroTari::zero();
for tx in transactions.values() {
amount += tx.amount;
}
assert_eq!(amount, expected_pending_incoming_balance);
assert_eq!(amount, fees1 + reward1 + fees2 + reward2);
assert_eq!(
ts_interface
.output_manager_service_handle
.get_balance()
.await
.unwrap()
.pending_incoming_balance,
expected_pending_incoming_balance
MicroTari::from(0)
);

// a requested coinbase transaction for a new height should be different
Expand All @@ -4024,21 +4021,20 @@ async fn test_coinbase_transaction_reused_for_same_height() {
.get_completed_transactions()
.await
.unwrap();
let expected_pending_incoming_balance = fees1 + reward1 + 2 * (fees2 + reward2);
assert_eq!(transactions.len(), 3);
let mut amount = MicroTari::zero();
for tx in transactions.values() {
amount += tx.amount;
}
assert_eq!(amount, expected_pending_incoming_balance);
assert_eq!(amount, fees1 + reward1 + fees2 + reward2 + fees2 + reward2);
assert_eq!(
ts_interface
.output_manager_service_handle
.get_balance()
.await
.unwrap()
.pending_incoming_balance,
expected_pending_incoming_balance
MicroTari::from(0)
);
}

Expand Down

0 comments on commit c6c47fc

Please sign in to comment.