diff --git a/Cargo.lock b/Cargo.lock index a66c9cd509..50f3c5d539 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -955,7 +955,7 @@ dependencies = [ "num-traits", "parking_lot", "primitives", - "prost", + "prost 0.12.6", "prost-build", "protobuf", "proxy_signature", @@ -1177,7 +1177,7 @@ version = "0.21.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "82e23f6ab56d5f031cde05b8b82a5fefd3a1a223595c79e32317a97189e612bc" dependencies = [ - "prost", + "prost 0.12.6", "prost-types", "tendermint-proto", ] @@ -4255,7 +4255,7 @@ dependencies = [ "parking_lot", "primitive-types", "primitives", - "prost", + "prost 0.12.6", "prost-build", "rand 0.6.5", "rand 0.7.3", @@ -4368,7 +4368,7 @@ dependencies = [ "mm2_number", "mm2_state_machine", "pin-project", - "prost", + "prost 0.12.6", "rand 0.7.3", "rustls 0.21.10", "serde", @@ -5190,7 +5190,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "deb1435c188b76130da55f17a466d252ff7b1418b2ad3e037d127b94e3411f29" dependencies = [ "bytes", - "prost-derive", + "prost-derive 0.12.6", +] + +[[package]] +name = "prost" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7231bd9b3d3d33c86b58adbac74b5ec0ad9f496b19d22801d773636feaa95f3d" +dependencies = [ + "bytes", + "prost-derive 0.14.1", ] [[package]] @@ -5207,7 +5217,7 @@ dependencies = [ "once_cell", "petgraph", "prettyplease", - "prost", + "prost 0.12.6", "prost-types", "regex", "syn 2.0.77", @@ -5227,13 +5237,26 @@ dependencies = [ "syn 2.0.77", ] +[[package]] +name = "prost-derive" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9120690fafc389a67ba3803df527d0ec9cbbc9cc45e4cc20b332996dfb672425" +dependencies = [ + "anyhow", + "itertools", + "proc-macro2", + "quote 1.0.37", + "syn 2.0.77", +] + [[package]] name = "prost-types" version = "0.12.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9091c90b0a32608e984ff2fa4091273cbdd755d54935c51d520887f4a1dbd5b0" dependencies = [ - "prost", + "prost 0.12.6", ] [[package]] @@ -7093,7 +7116,7 @@ dependencies = [ "k256", "num-traits", "once_cell", - "prost", + "prost 0.12.6", "prost-types", "ripemd", "serde", @@ -7133,7 +7156,7 @@ dependencies = [ "flex-error", "num-derive", "num-traits", - "prost", + "prost 0.12.6", "prost-types", "serde", "serde_bytes", @@ -7489,7 +7512,7 @@ dependencies = [ "hyper-timeout", "percent-encoding", "pin-project", - "prost", + "prost 0.12.6", "rustls 0.21.10", "rustls-pemfile 1.0.2", "tokio", @@ -7618,7 +7641,7 @@ dependencies = [ "js-sys", "lazy_static", "mm2_err_handle", - "prost", + "prost 0.14.1", "rand 0.7.3", "rpc_task", "serde", diff --git a/mm2src/coins/hd_wallet/confirm_address.rs b/mm2src/coins/hd_wallet/confirm_address.rs index 664713f0a2..574e35e5b2 100644 --- a/mm2src/coins/hd_wallet/confirm_address.rs +++ b/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_handle: RpcTaskHandleShared, statuses: HwConnectStatuses, trezor_message_type: TrezorMessageType, + trezor_script_type: Option, }, } @@ -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, statuses: HwConnectStatuses, trezor_message_type: TrezorMessageType, + trezor_script_type: Option, ) -> MmResult, 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, @@ -147,6 +154,7 @@ where derivation_path: DerivationPath, expected_address: String, trezor_message_type: &TrezorMessageType, + trezor_script_type: Option, ) -> MmResult<(), HDConfirmAddressError> { let confirm_statuses = TrezorRequestStatuses { on_button_request: Task::InProgressStatus::confirm_addr_status(expected_address.clone()), @@ -158,14 +166,19 @@ 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()) .await .map_mm_err()?, TrezorMessageType::Ethereum => trezor_session - .get_eth_address(derivation_path, SHOW_ADDRESS_ON_DISPLAY) + .get_eth_address(&derivation_path, SHOW_ADDRESS_ON_DISPLAY) .await .map_mm_err()? .process(pubkey_processor.clone()) diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index 5e1ecb47e7..f66230f9f9 100644 --- a/mm2src/coins/lp_coins.rs +++ b/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] diff --git a/mm2src/coins/rpc_command/get_new_address.rs b/mm2src/coins/rpc_command/get_new_address.rs index e279ec99d4..a9e465e501 100644 --- a/mm2src/coins/rpc_command/get_new_address.rs +++ b/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) -> Result> { + /// Caller to get and confirm a new HD address for an HD wallet async fn get_new_address_helper( ctx: &MmArc, coin: &Coin, params: GetNewAddressParams, task_handle: GetNewAddressTaskHandleShared, trezor_message_type: TrezorMessageType, + trezor_script_type: Option, ) -> MmResult::BalanceObject>, GetNewAddressRpcError> where Coin: GetNewAddressRpcOps + Send + Sync, @@ -330,31 +335,52 @@ impl RpcTask for InitGetNewAddressTask { on_ready: GetNewAddressInProgressStatus::RequestingAccountBalance, }; let confirm_address: RpcTaskConfirmAddress = - 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, @@ -362,6 +388,7 @@ impl RpcTask for InitGetNewAddressTask { self.req.params.clone(), task_handle, TrezorMessageType::Ethereum, + None, ) .await?, )), diff --git a/mm2src/coins/utxo.rs b/mm2src/coins/utxo.rs index a7d4774ac8..9a8b64d9bb 100644 --- a/mm2src/coins/utxo.rs +++ b/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/ diff --git a/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs b/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs index fa54623322..725e61377c 100644 --- a/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs +++ b/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 { let format_from_req = self.activation_params().address_format.clone(); let format_from_conf = json::from_value::>(self.conf()["address_format"].clone()) diff --git a/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs b/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs index 9e118ce17e..7f9ccc4d64 100644 --- a/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs +++ b/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs @@ -6768,8 +6768,16 @@ mod trezor_tests { .unwrap(); let _ = init_trezor_user_action(ctx.clone(), pin_req).await; }, - _ => { - panic!("Trezor passphrase is not supported in tests"); + HwRpcTaskAwaitingStatus::EnterTrezorPassphrase => { + let empty_passphrase = serde_json::from_value(json!({ + "task_id": task_id, + "user_action": { + "action_type": "TrezorPassphrase", + "passphrase": "" + } + })) + .unwrap(); + let _ = init_trezor_user_action(ctx.clone(), empty_passphrase).await; }, } }, @@ -6841,7 +6849,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 = json::from_value(json!({ @@ -6899,8 +6907,12 @@ mod trezor_tests { }); let _ = init_trezor_user_action_rpc(mm, init.result.task_id, pin_action).await; }, - _ => { - panic!("Trezor passphrase is not supported in tests"); + HwRpcTaskAwaitingStatus::EnterTrezorPassphrase => { + let empty_passphrase = json!({ + "action_type": "TrezorPassphrase", + "passphrase": "" + }); + let _ = init_trezor_user_action_rpc(mm, init.result.task_id, empty_passphrase).await; }, } }, diff --git a/mm2src/trezor/Cargo.toml b/mm2src/trezor/Cargo.toml index 55f40b5d3d..a5e7972fc3 100644 --- a/mm2src/trezor/Cargo.toml +++ b/mm2src/trezor/Cargo.toml @@ -14,7 +14,7 @@ derive_more.workspace = true futures = { workspace = true, features = ["compat", "async-await"] } hw_common = { path = "../hw_common" } mm2_err_handle = { path = "../mm2_err_handle" } -prost.workspace = true +prost = "0.14.1" rand = { workspace = true, features = ["std", "wasm-bindgen"] } rpc_task = { path = "../rpc_task" } serde.workspace = true diff --git a/mm2src/trezor/src/eth/eth_command.rs b/mm2src/trezor/src/eth/eth_command.rs index f7af48dae0..e92713e2c2 100644 --- a/mm2src/trezor/src/eth/eth_command.rs +++ b/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! { @@ -71,11 +76,11 @@ impl<'a> TrezorSession<'a> { /// Retrieves the EVM address associated with a given derivation path from the Trezor device. pub async fn get_eth_address<'b>( &'b mut self, - derivation_path: DerivationPath, + derivation_path: &DerivationPath, show_display: bool, ) -> TrezorResult>> { let req = proto_ethereum::EthereumGetAddress { - address_n: derivation_path.iter().map(|child| child.0).collect(), + address_n: serialize_derivation_path(derivation_path), show_display: Some(show_display), encoded_network: None, chunkify: None, @@ -90,11 +95,18 @@ impl<'a> TrezorSession<'a> { derivation_path: &DerivationPath, show_display: bool, ) -> TrezorResult> { - 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 } diff --git a/mm2src/trezor/src/proto/messages_ethereum.rs b/mm2src/trezor/src/proto/messages_ethereum.rs index bd4dee7e59..13b33b9bfb 100644 --- a/mm2src/trezor/src/proto/messages_ethereum.rs +++ b/mm2src/trezor/src/proto/messages_ethereum.rs @@ -6,7 +6,7 @@ #[derive(Clone, PartialEq, ::prost::Message)] pub struct EthereumGetAddress { /// BIP-32 path to derive the key from master node - #[prost(uint32, repeated, tag = "1")] + #[prost(uint32, repeated, packed = "false", tag = "1")] pub address_n: ::prost::alloc::vec::Vec, /// optionally show on display before sending the result #[prost(bool, optional, tag = "2")] @@ -40,7 +40,7 @@ pub struct EthereumAddress { #[derive(Clone, PartialEq, ::prost::Message)] pub struct EthereumGetPublicKey { // BIP-32 path to derive the key from master node - #[prost(uint32, repeated, tag = "1")] + #[prost(uint32, repeated, packed = "false", tag = "1")] pub address_n: ::prost::alloc::vec::Vec, // repeated uint32 address_n = 1; // optionally show on display before sending the result #[prost(bool, optional, tag = "2")] @@ -71,7 +71,7 @@ pub struct EthereumPublicKey { #[derive(Clone, PartialEq, ::prost::Message)] pub struct EthereumSignTx { /// BIP-32 path to derive the key from master node - #[prost(uint32, repeated, tag = "1")] + #[prost(uint32, repeated, packed = "false", tag = "1")] pub address_n: ::prost::alloc::vec::Vec, /// <=256 bit unsigned big endian #[prost(bytes = "vec", optional, tag = "2", default = "b\"\"")] @@ -117,7 +117,7 @@ pub struct EthereumSignTx { #[derive(Clone, PartialEq, ::prost::Message)] pub struct EthereumSignTxEIP1559 { /// BIP-32 path to derive the key from master node - #[prost(uint32, repeated, tag = "1")] + #[prost(uint32, repeated, packed = "false", tag = "1")] pub address_n: ::prost::alloc::vec::Vec, /// <=256 bit unsigned big endian #[prost(bytes = "vec", required, tag = "2")] diff --git a/mm2src/trezor/src/utxo/utxo_command.rs b/mm2src/trezor/src/utxo/utxo_command.rs index f513e831e7..d12491c080 100644 --- a/mm2src/trezor/src/utxo/utxo_command.rs +++ b/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, ) -> TrezorResult> { 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::::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), };