Fix ledger nano remainder finder (#1496)

* Fix ledger nano remainder finder * Changelog * Fix comment * Fmt * Nit * Remove label * Reword
iotaledger · Oct 24, 2023 · 474b1ba · 474b1ba
1 parent 02be942
commit 474b1ba
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 36 deletions.
diff --git a/sdk/CHANGELOG.md b/sdk/CHANGELOG.md
@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `Account::claim_outputs()` if an input has less amount than min storage deposit;
 - URLs aren't truncated after the hostname anymore;
+- Ledger nano potentially failing to identify the correct remainder output;
 
 ## 1.1.1 - 2023-10-11
 

diff --git a/sdk/src/client/secret/ledger_nano.rs b/sdk/src/client/secret/ledger_nano.rs
@@ -294,53 +294,46 @@ impl SecretManage for LedgerSecretManager {
                 .prepare_blind_signing(input_bip32_indices, essence_hash)
                 .map_err(Error::from)?;
         } else {
-            // figure out the remainder address and bip32 index (if there is one)
+            // figure out the remainder output and bip32 index (if there is one)
             #[allow(clippy::option_if_let_else)]
-            let (remainder_address, remainder_bip32): (Option<&Address>, LedgerBIP32Index) =
-                match &prepared_transaction.remainder {
-                    Some(a) => {
-                        if let Some(chain) = a.chain {
-                            (
-                                Some(&a.address),
-                                LedgerBIP32Index {
-                                    bip32_change: chain.change.harden().into(),
-                                    bip32_index: chain.address_index.harden().into(),
-                                },
-                            )
-                        } else {
-                            (None, LedgerBIP32Index::default())
-                        }
+            let (remainder_output, remainder_bip32) = match &prepared_transaction.remainder {
+                Some(remainder) => {
+                    if let Some(chain) = remainder.chain {
+                        (
+                            Some(&remainder.output),
+                            LedgerBIP32Index {
+                                bip32_change: chain.change.harden().into(),
+                                bip32_index: chain.address_index.harden().into(),
+                            },
+                        )
+                    } else {
+                        (None, LedgerBIP32Index::default())
                     }
-                    None => (None, LedgerBIP32Index::default()),
-                };
+                }
+                None => (None, LedgerBIP32Index::default()),
+            };
 
             let mut remainder_index = 0u16;
-            if let Some(remainder_address) = remainder_address {
+            if let Some(remainder_output) = remainder_output {
                 match &prepared_transaction.essence {
                     TransactionEssence::Regular(essence) => {
-                        // find the index of the remainder in the essence
-                        // this has to be done because outputs in essences are sorted
-                        // lexically and therefore the remainder is not always the last output.
-                        // The index within the essence and the bip32 index will be validated
-                        // by the hardware wallet.
-                        // The outputs in the essence already are sorted
-                        // at this place, so we can rely on their order and don't have to sort it again.
-                        'essence_outputs: for output in essence.outputs().iter() {
-                            if let Output::Basic(s) = output {
-                                if let Some(address) = s.unlock_conditions().address() {
-                                    if *remainder_address == *address.address() {
-                                        break 'essence_outputs;
-                                    }
-                                }
-                            } else {
+                        // Find the index of the remainder in the transaction because it is not always the last output.
+                        // The index within the transaction and the bip32 index will be validated by the hardware
+                        // wallet.
+                        for output in essence.outputs().iter() {
+                            if !output.is_basic() {
                                 log::debug!("[LEDGER] unsupported output");
                                 return Err(Error::MiscError.into());
                             }
 
+                            if remainder_output == output {
+                                break;
+                            }
+
                             remainder_index += 1;
                         }
 
-                        // was index found?
+                        // Was index found?
                         if remainder_index as usize == essence.outputs().len() {
                             log::debug!("[LEDGER] remainder_index not found");
                             return Err(Error::MiscError.into());
@@ -355,15 +348,15 @@ impl SecretManage for LedgerSecretManager {
                 "[LEDGER] {:?} {:02x?} {} {} {:?}",
                 input_bip32_indices,
                 essence_bytes,
-                remainder_address.is_some(),
+                remainder_output.is_some(),
                 remainder_index,
                 remainder_bip32
             );
             ledger
                 .prepare_signing(
                     input_bip32_indices,
                     essence_bytes,
-                    remainder_address.is_some(),
+                    remainder_output.is_some(),
                     remainder_index,
                     remainder_bip32,
                 )