From 91ba9c1536d48e377f0c6efad07ab1f1b433c14b Mon Sep 17 00:00:00 2001 From: jonesmarvin8 <83104039+jonesmarvin8@users.noreply.github.com> Date: Fri, 15 May 2026 18:09:40 -0400 Subject: [PATCH] adjust signer logic workflow --- wallet/src/cli/mod.rs | 53 ------ .../native_token_transfer/public.rs | 88 +++------ wallet/src/signing.rs | 178 ++++++++++-------- 3 files changed, 130 insertions(+), 189 deletions(-) diff --git a/wallet/src/cli/mod.rs b/wallet/src/cli/mod.rs index f76faf23..8bd0d157 100644 --- a/wallet/src/cli/mod.rs +++ b/wallet/src/cli/mod.rs @@ -146,59 +146,6 @@ impl CliAccountMention { } } - #[must_use] - pub const fn is_keycard(&self) -> bool { - matches!(self, Self::KeyPath(_)) - } - - #[must_use] - pub fn key_path(&self) -> Option<&str> { - match self { - Self::KeyPath(path) => Some(path), - Self::Id(_) | Self::Label(_) => None, - } - } - - /// Resolve to an [`AccountSigner`] for a sender — must sign, never `Foreign`. - pub fn to_signer(&self, wallet_core: &WalletCore) -> Result { - if let Self::KeyPath(path) = self { - return Ok(crate::signing::AccountSigner::Keycard(path.clone())); - } - let account = self.resolve(wallet_core.storage())?; - match account { - AccountIdWithPrivacy::Public(id) => Ok(crate::signing::AccountSigner::Local(id)), - AccountIdWithPrivacy::Private(_) => { - anyhow::bail!("Private accounts not supported as senders here") - } - } - } - - /// Resolve to an [`AccountSigner`] for a recipient — returns `Foreign` when the account - /// has no local key and no keycard path, meaning no signature or nonce is required. - pub fn to_recipient_signer( - &self, - wallet_core: &WalletCore, - ) -> Result { - if let Self::KeyPath(path) = self { - return Ok(crate::signing::AccountSigner::Keycard(path.clone())); - } - let account = self.resolve(wallet_core.storage())?; - match account { - AccountIdWithPrivacy::Public(id) => Ok( - match wallet_core - .storage() - .key_chain() - .pub_account_signing_key(id) - { - Some(_) => crate::signing::AccountSigner::Local(id), - None => crate::signing::AccountSigner::Foreign, - }, - ), - AccountIdWithPrivacy::Private(_) => { - anyhow::bail!("Private accounts not supported as recipients here") - } - } - } } impl FromStr for CliAccountMention { diff --git a/wallet/src/program_facades/native_token_transfer/public.rs b/wallet/src/program_facades/native_token_transfer/public.rs index de064272..60c42189 100644 --- a/wallet/src/program_facades/native_token_transfer/public.rs +++ b/wallet/src/program_facades/native_token_transfer/public.rs @@ -11,7 +11,7 @@ use sequencer_service_rpc::RpcClient as _; use super::NativeTokenTransfer; use crate::{ ExecutionFailureKind, cli::CliAccountMention, helperfunctions::read_pin, - signing::KeycardSessionContext, + signing::SigningGroups, }; impl NativeTokenTransfer<'_> { @@ -23,30 +23,26 @@ impl NativeTokenTransfer<'_> { from_mention: &CliAccountMention, to_mention: &CliAccountMention, ) -> Result { - let from_signer = from_mention.to_signer(self.0).map_err(|e| { - ExecutionFailureKind::KeycardError(pyo3::PyErr::new::(e.to_string())) - })?; - let to_signer = to_mention.to_recipient_signer(self.0).map_err(|e| { - ExecutionFailureKind::KeycardError(pyo3::PyErr::new::(e.to_string())) - })?; - - let account_ids = vec![from, to]; - let signing_ids: Vec = if to_signer.needs_signature() { - vec![from, to] - } else { - vec![from] - }; + let mut groups = SigningGroups::new(); + groups + .add_sender(from_mention, from, self.0) + .and_then(|()| groups.add_recipient(to_mention, to, self.0)) + .map_err(|e| { + ExecutionFailureKind::KeycardError(pyo3::PyErr::new::( + e.to_string(), + )) + })?; let program_id = Program::authenticated_transfer_program().id(); let nonces = self .0 - .get_accounts_nonces(signing_ids) + .get_accounts_nonces(groups.signing_ids()) .await .map_err(ExecutionFailureKind::SequencerError)?; let message = Message::try_new( program_id, - account_ids, + vec![from, to], nonces, AuthTransferInstruction::Transfer { amount: balance_to_move, @@ -54,7 +50,7 @@ impl NativeTokenTransfer<'_> { ) .map_err(ExecutionFailureKind::TransactionBuildError)?; - let pin = if from_mention.is_keycard() || to_mention.is_keycard() { + let pin = if groups.needs_pin() { read_pin() .map_err(|e| { ExecutionFailureKind::KeycardError(pyo3::PyErr::new::( @@ -67,30 +63,11 @@ impl NativeTokenTransfer<'_> { String::new() }; - let witness_set = pyo3::Python::with_gil(|py| -> pyo3::PyResult { - let mut ctx = KeycardSessionContext::new(&pin); - let hash = message.hash(); + let sigs = groups.sign_all(&message.hash(), &pin).map_err(|e| { + ExecutionFailureKind::KeycardError(pyo3::PyErr::new::(e.to_string())) + })?; - let (from_sig, from_pk) = from_signer - .sign(self.0, &mut ctx, py, &hash) - .expect("from signer always produces a signature") - .map_err(|e| pyo3::PyErr::new::(e.to_string()))?; - - let sigs_and_keys = match to_signer - .sign(self.0, &mut ctx, py, &hash) - .transpose() - .map_err(|e| pyo3::PyErr::new::(e.to_string()))? - { - Some((to_sig, to_pk)) => vec![(from_sig, from_pk), (to_sig, to_pk)], - None => vec![(from_sig, from_pk)], - }; - - ctx.close(py); - Ok(WitnessSet::from_raw_parts(sigs_and_keys)) - }) - .map_err(ExecutionFailureKind::KeycardError)?; - - let tx = PublicTransaction::new(message, witness_set); + let tx = PublicTransaction::new(message, WitnessSet::from_raw_parts(sigs)); Ok(self .0 .sequencer_client @@ -119,11 +96,16 @@ impl NativeTokenTransfer<'_> { ) .map_err(ExecutionFailureKind::TransactionBuildError)?; - let signer = account_mention.to_signer(self.0).map_err(|e| { - ExecutionFailureKind::KeycardError(pyo3::PyErr::new::(e.to_string())) - })?; + let mut groups = SigningGroups::new(); + groups + .add_sender(account_mention, from, self.0) + .map_err(|e| { + ExecutionFailureKind::KeycardError(pyo3::PyErr::new::( + e.to_string(), + )) + })?; - let pin = if account_mention.is_keycard() { + let pin = if groups.needs_pin() { read_pin() .map_err(|e| { ExecutionFailureKind::KeycardError(pyo3::PyErr::new::( @@ -136,21 +118,11 @@ impl NativeTokenTransfer<'_> { String::new() }; - let witness_set = pyo3::Python::with_gil(|py| -> pyo3::PyResult { - let mut ctx = KeycardSessionContext::new(&pin); - let hash = message.hash(); + let sigs = groups.sign_all(&message.hash(), &pin).map_err(|e| { + ExecutionFailureKind::KeycardError(pyo3::PyErr::new::(e.to_string())) + })?; - let (sig, pk) = signer - .sign(self.0, &mut ctx, py, &hash) - .expect("account signer always produces a signature") - .map_err(|e| pyo3::PyErr::new::(e.to_string()))?; - - ctx.close(py); - Ok(WitnessSet::from_raw_parts(vec![(sig, pk)])) - }) - .map_err(ExecutionFailureKind::KeycardError)?; - - let tx = PublicTransaction::new(message, witness_set); + let tx = PublicTransaction::new(message, WitnessSet::from_raw_parts(sigs)); Ok(self .0 .sequencer_client diff --git a/wallet/src/signing.rs b/wallet/src/signing.rs index 19d45fac..447039b9 100644 --- a/wallet/src/signing.rs +++ b/wallet/src/signing.rs @@ -1,93 +1,115 @@ use anyhow::Result; use keycard_wallet::{KeycardWallet, python_path}; -use nssa::{AccountId, PublicKey, Signature}; -use pyo3::Python; +use nssa::{AccountId, PrivateKey, PublicKey, Signature}; -use crate::WalletCore; +use crate::{WalletCore, cli::CliAccountMention}; -/// How a single account participates in signing a transaction. +/// Groups transaction signers by type to minimise Python GIL acquisition. /// -/// Created from [`crate::cli::CliAccountMention`] via `to_signer` / `to_recipient_signer`. -/// Used inside `Python::with_gil` blocks — does not cross async boundaries. -pub enum AccountSigner { - /// Account is in the local wallet; key is looked up from storage at sign time. - Local(AccountId), - /// Account is on a Keycard at the given BIP32 path. - Keycard(String), - /// Foreign account — no signature or nonce required. - Foreign, +/// Local signers are signed in pure Rust; all keycard signers share a single Python session +/// with one `connect` / `close_session` pair. +#[derive(Default)] +pub struct SigningGroups { + local: Vec<(AccountId, PrivateKey)>, + keycard: Vec<(AccountId, String)>, } -impl AccountSigner { +impl SigningGroups { #[must_use] - pub const fn needs_signature(&self) -> bool { - !matches!(self, Self::Foreign) + pub fn new() -> Self { + Self::default() } - /// Sign `hash` and return `(Signature, PublicKey)`, or `None` for `Foreign`. - pub fn sign( - &self, + /// Add a sender. Keycard paths are queued for the hardware session; local accounts + /// have their signing key resolved eagerly. Errors if no key is found. + pub fn add_sender( + &mut self, + mention: &CliAccountMention, + account_id: AccountId, wallet_core: &WalletCore, - ctx: &mut KeycardSessionContext, - py: Python<'_>, - hash: &[u8; 32], - ) -> Option> { - match self { - Self::Local(id) => { - let key = wallet_core - .storage() - .key_chain() - .pub_account_signing_key(*id); - Some(key.map_or_else( - || Err(anyhow::anyhow!("signing key not found for account {id}")), - |key| { - Ok(( - Signature::new(key, hash), - PublicKey::new_from_private_key(key), - )) - }, - )) - } - Self::Keycard(path) => Some( - ctx.get_or_connect(py) - .and_then(|w| w.sign_message_for_path(py, path, hash)) - .map_err(anyhow::Error::from), - ), - Self::Foreign => None, + ) -> Result<()> { + if let CliAccountMention::KeyPath(path) = mention { + self.keycard.push((account_id, path.clone())); + return Ok(()); } + let key = wallet_core + .storage() + .key_chain() + .pub_account_signing_key(account_id) + .ok_or_else(|| anyhow::anyhow!("signing key not found for account {account_id}"))? + .clone(); + self.local.push((account_id, key)); + Ok(()) + } + + /// Add a recipient. Same as [`add_sender`] but silently skips accounts with no local + /// key and no keycard path — they are foreign and require neither a signature nor a nonce. + pub fn add_recipient( + &mut self, + mention: &CliAccountMention, + account_id: AccountId, + wallet_core: &WalletCore, + ) -> Result<()> { + if let CliAccountMention::KeyPath(path) = mention { + self.keycard.push((account_id, path.clone())); + return Ok(()); + } + if let Some(key) = wallet_core + .storage() + .key_chain() + .pub_account_signing_key(account_id) + { + self.local.push((account_id, key.clone())); + } + Ok(()) + } + + /// Returns `true` when a PIN is required (at least one keycard signer is present). + #[must_use] + pub const fn needs_pin(&self) -> bool { + !self.keycard.is_empty() + } + + /// Account IDs that require a nonce (every non-foreign signer). + #[must_use] + pub fn signing_ids(&self) -> Vec { + self.local + .iter() + .map(|(id, _)| *id) + .chain(self.keycard.iter().map(|(id, _)| *id)) + .collect() + } + + /// Sign `hash` for every account in the group. + /// + /// Local accounts are signed in pure Rust. Keycard accounts share one Python session. + pub fn sign_all(&self, hash: &[u8; 32], pin: &str) -> Result> { + let mut sigs: Vec<(Signature, PublicKey)> = self + .local + .iter() + .map(|(_, key)| { + ( + Signature::new(key, hash), + PublicKey::new_from_private_key(key), + ) + }) + .collect(); + + if !self.keycard.is_empty() { + pyo3::Python::with_gil(|py| -> pyo3::PyResult<()> { + python_path::add_python_path(py)?; + let wallet = KeycardWallet::new(py)?; + wallet.connect(py, pin)?; + for (_, path) in &self.keycard { + sigs.push(wallet.sign_message_for_path(py, path, hash)?); + } + drop(wallet.close_session(py)); + Ok(()) + }) + .map_err(anyhow::Error::from)?; + } + + Ok(sigs) } } -/// Lazily opens and reuses a single Keycard session for all keycard signers in one transaction. -pub struct KeycardSessionContext { - pin: String, - wallet: Option, -} - -impl KeycardSessionContext { - pub fn new(pin: impl Into) -> Self { - Self { - pin: pin.into(), - wallet: None, - } - } - - pub fn get_or_connect<'py>( - &'py mut self, - py: Python<'py>, - ) -> pyo3::PyResult<&'py KeycardWallet> { - if self.wallet.is_none() { - python_path::add_python_path(py)?; - let wallet = KeycardWallet::new(py)?; - wallet.connect(py, &self.pin)?; - self.wallet = Some(wallet); - } - Ok(self.wallet.as_ref().unwrap()) - } - - pub fn close(self, py: Python<'_>) { - if let Some(w) = self.wallet { - drop(w.close_session(py)); - } - } -}