adjust signer logic workflow

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<crate::signing::AccountSigner> {
-        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<crate::signing::AccountSigner> {
-        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 {

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<HashType, ExecutionFailureKind> {
-        let from_signer = from_mention.to_signer(self.0).map_err(|e| {
-            ExecutionFailureKind::KeycardError(pyo3::PyErr::new::<PyRuntimeError, _>(e.to_string()))
-        })?;
-        let to_signer = to_mention.to_recipient_signer(self.0).map_err(|e| {
-            ExecutionFailureKind::KeycardError(pyo3::PyErr::new::<PyRuntimeError, _>(e.to_string()))
-        })?;
-
-        let account_ids = vec![from, to];
-        let signing_ids: Vec<AccountId> = 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::<PyRuntimeError, _>(
+                    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::<PyRuntimeError, _>(
@@ -67,30 +63,11 @@ impl NativeTokenTransfer<'_> {
             String::new()
         };
 
-        let witness_set = pyo3::Python::with_gil(|py| -> pyo3::PyResult<WitnessSet> {
-            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::<PyRuntimeError, _>(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::<PyRuntimeError, _>(e.to_string()))?;
-
-            let sigs_and_keys = match to_signer
-                .sign(self.0, &mut ctx, py, &hash)
-                .transpose()
-                .map_err(|e| pyo3::PyErr::new::<PyRuntimeError, _>(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::<PyRuntimeError, _>(e.to_string()))
-        })?;
+        let mut groups = SigningGroups::new();
+        groups
+            .add_sender(account_mention, from, self.0)
+            .map_err(|e| {
+                ExecutionFailureKind::KeycardError(pyo3::PyErr::new::<PyRuntimeError, _>(
+                    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::<PyRuntimeError, _>(
@@ -136,21 +118,11 @@ impl NativeTokenTransfer<'_> {
             String::new()
         };
 
-        let witness_set = pyo3::Python::with_gil(|py| -> pyo3::PyResult<WitnessSet> {
-            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::<PyRuntimeError, _>(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::<PyRuntimeError, _>(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

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<Result<(Signature, PublicKey)>> {
-        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<AccountId> {
+        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<Vec<(Signature, PublicKey)>> {
+        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<KeycardWallet>,
-}
-
-impl KeycardSessionContext {
-    pub fn new(pin: impl Into<String>) -> 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));
-        }
-    }
-}