Fix transaction creation input validation and fee rate calculation

CHANGELOG.md

@@ -6,6 +6,9 @@ page. See [DEVELOPMENT_CYCLE.md](DEVELOPMENT_CYCLE.md) for more details.
 ## [Unreleased]
 
 - Added support for Multipath (two-paths) descriptors.
+- 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
+- Enforced a size limit on `--add_data` and `--add_string` OP_RETURN payloads, matching Bitcoin Core v30's default `-datacarriersize` of 100000 bytes
 
 
 ## [4.0.0]

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,
 

src/handlers/dns/mod.rs

@@ -6,11 +6,12 @@ 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_op_return_data, parse_outpoint, parse_recipient,
+};
 use bdk_wallet::KeychainKind;
 use bdk_wallet::bitcoin::base64::Engine;
 use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD;
-use bdk_wallet::bitcoin::script::PushBytesBuf;
 use bdk_wallet::bitcoin::{Amount, FeeRate, OutPoint, ScriptBuf, Sequence};
 use clap::Parser;
 use std::collections::BTreeMap;
@@ -57,8 +58,8 @@ pub struct CreateDnsTxCommand {
     pub utxos: Option<Vec<OutPoint>>,
     #[arg(env = "CANT_SPEND_TXID:VOUT", long = "unspendable", value_parser = parse_outpoint)]
     pub unspendable: Option<Vec<OutPoint>>,
-    #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate")]
-    pub fee_rate: Option<f32>,
+    #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate", value_parser = parse_fee_rate)]
+    pub fee_rate: Option<FeeRate>,
     #[arg(env = "EXT_POLICY", long = "external_policy")]
     pub external_policy: Option<String>,
     #[arg(env = "INT_POLICY", long = "internal_policy")]
@@ -129,30 +130,20 @@ impl AsyncAppCommand<AppContext<OfflineOperations<'_>>> 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 {
-            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()))?;
-            tx_builder.add_data(
-                &PushBytesBuf::try_from(op_return_data)
-                    .map_err(|e| Error::Generic(e.to_string()))?,
-            );
+            let op_return_data = BASE64_STANDARD.decode(base64_data)?;
+            tx_builder.add_data(&parse_op_return_data(op_return_data)?);
         } else if let Some(string_data) = &self.add_string {
-            let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec())
-                .map_err(|e| Error::Generic(e.to_string()))?;
+            let data = parse_op_return_data(string_data.as_bytes().to_vec())?;
             tx_builder.add_data(&data);
         }
 

src/handlers/offline.rs

@@ -7,10 +7,9 @@ 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_op_return_data, parse_outpoint, parse_recipient};
 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::{KeychainKind, SignOptions};
 use clap::Parser;
@@ -217,8 +216,8 @@ pub struct CreateTxCommand {
     pub unspendable: Option<Vec<OutPoint>>,
 
     /// Fee rate to use in sat/vbyte.
-    #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate")]
-    pub fee_rate: Option<f32>,
+    #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate", value_parser = parse_fee_rate)]
+    pub fee_rate: Option<FeeRate>,
 
     /// Selects which policy should be used to satisfy the external descriptor.
     #[arg(env = "EXT_POLICY", long = "external_policy")]
@@ -228,7 +227,7 @@ pub struct CreateTxCommand {
     #[arg(env = "INT_POLICY", long = "internal_policy")]
     pub internal_policy: Option<String>,
 
-    /// Optionally create an OP_RETURN output containing given String in utf8 encoding (max 80 bytes)
+    /// Optionally create an OP_RETURN output containing given String in utf8 encoding (max 99_994 bytes)
     #[arg(
         env = "ADD_STRING",
         long = "add_string",
@@ -237,7 +236,7 @@ pub struct CreateTxCommand {
     )]
     pub add_string: Option<String>,
 
-    /// Optionally create an OP_RETURN output containing given base64 encoded String. (max 80 bytes)
+    /// Optionally create an OP_RETURN output containing given base64 encoded String. (max 99_994 bytes)
     #[arg(
         env = "ADD_DATA",
         long = "add_data",
@@ -281,25 +280,23 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> 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);
         }
 
         if let Some(utxos) = &self.utxos {
-            tx_builder.add_utxos(&utxos[..]).unwrap();
+            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).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(&parse_op_return_data(op_return_data)?);
         } else if let Some(string_data) = &self.add_string {
-            let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec()).unwrap();
+            let data = parse_op_return_data(string_data.as_bytes().to_vec())?;
             tx_builder.add_data(&data);
         }
 
@@ -319,8 +316,6 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> for CreateTxCommand {
 
         let psbt = tx_builder.finish()?;
 
-        // let psbt_base64 = BASE64_STANDARD.encode(psbt.serialize());
-
         Ok(PsbtResult::new(&psbt, Some(false)))
     }
 }
@@ -349,23 +344,23 @@ pub struct CreateSpTxCommand {
     #[arg(env = "CANT_SPEND_TXID:VOUT", long = "unspendable", value_parser = parse_outpoint)]
     pub unspendable: Option<Vec<OutPoint>>,
     /// Fee rate to use in sat/vbyte.
-    #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate")]
-    pub fee_rate: Option<f32>,
+    #[arg(env = "SATS_VBYTE", short = 'f', long = "fee_rate", value_parser = parse_fee_rate)]
+    pub fee_rate: Option<FeeRate>,
     /// Selects which policy should be used to satisfy the external descriptor.
     #[arg(env = "EXT_POLICY", long = "external_policy")]
     pub external_policy: Option<String>,
     /// Selects which policy should be used to satisfy the internal descriptor.
     #[arg(env = "INT_POLICY", long = "internal_policy")]
     pub internal_policy: Option<String>,
-    /// Optionally create an OP_RETURN output containing given String in utf8 encoding (max 80 bytes)
+    /// Optionally create an OP_RETURN output containing given String in utf8 encoding (max 99_994 bytes)
     #[arg(
         env = "ADD_STRING",
         long = "add_string",
         short = 's',
         conflicts_with = "add_data"
     )]
     pub add_string: Option<String>,
-    /// Optionally create an OP_RETURN output containing given base64 encoded String. (max 80 bytes)
+    /// Optionally create an OP_RETURN output containing given base64 encoded String. (max 99_994 bytes)
     #[arg(
         env = "ADD_DATA",
         long = "add_data",
@@ -436,33 +431,23 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> 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);
         }
 
         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()))?;
-            tx_builder.add_data(
-                &PushBytesBuf::try_from(op_return_data)
-                    .map_err(|e| Error::Generic(e.to_string()))?,
-            );
+            let op_return_data = BASE64_STANDARD.decode(base64_data)?;
+            tx_builder.add_data(&parse_op_return_data(op_return_data)?);
         } else if let Some(string_data) = &self.add_string {
-            let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec())
-                .map_err(|e| Error::Generic(e.to_string()))?;
+            let data = parse_op_return_data(string_data.as_bytes().to_vec())?;
             tx_builder.add_data(&data);
         }
 
@@ -573,9 +558,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<AppContext<OfflineOperations<'_>>> for BumpFeeCommand {
@@ -585,9 +571,7 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> 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();
@@ -599,7 +583,7 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> 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 {

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",

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<String>,
         blockchain_client: &BlockchainClient,
     ) -> Result<String, Error> {
@@ -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();