Update after PR review

LedgerHQ · Jan 14, 2025 · 1487624 · 1487624
1 parent 3c9bf0a
commit 1487624
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 91 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,11 +1,11 @@
 [package]
 name = "near"
-version = "2.3.3"
+version = "2.3.4"
 authors = ["dj8yf0μl", "polyprogrammist"]
 edition = "2021"
 
 [dependencies]
-ledger_device_sdk = "1.19.1"
+ledger_device_sdk = "1.19.2"
 include_gif = "1.2.0"
 hex = { version = "0.4.3", default-features = false, features = ["serde"] }
 bs58 = { version = "0.5.0", default-features = false }

diff --git a/src/handlers/sign_tx.rs b/src/handlers/sign_tx.rs
@@ -80,82 +80,74 @@ pub fn handler(mut stream: SingleTxStream<'_>) -> Result<Signature, AppSW> {
 
 use ledger_device_sdk::libcall::swap::CreateTxParams;
 
-pub fn handler_swap(
+/// Sign handler for the swap transaction
+/// This handler is called when the user wants to sign a swap transaction
+/// The swap transaction is a transfer transaction with a specific amount and destination address
+/// The handler checks the transaction parameters and signs the transaction
+pub fn swap_handler(
     mut stream: SingleTxStream<'_>,
     tx_params: &CreateTxParams,
 ) -> Result<Signature, AppSW> {
-    ledger_device_sdk::testing::debug_print("sign_tx.rs: handler_swap()\n");
+    ledger_device_sdk::testing::debug_print("sign_tx.rs: swap_handler()\n");
 
     let path = <crypto::PathBip32 as BorshDeserialize>::deserialize_reader(&mut stream)
         .map_err(|_| AppSW::Bip32PathParsingFail)?;
 
     ledger_device_sdk::testing::debug_print("sign_tx.rs: path computed\n");
 
+    // Get the public key from the transaction
     let mut stream = HashingStream::new(stream)?;
-
-    ledger_device_sdk::testing::debug_print("sign_tx.rs: hashing stream instantiated\n");
-
     let mut tx_prefix = parsing::types::transaction::prefix::Prefix::new();
-
     tx_prefix
         .deserialize_reader_in_place(&mut stream)
         .map_err(|_err| AppSW::TxParsingFail)?;
-
     let tx_public_key = match PublicKeyBe::try_from(tx_prefix.public_key) {
         Ok(tx_public_key) => tx_public_key,
         Err(_) => return Err(AppSW::PublicKeyMismatch),
     };
 
-    let matching_private_key = {
+    // Derive the public key from the path and compare it with the transaction public key
+    let dpath_public_key = {
         let pk = ledger_device_sdk::ecc::Ed25519::derive_from_path_slip10(&path.0)
             .public_key()
             .map_err(|_| AppSW::KeyDeriveFail)?;
         PublicKeyBe::from_little_endian(pk)
     };
 
-    if tx_public_key != matching_private_key {
+    if tx_public_key != dpath_public_key {
         return Err(AppSW::PublicKeyMismatch);
     }
 
-    match tx_prefix.number_of_actions {
-        1 => {
-            let action = crate::parsing::types::Action::deserialize_reader(&mut stream)
-                .map_err(|_err| AppSW::TxParsingFail)?;
-
-            match action {
-                crate::parsing::types::Action::Transfer => {
-                    let transfer = crate::parsing::types::Transfer::deserialize_reader(&mut stream)
-                        .map_err(|_err| AppSW::TxParsingFail)?;
-
-                    let amount_match = near_token::NearToken::from_yoctonear(u128::from_be_bytes(
-                        tx_params.amount,
-                    )) == transfer.deposit;
-
-                    if !amount_match {
-                        ledger_device_sdk::testing::debug_print(
-                            "sign_tx.rs: amounts do not not match\n",
-                        );
-                        return Err(AppSW::TxSignFail);
-                    }
-
-                    let dest_address_match = tx_prefix.receiver_id.as_str()
-                        == core::str::from_utf8(
-                            tx_params.dest_address[..tx_params.dest_address_len].as_ref(),
-                        )
-                        .unwrap();
-
-                    if !dest_address_match {
-                        ledger_device_sdk::testing::debug_print(
-                            "sign_tx.rs: receiver_id does not match with dest_address\n",
-                        );
-                        return Err(AppSW::TxSignFail);
-                    }
-
-                    finalize_sign::end(stream, &path)
-                }
-                _ => Err(AppSW::TxSignFail),
-            }
-        }
-        _ => Err(AppSW::TxSignFail),
+    // Check nb of actions (shall be == 1 == Transfer in swap context)
+    if tx_prefix.number_of_actions != 1 {
+        return Err(AppSW::TxSignFail);
+    }
+    let action = crate::parsing::types::Action::deserialize_reader(&mut stream)
+        .map_err(|_err| AppSW::TxParsingFail)?;
+    if action != crate::parsing::types::Action::Transfer {
+        return Err(AppSW::TxSignFail);
+    }
+
+    // Check the tx parameters match with the ones previously validated in Exchange app (tx_params)
+    let transfer = crate::parsing::types::Transfer::deserialize_reader(&mut stream)
+        .map_err(|_err| AppSW::TxParsingFail)?;
+
+    let amount_match = near_token::NearToken::from_yoctonear(u128::from_be_bytes(tx_params.amount))
+        == transfer.deposit;
+    if !amount_match {
+        ledger_device_sdk::testing::debug_print("sign_tx.rs: amounts do not not match\n");
+        return Err(AppSW::TxSignFail);
     }
+
+    let dest_address_match = tx_prefix.receiver_id.as_str()
+        == core::str::from_utf8(tx_params.dest_address[..tx_params.dest_address_len].as_ref())
+            .unwrap();
+    if !dest_address_match {
+        ledger_device_sdk::testing::debug_print(
+            "sign_tx.rs: receiver_id does not match with dest_address\n",
+        );
+        return Err(AppSW::TxSignFail);
+    }
+
+    finalize_sign::end(stream, &path)
 }
diff --git a/src/parsing/types/common/action/mod.rs b/src/parsing/types/common/action/mod.rs
@@ -16,6 +16,7 @@ pub mod function_call;
 pub mod stake;
 pub mod transfer;
 
+#[derive(PartialEq)]
 pub enum Action {
     CreateAccount,
     DeployContract,

diff --git a/src/swap.rs b/src/swap.rs
@@ -1,12 +1,13 @@
 use crate::utils::crypto::{PathBip32, PublicKeyBe};
+use fmt_buffer::Buffer;
 use near_token::{NearToken, TokenBuffer};
 
 use ledger_device_sdk::{
     ecc,
     io::Comm,
     libcall::{
         self,
-        swap::{self, CreateTxParams},
+        swap::{self, CheckAddressParams, CreateTxParams, PrintableAmountParams},
     },
     testing::debug_print,
 };
@@ -21,42 +22,22 @@ pub fn swap_main(arg0: u32) {
     match cmd {
         libcall::LibCallCommand::SwapCheckAddress => {
             let mut params = swap::get_check_address_params(arg0);
-
-            let mut res = 0i32;
-            match PathBip32::parse(&params.dpath[..params.dpath_len * 4]) {
-                Ok(path) => match ecc::Ed25519::derive_from_path_slip10(&path.0).public_key() {
-                    Ok(pk) => {
-                        let pk = PublicKeyBe::from_little_endian(pk);
-
-                        let mut buf = [0u8; 64];
-                        let address = pk.display_str_hex(&mut buf);
-                        let ref_address =
-                            core::str::from_utf8(&params.ref_address[..params.ref_address_len]);
-
-                        if address.eq(ref_address.unwrap()) {
-                            res = 1i32;
-                        }
-                    }
-                    Err(_) => {
-                        debug_print("Public key derivation failure\n");
-                    }
-                },
-                Err(_) => {
-                    debug_print("Derivation path failure\n");
+            let res = match check_address(&params) {
+                Ok(_) => 1,
+                Err(err) => {
+                    debug_print(err);
+                    0
                 }
-            }
+            };
             swap::swap_return(swap::SwapResult::CheckAddressResult(&mut params, res));
         }
         libcall::LibCallCommand::SwapGetPrintableAmount => {
             let mut params = swap::get_printable_amount_params(arg0);
-
-            let amount = u128::from_be_bytes(params.amount);
-            let near_token = NearToken::from_yoctonear(amount);
-            let mut near_token_buffer = TokenBuffer::new();
-            near_token.display_as_buffer(&mut near_token_buffer);
-            let s = near_token_buffer.as_str();
-
-            swap::swap_return(swap::SwapResult::PrintableAmountResult(&mut params, s));
+            let mut s = get_printable_amount(&params);
+            swap::swap_return(swap::SwapResult::PrintableAmountResult(
+                &mut params,
+                s.as_str(),
+            ));
         }
         libcall::LibCallCommand::SwapSignTransaction => {
             let mut params = swap::sign_tx_params(arg0);
@@ -72,14 +53,14 @@ pub fn swap_main(arg0: u32) {
 
                     debug_print("APDU received\n");
 
-                    handle_apdu(&mut comm, ins, &mut params);
+                    swap_handle_apdu(&mut comm, ins, &mut params);
                 }
             }
         }
     }
 }
 
-fn handle_apdu(comm: &mut Comm, ins: super::Instruction, tx_params: &mut CreateTxParams) {
+fn swap_handle_apdu(comm: &mut Comm, ins: super::Instruction, tx_params: &mut CreateTxParams) {
     match ins {
         super::Instruction::SignTx {
             is_last_chunk,
@@ -89,7 +70,7 @@ fn handle_apdu(comm: &mut Comm, ins: super::Instruction, tx_params: &mut CreateT
             let stream = SingleTxStream::new(comm, is_last_chunk, sign_mode);
             match sign_mode {
                 super::SignMode::Transaction => {
-                    let signature = crate::handlers::sign_tx::handler_swap(stream, tx_params);
+                    let signature = crate::handlers::sign_tx::swap_handler(stream, tx_params);
                     match signature {
                         Ok(sig) => {
                             comm.append(&sig.0);
@@ -118,3 +99,33 @@ fn handle_apdu(comm: &mut Comm, ins: super::Instruction, tx_params: &mut CreateT
         _ => comm.swap_reply(crate::AppSW::InsNotSupported),
     }
 }
+
+fn check_address(params: &CheckAddressParams) -> Result<(), &'static str> {
+    let path = PathBip32::parse(&params.dpath[..params.dpath_len * 4])
+        .map_err(|_| "Derivation path failure")?;
+
+    let pk = ecc::Ed25519::derive_from_path_slip10(&path.0)
+        .public_key()
+        .map_err(|_| "Public key derivation failure")?;
+
+    let pk = PublicKeyBe::from_little_endian(pk);
+    let mut buf = [0u8; 64];
+    let address = pk.display_str_hex(&mut buf);
+
+    let ref_address = core::str::from_utf8(&params.ref_address[..params.ref_address_len])
+        .map_err(|_| "Invalid UTF-8 in reference address")?;
+
+    if address == ref_address {
+        Ok(())
+    } else {
+        Err("Address mismatch")
+    }
+}
+
+fn get_printable_amount(params: &PrintableAmountParams) -> Buffer<30> {
+    let amount = u128::from_be_bytes(params.amount);
+    let near_token = NearToken::from_yoctonear(amount);
+    let mut near_token_buffer = TokenBuffer::new();
+    near_token.display_as_buffer(&mut near_token_buffer);
+    near_token_buffer
+}
diff --git a/tests/test_version_cmd.py b/tests/test_version_cmd.py
@@ -1,4 +1,5 @@
 from application_client.client import Nearbackend
+import toml
 
 
 # In this test we check that the get_version replies the right application version
@@ -8,5 +9,9 @@ def test_get_version_cmd(backend):
     # Send the get_version instruction to the app
     version = client.get_version().data
     assert len(version) == 3
-    # Assert that we have received the correct app version compared as Makefile data
-    assert (version[0], version[1], version[2]) == (2, 3, 3)
+    # Read version from Cargo.toml
+    with open('Cargo.toml', 'r') as f:
+        config = toml.load(f)
+        version = config['package']['version']
+        major, minor, patch = version.split('.')
+    assert (version[0], version[1], version[2]) == (int(major), int(minor), int(patch))