fix(Trezor): fix utxo and eth calls due to firmware changes

mm2src/coins/hd_wallet/confirm_address.rs

@@ -2,6 +2,7 @@ use async_trait::async_trait;
 use bip32::DerivationPath;
 use crypto::hw_rpc_task::HwConnectStatuses;
 use crypto::trezor::trezor_rpc_task::{TrezorRequestStatuses, TrezorRpcTaskProcessor, TryIntoUserAction};
+use crypto::trezor::utxo::TrezorInputScriptType;
 use crypto::trezor::{ProcessTrezorResponse, TrezorError, TrezorMessageType, TrezorProcessingError};
 use crypto::{CryptoCtx, CryptoCtxError, HardwareWalletArc, HwError, HwProcessingError};
 use enum_derives::{EnumFromInner, EnumFromStringify};
@@ -77,6 +78,7 @@ pub(crate) enum RpcTaskConfirmAddress<Task: RpcTask> {
         task_handle: RpcTaskHandleShared<Task>,
         statuses: HwConnectStatuses<Task::InProgressStatus, Task::AwaitingStatus>,
         trezor_message_type: TrezorMessageType,
+        trezor_script_type: Option<TrezorInputScriptType>,
     },
 }
 
@@ -99,6 +101,7 @@ where
                 task_handle,
                 statuses,
                 trezor_message_type,
+                trezor_script_type,
             } => {
                 Self::confirm_address_with_trezor(
                     hw_ctx,
@@ -108,6 +111,7 @@ where
                     derivation_path,
                     expected_address,
                     trezor_message_type,
+                    *trezor_script_type,
                 )
                 .await
             },
@@ -126,6 +130,7 @@ where
         task_handle: RpcTaskHandleShared<Task>,
         statuses: HwConnectStatuses<Task::InProgressStatus, Task::AwaitingStatus>,
         trezor_message_type: TrezorMessageType,
+        trezor_script_type: Option<TrezorInputScriptType>,
     ) -> MmResult<RpcTaskConfirmAddress<Task>, HDConfirmAddressError> {
         let crypto_ctx = CryptoCtx::from_ctx(ctx).map_mm_err()?;
         let hw_ctx = crypto_ctx
@@ -136,9 +141,11 @@ where
             task_handle,
             statuses,
             trezor_message_type,
+            trezor_script_type,
         })
     }
 
+    #[allow(clippy::too_many_arguments)]
     async fn confirm_address_with_trezor(
         hw_ctx: &HardwareWalletArc,
         task_handle: RpcTaskHandleShared<Task>,
@@ -147,6 +154,7 @@ where
         derivation_path: DerivationPath,
         expected_address: String,
         trezor_message_type: &TrezorMessageType,
+        trezor_script_type: Option<TrezorInputScriptType>,
     ) -> MmResult<(), HDConfirmAddressError> {
         let confirm_statuses = TrezorRequestStatuses {
             on_button_request: Task::InProgressStatus::confirm_addr_status(expected_address.clone()),
@@ -158,7 +166,12 @@ where
         let mut trezor_session = hw_ctx.trezor(pubkey_processor.clone()).await.map_mm_err()?;
         let address = match trezor_message_type {
             TrezorMessageType::Bitcoin => trezor_session
-                .get_utxo_address(derivation_path, trezor_coin, SHOW_ADDRESS_ON_DISPLAY)
+                .get_utxo_address(
+                    derivation_path,
+                    trezor_coin,
+                    SHOW_ADDRESS_ON_DISPLAY,
+                    trezor_script_type,
+                )
                 .await
                 .map_mm_err()?
                 .process(pubkey_processor.clone())

mm2src/coins/lp_coins.rs

@@ -4300,6 +4300,8 @@ impl CoinsContext {
 }
 
 /// This enum is used in coin activation requests.
+/// TODO: should we use #[serde(tag = "type", content = "params")] for this PrivKeyActivationPolicy like for the Eth policy,
+/// to have them identical in activation requests
 #[derive(Clone, Debug, Deserialize, Serialize, Default)]
 pub enum PrivKeyActivationPolicy {
     #[default]

mm2src/coins/rpc_command/get_new_address.rs

@@ -3,6 +3,7 @@ use crate::hd_wallet::{
     AddressDerivingError, ConfirmAddressStatus, HDConfirmAddress, HDConfirmAddressError, InvalidBip44ChainError,
     NewAddressDeriveConfirmError, NewAddressDerivingError, RpcTaskConfirmAddress,
 };
+use crate::utxo::UtxoCommonOps;
 use crate::{
     lp_coinfind_or_err, BalanceError, CoinBalance, CoinBalanceMap, CoinFindError, CoinsContext, MmCoinEnum,
     UnexpectedDerivationMethod,
@@ -12,11 +13,13 @@ use common::{HttpStatusCode, SuccessResponse};
 use crypto::hw_rpc_task::{
     HwConnectStatuses, HwRpcTaskAwaitingStatus, HwRpcTaskUserAction, HwRpcTaskUserActionRequest,
 };
+use crypto::trezor::utxo::TrezorInputScriptType;
 use crypto::trezor::TrezorMessageType;
 use crypto::{from_hw_error, Bip44Chain, HwError, HwRpcError, WithHwRpcError};
 use derive_more::Display;
 use enum_derives::EnumFromTrait;
 use http::StatusCode;
+use keys::AddressFormat;
 use mm2_core::mm_ctx::MmArc;
 use mm2_err_handle::prelude::*;
 use rpc_task::rpc_common::{
@@ -310,12 +313,14 @@ impl RpcTask for InitGetNewAddressTask {
     async fn cancel(self) {}
 
     async fn run(&mut self, task_handle: RpcTaskHandleShared<Self>) -> Result<Self::Item, MmError<Self::Error>> {
+        /// Caller to get and confirm a new HD address for an HD wallet
         async fn get_new_address_helper<Coin>(
             ctx: &MmArc,
             coin: &Coin,
             params: GetNewAddressParams,
             task_handle: GetNewAddressTaskHandleShared,
             trezor_message_type: TrezorMessageType,
+            trezor_script_type: Option<TrezorInputScriptType>,
         ) -> MmResult<GetNewAddressResponse<<Coin as GetNewAddressRpcOps>::BalanceObject>, GetNewAddressRpcError>
         where
             Coin: GetNewAddressRpcOps + Send + Sync,
@@ -330,38 +335,60 @@ impl RpcTask for InitGetNewAddressTask {
                 on_ready: GetNewAddressInProgressStatus::RequestingAccountBalance,
             };
             let confirm_address: RpcTaskConfirmAddress<InitGetNewAddressTask> =
-                RpcTaskConfirmAddress::new(ctx, task_handle, hw_statuses, trezor_message_type).map_mm_err()?;
+                RpcTaskConfirmAddress::new(ctx, task_handle, hw_statuses, trezor_message_type, trezor_script_type)
+                    .map_mm_err()?;
             coin.get_new_address_rpc(params, &confirm_address).await
         }
 
         match self.coin {
-            MmCoinEnum::UtxoCoin(ref utxo) => Ok(GetNewAddressResponseEnum::Map(
-                get_new_address_helper(
-                    &self.ctx,
-                    utxo,
-                    self.req.params.clone(),
-                    task_handle,
-                    TrezorMessageType::Bitcoin,
-                )
-                .await?,
-            )),
-            MmCoinEnum::QtumCoin(ref qtum) => Ok(GetNewAddressResponseEnum::Map(
-                get_new_address_helper(
-                    &self.ctx,
-                    qtum,
-                    self.req.params.clone(),
-                    task_handle,
-                    TrezorMessageType::Bitcoin,
-                )
-                .await?,
-            )),
+            MmCoinEnum::UtxoCoin(ref utxo) => {
+                // Set script type to enable Trezor to correctly validate the derivation path
+                let trezor_script_type = match utxo.addr_format() {
+                    AddressFormat::Standard | AddressFormat::CashAddress { .. } => {
+                        Some(TrezorInputScriptType::SpendAddress)
+                    },
+                    AddressFormat::Segwit => Some(TrezorInputScriptType::SpendWitness),
+                };
+                Ok(GetNewAddressResponseEnum::Map(
+                    get_new_address_helper(
+                        &self.ctx,
+                        utxo,
+                        self.req.params.clone(),
+                        task_handle,
+                        TrezorMessageType::Bitcoin,
+                        trezor_script_type,
+                    )
+                    .await?,
+                ))
+            },
+            MmCoinEnum::QtumCoin(ref qtum) => {
+                // Set script type to enable Trezor to correctly validate the derivation path
+                let trezor_script_type = match qtum.addr_format() {
+                    AddressFormat::Standard | AddressFormat::CashAddress { .. } => {
+                        Some(TrezorInputScriptType::SpendAddress)
+                    },
+                    AddressFormat::Segwit => Some(TrezorInputScriptType::SpendWitness),
+                };
+                Ok(GetNewAddressResponseEnum::Map(
+                    get_new_address_helper(
+                        &self.ctx,
+                        qtum,
+                        self.req.params.clone(),
+                        task_handle,
+                        TrezorMessageType::Bitcoin,
+                        trezor_script_type,
+                    )
+                    .await?,
+                ))
+            },
             MmCoinEnum::EthCoin(ref eth) => Ok(GetNewAddressResponseEnum::Map(
                 get_new_address_helper(
                     &self.ctx,
                     eth,
                     self.req.params.clone(),
                     task_handle,
                     TrezorMessageType::Ethereum,
+                    None,
                 )
                 .await?,
             )),

mm2src/coins/utxo.rs

@@ -577,6 +577,7 @@ pub struct UtxoCoinConf {
     pub tx_version: i32,
     /// Defines if Segwit is enabled for this coin.
     /// https://en.bitcoin.it/wiki/Segregated_Witness
+    /// NOTE: this does not make the coin itself 'segwit'. This just tells that segwit addresses are supported for this coin
     pub segwit: bool,
     /// Does coin require transactions to be notarized to be considered as confirmed?
     /// https://komodoplatform.com/security-delayed-proof-of-work-dpow/

mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs

@@ -517,6 +517,9 @@ pub trait UtxoCoinBuilderCommonOps {
 
     fn ticker(&self) -> &str;
 
+    /// This function basically defines 'my address' format (so whether a coin is segwit or not)
+    /// For that it looks for the "address_format" property first in the activation request then in the coins file.
+    /// This fn is called to set the address format in the derivaion_method in UtxoCoinFields, which creates my address.
     fn address_format(&self) -> UtxoCoinBuildResult<UtxoAddressFormat> {
         let format_from_req = self.activation_params().address_format.clone();
         let format_from_conf = json::from_value::<Option<UtxoAddressFormat>>(self.conf()["address_format"].clone())

mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs

@@ -6841,7 +6841,7 @@ mod trezor_tests {
             "method": "electrum",
             "coin": ticker,
             "servers": tbtc_electrums(),
-            "priv_key_policy": { "type": "Trezor" },
+            "priv_key_policy": "Trezor",
         });
         let activation_params = UtxoActivationParams::from_legacy_req(&enable_req).unwrap();
         let request: InitStandaloneCoinReq<UtxoActivationParams> = json::from_value(json!({

mm2src/trezor/src/eth/eth_command.rs

@@ -1,16 +1,20 @@
 use crate::proto::{
-    messages_ethereum as proto_ethereum, messages_ethereum_definitions as proto_ethereum_definitions, TrezorMessage,
+    messages_bitcoin as proto_bitcoin, messages_ethereum as proto_ethereum,
+    messages_ethereum_definitions as proto_ethereum_definitions, TrezorMessage,
 };
 use crate::response_processor::ProcessTrezorResponse;
 use crate::result_handler::ResultHandler;
-use crate::{serialize_derivation_path, OperationFailure, TrezorError, TrezorResponse, TrezorResult, TrezorSession};
+use crate::{
+    ecdsa_curve_to_string, serialize_derivation_path, OperationFailure, TrezorError, TrezorResponse, TrezorResult,
+    TrezorSession,
+};
 use ethcore_transaction::{
     eip155_methods::check_replay_protection, Action, Eip1559Transaction, LegacyTransaction, TransactionShared,
     TransactionWrapper, UnverifiedTransactionWrapper,
 };
 use ethereum_types::H256;
 use ethkey::Signature;
-use hw_common::primitives::{DerivationPath, XPub};
+use hw_common::primitives::{DerivationPath, EcdsaCurve, XPub};
 use lazy_static::lazy_static;
 use mm2_err_handle::map_mm_error::MapMmError;
 use mm2_err_handle::map_to_mm::MapToMmResult;
@@ -25,6 +29,7 @@ type StaticAddressBytes = &'static [u8];
 // new supported eth networks:
 const SEPOLIA_ID: u64 = 11155111;
 const EIP2930_NOT_SUPPORTED_ERROR: &str = "EIP-2930 tx not supported for Trezor";
+const TREZOR_COIN_TO_GET_PUBKEY: &str = "Bitcoin";
 
 lazy_static! {
 
@@ -90,11 +95,18 @@ impl<'a> TrezorSession<'a> {
         derivation_path: &DerivationPath,
         show_display: bool,
     ) -> TrezorResult<TrezorResponse<'a, 'b, XPub>> {
-        let req = proto_ethereum::EthereumGetPublicKey {
+        // We cannot use the EthereumGetPublicKey message (broken in the Safe/Model T firmware),
+        // so we use bitcoin GetPublicKey msg instead.
+        // Apparently this should work as Bitcoin and Ethereum both use "m/44'"
+        let req = proto_bitcoin::GetPublicKey {
             address_n: serialize_derivation_path(derivation_path),
+            ecdsa_curve_name: Some(ecdsa_curve_to_string(EcdsaCurve::Secp256k1)),
             show_display: Some(show_display),
+            coin_name: Some(TREZOR_COIN_TO_GET_PUBKEY.to_string()),
+            script_type: None,
+            ignore_xpub_magic: Some(true),
         };
-        let result_handler = ResultHandler::new(|m: proto_ethereum::EthereumPublicKey| Ok(m.xpub));
+        let result_handler = ResultHandler::new(|m: proto_bitcoin::PublicKey| Ok(m.xpub));
         self.call(req, result_handler).await
     }
 

mm2src/trezor/src/utxo/utxo_command.rs

@@ -4,6 +4,8 @@ use crate::result_handler::ResultHandler;
 use crate::{ecdsa_curve_to_string, serialize_derivation_path, TrezorResponse, TrezorResult};
 use hw_common::primitives::{DerivationPath, EcdsaCurve, XPub};
 
+use super::TrezorInputScriptType;
+
 pub const IGNORE_XPUB_MAGIC: bool = true;
 
 // Bitcoin(UTXO) operations.
@@ -13,13 +15,15 @@ impl<'a> TrezorSession<'a> {
         path: DerivationPath,
         coin: String,
         show_display: bool,
+        trezor_script_type: Option<TrezorInputScriptType>,
     ) -> TrezorResult<TrezorResponse<'a, 'b, String>> {
         let req = proto_bitcoin::GetAddress {
             address_n: serialize_derivation_path(&path),
             coin_name: Some(coin),
             show_display: Some(show_display),
             multisig: None,
-            script_type: None,
+            // Trezor validates the script type against the derivation path (e.g. "m'/84'" should match the SpendWitness type)
+            script_type: trezor_script_type.map(|s_t| Into::<proto_bitcoin::InputScriptType>::into(s_t) as i32),
             ignore_xpub_magic: None,
         };
         let result_handler = ResultHandler::new(|m: proto_bitcoin::Address| Ok(m.address));
@@ -39,6 +43,8 @@ impl<'a> TrezorSession<'a> {
             ecdsa_curve_name: Some(ecdsa_curve_to_string(ecdsa_curve)),
             show_display: Some(show_display),
             coin_name: Some(coin),
+            // Trezor defaults to the SpendAddress script type.
+            // This always produces one prefix xpub(tpub) instead of different magics for different script types (xpub, ypub, zpub).
             script_type: None,
             ignore_xpub_magic: Some(ignore_xpub_magic),
         };