From 3e7db1b03d9e7954c3d9208ae0eaa0835f8b0c2f Mon Sep 17 00:00:00 2001 From: Vihiga Tyonum Date: Sat, 26 Sep 2026 08:56:45 +0100 Subject: [PATCH 1/2] fix(create_tx,bump_fee): return errors instead of panicking `create_tx` and `bump_fee` called `.unwrap()` on `Result`s carrying user-supplied input, so a mistyped argument aborted the process with a Rust panic (exit 101) instead of a usage error (exit 1). - propagate `add_utxos` failures in `create_tx` and `bump_fee` with `?`, adding `BDKCliError::AddUtxoError` so the malformed outpoint is named - propagate `--add_data` base64 decoding and `PushBytesBuf` conversion failures in `create_tx` - stop flattening `add_utxos` errors into `CreateTxError::UnknownUtxo` in `create_sp_tx` and `create_dns_tx`, which discarded the real cause - add an integration test asserting exit 1, never 101, for an unknown outpoint and for malformed base64 Fixes #325 --- CHANGELOG.md | 1 + src/error.rs | 3 +++ src/handlers/dns/mod.rs | 8 ++------ src/handlers/offline.rs | 24 +++++++++++------------ tests/integration/offline.rs | 38 ++++++++++++++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec50ea1f..636e8034 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ page. See [DEVELOPMENT_CYCLE.md](DEVELOPMENT_CYCLE.md) for more details. - Added support for Multipath (two-paths) descriptors. - Fixed the data directory and config.toml permission being world-readable (0755/0644) to 0700/0600 on Unix. +- Fixed `create_tx` and `bump_fee` panicking on malformed `--utxos` and `--add_data` values instead of returning an error ## [4.0.0] diff --git a/src/error.rs b/src/error.rs index bfad510a..9ceb571a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,6 +5,9 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum BDKCliError { + #[error("Add UTXO error: {0}")] + AddUtxoError(#[from] bdk_wallet::tx_builder::AddUtxoError), + #[error("Cannot provide both a multipath descriptor and a separate internal descriptor.")] AmbiguousDescriptors, diff --git a/src/handlers/dns/mod.rs b/src/handlers/dns/mod.rs index 628bfbdb..6719db08 100644 --- a/src/handlers/dns/mod.rs +++ b/src/handlers/dns/mod.rs @@ -135,17 +135,13 @@ impl AsyncAppCommand>> for CreateDnsTxCommand { tx_builder.fee_rate(fee_rate); } if let Some(utxos) = &self.utxos { - tx_builder - .add_utxos(&utxos[..]) - .map_err(|_| bdk_wallet::error::CreateTxError::UnknownUtxo)?; + tx_builder.add_utxos(&utxos[..])?; } if let Some(unspendable) = &self.unspendable { tx_builder.unspendable(unspendable.to_vec()); } if let Some(base64_data) = &self.add_data { - let op_return_data = BASE64_STANDARD - .decode(base64_data) - .map_err(|e| Error::Generic(e.to_string()))?; + let op_return_data = BASE64_STANDARD.decode(base64_data)?; tx_builder.add_data( &PushBytesBuf::try_from(op_return_data) .map_err(|e| Error::Generic(e.to_string()))?, diff --git a/src/handlers/offline.rs b/src/handlers/offline.rs index 13955374..918a4473 100644 --- a/src/handlers/offline.rs +++ b/src/handlers/offline.rs @@ -288,7 +288,7 @@ impl AppCommand>> for CreateTxCommand { } if let Some(utxos) = &self.utxos { - tx_builder.add_utxos(&utxos[..]).unwrap(); + tx_builder.add_utxos(&utxos[..])?; } if let Some(unspendable) = &self.unspendable { @@ -296,10 +296,14 @@ impl AppCommand>> for CreateTxCommand { } if let Some(base64_data) = &self.add_data { - let op_return_data = BASE64_STANDARD.decode(base64_data).unwrap(); - tx_builder.add_data(&PushBytesBuf::try_from(op_return_data).unwrap()); + let op_return_data = BASE64_STANDARD.decode(base64_data)?; + tx_builder.add_data( + &PushBytesBuf::try_from(op_return_data) + .map_err(|e| Error::Generic(e.to_string()))?, + ); } else if let Some(string_data) = &self.add_string { - let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec()).unwrap(); + let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec()) + .map_err(|e| Error::Generic(e.to_string()))?; tx_builder.add_data(&data); } @@ -319,8 +323,6 @@ impl AppCommand>> for CreateTxCommand { let psbt = tx_builder.finish()?; - // let psbt_base64 = BASE64_STANDARD.encode(psbt.serialize()); - Ok(PsbtResult::new(&psbt, Some(false))) } } @@ -443,9 +445,7 @@ impl AppCommand>> for CreateSpTxCommand { } if let Some(utxos) = &self.utxos { - tx_builder - .add_utxos(&utxos[..]) - .map_err(|_| bdk_wallet::error::CreateTxError::UnknownUtxo)?; + tx_builder.add_utxos(&utxos[..])?; } if let Some(unspendable) = &self.unspendable { @@ -453,9 +453,7 @@ impl AppCommand>> for CreateSpTxCommand { } if let Some(base64_data) = &self.add_data { - let op_return_data = BASE64_STANDARD - .decode(base64_data) - .map_err(|e| Error::Generic(e.to_string()))?; + let op_return_data = BASE64_STANDARD.decode(base64_data)?; tx_builder.add_data( &PushBytesBuf::try_from(op_return_data) .map_err(|e| Error::Generic(e.to_string()))?, @@ -599,7 +597,7 @@ impl AppCommand>> for BumpFeeCommand { } if let Some(utxos) = &self.utxos { - tx_builder.add_utxos(&utxos[..]).unwrap(); + tx_builder.add_utxos(&utxos[..])?; } if let Some(unspendable) = &self.unspendable { diff --git a/tests/integration/offline.rs b/tests/integration/offline.rs index 27b6bd16..3bb0f891 100644 --- a/tests/integration/offline.rs +++ b/tests/integration/offline.rs @@ -7,6 +7,8 @@ mod test_offline { use tempfile::TempDir; static WALLET_NAME: &str = "test_config_wallet"; + /// A `--to` argument, the wallet is unfunded, so nothing is spent. + static RECIPIENT: &str = "tb1p4tp4l6glyr2gs94neqcpr5gha7344nfyznfkc8szkreflscsdkgqsdent4:10000"; /// Helper to spin up a sandboxed CLI with the generated descriptors fn setup_wallet_config() -> (BdkCli, Command) { @@ -151,6 +153,42 @@ mod test_offline { .stderr(predicate::str::contains("Invalid")); } + /// A malformed `create_tx` argument must be reported as an error (exit 1), + /// never as a panic (exit 101). + #[test] + fn test_create_tx_rejects_malformed_input_without_panicking() { + let (cli, mut cmd_init) = setup_wallet_config(); + cmd_init.assert().success(); + + // Unknown outpoint + cli.wallet_cmd(&[ + "--wallet", + WALLET_NAME, + "create_tx", + "--to", + RECIPIENT, + "--utxos", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:0", + ]) + .assert() + .code(1) + .stderr(predicate::str::contains("UTXO not found")); + + // Malformed base64 + cli.wallet_cmd(&[ + "--wallet", + WALLET_NAME, + "create_tx", + "--to", + RECIPIENT, + "--add_data", + "!!!not-base64!!!", + ]) + .assert() + .code(1) + .stderr(predicate::str::contains("Base64 decoding error")); + } + #[cfg(feature = "message_signer")] #[test] fn test_sign_message_and_verify_message() { From 3eacd051dc16b6ec61a80fa105742205bb729f32 Mon Sep 17 00:00:00 2001 From: Vihiga Tyonum Date: Sat, 26 Sep 2026 09:22:34 +0100 Subject: [PATCH 2/2] fix(fee_rate): parse fee_rate at clap and keep sub-sat/vB precision `--fee_rate` was taken as `f32` in the tx-building commands and cast `as u64`, which both truncates and saturates. When `FeeRate::from_sat_per_vb` returned `None` the value was silently skipped in `create_tx`, `create_sp_tx` and `create_dns_tx`, `bump_fee` fell back to `FeeRate::BROADCAST_MIN`, and `send_payjoin` took a `u64` and panicked, so the user got a fee they never asked for or no transaction at all. - add `parse_fee_rate` and pass it as a clap `value_parser`, so invalid values are rejected - store `FeeRate` instead of `f32`/`u64`, converting via sat/kwu so fractional rates keep 1/250 sat/vB precision rather than truncating - drop the `unwrap_or(FeeRate::BROADCAST_MIN)` fallback in `bump_fee` and the `expect` in `send_payjoin` - cover parsing and rejection with unit and integration tests Fixes #325 --- CHANGELOG.md | 2 + src/handlers/dns/mod.rs | 10 +- src/handlers/offline.rs | 27 ++-- src/handlers/online.rs | 10 +- src/handlers/payjoin/mod.rs | 4 +- src/utils/common.rs | 241 +++++++++++++++++++++++------------ tests/integration/offline.rs | 28 ++++ 7 files changed, 209 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 636e8034..5d127b14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ page. See [DEVELOPMENT_CYCLE.md](DEVELOPMENT_CYCLE.md) for more details. - Added support for Multipath (two-paths) descriptors. - Fixed the data directory and config.toml permission being world-readable (0755/0644) to 0700/0600 on Unix. - Fixed `create_tx` and `bump_fee` panicking on malformed `--utxos` and `--add_data` values instead of returning an error +- Fixed `--fee_rate` silently truncating to a whole sat/vB, falling back to a default, or producing a zero-fee transaction, unusable values are now rejected + ## [4.0.0] diff --git a/src/handlers/dns/mod.rs b/src/handlers/dns/mod.rs index 6719db08..3d94efa2 100644 --- a/src/handlers/dns/mod.rs +++ b/src/handlers/dns/mod.rs @@ -6,7 +6,7 @@ use crate::handlers::dns::dns_payment_instructions::{ }; use crate::handlers::{AppContext, AsyncAppCommand, Init, OfflineOperations}; use crate::utils::types::{PsbtResult, StatusResult}; -use crate::utils::{parse_dns_recipient, parse_outpoint, parse_recipient}; +use crate::utils::{parse_dns_recipient, parse_fee_rate, parse_outpoint, parse_recipient}; use bdk_wallet::KeychainKind; use bdk_wallet::bitcoin::base64::Engine; use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD; @@ -57,8 +57,8 @@ pub struct CreateDnsTxCommand { pub utxos: Option>, #[arg(env = "CANT_SPEND_TXID:VOUT", long = "unspendable", value_parser = parse_outpoint)] pub unspendable: Option>, - #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate")] - pub fee_rate: Option, + #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate", value_parser = parse_fee_rate)] + pub fee_rate: Option, #[arg(env = "EXT_POLICY", long = "external_policy")] pub external_policy: Option, #[arg(env = "INT_POLICY", long = "internal_policy")] @@ -129,9 +129,7 @@ impl AsyncAppCommand>> for CreateDnsTxCommand { if self.offline_signer { tx_builder.add_global_xpubs(); } - if let Some(fee_rate) = self.fee_rate - && let Some(fee_rate) = FeeRate::from_sat_per_vb(fee_rate as u64) - { + if let Some(fee_rate) = self.fee_rate { tx_builder.fee_rate(fee_rate); } if let Some(utxos) = &self.utxos { diff --git a/src/handlers/offline.rs b/src/handlers/offline.rs index 918a4473..37465850 100644 --- a/src/handlers/offline.rs +++ b/src/handlers/offline.rs @@ -7,7 +7,7 @@ use crate::utils::types::{ AddressResult, BalanceResult, KeychainPair, PsbtResult, RawPsbt, TransactionDetails, UnspentDetails, }; -use crate::utils::{parse_outpoint, parse_recipient}; +use crate::utils::{parse_fee_rate, parse_outpoint, parse_recipient}; use bdk_wallet::bitcoin::base64::Engine; use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD; use bdk_wallet::bitcoin::script::PushBytesBuf; @@ -217,8 +217,8 @@ pub struct CreateTxCommand { pub unspendable: Option>, /// Fee rate to use in sat/vbyte. - #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate")] - pub fee_rate: Option, + #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate", value_parser = parse_fee_rate)] + pub fee_rate: Option, /// Selects which policy should be used to satisfy the external descriptor. #[arg(env = "EXT_POLICY", long = "external_policy")] @@ -281,9 +281,7 @@ impl AppCommand>> for CreateTxCommand { tx_builder.add_global_xpubs(); } - if let Some(fee_rate) = self.fee_rate - && let Some(fee_rate) = FeeRate::from_sat_per_vb(fee_rate as u64) - { + if let Some(fee_rate) = self.fee_rate { tx_builder.fee_rate(fee_rate); } @@ -351,8 +349,8 @@ pub struct CreateSpTxCommand { #[arg(env = "CANT_SPEND_TXID:VOUT", long = "unspendable", value_parser = parse_outpoint)] pub unspendable: Option>, /// Fee rate to use in sat/vbyte. - #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate")] - pub fee_rate: Option, + #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate", value_parser = parse_fee_rate)] + pub fee_rate: Option, /// Selects which policy should be used to satisfy the external descriptor. #[arg(env = "EXT_POLICY", long = "external_policy")] pub external_policy: Option, @@ -438,9 +436,7 @@ impl AppCommand>> for CreateSpTxCommand { tx_builder.add_global_xpubs(); } - if let Some(fee_rate) = self.fee_rate - && let Some(fee_rate) = FeeRate::from_sat_per_vb(fee_rate as u64) - { + if let Some(fee_rate) = self.fee_rate { tx_builder.fee_rate(fee_rate); } @@ -571,9 +567,10 @@ pub struct BumpFeeCommand { env = "SATS_VBYTE", short = 'f', long = "fee_rate", - default_value = "1.0" + default_value = "1.0", + value_parser = parse_fee_rate )] - pub fee_rate: f32, + pub fee_rate: FeeRate, } impl AppCommand>> for BumpFeeCommand { @@ -583,9 +580,7 @@ impl AppCommand>> for BumpFeeCommand { let wallet = &mut ctx.state.wallet; let mut tx_builder = wallet.build_fee_bump(self.txid)?; - let fee_rate = - FeeRate::from_sat_per_vb(self.fee_rate as u64).unwrap_or(FeeRate::BROADCAST_MIN); - tx_builder.fee_rate(fee_rate); + tx_builder.fee_rate(self.fee_rate); if let Some(address) = &self.shrink_address { let script_pubkey = address.script_pubkey(); diff --git a/src/handlers/online.rs b/src/handlers/online.rs index f2900cbc..25baf889 100644 --- a/src/handlers/online.rs +++ b/src/handlers/online.rs @@ -1,9 +1,10 @@ -use clap::Parser; - #[cfg(feature = "electrum")] use crate::client::BlockchainClient::Electrum; #[cfg(feature = "cbf")] use crate::client::{BlockchainClient::KyotoClient, sync_kyoto_client}; +use crate::utils::parse_fee_rate; +use bdk_wallet::bitcoin::FeeRate; +use clap::Parser; #[cfg(feature = "esplora")] use {crate::client::BlockchainClient::Esplora, bdk_esplora::EsploraAsyncExt}; #[cfg(feature = "rpc")] @@ -438,9 +439,10 @@ pub struct SendPayjoinCommand { env = "PAYJOIN_SENDER_FEE_RATE", short = 'f', long = "fee_rate", - required = true + required = true, + value_parser = parse_fee_rate )] - fee_rate: u64, + fee_rate: FeeRate, } #[cfg(any( feature = "electrum", diff --git a/src/handlers/payjoin/mod.rs b/src/handlers/payjoin/mod.rs index 7a2fe36a..c78fa0af 100644 --- a/src/handlers/payjoin/mod.rs +++ b/src/handlers/payjoin/mod.rs @@ -172,7 +172,7 @@ impl<'a> PayjoinManager<'a> { pub async fn send_payjoin( &mut self, uri: String, - fee_rate: u64, + fee_rate: FeeRate, ohttp_relays: Vec, blockchain_client: &BlockchainClient, ) -> Result { @@ -189,8 +189,6 @@ impl<'a> PayjoinManager<'a> { .amount .ok_or_else(|| Error::Generic("Amount is not specified in the URI.".to_string()))?; - let fee_rate = FeeRate::from_sat_per_vb(fee_rate).expect("Provided fee rate is not valid."); - // Build and sign the original PSBT which pays to the receiver. let mut original_psbt = { let mut tx_builder = self.wallet.build_tx(); diff --git a/src/utils/common.rs b/src/utils/common.rs index 5102c034..1cc8835d 100644 --- a/src/utils/common.rs +++ b/src/utils/common.rs @@ -5,7 +5,7 @@ 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, Network, OutPoint, ScriptBuf}; +use bdk_wallet::bitcoin::{Address, FeeRate, Network, OutPoint, ScriptBuf}; #[cfg(any( feature = "electrum", feature = "esplora", @@ -26,6 +26,12 @@ pub(crate) const DIR_MODE: u32 = 0o700; /// Only owner has full read and write access. pub(crate) const FILE_MODE: u32 = 0o600; +/// 1 vB is 1/4 kwu, so 1 sat/vB is 250 sat/kwu. +const SAT_PER_KWU_PER_SAT_PER_VB: f64 = 250.0; + +/// Smallest fee rate `FeeRate` can represent, in sat/vB. +const MIN_SAT_PER_VB: f64 = 1.0 / SAT_PER_KWU_PER_SAT_PER_VB; + /// Determine if PSBT has final script sigs or witnesses for all unsigned tx inputs. #[cfg(any( feature = "electrum", @@ -84,7 +90,7 @@ pub(crate) fn parse_proxy_auth(s: &str) -> Result<(String, String), Error> { Ok((user, passwd)) } -/// Parse a outpoint (Txid:Vout) argument from cli input. +/// Parse a outpoint (Txid:Vout) argument from input. pub(crate) fn parse_outpoint(s: &str) -> Result { Ok(OutPoint::from_str(s)?) } @@ -95,6 +101,39 @@ pub(crate) fn parse_address(address_str: &str) -> Result { Ok(unchecked_address.assume_checked()) } +/// Parse a fee rate, given in sat/vB, from input. +/// +/// [`FeeRate`] counts sat/kwu, so fractional rates are kept at 1/250 sat/vB +/// precision rather than being truncated to a whole sat/vB. A rate that cannot be +/// represented is rejected instead of silently becoming zero or a default. +pub(crate) fn parse_fee_rate(s: &str) -> Result { + let sat_vb = f64::from_str(s.trim()).map_err(|_| { + Error::Generic(format!( + "Invalid fee rate '{s}', expected a number in sat/vB" + )) + })?; + + if !sat_vb.is_finite() { + return Err(Error::Generic(format!( + "Invalid fee rate '{s}', must be a finite number of sat/vB" + ))); + } + if sat_vb < MIN_SAT_PER_VB { + return Err(Error::Generic(format!( + "Fee rate '{s}' sat/vB is below the smallest usable rate of {MIN_SAT_PER_VB} sat/vB" + ))); + } + + let sat_kwu = (sat_vb * SAT_PER_KWU_PER_SAT_PER_VB).round(); + if sat_kwu >= u64::MAX as f64 { + return Err(Error::Generic(format!( + "Fee rate '{s}' sat/vB is too large to represent" + ))); + } + + Ok(FeeRate::from_sat_per_kwu(sat_kwu as u64)) +} + /// Prepare bdk-cli home directory /// /// This function is called to check if [`crate::CliOpts`] datadir is set. @@ -397,111 +436,145 @@ pub(crate) fn write_file_content(path: &Path, contents: &str) -> std::io::Result std::fs::write(path, contents) } -#[cfg(all(test, unix))] -mod datadir_file_permissions_tests { +#[cfg(test)] +mod tests { use super::*; - use std::fs; - use std::os::unix::fs::PermissionsExt; - use tempfile::TempDir; + #[cfg(unix)] + mod datadir_file_permissions_tests { + use super::*; + use std::fs; + use std::os::unix::fs::PermissionsExt; + use tempfile::TempDir; + + fn mode_of(path: &Path) -> u32 { + fs::metadata(path).unwrap().permissions().mode() & 0o777 + } - fn mode_of(path: &Path) -> u32 { - fs::metadata(path).unwrap().permissions().mode() & 0o777 - } + #[test] + fn test_write_file_content_creates_an_owner_only_file() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("config.toml"); - #[test] - fn test_write_file_content_creates_an_owner_only_file() { - let temp_dir = TempDir::new().unwrap(); - let path = temp_dir.path().join("config.toml"); + write_file_content(&path, "tprv").unwrap(); - write_file_content(&path, "tprv").unwrap(); + assert_eq!(mode_of(&path), FILE_MODE); + assert_eq!(fs::read_to_string(&path).unwrap(), "tprv"); + } - assert_eq!(mode_of(&path), FILE_MODE); - assert_eq!(fs::read_to_string(&path).unwrap(), "tprv"); - } + #[test] + fn test_limit_access_hardens_only_exposed_file() { + let temp_dir = TempDir::new().unwrap(); + let exposed = temp_dir.path().join("exposed.toml"); + let private = temp_dir.path().join("private.toml"); + fs::write(&exposed, "tprv").unwrap(); + fs::write(&private, "tprv").unwrap(); + fs::set_permissions(&exposed, fs::Permissions::from_mode(0o644)).unwrap(); + fs::set_permissions(&private, fs::Permissions::from_mode(0o400)).unwrap(); + + limit_access(&exposed, FILE_MODE).unwrap(); + limit_access(&private, FILE_MODE).unwrap(); + + assert_eq!(mode_of(&exposed), FILE_MODE); + assert_eq!( + mode_of(&private), + 0o400, + "an owner-only file is not updated" + ); + } - #[test] - fn test_limit_access_hardens_only_exposed_file() { - let temp_dir = TempDir::new().unwrap(); - let exposed = temp_dir.path().join("exposed.toml"); - let private = temp_dir.path().join("private.toml"); - fs::write(&exposed, "tprv").unwrap(); - fs::write(&private, "tprv").unwrap(); - fs::set_permissions(&exposed, fs::Permissions::from_mode(0o644)).unwrap(); - fs::set_permissions(&private, fs::Permissions::from_mode(0o400)).unwrap(); - - limit_access(&exposed, FILE_MODE).unwrap(); - limit_access(&private, FILE_MODE).unwrap(); - - assert_eq!(mode_of(&exposed), FILE_MODE); - assert_eq!( - mode_of(&private), - 0o400, - "an owner-only file is not updated" - ); - } + #[test] + fn test_limit_access_hardens_a_directory() { + let temp_dir = TempDir::new().unwrap(); + let dir = temp_dir.path().join("datadir"); + fs::create_dir(&dir).unwrap(); + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); - #[test] - fn test_limit_access_hardens_a_directory() { - let temp_dir = TempDir::new().unwrap(); - let dir = temp_dir.path().join("datadir"); - fs::create_dir(&dir).unwrap(); - fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); + limit_access(&dir, DIR_MODE).unwrap(); - limit_access(&dir, DIR_MODE).unwrap(); + assert_eq!(mode_of(&dir), DIR_MODE); + } - assert_eq!(mode_of(&dir), DIR_MODE); - } + #[test] + fn test_limit_access_ignores_a_missing_path() { + let temp_dir = TempDir::new().unwrap(); - #[test] - fn test_limit_access_ignores_a_missing_path() { - let temp_dir = TempDir::new().unwrap(); + limit_access(&temp_dir.path().join("absent"), FILE_MODE).unwrap(); + } - limit_access(&temp_dir.path().join("absent"), FILE_MODE).unwrap(); - } + #[test] + fn test_write_file_content_restricts_a_pre_existing_exposed_file() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("config.toml"); + fs::write(&path, "stale").unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); - #[test] - fn test_write_file_content_restricts_a_pre_existing_exposed_file() { - let temp_dir = TempDir::new().unwrap(); - let path = temp_dir.path().join("config.toml"); - fs::write(&path, "stale").unwrap(); - fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); + write_file_content(&path, "tprv").unwrap(); - write_file_content(&path, "tprv").unwrap(); + assert_eq!(mode_of(&path), FILE_MODE); + assert_eq!(fs::read_to_string(&path).unwrap(), "tprv"); + } - assert_eq!(mode_of(&path), FILE_MODE); - assert_eq!(fs::read_to_string(&path).unwrap(), "tprv"); - } + #[test] + fn test_prepare_home_dir_creates_owner_only_datadir() { + let temp_dir = TempDir::new().unwrap(); + let dir = temp_dir.path().join("datadir").join("nested"); - #[test] - fn test_prepare_home_dir_creates_owner_only_datadir() { - let temp_dir = TempDir::new().unwrap(); - let dir = temp_dir.path().join("datadir").join("nested"); + prepare_home_dir(Some(dir.clone())).unwrap(); - prepare_home_dir(Some(dir.clone())).unwrap(); + assert_eq!(mode_of(&dir), DIR_MODE); + assert_eq!(mode_of(dir.parent().unwrap()), DIR_MODE); + } - assert_eq!(mode_of(&dir), DIR_MODE); - assert_eq!(mode_of(dir.parent().unwrap()), DIR_MODE); - } + #[test] + fn test_prepare_home_dir_hardens_an_exposed_chosen_datadir() { + let temp_dir = TempDir::new().unwrap(); + let dir = temp_dir.path().join("shared"); + fs::create_dir(&dir).unwrap(); + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); - #[test] - fn test_prepare_home_dir_hardens_an_exposed_chosen_datadir() { - let temp_dir = TempDir::new().unwrap(); - let dir = temp_dir.path().join("shared"); - fs::create_dir(&dir).unwrap(); - fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); + prepare_home_dir(Some(dir.clone())).unwrap(); - prepare_home_dir(Some(dir.clone())).unwrap(); + assert_eq!(mode_of(&dir), DIR_MODE); + } - assert_eq!(mode_of(&dir), DIR_MODE); + #[test] + fn test_prepare_wallet_db_dir_is_owner_only() { + let temp_dir = TempDir::new().unwrap(); + let home = temp_dir.path(); + + let dir = prepare_wallet_db_dir(home, "hot").unwrap(); + + assert_eq!(mode_of(&dir), DIR_MODE); + } } #[test] - fn test_prepare_wallet_db_dir_is_owner_only() { - let temp_dir = TempDir::new().unwrap(); - let home = temp_dir.path(); - - let dir = prepare_wallet_db_dir(home, "hot").unwrap(); + fn parses_whole_and_fractional_fee_rates() { + let one_sat_vb = FeeRate::from_sat_per_vb(1).unwrap(); + assert_eq!(parse_fee_rate("1").unwrap(), one_sat_vb); + assert_eq!(parse_fee_rate("1.0").unwrap(), one_sat_vb); + assert_eq!(parse_fee_rate(" 1 ").unwrap(), one_sat_vb); + assert_eq!( + parse_fee_rate("2.7").unwrap(), + FeeRate::from_sat_per_kwu(675) + ); + assert_eq!( + parse_fee_rate("0.004").unwrap(), + FeeRate::from_sat_per_kwu(1) + ); + assert_eq!( + parse_fee_rate("0.9").unwrap(), + FeeRate::from_sat_per_kwu(225) + ); + } - assert_eq!(mode_of(&dir), DIR_MODE); + #[test] + fn rejects_fee_rates_that_cannot_be_honoured() { + for input in ["0", "-5", "NaN", "inf", "1e30", "abc", ""] { + assert!( + parse_fee_rate(input).is_err(), + "fee rate '{input}' should be rejected" + ); + } } } diff --git a/tests/integration/offline.rs b/tests/integration/offline.rs index 3bb0f891..4b678eba 100644 --- a/tests/integration/offline.rs +++ b/tests/integration/offline.rs @@ -189,6 +189,34 @@ mod test_offline { .stderr(predicate::str::contains("Base64 decoding error")); } + /// An invalid `--fee_rate` is rejected up front instead of + /// silently becoming a zero fee or the builder default. + #[test] + fn test_create_tx_rejects_unusable_fee_rates() { + let (cli, mut cmd_init) = setup_wallet_config(); + cmd_init.assert().success(); + + for (fee_rate, expected) in [ + ("0", "below the smallest usable rate"), + ("NaN", "must be a finite number"), + ("1e30", "too large to represent"), + ("abc", "expected a number in sat/vB"), + ] { + cli.wallet_cmd(&[ + "--wallet", + WALLET_NAME, + "create_tx", + "--to", + RECIPIENT, + "--fee_rate", + fee_rate, + ]) + .assert() + .code(2) + .stderr(predicate::str::contains(expected)); + } + } + #[cfg(feature = "message_signer")] #[test] fn test_sign_message_and_verify_message() {