More offset key fixes as per review

applications/minotari_console_wallet/src/init/mod.rs

@@ -842,11 +842,13 @@ pub fn prompt_wallet_type(
                     print!("Scanning for connected Ledger hardware device... ");
                     match get_transport() {
                         Ok(hid) => {
-                            drop(hid); // Need to release it before we reuse it.
                             println!("Device found.");
                             let account = prompt_ledger_account().expect("An account value");
                             let ledger = LedgerWallet::new(account, None);
-                            match ledger.build_command(Instruction::GetPublicKey, vec![]).execute() {
+                            match ledger
+                                .build_command(Instruction::GetPublicKey, vec![])
+                                .execute_with_transport(&hid)
+                            {
                                 Ok(result) => {
                                     debug!(target: LOG_TARGET, "result length: {}, data: {:?}", result.data().len(), result.data());
                                     if result.data().len() < 33 {

applications/minotari_ledger_wallet/wallet/src/handlers/get_public_key.rs

@@ -4,9 +4,7 @@
 use ledger_device_sdk::io::Comm;
 use tari_crypto::{keys::PublicKey, ristretto::RistrettoPublicKey, tari_utilities::ByteArray};
 
-use crate::{utils::derive_from_bip32_key, AppSW, RESPONSE_VERSION};
-
-const STATIC_INDEX: u64 = 42;
+use crate::{utils::derive_from_bip32_key, AppSW, KeyType, RESPONSE_VERSION, STATIC_ALPHA_INDEX};
 
 pub fn handler_get_public_key(comm: &mut Comm) -> Result<(), AppSW> {
     let data = comm.get_data().map_err(|_| AppSW::WrongApduLength)?;
@@ -15,7 +13,7 @@ pub fn handler_get_public_key(comm: &mut Comm) -> Result<(), AppSW> {
     account_bytes.clone_from_slice(&data[0..8]);
     let account = u64::from_le_bytes(account_bytes);
 
-    let pk = match derive_from_bip32_key(account, STATIC_INDEX) {
+    let pk = match derive_from_bip32_key(account, STATIC_ALPHA_INDEX, KeyType::Alpha) {
         Ok(k) => RistrettoPublicKey::from_secret_key(&k),
         Err(e) => return Err(e),
     };

applications/minotari_ledger_wallet/wallet/src/handlers/get_script_offset.rs

@@ -8,15 +8,16 @@ use crate::{
     alloc::string::ToString,
     utils::{derive_from_bip32_key, get_key_from_canonical_bytes, mask_a},
     AppSW,
+    KeyType,
     RESPONSE_VERSION,
+    STATIC_ALPHA_INDEX,
 };
 
 pub struct ScriptOffsetCtx {
     total_sender_offset_private_key: RistrettoSecretKey,
     total_script_private_key: RistrettoSecretKey,
     account: u64,
-    total_offset_keys: u64,
-    total_script_keys: u64,
+    total_offset_indexes: u64,
     total_commitment_keys: u64,
 }
 
@@ -27,8 +28,7 @@ impl ScriptOffsetCtx {
             total_sender_offset_private_key: RistrettoSecretKey::default(),
             total_script_private_key: RistrettoSecretKey::default(),
             account: 0,
-            total_offset_keys: 0,
-            total_script_keys: 0,
+            total_offset_indexes: 0,
             total_commitment_keys: 0,
         }
     }
@@ -38,8 +38,7 @@ impl ScriptOffsetCtx {
         self.total_sender_offset_private_key = RistrettoSecretKey::default();
         self.total_script_private_key = RistrettoSecretKey::default();
         self.account = 0;
-        self.total_offset_keys = 0;
-        self.total_script_keys = 0;
+        self.total_offset_indexes = 0;
         self.total_commitment_keys = 0;
     }
 }
@@ -51,11 +50,7 @@ fn read_instructions(offset_ctx: &mut ScriptOffsetCtx, data: &[u8]) {
 
     let mut total_offset_keys = [0u8; 8];
     total_offset_keys.clone_from_slice(&data[24..32]);
-    offset_ctx.total_offset_keys = u64::from_le_bytes(total_offset_keys);
-
-    let mut total_script_keys = [0u8; 8];
-    total_script_keys.clone_from_slice(&data[8..16]);
-    offset_ctx.total_script_keys = u64::from_le_bytes(total_script_keys);
+    offset_ctx.total_offset_indexes = u64::from_le_bytes(total_offset_keys);
 
     let mut total_commitment_keys = [0u8; 8];
     total_commitment_keys.clone_from_slice(&data[16..24]);
@@ -75,22 +70,26 @@ pub fn handler_get_script_offset(
         read_instructions(offset_ctx, data);
     }
 
-    if (1..offset_ctx.total_offset_keys).contains(&(chunk as u64)) {
+    if chunk == 1 {
+        // The sum of managed private keys
         let k = get_key_from_canonical_bytes(&data[0..32])?;
-        offset_ctx.total_sender_offset_private_key = &offset_ctx.total_sender_offset_private_key + &k;
+        offset_ctx.total_script_private_key = &offset_ctx.total_script_private_key + k;
     }
 
-    if (offset_ctx.total_offset_keys..offset_ctx.total_script_keys).contains(&(chunk as u64)) {
-        let k = get_key_from_canonical_bytes(&data[0..32])?;
-        offset_ctx.total_script_private_key = &offset_ctx.total_script_private_key + &k;
-    }
-
-    if (offset_ctx.total_script_keys..offset_ctx.total_commitment_keys).contains(&(chunk as u64)) {
+    let payload_offset = 2;
+    let end_offset_indexes = payload_offset + offset_ctx.total_offset_indexes;
+    if (payload_offset..end_offset_indexes).contains(&(chunk as u64)) {
         let mut index_bytes = [0u8; 8];
-        index_bytes.clone_from_slice(&data[32..40]);
+        index_bytes.clone_from_slice(&data[0..8]);
         let index = u64::from_le_bytes(index_bytes);
 
-        let alpha = derive_from_bip32_key(offset_ctx.account, index)?;
+        let offset = derive_from_bip32_key(offset_ctx.account, index, KeyType::ScriptOffset)?;
+        offset_ctx.total_offset_indexes = &offset_ctx.total_offset_indexes + &offset;
+    }
+
+    let end_commitment_keys = end_offset_indexes + offset_ctx.total_commitment_keys;
+    if (end_offset_indexes..end_commitment_keys).contains(&(chunk as u64)) {
+        let alpha = derive_from_bip32_key(offset_ctx.account, STATIC_ALPHA_INDEX, KeyType::Alpha)?;
         let commitment = get_key_from_canonical_bytes(&data[0..32])?;
         let k = mask_a(alpha, commitment)?;
 

applications/minotari_ledger_wallet/wallet/src/handlers/get_script_signature.rs

@@ -13,11 +13,11 @@ use crate::{
     alloc::string::ToString,
     utils::{derive_from_bip32_key, get_key_from_canonical_bytes, mask_a},
     AppSW,
+    KeyType,
     RESPONSE_VERSION,
+    STATIC_ALPHA_INDEX,
 };
 
-const STATIC_INDEX: u64 = 42;
-
 const MAX_TRANSACTION_LEN: usize = 312;
 pub struct ScriptSignatureCtx {
     payload: [u8; MAX_TRANSACTION_LEN],
@@ -74,7 +74,7 @@ pub fn handler_get_script_signature(
         return Ok(());
     }
 
-    let alpha = derive_from_bip32_key(signer_ctx.account, STATIC_INDEX)?;
+    let alpha = derive_from_bip32_key(signer_ctx.account, STATIC_ALPHA_INDEX, KeyType::Alpha)?;
     let commitment = get_key_from_canonical_bytes(&signer_ctx.payload[8..40])?;
     let script_private_key = mask_a(alpha, commitment)?;
 

applications/minotari_ledger_wallet/wallet/src/main.rs

@@ -107,6 +107,25 @@ pub enum Instruction {
 }
 
 const P2_MORE: u8 = 0x01;
+const STATIC_ALPHA_INDEX: u64 = 42;
+
+pub enum KeyType {
+    Alpha,
+    Nonce,
+    Recovery,
+    ScriptOffset,
+}
+
+impl KeyType {
+    fn to_byte(&self) -> u8 {
+        match self {
+            Self::Alpha => 1,
+            Self::Nonce => 2,
+            Self::Recovery => 3,
+            Self::ScriptOffset => 4,
+        }
+    }
+}
 
 impl TryFrom<ApduHeader> for Instruction {
     type Error = AppSW;

applications/minotari_ledger_wallet/wallet/src/utils.rs

@@ -22,6 +22,7 @@ use zeroize::Zeroizing;
 use crate::{
     alloc::string::{String, ToString},
     AppSW,
+    KeyType,
     BIP32_COIN_TYPE,
 };
 
@@ -229,17 +230,24 @@ pub fn mask_a(alpha: RistrettoSecretKey, commitment: RistrettoSecretKey) -> Resu
     Ok(private_key + alpha)
 }
 
-pub fn derive_from_bip32_key(u64_account: u64, u64_index: u64) -> Result<RistrettoSecretKey, AppSW> {
+pub fn derive_from_bip32_key(
+    u64_account: u64,
+    u64_index: u64,
+    u64_key_type: KeyType,
+) -> Result<RistrettoSecretKey, AppSW> {
     let account = u64_to_string(u64_account);
     let index = u64_to_string(u64_index);
+    let key_type = u64_to_string(u64_key_type.to_byte() as u64);
 
     let mut bip32_path = "m/44'/".to_string();
     bip32_path.push_str(&BIP32_COIN_TYPE.to_string());
     bip32_path.push_str(&"'/");
     bip32_path.push_str(&account);
     bip32_path.push_str(&"'/0/");
     bip32_path.push_str(&index);
-    let path: [u32; 5] = make_bip32_path(bip32_path.as_bytes());
+    bip32_path.push_str(&"'/");
+    bip32_path.push_str(&key_type);
+    let path: [u32; 6] = make_bip32_path(bip32_path.as_bytes());
 
     match get_raw_key(&path) {
         Ok(val) => get_key_from_uniform_bytes(&val.as_ref()),

base_layer/core/src/transactions/key_manager/inner.rs

@@ -682,22 +682,22 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
         script_key_ids: &[TariKeyId],
         sender_offset_key_ids: &[TariKeyId],
     ) -> Result<PrivateKey, TransactionError> {
-        let mut sender_offset_keys = vec![];
-        for script_key_id in sender_offset_key_ids {
-            sender_offset_keys.push(self.get_private_key(script_key_id).await?);
-        }
-
-        let mut managed_script_keys: Vec<PrivateKey> = vec![];
+        debug!(target: LOG_TARGET, "BRIAN HERE");
+        let mut total_script_private_key = PrivateKey::default();
         let mut derived_key_commitments = vec![];
         for script_key_id in script_key_ids {
             match script_key_id {
+                KeyId::Imported { .. } | KeyId::Managed { .. } | KeyId::Zero => {
+                    total_script_private_key = total_script_private_key + self.get_private_key(script_key_id).await?
+                },
                 KeyId::Derived {
                     branch,
                     label: _,
                     index,
                 } => {
                     if let WalletType::Software(_, _) = self.wallet_type {
-                        managed_script_keys.push(self.get_private_key(script_key_id).await?);
+                        total_script_private_key =
+                            total_script_private_key + self.get_private_key(script_key_id).await?;
                         continue;
                     }
 
@@ -710,39 +710,54 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
                     let branch_key = km
                         .get_private_key(*index)
                         .map_err(|e| TransactionError::KeyManagerError(e.to_string()))?;
-                    derived_key_commitments.push((branch_key, index));
-                },
-                KeyId::Imported { .. } | KeyId::Managed { .. } | KeyId::Zero => {
-                    managed_script_keys.push(self.get_private_key(script_key_id).await?)
+                    derived_key_commitments.push(branch_key);
                 },
             }
-            managed_script_keys.push(self.get_private_key(script_key_id).await?);
         }
 
         match &self.wallet_type {
+            WalletType::Software(_, _) => {
+                debug!(target: LOG_TARGET, "SOFTWARE WALLET");
+                let mut total_sender_offset_private_key = PrivateKey::default();
+                for sender_offset_key_id in sender_offset_key_ids {
+                    total_sender_offset_private_key =
+                        total_sender_offset_private_key + self.get_private_key(sender_offset_key_id).await?;
+                }
+                let script_offset = total_script_private_key - total_sender_offset_private_key;
+                Ok(script_offset)
+            },
             WalletType::Ledger(ledger) => {
-                let num_script_keys = managed_script_keys.len() as u64;
+                debug!(target: LOG_TARGET, "HARDWARE WALLET");
+                let mut sender_offset_indexes = vec![];
+                for sender_offset_key_id in sender_offset_key_ids {
+                    match sender_offset_key_id {
+                        TariKeyId::Managed { branch: _, index } |
+                        TariKeyId::Derived {
+                            branch: _,
+                            label: _,
+                            index,
+                        } => {
+                            sender_offset_indexes.push(index);
+                        },
+                        TariKeyId::Imported { .. } | TariKeyId::Zero => {},
+                    }
+                }
+
                 let num_commitments = derived_key_commitments.len() as u64;
-                let num_offset_key = sender_offset_keys.len() as u64;
+                let num_offset_key = sender_offset_indexes.len() as u64;
 
                 let mut instructions = num_offset_key.to_le_bytes().to_vec();
-                instructions.clone_from_slice(&num_script_keys.to_le_bytes());
                 instructions.clone_from_slice(&num_commitments.to_le_bytes());
 
                 let mut data: Vec<Vec<u8>> = vec![instructions.to_vec()];
-                for sender_offset_key in sender_offset_keys {
-                    data.push(sender_offset_key.to_vec());
-                }
-                for managed_script_key in managed_script_keys {
-                    // Managed keys will just be the key in the payload
-                    data.push(managed_script_key.to_vec());
+                data.push(total_script_private_key.to_vec());
+
+                for sender_offset_index in sender_offset_indexes {
+                    data.push(sender_offset_index.to_le_bytes().to_vec());
                 }
-                for (derived_key_commitment, index) in derived_key_commitments {
-                    // Derived keys will have the commitment and their index in the payload
-                    let mut derived = derived_key_commitment.to_vec();
-                    derived.copy_from_slice(&index.to_le_bytes());
 
-                    data.push(derived);
+                for derived_key_commitment in derived_key_commitments {
+                    data.push(derived_key_commitment.to_vec());
                 }
 
                 let commands = ledger.chunk_command(Instruction::GetScriptOffset, data);
@@ -774,18 +789,6 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
 
                 Err(LedgerDeviceError::Instruction("GetScriptOffset failed to process correctly".to_string()).into())
             },
-            WalletType::Software(_, _) => {
-                let mut total_sender_offset_private_key = PrivateKey::default();
-                for sender_offset_key_id in sender_offset_keys {
-                    total_sender_offset_private_key = total_sender_offset_private_key + sender_offset_key_id;
-                }
-                let mut total_script_private_key = PrivateKey::default();
-                for script_key_id in managed_script_keys {
-                    total_script_private_key = total_script_private_key + script_key_id;
-                }
-                let script_offset = total_script_private_key - total_sender_offset_private_key;
-                Ok(script_offset)
-            },
         }
     }
 

base_layer/wallet/src/wallet.rs

@@ -773,8 +773,8 @@ pub fn read_or_create_wallet_type<T: WalletBackend + 'static>(
     match (db_wallet_type, wallet_type) {
         (None, None) => {
             // this is most likely an older wallet pre ledger support, lets put it in software
-            let wallet_type = WalletType::Software;
-            db.set_wallet_type(wallet_type)?;
+            let wallet_type = WalletType::default();
+            db.set_wallet_type(wallet_type.clone())?;
             Ok(wallet_type)
         },
         (None, Some(t)) => {