From ca897079678163df37682aedc9c8b329e5e0dfa9 Mon Sep 17 00:00:00 2001 From: Hansie Odendaal Date: Tue, 22 Jun 2021 11:38:18 +0200 Subject: [PATCH] Fix small issues related to #3020 - As per reviewer comments Co-Authored-By: SW van Heerden --- .../core/src/transactions/aggregated_body.rs | 2 +- .../core/src/transactions/transaction.rs | 17 +++---- .../transaction_protocol/sender.rs | 2 + .../transaction_protocol/single_receiver.rs | 2 +- .../down.sql | 0 .../up.sql | 3 +- .../storage/sqlite_db.rs | 44 ++++++++++++++----- base_layer/wallet/src/schema.rs | 3 +- base_layer/wallet/src/storage/mod.rs | 9 ++-- base_layer/wallet/src/wallet.rs | 4 +- 10 files changed, 58 insertions(+), 28 deletions(-) rename base_layer/wallet/migrations/{2021-06-15-162255_sender_meta_signature => 2021-06-22-143855_sender_meta_signature}/down.sql (100%) rename base_layer/wallet/migrations/{2021-06-15-162255_sender_meta_signature => 2021-06-22-143855_sender_meta_signature}/up.sql (88%) diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index 61007cbadf..b60901667b 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -416,7 +416,7 @@ impl AggregateBody { fn validate_sender_signatures(&self) -> Result<(), TransactionError> { trace!(target: LOG_TARGET, "Checking sender signatures"); for o in &self.outputs { - let _ = o.verify_sender_signature()?; + o.verify_sender_signature()?; } Ok(()) } diff --git a/base_layer/core/src/transactions/transaction.rs b/base_layer/core/src/transactions/transaction.rs index 003017531e..205ca799d1 100644 --- a/base_layer/core/src/transactions/transaction.rs +++ b/base_layer/core/src/transactions/transaction.rs @@ -201,6 +201,7 @@ pub enum TransactionError { /// An unblinded output is one where the value and spending key (blinding factor) are known. This can be used to /// build both inputs and outputs (every input comes from an output) +// TODO: Try to get rid of 'Serialize' and 'Deserialize' traits here; see related comment at 'struct RawTransactionInfo' #[derive(Debug, Clone, Serialize, Deserialize)] pub struct UnblindedOutput { pub value: MicroTari, @@ -374,7 +375,7 @@ pub struct TransactionInput { pub height: u64, /// A signature with k_s, signing the script, input data, and mined height pub script_signature: Signature, - /// The offset pubkey, K_O + /// The offset public key, K_O pub script_offset_public_key: PublicKey, } @@ -412,7 +413,7 @@ impl TransactionInput { } /// This will check if the input and the output is the same commitment by looking at the commitment and features. - /// This will ignore the output rangeproof + /// This will ignore the output range proof pub fn is_equal_to(&self, output: &TransactionOutput) -> bool { self.commitment == output.commitment && self.features == output.features } @@ -601,7 +602,7 @@ impl TransactionOutput { } /// This will check if the input and the output is the same commitment by looking at the commitment and features. - /// This will ignore the output rangeproof + /// This will ignore the output range proof #[inline] pub fn is_equal_to(&self, output: &TransactionInput) -> bool { self.commitment == output.commitment && self.features == output.features @@ -617,18 +618,18 @@ impl TransactionOutput { script: &TariScript, features: &OutputFeatures, script_offset_public_key: &PublicKey, - puplic_nonce: &PublicKey, + public_nonce: &PublicKey, ) -> MessageHash { Challenge::new() .chain(script.as_bytes()) .chain(features.to_bytes()) .chain(script_offset_public_key.as_bytes()) - .chain(puplic_nonce.as_bytes()) + .chain(public_nonce.as_bytes()) .result() .to_vec() } - /// Create sender signature fore the output meta data + /// Create sender signature for the output meta data pub fn create_sender_signature( script: &TariScript, output_features: &OutputFeatures, @@ -1011,7 +1012,7 @@ impl Transaction { .fold(0, |max_maturity, input| max(max_maturity, input.features.maturity)) } - /// Returns the maximum timelock of the kernels inside of the transaction + /// Returns the maximum time lock of the kernels inside of the transaction pub fn max_kernel_timelock(&self) -> u64 { self.body .kernels() @@ -1331,7 +1332,7 @@ mod test { let mut kernel = create_test_kernel(0.into(), 0); let mut tx = Transaction::new(Vec::new(), Vec::new(), Vec::new(), 0.into(), 0.into()); - // lets add timelocks + // lets add time locks input.features.maturity = 5; kernel.lock_height = 2; tx.body.add_input(input.clone()); diff --git a/base_layer/core/src/transactions/transaction_protocol/sender.rs b/base_layer/core/src/transactions/transaction_protocol/sender.rs index ef57f18af7..431164b963 100644 --- a/base_layer/core/src/transactions/transaction_protocol/sender.rs +++ b/base_layer/core/src/transactions/transaction_protocol/sender.rs @@ -58,6 +58,8 @@ use tari_crypto::{ /// This struct contains all the information that a transaction initiator (the sender) will manage throughout the /// Transaction construction process. +// TODO: Investigate necessity to use the 'Serialize' and 'Deserialize' traits here; this could potentially leak +// TODO: information when least expected. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub(super) struct RawTransactionInfo { pub num_recipients: usize, diff --git a/base_layer/core/src/transactions/transaction_protocol/single_receiver.rs b/base_layer/core/src/transactions/transaction_protocol/single_receiver.rs index 3ba24e9c9c..b1b3825b5e 100644 --- a/base_layer/core/src/transactions/transaction_protocol/single_receiver.rs +++ b/base_layer/core/src/transactions/transaction_protocol/single_receiver.rs @@ -117,7 +117,7 @@ impl SingleReceiverTransactionProtocol { sender_info.script_offset_public_key.clone(), sender_info.sender_metadata_signature.clone(), ); - let _ = output.verify_sender_signature()?; + output.verify_sender_signature()?; Ok(output) } } diff --git a/base_layer/wallet/migrations/2021-06-15-162255_sender_meta_signature/down.sql b/base_layer/wallet/migrations/2021-06-22-143855_sender_meta_signature/down.sql similarity index 100% rename from base_layer/wallet/migrations/2021-06-15-162255_sender_meta_signature/down.sql rename to base_layer/wallet/migrations/2021-06-22-143855_sender_meta_signature/down.sql diff --git a/base_layer/wallet/migrations/2021-06-15-162255_sender_meta_signature/up.sql b/base_layer/wallet/migrations/2021-06-22-143855_sender_meta_signature/up.sql similarity index 88% rename from base_layer/wallet/migrations/2021-06-15-162255_sender_meta_signature/up.sql rename to base_layer/wallet/migrations/2021-06-22-143855_sender_meta_signature/up.sql index 1efa598904..846a2e4fa4 100644 --- a/base_layer/wallet/migrations/2021-06-15-162255_sender_meta_signature/up.sql +++ b/base_layer/wallet/migrations/2021-06-22-143855_sender_meta_signature/up.sql @@ -18,7 +18,8 @@ CREATE TABLE outputs ( height INTEGER NOT NULL, script_private_key BLOB NOT NULL, script_offset_public_key BLOB NOT NULL, - sender_metadata_signature TEXT NOT NULL, + sender_metadata_signature_key BLOB NOT NULL, + sender_metadata_signature_nonce BLOB NOT NULL, CONSTRAINT unique_commitment UNIQUE (commitment) ); PRAGMA foreign_keys=on; diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db.rs index c2ac61d110..746d04070f 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db.rs @@ -57,7 +57,7 @@ use tari_core::{ transactions::{ tari_amount::MicroTari, transaction::{OutputFeatures, OutputFlags, UnblindedOutput}, - types::{Commitment, CryptoFactories, PrivateKey, PublicKey}, + types::{Commitment, CryptoFactories, PrivateKey, PublicKey, Signature}, }, }; use tari_crypto::{ @@ -883,7 +883,8 @@ struct NewOutputSql { height: i64, script_private_key: Vec, script_offset_public_key: Vec, - sender_metadata_signature: String, + sender_metadata_signature_key: Vec, + sender_metadata_signature_nonce: Vec, } impl NewOutputSql { @@ -892,8 +893,6 @@ impl NewOutputSql { status: OutputStatus, tx_id: Option, ) -> Result { - let sender_metadata_signature = serde_json::to_string(&output.unblinded_output.sender_metadata_signature) - .map_err(|e| OutputManagerStorageError::UnexpectedResult(e.to_string()))?; Ok(Self { commitment: Some(output.commitment.to_vec()), spending_key: output.unblinded_output.spending_key.to_vec(), @@ -908,7 +907,16 @@ impl NewOutputSql { height: output.unblinded_output.height as i64, script_private_key: output.unblinded_output.script_private_key.to_vec(), script_offset_public_key: output.unblinded_output.script_offset_public_key.to_vec(), - sender_metadata_signature, + sender_metadata_signature_key: output + .unblinded_output + .sender_metadata_signature + .get_signature() + .to_vec(), + sender_metadata_signature_nonce: output + .unblinded_output + .sender_metadata_signature + .get_public_nonce() + .to_vec(), }) } @@ -950,7 +958,8 @@ struct OutputSql { height: i64, script_private_key: Vec, script_offset_public_key: Vec, - sender_metadata_signature: String, + sender_metadata_signature_key: Vec, + sender_metadata_signature_nonce: Vec, } impl OutputSql { @@ -1133,12 +1142,26 @@ impl TryFrom for DbUnblindedOutput { PublicKey::from_vec(&o.script_offset_public_key).map_err(|_| { error!( target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might be encrypted" + "Could not create PublicKey from stored bytes, They might be encrypted" ); OutputManagerStorageError::ConversionError })?, - serde_json::from_str(&o.sender_metadata_signature) - .map_err(|e| OutputManagerStorageError::UnexpectedResult(e.to_string()))?, + Signature::new( + PublicKey::from_vec(&o.sender_metadata_signature_nonce).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PublicKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError + })?, + PrivateKey::from_vec(&o.sender_metadata_signature_key).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PrivateKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError + })?, + ), ); let hash = match o.hash { @@ -1196,7 +1219,8 @@ impl From for NewOutputSql { height: o.height, script_private_key: o.script_private_key, script_offset_public_key: o.script_offset_public_key, - sender_metadata_signature: o.sender_metadata_signature, + sender_metadata_signature_key: o.sender_metadata_signature_key, + sender_metadata_signature_nonce: o.sender_metadata_signature_nonce, } } } diff --git a/base_layer/wallet/src/schema.rs b/base_layer/wallet/src/schema.rs index 2f4b01644f..22bb6696d9 100644 --- a/base_layer/wallet/src/schema.rs +++ b/base_layer/wallet/src/schema.rs @@ -100,7 +100,8 @@ table! { height -> BigInt, script_private_key -> Binary, script_offset_public_key -> Binary, - sender_metadata_signature -> Text, + sender_metadata_signature_key -> Binary, + sender_metadata_signature_nonce -> Binary, } } diff --git a/base_layer/wallet/src/storage/mod.rs b/base_layer/wallet/src/storage/mod.rs index 6c4a16887e..b4dbde2225 100644 --- a/base_layer/wallet/src/storage/mod.rs +++ b/base_layer/wallet/src/storage/mod.rs @@ -22,10 +22,11 @@ // Note: For help in getting started with diesel as well as how to update the tables look here: // http://diesel.rs/guides/getting-started/ -// - You also need to ensure that you installed diesel with the sqlite feature flag cargo install diesel_cli -// --no-default-features --features sqlite -// - If you updated the tables the following needs to be run from the base_layer/wallet/ folder: diesel setup -// --database-url test.sqlite3 diesel migration run --database-url test.sqlite3 +// - You also need to ensure that you installed diesel with the sqlite feature flag: +// - 'cargo install diesel_cli --no-default-features --features sqlite' +// - If you updated the tables the following needs to be run from the base_layer/wallet/ folder: +// - 'diesel setup --database-url test.sqlite3' +// - 'diesel migration run --database-url test.sqlite3' // - After running this, make sure that the diesel update did not change BigInt to Integer in 'schema.rs' (check for // any unwanted changes) diff --git a/base_layer/wallet/src/wallet.rs b/base_layer/wallet/src/wallet.rs index 6d5410e69f..c937af1eb4 100644 --- a/base_layer/wallet/src/wallet.rs +++ b/base_layer/wallet/src/wallet.rs @@ -309,7 +309,7 @@ where source_public_key: &CommsPublicKey, features: OutputFeatures, message: String, - sender_signature: Signature, + sender_metadata_signature: Signature, script_private_key: &PrivateKey, script_offset_public_key: &PublicKey, ) -> Result { @@ -322,7 +322,7 @@ where 0, script_private_key.clone(), script_offset_public_key.clone(), - sender_signature, + sender_metadata_signature, ); let tx_id = self