From 2b22c1ad8c70350e838db979d008f1369552fbc4 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Thu, 11 Jul 2024 16:20:12 +0200 Subject: [PATCH] fix: script dependance on party order (#6398) Description --- fixes script dependence on party order Motivation and Context --- Currently, the m-of-n ceremony is dependant on the leader selection to succeed or not. This fixes the process so that the leader correctly constructs the input stack for the script in the correct order. How Has This Been Tested? --- manual --- .../src/automation/commands.rs | 33 +++++++++------- .../minotari_console_wallet/src/cli.rs | 2 +- .../src/output_manager_service/handle.rs | 14 +++---- .../src/output_manager_service/service.rs | 39 ++++++++++--------- .../wallet/src/transaction_service/handle.rs | 26 +++++-------- .../wallet/src/transaction_service/service.rs | 8 ++-- base_layer/wallet_ffi/src/lib.rs | 6 +-- 7 files changed, 62 insertions(+), 66 deletions(-) diff --git a/applications/minotari_console_wallet/src/automation/commands.rs b/applications/minotari_console_wallet/src/automation/commands.rs index 24698586c3..242de2c0df 100644 --- a/applications/minotari_console_wallet/src/automation/commands.rs +++ b/applications/minotari_console_wallet/src/automation/commands.rs @@ -21,6 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use std::{ + collections::HashMap, convert::TryInto, fs, fs::File, @@ -85,7 +86,7 @@ use tari_core::{ }; use tari_crypto::ristretto::{pedersen::PedersenCommitment, RistrettoSecretKey}; use tari_key_manager::key_manager_service::KeyManagerInterface; -use tari_script::{script, ExecutionStack, TariScript}; +use tari_script::{script, CheckSigSchnorrSignature, ExecutionStack, TariScript}; use tari_utilities::{hex::Hex, ByteArray}; use tokio::{ sync::{broadcast, mpsc}, @@ -144,13 +145,13 @@ pub async fn burn_tari( /// encumbers a n-of-m transaction #[allow(clippy::too_many_arguments)] +#[allow(clippy::mutable_key_type)] async fn encumber_aggregate_utxo( mut wallet_transaction_service: TransactionServiceHandle, fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -163,7 +164,6 @@ async fn encumber_aggregate_utxo( output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, @@ -751,8 +751,7 @@ pub async fn command_runner( println!( "Party details created with: - 1. script input signature: ({},{}), - 2. wallet public spend key: {}, + 1. script input share: ({},{},{}), 3. wallet public spend key_id: {}, 4. spend nonce key_id: {}, 5. public spend nonce key: {}, @@ -761,9 +760,9 @@ pub async fn command_runner( 8. sender offset nonce key_id: {}, 9. public sender offset nonce key: {}, 10. public shared secret: {}", + wallet_public_spend_key, script_input_signature.get_signature().to_hex(), script_input_signature.get_public_nonce().to_hex(), - wallet_public_spend_key, wallet_spend_key_id, script_nonce_key_id, public_script_nonce, @@ -775,19 +774,23 @@ pub async fn command_runner( ); }, FaucetEncumberAggregateUtxo(args) => { + #[allow(clippy::mutable_key_type)] + let mut input_shares = HashMap::new(); + for share in args.script_input_shares { + let data = share.split(',').collect::>(); + let public_key = PublicKey::from_hex(data[0])?; + let signature = PrivateKey::from_hex(data[1])?; + let public_nonce = PublicKey::from_hex(data[2])?; + let sig = CheckSigSchnorrSignature::new(public_nonce, signature); + input_shares.insert(public_key, sig); + } + match encumber_aggregate_utxo( transaction_service.clone(), args.fee_per_gram, args.output_hash, Commitment::from_hex(&args.commitment)?, - args.script_input_shares - .iter() - .map(|v| v.clone().into()) - .collect::>(), - args.script_public_key_shares - .iter() - .map(|v| v.clone().into()) - .collect::>(), + input_shares, args.script_signature_public_nonces .iter() .map(|v| v.clone().into()) diff --git a/applications/minotari_console_wallet/src/cli.rs b/applications/minotari_console_wallet/src/cli.rs index 2b6fb23abc..3f10360494 100644 --- a/applications/minotari_console_wallet/src/cli.rs +++ b/applications/minotari_console_wallet/src/cli.rs @@ -214,7 +214,7 @@ pub struct FaucetEncumberAggregateUtxoArgs { #[clap(long)] pub output_hash: String, #[clap(long)] - pub script_input_shares: Vec, + pub script_input_shares: Vec, #[clap(long)] pub script_public_key_shares: Vec, #[clap(long)] diff --git a/base_layer/wallet/src/output_manager_service/handle.rs b/base_layer/wallet/src/output_manager_service/handle.rs index c99400061c..cef456bbae 100644 --- a/base_layer/wallet/src/output_manager_service/handle.rs +++ b/base_layer/wallet/src/output_manager_service/handle.rs @@ -20,12 +20,12 @@ // 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. -use std::{fmt, fmt::Formatter, sync::Arc}; +use std::{collections::HashMap, fmt, fmt::Formatter, sync::Arc}; use tari_common_types::{ tari_address::TariAddress, transaction::TxId, - types::{Commitment, FixedHash, HashOutput, PublicKey, Signature}, + types::{Commitment, FixedHash, HashOutput, PublicKey}, }; use tari_core::{ covenants::Covenant, @@ -38,7 +38,7 @@ use tari_core::{ }, }; use tari_crypto::ristretto::pedersen::PedersenCommitment; -use tari_script::TariScript; +use tari_script::{CheckSigSchnorrSignature, TariScript}; use tari_service_framework::reply_channel::SenderService; use tari_utilities::hex::Hex; use tokio::sync::broadcast; @@ -66,8 +66,7 @@ pub enum OutputManagerRequest { fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -759,14 +758,14 @@ impl OutputManagerHandle { } } + #[allow(clippy::mutable_key_type)] pub async fn encumber_aggregate_utxo( &mut self, tx_id: TxId, fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -791,7 +790,6 @@ impl OutputManagerHandle { output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 185a4f2832..aefd16e3e0 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -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. -use std::{convert::TryInto, fmt, sync::Arc}; +use std::{collections::HashMap, convert::TryInto, fmt, sync::Arc}; use blake2::Blake2b; use diesel::result::{DatabaseErrorKind, Error as DieselError}; @@ -32,7 +32,7 @@ use tari_common::configuration::Network; use tari_common_types::{ tari_address::TariAddress, transaction::TxId, - types::{BlockHash, Commitment, FixedHash, HashOutput, PrivateKey, PublicKey, Signature}, + types::{BlockHash, Commitment, FixedHash, HashOutput, PrivateKey, PublicKey}, }; use tari_comms::{types::CommsDHKE, NodeIdentity}; use tari_core::{ @@ -252,7 +252,6 @@ where output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, @@ -265,7 +264,6 @@ where output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, @@ -1176,14 +1174,14 @@ where /// Create a partial transaction in order to prepare output #[allow(clippy::too_many_lines)] + #[allow(clippy::mutable_key_type)] pub async fn encumber_aggregate_utxo( &mut self, tx_id: TxId, fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + mut script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -1233,6 +1231,7 @@ where .iter() .fold(tari_common_types::types::PublicKey::default(), |acc, x| acc + x); let encryption_private_key = public_key_to_output_encryption_key(&sum_public_keys)?; + let mut aggregated_script_public_key_shares = PublicKey::default(); // Decrypt the output secrets and create a new input as WalletOutput (unblinded) let input = if let Ok((amount, spending_key, payment_id)) = EncryptedData::decrypt_data(&encryption_private_key, &output.commitment, &output.encrypted_data) @@ -1250,15 +1249,21 @@ where .key_manager .sign_script_message(&self.resources.wallet_identity.wallet_node_key_id, &script_challange) .await?; - script_signatures.push(StackItem::Signature(CheckSigSchnorrSignature::new( - self_signature.get_public_nonce().clone(), - self_signature.get_signature().clone(), - ))); - for signature in &script_input_shares { - script_signatures.push(StackItem::Signature(CheckSigSchnorrSignature::new( - signature.get_public_nonce().clone(), - signature.get_signature().clone(), - ))); + script_input_shares.insert( + self.resources.wallet_identity.address.public_spend_key().clone(), + self_signature, + ); + + // the order here is important, we need to add the signatures in the same order as public keys where + // added to the script originally + for key in public_keys { + if let Some(signature) = script_input_shares.get(&key) { + script_signatures.push(StackItem::Signature(signature.clone())); + // our own key should not be added yet, it will be added with the script signing + if &key != self.resources.wallet_identity.address.public_spend_key() { + aggregated_script_public_key_shares = aggregated_script_public_key_shares + key; + } + } } let spending_key_id = self.resources.key_manager.import_key(spending_key).await?; WalletOutput::new_with_rangeproof( @@ -1462,9 +1467,7 @@ where let aggregated_script_signature_public_nonces = script_signature_public_nonces .iter() .fold(PublicKey::default(), |acc, x| acc + x); - let aggregated_script_public_key_shares = script_public_key_shares - .iter() - .fold(PublicKey::default(), |acc, x| acc + x); + // Update the input's script signature let (updated_input, total_script_public_key) = input .to_transaction_input_with_multi_party_script_signature( diff --git a/base_layer/wallet/src/transaction_service/handle.rs b/base_layer/wallet/src/transaction_service/handle.rs index b865c4feae..841c2fdfc7 100644 --- a/base_layer/wallet/src/transaction_service/handle.rs +++ b/base_layer/wallet/src/transaction_service/handle.rs @@ -53,6 +53,7 @@ use tari_core::{ }, }; use tari_crypto::ristretto::pedersen::PedersenCommitment; +use tari_script::CheckSigSchnorrSignature; use tari_service_framework::reply_channel::SenderService; use tari_utilities::hex::Hex; use tokio::sync::broadcast; @@ -114,8 +115,7 @@ pub enum TransactionServiceRequest { fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -233,7 +233,6 @@ impl fmt::Display for TransactionServiceRequest { output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, @@ -242,24 +241,20 @@ impl fmt::Display for TransactionServiceRequest { .. } => f.write_str(&format!( "Creating encumber n-of-m utxo with: fee_per_gram = {}, output_hash = {}, commitment = {}, \ - script_input_shares = {:?}, script_public_key_shares = {:?}, script_signature_shares = {:?}, \ - sender_offset_public_key_shares = {:?}, metadata_ephemeral_public_key_shares = {:?}, \ - dh_shared_secret_shares = {:?}, recipient_address = {}", + script_input_shares = {:?},, script_signature_shares = {:?}, sender_offset_public_key_shares = {:?}, \ + metadata_ephemeral_public_key_shares = {:?}, dh_shared_secret_shares = {:?}, recipient_address = {}", fee_per_gram, output_hash, expected_commitment.to_hex(), script_input_shares .iter() .map(|v| format!( - "(sig: {}, nonce: {})", - v.get_signature().to_hex(), - v.get_public_nonce().to_hex() + "(public_key: {}, sig: {}, nonce: {})", + v.0.to_hex(), + v.1.get_signature().to_hex(), + v.1.get_public_nonce().to_hex() )) .collect::>(), - script_public_key_shares - .iter() - .map(|v| v.to_hex()) - .collect::>(), script_signature_public_nonces .iter() .map(|v| format!("(public nonce: {})", v.to_hex(),)) @@ -731,13 +726,13 @@ impl TransactionServiceHandle { } } + #[allow(clippy::mutable_key_type)] pub async fn encumber_aggregate_utxo( &mut self, fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -751,7 +746,6 @@ impl TransactionServiceHandle { output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, diff --git a/base_layer/wallet/src/transaction_service/service.rs b/base_layer/wallet/src/transaction_service/service.rs index 7ee191d95d..e13e0c5be3 100644 --- a/base_layer/wallet/src/transaction_service/service.rs +++ b/base_layer/wallet/src/transaction_service/service.rs @@ -88,6 +88,7 @@ use tari_script::{ push_pubkey_script, script, slice_to_boxed_message, + CheckSigSchnorrSignature, ExecutionStack, ScriptContext, TariScript, @@ -723,7 +724,6 @@ where output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, @@ -735,7 +735,6 @@ where output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, @@ -1377,13 +1376,13 @@ where } /// Creates an encumbered uninitialized transaction + #[allow(clippy::mutable_key_type)] pub async fn encumber_aggregate_tx( &mut self, fee_per_gram: MicroMinotari, output_hash: String, expected_commitment: PedersenCommitment, - script_input_shares: Vec, - script_public_key_shares: Vec, + script_input_shares: HashMap, script_signature_public_nonces: Vec, sender_offset_public_key_shares: Vec, metadata_ephemeral_public_key_shares: Vec, @@ -1401,7 +1400,6 @@ where output_hash, expected_commitment, script_input_shares, - script_public_key_shares, script_signature_public_nonces, sender_offset_public_key_shares, metadata_ephemeral_public_key_shares, diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index 89a1578759..fe7fef795a 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -9522,19 +9522,19 @@ mod test { #[test] fn test_emoji_convert() { unsafe { - let byte = 0 as u8; + let byte = 0u8; let emoji_ptr = byte_to_emoji(byte); let emoji = CStr::from_ptr(emoji_ptr); assert_eq!(emoji.to_str().unwrap(), EMOJI[0].to_string()); - let byte = 50 as u8; + let byte = 50u8; let emoji_ptr = byte_to_emoji(byte); let emoji = CStr::from_ptr(emoji_ptr); assert_eq!(emoji.to_str().unwrap(), EMOJI[50].to_string()); - let byte = 125 as u8; + let byte = 125u8; let emoji_ptr = byte_to_emoji(byte); let emoji = CStr::from_ptr(emoji_ptr);