diff --git a/cli/src/cli.rs b/cli/src/cli.rs index aaea31b66..17f5cb5a6 100644 --- a/cli/src/cli.rs +++ b/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> { + fn try_pubkey(&self) -> Result { Ok(self.pubkey()) } @@ -1988,7 +1988,7 @@ impl Signer for FaucetKeypair { self.transaction.signatures[0] } - fn try_sign_message(&self, message: &[u8]) -> Result> { + fn try_sign_message(&self, message: &[u8]) -> Result { Ok(self.sign_message(message)) } } diff --git a/client/src/client_error.rs b/client/src/client_error.rs index 8eb6ca743..a9ccaf469 100644 --- a/client/src/client_error.rs +++ b/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 for ClientError { - fn from(err: io::Error) -> ClientError { - ClientError::Io(err) - } -} - -impl From for ClientError { - fn from(err: reqwest::Error) -> ClientError { - ClientError::Reqwest(err) - } -} - -impl From for ClientError { - fn from(err: rpc_request::RpcError) -> ClientError { - ClientError::RpcError(err) - } -} - -impl From for ClientError { - fn from(err: serde_json::error::Error) -> ClientError { - ClientError::SerdeJson(err) - } -} - -impl From for ClientError { - fn from(err: TransactionError) -> ClientError { - ClientError::TransactionError(err) - } -} diff --git a/client/src/rpc_client.rs b/client/src/rpc_client.rs index 1aaa9a5eb..a2954ff33 100644 --- a/client/src/rpc_client.rs +++ b/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(()) } diff --git a/remote-wallet/src/remote_keypair.rs b/remote-wallet/src/remote_keypair.rs index 2b499b9f5..06ebaa173 100644 --- a/remote-wallet/src/remote_keypair.rs +++ b/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> { + fn try_pubkey(&self) -> Result { 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> { + fn try_sign_message(&self, message: &[u8]) -> Result { match &self.wallet_type { RemoteWalletType::Ledger(wallet) => wallet .sign_message(&self.derivation_path, message) diff --git a/remote-wallet/src/remote_wallet.rs b/remote-wallet/src/remote_wallet.rs index 72c04574b..277d8222f 100644 --- a/remote-wallet/src/remote_wallet.rs +++ b/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 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>, diff --git a/sdk/src/message.rs b/sdk/src/message.rs index 8fe4c56c8..f444ca786 100644 --- a/sdk/src/message.rs +++ b/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, + 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 { bincode::serialize(self).unwrap() } diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index 3dd79145a..2d3a9fcf5 100644 --- a/sdk/src/signature.rs +++ b/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>; + fn try_pubkey(&self) -> Result; fn sign_message(&self, message: &[u8]) -> Signature { self.try_sign_message(message).unwrap_or_default() } - fn try_sign_message(&self, message: &[u8]) -> Result>; + fn try_sign_message(&self, message: &[u8]) -> Result; } impl Signer for Keypair { @@ -145,7 +145,7 @@ impl Signer for Keypair { Pubkey::new(self.0.public.as_ref()) } - fn try_pubkey(&self) -> Result> { + fn try_pubkey(&self) -> Result { 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> { + fn try_sign_message(&self, message: &[u8]) -> Result { 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> { + fn try_pubkey(&self) -> Result { Ok(self.pubkey) } - fn try_sign_message(&self, message: &[u8]) -> Result> { + fn try_sign_message(&self, message: &[u8]) -> Result { if self.signature.verify(self.pubkey.as_ref(), message) { Ok(self.signature) } else { diff --git a/sdk/src/signers.rs b/sdk/src/signers.rs index d62baddf9..67b750796 100644 --- a/sdk/src/signers.rs +++ b/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; + fn try_pubkeys(&self) -> Result, SignerError>; fn sign_message(&self, message: &[u8]) -> Vec; + fn try_sign_message(&self, message: &[u8]) -> Result, 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, 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 { self.iter() .map(|keypair| keypair.sign_message(message)) .collect() } + + fn try_sign_message(&self, message: &[u8]) -> Result, 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 Signers for Vec<&T> { #[cfg(test)] mod tests { use super::*; - use std::error; struct Foo; impl Signer for Foo { - fn try_pubkey(&self) -> Result> { + fn try_pubkey(&self) -> Result { Ok(Pubkey::default()) } - fn try_sign_message(&self, _message: &[u8]) -> Result> { + fn try_sign_message(&self, _message: &[u8]) -> Result { Ok(Signature::default()) } } struct Bar; impl Signer for Bar { - fn try_pubkey(&self) -> Result> { + fn try_pubkey(&self) -> Result { Ok(Pubkey::default()) } - fn try_sign_message(&self, _message: &[u8]) -> Result> { + fn try_sign_message(&self, _message: &[u8]) -> Result { Ok(Signature::default()) } } diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index c552bc359..89796f5fa 100644 --- a/sdk/src/transaction.rs +++ b/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(&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(&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 = 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, 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( + &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( + &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 = 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( + &mut self, + keypairs: &T, + positions: Vec, + 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