From dfb5e3eb7ce9a67ad0ec5d645c4bb2c70abbd1c9 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 2 May 2024 13:06:19 -0400 Subject: [PATCH] Update error handling --- cmd/crates/stellar-ledger/src/lib.rs | 58 +++++++++++++++++-------- cmd/crates/stellar-ledger/src/signer.rs | 6 +-- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/cmd/crates/stellar-ledger/src/lib.rs b/cmd/crates/stellar-ledger/src/lib.rs index 047ca1d31..fa5a8337b 100644 --- a/cmd/crates/stellar-ledger/src/lib.rs +++ b/cmd/crates/stellar-ledger/src/lib.rs @@ -60,11 +60,14 @@ pub enum LedgerError { #[error("Error occurred while initializing Ledger HID transport: {0}")] LedgerHidError(#[from] LedgerHIDError), - #[error("Error with ADPU exchange with Ledger device: {0}")] //TODO update this message + #[error("Error with ADPU exchange with Ledger device: {0}")] APDUExchangeError(String), #[error("Error occurred while exchanging with Ledger device: {0}")] LedgerConnectionError(String), + + #[error("Error occurred while parsing BIP32 path: {0}")] + Bip32PathError(String), } pub struct LedgerOptions { @@ -89,7 +92,7 @@ where &self, index: u32, ) -> Result { - let hd_path = bip_path_from_index(index); + let hd_path = bip_path_from_index(index)?; Self::get_public_key_with_display_flag(self, hd_path, false).await } @@ -111,7 +114,7 @@ where /// based on impl from [https://github.com/LedgerHQ/ledger-live/blob/develop/libs/ledgerjs/packages/hw-app-str/src/Str.ts#L166](https://github.com/LedgerHQ/ledger-live/blob/develop/libs/ledgerjs/packages/hw-app-str/src/Str.ts#L166) /// # Errors /// Returns an error if there is an issue with connecting with the device or signing the given tx on the device. Or, if the device has not enabled hash signing - pub async fn sign_transaction_hash( + async fn sign_transaction_hash( &self, hd_path: slip10::BIP32Path, transaction_hash: Vec, @@ -139,8 +142,8 @@ where /// Sign a Stellar transaction with the account on the Ledger device /// # Errors /// Returns an error if there is an issue with connecting with the device or signing the given tx on the device - #[allow(clippy::missing_panics_doc)] // TODO: handle panics/unwraps - pub async fn sign_transaction( + #[allow(clippy::missing_panics_doc)] + async fn sign_transaction( &self, hd_path: slip10::BIP32Path, transaction: Transaction, @@ -152,7 +155,9 @@ where network_id: network_hash, tagged_transaction, }; - let mut signature_payload_as_bytes = signature_payload.to_xdr(Limits::none()).unwrap(); + let mut signature_payload_as_bytes = signature_payload + .to_xdr(Limits::none()) + .expect("tx payload should be able to be written as xdr"); let mut hd_path_to_bytes = hd_path_to_bytes(&hd_path); @@ -230,7 +235,8 @@ where tracing::info!("APDU in: {}", hex::encode(command.serialize())); match self.send_command_to_ledger(command).await { - Ok(value) => Ok(stellar_strkey::ed25519::PublicKey::from_payload(&value).unwrap()), + Ok(value) => Ok(stellar_strkey::ed25519::PublicKey::from_payload(&value) + .expect("payload should be able to be converted into PublicKey")), Err(err) => Err(err), } } @@ -267,7 +273,7 @@ impl Stellar for LedgerSigner { type Init = LedgerOptions; fn new(network_passphrase: &str, options: Option>) -> Self { - let options_unwrapped = options.unwrap(); + let options_unwrapped = options.expect("LedgerSigner should have LedgerOptions passed in"); LedgerSigner { network_passphrase: network_passphrase.to_string(), transport: options_unwrapped.exchange, @@ -282,12 +288,17 @@ impl Stellar for LedgerSigner { fn sign_txn_hash( &self, txn: [u8; 32], - _source_account: &stellar_strkey::Strkey, + source_account: &stellar_strkey::Strkey, ) -> Result { let signature = block_on(self.sign_transaction_hash(self.hd_path.clone(), txn.to_vec())) //TODO: refactor sign_transaction_hash - .unwrap(); // FIXME: handle error + .map_err(|e| { + tracing::error!("Error signing transaction hash with Ledger device: {e}"); + Error::MissingSignerForAddress { + address: source_account.to_string(), + } + })?; - let sig_bytes = signature.try_into().unwrap(); // FIXME: handle error + let sig_bytes = signature.try_into()?; Ok(DecoratedSignature { hint: SignatureHint([0u8; 4]), //FIXME signature: Signature(sig_bytes), @@ -299,9 +310,15 @@ impl Stellar for LedgerSigner { txn: Transaction, _source_account: &stellar_strkey::Strkey, ) -> Result { - let signature = block_on(self.sign_transaction(self.hd_path.clone(), txn.clone())).unwrap(); // FIXME: handle error + let signature = block_on(self.sign_transaction(self.hd_path.clone(), txn.clone())) + .map_err(|e| { + tracing::error!("Error signing transaction with Ledger device: {e}"); + Error::MissingSignerForAddress { + address: "source_account".to_string(), + } + })?; - let sig_bytes = signature.try_into().unwrap(); // FIXME: handle error + let sig_bytes = signature.try_into()?; let decorated_signature = DecoratedSignature { hint: SignatureHint([0u8; 4]), //FIXME signature: Signature(sig_bytes), @@ -309,22 +326,25 @@ impl Stellar for LedgerSigner { Ok(TransactionEnvelope::Tx(TransactionV1Envelope { tx: txn, - signatures: vec![decorated_signature].try_into().unwrap(), //fixme: remove unwrap + signatures: vec![decorated_signature].try_into()?, })) } } -fn bip_path_from_index(index: u32) -> slip10::BIP32Path { +fn bip_path_from_index(index: u32) -> Result { let path = format!("m/44'/148'/{index}'"); - path.parse().unwrap() // this is basically the same thing as slip10::BIP32Path::from_str - - // the device handles this part: https://github.com/AhaLabs/rs-sep5/blob/9d6e3886b4b424dd7b730ec24c865f6fad5d770c/src/seed_phrase.rs#L86 + path.parse().map_err(|e| { + let error_string = format!("Error parsing BIP32 path: {e}"); + LedgerError::Bip32PathError(error_string) + }) } fn hd_path_to_bytes(hd_path: &slip10::BIP32Path) -> Vec { (0..hd_path.depth()) .flat_map(|index| { - let value = *hd_path.index(index).unwrap(); + let value = *hd_path + .index(index) + .expect("should be able to get index of hd path"); value.to_be_bytes() }) .collect::>() diff --git a/cmd/crates/stellar-ledger/src/signer.rs b/cmd/crates/stellar-ledger/src/signer.rs index e403c19ec..80032c23b 100644 --- a/cmd/crates/stellar-ledger/src/signer.rs +++ b/cmd/crates/stellar-ledger/src/signer.rs @@ -5,12 +5,12 @@ use soroban_env_host::xdr::{ TransactionV1Envelope, WriteXdr, }; -use soroban_rpc::Error as RpcError; - #[derive(thiserror::Error, Debug)] pub enum Error { #[error(transparent)] - RpcError(#[from] RpcError), + Xdr(#[from] xdr::Error), + #[error("Error signing transaction {address}")] + MissingSignerForAddress { address: String }, } /// A trait for signing Stellar transactions and Soroban authorization entries