diff --git a/Cargo.lock b/Cargo.lock index 101be123c..f526325ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5100,17 +5100,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "messages" -version = "0.1.0" -source = "git+https://github.com/mintlayer/mintlayer-ledger-app?rev=a544449d0c649c7c6b27ec0bd8e03f43861f60a7#a544449d0c649c7c6b27ec0bd8e03f43861f60a7" -dependencies = [ - "derive_more 2.1.1", - "mintlayer-core-primitives", - "num_enum", - "parity-scale-codec", -] - [[package]] name = "metal" version = "0.27.0" @@ -5186,6 +5175,17 @@ dependencies = [ "mintlayer-core-primitives", ] +[[package]] +name = "mintlayer-messages" +version = "0.1.0" +source = "git+https://github.com/mintlayer/mintlayer-ledger-app?rev=16dce6ab619529d5b8f506cb6c2e4f62199b25d7#16dce6ab619529d5b8f506cb6c2e4f62199b25d7" +dependencies = [ + "derive_more 2.1.1", + "mintlayer-core-primitives", + "num_enum", + "parity-scale-codec", +] + [[package]] name = "mintlayer-test" version = "1.3.1" @@ -9882,7 +9882,8 @@ dependencies = [ "ledger-lib", "logging", "mempool", - "messages", + "mintlayer-core-primitives", + "mintlayer-messages", "orders-accounting", "parity-scale-codec", "pos-accounting", diff --git a/Cargo.toml b/Cargo.toml index 9b493a686..dcb0342e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -300,9 +300,9 @@ rev = "c8ed12e89788e78d77cdc0dc9fb8a4bd4dc24b89" [workspace.dependencies.mintlayer-ledger-messages] git = "https://github.com/mintlayer/mintlayer-ledger-app" -# The commit "Merge pull request #3 from mintlayer/update_ml_primitives_rev" -rev = "a544449d0c649c7c6b27ec0bd8e03f43861f60a7" -package = "messages" +# The commit "Add technical specification" +rev = "16dce6ab619529d5b8f506cb6c2e4f62199b25d7" +package = "mintlayer-messages" [workspace.dependencies.trezor-client] git = "https://github.com/mintlayer/mintlayer-trezor-firmware" diff --git a/wallet/Cargo.toml b/wallet/Cargo.toml index fba23b3a6..2c8c1b3c9 100644 --- a/wallet/Cargo.toml +++ b/wallet/Cargo.toml @@ -33,6 +33,7 @@ bip39 = { workspace = true, default-features = false, features = [ derive_more.workspace = true hex.workspace = true itertools.workspace = true +ml_primitives.workspace = true parity-scale-codec.workspace = true semver.workspace = true serde.workspace = true diff --git a/wallet/src/signer/ledger_signer/ledger_messages.rs b/wallet/src/signer/ledger_signer/ledger_messages.rs index efdbcc5b0..8797dda07 100644 --- a/wallet/src/signer/ledger_signer/ledger_messages.rs +++ b/wallet/src/signer/ledger_signer/ledger_messages.rs @@ -15,7 +15,9 @@ use std::{collections::BTreeMap, mem::size_of_val, time::Duration}; -use crate::signer::ledger_signer::LedgerError; +use ledger_lib::{Device, Exchange}; +use mintlayer_ledger_messages as ledger_msg; + use crypto::key::{ extended::ExtendedPublicKey, hdkd::{chain_code::ChainCode, derivation_path::DerivationPath}, @@ -23,8 +25,23 @@ use crypto::key::{ }; use utils::ensure; -use ledger_lib::{Device, Exchange}; -use mintlayer_ledger_messages as ledger_msg; +use crate::signer::ledger_signer::LedgerError; + +use super::{LSighashInputCommitment, LedgerSignature}; + +macro_rules! ensure_response_type { + ($resp:expr, $pattern:pat $(if $guard:expr)?, $out:expr) => { + { + let to_match = $resp; + match to_match { + $pattern $(if $guard)? => $out, + _ => { + return Err(LedgerError::WrongResponse); + } + } + } + }; +} /// Timeout duration for normal Ledger operations const TIMEOUT_DUR: Duration = Duration::from_secs(100); @@ -32,23 +49,10 @@ const TIMEOUT_DUR: Duration = Duration::from_secs(100); /// Used in between normal operations when the screen is showing success/failure, /// and the Ledger app doesn't respond with any response so no need to wait for a long time. const SHORT_TIMEOUT_DUR: Duration = Duration::from_millis(200); -/// The supported transaction version by the ledger app -const TX_VERSION: u8 = 1; - -/// Ledger Signer errors -#[derive(thiserror::Error, Debug)] -pub enum LedgerMessagesError { - #[error("Device error: {0}")] - DeviceError(ledger_lib::Error), - #[error("Derivation path too long")] - DerivationPathTooLong, - #[error("APDU message too long")] - ApduMessageTooLong, -} /// Check that the response ends with the OK status code and return the rest of the response back -pub fn ok_response(mut resp: Vec) -> Result, LedgerError> { - let (_, status_code) = resp.split_last_chunk().ok_or(LedgerError::InvalidResponse)?; +fn extract_response_apdu_data(mut resp: Vec) -> Result, LedgerError> { + let (_, status_code) = resp.split_last_chunk().ok_or(LedgerError::InvalidResponseApdu)?; let response_status = u16::from_be_bytes(*status_code); let status_word_ok: u16 = ledger_msg::StatusWord::Ok.into(); @@ -71,11 +75,20 @@ async fn exchange_message( ledger: &mut L, msg_buf: &[u8], ) -> Result, LedgerError> { - let resp = ledger - .exchange(msg_buf, TIMEOUT_DUR) - .await - .map_err(LedgerMessagesError::DeviceError)?; - ok_response(resp) + exchange_message_with_timeout(ledger, msg_buf, TIMEOUT_DUR).await +} + +async fn exchange_message_with_timeout( + ledger: &mut L, + msg_buf: &[u8], + timeout: Duration, +) -> Result, LedgerError> { + let resp = ledger.exchange(msg_buf, timeout).await?; + extract_response_apdu_data(resp) +} + +fn decode_response(resp_data: &[u8]) -> Result { + ledger_msg::decode_all(resp_data).ok_or(LedgerError::CannotDecodeResponse) } /// Send a message in chunks to the ledger as the max size of a message is 255 bytes @@ -96,22 +109,21 @@ async fn send_chunked( resp = exchange_message(ledger, &msg_buf).await?; if !chunk.is_last() { - ensure!(resp.is_empty(), LedgerError::InvalidResponse); + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::ExpectingNextChunk, ()); } } Ok(resp) } -async fn send_chunked_expect_empty_ok_response( - ledger: &mut L, - ins: u8, - p1: u8, - message: &[u8], -) -> Result<(), LedgerError> { - let resp = send_chunked(ledger, ins, p1, message).await?; - ensure!(resp.is_empty(), LedgerError::InvalidResponse); - Ok(()) +fn make_apdu<'a>( + instruction_byte: u8, + param1_byte: u8, + command_data: &'a [u8], +) -> Result, LedgerError> { + ledger_msg::Apdu::new_with_data(instruction_byte, param1_byte, command_data) + .ok_or(LedgerError::ApduMessageTooLong) } pub async fn sign_challenge( @@ -120,20 +132,22 @@ pub async fn sign_challenge( path: ledger_msg::Bip32Path, addr_type: ledger_msg::AddrType, message: &[u8], -) -> Result, LedgerError> { +) -> Result { let req = ledger_msg::SignMessageReq { coin, addr_type, path, }; - send_chunked_expect_empty_ok_response( + let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_MSG, ledger_msg::SignP1::Start.into(), &ledger_msg::encode(req), ) .await?; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::MessageSetup, ()); let resp = send_chunked( ledger, @@ -143,16 +157,16 @@ pub async fn sign_challenge( ) .await?; - let sig: ledger_msg::MsgSignature = - ledger_msg::decode_all(&resp).ok_or(LedgerError::InvalidResponse)?; + let resp = decode_response(&resp)?; + let resp = ensure_response_type!(resp, ledger_msg::Response::MessageSignature(resp), resp); - Ok(sig.signature.to_vec()) + Ok(resp.signature) } pub async fn check_current_app( ledger: &mut L, ) -> Result { - let info = ledger.app_info(TIMEOUT_DUR).await.map_err(LedgerMessagesError::DeviceError)?; + let info = ledger.app_info(TIMEOUT_DUR).await?; let name = info.name; let app_version = info.version; @@ -161,31 +175,17 @@ pub async fn check_current_app( Ok(app_version) } -pub async fn get_extended_public_key_raw( - ledger: &mut L, - coin_type: ledger_msg::CoinType, - derivation_path: &DerivationPath, -) -> Result, LedgerMessagesError> { - let path = ledger_msg::Bip32Path( - derivation_path.as_slice().iter().map(|c| c.into_encoded_index()).collect(), - ); - let req = ledger_msg::PublicKeyReq { coin_type, path }; - let encoded_req = ledger_msg::encode(req); - - let apdu = ledger_msg::Apdu::new_with_data( - ledger_msg::Ins::PUB_KEY, - ledger_msg::PubKeyP1::NoDisplayAddress.into(), - &encoded_req, - ) - .ok_or(LedgerMessagesError::DerivationPathTooLong)?; +pub async fn ping(ledger: &mut L) -> Result<(), LedgerError> { + let apdu = make_apdu(ledger_msg::Ins::PING, ledger_msg::PingP1::Start.into(), &[])?; let mut msg_buf = Vec::with_capacity(apdu.bytes_count()); apdu.write_bytes(&mut msg_buf); - ledger - .exchange(&msg_buf, SHORT_TIMEOUT_DUR) - .await - .map_err(LedgerMessagesError::DeviceError) + let resp = exchange_message_with_timeout(ledger, &msg_buf, SHORT_TIMEOUT_DUR).await?; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::Pong, ()); + + Ok(()) } pub async fn get_extended_public_key( @@ -206,13 +206,13 @@ pub async fn get_extended_public_key( ) .await?; - let resp: ledger_msg::GetPublicKeyRespones = - ledger_msg::decode_all(&resp).ok_or(LedgerError::InvalidResponse)?; + let resp = decode_response(&resp)?; + let resp = ensure_response_type!(resp, ledger_msg::Response::PublicKey(resp), resp); let extended_public_key = Secp256k1ExtendedPublicKey::new_unchecked( derivation_path, - ChainCode::from(resp.chain_code), - Secp256k1PublicKey::from_bytes(&resp.public_key).map_err(|_| LedgerError::InvalidKey)?, + ChainCode::from(resp.chain_code.0), + Secp256k1PublicKey::from_bytes(&resp.public_key.0).map_err(|_| LedgerError::InvalidKey)?, ); Ok(ExtendedPublicKey::new(extended_public_key)) @@ -222,93 +222,99 @@ pub async fn sign_tx( ledger: &mut L, chain_type: ledger_msg::CoinType, inputs: Vec, - input_commitments: Vec, + input_commitments: Vec, outputs: Vec, -) -> Result>, LedgerError> { +) -> Result>, LedgerError> { let metadata = ledger_msg::encode(ledger_msg::TxMetadataReq { coin: chain_type, - version: TX_VERSION, - num_inputs: inputs.len() as u32, - num_outputs: outputs.len() as u32, + version: ledger_msg::TxMetadataVersionReq::V1(ledger_msg::TxMetadataV1Req { + num_inputs: inputs.len() as u32, + num_outputs: outputs.len() as u32, + }), }); - send_chunked_expect_empty_ok_response( + + let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_TX, ledger_msg::SignP1::Start.into(), &metadata, ) .await?; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::TxSetup, ()); for inp in inputs { - send_chunked_expect_empty_ok_response( + let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_TX, ledger_msg::SignP1::Next.into(), - &ledger_msg::encode(ledger_msg::SignTxReq::Input(inp)), + &ledger_msg::encode(ledger_msg::SignTxReq::Input(Box::new(inp))), ) .await?; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::TxNext, ()); } for commitment in input_commitments { - send_chunked_expect_empty_ok_response( + let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_TX, ledger_msg::SignP1::Next.into(), - &ledger_msg::encode(ledger_msg::SignTxReq::InputCommitment(commitment)), + &ledger_msg::encode(ledger_msg::SignTxReq::InputCommitment(Box::new(commitment))), ) .await?; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::TxNext, ()); } - let mut resp = vec![]; + // Send tx outputs and retrieve the first signature from the response for the last output. + // TODO: this won't work if the tx has zero outputs. + let mut sig_resp = vec![]; let num_outputs = outputs.len(); for (idx, o) in outputs.into_iter().enumerate() { + let resp = send_chunked( + ledger, + ledger_msg::Ins::SIGN_TX, + ledger_msg::SignP1::Next.into(), + &ledger_msg::encode(ledger_msg::SignTxReq::Output(Box::new(o))), + ) + .await?; + if idx < num_outputs - 1 { - send_chunked_expect_empty_ok_response( - ledger, - ledger_msg::Ins::SIGN_TX, - ledger_msg::SignP1::Next.into(), - &ledger_msg::encode(ledger_msg::SignTxReq::Output(o)), - ) - .await?; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::TxNext, ()); } else { // the response from the last output will have the first signature returned - resp = send_chunked( - ledger, - ledger_msg::Ins::SIGN_TX, - ledger_msg::SignP1::Next.into(), - &ledger_msg::encode(ledger_msg::SignTxReq::Output(o)), - ) - .await?; - } + sig_resp = resp; + }; } let mut signatures: BTreeMap<_, Vec<_>> = BTreeMap::new(); let next_sig = ledger_msg::encode(ledger_msg::SignTxReq::NextSignature); - let apdu = ledger_msg::Apdu::new_with_data( + let apdu = make_apdu( ledger_msg::Ins::SIGN_TX, ledger_msg::SignP1::Next.into(), &next_sig, - ) - .ok_or(LedgerMessagesError::ApduMessageTooLong)?; + )?; let mut msg_buf = Vec::with_capacity(apdu.bytes_count()); apdu.write_bytes(&mut msg_buf); loop { - let ledger_msg::SignatureResponse { - signature, - input_idx, - has_next, - } = ledger_msg::decode_all(&resp).ok_or(LedgerError::InvalidResponse)?; + let resp = decode_response(&sig_resp)?; + let resp = ensure_response_type!(resp, ledger_msg::Response::TxSignature(resp), resp); - signatures.entry(input_idx as usize).or_default().push(signature); + signatures.entry(resp.input_idx as usize).or_default().push(LedgerSignature { + signature: resp.signature, + multisig_idx: resp.multisig_idx, + }); - if !has_next { + if !resp.has_next { break; } - resp = exchange_message(ledger, &msg_buf).await?; + sig_resp = exchange_message(ledger, &msg_buf).await?; } Ok(signatures) diff --git a/wallet/src/signer/ledger_signer/mod.rs b/wallet/src/signer/ledger_signer/mod.rs index 27c124a06..e600196a6 100644 --- a/wallet/src/signer/ledger_signer/mod.rs +++ b/wallet/src/signer/ledger_signer/mod.rs @@ -17,22 +17,17 @@ mod ledger_messages; use std::{collections::BTreeMap, sync::Arc}; -use crate::{ - Account, WalletResult, - key_chain::{AccountKeyChainImplHardware, AccountKeyChains, FoundPubKey, make_account_path}, - signer::{ - Signer, SignerError, SignerProvider, SignerResult, - hardware_signer_utils::{ - StandaloneInput, StandaloneInputs, arbitrary_message_signature_from_raw_sig, - sign_with_standalone_private_keys, - }, - ledger_signer::ledger_messages::{ - LedgerMessagesError, check_current_app, get_extended_public_key, - get_extended_public_key_raw, sign_challenge, sign_tx, - }, - utils::{is_htlc_utxo, produce_uniparty_signature_for_input}, - }, +use async_trait::async_trait; +use itertools::{Itertools, izip}; +use ledger_lib::{Exchange, Filters, LedgerHandle, LedgerProvider, Transport, info::Model}; +use mintlayer_ledger_messages::{ + AdditionalOrderInfo, AdditionalUtxoInfo, AddrType, Bip32Path as LedgerBip32Path, CoinType, + InputAddressPath as LedgerInputAddressPath, SignatureResponse as LedgerSignatureResponse, + TxInputReq, TxInputWithAdditionalInfo, TxOutputReq, }; +use ml_primitives::SighashInputCommitment as LSighashInputCommitment; +use tokio::sync::Mutex; + use common::{ chain::{ AccountCommand, ChainConfig, Destination, DestinationTag, OrderAccountCommand, @@ -64,6 +59,7 @@ use crypto::key::{ hdkd::{derivable::Derivable, u31::U31}, signature::SignatureKind, }; +use randomness::make_true_rng; use serialization::Encode; use utils::ensure; use wallet_storage::{ @@ -81,78 +77,155 @@ use wallet_types::{ signature_status::SignatureStatus, }; -use async_trait::async_trait; -use itertools::{Itertools, izip}; -use ledger_lib::{Exchange, Filters, LedgerHandle, LedgerProvider, Transport, info::Model}; -use mintlayer_ledger_messages::{ - AdditionalOrderInfo, AdditionalUtxoInfo, AddrType, Bip32Path as LedgerBip32Path, CoinType, - InputAddressPath as LedgerInputAddressPath, SighashInputCommitment as LSighashInputCommitment, - Signature as LedgerSignature, TxInputReq, TxInputWithAdditionalInfo, TxOutputReq, +use crate::{ + Account, WalletResult, + key_chain::{AccountKeyChainImplHardware, AccountKeyChains, FoundPubKey, make_account_path}, + signer::{ + Signer, SignerError, SignerProvider, SignerResult, + hardware_signer_utils::{ + StandaloneInput, StandaloneInputs, arbitrary_message_signature_from_raw_sig, + sign_with_standalone_private_keys, + }, + ledger_signer::ledger_messages::{ + check_current_app, get_extended_public_key, ping, sign_challenge, sign_tx, + }, + utils::{is_htlc_utxo, produce_uniparty_signature_for_input}, + }, }; -use randomness::make_true_rng; -use tokio::sync::Mutex; /// Ledger Signer errors #[derive(thiserror::Error, Debug, Eq, PartialEq)] pub enum LedgerError { + #[error(transparent)] + DeviceError(#[from] LedgerDeviceError), + #[error("No connected Ledger device found")] NoDeviceFound, + #[error("Device timeout")] DeviceTimeout, + #[error( "A different app is currently open on your Ledger device: \"{0}\". Please close it and open the Mintlayer app instead." )] DifferentActiveApp(String), - #[error("Received an invalid response from the Ledger device")] - InvalidResponse, + + #[error("Received an invalid response APDU from the Ledger device")] + InvalidResponseApdu, + + #[error("Received response cannot be decoded")] + CannotDecodeResponse, + + #[error("Received a wrong response from the Ledger device")] + WrongResponse, + #[error("Received an error response from the Ledger device: {0}")] ErrorResponse(String), - #[error("Device error: {0}")] - DeviceError(String), + #[error("Derivation path too long")] DerivationPathTooLong, + #[error("APDU message too long")] ApduMessageTooLong, + #[error("Invalid public key returned from Ledger")] InvalidKey, + #[error( - "The file being loaded is a software wallet and cannot be used with the connected Ledger wallet" + "The file being loaded is a software wallet and cannot be used with the connected Ledger device" )] WalletFileIsSoftwareWallet, + #[error( - "The file being loaded is a trezor wallet and cannot be used with the connected Ledger wallet" + "The file being loaded is a Trezor wallet and cannot be used with the connected Ledger device" )] WalletFileIsTrezorWallet, + #[error("Public keys mismatch - wrong device or passphrase")] HardwareWalletDifferentMnemonicOrPassphrase, + #[error("A multisig signature was returned for a single address from Device")] MultisigSignatureReturned, + #[error("Multiple signatures returned for a single address from Device")] MultipleSignaturesReturned, + #[error("Missing multisig index for signature returned from Device")] MissingMultisigIndexForSignature, + #[error("Signature error: {0}")] SignatureError(#[from] SignatureError), + #[error("Input commitments version 0 is not supported by the Ledger app")] InputCommitmentVersion1NotSupported, } -impl From for LedgerError { +#[derive(Debug, Eq, PartialEq, Copy, Clone)] +pub enum LedgerDeviceErrorKind { + Hid, + Tcp, + Ble, + Timeout, + Other, +} + +/// Ledger device error. +/// +/// Note: we can't put `ledger_lib::Error` into `LedgerError` directly because the latter needs `Eq`, +/// which the former doesn't implement. +#[derive(thiserror::Error, Debug, Eq, PartialEq)] +#[error("{message}")] +pub struct LedgerDeviceError { + pub kind: LedgerDeviceErrorKind, + pub message: String, +} + +impl From for LedgerDeviceError { fn from(value: ledger_lib::Error) -> Self { - Self::DeviceError(value.to_string()) + let kind = match &value { + ledger_lib::Error::Hid(_) => LedgerDeviceErrorKind::Hid, + ledger_lib::Error::Tcp(_) => LedgerDeviceErrorKind::Tcp, + ledger_lib::Error::Ble(_) => LedgerDeviceErrorKind::Ble, + ledger_lib::Error::Timeout => LedgerDeviceErrorKind::Timeout, + + ledger_lib::Error::ApduSentToUnknownDeviceHandle + | ledger_lib::Error::RequestChannelClosed + | ledger_lib::Error::RequestResponseChannelClosed + | ledger_lib::Error::UnexpectedResponseWhileListingDevices + | ledger_lib::Error::UnexpectedResponseWhileConnecting + | ledger_lib::Error::UnexpectedResponseWhileExchangingData + | ledger_lib::Error::NoDevices + | ledger_lib::Error::InvalidDeviceIndex(_) + | ledger_lib::Error::Apdu(_) + | ledger_lib::Error::Status(_) + | ledger_lib::Error::UnknownStatus(_, _) + | ledger_lib::Error::Closed + | ledger_lib::Error::EmptyResponse + | ledger_lib::Error::UnexpectedResponse + | ledger_lib::Error::DeviceInUse + | ledger_lib::Error::CannotReadBleDeviceProperties + | ledger_lib::Error::CannotFindBleDeviceSpecs + | ledger_lib::Error::NotConnectedAfterSuccessfulBleConnect + | ledger_lib::Error::MissingReadOrWriteBleCharacteristics + | ledger_lib::Error::UnexpectedMtuResponse => LedgerDeviceErrorKind::Other, + }; + let message = value.to_string(); + + Self { kind, message } } } -impl From for LedgerError { - fn from(value: LedgerMessagesError) -> Self { - match value { - LedgerMessagesError::DerivationPathTooLong => Self::DerivationPathTooLong, - LedgerMessagesError::ApduMessageTooLong => Self::ApduMessageTooLong, - LedgerMessagesError::DeviceError(err) => err.into(), - } +impl From for LedgerError { + fn from(value: ledger_lib::Error) -> Self { + Self::DeviceError(value.into()) } } +pub struct LedgerSignature { + pub signature: LedgerSignatureResponse, + pub multisig_idx: Option, +} + #[async_trait] pub trait LedgerFinder { type Ledger; @@ -212,11 +285,9 @@ where let mut client = self.client.lock().await; // Try and wait around 50 * TIMEOUT_DUR for the screen to clear after a signing operation ends let mut num_tries = 50; - let derivation_path = make_account_path(&self.chain_config, key_chain.account_index()); - let coin_type = to_ledger_chain_type(&self.chain_config); loop { - match get_extended_public_key_raw(&mut *client, coin_type, &derivation_path).await { - Ok(_) => { + match ping(&mut *client).await { + Ok(()) => { check_public_keys_against_key_chain( db_tx, &mut *client, @@ -226,40 +297,56 @@ where .await?; return Ok(()); } - // After finishing a signing operation the device shows a status success/failed. - // At those times any command sent is not handled so waiting for a response will - // just timeout. - Err(LedgerMessagesError::DeviceError(ledger_lib::Error::Timeout)) => { - num_tries -= 1; - if num_tries > 0 { - continue; - } else { - return Err(SignerError::LedgerError(LedgerError::DeviceTimeout)); - } - } - // In case of a communication error try to reconnect, and try again - Err(LedgerMessagesError::DeviceError( - ledger_lib::Error::Hid(_) - | ledger_lib::Error::Tcp(_) - | ledger_lib::Error::Ble(_), - )) => { - let (mut new_client, _data) = self - .provider - .find_ledger_device_from_db(db_tx, self.chain_config.clone()) - .await?; + Err(err) => { + match &err { + LedgerError::DeviceError(device_err) => { + match device_err.kind { + // After finishing a signing operation the device shows a status success/failed. + // At those times any command sent is not handled so waiting for a response will + // just timeout. + LedgerDeviceErrorKind::Timeout => { + num_tries -= 1; + if num_tries > 0 { + continue; + } else { + return Err(SignerError::LedgerError( + LedgerError::DeviceTimeout, + )); + } + } - check_public_keys_against_key_chain( - db_tx, - &mut new_client, - key_chain, - &self.chain_config, - ) - .await?; + // In case of a communication error try to reconnect, and try again + LedgerDeviceErrorKind::Hid + | LedgerDeviceErrorKind::Tcp + | LedgerDeviceErrorKind::Ble => { + let (mut new_client, _data) = self + .provider + .find_ledger_device_from_db( + db_tx, + self.chain_config.clone(), + ) + .await?; + + check_public_keys_against_key_chain( + db_tx, + &mut new_client, + key_chain, + &self.chain_config, + ) + .await?; + + *client = new_client; + return Ok(()); + } - *client = new_client; - return Ok(()); + LedgerDeviceErrorKind::Other => { + return Err(err.into()); + } + } + } + _ => return Err(err.into()), + } } - Err(err) => return Err(SignerError::LedgerError(err.into())), } } } @@ -311,7 +398,7 @@ where .ok_or(SignerError::DestinationNotFromThisWallet)? .into_public_key(); let sig = Signature::from_raw_data( - signature.signature, + signature.signature.0, SignatureKind::Secp256k1Schnorr, ) .map_err(LedgerError::SignatureError)?; @@ -338,7 +425,7 @@ where Destination::PublicKey(_) => { if let Some(signature) = single_signature(signatures)? { let sig = Signature::from_raw_data( - signature.signature, + signature.signature.0, SignatureKind::Secp256k1Schnorr, ) .map_err(LedgerError::SignatureError)?; @@ -444,7 +531,7 @@ where ) -> SignerResult<(AuthorizedClassicalMultisigSpend, SignatureStatus)> { for sig in signatures { let idx = sig.multisig_idx.ok_or(LedgerError::MissingMultisigIndexForSignature)?; - let sig = Signature::from_raw_data(sig.signature, SignatureKind::Secp256k1Schnorr) + let sig = Signature::from_raw_data(sig.signature.0, SignatureKind::Secp256k1Schnorr) .map_err(LedgerError::SignatureError)?; current_signatures.add_signature(idx as u8, sig); } @@ -727,7 +814,7 @@ where ) .await?; - arbitrary_message_signature_from_raw_sig(&sig, destination.into(), xpub) + arbitrary_message_signature_from_raw_sig(&sig.0, destination.into(), xpub) } Some(FoundPubKey::Standalone(acc_public_key)) => { let standalone_pk = db_tx @@ -821,7 +908,7 @@ fn to_ledger_tx_input_with_additional_info( let pool_info = additional_info .get_pool_info(pool_id) .ok_or(SignerError::MissingTxExtraInfo)?; - AdditionalUtxoInfo::PoolData { + AdditionalUtxoInfo::UtxoWithPoolData { utxo: utxo.clone().try_convert_into()?, staker_balance: pool_info.staker_balance.try_convert_into()?, } diff --git a/wallet/src/signer/ledger_signer/tests/mod.rs b/wallet/src/signer/ledger_signer/tests/mod.rs index f6f71462b..01ccba31e 100644 --- a/wallet/src/signer/ledger_signer/tests/mod.rs +++ b/wallet/src/signer/ledger_signer/tests/mod.rs @@ -41,9 +41,7 @@ use crate::signer::{ SignerError, SignerResult, ledger_signer::{ LedgerError, LedgerFinder, LedgerSigner, - ledger_messages::{ - check_current_app, get_extended_public_key, get_extended_public_key_raw, - }, + ledger_messages::{check_current_app, get_extended_public_key, ping}, }, tests::{ generic_fixed_signature_tests::test_fixed_signatures_generic_no_legacy, @@ -142,10 +140,9 @@ impl LedgerFinder for DummyProvider { async fn wait_for_valid_reponse(device: &mut TcpDevice) { let mut tries = 0; - let derivation_path = DerivationPath::from_str("m/44h/19788h/0h").unwrap(); loop { - match get_extended_public_key_raw(device, CoinType::Mainnet, &derivation_path).await { - Ok(_) => break, + match ping(device).await { + Ok(()) => break, Err(_) => { tries += 1; assert!( @@ -355,7 +352,7 @@ async fn test_sign_message_sig_consistency(#[case] seed: Seed) { test_sign_message_generic( &mut rng, - MessageToSign::Random, + MessageToSign::RandomShort, make_ledger_signer, Some(make_deterministic_software_signer), ) diff --git a/wallet/src/signer/software_signer/tests.rs b/wallet/src/signer/software_signer/tests.rs index 8ec1aa2b3..7a4b9915a 100644 --- a/wallet/src/signer/software_signer/tests.rs +++ b/wallet/src/signer/software_signer/tests.rs @@ -24,23 +24,24 @@ use crate::signer::tests::{ test_fixed_signatures_generic_no_legacy, test_fixed_signatures_generic2, }, generic_tests::{ - MessageToSign, test_sign_message_generic, test_sign_transaction_generic, - test_sign_transaction_intent_generic, + MessageToSign, sign_message_test_params, test_sign_message_generic, + test_sign_transaction_generic, test_sign_transaction_intent_generic, }, make_deterministic_software_signer, make_software_signer, make_software_signer_for_cold_wallet, no_another_signer, }; +#[rstest_reuse::apply(sign_message_test_params)] #[rstest] #[trace] #[case(Seed::from_entropy())] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] -async fn test_sign_message(#[case] seed: Seed) { +async fn test_sign_message(#[case] seed: Seed, message_to_sign: MessageToSign) { let mut rng = make_seedable_rng(seed); test_sign_message_generic( &mut rng, - MessageToSign::Random, + message_to_sign, make_software_signer, no_another_signer(), ) diff --git a/wallet/src/signer/tests/generic_tests.rs b/wallet/src/signer/tests/generic_tests.rs index f4c2a04db..14090779b 100644 --- a/wallet/src/signer/tests/generic_tests.rs +++ b/wallet/src/signer/tests/generic_tests.rs @@ -77,24 +77,25 @@ use crate::{ #[derive(Debug)] pub enum MessageToSign { - Random, + RandomShort, + + RandomLong, // We only use this in trezor tests at this moment. #[allow(dead_code)] Predefined(Vec), } -#[cfg(any( - feature = "enable-trezor-device-tests", - feature = "enable-ledger-device-tests" -))] #[rstest_reuse::template] pub fn sign_message_test_params( #[values( - MessageToSign::Random, + MessageToSign::RandomShort, + MessageToSign::RandomLong, // Special case: an "overlong" utf-8 string (basically, the letter 'K' encoded with 2 bytes // instead of 1). Trezor firmware used to have troubles with this. - MessageToSign::Predefined(vec![193, 139]) + MessageToSign::Predefined(vec![193, 139]), + // Special case: ASCII control characters. Ledger was not handling them properly. + MessageToSign::Predefined(vec![0, 3]) )] message_to_sign: MessageToSign, ) { @@ -122,9 +123,8 @@ pub async fn test_sign_message_generic( let mut rng = make_seedable_rng(rng.random()); move || { let msg = match &message_to_sign { - MessageToSign::Random => { - vec![rng.random::(), rng.random::(), rng.random::()] - } + MessageToSign::RandomShort => gen_random_bytes(&mut rng, 3, 5), + MessageToSign::RandomLong => gen_random_bytes(&mut rng, 300, 500), MessageToSign::Predefined(msg) => msg.clone(), }; log::debug!("Using message: {msg:?}"); @@ -391,7 +391,8 @@ pub async fn test_sign_transaction_generic( .unwrap(); let standalone_pk_destination = Destination::PublicKey(standalone_pk.clone()); - let coin_input_amounts: Vec = (0..rng.random_range(2..7)) + let min_input_amounts = 4; // 1 utxo, 1 standalone, 1 create pool, 1 htlc + let coin_input_amounts: Vec = (0..rng.random_range(min_input_amounts..7)) .map(|_| Amount::from_atoms(rng.random_range(100..1000))) .collect(); @@ -426,7 +427,7 @@ pub async fn test_sign_transaction_generic( TxOutput::ProduceBlockFromStake(random_destination(rng), decommissioned_pool_id); let utxos: Vec = coin_input_amounts .iter() - .skip(1) + .skip(min_input_amounts - 1) .map(|a| { let dest = destination_from_account(&mut account, &mut db_tx, rng); @@ -563,10 +564,8 @@ pub async fn test_sign_transaction_generic( }; let htlc_input = TxInput::from_utxo(source_id, rng.next_u32()); - let htlc_utxo = TxOutput::Htlc( - OutputValue::Coin(Amount::from_atoms(rng.random::() as u128)), - Box::new(htlc.clone()), - ); + let htlc_amount = coin_input_amounts[1]; + let htlc_utxo = TxOutput::Htlc(OutputValue::Coin(htlc_amount), Box::new(htlc.clone())); let token_id = TokenId::new(H256::random_using(rng)); let token_mint_amount = Amount::from_atoms(rng.random_range(100..200)); @@ -577,6 +576,7 @@ pub async fn test_sign_transaction_generic( let coin_burn_amount = total_coin_input_amount.div(rng.random_range(10..20)).unwrap(); let delegate_staking_amount = total_coin_input_amount.div(rng.random_range(10..20)).unwrap(); let htlc_transfer_amount = total_coin_input_amount.div(rng.random_range(10..20)).unwrap(); + let created_order_ask = total_coin_input_amount.div(rng.random_range(10..20)).unwrap(); let filled_order1_id = OrderId::new(H256::random_using(rng)); let filled_order2_id = OrderId::new(H256::random_using(rng)); @@ -693,8 +693,9 @@ pub async fn test_sign_transaction_generic( let created_pool_id = PoolId::new(H256::random_using(rng)); let delegation_id = DelegationId::new(H256::random_using(rng)); + let pool_amount = coin_input_amounts[2]; let pool_data = StakePoolData::new( - Amount::from_atoms(rng.random_range(100..200)), + pool_amount, Destination::PublicKey(dest_pub.clone()), vrf_public_key, Destination::PublicKey(dest_pub.clone()), @@ -744,8 +745,8 @@ pub async fn test_sign_transaction_generic( let created_order_data = OrderData::new( Destination::PublicKey(dest_pub.clone()), - OutputValue::Coin(Amount::from_atoms(rng.random_range(100..200))), OutputValue::TokenV1(token_id, Amount::from_atoms(rng.random_range(100..200))), + OutputValue::Coin(created_order_ask), ); let outputs = vec![ diff --git a/wallet/src/signer/trezor_signer/mod.rs b/wallet/src/signer/trezor_signer/mod.rs index d386bfd13..5df7b5358 100644 --- a/wallet/src/signer/trezor_signer/mod.rs +++ b/wallet/src/signer/trezor_signer/mod.rs @@ -155,11 +155,11 @@ pub enum TrezorError { #[error("A multisig signature was returned for a single address from Device")] MultisigSignatureReturned, #[error( - "The file being loaded is a Ledger wallet and cannot be used with the connected Trezor wallet" + "The file being loaded is a Ledger wallet and cannot be used with the connected Trezor device" )] WalletFileIsLedgerWallet, #[error( - "The file being loaded is a software wallet and cannot be used with the connected Trezor wallet" + "The file being loaded is a software wallet and cannot be used with the connected Trezor device" )] WalletFileIsSoftwareWallet, #[error( diff --git a/wallet/src/signer/trezor_signer/tests.rs b/wallet/src/signer/trezor_signer/tests.rs index ad0c763e5..bf58f9c41 100644 --- a/wallet/src/signer/trezor_signer/tests.rs +++ b/wallet/src/signer/trezor_signer/tests.rs @@ -233,7 +233,7 @@ async fn test_sign_message_sig_consistency(#[case] seed: Seed) { test_sign_message_generic( &mut rng, - MessageToSign::Random, + MessageToSign::RandomShort, make_deterministic_trezor_signer, Some(make_deterministic_software_signer), )