Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: wallet validation #6468

Merged
merged 4 commits into from
Aug 15, 2024
Merged

fix: wallet validation #6468

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b74b1d3
WIP on development
SWvheerden Aug 14, 2024
20fbdd8
fix test
SWvheerden Aug 15, 2024
151f88c
update config
SWvheerden Aug 15, 2024
c9340d8
fix comment
SWvheerden Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions applications/minotari_console_wallet/src/ui/components/transactions_tab.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,17 @@ impl TransactionsTab {
);

let confirmation_count = app_state.get_confirmations(tx.tx_id);
let confirmations_msg = if tx.status == TransactionStatus::MinedConfirmed && tx.cancelled.is_none() {
let confirmations_msg = if (tx.status == TransactionStatus::MinedConfirmed ||
tx.status == TransactionStatus::OneSidedConfirmed ||
tx.status == TransactionStatus::CoinbaseConfirmed) &&
tx.cancelled.is_none()
{
format!("{} required confirmations met", required_confirmations)
} else if tx.status == TransactionStatus::MinedUnconfirmed && tx.cancelled.is_none() {
} else if (tx.status == TransactionStatus::MinedUnconfirmed ||
tx.status == TransactionStatus::OneSidedUnconfirmed ||
tx.status == TransactionStatus::CoinbaseUnconfirmed) &&
tx.cancelled.is_none()
{
if let Some(count) = confirmation_count {
format!("{} of {} required confirmations met", count, required_confirmations)
} else {
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/base_node_service/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct BaseNodeServiceConfig {
impl Default for BaseNodeServiceConfig {
fn default() -> Self {
Self {
base_node_monitor_max_refresh_interval: Duration::from_secs(90),
base_node_monitor_max_refresh_interval: Duration::from_secs(30),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update the default value in the corresponding config.toml file as well

base_node_rpc_pool_size: 10,
event_channel_size: 250,
}
Expand Down
2 changes: 0 additions & 2 deletions base_layer/wallet/src/output_manager_service/tasks/txo_validation_task.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ use crate::{
database::{OutputManagerBackend, OutputManagerDatabase},
models::DbWalletOutput,
sqlite_db::{ReceivedOutputInfoForBatch, SpentOutputInfoForBatch},
OutputStatus,
},
},
};
Expand Down Expand Up @@ -351,7 +350,6 @@ where

let unmined_and_invalid: Vec<_> = unmined
.iter()
.filter(|uo| uo.status == OutputStatus::UnspentMinedUnconfirmed)
.map(|uo| {
info!(
target: LOG_TARGET,
Expand Down
11 changes: 6 additions & 5 deletions base_layer/wallet/src/transaction_service/tasks/check_faux_transaction_status.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

const SAFETY_HEIGHT_MARGIN: u64 = 1000;
const SAFETY_HEIGHT_MARGIN: u64 = 3000;

use std::sync::Arc;

Expand Down Expand Up @@ -52,11 +52,12 @@ pub async fn check_detected_transactions<TBackend: 'static + TransactionBackend>
// height or current tip height with safety margin to determine if these should be returned
let last_mined_transaction = db.fetch_last_mined_transaction().unwrap_or_default();

let height_with_margin = tip_height.saturating_sub(SAFETY_HEIGHT_MARGIN);
let check_height = if let Some(tx) = last_mined_transaction {
tx.mined_height.unwrap_or(height_with_margin)
tx.mined_height
.unwrap_or(tip_height)
.saturating_sub(SAFETY_HEIGHT_MARGIN)
} else {
height_with_margin
tip_height.saturating_sub(SAFETY_HEIGHT_MARGIN)
};

let mut all_detected_transactions: Vec<CompletedTransaction> = match db.get_imported_transactions() {
Expand All @@ -78,7 +79,7 @@ pub async fn check_detected_transactions<TBackend: 'static + TransactionBackend>
};
all_detected_transactions.append(&mut unconfirmed_detected);

let mut unmined_coinbases_detected = match db.get_unmined_coinbase_transactions(height_with_margin) {
let mut unmined_coinbases_detected = match db.get_unmined_coinbase_transactions(check_height) {
Ok(txs) => txs,
Err(e) => {
error!(
Expand Down
74 changes: 47 additions & 27 deletions base_layer/wallet/tests/output_manager_service_tests/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ async fn test_txo_validation() {
&oms.key_manager_handle,
)
.await;
let output3_tx_output = output3.to_transaction_output(&oms.key_manager_handle).await.unwrap();

oms.output_manager_handle
.add_output_with_tx_id(TxId::from(3u64), output3.clone(), None)
Expand All @@ -1386,7 +1387,7 @@ async fn test_txo_validation() {
block_headers.insert(4, block4_header.clone());
oms.base_node_wallet_rpc_mock_state.set_blocks(block_headers.clone());

// These responses will mark outputs 1 and 2 and mined confirmed
// These responses will mark outputs 1,2,3 and mined confirmed
let responses = vec![
UtxoQueryResponse {
output: Some(output1_tx_output.clone().try_into().unwrap()),
Expand All @@ -1402,6 +1403,13 @@ async fn test_txo_validation() {
output_hash: output2_tx_output.hash().to_vec(),
mined_timestamp: 0,
},
UtxoQueryResponse {
output: Some(output3_tx_output.clone().try_into().unwrap()),
mined_at_height: 1,
mined_in_block: block1_header.hash().to_vec(),
output_hash: output3_tx_output.hash().to_vec(),
mined_timestamp: 0,
},
];

let utxo_query_responses = UtxoQueryResponses {
Expand All @@ -1413,7 +1421,7 @@ async fn test_txo_validation() {
oms.base_node_wallet_rpc_mock_state
.set_utxo_query_response(utxo_query_responses.clone());

// This response sets output1 and output2 as mined, not spent
// This response sets output1 and output2, output3 as mined, not spent
let query_deleted_response = QueryDeletedResponse {
best_block_hash: block4_header.hash().to_vec(),
best_block_height: 4,
Expand All @@ -1430,6 +1438,12 @@ async fn test_txo_validation() {
height_deleted_at: 0,
block_deleted_in: Vec::new(),
},
QueryDeletedData {
mined_at_height: 1,
block_mined_in: block1_header.hash().to_vec(),
height_deleted_at: 0,
block_deleted_in: Vec::new(),
},
],
};

Expand Down Expand Up @@ -1511,7 +1525,7 @@ async fn test_txo_validation() {

// Output 1: Spent in Block 5 - Unconfirmed
// Output 2: Mined block 1 Confirmed Block 4
// Output 3: Imported so will have Unspent status.
// Output 3: Mined block 1 Confirmed Block 4.
// Output 4: Received in Block 5 - Unconfirmed - Change from spending Output 1
// Output 5: Received in Block 5 - Unconfirmed
// Output 6: Coinbase from Block 5 - Unconfirmed
Expand All @@ -1536,6 +1550,13 @@ async fn test_txo_validation() {
output_hash: output2_tx_output.hash().to_vec(),
mined_timestamp: 0,
},
UtxoQueryResponse {
output: Some(output3_tx_output.clone().try_into().unwrap()),
mined_at_height: 1,
mined_in_block: block1_header.hash().to_vec(),
output_hash: output3_tx_output.hash().to_vec(),
mined_timestamp: 0,
},
UtxoQueryResponse {
output: Some(output4_tx_output.clone().try_into().unwrap()),
mined_at_height: 5,
Expand Down Expand Up @@ -1578,6 +1599,12 @@ async fn test_txo_validation() {
height_deleted_at: 0,
block_deleted_in: Vec::new(),
},
QueryDeletedData {
mined_at_height: 1,
block_mined_in: block1_header.hash().to_vec(),
height_deleted_at: 0,
block_deleted_in: Vec::new(),
},
QueryDeletedData {
mined_at_height: 5,
block_mined_in: block5_header.hash().to_vec(),
Expand Down Expand Up @@ -1610,14 +1637,14 @@ async fn test_txo_validation() {
.await
.unwrap();

assert_eq!(utxo_query_calls[0].len(), 3);
assert_eq!(utxo_query_calls[0].len(), 2);

let query_deleted_calls = oms
.base_node_wallet_rpc_mock_state
.wait_pop_query_deleted(1, Duration::from_secs(60))
.await
.unwrap();
assert_eq!(query_deleted_calls[0].hashes.len(), 4);
assert_eq!(query_deleted_calls[0].hashes.len(), 5);

let balance = oms.output_manager_handle.get_balance().await.unwrap();
assert_eq!(
Expand Down Expand Up @@ -1657,15 +1684,15 @@ async fn test_txo_validation() {
.unwrap();

// The spent transaction is not checked during this second validation
assert_eq!(utxo_query_calls[0].len(), 3);
assert_eq!(utxo_query_calls[0].len(), 2);

let query_deleted_calls = oms
.base_node_wallet_rpc_mock_state
.wait_pop_query_deleted(1, Duration::from_secs(60))
.await
.unwrap();

assert_eq!(query_deleted_calls[0].hashes.len(), 4);
assert_eq!(query_deleted_calls[0].hashes.len(), 5);

let balance = oms.output_manager_handle.get_balance().await.unwrap();
assert_eq!(
Expand All @@ -1679,26 +1706,6 @@ async fn test_txo_validation() {
assert_eq!(balance.pending_incoming_balance, MicroMinotari::from(0));
assert_eq!(MicroMinotari::from(0), balance.time_locked_balance.unwrap());

// Trigger another validation and only Output3 should be checked
oms.output_manager_handle.validate_txos().await.unwrap();

let utxo_query_calls = oms
.base_node_wallet_rpc_mock_state
.wait_pop_utxo_query_calls(1, Duration::from_secs(60))
.await
.unwrap();
assert_eq!(utxo_query_calls.len(), 1);
assert_eq!(utxo_query_calls[0].len(), 1);
assert_eq!(
utxo_query_calls[0][0],
output3
.to_transaction_output(&oms.key_manager_handle)
.await
.unwrap()
.hash()
.to_vec()
);

// Now we will create responses that result in a reorg of block 5, keeping block4 the same.
// Output 1: Spent in Block 5 - Unconfirmed
// Output 2: Mined block 1 Confirmed Block 4
Expand Down Expand Up @@ -1730,6 +1737,13 @@ async fn test_txo_validation() {
output_hash: output2_tx_output.hash().to_vec(),
mined_timestamp: 0,
},
UtxoQueryResponse {
output: Some(output3_tx_output.clone().try_into().unwrap()),
mined_at_height: 1,
mined_in_block: block1_header.hash().to_vec(),
output_hash: output3_tx_output.hash().to_vec(),
mined_timestamp: 0,
},
UtxoQueryResponse {
output: Some(output4_tx_output.clone().try_into().unwrap()),
mined_at_height: 5,
Expand Down Expand Up @@ -1765,6 +1779,12 @@ async fn test_txo_validation() {
height_deleted_at: 0,
block_deleted_in: Vec::new(),
},
QueryDeletedData {
mined_at_height: 1,
block_mined_in: block1_header.hash().to_vec(),
height_deleted_at: 0,
block_deleted_in: Vec::new(),
},
QueryDeletedData {
mined_at_height: 5,
block_mined_in: block5_header_reorg.hash().to_vec(),
Expand Down
2 changes: 1 addition & 1 deletion common/config/presets/d_console_wallet.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ event_channel_size = 3500
[wallet.base_node]
# Configuration for the wallet's base node service
# The refresh interval (default = 3 s)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# The refresh interval (default = 3 s)
# The refresh interval (default = 30 s)

#base_node_monitor_max_refresh_interval = 3
#base_node_monitor_max_refresh_interval = 30
# The RPC client pool size (default = 5)
#base_node_rpc_pool_size = 5
# This is the size of the event channel used to communicate base node events to the wallet. (default = 250).
Expand Down
Loading