refactor(op_return): parse OP_RETURN data at the clap boundary

- `create_tx`, `create_sp_tx` and `create_dns_tx` each decoded
`--add_data`/`--add_string` into a `PushBytesBuf` in the handler, with
the same block. Parse both arguments with clap value parsers.

- Malformed `--add_data` base64 is now rejected at argument parsing
(exit 2) before a wallet is loaded, rather than when building (exit 1).

diff --git a/src/handlers/dns/mod.rs b/src/handlers/dns/mod.rs
index 645672c11fa6d2f85745cc9d6b965ce8d7b2a574..ca83991de307ca06ac113827bda2d6686d45cd3d 100644 (file)
--- a/src/handlers/dns/mod.rs
+++ b/src/handlers/dns/mod.rs
@@ -6,12 +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_fee_rate, parse_outpoint, parse_recipient};
+use crate::utils::{
+    parse_dns_recipient, parse_fee_rate, parse_op_return_base64, parse_op_return_string,
+    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 bdk_wallet::bitcoin::{Amount, FeeRate, OutPoint, ScriptBuf, Sequence, script::PushBytesBuf};
 use clap::Parser;
 use std::collections::BTreeMap;
 
@@ -67,16 +67,18 @@ pub struct CreateDnsTxCommand {
         env = "ADD_STRING",
         long = "add_string",
         short = 's',
-        conflicts_with = "add_data"
+        conflicts_with = "add_data",
+        value_parser = parse_op_return_string,
     )]
-    pub add_string: Option<String>,
+    pub add_string: Option<PushBytesBuf>,
     #[arg(
         env = "ADD_DATA",
         long = "add_data",
         short = 'o',
-        conflicts_with = "add_string"
+        conflicts_with = "add_string",
+        value_parser = parse_op_return_base64,
     )]
-    pub add_data: Option<String>,
+    pub add_data: Option<PushBytesBuf>,
 }
 
 impl AsyncAppCommand<AppContext<OfflineOperations<'_>>> for CreateDnsTxCommand {
@@ -138,12 +140,8 @@ impl AsyncAppCommand<AppContext<OfflineOperations<'_>>> for CreateDnsTxCommand {
         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)?;
-            tx_builder.add_data(&PushBytesBuf::try_from(op_return_data)?);
-        } else if let Some(string_data) = &self.add_string {
-            let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec())?;
-            tx_builder.add_data(&data);
+        if let Some(data) = self.add_data.as_ref().or(self.add_string.as_ref()) {
+            tx_builder.add_data(data);
         }
 
         let policies = vec![

diff --git a/src/handlers/offline.rs b/src/handlers/offline.rs
index 15cdd26b2cb97001434154df4e18a892fe6a8dc6..20a2082e5a325a3fae067c76be7482f78fd1dc9a 100644 (file)
--- a/src/handlers/offline.rs
+++ b/src/handlers/offline.rs
@@ -2,12 +2,14 @@ use crate::commands::OfflineWalletSubCommand;
 use crate::error::BDKCliError as Error;
 use crate::handlers::{AppCommand, AppContext, OfflineOperations};
 use crate::utils::output::{FormatOutput, ListResult};
-use crate::utils::parse_address;
 use crate::utils::types::{
     AddressResult, BalanceResult, KeychainPair, PsbtResult, RawPsbt, TransactionDetails,
     UnspentDetails,
 };
-use crate::utils::{parse_fee_rate, parse_outpoint, parse_recipient};
+use crate::utils::{
+    parse_address, parse_fee_rate, parse_op_return_base64, parse_op_return_string, parse_outpoint,
+    parse_recipient,
+};
 use bdk_wallet::bitcoin::base64::Engine;
 use bdk_wallet::bitcoin::base64::prelude::BASE64_STANDARD;
 use bdk_wallet::bitcoin::script::PushBytesBuf;
@@ -233,18 +235,20 @@ pub struct CreateTxCommand {
         env = "ADD_STRING",
         long = "add_string",
         short = 's',
-        conflicts_with = "add_data"
+        conflicts_with = "add_data",
+        value_parser = parse_op_return_string,
     )]
-    pub add_string: Option<String>,
+    pub add_string: Option<PushBytesBuf>,
 
     /// Optionally create an OP_RETURN output containing given base64 encoded String. (max 80 bytes)
     #[arg(
         env = "ADD_DATA",
         long = "add_data",
         short = 'o',
-        conflicts_with = "add_string"
+        conflicts_with = "add_string",
+        value_parser = parse_op_return_base64,
     )]
-    pub add_data: Option<String>,
+    pub add_data: Option<PushBytesBuf>,
 }
 
 impl AppCommand<AppContext<OfflineOperations<'_>>> for CreateTxCommand {
@@ -293,12 +297,8 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> for CreateTxCommand {
             tx_builder.unspendable(unspendable.to_vec());
         }
 
-        if let Some(base64_data) = &self.add_data {
-            let op_return_data = BASE64_STANDARD.decode(base64_data)?;
-            tx_builder.add_data(&PushBytesBuf::try_from(op_return_data)?);
-        } else if let Some(string_data) = &self.add_string {
-            let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec())?;
-            tx_builder.add_data(&data);
+        if let Some(data) = self.add_data.as_ref().or(self.add_string.as_ref()) {
+            tx_builder.add_data(data);
         }
 
         let policies = vec![
@@ -358,17 +358,19 @@ pub struct CreateSpTxCommand {
         env = "ADD_STRING",
         long = "add_string",
         short = 's',
-        conflicts_with = "add_data"
+        conflicts_with = "add_data",
+        value_parser = parse_op_return_string,
     )]
-    pub add_string: Option<String>,
+    pub add_string: Option<PushBytesBuf>,
     /// Optionally create an OP_RETURN output containing given base64 encoded String. (max 80 bytes)
     #[arg(
         env = "ADD_DATA",
         long = "add_data",
         short = 'o',
-        conflicts_with = "add_string"
+        conflicts_with = "add_string",
+        value_parser = parse_op_return_base64,
     )]
-    pub add_data: Option<String>, //base 64 econding
+    pub add_data: Option<PushBytesBuf>,
 }
 
 #[cfg(feature = "silent-payments")]
@@ -444,12 +446,8 @@ impl AppCommand<AppContext<OfflineOperations<'_>>> for CreateSpTxCommand {
             tx_builder.unspendable(unspendable.to_vec());
         }
 
-        if let Some(base64_data) = &self.add_data {
-            let op_return_data = BASE64_STANDARD.decode(base64_data)?;
-            tx_builder.add_data(&PushBytesBuf::try_from(op_return_data)?);
-        } else if let Some(string_data) = &self.add_string {
-            let data = PushBytesBuf::try_from(string_data.as_bytes().to_vec())?;
-            tx_builder.add_data(&data);
+        if let Some(data) = self.add_data.as_ref().or(self.add_string.as_ref()) {
+            tx_builder.add_data(data);
         }
 
         let policies = vec![

diff --git a/src/utils/common.rs b/src/utils/common.rs
index 7a2d8dba98fbb17f36f8a5e13c981a9ab85ce187..775bb93c103453249a1d029dfb51b1009d75da3a 100644 (file)
--- a/src/utils/common.rs
+++ b/src/utils/common.rs
@@ -6,6 +6,10 @@ use bdk_message_signer::SignatureFormat;
 #[cfg(feature = "silent-payments")]
 use bdk_sp::encoding::SilentPaymentCode;
 use bdk_wallet::bitcoin::{Address, FeeRate, Network, OutPoint, ScriptBuf};
+use bdk_wallet::bitcoin::{
+    base64::{Engine, prelude::BASE64_STANDARD},
+    script::PushBytesBuf,
+};
 #[cfg(any(
     feature = "electrum",
     feature = "esplora",
@@ -135,6 +139,16 @@ pub(crate) fn parse_fee_rate(s: &str) -> Result<FeeRate, Error> {
     Ok(FeeRate::from_sat_per_kwu(sat_kwu as u64))
 }
 
+/// Parse an `OP_RETURN` payload given as a UTF-8 string.
+pub(crate) fn parse_op_return_string(s: &str) -> Result<PushBytesBuf, Error> {
+    Ok(PushBytesBuf::try_from(s.as_bytes().to_vec())?)
+}
+
+/// Parse an `OP_RETURN` payload given as base64.
+pub(crate) fn parse_op_return_base64(s: &str) -> Result<PushBytesBuf, Error> {
+    Ok(PushBytesBuf::try_from(BASE64_STANDARD.decode(s)?)?)
+}
+
 /// Prepare bdk-cli home directory
 ///
 /// This function is called to check if [`crate::CliOpts`] datadir is set.
@@ -579,6 +593,13 @@ mod tests {
         );
     }
 
+    #[test]
+    fn parses_op_return_payloads() {
+        assert_eq!(parse_op_return_string("hi").unwrap().as_bytes(), b"hi");
+        assert_eq!(parse_op_return_base64("aGk=").unwrap().as_bytes(), b"hi");
+        assert!(parse_op_return_base64("!!!not-base64!!!").is_err());
+    }
+
     #[test]
     fn rejects_fee_rates_that_cannot_be_honoured() {
         for input in ["0", "-5", "NaN", "inf", "1e30", "abc", ""] {

diff --git a/tests/integration/offline.rs b/tests/integration/offline.rs
index 417bf72b68b2c8bfe6e87c5e9297d850cd849018..c4be0a21b38dc60f7edc0063f8d70be0572f06d2 100644 (file)
--- a/tests/integration/offline.rs
+++ b/tests/integration/offline.rs
@@ -148,7 +148,8 @@ mod test_offline {
     }
 
     /// A malformed `create_tx` argument must be reported as an error (exit 1),
-    /// never as a panic (exit 101).
+    /// never as a panic (exit 101). an unknown outpoint when building (exit 1), and
+    /// malformed base64 already at argument parsing (exit 2).
     #[test]
     fn test_create_tx_rejects_malformed_input_without_panicking() {
         let (cli, mut cmd_init) = setup_wallet_config();
@@ -179,7 +180,7 @@ mod test_offline {
             "!!!not-base64!!!",
         ])
         .assert()
-        .code(1)
+        .code(2)
         .stderr(predicate::str::contains("Base64 decoding error"));
     }