From ca7cbac320b729741f14eae97d986d712ab7c9ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Wed, 26 Nov 2025 10:45:01 +0000 Subject: [PATCH] chore: don't rederive wallet keys on every tx (#6213) * chore: make 'DirectSecp256k1HdWallet' only derive its keys once on construction Previously all the keys and account information was being derived for every transaction signed * no longer keep account seed on the wallet struct --- Cargo.lock | 1 + .../client-libs/validator-client/Cargo.toml | 1 + .../examples/offline_signing.rs | 17 ++- .../validator-client/src/client.rs | 15 ++- .../client-libs/validator-client/src/error.rs | 7 + .../client-libs/validator-client/src/lib.rs | 5 + .../contract_traits/dkg_signing_client.rs | 2 +- .../contract_traits/ecash_signing_client.rs | 2 +- .../contract_traits/group_signing_client.rs | 2 +- .../contract_traits/mixnet_signing_client.rs | 2 +- .../multisig_signing_client.rs | 2 +- .../performance_signing_client.rs | 2 +- .../contract_traits/vesting_signing_client.rs | 2 +- .../src/nyxd/cosmwasm_client/mod.rs | 2 +- .../validator-client/src/nyxd/mod.rs | 25 ++-- .../validator-client/src/rpc/reqwest.rs | 4 + .../src/signing/direct_wallet.rs | 125 +++++++----------- .../validator-client/src/signing/mod.rs | 56 +++++++- .../validator-client/src/signing/signer.rs | 29 ++-- .../validator-client/src/signing/tx_signer.rs | 2 +- .../commands/src/validator/account/create.rs | 6 +- .../commands/src/validator/account/pubkey.rs | 31 +++-- .../commands/src/validator/signature/sign.rs | 51 +++---- common/types/src/error.rs | 4 + .../src/operations/mixnet/account.rs | 8 +- .../src/operations/signatures/sign.rs | 2 +- .../testnet-manager/src/manager/contract.rs | 10 +- 27 files changed, 234 insertions(+), 181 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65d9405cab..b507cb04de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7406,6 +7406,7 @@ dependencies = [ name = "nym-validator-client" version = "0.1.0" dependencies = [ + "anyhow", "async-trait", "base64 0.22.1", "bip32", diff --git a/common/client-libs/validator-client/Cargo.toml b/common/client-libs/validator-client/Cargo.toml index afb988c98c..ec4eb62bec 100644 --- a/common/client-libs/validator-client/Cargo.toml +++ b/common/client-libs/validator-client/Cargo.toml @@ -75,6 +75,7 @@ workspace = true features = ["json", "rustls-tls"] [dev-dependencies] +anyhow = { workspace = true } bip39 = { workspace = true } cosmrs = { workspace = true, features = ["bip32"] } ts-rs = { workspace = true } diff --git a/common/client-libs/validator-client/examples/offline_signing.rs b/common/client-libs/validator-client/examples/offline_signing.rs index 62c160f1a8..c0a8295307 100644 --- a/common/client-libs/validator-client/examples/offline_signing.rs +++ b/common/client-libs/validator-client/examples/offline_signing.rs @@ -7,6 +7,7 @@ use cosmrs::{tx, AccountId, Coin, Denom}; use nym_validator_client::http_client; use nym_validator_client::nyxd::CosmWasmClient; use nym_validator_client::signing::direct_wallet::DirectSecp256k1HdWallet; +use nym_validator_client::signing::signer::OfflineSigner; use nym_validator_client::signing::tx_signer::TxSigner; use nym_validator_client::signing::SignerData; @@ -19,8 +20,8 @@ async fn main() { let validator = "https://rpc.sandbox.nymtech.net"; let to_address: AccountId = "n1pefc2utwpy5w78p2kqdsfmpjxfwmn9d39k5mqa".parse().unwrap(); - let signer = DirectSecp256k1HdWallet::from_mnemonic(prefix, signer_mnemonic); - let signer_address = signer.try_derive_accounts().unwrap()[0].address().clone(); + let signer = DirectSecp256k1HdWallet::checked_from_mnemonic(prefix, signer_mnemonic).unwrap(); + let signer_address = signer.signer_addresses()[0].clone(); // local 'client' ONLY signing messages let tx_signer = signer; @@ -57,9 +58,15 @@ async fn main() { 100000u32, ); - let tx_raw = tx_signer - .sign_direct(&signer_address, vec![send_msg], fee, memo, signer_data) - .unwrap(); + let tx_raw = TxSigner::sign_direct( + &tx_signer, + &signer_address, + vec![send_msg], + fee, + memo, + signer_data, + ) + .unwrap(); let tx_bytes = tx_raw.to_bytes().unwrap(); // compare balances from before and after the tx diff --git a/common/client-libs/validator-client/src/client.rs b/common/client-libs/validator-client/src/client.rs index 2758369ede..eb23ba60ca 100644 --- a/common/client-libs/validator-client/src/client.rs +++ b/common/client-libs/validator-client/src/client.rs @@ -5,8 +5,7 @@ use crate::nyxd::{self, NyxdClient}; use crate::signing::direct_wallet::DirectSecp256k1HdWallet; use crate::signing::signer::{NoSigner, OfflineSigner}; use crate::{ - DirectSigningReqwestRpcValidatorClient, QueryReqwestRpcValidatorClient, ReqwestRpcClient, - ValidatorClientError, + DirectSigningReqwestRpcValidatorClient, QueryReqwestRpcValidatorClient, ValidatorClientError, }; use nym_api_requests::ecash::models::{ AggregatedCoinIndicesSignatureResponse, AggregatedExpirationDateSignatureResponse, @@ -164,7 +163,7 @@ impl Client { ) -> Result { let rpc_client = http_client(config.nyxd_url.as_str())?; let prefix = &config.nyxd_config.chain_details.bech32_account_prefix; - let wallet = DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic); + let wallet = DirectSecp256k1HdWallet::checked_from_mnemonic(prefix, mnemonic)?; Ok(Self::new_signing_with_rpc_client( config, rpc_client, wallet, @@ -177,12 +176,13 @@ impl Client { } } -impl Client { +#[allow(deprecated)] +impl Client { pub fn new_reqwest_signing( config: Config, mnemonic: bip39::Mnemonic, ) -> DirectSigningReqwestRpcValidatorClient { - let rpc_client = ReqwestRpcClient::new(config.nyxd_url.clone()); + let rpc_client = crate::ReqwestRpcClient::new(config.nyxd_url.clone()); let prefix = &config.nyxd_config.chain_details.bech32_account_prefix; let wallet = DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic); @@ -203,9 +203,10 @@ impl Client { } } -impl Client { +#[allow(deprecated)] +impl Client { pub fn new_reqwest_query(config: Config) -> QueryReqwestRpcValidatorClient { - let rpc_client = ReqwestRpcClient::new(config.nyxd_url.clone()); + let rpc_client = crate::ReqwestRpcClient::new(config.nyxd_url.clone()); Self::new_with_rpc_client(config, rpc_client) } } diff --git a/common/client-libs/validator-client/src/error.rs b/common/client-libs/validator-client/src/error.rs index c3979c0784..09fb7c8171 100644 --- a/common/client-libs/validator-client/src/error.rs +++ b/common/client-libs/validator-client/src/error.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::nym_api; +use crate::signing::direct_wallet::DirectSecp256k1HdWalletError; pub use tendermint_rpc::error::Error as TendermintRpcError; use thiserror::Error; @@ -26,6 +27,12 @@ pub enum ValidatorClientError { #[error("No validator API url has been provided")] NoAPIUrlAvailable, + + #[error("failed to derive signing accounts: {source}")] + AccountDerivationFailure { + #[from] + source: DirectSecp256k1HdWalletError, + }, } impl From for ValidatorClientError { diff --git a/common/client-libs/validator-client/src/lib.rs b/common/client-libs/validator-client/src/lib.rs index db86658047..1736616dda 100644 --- a/common/client-libs/validator-client/src/lib.rs +++ b/common/client-libs/validator-client/src/lib.rs @@ -12,6 +12,7 @@ pub mod rpc; pub mod signing; pub use crate::error::ValidatorClientError; +#[allow(deprecated)] pub use crate::rpc::reqwest::ReqwestRpcClient; pub use crate::signing::direct_wallet::DirectSecp256k1HdWallet; pub use client::{Client, Config, EcashApiClient}; @@ -38,9 +39,13 @@ pub type DirectSigningHttpRpcValidatorClient = Client; +#[allow(deprecated)] pub type QueryReqwestRpcValidatorClient = Client; +#[allow(deprecated)] pub type QueryReqwestRpcNyxdClient = nyxd::NyxdClient; +#[allow(deprecated)] pub type DirectSigningReqwestRpcValidatorClient = Client; +#[allow(deprecated)] pub type DirectSigningReqwestRpcNyxdClient = nyxd::NyxdClient; diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs index 8371f33133..bebc6aab69 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs @@ -178,7 +178,7 @@ where .ok_or_else(|| NyxdError::unavailable_contract_address("dkg contract"))?; let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute(signer_address, dkg_contract_address, &msg, fee, memo, funds) .await diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/ecash_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/ecash_signing_client.rs index 4c40a2b2fb..72ba43053b 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/ecash_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/ecash_signing_client.rs @@ -99,7 +99,7 @@ where .ok_or_else(|| NyxdError::unavailable_contract_address("coconut bandwidth contract"))?; let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute( signer_address, diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/group_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/group_signing_client.rs index 5ac1954849..8ce18625b0 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/group_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/group_signing_client.rs @@ -95,7 +95,7 @@ where let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute( signer_address, group_contract_address, diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/mixnet_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/mixnet_signing_client.rs index 7ba4b3cd98..69aaac59d5 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/mixnet_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/mixnet_signing_client.rs @@ -667,7 +667,7 @@ where let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); let memo = msg.default_memo(); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute( signer_address, mixnet_contract_address, diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/multisig_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/multisig_signing_client.rs index 715a38d457..eefa56da36 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/multisig_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/multisig_signing_client.rs @@ -133,7 +133,7 @@ where let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute( signer_address, multisig_contract_address, diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/performance_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/performance_signing_client.rs index 78b960265a..d0380c0452 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/performance_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/performance_signing_client.rs @@ -165,7 +165,7 @@ where let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute( signer_address, performance_contract_address, diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/vesting_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/vesting_signing_client.rs index 2f50016f91..2a46cf04ac 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/vesting_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/vesting_signing_client.rs @@ -375,7 +375,7 @@ where let fee = fee.unwrap_or(Fee::Auto(Some(self.simulated_gas_multiplier()))); let memo = msg.name().to_string(); - let signer_address = &self.signer_addresses()?[0]; + let signer_address = &self.signer_addresses()[0]; self.execute( signer_address, vesting_contract_address, diff --git a/common/client-libs/validator-client/src/nyxd/cosmwasm_client/mod.rs b/common/client-libs/validator-client/src/nyxd/cosmwasm_client/mod.rs index 6516588339..df7f03dd34 100644 --- a/common/client-libs/validator-client/src/nyxd/cosmwasm_client/mod.rs +++ b/common/client-libs/validator-client/src/nyxd/cosmwasm_client/mod.rs @@ -324,7 +324,7 @@ where { type Error = S::Error; - fn get_accounts(&self) -> Result, Self::Error> { + fn get_accounts(&self) -> &[AccountData] { self.signer.get_accounts() } diff --git a/common/client-libs/validator-client/src/nyxd/mod.rs b/common/client-libs/validator-client/src/nyxd/mod.rs index 85b08ab12a..d7768b4bfd 100644 --- a/common/client-libs/validator-client/src/nyxd/mod.rs +++ b/common/client-libs/validator-client/src/nyxd/mod.rs @@ -19,7 +19,7 @@ use crate::signing::signer::NoSigner; use crate::signing::signer::OfflineSigner; use crate::signing::tx_signer::TxSigner; use crate::signing::AccountData; -use crate::{DirectSigningReqwestRpcNyxdClient, QueryReqwestRpcNyxdClient, ReqwestRpcClient}; +use crate::{DirectSigningReqwestRpcNyxdClient, QueryReqwestRpcNyxdClient}; use async_trait::async_trait; use cosmrs::tendermint::{abci, evidence::Evidence, Genesis}; use cosmrs::tx::{Raw, SignDoc}; @@ -158,12 +158,13 @@ impl NyxdClient { } } -impl NyxdClient { +#[allow(deprecated)] +impl NyxdClient { pub fn connect_reqwest( config: Config, endpoint: Url, ) -> Result { - let client = ReqwestRpcClient::new(endpoint); + let client = crate::ReqwestRpcClient::new(endpoint); Ok(NyxdClient { client: MaybeSigningClient::new(client, (&config).into()), @@ -195,18 +196,19 @@ impl NyxdClient { let client = http_client(endpoint)?; let prefix = &config.chain_details.bech32_account_prefix; - let wallet = DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic); + let wallet = DirectSecp256k1HdWallet::checked_from_mnemonic(prefix, mnemonic)?; Ok(Self::connect_with_signer(config, client, wallet)) } } -impl NyxdClient { +#[allow(deprecated)] +impl NyxdClient { pub fn connect_reqwest_with_mnemonic( config: Config, endpoint: Url, mnemonic: bip39::Mnemonic, ) -> DirectSigningReqwestRpcNyxdClient { - let client = ReqwestRpcClient::new(endpoint); + let client = crate::ReqwestRpcClient::new(endpoint); let prefix = &config.chain_details.bech32_account_prefix; let wallet = DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic); @@ -391,17 +393,12 @@ where S: OfflineSigner + Send + Sync, NyxdError: From<::Error>, { - pub fn signing_account(&self) -> Result { + pub fn signing_account(&self) -> Result<&AccountData, NyxdError> { Ok(self.find_account(&self.address())?) } pub fn address(&self) -> AccountId { - match self.client.signer_addresses() { - Ok(addresses) => addresses[0].clone(), - Err(_) => { - panic!("key derivation failure") - } - } + self.client.signer_addresses()[0].clone() } pub fn mix_coin(&self, amount: u128) -> Coin { @@ -867,7 +864,7 @@ where { type Error = S::Error; - fn get_accounts(&self) -> Result, Self::Error> { + fn get_accounts(&self) -> &[AccountData] { self.client.get_accounts() } diff --git a/common/client-libs/validator-client/src/rpc/reqwest.rs b/common/client-libs/validator-client/src/rpc/reqwest.rs index 09312b28c3..b4cd98cd2f 100644 --- a/common/client-libs/validator-client/src/rpc/reqwest.rs +++ b/common/client-libs/validator-client/src/rpc/reqwest.rs @@ -42,12 +42,15 @@ macro_rules! perform_with_compat { }}; } +// the separate implementation is now completely redundant +#[deprecated(note = "use HttpClient directly instead")] pub struct ReqwestRpcClient { compat: CompatMode, inner: reqwest::Client, url: Url, } +#[allow(deprecated)] impl ReqwestRpcClient { pub fn new(url: Url) -> Self { ReqwestRpcClient { @@ -131,6 +134,7 @@ impl TendermintRpcErrorMap for reqwest::Error { } } +#[allow(deprecated)] #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] #[cfg_attr(not(target_arch = "wasm32"), async_trait)] impl TendermintRpcClient for ReqwestRpcClient { diff --git a/common/client-libs/validator-client/src/signing/direct_wallet.rs b/common/client-libs/validator-client/src/signing/direct_wallet.rs index 9d44255129..6192ead995 100644 --- a/common/client-libs/validator-client/src/signing/direct_wallet.rs +++ b/common/client-libs/validator-client/src/signing/direct_wallet.rs @@ -3,17 +3,13 @@ use crate::signing::signer::{OfflineSigner, SigningError}; use crate::signing::{AccountData, Secp256k1Derivation}; -use cosmrs::bip32::{DerivationPath, XPrv}; -use cosmrs::crypto::secp256k1::SigningKey; -use cosmrs::crypto::PublicKey; +use cosmrs::bip32::DerivationPath; use cosmrs::tx; use cosmrs::tx::SignDoc; use nym_config::defaults; use thiserror::Error; use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; -type Secp256k1Keypair = (SigningKey, PublicKey); - #[derive(Debug, Error)] pub enum DirectSecp256k1HdWalletError { #[error(transparent)] @@ -36,28 +32,22 @@ pub enum DirectSecp256k1HdWalletError { } // TODO: maybe lock this one behind feature flag? -#[derive(Debug, Clone, Zeroize, ZeroizeOnDrop)] +#[derive(Zeroize, ZeroizeOnDrop)] pub struct DirectSecp256k1HdWallet { /// Base secret secret: bip39::Mnemonic, - /// BIP39 seed - seed: [u8; 64], - - // An unfortunate result of immature rust async story is that async traits (only available in the separate package) - // can't yet figure out everything and if we stored our derived account data on the struct, - // that would include the secret key which is a dyn EcdsaSigner and hence not Sync making the wallet - // not Sync and if used on the signing client in an async trait, it wouldn't be Send - /// Derivation instructions + /// Derived accounts #[zeroize(skip)] - accounts: Vec, + // unfortunately `dyn EcdsaSigner` does not guarantee Zeroize + accounts: Vec, } impl OfflineSigner for DirectSecp256k1HdWallet { type Error = DirectSecp256k1HdWalletError; - fn get_accounts(&self) -> Result, Self::Error> { - self.try_derive_accounts() + fn get_accounts(&self) -> &[AccountData] { + &self.accounts } fn sign_direct_with_account( @@ -77,55 +67,27 @@ impl DirectSecp256k1HdWallet { } /// Restores a wallet from the given BIP39 mnemonic using default options. + #[deprecated( + note = "this function can potentially panic if accounts can't be derived correctly. please use .checked_from_mnemonic() instead" + )] pub fn from_mnemonic(prefix: &str, mnemonic: bip39::Mnemonic) -> Self { + // unfortunately due to backwards compatibility requirements, + // we can't change signature of this method + #[allow(deprecated)] DirectSecp256k1HdWalletBuilder::new(prefix).build(mnemonic) } + /// Restores a wallet from the given BIP39 mnemonic using default options. + pub fn checked_from_mnemonic( + prefix: &str, + mnemonic: bip39::Mnemonic, + ) -> Result { + DirectSecp256k1HdWalletBuilder::new(prefix).try_build(mnemonic) + } + pub fn generate(prefix: &str, word_count: usize) -> Result { let mneomonic = bip39::Mnemonic::generate(word_count)?; - Ok(Self::from_mnemonic(prefix, mneomonic)) - } - - fn derive_keypair( - &self, - hd_path: &DerivationPath, - ) -> Result { - let extended_private_key = XPrv::derive_from_path(self.seed, hd_path)?; - - let private_key: SigningKey = extended_private_key.into(); - let public_key = private_key.public_key(); - - Ok((private_key, public_key)) - } - - pub fn derive_extended_private_key( - &self, - hd_path: &DerivationPath, - ) -> Result { - Ok(XPrv::derive_from_path(self.seed, hd_path)?) - } - - pub fn try_derive_accounts(&self) -> Result, DirectSecp256k1HdWalletError> { - let mut accounts = Vec::with_capacity(self.accounts.len()); - for derivation_info in &self.accounts { - let keypair = self.derive_keypair(&derivation_info.hd_path)?; - - // it seems this can only fail if the provided account prefix is invalid - let address = keypair - .1 - .account_id(&derivation_info.prefix) - .map_err( - |source| DirectSecp256k1HdWalletError::AccountDerivationError { source }, - )?; - - accounts.push(AccountData { - address, - public_key: keypair.1, - private_key: keypair.0, - }) - } - - Ok(accounts) + Self::checked_from_mnemonic(prefix, mneomonic) } pub fn secret(&self) -> &bip39::Mnemonic { @@ -188,23 +150,39 @@ impl DirectSecp256k1HdWalletBuilder { self } + #[deprecated( + note = "this function can potentially panic if accounts can't be derived correctly. please use .try_build() instead" + )] pub fn build(self, mnemonic: bip39::Mnemonic) -> DirectSecp256k1HdWallet { - let seed = mnemonic.to_seed(&self.bip39_password); + // unfortunately due to backwards compatibility requirements, + // we can't change signature of this method + #[allow(clippy::expect_used)] + self.try_build(mnemonic) + .expect("account derivation failure") + } + + pub fn try_build( + self, + mnemonic: bip39::Mnemonic, + ) -> Result { + let seed = Zeroizing::new(mnemonic.to_seed(&self.bip39_password)); let prefix = self.prefix.clone(); let accounts = self .hd_paths .iter() - .map(|hd_path| Secp256k1Derivation { - hd_path: hd_path.clone(), - prefix: prefix.clone(), + .map(|hd_path| { + Secp256k1Derivation { + hd_path: hd_path.clone(), + prefix: prefix.clone(), + } + .try_derive_account(&seed) }) - .collect(); + .collect::>()?; - DirectSecp256k1HdWallet { + Ok(DirectSecp256k1HdWallet { accounts, - seed, secret: mnemonic, - } + }) } } @@ -215,7 +193,7 @@ mod tests { use super::*; #[test] - fn generating_account_addresses() { + fn generating_account_addresses() -> anyhow::Result<()> { // test vectors produced from our js wallet let mnemonics = ["crush minute paddle tobacco message debate cabin peace bar jacket execute twenty winner view sure mask popular couch penalty fragile demise fresh pizza stove", "acquire rebel spot skin gun such erupt pull swear must define ill chief turtle today flower chunk truth battle claw rigid detail gym feel", @@ -230,11 +208,10 @@ mod tests { "n17n9flp6jflljg6fp05dsy07wcprf2uuu8g40rf", ]; for (idx, mnemonic) in mnemonics.iter().enumerate() { - let wallet = DirectSecp256k1HdWallet::from_mnemonic(&prefix, mnemonic.parse().unwrap()); - assert_eq!( - wallet.try_derive_accounts().unwrap()[0].address, - addrs[idx].parse().unwrap() - ) + let wallet = + DirectSecp256k1HdWallet::checked_from_mnemonic(&prefix, mnemonic.parse()?)?; + assert_eq!(wallet.signer_addresses()[0], addrs[idx].parse().unwrap()); } + Ok(()) } } diff --git a/common/client-libs/validator-client/src/signing/mod.rs b/common/client-libs/validator-client/src/signing/mod.rs index d9fa0c30e4..e3f6174115 100644 --- a/common/client-libs/validator-client/src/signing/mod.rs +++ b/common/client-libs/validator-client/src/signing/mod.rs @@ -1,6 +1,8 @@ // Copyright 2023 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 +use crate::signing::direct_wallet::DirectSecp256k1HdWalletError; +use bip32::XPrv; use cosmrs::bip32::DerivationPath; use cosmrs::crypto::secp256k1::SigningKey; use cosmrs::crypto::PublicKey; @@ -12,14 +14,64 @@ pub mod direct_wallet; pub mod signer; pub mod tx_signer; +pub(crate) type Secp256k1Keypair = (SigningKey, PublicKey); + /// Derivation information required to derive a keypair and an address from a mnemonic. #[derive(Debug, Clone)] -struct Secp256k1Derivation { +pub(crate) struct Secp256k1Derivation { hd_path: DerivationPath, prefix: String, } -// TODO: is this struct going to be derivable with other signer types? +impl Secp256k1Derivation { + pub(crate) fn try_derive_account( + &self, + seed: S, + ) -> Result + where + S: AsRef<[u8]>, + { + let keypair = derive_keypair(seed, &self.hd_path)?; + + // it seems this can only fail if the provided account prefix is invalid + let address = keypair + .1 + .account_id(&self.prefix) + .map_err(|source| DirectSecp256k1HdWalletError::AccountDerivationError { source })?; + + Ok(AccountData { + address, + public_key: keypair.1, + private_key: keypair.0, + }) + } +} + +fn derive_keypair( + seed: S, + hd_path: &DerivationPath, +) -> Result +where + S: AsRef<[u8]>, +{ + let extended_private_key = derive_extended_private_key(seed, hd_path)?; + + let private_key: SigningKey = extended_private_key.into(); + let public_key = private_key.public_key(); + + Ok((private_key, public_key)) +} + +fn derive_extended_private_key( + seed: S, + hd_path: &DerivationPath, +) -> Result +where + S: AsRef<[u8]>, +{ + Ok(XPrv::derive_from_path(seed, hd_path)?) +} + pub struct AccountData { pub address: AccountId, diff --git a/common/client-libs/validator-client/src/signing/signer.rs b/common/client-libs/validator-client/src/signing/signer.rs index 4b620291fe..af7fb584e0 100644 --- a/common/client-libs/validator-client/src/signing/signer.rs +++ b/common/client-libs/validator-client/src/signing/signer.rs @@ -33,23 +33,18 @@ pub enum SignerType { pub trait OfflineSigner { type Error: From; - // I really dislike existence of this function because it makes you re-derive your key **twice** for each contract transaction - fn signer_addresses(&self) -> Result, Self::Error> { - let derived_addresses = self - .get_accounts()? - .into_iter() - .map(|account| account.address) - .collect(); - Ok(derived_addresses) + fn signer_addresses(&self) -> Vec { + self.get_accounts() + .iter() + .map(|account| account.address.clone()) + .collect() } - fn get_accounts(&self) -> Result, Self::Error>; + fn get_accounts(&self) -> &[AccountData]; - fn find_account(&self, signer_address: &AccountId) -> Result { - // TODO: we could really use some zeroize action here - let accounts = self.get_accounts()?; - accounts - .into_iter() + fn find_account(&self, signer_address: &AccountId) -> Result<&AccountData, Self::Error> { + self.get_accounts() + .iter() .find(|account| &account.address == signer_address) .ok_or_else(|| { SigningError::AccountNotFound { @@ -76,7 +71,7 @@ pub trait OfflineSigner { message: M, ) -> Result { let signer = self.find_account(signer_address)?; - self.sign_raw_with_account(&signer, message) + self.sign_raw_with_account(signer, message) } fn sign_direct( @@ -85,7 +80,7 @@ pub trait OfflineSigner { sign_doc: SignDoc, ) -> Result { let signer = self.find_account(signer_address)?; - self.sign_direct_with_account(&signer, sign_doc) + self.sign_direct_with_account(signer, sign_doc) } // unless explicitly defined, each signing method is unsupported @@ -122,7 +117,7 @@ pub struct NoSigner; // impl OfflineSigner for NoSigner { // type Error = SignerUnavailable; // -// fn get_accounts(&self) -> Result, Self::Error> { +// fn get_accounts(&self) -> &[AccountData] { // return Err(SignerUnavailable); // } // } diff --git a/common/client-libs/validator-client/src/signing/tx_signer.rs b/common/client-libs/validator-client/src/signing/tx_signer.rs index f04047c39e..59289bbe09 100644 --- a/common/client-libs/validator-client/src/signing/tx_signer.rs +++ b/common/client-libs/validator-client/src/signing/tx_signer.rs @@ -50,7 +50,7 @@ pub trait TxSigner: OfflineSigner { ) .map_err(|source| SigningError::SignDocFailure { source })?; - self.sign_direct_with_account(&account_from_signer, sign_doc) + self.sign_direct_with_account(account_from_signer, sign_doc) } } diff --git a/common/commands/src/validator/account/create.rs b/common/commands/src/validator/account/create.rs index f2cd4eb336..0839ff1279 100644 --- a/common/commands/src/validator/account/create.rs +++ b/common/commands/src/validator/account/create.rs @@ -3,6 +3,7 @@ use clap::Parser; use nym_validator_client::signing::direct_wallet::DirectSecp256k1HdWallet; +use nym_validator_client::signing::signer::OfflineSigner; #[derive(Debug, Parser)] pub struct Args { @@ -15,9 +16,10 @@ pub fn create_account(args: Args, prefix: &str) { let word_count = args.word_count.unwrap_or(24); let mnemonic = bip39::Mnemonic::generate(word_count).expect("failed to generate mnemonic!"); - let wallet = DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic); + let wallet = DirectSecp256k1HdWallet::checked_from_mnemonic(prefix, mnemonic) + .expect("failed to derive accounts!"); // Output address and mnemonics into separate lines for easier parsing println!("{}", wallet.mnemonic_string().as_str()); - println!("{}", wallet.try_derive_accounts().unwrap()[0].address()); + println!("{}", wallet.signer_addresses()[0]); } diff --git a/common/commands/src/validator/account/pubkey.rs b/common/commands/src/validator/account/pubkey.rs index ed419c1101..fc9bf25f97 100644 --- a/common/commands/src/validator/account/pubkey.rs +++ b/common/commands/src/validator/account/pubkey.rs @@ -1,13 +1,13 @@ // Copyright 2021 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 +use crate::context::QueryClient; +use crate::utils::show_error; use clap::Parser; use log::{error, info}; use nym_validator_client::nyxd::{AccountId, CosmWasmClient}; use nym_validator_client::signing::direct_wallet::DirectSecp256k1HdWallet; - -use crate::context::QueryClient; -use crate::utils::show_error; +use nym_validator_client::signing::signer::OfflineSigner; #[derive(Debug, Parser)] pub struct Args { @@ -50,20 +50,19 @@ pub async fn get_pubkey( } pub fn get_pubkey_from_mnemonic(address: AccountId, prefix: &str, mnemonic: bip39::Mnemonic) { - let wallet = DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic); - match wallet.try_derive_accounts() { - Ok(accounts) => match accounts.iter().find(|a| *a.address() == address) { - Some(account) => { - println!("{}", account.public_key().to_string()); - } - None => { - error!("Could not derive key that matches {address}") - } - }, - Err(e) => { - error!("Failed to derive accounts. {e}"); + let wallet = match DirectSecp256k1HdWallet::checked_from_mnemonic(prefix, mnemonic) { + Ok(wallet) => wallet, + Err(err) => { + error!("Failed to derive accounts. {err}"); + return; } - } + }; + + let Ok(account) = wallet.find_account(&address) else { + error!("Could not derive key that matches {address}"); + return; + }; + println!("{}", account.public_key().to_string()); } pub async fn get_pubkey_from_chain(address: AccountId, client: &QueryClient) { diff --git a/common/commands/src/validator/signature/sign.rs b/common/commands/src/validator/signature/sign.rs index 7df1d12eb6..6a2e3254b9 100644 --- a/common/commands/src/validator/signature/sign.rs +++ b/common/commands/src/validator/signature/sign.rs @@ -36,32 +36,35 @@ pub fn sign(args: Args, prefix: &str, mnemonic: Option) { return; } - let wallet = - DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic.expect("mnemonic not set")); - match wallet.try_derive_accounts() { - Ok(accounts) => match accounts.first() { - Some(account) => { - let msg = args.message.into_bytes(); - match wallet.sign_raw_with_account(account, msg) { - Ok(signature) => { - let output = SignatureOutputJson { - account_id: account.address().to_string(), - public_key: account.public_key(), - signature_as_hex: signature.to_string(), - }; - println!("{}", json!(output)); - } - Err(e) => { - error!("Failed to sign message. {e}"); - } + let wallet = match DirectSecp256k1HdWallet::checked_from_mnemonic( + prefix, + mnemonic.expect("mnemonic not set"), + ) { + Ok(wallet) => wallet, + Err(err) => { + error!("Could not derive an account key from the mnemonic: {err}"); + return; + } + }; + match wallet.get_accounts().first() { + Some(account) => { + let msg = args.message.into_bytes(); + match wallet.sign_raw_with_account(account, msg) { + Ok(signature) => { + let output = SignatureOutputJson { + account_id: account.address().to_string(), + public_key: account.public_key(), + signature_as_hex: signature.to_string(), + }; + println!("{}", json!(output)); + } + Err(e) => { + error!("Failed to sign message. {e}"); } } - None => { - error!("Could not derive an account key from the mnemonic",) - } - }, - Err(e) => { - error!("Failed to derive accounts. {e}"); + } + None => { + error!("Could not derive an account key from the mnemonic",) } } } diff --git a/common/types/src/error.rs b/common/types/src/error.rs index b72a4a18d6..e8d803b077 100644 --- a/common/types/src/error.rs +++ b/common/types/src/error.rs @@ -1,6 +1,7 @@ use nym_mixnet_contract_common::ContractsCommonError; use nym_validator_client::error::TendermintRpcError; use nym_validator_client::nym_api::error::NymAPIError; +use nym_validator_client::signing::direct_wallet::DirectSecp256k1HdWalletError; use nym_validator_client::{nyxd::error::NyxdError, ValidatorClientError}; use serde::{Serialize, Serializer}; use std::io; @@ -59,6 +60,8 @@ pub enum TypesError { #[from] source: cosmwasm_std::DecimalRangeExceeded, }, + #[error(transparent)] + AccountDerivationFailure(#[from] DirectSecp256k1HdWalletError), #[error("No nym API URL configured")] NoNymApiUrlConfigured, #[error("{0} is not a valid amount string")] @@ -113,6 +116,7 @@ impl From for TypesError { ValidatorClientError::InconsistentPagedMetadata => { TypesError::InconsistentPagedMetadata } + ValidatorClientError::AccountDerivationFailure { source } => source.into(), } } } diff --git a/nym-wallet/src-tauri/src/operations/mixnet/account.rs b/nym-wallet/src-tauri/src/operations/mixnet/account.rs index 3319363be1..63dc75e884 100644 --- a/nym-wallet/src-tauri/src/operations/mixnet/account.rs +++ b/nym-wallet/src-tauri/src/operations/mixnet/account.rs @@ -11,7 +11,7 @@ use nym_config::defaults::{NymNetworkDetails, COSMOS_DERIVATION_PATH}; use nym_types::account::{Account, AccountEntry, Balance}; use nym_validator_client::nyxd::CosmWasmClient; use nym_validator_client::signing::direct_wallet::DirectSecp256k1HdWallet; -use nym_validator_client::signing::AccountData; +use nym_validator_client::signing::signer::OfflineSigner; use nym_validator_client::DirectSigningHttpRpcValidatorClient; use nym_wallet_types::network::Network as WalletNetwork; use std::collections::HashMap; @@ -575,10 +575,10 @@ fn derive_address( prefix: &str, ) -> Result { // note: the ephemeral wallet will zeroize the mnemonic on drop - DirectSecp256k1HdWallet::from_mnemonic(prefix, mnemonic) - .try_derive_accounts()? + DirectSecp256k1HdWallet::checked_from_mnemonic(prefix, mnemonic) + .map_err(|_| BackendError::FailedToDeriveAddress)? + .signer_addresses() .first() - .map(AccountData::address) .cloned() .ok_or(BackendError::FailedToDeriveAddress) } diff --git a/nym-wallet/src-tauri/src/operations/signatures/sign.rs b/nym-wallet/src-tauri/src/operations/signatures/sign.rs index e93edd57f4..16bb74ecef 100644 --- a/nym-wallet/src-tauri/src/operations/signatures/sign.rs +++ b/nym-wallet/src-tauri/src/operations/signatures/sign.rs @@ -24,7 +24,7 @@ pub async fn sign( ) -> Result { let guard = state.read().await; let client = guard.current_client()?; - let derived_accounts = client.nyxd.get_accounts()?; + let derived_accounts = client.nyxd.get_accounts(); let account = derived_accounts.first().ok_or_else(|| { log::error!(">>> Unable to derive account"); BackendError::SignatureError("unable to derive account".to_string()) diff --git a/tools/internal/testnet-manager/src/manager/contract.rs b/tools/internal/testnet-manager/src/manager/contract.rs index 5792dbd23e..4c04ade5d0 100644 --- a/tools/internal/testnet-manager/src/manager/contract.rs +++ b/tools/internal/testnet-manager/src/manager/contract.rs @@ -7,6 +7,7 @@ use nym_validator_client::nyxd::cosmwasm_client::types::{ ContractCodeId, InstantiateResult, MigrateResult, UploadResult, }; use nym_validator_client::nyxd::{AccountId, Hash}; +use nym_validator_client::signing::signer::OfflineSigner; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; @@ -145,12 +146,9 @@ impl Account { pub(crate) fn new() -> Account { let mnemonic = bip39::Mnemonic::generate(24).unwrap(); // sure, we're using hardcoded prefix, but realistically this will never change - let wallet = DirectSecp256k1HdWallet::from_mnemonic("n", mnemonic.clone()); - let acc = wallet.try_derive_accounts().unwrap().pop().unwrap(); - Account { - address: acc.address, - mnemonic, - } + let wallet = DirectSecp256k1HdWallet::checked_from_mnemonic("n", mnemonic.clone()).unwrap(); + let address = wallet.signer_addresses().pop().unwrap(); + Account { address, mnemonic } } pub(crate) fn address(&self) -> AccountId {