diff --git a/CHANGELOG.md b/CHANGELOG.md index ccc29bdb..6df280f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ page. See [DEVELOPMENT_CYCLE.md](DEVELOPMENT_CYCLE.md) for more details. - Fixed `warn!` messages, including the mainnet notice, being hidden unless `RUST_LOG` was set; warnings are now shown by default - Added `hwi` feature with a hardware wallet `hwi` command to list connected devices, register a wallet policy, display a receive address, and sign PSBTs. Supports Ledger, Coldcard, BitBox02, Jade and Specter. - Replaced plain `wallets` with `wallets list`, and added `wallets delete ` for unused saved wallet configurations. +- Fixed `create_tx`, `create_sp_tx` and `create_dns_tx` to reject `--to` addresses and `--to-sp` silent payment codes from a different network instead of building an unspendable PSBT - Removed the redundant `wallet` field from `wallets list` output; the wallet name is still available as the top-level key - Fixed `compile` silently accepting placeholder keys/hashes instead of validating them; added `--placeholders` to opt back into the old permissive behavior - Fixed `compile` panicking instead of returning an error when a key is valid on its own but the wrong shape for the requested script type (e.g. an x-only key under `sh`/`wsh`/`sh-wsh`, or an uncompressed key under `tr`) diff --git a/src/handlers/dns/mod.rs b/src/handlers/dns/mod.rs index e1c5b8df..01d60e6b 100644 --- a/src/handlers/dns/mod.rs +++ b/src/handlers/dns/mod.rs @@ -9,9 +9,11 @@ use crate::utils::types::{PsbtResult, StatusResult}; use crate::utils::{ check_standard_weight, max_signed_weight, parse_dns_recipient, parse_fee_rate, parse_op_return_base64, parse_op_return_string, parse_outpoint, parse_recipient, + recipient_script, }; use bdk_wallet::KeychainKind; -use bdk_wallet::bitcoin::{Amount, FeeRate, OutPoint, ScriptBuf, Sequence, script::PushBytesBuf}; +use bdk_wallet::bitcoin::address::NetworkUnchecked; +use bdk_wallet::bitcoin::{Address, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, script::PushBytesBuf}; use clap::Parser; use std::collections::BTreeMap; @@ -42,7 +44,7 @@ impl AsyncAppCommand> for ResolveDnsRecipientCommand { #[derive(Parser, Debug, Clone, PartialEq)] pub struct CreateDnsTxCommand { #[arg(env = "ADDRESS_SAT", long = "to", value_parser = parse_recipient)] - pub recipients: Vec<(ScriptBuf, u64)>, + pub recipients: Vec<(Address, u64)>, #[arg(long = "to_dns", value_parser = parse_dns_recipient)] pub dns_recipients: Vec<(String, u64)>, #[arg(long = "dns_resolver", default_value = "8.8.8.8")] @@ -88,8 +90,12 @@ impl AsyncAppCommand>> for CreateDnsTxCommand { &self, ctx: &mut AppContext>, ) -> Result { - let network = ctx.network; - let mut recipients: Vec<(ScriptBuf, u64)> = self.recipients.clone(); + let network = ctx.state.wallet.network(); + let mut recipients: Vec<(ScriptBuf, u64)> = self + .recipients + .iter() + .map(|(address, amount)| Ok((recipient_script(address, network)?, *amount))) + .collect::>()?; for (hrn, amount_sat) in &self.dns_recipients { log::info!("Resolving DNS instructions for recipient {hrn}"); diff --git a/src/handlers/offline.rs b/src/handlers/offline.rs index 80a6de47..9151b9d2 100644 --- a/src/handlers/offline.rs +++ b/src/handlers/offline.rs @@ -9,11 +9,13 @@ use crate::utils::types::{ use crate::utils::{ check_standard_weight, max_signed_weight, parse_address, parse_fee_rate, parse_op_return_base64, parse_op_return_string, parse_outpoint, parse_recipient, + recipient_script, }; +use bdk_wallet::bitcoin::address::NetworkUnchecked; use bdk_wallet::bitcoin::base64::Engine; use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD; use bdk_wallet::bitcoin::script::PushBytesBuf; -use bdk_wallet::bitcoin::{Address, Amount, FeeRate, OutPoint, Psbt, ScriptBuf, Sequence, Txid}; +use bdk_wallet::bitcoin::{Address, Amount, FeeRate, OutPoint, Psbt, Sequence, Txid}; use bdk_wallet::{KeychainKind, SignOptions}; use clap::Parser; use serde_json::json; @@ -26,6 +28,7 @@ use { encoding::SilentPaymentCode, send::psbt::derive_sp, }, + bdk_wallet::bitcoin::ScriptBuf, bdk_wallet::bitcoin::key::Secp256k1, bdk_wallet::keys::{DescriptorPublicKey, DescriptorSecretKey, SinglePubKey}, std::collections::HashMap, @@ -196,7 +199,7 @@ impl AppCommand>> for BalanceCommand { pub struct CreateTxCommand { /// Adds a recipient to the transaction. #[arg(env = "ADDRESS_SAT", long = "to", required = true, value_parser = parse_recipient)] - pub recipients: Vec<(ScriptBuf, u64)>, + pub recipients: Vec<(Address, u64)>, /// Sends all the funds (or all the selected utxos). Requires only one recipient with value 0. #[arg(long = "send_all", short = 'a')] @@ -255,13 +258,14 @@ impl AppCommand>> for CreateTxCommand { type Output = PsbtResult; fn execute(&self, ctx: &mut AppContext>) -> Result { + let network = ctx.state.wallet.network(); let mut tx_builder = ctx.state.wallet.build_tx(); if self.send_all { if self.recipients.len() == 1 { tx_builder .drain_wallet() - .drain_to(self.recipients[0].0.clone()); + .drain_to(recipient_script(&self.recipients[0].0, network)?); } else { return Err(Error::Generic( "Wallet can only be drained to a single output".to_string(), @@ -270,10 +274,14 @@ impl AppCommand>> for CreateTxCommand { } else { let recipients = self .recipients - .clone() - .into_iter() - .map(|(script, amount)| (script, Amount::from_sat(amount))) - .collect(); + .iter() + .map(|(address, amount)| { + Ok(( + recipient_script(address, network)?, + Amount::from_sat(*amount), + )) + }) + .collect::>()?; tx_builder.set_recipients(recipients); } @@ -330,7 +338,7 @@ pub struct CreateSpTxCommand { // Clap Doesn't support complex vector parsing https://github.com/clap-rs/clap/issues/1704. // Address and amount parsing is done at run time in handler function. #[arg(env = "ADDRESS_SAT", long = "to", required = false, value_parser = parse_recipient)] - pub recipients: Option>, + pub recipients: Option, u64)>>, /// Parse silent payment recipients #[arg(long = "to-sp", required = true, value_parser = parse_sp_code_value_pairs)] pub silent_payment_recipients: Vec<(SilentPaymentCode, u64)>, @@ -380,13 +388,22 @@ impl AppCommand>> for CreateSpTxCommand { type Output = RawPsbt; fn execute(&self, ctx: &mut AppContext>) -> Result { + let network = ctx.state.wallet.network(); let mut tx_builder = ctx.state.wallet.build_tx(); let sp_recipients: Vec = self .silent_payment_recipients .iter() - .map(|(sp_code, _)| sp_code.clone()) - .collect(); + .map(|(sp_code, _)| { + if sp_code.network != network { + return Err(Error::Generic(format!( + "Silent payment code network mismatch: expected {network}, found {} for {sp_code}", + sp_code.network + ))); + } + Ok(sp_code.clone()) + }) + .collect::>()?; if self.send_all { if sp_recipients.len() == 1 && self.recipients.is_none() { @@ -397,7 +414,9 @@ impl AppCommand>> for CreateSpTxCommand { && sp_recipients.is_empty() { if recipients.len() == 1 { - tx_builder.drain_wallet().drain_to(recipients[0].0.clone()); + tx_builder + .drain_wallet() + .drain_to(recipient_script(&recipients[0].0, network)?); } else { return Err(Error::Generic( "Wallet can only be drain to a single output".to_string(), @@ -421,7 +440,13 @@ impl AppCommand>> for CreateSpTxCommand { if let Some(recipients) = &self.recipients { let recipients = recipients .iter() - .map(|(script, amount)| (script.clone(), Amount::from_sat(*amount))); + .map(|(address, amount)| { + Ok(( + recipient_script(address, network)?, + Amount::from_sat(*amount), + )) + }) + .collect::, Error>>()?; outputs.extend(recipients); } diff --git a/src/utils/common.rs b/src/utils/common.rs index 1e3cbbe7..935a1071 100644 --- a/src/utils/common.rs +++ b/src/utils/common.rs @@ -6,6 +6,13 @@ use bdk_kyoto::{Info, Receiver, UnboundedReceiver, Warning}; use bdk_message_signer::SignatureFormat; #[cfg(feature = "silent-payments")] use bdk_sp::encoding::SilentPaymentCode; +use bdk_wallet::bitcoin::address::NetworkUnchecked; +use bdk_wallet::bitcoin::base64::Engine; +use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD; +use bdk_wallet::bitcoin::script::PushBytesBuf; +use bdk_wallet::bitcoin::{ + Address, FeeRate, Network, OutPoint, Psbt, ScriptBuf, Transaction, Weight, +}; use bdk_wallet::Wallet; #[cfg(any( feature = "electrum", @@ -14,12 +21,6 @@ use bdk_wallet::Wallet; feature = "rpc" ))] use bdk_wallet::WalletEvent; -use bdk_wallet::bitcoin::base64::Engine; -use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD; -use bdk_wallet::bitcoin::script::PushBytesBuf; -use bdk_wallet::bitcoin::{ - Address, FeeRate, Network, OutPoint, Psbt, ScriptBuf, Transaction, Weight, -}; use std::{ path::{Path, PathBuf}, str::FromStr, @@ -69,17 +70,44 @@ pub(crate) fn is_final(psbt: &Psbt) -> Result<(), Error> { } /// Parse the recipient (Address,Amount) argument from cli input. -pub(crate) fn parse_recipient(s: &str) -> Result<(ScriptBuf, u64), String> { +pub(crate) fn parse_recipient(s: &str) -> Result<(Address, u64), String> { let parts: Vec<_> = s.split(':').collect(); if parts.len() != 2 { return Err("Invalid format".to_string()); } - let addr = Address::from_str(parts[0]) - .map_err(|e| e.to_string())? - .assume_checked(); + let addr = Address::from_str(parts[0]).map_err(|e| e.to_string())?; let val = u64::from_str(parts[1]).map_err(|e| e.to_string())?; - Ok((addr.script_pubkey(), val)) + Ok((addr, val)) +} + +/// Return the network an unchecked address belongs to. +fn address_network(address: &Address) -> Result { + [ + Network::Bitcoin, + Network::Testnet, + Network::Testnet4, + Network::Signet, + Network::Regtest, + ] + .into_iter() + .find(|network| address.is_valid_for_network(*network)) + .ok_or(Error::Generic(format!("Invalid address: {address:?}"))) +} + +/// Convert a recipient address to a script pubkey, requiring the wallet's network. +pub(crate) fn recipient_script( + address: &Address, + network: Network, +) -> Result { + match address.clone().require_network(network) { + Ok(checked) => Ok(checked.script_pubkey()), + Err(_) => Err(Error::Generic(format!( + "Network mismatch: expected {network}, found {} for {}", + address_network(address)?, + address.clone().assume_checked() + ))), + } } #[cfg(any(feature = "electrum", feature = "esplora", feature = "rpc"))] diff --git a/tests/integration/offline.rs b/tests/integration/offline.rs index c4be0a21..4106b2b5 100644 --- a/tests/integration/offline.rs +++ b/tests/integration/offline.rs @@ -8,7 +8,7 @@ mod test_offline { static WALLET_NAME: &str = "test_config_wallet"; /// A `--to` argument, the wallet is unfunded, so nothing is spent. - static RECIPIENT: &str = "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4:10000"; + static RECIPIENT: &str = "bcrt1qw508d6qejxtdg4y5r3zarvary0c5xw7kygt080:10000"; /// Helper to spin up a sandboxed CLI with the generated descriptors fn setup_wallet_config() -> (BdkCli, Command) { @@ -116,17 +116,18 @@ mod test_offline { let (cli, mut cmd_init) = setup_wallet_config(); cmd_init.assert().success(); + let address = cli + .wallet_cmd(&["--wallet", WALLET_NAME, "new_address"]) + .output() + .unwrap(); + let address_json: Value = serde_json::from_slice(&address.stdout).unwrap(); + let recipient = format!("{}:1000", address_json["address"].as_str().unwrap()); + // create transaction - cli.wallet_cmd(&[ - "--wallet", - WALLET_NAME, - "create_tx", - "--to", - "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4:1000", - ]) - .assert() - .failure() - .stderr(predicate::str::contains("Insufficient")); + cli.wallet_cmd(&["--wallet", WALLET_NAME, "create_tx", "--to", &recipient]) + .assert() + .failure() + .stderr(predicate::str::contains("Insufficient")); } #[test] @@ -322,6 +323,112 @@ mod test_offline { .stdout(predicate::str::contains("\"valid\": false")); } + #[test] + fn test_create_tx_rejects_address_from_wrong_network() { + let (cli, mut cmd_init) = setup_wallet_config(); + cmd_init.assert().success(); + + // Wallet is regtest, recipient is testnet. + let recipient = "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4:1000"; + cli.wallet_cmd(&["--wallet", WALLET_NAME, "create_tx", "--to", recipient]) + .assert() + .failure() + .stderr(predicate::str::contains( + "Network mismatch: expected regtest, found testnet", + )); + } + + #[cfg(feature = "silent-payments")] + #[test] + fn test_create_sp_tx_rejects_address_from_wrong_network() { + let (cli, mut cmd_init) = setup_wallet_config(); + cmd_init.assert().success(); + + let sp_code = cli + .cmd( + "silent_payment_code", + &[ + "--scan_key", + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + "--spend_key", + "02c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5", + ], + ) + .output() + .unwrap(); + let sp_code: Value = serde_json::from_slice(&sp_code.stdout).unwrap(); + let sp_recipient = format!("{}:1000", sp_code["message"].as_str().unwrap()); + + // Wallet is regtest, recipient is testnet. + let recipient = "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4:1000"; + cli.wallet_cmd(&[ + "--wallet", + WALLET_NAME, + "create_sp_tx", + "--to", + recipient, + "--to-sp", + &sp_recipient, + ]) + .assert() + .failure() + .stderr(predicate::str::contains( + "Network mismatch: expected regtest, found testnet", + )); + } + + #[cfg(feature = "silent-payments")] + #[test] + fn test_create_sp_tx_rejects_code_from_wrong_network() { + let (cli, mut cmd_init) = setup_wallet_config(); + cmd_init.assert().success(); + + // Wallet is regtest, silent payment code is testnet. + let sp_code = BdkCli::new("testnet", None) + .cmd( + "silent_payment_code", + &[ + "--scan_key", + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + "--spend_key", + "02c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5", + ], + ) + .output() + .unwrap(); + let sp_code: Value = serde_json::from_slice(&sp_code.stdout).unwrap(); + let sp_recipient = format!("{}:1000", sp_code["message"].as_str().unwrap()); + + cli.wallet_cmd(&[ + "--wallet", + WALLET_NAME, + "create_sp_tx", + "--to-sp", + &sp_recipient, + ]) + .assert() + .failure() + .stderr(predicate::str::contains( + "Silent payment code network mismatch: expected regtest, found testnet", + )); + } + + #[cfg(feature = "dns_payment")] + #[test] + fn test_create_dns_tx_rejects_address_from_wrong_network() { + let (cli, mut cmd_init) = setup_wallet_config(); + cmd_init.assert().success(); + + // Wallet is regtest, recipient is testnet. + let recipient = "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4:1000"; + cli.wallet_cmd(&["--wallet", WALLET_NAME, "create_dns_tx", "--to", recipient]) + .assert() + .failure() + .stderr(predicate::str::contains( + "Network mismatch: expected regtest, found testnet", + )); + } + #[test] fn test_create_tx_send_all_rejects_multiple_recipients() { let (cli, mut cmd_init) = setup_wallet_config(); diff --git a/tests/integration/online.rs b/tests/integration/online.rs index d7901ce3..472c2d53 100644 --- a/tests/integration/online.rs +++ b/tests/integration/online.rs @@ -10,7 +10,7 @@ mod test_online { use tempfile::TempDir; static WALLET_NAME: &str = "test_online_wallet"; - static RECIPIENT: &str = "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4"; + static RECIPIENT: &str = "bcrt1qw508d6qejxtdg4y5r3zarvary0c5xw7kygt080"; /// Spins up the envt, a Regtest node + electrs, and saves the wallet config. fn setup_online_wallet() -> (BdkCli, Command, TestEnv) {