From 9064b830c04000683aecf7b2972ffeabe5d90f08 Mon Sep 17 00:00:00 2001 From: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com> Date: Tue, 9 Nov 2021 10:58:10 +0200 Subject: [PATCH] feat: optimize transaction validation for wallet (#3537) Description --- Optimized the transaction validation memory footprint by restricting all queries to the database to only return the information that is needed. This holds two considerable advantages for databases with many or large transactions: - The memory footprint is reduced. - Overall performance is increased. Motivation and Context --- See above. How Has This Been Tested? --- Unit tests Cucumber tests System-level tests with a large wallet --- .../src/chain_storage/blockchain_database.rs | 2 +- .../core/src/transactions/aggregated_body.rs | 2 +- .../core/src/transactions/transaction.rs | 4 +- .../down.sql | 48 +++++ .../up.sql | 5 + base_layer/wallet/src/schema.rs | 2 + .../wallet/src/transaction_service/error.rs | 4 + .../transaction_validation_protocol.rs | 117 ++++++----- .../transaction_service/storage/database.rs | 19 +- .../src/transaction_service/storage/models.rs | 32 ++- .../transaction_service/storage/sqlite_db.rs | 192 ++++++++++++++++-- .../tests/transaction_service/service.rs | 10 +- .../tests/transaction_service/storage.rs | 7 +- comms/dht/src/crypt.rs | 2 +- 14 files changed, 354 insertions(+), 92 deletions(-) create mode 100644 base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/down.sql create mode 100644 base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/up.sql diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index 52c1ce2099..5b3b07abf8 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -1141,7 +1141,7 @@ pub fn calculate_mmr_roots(db: &T, block: &Block) -> Resul output_mmr.compress(); - // TODO: #testnetreset clean up this code + // TODO: #testnet_reset clean up this code let input_mr = if header.version == 1 { MutableMmr::::new(input_mmr.get_pruned_hash_set()?, Bitmap::create())?.get_merkle_root()? } else { diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index a7f3134678..45a30c730b 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -221,7 +221,7 @@ impl AggregateBody { } self.inputs.sort(); self.outputs.sort(); - // TODO: #testnetreset clean up this code + // TODO: #testnet_reset clean up this code if version <= 1 { self.kernels.sort_by(|a, b| a.deprecated_cmp(b)); } else { diff --git a/base_layer/core/src/transactions/transaction.rs b/base_layer/core/src/transactions/transaction.rs index 9c59bbaddf..9b806a73f0 100644 --- a/base_layer/core/src/transactions/transaction.rs +++ b/base_layer/core/src/transactions/transaction.rs @@ -779,7 +779,7 @@ impl TransactionOutput { Challenge::new() .chain(public_commitment_nonce.as_bytes()) .chain(script.as_bytes()) - // TODO: Use consensus encoded bytes #testnet reset + // TODO: Use consensus encoded bytes #testnet_reset .chain(features.to_v1_bytes()) .chain(sender_offset_public_key.as_bytes()) .chain(commitment.as_bytes()) @@ -887,7 +887,7 @@ impl TransactionOutput { impl Hashable for TransactionOutput { fn hash(&self) -> Vec { HashDigest::new() - // TODO: use consensus encoding #testnetreset + // TODO: use consensus encoding #testnet_reset .chain(self.features.to_v1_bytes()) .chain(self.commitment.as_bytes()) // .chain(range proof) // See docs as to why we exclude this diff --git a/base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/down.sql b/base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/down.sql new file mode 100644 index 0000000000..e6f0ea5bae --- /dev/null +++ b/base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/down.sql @@ -0,0 +1,48 @@ +PRAGMA foreign_keys=OFF; +ALTER TABLE completed_transactions + RENAME TO completed_transactions_old; +CREATE TABLE completed_transactions ( + tx_id INTEGER PRIMARY KEY NOT NULL, + source_public_key BLOB NOT NULL, + destination_public_key BLOB NOT NULL, + amount INTEGER NOT NULL, + fee INTEGER NOT NULL, + transaction_protocol TEXT NOT NULL, + status INTEGER NOT NULL, + message TEXT NOT NULL, + timestamp DATETIME NOT NULL, + cancelled INTEGER NOT NULL DEFAULT 0, + direction INTEGER NULL DEFAULT NULL, + coinbase_block_height INTEGER NULL DEFAULT NULL, + send_count INTEGER NOT NULL DEFAULT 0, + last_send_timestamp DATETIME NULL DEFAULT NULL, + valid INTEGER NOT NULL DEFAULT 0, + confirmations INTEGER NULL DEFAULT NULL, + mined_height BIGINT NULL DEFAULT NULL, + mined_in_block BLOB NULL DEFAULT NULL + +); +INSERT INTO completed_transactions (tx_id, source_public_key, destination_public_key, amount, fee, transaction_protocol, + status, message, timestamp, cancelled, direction, coinbase_block_height, send_count, + last_send_timestamp, valid, confirmations) +SELECT tx_id, + source_public_key, + destination_public_key, + amount, + fee, + transaction_protocol, + status, + message, + timestamp, + cancelled, + direction, + coinbase_block_height, + send_count, + last_send_timestamp, + valid, + confirmations, + mined_height, + mined_in_block +FROM completed_transactions_old; +DROP TABLE completed_transactions_old; +PRAGMA foreign_keys=ON; diff --git a/base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/up.sql b/base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/up.sql new file mode 100644 index 0000000000..179e7b39bb --- /dev/null +++ b/base_layer/wallet/migrations/2021-10-28-164318_add_transaction_signature/up.sql @@ -0,0 +1,5 @@ +ALTER TABLE completed_transactions + ADD transaction_signature_nonce BLOB NOT NULL DEFAULT 0; + +ALTER TABLE completed_transactions + ADD transaction_signature_key BLOB NOT NULL DEFAULT 0; diff --git a/base_layer/wallet/src/schema.rs b/base_layer/wallet/src/schema.rs index 269b965f5d..570ae8cae1 100644 --- a/base_layer/wallet/src/schema.rs +++ b/base_layer/wallet/src/schema.rs @@ -25,6 +25,8 @@ table! { confirmations -> Nullable, mined_height -> Nullable, mined_in_block -> Nullable, + transaction_signature_nonce -> Binary, + transaction_signature_key -> Binary, } } diff --git a/base_layer/wallet/src/transaction_service/error.rs b/base_layer/wallet/src/transaction_service/error.rs index 17f3b324a8..bf7077ec72 100644 --- a/base_layer/wallet/src/transaction_service/error.rs +++ b/base_layer/wallet/src/transaction_service/error.rs @@ -166,6 +166,10 @@ pub enum TransactionKeyError { Source(ByteArrayError), #[error("Invalid destination PublicKey")] Destination(ByteArrayError), + #[error("Invalid transaction signature nonce")] + SignatureNonce(ByteArrayError), + #[error("Invalid transaction signature key")] + SignatureKey(ByteArrayError), } #[derive(Debug, Error)] diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs index fff77520b0..eff2c9f0cc 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs @@ -29,7 +29,7 @@ use crate::{ handle::{TransactionEvent, TransactionEventSender}, storage::{ database::{TransactionBackend, TransactionDatabase}, - models::CompletedTransaction, + sqlite_db::UnconfirmedTransactionInfo, }, }, }; @@ -39,7 +39,10 @@ use std::{ convert::{TryFrom, TryInto}, sync::Arc, }; -use tari_common_types::{transaction::TransactionStatus, types::BlockHash}; +use tari_common_types::{ + transaction::{TransactionStatus, TxId}, + types::BlockHash, +}; use tari_comms::protocol::rpc::{RpcError::RequestFailed, RpcStatusCode::NotFound}; use tari_core::{ base_node::{ @@ -61,6 +64,7 @@ pub struct TransactionValidationProtocol TransactionValidationProtocol @@ -99,14 +103,15 @@ where target: LOG_TARGET, "Checking if transactions have been mined since last we checked" ); - let unmined_transactions = self + // Fetch completed but unconfirmed transactions that were not imported + let unconfirmed_transactions = self .db - .fetch_unconfirmed_transactions() + .fetch_unconfirmed_transactions_info() .await .for_protocol(self.operation_id) .unwrap(); - for batch in unmined_transactions.chunks(self.config.max_tx_query_batch_size) { + for batch in unconfirmed_transactions.chunks(self.config.max_tx_query_batch_size) { let (mined, unmined, tip_info) = self .query_base_node_for_transactions(batch, &mut *base_node_wallet_client) .await @@ -117,22 +122,28 @@ where mined.len(), unmined.len() ); - for (tx, mined_height, mined_in_block, num_confirmations) in &mined { - info!(target: LOG_TARGET, "Updating transaction {} as mined", tx.tx_id); - self.update_transaction_as_mined(tx, mined_in_block, *mined_height, *num_confirmations) - .await?; + for (mined_tx, mined_height, mined_in_block, num_confirmations) in &mined { + info!(target: LOG_TARGET, "Updating transaction {} as mined", mined_tx.tx_id); + self.update_transaction_as_mined( + mined_tx.tx_id, + &mined_tx.status, + mined_in_block, + *mined_height, + *num_confirmations, + ) + .await?; } if let Some((tip_height, tip_block)) = tip_info { - for tx in &unmined { + for unmined_tx in &unmined { // Treat coinbases separately - if tx.is_coinbase() { - if tx.coinbase_block_height.unwrap_or_default() <= tip_height { - info!(target: LOG_TARGET, "Updated coinbase {} as abandoned", tx.tx_id); + if unmined_tx.is_coinbase() { + if unmined_tx.coinbase_block_height.unwrap_or_default() <= tip_height { + info!(target: LOG_TARGET, "Updated coinbase {} as abandoned", unmined_tx.tx_id); self.update_coinbase_as_abandoned( - tx, + unmined_tx.tx_id, &tip_block, tip_height, - tip_height.saturating_sub(tx.coinbase_block_height.unwrap_or_default()), + tip_height.saturating_sub(unmined_tx.coinbase_block_height.unwrap_or_default()), ) .await?; } else { @@ -140,13 +151,17 @@ where target: LOG_TARGET, "Coinbase not found, but it is for a block that is not yet in the chain. Coinbase \ height: {}, tip height:{}", - tx.coinbase_block_height.unwrap_or_default(), + unmined_tx.coinbase_block_height.unwrap_or_default(), tip_height ); } } else { - info!(target: LOG_TARGET, "Updated transaction {} as unmined", tx.tx_id); - self.update_transaction_as_unmined(tx).await?; + info!( + target: LOG_TARGET, + "Updated transaction {} as unmined", unmined_tx.tx_id + ); + self.update_transaction_as_unmined(unmined_tx.tx_id, &unmined_tx.status) + .await?; } } } @@ -216,7 +231,8 @@ where .map(|k| k.excess.to_hex()) .unwrap() ); - self.update_transaction_as_unmined(&last_mined_transaction).await?; + self.update_transaction_as_unmined(last_mined_transaction.tx_id, &last_mined_transaction.status) + .await?; } else { info!( target: LOG_TARGET, @@ -230,12 +246,12 @@ where async fn query_base_node_for_transactions( &self, - batch: &[CompletedTransaction], + batch: &[UnconfirmedTransactionInfo], base_node_client: &mut BaseNodeWalletRpcClient, ) -> Result< ( - Vec<(CompletedTransaction, u64, BlockHash, u64)>, - Vec, + Vec<(UnconfirmedTransactionInfo, u64, BlockHash, u64)>, + Vec, Option<(u64, BlockHash)>, ), TransactionServiceError, @@ -244,10 +260,10 @@ where let mut unmined = vec![]; let mut batch_signatures = HashMap::new(); - for tx in batch.iter() { - // Imported transactions do not have a signature - if let Some(sig) = tx.transaction.first_kernel_excess_sig() { - batch_signatures.insert(sig.clone(), tx); + for tx_info in batch.iter() { + // Imported transactions do not have a signature; this is represented by the default signature in info + if tx_info.signature != Signature::default() { + batch_signatures.insert(tx_info.signature.clone(), tx_info); } } @@ -259,7 +275,7 @@ where info!( target: LOG_TARGET, "Asking base node for location of {} transactions by excess signature", - batch.len() + batch_signatures.len() ); let batch_response = base_node_client @@ -275,16 +291,16 @@ where let response = TxQueryBatchResponse::try_from(response_proto) .map_err(TransactionServiceError::ProtobufConversionError)?; let sig = response.signature; - if let Some(completed_tx) = batch_signatures.get(&sig) { + if let Some(unconfirmed_tx) = batch_signatures.get(&sig) { if response.location == TxLocation::Mined { mined.push(( - (*completed_tx).clone(), + (*unconfirmed_tx).clone(), response.block_height, response.block_hash.unwrap(), response.confirmations, )); } else { - unmined.push((*completed_tx).clone()); + unmined.push((*unconfirmed_tx).clone()); } } } @@ -333,14 +349,15 @@ where #[allow(clippy::ptr_arg)] async fn update_transaction_as_mined( &mut self, - tx: &CompletedTransaction, + tx_id: TxId, + status: &TransactionStatus, mined_in_block: &BlockHash, mined_height: u64, num_confirmations: u64, ) -> Result<(), TransactionServiceProtocolError> { self.db .set_transaction_mined_height( - tx.tx_id, + tx_id, true, mined_height, mined_in_block.clone(), @@ -351,23 +368,20 @@ where .for_protocol(self.operation_id)?; if num_confirmations >= self.config.num_confirmations_required { - self.publish_event(TransactionEvent::TransactionMined { - tx_id: tx.tx_id, - is_valid: true, - }) + self.publish_event(TransactionEvent::TransactionMined { tx_id, is_valid: true }) } else { self.publish_event(TransactionEvent::TransactionMinedUnconfirmed { - tx_id: tx.tx_id, + tx_id, num_confirmations, is_valid: true, }) } - if tx.status == TransactionStatus::Coinbase { - if let Err(e) = self.output_manager_handle.set_coinbase_abandoned(tx.tx_id, false).await { + if *status == TransactionStatus::Coinbase { + if let Err(e) = self.output_manager_handle.set_coinbase_abandoned(tx_id, false).await { warn!( target: LOG_TARGET, - "Could not mark coinbase output for TxId: {} as not abandoned: {}", tx.tx_id, e + "Could not mark coinbase output for TxId: {} as not abandoned: {}", tx_id, e ); }; } @@ -378,14 +392,14 @@ where #[allow(clippy::ptr_arg)] async fn update_coinbase_as_abandoned( &mut self, - tx: &CompletedTransaction, + tx_id: TxId, mined_in_block: &BlockHash, mined_height: u64, num_confirmations: u64, ) -> Result<(), TransactionServiceProtocolError> { self.db .set_transaction_mined_height( - tx.tx_id, + tx_id, false, mined_height, mined_in_block.clone(), @@ -395,37 +409,38 @@ where .await .for_protocol(self.operation_id)?; - if let Err(e) = self.output_manager_handle.set_coinbase_abandoned(tx.tx_id, true).await { + if let Err(e) = self.output_manager_handle.set_coinbase_abandoned(tx_id, true).await { warn!( target: LOG_TARGET, - "Could not mark coinbase output for TxId: {} as abandoned: {}", tx.tx_id, e + "Could not mark coinbase output for TxId: {} as abandoned: {}", tx_id, e ); }; - self.publish_event(TransactionEvent::TransactionCancelled(tx.tx_id)); + self.publish_event(TransactionEvent::TransactionCancelled(tx_id)); Ok(()) } async fn update_transaction_as_unmined( &mut self, - tx: &CompletedTransaction, + tx_id: TxId, + status: &TransactionStatus, ) -> Result<(), TransactionServiceProtocolError> { self.db - .set_transaction_as_unmined(tx.tx_id) + .set_transaction_as_unmined(tx_id) .await .for_protocol(self.operation_id)?; - if tx.status == TransactionStatus::Coinbase { - if let Err(e) = self.output_manager_handle.set_coinbase_abandoned(tx.tx_id, false).await { + if *status == TransactionStatus::Coinbase { + if let Err(e) = self.output_manager_handle.set_coinbase_abandoned(tx_id, false).await { warn!( target: LOG_TARGET, - "Could not mark coinbase output for TxId: {} as not abandoned: {}", tx.tx_id, e + "Could not mark coinbase output for TxId: {} as not abandoned: {}", tx_id, e ); }; } - self.publish_event(TransactionEvent::TransactionBroadcast(tx.tx_id)); + self.publish_event(TransactionEvent::TransactionBroadcast(tx_id)); Ok(()) } } diff --git a/base_layer/wallet/src/transaction_service/storage/database.rs b/base_layer/wallet/src/transaction_service/storage/database.rs index b8ab7a7a65..8d97ebe971 100644 --- a/base_layer/wallet/src/transaction_service/storage/database.rs +++ b/base_layer/wallet/src/transaction_service/storage/database.rs @@ -28,7 +28,10 @@ use aes_gcm::Aes256Gcm; use chrono::Utc; use log::*; -use crate::transaction_service::storage::{models::WalletTransaction, sqlite_db::InboundTransactionSenderInfo}; +use crate::transaction_service::storage::{ + models::WalletTransaction, + sqlite_db::{InboundTransactionSenderInfo, UnconfirmedTransactionInfo}, +}; use std::{ collections::HashMap, fmt, @@ -54,7 +57,8 @@ pub trait TransactionBackend: Send + Sync + Clone { fn fetch_last_mined_transaction(&self) -> Result, TransactionStorageError>; - fn fetch_unconfirmed_transactions(&self) -> Result, TransactionStorageError>; + /// Light weight method to retrieve pertinent unconfirmed transactions info from completed transactions + fn fetch_unconfirmed_transactions_info(&self) -> Result, TransactionStorageError>; fn get_transactions_to_be_broadcast(&self) -> Result, TransactionStorageError>; @@ -131,9 +135,9 @@ pub trait TransactionBackend: Send + Sync + Clone { ) -> Result<(), TransactionStorageError>; /// Clears the mined block and height of a transaction fn set_transaction_as_unmined(&self, tx_id: TxId) -> Result<(), TransactionStorageError>; - /// Mark all transactions as unvalidated + /// Reset optional 'mined height' and 'mined in block' fields to nothing fn mark_all_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError>; - /// Get transaction sender info for all pending inbound transactions + /// Light weight method to retrieve pertinent transaction sender info for all pending inbound transactions fn get_pending_inbound_transaction_sender_info( &self, ) -> Result, TransactionStorageError>; @@ -431,8 +435,11 @@ where T: TransactionBackend + 'static self.db.fetch_last_mined_transaction() } - pub async fn fetch_unconfirmed_transactions(&self) -> Result, TransactionStorageError> { - self.db.fetch_unconfirmed_transactions() + /// Light weight method to return completed but unconfirmed transactions that were not imported + pub async fn fetch_unconfirmed_transactions_info( + &self, + ) -> Result, TransactionStorageError> { + self.db.fetch_unconfirmed_transactions_info() } /// This method returns all completed transactions that must be broadcast diff --git a/base_layer/wallet/src/transaction_service/storage/models.rs b/base_layer/wallet/src/transaction_service/storage/models.rs index 2125081a0b..a0925364a8 100644 --- a/base_layer/wallet/src/transaction_service/storage/models.rs +++ b/base_layer/wallet/src/transaction_service/storage/models.rs @@ -24,7 +24,7 @@ use chrono::NaiveDateTime; use serde::{Deserialize, Serialize}; use tari_common_types::{ transaction::{TransactionDirection, TransactionStatus, TxId}, - types::{BlockHash, PrivateKey}, + types::{BlockHash, PrivateKey, Signature}, }; use tari_comms::types::CommsPublicKey; use tari_core::transactions::{ @@ -138,6 +138,7 @@ pub struct CompletedTransaction { pub send_count: u32, pub last_send_timestamp: Option, pub valid: bool, + pub transaction_signature: Signature, pub confirmations: Option, pub mined_height: Option, pub mined_in_block: Option, @@ -158,6 +159,11 @@ impl CompletedTransaction { direction: TransactionDirection, coinbase_block_height: Option, ) -> Self { + let transaction_signature = if let Some(excess_sig) = transaction.first_kernel_excess_sig() { + excess_sig.clone() + } else { + Signature::default() + }; Self { tx_id, source_public_key, @@ -174,6 +180,7 @@ impl CompletedTransaction { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature, confirmations: None, mined_height: None, mined_in_block: None, @@ -181,7 +188,11 @@ impl CompletedTransaction { } pub fn is_coinbase(&self) -> bool { - self.coinbase_block_height.is_some() + if let Some(height) = self.coinbase_block_height { + height > 0 + } else { + false + } } } @@ -224,6 +235,19 @@ impl From for OutboundTransaction { impl From for CompletedTransaction { fn from(tx: OutboundTransaction) -> Self { + let transaction = if tx.sender_protocol.is_finalized() { + match tx.sender_protocol.get_transaction() { + Ok(tx) => tx.clone(), + Err(_) => Transaction::new(vec![], vec![], vec![], PrivateKey::default(), PrivateKey::default()), + } + } else { + Transaction::new(vec![], vec![], vec![], PrivateKey::default(), PrivateKey::default()) + }; + let transaction_signature = if let Some(excess_sig) = transaction.first_kernel_excess_sig() { + excess_sig.clone() + } else { + Signature::default() + }; Self { tx_id: tx.tx_id, source_public_key: Default::default(), @@ -234,12 +258,13 @@ impl From for CompletedTransaction { message: tx.message, timestamp: tx.timestamp, cancelled: tx.cancelled, - transaction: Transaction::new(vec![], vec![], vec![], PrivateKey::default(), PrivateKey::default()), + transaction, direction: TransactionDirection::Outbound, coinbase_block_height: None, send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature, confirmations: None, mined_height: None, mined_in_block: None, @@ -265,6 +290,7 @@ impl From for CompletedTransaction { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: Signature::default(), confirmations: None, mined_height: None, mined_in_block: None, diff --git a/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs b/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs index bc72c94366..307eee85fe 100644 --- a/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs +++ b/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs @@ -53,7 +53,7 @@ use tari_common_types::{ TransactionStatus, TxId, }, - types::{BlockHash, PublicKey}, + types::{BlockHash, PrivateKey, PublicKey, Signature}, }; use tari_comms::types::CommsPublicKey; use tari_core::transactions::tari_amount::MicroTari; @@ -75,10 +75,75 @@ pub struct TransactionServiceSqliteDatabase { impl TransactionServiceSqliteDatabase { pub fn new(database_connection: WalletDbConnection, cipher: Option) -> Self { - Self { + let mut new_self = Self { database_connection, cipher: Arc::new(RwLock::new(cipher)), + }; + + // TODO: Remove this call for the next #testnet_reset + match new_self.add_transaction_signature_for_legacy_transactions() { + Ok(count) => { + if count > 0 { + info!( + target: LOG_TARGET, + "Updated transaction signatures for {} legacy transactions", count + ); + } + }, + Err(e) => warn!( + target: LOG_TARGET, + "Legacy transaction signatures could not be updated: {:?}", e + ), + }; + + new_self + } + + // TODO: Remove this function for the next #testnet_reset + fn add_transaction_signature_for_legacy_transactions(&mut self) -> Result { + let conn = self.database_connection.acquire_lock(); + let txs_sql = completed_transactions::table + .filter( + completed_transactions::transaction_signature_nonce + .eq(completed_transactions::transaction_signature_key), + ) + .load::(&*conn)?; + + let mut count = 0u32; + if !txs_sql.is_empty() { + info!( + target: LOG_TARGET, + "Updating transaction signatures for {} legacy transactions...", + txs_sql.len() + ); + for mut tx_sql in txs_sql { + self.decrypt_if_necessary(&mut tx_sql)?; + let tx = CompletedTransaction::try_from(tx_sql.clone())?; + let (transaction_signature_nonce, transaction_signature_key) = + if let Some(transaction_signature) = tx.transaction.first_kernel_excess_sig() { + ( + Some(transaction_signature.get_public_nonce().as_bytes().to_vec()), + Some(transaction_signature.get_signature().as_bytes().to_vec()), + ) + } else { + ( + Some(Signature::default().get_public_nonce().as_bytes().to_vec()), + Some(Signature::default().get_signature().as_bytes().to_vec()), + ) + }; + tx_sql.update( + UpdateCompletedTransactionSql { + transaction_signature_nonce, + transaction_signature_key, + ..Default::default() + }, + &(*conn), + )?; + count += 1; + } } + + Ok(count) } fn insert(&self, kvp: DbKeyValuePair, conn: MutexGuard) -> Result<(), TransactionStorageError> { @@ -1042,34 +1107,28 @@ impl TransactionBackend for TransactionServiceSqliteDatabase { Ok(result) } - fn fetch_unconfirmed_transactions(&self) -> Result, TransactionStorageError> { + fn fetch_unconfirmed_transactions_info(&self) -> Result, TransactionStorageError> { let start = Instant::now(); let conn = self.database_connection.acquire_lock(); let acquire_lock = start.elapsed(); - let txs = completed_transactions::table - .filter( - completed_transactions::mined_height - .is_null() - .or(completed_transactions::status.eq(TransactionStatus::MinedUnconfirmed as i32)), - ) - .filter(completed_transactions::cancelled.eq(false as i32)) - .order_by(completed_transactions::tx_id) - .load::(&*conn)?; - - let mut result = vec![]; - for mut tx in txs { - self.decrypt_if_necessary(&mut tx)?; - result.push(tx.try_into()?); + let mut tx_info: Vec = vec![]; + match UnconfirmedTransactionInfoSql::fetch_unconfirmed_transactions_info(&*conn) { + Ok(info) => { + for item in info { + tx_info.push(UnconfirmedTransactionInfo::try_from(item)?); + } + }, + Err(e) => return Err(e), } trace!( target: LOG_TARGET, - "sqlite profile - fetch_unconfirmed_transactions: lock {} + db_op {} = {} ms", + "sqlite profile - fetch_unconfirmed_transactions_info: lock {} + db_op {} = {} ms", acquire_lock.as_millis(), (start.elapsed() - acquire_lock).as_millis(), start.elapsed().as_millis() ); - Ok(result) + Ok(tx_info) } fn get_transactions_to_be_broadcast(&self) -> Result, TransactionStorageError> { @@ -1591,6 +1650,8 @@ struct CompletedTransactionSql { confirmations: Option, mined_height: Option, mined_in_block: Option>, + transaction_signature_nonce: Vec, + transaction_signature_key: Vec, } impl CompletedTransactionSql { @@ -1807,6 +1868,8 @@ impl TryFrom for CompletedTransactionSql { confirmations: c.confirmations.map(|ic| ic as i64), mined_height: c.mined_height.map(|ic| ic as i64), mined_in_block: c.mined_in_block, + transaction_signature_nonce: c.transaction_signature.get_public_nonce().to_vec(), + transaction_signature_key: c.transaction_signature.get_signature().to_vec(), }) } } @@ -1827,6 +1890,13 @@ impl TryFrom for CompletedTransaction { type Error = CompletedTransactionConversionError; fn try_from(c: CompletedTransactionSql) -> Result { + let transaction_signature = match PublicKey::from_vec(&c.transaction_signature_nonce) { + Ok(public_nonce) => match PrivateKey::from_vec(&c.transaction_signature_key) { + Ok(signature) => Signature::new(public_nonce, signature), + Err(_) => Signature::default(), + }, + Err(_) => Signature::default(), + }; Ok(Self { tx_id: c.tx_id as u64, source_public_key: PublicKey::from_vec(&c.source_public_key).map_err(TransactionKeyError::Source)?, @@ -1844,6 +1914,7 @@ impl TryFrom for CompletedTransaction { send_count: c.send_count as u32, last_send_timestamp: c.last_send_timestamp, valid: c.valid != 0, + transaction_signature, confirmations: c.confirmations.map(|ic| ic as u64), mined_height: c.mined_height.map(|ic| ic as u64), mined_in_block: c.mined_in_block, @@ -1865,6 +1936,77 @@ pub struct UpdateCompletedTransactionSql { confirmations: Option>, mined_height: Option>, mined_in_block: Option>>, + transaction_signature_nonce: Option>, + transaction_signature_key: Option>, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct UnconfirmedTransactionInfo { + pub tx_id: TxId, + pub signature: Signature, + pub status: TransactionStatus, + pub coinbase_block_height: Option, +} + +impl UnconfirmedTransactionInfo { + pub fn is_coinbase(&self) -> bool { + if let Some(height) = self.coinbase_block_height { + height > 0 + } else { + false + } + } +} + +impl TryFrom for UnconfirmedTransactionInfo { + type Error = TransactionStorageError; + + fn try_from(i: UnconfirmedTransactionInfoSql) -> Result { + Ok(Self { + tx_id: i.tx_id as u64, + signature: Signature::new( + PublicKey::from_vec(&i.transaction_signature_nonce)?, + PrivateKey::from_vec(&i.transaction_signature_key)?, + ), + status: TransactionStatus::try_from(i.status)?, + coinbase_block_height: i.coinbase_block_height.map(|b| b as u64), + }) + } +} + +#[derive(Clone, Queryable)] +pub struct UnconfirmedTransactionInfoSql { + pub tx_id: i64, + pub status: i32, + pub transaction_signature_nonce: Vec, + pub transaction_signature_key: Vec, + pub coinbase_block_height: Option, +} + +impl UnconfirmedTransactionInfoSql { + /// This method returns completed but unconfirmed transactions + pub fn fetch_unconfirmed_transactions_info( + conn: &SqliteConnection, + ) -> Result, TransactionStorageError> { + // TODO: Should we not return cancelled transactions as well and handle it upstream? It could be mined. + let query_result = completed_transactions::table + .select(( + completed_transactions::tx_id, + completed_transactions::status, + completed_transactions::transaction_signature_nonce, + completed_transactions::transaction_signature_key, + completed_transactions::coinbase_block_height, + )) + .filter( + completed_transactions::mined_height + .is_null() + .or(completed_transactions::status.eq(TransactionStatus::MinedUnconfirmed as i32)), + ) + .filter(completed_transactions::cancelled.eq(false as i32)) + .order_by(completed_transactions::tx_id) + .load::(&*conn)?; + Ok(query_result) + } } #[cfg(test)] @@ -1887,7 +2029,7 @@ mod test { use tari_common_types::{ transaction::{TransactionDirection, TransactionStatus}, - types::{HashDigest, PrivateKey, PublicKey}, + types::{HashDigest, PrivateKey, PublicKey, Signature}, }; use tari_core::transactions::{ tari_amount::MicroTari, @@ -2094,6 +2236,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2114,6 +2257,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2243,6 +2387,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2264,6 +2409,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2275,7 +2421,7 @@ mod test { destination_public_key: PublicKey::from_secret_key(&PrivateKey::random(&mut OsRng)), amount, fee: MicroTari::from(100), - transaction: tx, + transaction: tx.clone(), status: TransactionStatus::Coinbase, message: "Hey!".to_string(), timestamp: Utc::now().naive_utc(), @@ -2285,6 +2431,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2396,6 +2543,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: Signature::default(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2478,6 +2626,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: Signature::default(), confirmations: None, mined_height: None, mined_in_block: None, @@ -2562,6 +2711,7 @@ mod test { send_count: 0, last_send_timestamp: None, valid, + transaction_signature: Signature::default(), confirmations: None, mined_height: None, mined_in_block: None, diff --git a/base_layer/wallet/tests/transaction_service/service.rs b/base_layer/wallet/tests/transaction_service/service.rs index d6134ccbc6..32ac55970c 100644 --- a/base_layer/wallet/tests/transaction_service/service.rs +++ b/base_layer/wallet/tests/transaction_service/service.rs @@ -1816,6 +1816,7 @@ fn test_power_mode_updates() { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -1827,7 +1828,7 @@ fn test_power_mode_updates() { destination_public_key: PublicKey::from_secret_key(&PrivateKey::random(&mut OsRng)), amount: 6000 * uT, fee: MicroTari::from(200), - transaction: tx, + transaction: tx.clone(), status: TransactionStatus::Completed, message: "Yo!".to_string(), timestamp: Utc::now().naive_utc(), @@ -1837,6 +1838,7 @@ fn test_power_mode_updates() { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -4980,7 +4982,7 @@ fn broadcast_all_completed_transactions_on_startup() { destination_public_key: PublicKey::from_secret_key(&PrivateKey::random(&mut OsRng)), amount: 5000 * uT, fee: MicroTari::from(20), - transaction: tx, + transaction: tx.clone(), status: TransactionStatus::Completed, message: "Yo!".to_string(), timestamp: Utc::now().naive_utc(), @@ -4990,6 +4992,7 @@ fn broadcast_all_completed_transactions_on_startup() { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -5125,7 +5128,7 @@ fn dont_broadcast_invalid_transactions() { destination_public_key: PublicKey::from_secret_key(&PrivateKey::random(&mut OsRng)), amount: 5000 * uT, fee: MicroTari::from(20), - transaction: tx, + transaction: tx.clone(), status: TransactionStatus::Completed, message: "Yo!".to_string(), timestamp: Utc::now().naive_utc(), @@ -5135,6 +5138,7 @@ fn dont_broadcast_invalid_transactions() { send_count: 0, last_send_timestamp: None, valid: false, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, diff --git a/base_layer/wallet/tests/transaction_service/storage.rs b/base_layer/wallet/tests/transaction_service/storage.rs index 0e9480035e..da77f6bad5 100644 --- a/base_layer/wallet/tests/transaction_service/storage.rs +++ b/base_layer/wallet/tests/transaction_service/storage.rs @@ -28,7 +28,7 @@ use chrono::Utc; use rand::rngs::OsRng; use tari_common_types::{ transaction::{TransactionDirection, TransactionStatus}, - types::{HashDigest, PrivateKey, PublicKey}, + types::{HashDigest, PrivateKey, PublicKey, Signature}, }; use tari_core::transactions::{ tari_amount::{uT, MicroTari}, @@ -266,6 +266,7 @@ pub fn test_db_backend(backend: T) { send_count: 0, last_send_timestamp: None, valid: true, + transaction_signature: tx.first_kernel_excess_sig().unwrap_or(&Signature::default()).clone(), confirmations: None, mined_height: None, mined_in_block: None, @@ -533,7 +534,7 @@ pub fn test_db_backend(backend: T) { panic!("Should have found cancelled outbound tx"); } - let unmined_txs = runtime.block_on(db.fetch_unconfirmed_transactions()).unwrap(); + let unmined_txs = runtime.block_on(db.fetch_unconfirmed_transactions_info()).unwrap(); assert_eq!(unmined_txs.len(), 4); @@ -541,7 +542,7 @@ pub fn test_db_backend(backend: T) { .block_on(db.set_transaction_as_unmined(completed_txs[0].tx_id)) .unwrap(); - let unmined_txs = runtime.block_on(db.fetch_unconfirmed_transactions()).unwrap(); + let unmined_txs = runtime.block_on(db.fetch_unconfirmed_transactions_info()).unwrap(); assert_eq!(unmined_txs.len(), 5); } diff --git a/comms/dht/src/crypt.rs b/comms/dht/src/crypt.rs index c34b96f2b2..d6ea8c2978 100644 --- a/comms/dht/src/crypt.rs +++ b/comms/dht/src/crypt.rs @@ -105,7 +105,7 @@ pub fn create_origin_mac_challenge_parts( body: &[u8], ) -> Challenge { let mut mac_challenge = Challenge::new(); - // TODO: #testnetreset remove conditional + // TODO: #testnet_reset remove conditional if protocol_version.as_major() > 1 { mac_challenge.update(&protocol_version.to_bytes()); mac_challenge.update(destination.to_inner_bytes().as_slice());