diff --git a/.changelog/unreleased/SDK/4994-pubkeys-for-mock-sigs.md b/.changelog/unreleased/SDK/4994-pubkeys-for-mock-sigs.md new file mode 100644 index 00000000000..2dc4e2eb01a --- /dev/null +++ b/.changelog/unreleased/SDK/4994-pubkeys-for-mock-sigs.md @@ -0,0 +1,3 @@ +- Changed the SDK signing functions to accept a public key instead of a + private one when mocking a signature. ([\#4994](https://github.com/namada- + net/namada/pull/4994)) \ No newline at end of file diff --git a/crates/account/src/auth.rs b/crates/account/src/auth.rs index f981df7826c..ed51180404e 100644 --- a/crates/account/src/auth.rs +++ b/crates/account/src/auth.rs @@ -6,6 +6,7 @@ use borsh::{BorshDeserialize, BorshSerialize}; use namada_core::collections::HashMap; use namada_core::hints; use namada_core::key::common; +use namada_core::key::common::SigOrPubKey; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] use namada_migrations::*; @@ -77,16 +78,15 @@ impl AccountPublicKeysMap { self.pk_to_idx.get(public_key).cloned() } - /// Index the given set of secret keys - pub fn index_secret_keys( - &self, - secret_keys: Vec, - ) -> BTreeMap { - secret_keys - .into_iter() - .filter_map(|secret_key: common::SecretKey| { - self.get_index_from_public_key(&secret_key.to_public()) - .map(|index| (index, secret_key)) + /// Index the given set of keys + pub fn index_keys(&self, keys: Vec) -> BTreeMap + where + KEY: SigOrPubKey, + { + keys.into_iter() + .filter_map(|key| { + self.get_index_from_public_key(&key.pubkey()) + .map(|index| (index, key)) }) .collect() } diff --git a/crates/benches/host_env.rs b/crates/benches/host_env.rs index a8c76385a79..fa64d364c54 100644 --- a/crates/benches/host_env.rs +++ b/crates/benches/host_env.rs @@ -41,7 +41,7 @@ fn tx_section_signature_validation(c: &mut Criterion) { let multisig = Authorization::new( vec![section_hash], - pkim.index_secret_keys(vec![defaults::albert_keypair()]), + pkim.index_keys(vec![defaults::albert_keypair()]), None, ); diff --git a/crates/core/src/key/common.rs b/crates/core/src/key/common.rs index 020930bb429..d50c87559a7 100644 --- a/crates/core/src/key/common.rs +++ b/crates/core/src/key/common.rs @@ -517,18 +517,68 @@ impl super::SigScheme for SigScheme { } } - fn mock(keypair: &SecretKey) -> Self::Signature { - match keypair { - SecretKey::Ed25519(kp) => { - Signature::Ed25519(ed25519::SigScheme::mock(kp)) + fn mock(pubkey: &Self::PublicKey) -> Self::Signature { + match pubkey { + PublicKey::Ed25519(pk) => { + Signature::Ed25519(ed25519::SigScheme::mock(pk)) } - SecretKey::Secp256k1(kp) => { - Signature::Secp256k1(secp256k1::SigScheme::mock(kp)) + PublicKey::Secp256k1(pk) => { + Signature::Secp256k1(secp256k1::SigScheme::mock(pk)) } } } } +/// Share behavior for both private and public keys. Useful to dispatch function +/// calls when mocking signatures +pub trait SigOrPubKey { + /// Get the public key + fn pubkey(&self) -> PublicKey; + + /// Get the private key if possible ([`None`] if the underlying type is a + /// public key) + fn sigkey(&self) -> Option; + + /// Produce a signature (either a valid or a mock one) + fn sign( + &self, + data: impl SignableBytes, + ) -> ::Signature; +} + +impl SigOrPubKey for SecretKey { + fn pubkey(&self) -> PublicKey { + self.ref_to() + } + + fn sigkey(&self) -> Option { + Some(self.to_owned()) + } + + fn sign( + &self, + data: impl SignableBytes, + ) -> ::Signature { + SigScheme::sign(self, data) + } +} +impl SigOrPubKey for PublicKey { + fn pubkey(&self) -> PublicKey { + self.to_owned() + } + + fn sigkey(&self) -> Option { + None + } + + fn sign( + &self, + _: impl SignableBytes, + ) -> ::Signature { + SigScheme::mock(self) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/core/src/key/ed25519.rs b/crates/core/src/key/ed25519.rs index 0eefd596a4b..a7579456a5e 100644 --- a/crates/core/src/key/ed25519.rs +++ b/crates/core/src/key/ed25519.rs @@ -403,7 +403,7 @@ impl super::SigScheme for SigScheme { } } - fn mock(_: &Self::SecretKey) -> Self::Signature { + fn mock(_: &Self::PublicKey) -> Self::Signature { Signature(ed25519_consensus::Signature::from([0; 64])) } } diff --git a/crates/core/src/key/mod.rs b/crates/core/src/key/mod.rs index 447cc6a68fd..7d960920dae 100644 --- a/crates/core/src/key/mod.rs +++ b/crates/core/src/key/mod.rs @@ -270,7 +270,7 @@ pub trait SigScheme: Eq + Ord + Debug + Serialize + Default { } /// Provide a dummy signature - fn mock(keypair: &Self::SecretKey) -> Self::Signature; + fn mock(pubkey: &Self::PublicKey) -> Self::Signature; } /// Public key hash derived from `common::Key` borsh encoded bytes (hex string diff --git a/crates/core/src/key/secp256k1.rs b/crates/core/src/key/secp256k1.rs index d02f455af35..1aec054fcbb 100644 --- a/crates/core/src/key/secp256k1.rs +++ b/crates/core/src/key/secp256k1.rs @@ -603,7 +603,7 @@ impl super::SigScheme for SigScheme { } } - fn mock(_: &Self::SecretKey) -> Self::Signature { + fn mock(_: &Self::PublicKey) -> Self::Signature { Signature( k256::ecdsa::Signature::from_scalars( k256::Scalar::ONE, diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index f8ca8722fb4..28fd118c9b2 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -17,7 +17,6 @@ use namada_core::address::{Address, ImplicitAddress, MASP}; use namada_core::arith::checked; use namada_core::collections::{HashMap, HashSet}; use namada_core::ibc::primitives::IntoHostTime; -use namada_core::key::common::{CommonPublicKey, CommonSignature}; use namada_core::key::*; use namada_core::masp::{AssetData, MaspTxId, PaymentAddress}; use namada_core::tendermint::Time as TmTime; @@ -38,9 +37,7 @@ use namada_token::storage_key::balance_key; use namada_tx::data::pgf::UpdateStewardCommission; use namada_tx::data::pos; use namada_tx::data::pos::BecomeValidator; -use namada_tx::{ - Authorization, MaspBuilder, Section, SignatureIndex, Signer, Tx, -}; +use namada_tx::{Authorization, MaspBuilder, Section, SignatureIndex, Tx}; use rand::rngs::OsRng; use serde::{Deserialize, Serialize}; use tokio::sync::RwLock; @@ -277,42 +274,6 @@ pub async fn default_sign( ))) } -/// When mocking signatures, if a key is in a hardware wallet, -/// use this function instead to mock the signature -fn mock_hw_sig(tx: &mut Tx, pk: common::PublicKey, signable: Signable) { - let targets = match signable { - Signable::FeeRawHeader => { - vec![tx.sechashes(), vec![tx.raw_header_hash()]] - } - Signable::RawHeader => vec![vec![tx.raw_header_hash()]], - }; - tx.protocol_filter(); - let sig = match pk { - CommonPublicKey::Ed25519(_) => { - // this key has no effect other than to satisfy api constraints - let kp = ed25519::SigScheme::from_bytes([0; 32]); - CommonSignature::Ed25519(ed25519::SigScheme::mock(&kp)) - } - CommonPublicKey::Secp256k1(_) => { - // this key has no effect other than to satisfy api constraints - const SECRET_KEY_HEX: &str = "c2c72dfbff11dfb4e9d5b0a20c620c58b15bb7552753601f043db91331b0db15"; - let sk_bytes = HEXLOWER.decode(SECRET_KEY_HEX.as_bytes()).unwrap(); - let kp = - secp256k1::SecretKey::try_from_slice(&sk_bytes[..]).unwrap(); - CommonSignature::Secp256k1(secp256k1::SigScheme::mock(&kp)) - } - }; - for t in targets { - let mut signatures = BTreeMap::new(); - signatures.insert(0, sig.clone()); - tx.add_section(Section::Authorization(Authorization { - targets: t, - signer: Signer::PubKeys(vec![pk.clone()]), - signatures, - })); - } -} - /// Sign a transaction with a given signing key or public key of a given signer. /// If no explicit signer given, use the `default`. If no `default` is given, /// Error. @@ -361,40 +322,55 @@ where signing_tx_data.account_public_keys_map { let mut wallet = wallet.write().await; - let mut signing_tx_keypairs = vec![]; - - for public_key in &signing_tx_data.public_keys { - if !used_pubkeys.contains(public_key) { - let Ok(secret_key) = - find_key_by_pk(&mut wallet, args, public_key) - else { - // If the secret key is not found, continue because the - // hardware wallet may still be able to sign this - continue; - }; - used_pubkeys.insert(public_key.clone()); - signing_tx_keypairs.push(secret_key); + + if args.dry_run.is_some() { + // For dry-runs produce mock signatures + let mut signing_tx_pubkeys = vec![]; + + for public_key in &signing_tx_data.public_keys { + if !used_pubkeys.contains(public_key) { + used_pubkeys.insert(public_key.clone()); + signing_tx_pubkeys.push(public_key.to_owned()); + } } - } - if !signing_tx_keypairs.is_empty() { - if args.dry_run.is_some() { + if !signing_tx_pubkeys.is_empty() { tx.mock( - signing_tx_keypairs, + signing_tx_pubkeys, account_public_keys_map, signing_tx_data.owner, - ) - } else { + ); + } + } else { + let mut signing_tx_keypairs = vec![]; + + for public_key in &signing_tx_data.public_keys { + if !used_pubkeys.contains(public_key) { + let Ok(secret_key) = + find_key_by_pk(&mut wallet, args, public_key) + else { + // If the secret key is not found, continue because + // the hardware wallet + // may still be able to sign this + continue; + }; + used_pubkeys.insert(public_key.clone()); + signing_tx_keypairs.push(secret_key); + } + } + + if !signing_tx_keypairs.is_empty() { tx.sign_raw( signing_tx_keypairs, account_public_keys_map, signing_tx_data.owner, - ) - }; + ); + } } } - // Then try to sign the raw header using the hardware wallet + // Then try to sign the raw header using the hardware wallet (only if + // this is not a dry-run requiring mock signatures) for pubkey in &signing_tx_data.public_keys { if !used_pubkeys.contains(pubkey) { match &fee_auth { @@ -407,14 +383,10 @@ where used_pubkeys.insert(pubkey.clone()); } _ => { - if args.dry_run.is_some() { - mock_hw_sig( - tx, - pubkey.clone(), - Signable::RawHeader, - ); - used_pubkeys.insert(pubkey.clone()); - } else if let Ok(ntx) = sign( + // No need to account for mock signatures when dealing + // with the hardware wallet as those are produced fully + // in software here above + if let Ok(ntx) = sign( tx.clone(), pubkey.clone(), Signable::RawHeader, @@ -456,25 +428,22 @@ where pubkey, disposable_fee_payer: _, }) => { - let key = { - // Lock the wallet just long enough to extract a key from it - // without interfering with the sign closure - // call - let mut wallet = wallet.write().await; - find_key_by_pk(&mut *wallet, args, pubkey) - }; - match key { - Ok(fee_payer_keypair) => { - if args.dry_run.is_some() { - tx.mock_sign_wrapper(fee_payer_keypair); - } else { + if args.dry_run.is_some() { + tx.mock_sign_wrapper(pubkey.to_owned()); + } else { + let key = { + // Lock the wallet just long enough to extract a key from it + // without interfering with the sign closure + // call + let mut wallet = wallet.write().await; + find_key_by_pk(&mut *wallet, args, pubkey) + }; + + match key { + Ok(fee_payer_keypair) => { tx.sign_wrapper(fee_payer_keypair); } - } - Err(_) => { - if args.dry_run.is_some() { - mock_hw_sig(tx, pubkey.clone(), Signable::FeeRawHeader); - } else { + Err(_) => { *tx = sign( tx.clone(), pubkey.clone(), diff --git a/crates/tx/src/section.rs b/crates/tx/src/section.rs index 19728e58d65..d9a4598145b 100644 --- a/crates/tx/src/section.rs +++ b/crates/tx/src/section.rs @@ -6,6 +6,7 @@ use masp_primitives::transaction::builder::Builder; use masp_primitives::transaction::components::sapling::builder::SaplingMetadata; use masp_primitives::zip32::ExtendedFullViewingKey; use namada_account::AccountPublicKeysMap; +use namada_account::common::SigOrPubKey; use namada_core::address::Address; use namada_core::borsh::{ self, BorshDeserialize, BorshSchema, BorshSerialize, BorshSerializeExt, @@ -510,51 +511,37 @@ impl Authorization { secret_keys: BTreeMap, signer: Option
, ) -> Self { - Self::create_signatures( - targets, - secret_keys, - signer, - common::SigScheme::sign, - ) + Self::create_signatures(targets, secret_keys, signer) } /// Place mock signatures over the given section hash with /// the given key and return a section pub fn mock( targets: Vec, - secret_keys: BTreeMap, + pubkeys: BTreeMap, signer: Option
, ) -> Self { - Self::create_signatures(targets, secret_keys, signer, |kp, _| { - common::SigScheme::mock(kp) - }) + Self::create_signatures(targets, pubkeys, signer) } - fn create_signatures( + pub(crate) fn create_signatures( targets: Vec, - secret_keys: BTreeMap, + keys: BTreeMap, signer: Option
, - create_sig: F, - ) -> Self - where - F: Fn(&common::SecretKey, namada_core::hash::Hash) -> common::Signature, - { + ) -> Self { // If no signer address is given, then derive the signer's public keys - // from the given secret keys. + // from the given keys. let signer = if let Some(addr) = signer { Signer::Address(addr) } else { // Make sure the corresponding public keys can be represented by a // vector instead of a map assert!( - secret_keys - .keys() - .cloned() - .eq(0..(u8::try_from(secret_keys.len()) - .expect("Number of SKs must not exceed `u8::MAX`"))), - "secret keys must be enumerated when signer address is absent" + keys.keys().cloned().eq(0..(u8::try_from(keys.len()) + .expect("Number of keys must not exceed `u8::MAX`"))), + "keys must be enumerated when signer address is absent" ); - Signer::PubKeys(secret_keys.values().map(RefTo::ref_to).collect()) + Signer::PubKeys(keys.values().map(|key| key.pubkey()).collect()) }; // Commit to the given targets @@ -564,11 +551,11 @@ impl Authorization { signatures: BTreeMap::new(), }; let target = partial.get_raw_hash(); - // Turn the map of secret keys into a map of signatures over the - // commitment made above - let signatures = secret_keys + // Turn the map of keys into a map of signatures over the commitment + // made above + let signatures = keys .iter() - .map(|(index, secret_key)| (*index, create_sig(secret_key, target))) + .map(|(idx, key)| (*idx, key.sign(target))) .collect(); Self { signatures, diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 7a3539642f5..d90d5df1552 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -8,6 +8,7 @@ use std::str::FromStr; use masp_primitives::transaction::Transaction; use namada_account::AccountPublicKeysMap; +use namada_account::common::SigOrPubKey; use namada_core::address::Address; use namada_core::borsh::{ self, BorshDeserialize, BorshSchema, BorshSerialize, BorshSerializeExt, @@ -647,7 +648,7 @@ impl Tx { let mut signatures = Vec::new(); let section = Authorization::new( targets, - public_keys_index_map.index_secret_keys(secret_keys.to_vec()), + public_keys_index_map.index_keys(secret_keys.to_vec()), signer, ); match section.signer { @@ -830,35 +831,32 @@ impl Tx { /// Add fee payer keypair to the tx builder pub fn sign_wrapper(&mut self, keypair: common::SecretKey) -> &mut Self { - self.create_wrapper_sig(keypair, Authorization::new) + self.create_wrapper_sig(keypair) } /// Mock adding fee payer keypair to the tx builder pub fn mock_sign_wrapper( &mut self, - keypair: common::SecretKey, + pubkey: common::PublicKey, ) -> &mut Self { - self.create_wrapper_sig(keypair, Authorization::mock) + self.create_wrapper_sig(pubkey) } - fn create_wrapper_sig( - &mut self, - keypair: common::SecretKey, - create_sig: F, - ) -> &mut Self - where - F: Fn( - Vec, - BTreeMap, - Option
, - ) -> Authorization, - { + fn create_wrapper_sig(&mut self, keypair: impl SigOrPubKey) -> &mut Self { self.protocol_filter(); - self.add_section(Section::Authorization(create_sig( - self.sechashes(), - [(0, keypair)].into_iter().collect(), - None, - ))); + let auth = match keypair.sigkey() { + Some(secret_key) => Authorization::new( + self.sechashes(), + [(0, secret_key)].into_iter().collect(), + None, + ), + None => Authorization::mock( + self.sechashes(), + [(0, keypair.pubkey())].into_iter().collect(), + None, + ), + }; + self.add_section(Section::Authorization(auth)); self } @@ -869,59 +867,42 @@ impl Tx { account_public_keys_map: AccountPublicKeysMap, signer: Option
, ) -> &mut Self { - self.create_sig_raw( - keypairs, - account_public_keys_map, - signer, - Authorization::new, - ) + self.create_sig_raw(keypairs, account_public_keys_map, signer) } /// Add signing keys to the tx builder with mock /// signatures pub fn mock( &mut self, - keypairs: Vec, + pubkeys: Vec, account_public_keys_map: AccountPublicKeysMap, signer: Option
, ) -> &mut Self { - self.create_sig_raw( - keypairs, - account_public_keys_map, - signer, - Authorization::mock, - ) + self.create_sig_raw(pubkeys, account_public_keys_map, signer) } - fn create_sig_raw( + fn create_sig_raw( &mut self, - keypairs: Vec, + keys: Vec, account_public_keys_map: AccountPublicKeysMap, signer: Option
, - create_sig: F, ) -> &mut Self where - F: Fn( - Vec, - BTreeMap, - Option
, - ) -> Authorization, + KEY: SigOrPubKey, { // The inner tx signer signs the Raw version of the Header let hashes = vec![self.raw_header_hash()]; self.protocol_filter(); - let secret_keys = if signer.is_some() { - account_public_keys_map.index_secret_keys(keypairs) + let signing_keys = if signer.is_some() { + account_public_keys_map.index_keys(keys) } else { - (0..).zip(keypairs).collect() + (0..).zip(keys).collect() }; - self.add_section(Section::Authorization(create_sig( - hashes, - secret_keys, - signer, - ))); + let auth = + Authorization::create_signatures(hashes, signing_keys, signer); + self.add_section(Section::Authorization(auth)); self } diff --git a/wasm/vp_implicit/src/lib.rs b/wasm/vp_implicit/src/lib.rs index 8ad708c6de6..86df82a99ba 100644 --- a/wasm/vp_implicit/src/lib.rs +++ b/wasm/vp_implicit/src/lib.rs @@ -667,7 +667,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); @@ -808,7 +808,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); @@ -988,7 +988,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1110,7 +1110,7 @@ mod tests { tx.add_memo(&[1, 2, 3]); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); diff --git a/wasm/vp_user/src/lib.rs b/wasm/vp_user/src/lib.rs index d53ea783044..8e2bce0117f 100644 --- a/wasm/vp_user/src/lib.rs +++ b/wasm/vp_user/src/lib.rs @@ -466,7 +466,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![keypair]), + pks_map.index_keys(vec![keypair]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -694,7 +694,7 @@ mod tests { tx_data.set_code(Code::new(vec![], None)); tx_data.add_section(Section::Authorization(Authorization::new( vec![tx_data.raw_header_hash()], - pks_map.index_secret_keys(vec![sk3]), + pks_map.index_keys(vec![sk3]), None, ))); let signed_tx = tx_data.batch_first_tx(); @@ -1006,7 +1006,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1102,7 +1102,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1209,7 +1209,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![secret_key]), + pks_map.index_keys(vec![secret_key]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1382,7 +1382,7 @@ mod tests { tx.set_data(Data::new(vec![])); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![keypair]), + pks_map.index_keys(vec![keypair]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1480,7 +1480,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![keypair]), + pks_map.index_keys(vec![keypair]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1531,7 +1531,7 @@ mod tests { tx.set_code(Code::new(vec![], None)); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![keypair]), + pks_map.index_keys(vec![keypair]), None, ))); let signed_tx = tx.batch_first_tx(); @@ -1605,7 +1605,7 @@ mod tests { tx.add_memo(&[1, 2, 3]); tx.add_section(Section::Authorization(Authorization::new( vec![tx.raw_header_hash()], - pks_map.index_secret_keys(vec![keypair]), + pks_map.index_keys(vec![keypair]), None, )));