Format amount and expert mode (#90)

* A safer interface to retrieve settings

* Add expert mode check if an unknown token is present in transaction

* Add expert mode support and new tests in zemu

* Update cargo

* update snapshots

* Fix ui and update snapshot

* Improve safety using get method on slices and return proper error

* Clippy happy

* fix fmt

app/Cargo.toml

@@ -9,7 +9,7 @@ test = false
 path = "bin-app/app.rs"
 
 [dependencies]
-# ledger_device_sdk = { git = "https://github.com/Zondax/ledger-device-rust-sdk", rev = "01b9de4dbf67d6e2bf06e3445f1a96c6e7f882d4", optional = true }
+# ledger_device_sdk = { git = "https://github.com/Zondax/ledger-device-rust-sdk", rev = "446bd4c11c328e422e68f056b08ed38952a0853f", optional = true }
 ledger_device_sdk = { version = "1.18.3", optional = true }
 #ledger_device_sdk = { path="../ledger-device-rust-sdk/ledger_device_sdk" , optional = true}
 include_gif = "1.2.0"

app/src/nvm/dkg_keys.rs

@@ -65,15 +65,16 @@ impl DkgKeys {
     }
 
     #[inline(never)]
-    pub fn get_element(&self, index: usize) -> u8 {
+    pub fn get_element(&self, index: usize) -> Option<u8> {
         let buffer_ref: [u8; DKG_KEYS_MAX_SIZE] = unsafe { *DATA.get_mut().get_ref() };
-        buffer_ref[index]
+        buffer_ref.get(index).copied()
     }
 
     #[inline(never)]
-    pub fn get_u16(&self, start_pos: usize) -> usize {
+    pub fn get_u16(&self, start_pos: usize) -> Option<usize> {
         let buffer_ref: [u8; DKG_KEYS_MAX_SIZE] = unsafe { *DATA.get_mut().get_ref() };
-        ((buffer_ref[start_pos] as u16) << 8 | buffer_ref[start_pos + 1] as u16) as usize
+        let bytes = buffer_ref[start_pos..start_pos + 2].try_into().ok()?;
+        Some(u16::from_be_bytes(bytes) as usize)
     }
 
     #[inline(never)]
@@ -139,7 +140,10 @@ impl DkgKeys {
         let mut updated_data: [u8; DKG_KEYS_MAX_SIZE] = unsafe { *DATA.get_mut().get_ref() };
 
         // Convert u16 to big-endian bytes and copy them
-        updated_data[index..index + 2].copy_from_slice(&value.to_be_bytes());
+        updated_data
+            .get_mut(index..index + 2)
+            .map(|slice| slice.copy_from_slice(&value.to_be_bytes()))
+            .ok_or(AppSW::BufferOutOfBounds)?;
 
         unsafe {
             DATA.get_mut().update(&updated_data);
@@ -221,8 +225,13 @@ impl DkgKeys {
         }
 
         // Read where the previous data end up
-        let identities_pos: usize = self.get_u16(IDENTITIES_POS);
-        let identities_len: usize = self.get_u16(identities_pos);
+        let identities_pos: usize = self
+            .get_u16(IDENTITIES_POS)
+            .ok_or(AppSW::InvalidDkgStatus)?;
+
+        let identities_len: usize = self
+            .get_u16(identities_pos)
+            .ok_or(AppSW::InvalidDkgStatus)?;
         let mut pos = identities_pos + 2 + identities_len;
 
         self.set_u16(KEY_PACKAGE_POS, pos as u16)?;

app/src/nvm/settings.rs

@@ -31,26 +31,31 @@ impl Settings {
         return unsafe { DATA.get_ref() };
     }
 
-    pub fn get_element(&self, index: usize) -> u8 {
+    pub fn get_element(&self, index: usize) -> Option<u8> {
         let storage = unsafe { DATA.get_ref() };
         let settings = storage.get_ref();
-        settings[index]
+        settings.get(index).copied()
     }
 
     pub fn set_element(&self, index: usize, value: u8) {
+        if index >= SETTINGS_SIZE {
+            return;
+        }
         let storage = unsafe { DATA.get_mut() };
         let mut updated_data = *storage.get_ref();
         updated_data[index] = value;
         storage.update(&updated_data);
     }
 
     pub fn app_expert_mode(&self) -> bool {
-        self.get_element(EXPERT_MODE_FLAG) == ON_STATE
+        self.get_element(EXPERT_MODE_FLAG)
+            .map(|mode| mode == ON_STATE)
+            .unwrap_or(false)
     }
 
     pub fn toggle_expert_mode(&self) {
         match self.get_element(EXPERT_MODE_FLAG) {
-            OFF_STATE => Settings.set_element(EXPERT_MODE_FLAG, ON_STATE),
+            Some(OFF_STATE) => Settings.set_element(EXPERT_MODE_FLAG, ON_STATE),
             _ => Settings.set_element(EXPERT_MODE_FLAG, OFF_STATE),
         }
     }

app/src/parser/error.rs

@@ -23,6 +23,7 @@ pub enum ParserError {
 
     BufferFull,
     InvalidTokenList,
+    UnknownToken,
 
     UnexpectedError,
 }

app/src/parser/note.rs

@@ -1,12 +1,17 @@
+use alloc::{string::String, vec::Vec};
 use core::{mem::MaybeUninit, ptr::addr_of_mut};
-
 use jubjub::AffinePoint;
 use nom::number::complete::le_u64;
 
+#[cfg(feature = "ledger")]
+use crate::nvm::settings::Settings;
 use crate::{
+    bolos::zlog_stack,
     crypto::{decrypt, read_scalar},
     ironfish::{errors::IronfishError, public_address::PublicAddress},
     parser::AssetIdentifier,
+    token::TokenList,
+    utils::int_format::{token_to_fp_str, u64_to_str},
     FromBytes,
 };
 
@@ -135,3 +140,65 @@ impl Note {
     //        .get_u()
     //}
 }
+
+impl Note {
+    /// Returns the fields to be displayed for this note:
+    /// To: destination of the funds
+    /// Amount: the amount to be send
+    /// asset_id: Only if token is unknown
+    pub fn review_fields(
+        &self,
+        token_list: &TokenList,
+        fields: &mut Vec<(String, String)>,
+    ) -> Result<(), ParserError> {
+        use lexical_core::FormattedSize;
+
+        zlog_stack("Note::review_fields\n");
+        // Format To:
+        let to = String::from("To ");
+        let address = hex::encode(self.owner.public_address());
+        fields.push((to, address));
+
+        let mut buffer = [0; u64::FORMATTED_SIZE_DECIMAL + 2];
+        let asset_id = hex::encode(self.asset_id.as_bytes());
+
+        // Format amount and asset_id
+        if let Some(token) = token_list.token(&asset_id) {
+            let amount_label = String::from("Amount");
+            // value
+            let amount_formatted =
+                token_to_fp_str(self.value, &mut buffer[..], token.decimals as usize)?;
+            let mut amount_formatted = String::from(
+                core::str::from_utf8(amount_formatted).map_err(|_| ParserError::UnexpectedValue)?,
+            );
+            amount_formatted.push(' ');
+            amount_formatted.push_str(token.symbol);
+
+            // push values
+            fields.push((amount_label, amount_formatted));
+        } else {
+            zlog_stack("Note::unknown_token\n");
+
+            #[cfg(feature = "ledger")]
+            if !Settings.app_expert_mode() {
+                return Err(ParserError::UnknownToken);
+            }
+
+            let amount_label = String::from("Raw Amount ");
+            let value_str = u64_to_str(self.value, &mut buffer)?;
+
+            let value_str =
+                core::str::from_utf8(value_str).map_err(|_| ParserError::UnexpectedValue)?;
+
+            // push values
+            fields.push((amount_label, String::from(value_str)));
+
+            // Add asset_id
+            let label = String::from("AssetId ");
+            let asset_id = hex::encode(self.asset_id.as_bytes());
+            fields.push((label, asset_id));
+        }
+
+        Ok(())
+    }
+}

app/src/parser/snapshots/ironfish_dkg__parser__transaction__review_transaction_test__tx_ui.snap

@@ -5,14 +5,13 @@ input_file: src/parser/testvectors/transaction_simple.json
 ---
 [
     "Tx Version": "V1",
-    "To 1": "40fae059d8ee3361b7a08429867254523f937c7654ae6fe7b188c1f0da57e9cd",
-    "Amount(IRON) ": "0.0000004",
-    "Fee(IRON) ": "0.00000001",
-    "To 2": "40fae059d8ee3361b7a08429867254523f937c7654ae6fe7b188c1f0da57e9cd",
+    "To ": "40fae059d8ee3361b7a08429867254523f937c7654ae6fe7b188c1f0da57e9cd",
+    "Amount": "0.0000004 IRON",
+    "To ": "40fae059d8ee3361b7a08429867254523f937c7654ae6fe7b188c1f0da57e9cd",
     "Raw Amount ": "5",
-    "Raw Fee": "1",
-    "To 3": "40fae059d8ee3361b7a08429867254523f937c7654ae6fe7b188c1f0da57e9cd",
-    "Amount(IRON) ": "0.00000001",
-    "Fee(IRON) ": "0.00000001",
+    "AssetId ": "da40fc530a12327a1c1c367b8bd66ada35100501228cdb355b8fb6c40d048a64",
+    "To ": "40fae059d8ee3361b7a08429867254523f937c7654ae6fe7b188c1f0da57e9cd",
+    "Amount": "0.00000001 IRON",
+    "Fee": "0.00000001 IRON",
     "Expiration": "0",
 ]

app/src/parser/transaction.rs

@@ -17,21 +17,18 @@ use crate::{
         constants::{KEY_LENGTH, REDJUBJUB_SIGNATURE_LEN},
         SIGNATURE_HASH_PERSONALIZATION, TRANSACTION_SIGNATURE_VERSION, TX_HASH_LEN,
     },
-    token::{get_token_list, TokenList},
-    utils::int_format::{intstr_to_fpstr_inplace, token_to_fp_str, u64_to_str},
+    token::get_token_list,
+    utils::int_format::intstr_to_fpstr_inplace,
 };
 
-use lexical_core::FormattedSize;
-
 mod burns;
 mod mints;
 mod outputs;
 mod spends;
 
 use self::mints::MintList;
 
-use super::{FromBytes, Note, ObjectList, ParserError, TransactionVersion};
-use crate::utils::int_to_str;
+use super::{FromBytes, ObjectList, ParserError, TransactionVersion};
 pub use burns::Burn;
 pub use mints::Mint;
 pub use outputs::Output;
@@ -58,7 +55,7 @@ impl<'a> FromBytes<'a> for Transaction<'a> {
     fn from_bytes_into(
         input: &'a [u8],
         out: &mut core::mem::MaybeUninit<Self>,
-    ) -> Result<&'a [u8], nom::Err<super::ParserError>> {
+    ) -> Result<&'a [u8], nom::Err<ParserError>> {
         zlog_stack("Transaction::from_bytes_into\n");
         let out = out.as_mut_ptr();
 
@@ -156,9 +153,7 @@ impl<'a> Transaction<'a> {
 
         let token_list = get_token_list()?;
 
-        for (i, output) in self.outputs.iter().enumerate() {
-            let output_number = i + 1;
-
+        for output in self.outputs.iter() {
             // Safe to unwrap because MerkleNote was also parsed in outputs from_bytes impl
             let Ok(merkle_note) = output.note() else {
                 return Err(IronfishError::InvalidData);
@@ -167,16 +162,23 @@ impl<'a> Transaction<'a> {
             // now get the encrypted Note
             let note = merkle_note.decrypt_note_for_spender(ovk)?;
 
-            let owner_value = hex::encode(note.owner.public_address());
-            let mut owner_label = String::from("To ");
-            let output_num_str = int_to_str(output_number as u8);
-            owner_label.push_str(output_num_str.as_str());
-            fields.push((owner_label, owner_value));
-
             // Now process amount and fees
-            self.format_output(&token_list, &note, &mut fields)?;
+            note.review_fields(&token_list, &mut fields)?;
         }
+        // Safe to unwrap, IRON is the oficial token
+        let Some(token) = token_list.toke_by_symbol("IRON") else {
+            return Err(IronfishError::InvalidData);
+        };
+
         let mut buffer = [0; lexical_core::BUFFER_SIZE];
+        // Add fee
+        lexical_core::write(self.fee, &mut buffer[..]);
+        let raw = intstr_to_fpstr_inplace(&mut buffer[..], token.decimals as usize)?;
+        let mut fee = String::from(core::str::from_utf8(raw).unwrap());
+        let fee_label = String::from("Fee");
+        fee.push(' ');
+        fee.push_str(token.symbol);
+        fields.push((fee_label, fee));
 
         // Add expiration
         let raw = lexical_core::write(self.expiration, &mut buffer);
@@ -186,62 +188,6 @@ impl<'a> Transaction<'a> {
         Ok(fields)
     }
 
-    fn format_output(
-        &self,
-        // token_list: &TokenList<'_>,
-        token_list: &TokenList<'_>,
-        note: &Note,
-        fields: &mut Vec<(String, String)>,
-    ) -> Result<(), ParserError> {
-        zlog_stack("Transaction::format_output\n");
-        let mut buffer = [0; u64::FORMATTED_SIZE_DECIMAL + 2];
-        let asset_id = hex::encode(note.asset_id.as_bytes());
-
-        if let Some(token) = token_list.token(&asset_id) {
-            zlog_stack("Transaction::token_found\n");
-            let mut amount_label = String::from("Amount(");
-            amount_label.push_str(token.symbol);
-            amount_label.push_str(") ");
-            // value
-            let amount_formatted =
-                token_to_fp_str(note.value, &mut buffer[..], token.decimals as usize)?;
-            let amount_formatted =
-                core::str::from_utf8(amount_formatted).map_err(|_| ParserError::UnexpectedValue)?;
-
-            // push values
-            fields.push((amount_label, amount_formatted.to_string()));
-
-            buffer.fill(0u8);
-            // Add fee
-            lexical_core::write(self.fee, &mut buffer[..]);
-            let raw = intstr_to_fpstr_inplace(&mut buffer[..], token.decimals as usize)?;
-            let fee = core::str::from_utf8(raw).unwrap();
-            let mut fee_label = String::from("Fee(");
-            fee_label.push_str(token.symbol);
-            fee_label.push_str(") ");
-            fields.push((fee_label, String::from(fee)));
-        } else {
-            zlog_stack("Transaction::unknown_token\n");
-            let amount_label = String::from("Raw Amount ");
-            let value_str = u64_to_str(note.value, &mut buffer)?;
-
-            let value_str =
-                core::str::from_utf8(value_str).map_err(|_| ParserError::UnexpectedValue)?;
-
-            // push values
-            fields.push((amount_label, value_str.to_string()));
-
-            buffer.fill(0u8);
-
-            // Add fee
-            let raw = lexical_core::write(self.fee, &mut buffer);
-            let fee = core::str::from_utf8(raw).unwrap();
-            fields.push(("Raw Fee".to_string(), String::from(fee)));
-        }
-
-        Ok(())
-    }
-
     #[inline(never)]
     pub fn hash(&self) -> [u8; TX_HASH_LEN] {
         use blake2b_simd::Params as Blake2b;

app/src/status.rs

@@ -42,6 +42,7 @@ pub enum AppSW {
     InvalidDkgKeysVersion = 0xB023,
     TooManyParticipants = 0xB024,
     InvalidTxHash = 0xB025,
+    InvalidToken = 0xB026,
     #[cfg(feature = "ledger")]
     WrongApduLength = StatusWords::BadLen as u16,
     Ok = 0x9000,
@@ -87,6 +88,7 @@ impl From<ParserError> for AppSW {
             ParserError::InvalidScalar => AppSW::InvalidScalar,
             ParserError::BufferFull => AppSW::BufferOutOfBounds,
             ParserError::InvalidTokenList => AppSW::InvalidPublicPackage,
+            ParserError::UnknownToken => AppSW::InvalidToken,
         }
     }
 }
@@ -130,6 +132,7 @@ impl From<ParserError> for IronfishError {
             ParserError::BufferFull => IronfishError::InvalidData,
             ParserError::InvalidTokenList => IronfishError::InvalidAssetIdentifier,
             ParserError::UnexpectedError => IronfishError::InvalidData,
+            ParserError::UnknownToken => IronfishError::InvalidData,
         }
     }
 }