Merge pull request #1961 from mintlayer/trezor_fw_sign_message_fix_tests_update

Update `trezor_signer::tests::test_sign_message` after the corresponding firmware fix

Author: ImplOfAnImpl

Cargo.lock

@@ -251,8 +251,8 @@ zeroize = "1.5"
 
 [workspace.dependencies.trezor-client]
 git = "https://github.com/mintlayer/mintlayer-trezor-firmware"
-# The commit "Mintlayer firmware versioning"
-rev = "397155e26660993a044e78447dad893852fa44d1"
+# The commit "Merge pull request #16 from mintlayer/sign_message_convert_to_hex_if_non_ascii"
+rev = "8154c7a3bc65d27d64186f1b733750e029f8cad0"
 features = ["bitcoin", "mintlayer"]
 
 [workspace.metadata.dist.dependencies.apt]

Cargo.toml

@@ -24,7 +24,7 @@ use crate::signer::tests::{
     },
     generic_tests::{
         test_sign_message_generic, test_sign_transaction_generic,
-        test_sign_transaction_intent_generic,
+        test_sign_transaction_intent_generic, MessageToSign,
     },
     make_deterministic_software_signer, make_software_signer, no_another_signer,
 };
@@ -35,7 +35,12 @@ use crate::signer::tests::{
 fn test_sign_message(#[case] seed: Seed) {
     let mut rng = make_seedable_rng(seed);
 
-    test_sign_message_generic(&mut rng, make_software_signer, no_another_signer());
+    test_sign_message_generic(
+        &mut rng,
+        MessageToSign::Random,
+        make_software_signer,
+        no_another_signer(),
+    );
 }
 
 #[rstest]

wallet/src/signer/software_signer/tests.rs

@@ -51,7 +51,10 @@ use crypto::{
 use logging::log;
 use randomness::{CryptoRng, Rng};
 use serialization::extras::non_empty_vec::DataOrNoVec;
-use test_utils::{random::gen_random_bytes, random_ascii_alphanumeric_string};
+use test_utils::{
+    random::{gen_random_bytes, make_seedable_rng},
+    random_ascii_alphanumeric_string,
+};
 use tx_verifier::error::{InputCheckErrorPayload, ScriptError};
 use wallet_storage::{DefaultBackend, Store, TransactionRwUnlocked, Transactional};
 use wallet_types::{
@@ -69,8 +72,15 @@ use crate::{
     Account, SendRequest,
 };
 
+#[derive(Debug)]
+pub enum MessageToSign {
+    Random,
+    Predefined(Vec<u8>),
+}
+
 pub fn test_sign_message_generic<MkS1, MkS2, S1, S2>(
     rng: &mut (impl Rng + CryptoRng),
+    message_to_sign: MessageToSign,
     make_signer: MkS1,
     make_another_signer: Option<MkS2>,
 ) where
@@ -85,6 +95,21 @@ pub fn test_sign_message_generic<MkS1, MkS2, S1, S2>(
 
     let mut account = account_from_mnemonic(&chain_config, &mut db_tx, DEFAULT_ACCOUNT_INDEX);
 
+    let mut make_message = {
+        // Note: can't pass rng to the closure without knowing its exact type, so we create a new one.
+        let mut rng = make_seedable_rng(rng.gen());
+        move || {
+            let msg = match &message_to_sign {
+                MessageToSign::Random => {
+                    vec![rng.r#gen::<u8>(), rng.r#gen::<u8>(), rng.r#gen::<u8>()]
+                }
+                MessageToSign::Predefined(msg) => msg.clone(),
+            };
+            log::debug!("Using message: {msg:?}");
+            msg
+        }
+    };
+
     let pkh_destination = account
         .get_new_address(&mut db_tx, KeyPurpose::ReceiveFunds)
         .unwrap()
@@ -106,7 +131,7 @@ pub fn test_sign_message_generic<MkS1, MkS2, S1, S2>(
     let standalone_pk_destination = Destination::PublicKey(standalone_pk);
 
     for destination in [pkh_destination, pk_destination, standalone_pk_destination] {
-        let message = vec![rng.gen::<u8>(), rng.gen::<u8>(), rng.gen::<u8>()];
+        let message = make_message();
         let message_challenge = produce_message_challenge(&message);
 
         let mut signer = make_signer(chain_config.clone(), account.account_index());
@@ -136,7 +161,7 @@ pub fn test_sign_message_generic<MkS1, MkS2, S1, S2>(
 
     let mut signer = make_signer(chain_config.clone(), account.account_index());
 
-    let message = vec![rng.gen::<u8>(), rng.gen::<u8>(), rng.gen::<u8>()];
+    let message = make_message();
     let err = signer
         .sign_challenge(
             &message,

wallet/src/signer/tests/generic_tests.rs

@@ -30,7 +30,7 @@ use crate::signer::{
         },
         generic_tests::{
             test_sign_message_generic, test_sign_transaction_generic,
-            test_sign_transaction_intent_generic,
+            test_sign_transaction_intent_generic, MessageToSign,
         },
         make_deterministic_software_signer, no_another_signer,
     },
@@ -75,14 +75,28 @@ pub fn make_deterministic_trezor_signer(
 #[trace]
 #[serial]
 #[case(Seed::from_entropy())]
-fn test_sign_message(#[case] seed: Seed) {
+fn test_sign_message(
+    #[case] seed: Seed,
+    #[values(
+        MessageToSign::Random,
+        // Special case: an "overlong" utf-8 string (basically, the letter 'K' encoded with 2 bytes
+        // instead of 1). The firmware used to have troubles with this.
+        MessageToSign::Predefined(vec![193, 139])
+    )]
+    message_to_sign: MessageToSign,
+) {
     log::debug!("test_sign_message, seed = {seed:?}");
 
     let _join_guard = maybe_spawn_auto_confirmer();
 
     let mut rng = make_seedable_rng(seed);
 
-    test_sign_message_generic(&mut rng, make_trezor_signer, no_another_signer());
+    test_sign_message_generic(
+        &mut rng,
+        message_to_sign,
+        make_trezor_signer,
+        no_another_signer(),
+    );
 }
 
 #[rstest]
@@ -175,6 +189,7 @@ fn test_sign_message_sig_consistency(#[case] seed: Seed) {
 
     test_sign_message_generic(
         &mut rng,
+        MessageToSign::Random,
         make_deterministic_trezor_signer,
         Some(make_deterministic_software_signer),
     );