Add handling for fallible signers (#8367)

automerge

cli/src/cli.rs

@@ -36,7 +36,7 @@ use solana_sdk::{
     native_token::lamports_to_sol,
     program_utils::DecodeError,
     pubkey::Pubkey,
-    signature::{keypair_from_seed, Keypair, Signature, Signer},
+    signature::{keypair_from_seed, Keypair, Signature, Signer, SignerError},
     system_instruction::{self, create_address_with_seed, SystemError, MAX_ADDRESS_SEED_LEN},
     system_transaction,
     transaction::{Transaction, TransactionError},
@@ -1980,7 +1980,7 @@ impl Signer for FaucetKeypair {
         self.transaction.message().account_keys[0]
     }
 
-    fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>> {
+    fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
         Ok(self.pubkey())
     }
 
@@ -1988,7 +1988,7 @@ impl Signer for FaucetKeypair {
         self.transaction.signatures[0]
     }
 
-    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, Box<dyn error::Error>> {
+    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, SignerError> {
         Ok(self.sign_message(message))
     }
 }

client/src/client_error.rs

@@ -1,14 +1,16 @@
 use crate::rpc_request;
-use solana_sdk::transaction::TransactionError;
+use solana_sdk::{signature::SignerError, transaction::TransactionError};
 use std::{fmt, io};
+use thiserror::Error;
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum ClientError {
-    Io(io::Error),
-    Reqwest(reqwest::Error),
-    RpcError(rpc_request::RpcError),
-    SerdeJson(serde_json::error::Error),
-    TransactionError(TransactionError),
+    Io(#[from] io::Error),
+    Reqwest(#[from] reqwest::Error),
+    RpcError(#[from] rpc_request::RpcError),
+    SerdeJson(#[from] serde_json::error::Error),
+    SigningError(#[from] SignerError),
+    TransactionError(#[from] TransactionError),
 }
 
 impl fmt::Display for ClientError {
@@ -16,35 +18,3 @@ impl fmt::Display for ClientError {
         write!(f, "solana client error")
     }
 }
-
-impl std::error::Error for ClientError {}
-
-impl From<io::Error> for ClientError {
-    fn from(err: io::Error) -> ClientError {
-        ClientError::Io(err)
-    }
-}
-
-impl From<reqwest::Error> for ClientError {
-    fn from(err: reqwest::Error) -> ClientError {
-        ClientError::Reqwest(err)
-    }
-}
-
-impl From<rpc_request::RpcError> for ClientError {
-    fn from(err: rpc_request::RpcError) -> ClientError {
-        ClientError::RpcError(err)
-    }
-}
-
-impl From<serde_json::error::Error> for ClientError {
-    fn from(err: serde_json::error::Error) -> ClientError {
-        ClientError::SerdeJson(err)
-    }
-}
-
-impl From<TransactionError> for ClientError {
-    fn from(err: TransactionError) -> ClientError {
-        ClientError::TransactionError(err)
-    }
-}

client/src/rpc_client.rs

@@ -517,13 +517,11 @@ impl RpcClient {
             // Re-sign any failed transactions with a new blockhash and retry
             let (blockhash, _fee_calculator) =
                 self.get_new_blockhash(&transactions_signatures[0].0.message().recent_blockhash)?;
-            transactions = transactions_signatures
-                .into_iter()
-                .map(|(mut transaction, _)| {
-                    transaction.sign(signer_keys, blockhash);
-                    transaction
-                })
-                .collect();
+            transactions = vec![];
+            for (mut transaction, _) in transactions_signatures.into_iter() {
+                transaction.try_sign(signer_keys, blockhash)?;
+                transactions.push(transaction);
+            }
         }
     }
 
@@ -534,7 +532,7 @@ impl RpcClient {
     ) -> Result<(), ClientError> {
         let (blockhash, _fee_calculator) =
             self.get_new_blockhash(&tx.message().recent_blockhash)?;
-        tx.sign(signer_keys, blockhash);
+        tx.try_sign(signer_keys, blockhash)?;
         Ok(())
     }
 

remote-wallet/src/remote_keypair.rs

@@ -6,9 +6,8 @@ use crate::{
 };
 use solana_sdk::{
     pubkey::Pubkey,
-    signature::{Signature, Signer},
+    signature::{Signature, Signer, SignerError},
 };
-use std::error;
 
 pub struct RemoteKeypair {
     pub wallet_type: RemoteWalletType,
@@ -25,7 +24,7 @@ impl RemoteKeypair {
 }
 
 impl Signer for RemoteKeypair {
-    fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>> {
+    fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
         match &self.wallet_type {
             RemoteWalletType::Ledger(wallet) => wallet
                 .get_pubkey(&self.derivation_path)
@@ -33,7 +32,7 @@ impl Signer for RemoteKeypair {
         }
     }
 
-    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, Box<dyn error::Error>> {
+    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, SignerError> {
         match &self.wallet_type {
             RemoteWalletType::Ledger(wallet) => wallet
                 .sign_message(&self.derivation_path, message)

remote-wallet/src/remote_wallet.rs

@@ -1,7 +1,10 @@
 use crate::ledger::{is_valid_ledger, LedgerWallet};
 use log::*;
 use parking_lot::{Mutex, RwLock};
-use solana_sdk::{pubkey::Pubkey, signature::Signature};
+use solana_sdk::{
+    pubkey::Pubkey,
+    signature::{Signature, SignerError},
+};
 use std::{
     fmt,
     str::FromStr,
@@ -47,6 +50,23 @@ pub enum RemoteWalletError {
     UserCancel,
 }
 
+impl From<RemoteWalletError> for SignerError {
+    fn from(err: RemoteWalletError) -> SignerError {
+        match err {
+            RemoteWalletError::Hid(hid_error) => {
+                SignerError::ConnectionError(hid_error.to_string())
+            }
+            RemoteWalletError::DeviceTypeMismatch => SignerError::ConnectionError(err.to_string()),
+            RemoteWalletError::InvalidDevice => SignerError::ConnectionError(err.to_string()),
+            RemoteWalletError::InvalidInput(input) => SignerError::InvalidInput(input),
+            RemoteWalletError::NoDeviceFound => SignerError::NoDeviceFound,
+            RemoteWalletError::Protocol(e) => SignerError::Protocol(e.to_string()),
+            RemoteWalletError::UserCancel => SignerError::UserCancel,
+            _ => SignerError::CustomError(err.to_string()),
+        }
+    }
+}
+
 /// Collection of conntected RemoteWallets
 pub struct RemoteWalletManager {
     usb: Arc<Mutex<hidapi::HidApi>>,

sdk/src/message.rs

@@ -4,7 +4,7 @@ use crate::{
     hash::Hash,
     instruction::{AccountMeta, CompiledInstruction, Instruction},
     pubkey::Pubkey,
-    short_vec,
+    short_vec, system_instruction,
 };
 use itertools::Itertools;
 
@@ -208,6 +208,20 @@ impl Message {
         )
     }
 
+    pub fn new_with_nonce(
+        mut instructions: Vec<Instruction>,
+        payer: Option<&Pubkey>,
+        nonce_account_pubkey: &Pubkey,
+        nonce_authority_pubkey: &Pubkey,
+    ) -> Self {
+        let nonce_ix = system_instruction::advance_nonce_account(
+            &nonce_account_pubkey,
+            &nonce_authority_pubkey,
+        );
+        instructions.insert(0, nonce_ix);
+        Self::new_with_payer(instructions, payer)
+    }
+
     pub fn serialize(&self) -> Vec<u8> {
         bincode::serialize(self).unwrap()
     }

sdk/src/signature.rs

@@ -1,6 +1,6 @@
 //! The `signature` module provides functionality for public, and private keys.
 
-use crate::pubkey::Pubkey;
+use crate::{pubkey::Pubkey, transaction::TransactionError};
 use generic_array::{typenum::U64, GenericArray};
 use hmac::Hmac;
 use rand::{rngs::OsRng, CryptoRng, RngCore};
@@ -132,11 +132,11 @@ pub trait Signer {
     fn pubkey(&self) -> Pubkey {
         self.try_pubkey().unwrap_or_default()
     }
-    fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>>;
+    fn try_pubkey(&self) -> Result<Pubkey, SignerError>;
     fn sign_message(&self, message: &[u8]) -> Signature {
         self.try_sign_message(message).unwrap_or_default()
     }
-    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, Box<dyn error::Error>>;
+    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, SignerError>;
 }
 
 impl Signer for Keypair {
@@ -145,7 +145,7 @@ impl Signer for Keypair {
         Pubkey::new(self.0.public.as_ref())
     }
 
-    fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>> {
+    fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
         Ok(self.pubkey())
     }
 
@@ -153,7 +153,7 @@ impl Signer for Keypair {
         Signature::new(&self.0.sign(message).to_bytes())
     }
 
-    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, Box<dyn error::Error>> {
+    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, SignerError> {
         Ok(self.sign_message(message))
     }
 }
@@ -167,6 +167,41 @@ where
     }
 }
 
+#[derive(Debug, Error, PartialEq)]
+pub enum SignerError {
+    #[error("keypair-pubkey mismatch")]
+    KeypairPubkeyMismatch,
+
+    #[error("not enough signers")]
+    NotEnoughSigners,
+
+    #[error("transaction error")]
+    TransactionError(#[from] TransactionError),
+
+    #[error("custom error: {0}")]
+    CustomError(String),
+
+    // Presigner-specific Errors
+    #[error("presigner error")]
+    PresignerError(#[from] PresignerError),
+
+    // Remote Keypair-specific Errors
+    #[error("connection error: {0}")]
+    ConnectionError(String),
+
+    #[error("invalid input: {0}")]
+    InvalidInput(String),
+
+    #[error("no device found")]
+    NoDeviceFound,
+
+    #[error("device protocol error: {0}")]
+    Protocol(String),
+
+    #[error("operation has been cancelled")]
+    UserCancel,
+}
+
 #[derive(Debug, Default)]
 pub struct Presigner {
     pubkey: Pubkey,
@@ -184,17 +219,17 @@ impl Presigner {
 }
 
 #[derive(Debug, Error, PartialEq)]
-enum PresignerError {
+pub enum PresignerError {
     #[error("pre-generated signature cannot verify data")]
     VerificationFailure,
 }
 
 impl Signer for Presigner {
-    fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>> {
+    fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
         Ok(self.pubkey)
     }
 
-    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, Box<dyn error::Error>> {
+    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, SignerError> {
         if self.signature.verify(self.pubkey.as_ref(), message) {
             Ok(self.signature)
         } else {

sdk/src/signers.rs

@@ -1,11 +1,13 @@
 use crate::{
     pubkey::Pubkey,
-    signature::{Signature, Signer},
+    signature::{Signature, Signer, SignerError},
 };
 
 pub trait Signers {
     fn pubkeys(&self) -> Vec<Pubkey>;
+    fn try_pubkeys(&self) -> Result<Vec<Pubkey>, SignerError>;
     fn sign_message(&self, message: &[u8]) -> Vec<Signature>;
+    fn try_sign_message(&self, message: &[u8]) -> Result<Vec<Signature>, SignerError>;
 }
 
 macro_rules! default_keypairs_impl {
@@ -14,11 +16,27 @@ macro_rules! default_keypairs_impl {
                 self.iter().map(|keypair| keypair.pubkey()).collect()
             }
 
+            fn try_pubkeys(&self) -> Result<Vec<Pubkey>, SignerError> {
+                let mut pubkeys = Vec::new();
+                for keypair in self.iter() {
+                    pubkeys.push(keypair.try_pubkey()?);
+                }
+                Ok(pubkeys)
+            }
+
             fn sign_message(&self, message: &[u8]) -> Vec<Signature> {
                 self.iter()
                     .map(|keypair| keypair.sign_message(message))
                     .collect()
             }
+
+            fn try_sign_message(&self, message: &[u8]) -> Result<Vec<Signature>, SignerError> {
+                let mut signatures = Vec::new();
+                for keypair in self.iter() {
+                    signatures.push(keypair.try_sign_message(message)?);
+                }
+                Ok(signatures)
+            }
     );
 }
 
@@ -57,24 +75,23 @@ impl<T: Signer> Signers for Vec<&T> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::error;
 
     struct Foo;
     impl Signer for Foo {
-        fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>> {
+        fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
             Ok(Pubkey::default())
         }
-        fn try_sign_message(&self, _message: &[u8]) -> Result<Signature, Box<dyn error::Error>> {
+        fn try_sign_message(&self, _message: &[u8]) -> Result<Signature, SignerError> {
             Ok(Signature::default())
         }
     }
 
     struct Bar;
     impl Signer for Bar {
-        fn try_pubkey(&self) -> Result<Pubkey, Box<dyn error::Error>> {
+        fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
             Ok(Pubkey::default())
         }
-        fn try_sign_message(&self, _message: &[u8]) -> Result<Signature, Box<dyn error::Error>> {
+        fn try_sign_message(&self, _message: &[u8]) -> Result<Signature, SignerError> {
             Ok(Signature::default())
         }
     }

sdk/src/transaction.rs

@@ -6,7 +6,7 @@ use crate::{
     message::Message,
     pubkey::Pubkey,
     short_vec,
-    signature::Signature,
+    signature::{Signature, SignerError},
     signers::Signers,
     system_instruction,
 };
@@ -214,23 +214,18 @@ impl Transaction {
 
     /// Check keys and keypair lengths, then sign this transaction.
     pub fn sign<T: Signers>(&mut self, keypairs: &T, recent_blockhash: Hash) {
-        self.partial_sign(keypairs, recent_blockhash);
-
-        assert_eq!(self.is_signed(), true, "not enough keypairs");
+        if let Err(e) = self.try_sign(keypairs, recent_blockhash) {
+            panic!("Transaction::sign failed with error {:?}", e);
+        }
     }
 
     /// Sign using some subset of required keys
     ///  if recent_blockhash is not the same as currently in the transaction,
     ///  clear any prior signatures and update recent_blockhash
     pub fn partial_sign<T: Signers>(&mut self, keypairs: &T, recent_blockhash: Hash) {
-        let positions = self
-            .get_signing_keypair_positions(&keypairs.pubkeys())
-            .expect("account_keys doesn't contain num_required_signatures keys");
-        let positions: Vec<usize> = positions
-            .iter()
-            .map(|pos| pos.expect("keypair-pubkey mismatch"))
-            .collect();
-        self.partial_sign_unchecked(keypairs, positions, recent_blockhash)
+        if let Err(e) = self.try_partial_sign(keypairs, recent_blockhash) {
+            panic!("Transaction::partial_sign failed with error {:?}", e);
+        }
     }
 
     /// Sign the transaction and place the signatures in their associated positions in `signatures`
@@ -241,6 +236,55 @@ impl Transaction {
         positions: Vec<usize>,
         recent_blockhash: Hash,
     ) {
+        if let Err(e) = self.try_partial_sign_unchecked(keypairs, positions, recent_blockhash) {
+            panic!(
+                "Transaction::partial_sign_unchecked failed with error {:?}",
+                e
+            );
+        }
+    }
+
+    /// Check keys and keypair lengths, then sign this transaction, returning any signing errors
+    /// encountered
+    pub fn try_sign<T: Signers>(
+        &mut self,
+        keypairs: &T,
+        recent_blockhash: Hash,
+    ) -> result::Result<(), SignerError> {
+        self.try_partial_sign(keypairs, recent_blockhash)?;
+
+        if !self.is_signed() {
+            Err(SignerError::NotEnoughSigners)
+        } else {
+            Ok(())
+        }
+    }
+
+    ///  Sign using some subset of required keys, returning any signing errors encountered. If
+    ///  recent_blockhash is not the same as currently in the transaction, clear any prior
+    ///  signatures and update recent_blockhash
+    pub fn try_partial_sign<T: Signers>(
+        &mut self,
+        keypairs: &T,
+        recent_blockhash: Hash,
+    ) -> result::Result<(), SignerError> {
+        let positions = self.get_signing_keypair_positions(&keypairs.pubkeys())?;
+        if positions.iter().any(|pos| pos.is_none()) {
+            return Err(SignerError::KeypairPubkeyMismatch);
+        }
+        let positions: Vec<usize> = positions.iter().map(|pos| pos.unwrap()).collect();
+        self.try_partial_sign_unchecked(keypairs, positions, recent_blockhash)
+    }
+
+    /// Sign the transaction, returning any signing errors encountered, and place the
+    /// signatures in their associated positions in `signatures` without checking that the
+    /// positions are correct.
+    pub fn try_partial_sign_unchecked<T: Signers>(
+        &mut self,
+        keypairs: &T,
+        positions: Vec<usize>,
+        recent_blockhash: Hash,
+    ) -> result::Result<(), SignerError> {
         // if you change the blockhash, you're re-signing...
         if recent_blockhash != self.message.recent_blockhash {
             self.message.recent_blockhash = recent_blockhash;
@@ -249,10 +293,11 @@ impl Transaction {
                 .for_each(|signature| *signature = Signature::default());
         }
 
-        let signatures = keypairs.sign_message(&self.message_data());
+        let signatures = keypairs.try_sign_message(&self.message_data())?;
         for i in 0..positions.len() {
             self.signatures[positions[i]] = signatures[i];
         }
+        Ok(())
     }
 
     /// Verify the transaction