diff --git a/Cargo.lock b/Cargo.lock index bf292ffe6f..d2a704a527 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5177,11 +5177,12 @@ dependencies = [ [[package]] name = "mintlayer-messages" -version = "0.1.0" -source = "git+https://github.com/mintlayer/mintlayer-ledger-app?rev=16dce6ab619529d5b8f506cb6c2e4f62199b25d7#16dce6ab619529d5b8f506cb6c2e4f62199b25d7" +version = "1.0.0" +source = "git+https://github.com/mintlayer/mintlayer-ledger-app?rev=44988a5ba1efc8b008a8ec61f32e4d7596999023#44988a5ba1efc8b008a8ec61f32e4d7596999023" dependencies = [ "derive_more 2.1.1", "mintlayer-core-primitives", + "num-traits", "num_enum", "parity-scale-codec", ] @@ -9882,7 +9883,6 @@ dependencies = [ "ledger-lib", "logging", "mempool", - "mintlayer-core-primitives", "mintlayer-messages", "orders-accounting", "parity-scale-codec", diff --git a/Cargo.toml b/Cargo.toml index dcb0342e3b..f89b374602 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -300,8 +300,8 @@ rev = "c8ed12e89788e78d77cdc0dc9fb8a4bd4dc24b89" [workspace.dependencies.mintlayer-ledger-messages] git = "https://github.com/mintlayer/mintlayer-ledger-app" -# The commit "Add technical specification" -rev = "16dce6ab619529d5b8f506cb6c2e4f62199b25d7" +# The commit "Merge pull request #22 from mintlayer/fix_core_repo_build" +rev = "44988a5ba1efc8b008a8ec61f32e4d7596999023" package = "mintlayer-messages" [workspace.dependencies.trezor-client] diff --git a/common/src/chain/transaction/signature/sighash/input_commitments/mod.rs b/common/src/chain/transaction/signature/sighash/input_commitments/mod.rs index fc9417d7f5..3a7a42f8ed 100644 --- a/common/src/chain/transaction/signature/sighash/input_commitments/mod.rs +++ b/common/src/chain/transaction/signature/sighash/input_commitments/mod.rs @@ -44,6 +44,14 @@ pub use info_providers::{ /// with the `Option`, provided that only `None` and `Utxo` variants are used. /// 2) The `ProduceBlockFromStakeUtxo`, `FillOrderAccountCommand` and `ConcludeOrderAccountCommand` /// commitments are enabled since `SighashInputCommitmentVersion::V1`. +/// +/// TODO: next time there is a need to update input commitments, consider also committing to +/// the destination that the input is signed against (i.e. order conclude key, token authority, +/// delegation owner); this will allow hardware wallets to prove (and therefore claim on the screen +/// during input review) that a particular input is signed with a particular key (note that in the +/// SigHashType::ALL case each input signature is a signature over the entire tx, so the host may +/// claim that inputs x and y are signed with keys A and B respectively, while in reality it's vice +/// versa, and hardware wallets have no way of verifying this in the general case). #[derive(Clone, Debug, Encode, Decode, Eq, PartialEq, EnumDiscriminants)] #[strum_discriminants(name(SighashInputCommitmentTag), derive(EnumIter))] pub enum SighashInputCommitment<'a> { diff --git a/wallet/Cargo.toml b/wallet/Cargo.toml index 2c8c1b3c9f..fba23b3a6e 100644 --- a/wallet/Cargo.toml +++ b/wallet/Cargo.toml @@ -33,7 +33,6 @@ 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 8797dda071..8eaa692e23 100644 --- a/wallet/src/signer/ledger_signer/ledger_messages.rs +++ b/wallet/src/signer/ledger_signer/ledger_messages.rs @@ -27,7 +27,7 @@ use utils::ensure; use crate::signer::ledger_signer::LedgerError; -use super::{LSighashInputCommitment, LedgerSignature}; +use super::LedgerSignature; macro_rules! ensure_response_type { ($resp:expr, $pattern:pat $(if $guard:expr)?, $out:expr) => { @@ -132,8 +132,8 @@ pub async fn sign_challenge( path: ledger_msg::Bip32Path, addr_type: ledger_msg::AddrType, message: &[u8], -) -> Result { - let req = ledger_msg::SignMessageReq { +) -> Result { + let req = ledger_msg::SignMessageStartReq { coin, addr_type, path, @@ -142,7 +142,7 @@ pub async fn sign_challenge( let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_MSG, - ledger_msg::SignP1::Start.into(), + ledger_msg::SignMsgP1::Start.into(), &ledger_msg::encode(req), ) .await?; @@ -152,7 +152,7 @@ pub async fn sign_challenge( let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_MSG, - ledger_msg::SignP1::Next.into(), + ledger_msg::SignMsgP1::Next.into(), message, ) .await?; @@ -176,7 +176,7 @@ pub async fn check_current_app( } pub async fn ping(ledger: &mut L) -> Result<(), LedgerError> { - let apdu = make_apdu(ledger_msg::Ins::PING, ledger_msg::PingP1::Start.into(), &[])?; + let apdu = make_apdu(ledger_msg::Ins::PING, ledger_msg::PingP1::Dummy.into(), &[])?; let mut msg_buf = Vec::with_capacity(apdu.bytes_count()); apdu.write_bytes(&mut msg_buf); @@ -196,12 +196,12 @@ pub async fn get_extended_public_key( 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 req = ledger_msg::GetPubKeyReq { coin_type, path }; let resp = send_chunked( ledger, - ledger_msg::Ins::PUB_KEY, - ledger_msg::PubKeyP1::NoDisplayAddress.into(), + ledger_msg::Ins::GET_PUB_KEY, + ledger_msg::GetPubKeyP1::NoDisplayAddress.into(), &ledger_msg::encode(req), ) .await?; @@ -221,34 +221,33 @@ pub async fn get_extended_public_key( pub async fn sign_tx( ledger: &mut L, chain_type: ledger_msg::CoinType, - inputs: Vec, - input_commitments: Vec, - outputs: Vec, + inputs: Vec, + input_commitments: Vec, + outputs: Vec, ) -> Result>, LedgerError> { - let metadata = ledger_msg::encode(ledger_msg::TxMetadataReq { + let start_req = ledger_msg::encode(ledger_msg::SignTxStartReq { coin: chain_type, - version: ledger_msg::TxMetadataVersionReq::V1(ledger_msg::TxMetadataV1Req { - num_inputs: inputs.len() as u32, - num_outputs: outputs.len() as u32, - }), + version: ledger_msg::TransactionVersion::V1, + num_inputs: inputs.len() as u32, + num_outputs: outputs.len() as u32, }); let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_TX, - ledger_msg::SignP1::Start.into(), - &metadata, + ledger_msg::SignTxP1::Start.into(), + &start_req, ) .await?; let resp = decode_response(&resp)?; ensure_response_type!(resp, ledger_msg::Response::TxSetup, ()); - for inp in inputs { + for input in inputs { let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_TX, - ledger_msg::SignP1::Next.into(), - &ledger_msg::encode(ledger_msg::SignTxReq::Input(Box::new(inp))), + ledger_msg::SignTxP1::Next.into(), + &ledger_msg::encode(ledger_msg::SignTxNextReq::ProcessInput(Box::new(input))), ) .await?; let resp = decode_response(&resp)?; @@ -259,51 +258,49 @@ pub async fn sign_tx( let resp = send_chunked( ledger, ledger_msg::Ins::SIGN_TX, - ledger_msg::SignP1::Next.into(), - &ledger_msg::encode(ledger_msg::SignTxReq::InputCommitment(Box::new(commitment))), + ledger_msg::SignTxP1::Next.into(), + &ledger_msg::encode(ledger_msg::SignTxNextReq::ProcessInputCommitment(Box::new( + ledger_msg::TxInputCommitmentData { commitment }, + ))), ) .await?; let resp = decode_response(&resp)?; ensure_response_type!(resp, ledger_msg::Response::TxNext, ()); } - // 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() { + for output in outputs { 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))), + ledger_msg::SignTxP1::Next.into(), + &ledger_msg::encode(ledger_msg::SignTxNextReq::ProcessOutput(Box::new(output))), ) .await?; - if idx < num_outputs - 1 { - 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 - sig_resp = resp; - }; + let resp = decode_response(&resp)?; + ensure_response_type!(resp, ledger_msg::Response::TxNext, ()); } - let mut signatures: BTreeMap<_, Vec<_>> = BTreeMap::new(); + let next_sig_raw_req = { + let next_sig = ledger_msg::encode(ledger_msg::SignTxNextReq::ReturnNextSignature); + let apdu = make_apdu( + ledger_msg::Ins::SIGN_TX, + ledger_msg::SignTxP1::Next.into(), + &next_sig, + )?; - let next_sig = ledger_msg::encode(ledger_msg::SignTxReq::NextSignature); - let apdu = make_apdu( - ledger_msg::Ins::SIGN_TX, - ledger_msg::SignP1::Next.into(), - &next_sig, - )?; + let mut msg_buf = Vec::with_capacity(apdu.bytes_count()); + apdu.write_bytes(&mut msg_buf); + msg_buf + }; - let mut msg_buf = Vec::with_capacity(apdu.bytes_count()); - apdu.write_bytes(&mut msg_buf); + let mut signatures: BTreeMap<_, Vec<_>> = BTreeMap::new(); loop { + let sig_resp = exchange_message(ledger, &next_sig_raw_req).await?; + let resp = decode_response(&sig_resp)?; - let resp = ensure_response_type!(resp, ledger_msg::Response::TxSignature(resp), resp); + let resp = ensure_response_type!(resp, ledger_msg::Response::TxInputSignature(resp), resp); signatures.entry(resp.input_idx as usize).or_default().push(LedgerSignature { signature: resp.signature, @@ -313,8 +310,6 @@ pub async fn sign_tx( if !resp.has_next { break; } - - 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 e600196a6e..42a812f372 100644 --- a/wallet/src/signer/ledger_signer/mod.rs +++ b/wallet/src/signer/ledger_signer/mod.rs @@ -20,12 +20,7 @@ use std::{collections::BTreeMap, sync::Arc}; 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 mintlayer_ledger_messages as ledger_msg; use tokio::sync::Mutex; use common::{ @@ -222,7 +217,7 @@ impl From for LedgerError { } pub struct LedgerSignature { - pub signature: LedgerSignatureResponse, + pub signature: ledger_msg::Signature, pub multisig_idx: Option, } @@ -479,16 +474,16 @@ where } } - fn to_ledger_output_msgs( + fn to_ledger_output_data( &self, ptx: &PartiallySignedTransaction, - ) -> SignerResult> { + ) -> SignerResult> { ptx.tx() .outputs() .iter() .map(|out| { - Ok(TxOutputReq { - out: out.clone().try_convert_into()?, + Ok(ledger_msg::TxOutputData { + output: out.clone().try_convert_into()?, }) }) .collect() @@ -562,8 +557,8 @@ where )> { let (inputs, standalone_inputs) = to_ledger_input_msgs(&ptx, key_chain, &db_tx)?; let input_commitments = to_ledger_input_commitments_reqs(&ptx)?; - let outputs = self.to_ledger_output_msgs(&ptx)?; - let coin_type = to_ledger_chain_type(&self.chain_config); + let outputs = self.to_ledger_output_data(&ptx)?; + let coin_type = make_ledger_coin_type(&self.chain_config); let input_commitment_version = self .chain_config @@ -800,7 +795,7 @@ where Some(FoundPubKey::Hierarchy(xpub)) => { let address_n = to_ledger_bip32_path(&xpub); let addr_type = to_ledger_addr_type(destination.into())?; - let coin_type = to_ledger_chain_type(&self.chain_config); + let coin_type = make_ledger_coin_type(&self.chain_config); let message = message.to_vec(); let sig = self .perform_ledger_operation( @@ -868,7 +863,7 @@ fn to_ledger_input_msgs( ptx: &PartiallySignedTransaction, key_chain: &impl AccountKeyChains, db_tx: &impl WalletStorageReadUnlocked, -) -> SignerResult<(Vec, StandaloneInputs)> { +) -> SignerResult<(Vec, StandaloneInputs)> { let res: (Vec<_>, BTreeMap<_, _>) = itertools::process_results( ptx.tx() .inputs() @@ -882,8 +877,12 @@ fn to_ledger_input_msgs( destination_to_address_paths(key_chain, dest, db_tx) })?; - let input = TxInputReq { - inp: to_ledger_tx_input_with_additional_info(inp, utxo, ptx.additional_info())?, + let input = ledger_msg::TxInputData { + input: to_ledger_tx_input_with_additional_info( + inp, + utxo, + ptx.additional_info(), + )?, addresses: address_paths, }; @@ -899,7 +898,7 @@ fn to_ledger_tx_input_with_additional_info( inp: &TxInput, utxo: &Option, additional_info: &PtxAdditionalInfo, -) -> SignerResult { +) -> SignerResult { let inp = match inp { TxInput::Utxo(outpoint) => { let utxo = utxo.as_ref().ok_or(SignerError::MissingUtxo)?; @@ -908,30 +907,32 @@ fn to_ledger_tx_input_with_additional_info( let pool_info = additional_info .get_pool_info(pool_id) .ok_or(SignerError::MissingTxExtraInfo)?; - AdditionalUtxoInfo::UtxoWithPoolData { + ledger_msg::AdditionalUtxoInfo::UtxoWithPoolData { utxo: utxo.clone().try_convert_into()?, staker_balance: pool_info.staker_balance.try_convert_into()?, } } - _ => AdditionalUtxoInfo::Utxo(utxo.clone().try_convert_into()?), + _ => ledger_msg::AdditionalUtxoInfo::Utxo(utxo.clone().try_convert_into()?), }; - TxInputWithAdditionalInfo::Utxo(outpoint.clone().try_convert_into()?, info) + ledger_msg::TxInputWithAdditionalInfo::Utxo(outpoint.clone().try_convert_into()?, info) } TxInput::Account(acc) => { - TxInputWithAdditionalInfo::Account(acc.clone().try_convert_into()?) + ledger_msg::TxInputWithAdditionalInfo::Account(acc.clone().try_convert_into()?) + } + TxInput::AccountCommand(nonce, cmd) => { + ledger_msg::TxInputWithAdditionalInfo::AccountCommand( + (*nonce).try_convert_into()?, + cmd.clone().try_convert_into()?, + ) } - TxInput::AccountCommand(nonce, cmd) => TxInputWithAdditionalInfo::AccountCommand( - (*nonce).try_convert_into()?, - cmd.clone().try_convert_into()?, - ), TxInput::OrderAccountCommand(cmd) => { let info = additional_info .get_order_info(cmd.order_id()) .ok_or(SignerError::MissingTxExtraInfo)?; - TxInputWithAdditionalInfo::OrderAccountCommand( + ledger_msg::TxInputWithAdditionalInfo::OrderAccountCommand( cmd.clone().try_convert_into()?, - AdditionalOrderInfo { + ledger_msg::AdditionalOrderInfo { initially_asked: info.initially_asked.clone().try_convert_into()?, initially_given: info.initially_given.clone().try_convert_into()?, ask_balance: info.ask_balance.try_convert_into()?, @@ -948,7 +949,7 @@ fn destination_to_address_paths( key_chain: &impl AccountKeyChains, dest: &Destination, db_tx: &impl WalletStorageReadUnlocked, -) -> SignerResult<(Vec, Vec)> { +) -> SignerResult<(Vec, Vec)> { destination_to_address_paths_impl(key_chain, dest, None, db_tx) } @@ -957,7 +958,7 @@ fn destination_to_address_paths_impl( dest: &Destination, multisig_idx: Option, db_tx: &impl WalletStorageReadUnlocked, -) -> SignerResult<(Vec, Vec)> { +) -> SignerResult<(Vec, Vec)> { match key_chain.find_public_key(dest) { Some(FoundPubKey::Hierarchy(xpub)) => { let address_n = xpub @@ -967,8 +968,8 @@ fn destination_to_address_paths_impl( .map(|c| c.into_encoded_index()) .collect(); Ok(( - vec![LedgerInputAddressPath { - path: LedgerBip32Path(address_n), + vec![ledger_msg::InputAddressPath { + path: ledger_msg::Bip32Path(address_n), multisig_idx, }], vec![], @@ -1012,7 +1013,7 @@ fn destination_to_address_paths_impl( fn to_ledger_input_commitments_reqs( ptx: &PartiallySignedTransaction, -) -> SignerResult> { +) -> SignerResult> { ptx.input_utxos() .iter() .zip(ptx.tx().inputs()) @@ -1026,15 +1027,17 @@ fn to_ledger_input_commitments_reqs( .additional_info() .get_pool_info(pool_id) .ok_or(SignerError::MissingTxExtraInfo)?; - LSighashInputCommitment::ProduceBlockFromStakeUtxo { + ledger_msg::SighashInputCommitment::ProduceBlockFromStakeUtxo { utxo: utxo.clone().try_convert_into()?, staker_balance: pool_info.staker_balance.try_convert_into()?, } } - _ => LSighashInputCommitment::Utxo(utxo.clone().try_convert_into()?), + _ => ledger_msg::SighashInputCommitment::Utxo( + utxo.clone().try_convert_into()?, + ), } } - TxInput::Account(_) => LSighashInputCommitment::None, + TxInput::Account(_) => ledger_msg::SighashInputCommitment::None, TxInput::AccountCommand(_, cmd) => match cmd { AccountCommand::MintTokens(_, _) | AccountCommand::UnmintTokens(_) @@ -1042,13 +1045,15 @@ fn to_ledger_input_commitments_reqs( | AccountCommand::FreezeToken(_, _) | AccountCommand::UnfreezeToken(_) | AccountCommand::ChangeTokenAuthority(_, _) - | AccountCommand::ChangeTokenMetadataUri(_, _) => LSighashInputCommitment::None, + | AccountCommand::ChangeTokenMetadataUri(_, _) => { + ledger_msg::SighashInputCommitment::None + } AccountCommand::FillOrder(order_id, _, _) => { let order_info = ptx .additional_info() .get_order_info(order_id) .ok_or(SignerError::MissingTxExtraInfo)?; - LSighashInputCommitment::FillOrderAccountCommand { + ledger_msg::SighashInputCommitment::FillOrderAccountCommand { initially_asked: order_info .initially_asked .clone() @@ -1064,7 +1069,7 @@ fn to_ledger_input_commitments_reqs( .additional_info() .get_order_info(order_id) .ok_or(SignerError::MissingTxExtraInfo)?; - LSighashInputCommitment::ConcludeOrderAccountCommand { + ledger_msg::SighashInputCommitment::ConcludeOrderAccountCommand { initially_asked: order_info .initially_asked .clone() @@ -1084,7 +1089,7 @@ fn to_ledger_input_commitments_reqs( .additional_info() .get_order_info(order_id) .ok_or(SignerError::MissingTxExtraInfo)?; - LSighashInputCommitment::FillOrderAccountCommand { + ledger_msg::SighashInputCommitment::FillOrderAccountCommand { initially_asked: order_info .initially_asked .clone() @@ -1100,7 +1105,7 @@ fn to_ledger_input_commitments_reqs( .additional_info() .get_order_info(order_id) .ok_or(SignerError::MissingTxExtraInfo)?; - LSighashInputCommitment::ConcludeOrderAccountCommand { + ledger_msg::SighashInputCommitment::ConcludeOrderAccountCommand { initially_asked: order_info .initially_asked .clone() @@ -1113,7 +1118,7 @@ fn to_ledger_input_commitments_reqs( give_balance: order_info.give_balance.try_convert_into()?, } } - OrderAccountCommand::FreezeOrder(_) => LSighashInputCommitment::None, + OrderAccountCommand::FreezeOrder(_) => ledger_msg::SighashInputCommitment::None, }, }; Ok(additional_info) @@ -1121,19 +1126,19 @@ fn to_ledger_input_commitments_reqs( .collect() } -fn to_ledger_chain_type(chain_config: &ChainConfig) -> CoinType { +fn make_ledger_coin_type(chain_config: &ChainConfig) -> ledger_msg::CoinType { match chain_config.chain_type() { - ChainType::Mainnet => CoinType::Mainnet, - ChainType::Testnet => CoinType::Testnet, - ChainType::Signet => CoinType::Signet, - ChainType::Regtest => CoinType::Regtest, + ChainType::Mainnet => ledger_msg::CoinType::Mainnet, + ChainType::Testnet => ledger_msg::CoinType::Testnet, + ChainType::Signet => ledger_msg::CoinType::Signet, + ChainType::Regtest => ledger_msg::CoinType::Regtest, } } -fn to_ledger_addr_type(dest: DestinationTag) -> SignerResult { +fn to_ledger_addr_type(dest: DestinationTag) -> SignerResult { match dest { - DestinationTag::PublicKey => Ok(AddrType::PublicKey), - DestinationTag::PublicKeyHash => Ok(AddrType::PublicKeyHash), + DestinationTag::PublicKey => Ok(ledger_msg::AddrType::PublicKey), + DestinationTag::PublicKeyHash => Ok(ledger_msg::AddrType::PublicKeyHash), DestinationTag::AnyoneCanSpend => { Err(SignerError::SigningError( DestinationSigError::AttemptedToProduceSignatureForAnyoneCanSpend, @@ -1152,8 +1157,8 @@ fn to_ledger_addr_type(dest: DestinationTag) -> SignerResult { } } -fn to_ledger_bip32_path(xpub: &ExtendedPublicKey) -> LedgerBip32Path { - LedgerBip32Path( +fn to_ledger_bip32_path(xpub: &ExtendedPublicKey) -> ledger_msg::Bip32Path { + ledger_msg::Bip32Path( xpub.get_derivation_path() .as_slice() .iter() @@ -1237,7 +1242,7 @@ async fn fetch_extended_pub_key( account_index: U31, ) -> SignerResult { let derivation_path = make_account_path(chain_config, account_index); - let coin_type = to_ledger_chain_type(chain_config); + let coin_type = make_ledger_coin_type(chain_config); get_extended_public_key(client, coin_type, derivation_path) .await diff --git a/wallet/src/signer/ledger_signer/tests/mod.rs b/wallet/src/signer/ledger_signer/tests/mod.rs index 01ccba31e9..5cb8f36418 100644 --- a/wallet/src/signer/ledger_signer/tests/mod.rs +++ b/wallet/src/signer/ledger_signer/tests/mod.rs @@ -48,6 +48,7 @@ use crate::signer::{ generic_tests::{ MessageToSign, sign_message_test_params, test_sign_message_generic, test_sign_transaction_generic, test_sign_transaction_intent_generic, + test_sign_transaction_with_no_outputs_generic, }, make_deterministic_software_signer, no_another_signer, }, @@ -87,6 +88,36 @@ fn should_auto_confirm() -> bool { bool_from_env("LEDGER_TESTS_AUTO_CONFIRM").unwrap().unwrap_or(true) } +// Possible "transaction titles"; one of this is shown when the user is to confirm the tx signing. +// The list comes from the `transaction_title` function in the ledger app. +fn transaction_titles() -> &'static [&'static str] { + &[ + "Sign transaction", + "Sign transfer transaction", + "Sign burn transaction", + "Sign create HTLC transaction", + "Sign create delegation transaction", + "Sign delegate staking transaction", + "Sign delegation withdrawal transaction", + "Sign create stake pool transaction", + "Sign decommission stake pool transaction", + "Sign create NFT transaction", + "Sign create token transaction", + "Sign mint tokens transaction", + "Sign unmint tokens transaction", + "Sign freeze tokens transaction", + "Sign unfreeze tokens transaction", + "Sign lock token supply transaction", + "Sign change token authority transaction", + "Sign change token metadata URI transaction", + "Sign create order transaction", + "Sign fill order transaction", + "Sign freeze order transaction", + "Sign conclude order transaction", + "Sign data deposit transaction", + ] +} + async fn auto_confirmer(mut control_msg_rx: mpsc::Receiver, handle: Handle) { println!("Starting auto-confirmer for device: {:?}", handle.device()); @@ -94,9 +125,17 @@ async fn auto_confirmer(mut control_msg_rx: mpsc::Receiver, hand tokio::select! { _ = sleep(Duration::from_millis(500)) => { if let Ok(events) = handle.current_screen_events().await { - let to_sign = events.iter().any(|e| - e.contains("Sign transaction") || e.contains("Sign message") - ); + log::debug!("Screen events: {events:?}"); + + // Note: the events can look like `["Sign freeze tokens ", "transaction"]`, + // so we concatenate them before matching. + // On the other hand, `events` may also contain strings from different screens, + // so after the concatenation we do an exact substring search instead of + // using a regex. + let concatenated_events = events.concat(); + + let to_sign = concatenated_events.contains("Sign message") + || transaction_titles().iter().any(|title| concatenated_events.contains(title)); if to_sign { let _ = handle.confirm().await; @@ -225,7 +264,7 @@ async fn test_app_name() { let info = device.app_info(Duration::from_millis(500)).await.unwrap(); let version = check_current_app(&mut device).await.unwrap(); - assert_eq!(version, "0.1.0"); + assert_eq!(version, "1.0.0"); assert_eq!(version, info.version); } @@ -427,6 +466,31 @@ async fn test_fixed_signatures_no_legacy(#[case] seed: Seed) { test_fixed_signatures_generic_no_legacy(&mut rng, make_deterministic_software_signer).await; } +#[rstest] +#[case(Seed::from_entropy())] +#[trace] +#[serial_test::serial] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn test_sign_transaction_with_no_outputs(#[case] seed: Seed) { + log::debug!("test_sign_transaction_with_no_outputs, seed = {seed:?}"); + + let (auto_confirmer_handle, control_msg_tx, make_ledger_signer) = setup(true).await; + + let mut rng = make_seedable_rng(seed); + + test_sign_transaction_with_no_outputs_generic( + &mut rng, + make_ledger_signer, + Some(make_deterministic_software_signer), + ) + .await; + + if let Some(auto_confirmer_handle) = auto_confirmer_handle { + control_msg_tx.send(ControlMessage::Finish).await.unwrap(); + auto_confirmer_handle.await.unwrap(); + } +} + async fn create_device_connection() -> TcpDevice { let mut transport = TcpTransport::new().unwrap(); transport diff --git a/wallet/src/signer/software_signer/tests.rs b/wallet/src/signer/software_signer/tests.rs index 7a4b9915a5..a59b518249 100644 --- a/wallet/src/signer/software_signer/tests.rs +++ b/wallet/src/signer/software_signer/tests.rs @@ -26,6 +26,7 @@ use crate::signer::tests::{ generic_tests::{ MessageToSign, sign_message_test_params, test_sign_message_generic, test_sign_transaction_generic, test_sign_transaction_intent_generic, + test_sign_transaction_with_no_outputs_generic, }, make_deterministic_software_signer, make_software_signer, make_software_signer_for_cold_wallet, no_another_signer, @@ -160,3 +161,18 @@ async fn test_fixed_signatures_htlc_refunding( ) .await; } + +#[rstest] +#[case(Seed::from_entropy())] +#[trace] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn test_sign_transaction_with_no_outputs(#[case] seed: Seed) { + let mut rng = make_seedable_rng(seed); + + test_sign_transaction_with_no_outputs_generic( + &mut rng, + make_software_signer, + no_another_signer(), + ) + .await; +} diff --git a/wallet/src/signer/tests/generic_tests.rs b/wallet/src/signer/tests/generic_tests.rs index 14090779b1..fdcf66a971 100644 --- a/wallet/src/signer/tests/generic_tests.rs +++ b/wallet/src/signer/tests/generic_tests.rs @@ -1006,6 +1006,101 @@ pub async fn test_sign_transaction_generic( } } +// Test signing a tx that only have inputs but no outputs. +pub async fn test_sign_transaction_with_no_outputs_generic( + rng: &mut impl CryptoRng, + make_signer: MkS1, + make_another_signer: Option, +) where + MkS1: Fn(Arc, U31) -> S1, + MkS2: Fn(Arc, U31) -> S2, + S1: Signer, + S2: Signer, +{ + let chain_config = Arc::new(chain::config::create_unit_test_config()); + + let mut db = Store::new(DefaultBackend::new_in_memory()).unwrap(); + let mut db_tx = db.transaction_rw_unlocked(None).unwrap(); + let mut account = account_from_mnemonic(&chain_config, &mut db_tx, DEFAULT_ACCOUNT_INDEX); + + let dest1 = destination_from_account(&mut account, &mut db_tx, rng); + let dest2 = destination_from_account(&mut account, &mut db_tx, rng); + + let tx_id = Id::::random_using(rng); + let token_id = TokenId::random_using(rng); + + let utxo = TxOutput::Transfer( + OutputValue::Coin(Amount::from_atoms(rng.random_range(100..200))), + dest1, + ); + let req = SendRequest::new() + .with_inputs( + [( + TxInput::from_utxo(tx_id.into(), rng.random_range(0..10)), + utxo.clone(), + None, + )], + &|_| None, + ) + .unwrap() + .with_inputs_and_destinations([( + TxInput::AccountCommand( + AccountNonce::new(rng.next_u64()), + AccountCommand::FreezeToken(token_id, IsTokenUnfreezable::Yes), + ), + dest2, + )]); + + let orig_ptx = req.into_partially_signed_tx(PtxAdditionalInfo::new()).unwrap(); + + let mut signer = make_signer(chain_config.clone(), account.account_index()); + let (ptx, _, _) = signer + .sign_tx( + orig_ptx.clone(), + &TokensAdditionalInfo::new(), + account.key_chain(), + &mut db_tx, + BlockHeight::one(), + ) + .await + .unwrap(); + + if let Some(make_another_signer) = &make_another_signer { + let mut another_signer = make_another_signer(chain_config.clone(), account.account_index()); + let (another_ptx, _, _) = another_signer + .sign_tx( + orig_ptx, + &TokensAdditionalInfo::new(), + account.key_chain(), + &mut db_tx, + BlockHeight::one(), + ) + .await + .unwrap(); + + assert_eq!(ptx, another_ptx); + } + + let expected_input_commitments = ptx + .make_sighash_input_commitments(SighashInputCommitmentVersion::V1) + .unwrap() + .into_iter() + .map(|comm| comm.deep_clone()) + .collect_vec(); + + for (i, (dest, utxo)) in ptx.destinations().iter().zip(ptx.input_utxos().iter()).enumerate() { + tx_verifier::input_check::signature_only_check::verify_tx_signature( + &chain_config, + dest.as_ref().unwrap(), + &ptx, + &expected_input_commitments, + i, + utxo.clone(), + ) + .unwrap(); + } +} + fn random_order_info( ask_currency: &Currency, give_currency: &Currency, diff --git a/wallet/src/signer/trezor_signer/tests.rs b/wallet/src/signer/trezor_signer/tests.rs index bf58f9c417..fd2a8e7355 100644 --- a/wallet/src/signer/trezor_signer/tests.rs +++ b/wallet/src/signer/trezor_signer/tests.rs @@ -32,6 +32,7 @@ use crate::signer::{ generic_tests::{ MessageToSign, sign_message_test_params, test_sign_message_generic, test_sign_transaction_generic, test_sign_transaction_intent_generic, + test_sign_transaction_with_no_outputs_generic, }, make_deterministic_software_signer, no_another_signer, }, @@ -292,3 +293,23 @@ async fn test_sign_transaction_sig_consistency( ) .await; } + +#[rstest] +#[case(Seed::from_entropy())] +#[trace] +#[serial] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn test_sign_transaction_with_no_outputs(#[case] seed: Seed) { + log::debug!("test_sign_transaction_with_no_outputs, seed = {seed:?}"); + + let _join_guard = maybe_spawn_auto_confirmer(); + + let mut rng = make_seedable_rng(seed); + + test_sign_transaction_with_no_outputs_generic( + &mut rng, + make_trezor_signer, + Some(make_deterministic_software_signer), + ) + .await; +}