chore: remove unnecessary commitment hashing (tari-project#6416)

Description --- Removes unnecessary hashing of commitments used in faucet-related signatures. Motivation and Context --- Signatures used for faucet functionality unnecessarily hash commitments before passing them as an input message. This may have been done on the assumption that the signing functions often refer to the message as a challenge, despite this not being the case. This PR removes the unnecessary hashing operations. It may be worthwhile to refactor the associated signing functions to avoid any possible confusion about the nature of messages and challenges (as suggested in tari-project#6418). How Has This Been Tested? --- Existing tests pass. What process can a PR reviewer use to test or verify this change? --- Check that the commitment data is being passed into the signing functions correctly, and that those functions do in face assume the input is an arbitrary message and not a challenge.
hansieodendaal · Jul 22, 2024 · 1bd2fde · 1bd2fde
1 parent 54bccd7
commit 1bd2fde
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 31 deletions.
diff --git a/applications/minotari_console_wallet/src/automation/commands.rs b/applications/minotari_console_wallet/src/automation/commands.rs
@@ -31,9 +31,8 @@ use std::{
     time::{Duration, Instant},
 };
 
-use blake2::Blake2b;
 use chrono::{DateTime, Utc};
-use digest::{consts::U32, crypto_common::rand_core::OsRng, Digest};
+use digest::{crypto_common::rand_core::OsRng, Digest};
 use futures::FutureExt;
 use log::*;
 use minotari_app_grpc::tls::certs::{generate_self_signed_certs, print_warning, write_cert_to_disk};
@@ -64,9 +63,7 @@ use tari_comms::{
 };
 use tari_comms_dht::{envelope::NodeDestination, DhtDiscoveryRequester};
 use tari_core::{
-    consensus::DomainSeparatedConsensusHasher,
     covenants::Covenant,
-    one_sided::FaucetHashDomain,
     transactions::{
         key_manager::TransactionKeyManagerInterface,
         tari_amount::{uT, MicroMinotari, Minotari},
@@ -816,11 +813,6 @@ pub async fn command_runner(
                 let session_info = read_session_info(args.input_file.clone())?;
 
                 let commitment = Commitment::from_hex(&session_info.commitment_to_spend)?;
-                let commitment_hash: [u8; 32] =
-                    DomainSeparatedConsensusHasher::<FaucetHashDomain, Blake2b<U32>>::new("com_hash")
-                        .chain(&commitment)
-                        .finalize()
-                        .into();
                 let shared_secret = key_manager_service
                     .get_diffie_hellman_shared_secret(
                         &sender_offset_key.key_id,
@@ -833,7 +825,7 @@ pub async fn command_runner(
                 let shared_secret_public_key = PublicKey::from_canonical_bytes(shared_secret.as_bytes())?;
 
                 let script_input_signature = key_manager_service
-                    .sign_script_message(&wallet_spend_key.key_id, &commitment_hash)
+                    .sign_script_message(&wallet_spend_key.key_id, commitment.as_bytes())
                     .await?;
 
                 let out_dir = out_dir(&session_info.session_id)?;

diff --git a/base_layer/core/src/blocks/faucets/mod.rs b/base_layer/core/src/blocks/faucets/mod.rs
@@ -24,8 +24,6 @@ mod test {
 
     use std::{convert::TryFrom, fs::File, io::Write};
 
-    use blake2::Blake2b;
-    use digest::consts::U32;
     use rand::rngs::OsRng;
     use tari_common_types::{
         tari_address::TariAddress,
@@ -34,10 +32,10 @@ mod test {
     use tari_crypto::keys::{PublicKey as PkTrait, SecretKey as SkTrait};
     use tari_key_manager::key_manager_service::KeyManagerInterface;
     use tari_script::{ExecutionStack, Opcode::CheckMultiSigVerifyAggregatePubKey, TariScript};
+    use tari_utilities::ByteArray;
 
     use crate::{
-        consensus::DomainSeparatedConsensusHasher,
-        one_sided::{public_key_to_output_encryption_key, FaucetHashDomain},
+        one_sided::public_key_to_output_encryption_key,
         transactions::{
             key_manager::{
                 create_memory_db_key_manager,
@@ -91,10 +89,8 @@ mod test {
                 .get_commitment(&commitment_mask.key_id, &amount.into())
                 .await
                 .unwrap();
-            let com_hash: [u8; 32] = DomainSeparatedConsensusHasher::<FaucetHashDomain, Blake2b<U32>>::new("com_hash")
-                .chain(&commitment)
-                .finalize()
-                .into();
+            let mut commitment_bytes = [0u8; 32];
+            commitment_bytes.clone_from_slice(commitment.as_bytes());
 
             let sender_offset = key_manager
                 .get_next_key(TransactionKeyManagerBranch::SenderOffset.get_branch_key())
@@ -104,7 +100,7 @@ mod test {
                 signature_threshold,
                 address_len,
                 list_of_spend_keys.clone(),
-                Box::new(com_hash),
+                Box::new(commitment_bytes),
             )]);
             let output = WalletOutputBuilder::new(amount, commitment_mask.key_id)
                 .with_features(OutputFeatures::new(

diff --git a/base_layer/core/src/common/one_sided.rs b/base_layer/core/src/common/one_sided.rs
@@ -44,8 +44,6 @@ hash_domain!(
     1
 );
 
-hash_domain!(FaucetHashDomain, "com.tari.base_layer.wallet.faucet", 0);
-
 type WalletOutputEncryptionKeysDomainHasher = DomainSeparatedHasher<Blake2b<U64>, WalletOutputEncryptionKeysDomain>;
 type WalletOutputSpendingKeysDomainHasher = DomainSeparatedHasher<Blake2b<U64>, WalletOutputSpendingKeysDomain>;
 

diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs
@@ -22,9 +22,7 @@
 
 use std::{collections::HashMap, convert::TryInto, fmt, sync::Arc};
 
-use blake2::Blake2b;
 use diesel::result::{DatabaseErrorKind, Error as DieselError};
-use digest::consts::U32;
 use futures::{pin_mut, StreamExt};
 use log::*;
 use rand::{rngs::OsRng, RngCore};
@@ -36,14 +34,13 @@ use tari_common_types::{
 use tari_comms::types::CommsDHKE;
 use tari_core::{
     borsh::SerializedSize,
-    consensus::{ConsensusConstants, DomainSeparatedConsensusHasher},
+    consensus::ConsensusConstants,
     covenants::Covenant,
     one_sided::{
         public_key_to_output_encryption_key,
         shared_secret_to_output_encryption_key,
         shared_secret_to_output_spending_key,
         stealth_address_script_spending_key,
-        FaucetHashDomain,
     },
     proto::base_node::FetchMatchingUtxos,
     transactions::{
@@ -1237,17 +1234,12 @@ where
             if output.verify_mask(&self.resources.factories.range_proof, &spending_key, amount.as_u64())? {
                 let mut script_signatures = Vec::new();
                 // lets add our own signature to the list
-                let script_challange: [u8; 32] =
-                    DomainSeparatedConsensusHasher::<FaucetHashDomain, Blake2b<U32>>::new("com_hash")
-                        .chain(output.commitment())
-                        .finalize()
-                        .into();
                 let self_signature = self
                     .resources
                     .key_manager
                     .sign_script_message(
                         &self.resources.key_manager.get_spend_key().await?.key_id,
-                        &script_challange,
+                        output.commitment.as_bytes(),
                     )
                     .await?;
                 script_input_shares.insert(
-Original file line number
+Diff line change
@@ Expand Up / @@ -44,8 +44,6 @@ hash_domain!( @@
     );
-    hash_domain!(FaucetHashDomain, "com.tari.base_layer.wallet.faucet", 0);
     type WalletOutputEncryptionKeysDomainHasher = DomainSeparatedHasher<Blake2b<U64>, WalletOutputEncryptionKeysDomain>;
     type WalletOutputSpendingKeysDomainHasher = DomainSeparatedHasher<Blake2b<U64>, WalletOutputSpendingKeysDomain>;
@@ Expand Down @@