Update error handling

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<T: Exchange> {
@@ -89,7 +92,7 @@ where
         &self,
         index: u32,
     ) -> Result<stellar_strkey::ed25519::PublicKey, LedgerError> {
-        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<u8>,
@@ -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<T: Exchange> Stellar for LedgerSigner<T> {
     type Init = LedgerOptions<T>;
 
     fn new(network_passphrase: &str, options: Option<LedgerOptions<T>>) -> 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<T: Exchange> Stellar for LedgerSigner<T> {
     fn sign_txn_hash(
         &self,
         txn: [u8; 32],
-        _source_account: &stellar_strkey::Strkey,
+        source_account: &stellar_strkey::Strkey,
     ) -> Result<DecoratedSignature, Error> {
         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,32 +310,41 @@ impl<T: Exchange> Stellar for LedgerSigner<T> {
         txn: Transaction,
         _source_account: &stellar_strkey::Strkey,
     ) -> Result<TransactionEnvelope, Error> {
-        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),
         };
 
         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<slip10::BIP32Path, LedgerError> {
     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<u8> {
     (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::<Vec<u8>>()

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