Avoid stringly typed arguments in payjoin-cli App trait (#479)

A more comprehensive approach might be to allow explicit units
to be parsed from the string form of these arguments (see also
rust-bitcoin/rust-bitcoin#1786) that may be undesirable since it
complicates payjoin-cli, and as a reference implementation it
should prioritize readability and simplicity over
convenience/flexibility of the CLI.

Author: DanGould

payjoin-cli/src/app/mod.rs

@@ -7,6 +7,7 @@ use bitcoin::TxIn;
 use bitcoincore_rpc::bitcoin::Amount;
 use bitcoincore_rpc::RpcApi;
 use payjoin::bitcoin::psbt::Psbt;
+use payjoin::bitcoin::FeeRate;
 use payjoin::receive::InputPair;
 use payjoin::{bitcoin, PjUri};
 
@@ -27,18 +28,15 @@ pub trait App {
     where
         Self: Sized;
     fn bitcoind(&self) -> Result<bitcoincore_rpc::Client>;
-    async fn send_payjoin(&self, bip21: &str, fee_rate: &f32) -> Result<()>;
-    async fn receive_payjoin(self, amount_arg: &str) -> Result<()>;
+    async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()>;
+    async fn receive_payjoin(self, amount: Amount) -> Result<()>;
 
-    fn create_original_psbt(&self, uri: &PjUri, fee_rate: &f32) -> Result<Psbt> {
+    fn create_original_psbt(&self, uri: &PjUri, fee_rate: FeeRate) -> Result<Psbt> {
         let amount = uri.amount.ok_or_else(|| anyhow!("please specify the amount in the Uri"))?;
 
         // wallet_create_funded_psbt requires a HashMap<address: String, Amount>
         let mut outputs = HashMap::with_capacity(1);
         outputs.insert(uri.address.to_string(), amount);
-        let fee_rate_sat_per_kwu = fee_rate * 250.0_f32;
-        let fee_rate: bitcoin::FeeRate =
-            bitcoin::FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu.ceil() as u64);
         let fee_sat_per_kvb =
             fee_rate.to_sat_per_kwu().checked_mul(4).ok_or(anyhow!("Invalid fee rate"))?;
         let fee_per_kvb = Amount::from_sat(fee_sat_per_kvb);

payjoin-cli/src/app/v1.rs

@@ -68,14 +68,12 @@ impl AppTrait for App {
         .with_context(|| "Failed to connect to bitcoind")
     }
 
-    async fn send_payjoin(&self, bip21: &str, fee_rate: &f32) -> Result<()> {
+    async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()> {
         let uri =
             Uri::try_from(bip21).map_err(|e| anyhow!("Failed to create URI from BIP21: {}", e))?;
         let uri = uri.assume_checked();
         let uri = uri.check_pj_supported().map_err(|_| anyhow!("URI does not support Payjoin"))?;
         let psbt = self.create_original_psbt(&uri, fee_rate)?;
-        let fee_rate_sat_per_kwu = fee_rate * 250.0_f32;
-        let fee_rate = FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu.ceil() as u64);
         let (req, ctx) = SenderBuilder::new(psbt, uri.clone())
             .build_recommended(fee_rate)
             .with_context(|| "Failed to build payjoin request")?
@@ -109,8 +107,8 @@ impl AppTrait for App {
         Ok(())
     }
 
-    async fn receive_payjoin(self, amount_arg: &str) -> Result<()> {
-        let pj_uri_string = self.construct_payjoin_uri(amount_arg, None)?;
+    async fn receive_payjoin(self, amount: Amount) -> Result<()> {
+        let pj_uri_string = self.construct_payjoin_uri(amount, None)?;
         println!(
             "Listening at {}. Configured to accept payjoin at BIP 21 Payjoin Uri:",
             self.config.port
@@ -125,11 +123,10 @@ impl AppTrait for App {
 impl App {
     fn construct_payjoin_uri(
         &self,
-        amount_arg: &str,
+        amount: Amount,
         fallback_target: Option<&str>,
     ) -> Result<String> {
         let pj_receiver_address = self.bitcoind()?.get_new_address(None, None)?.assume_checked();
-        let amount = Amount::from_sat(amount_arg.parse()?);
         let pj_part = match fallback_target {
             Some(target) => target,
             None => self.config.pj_endpoint.as_str(),

payjoin-cli/src/app/v2.rs

@@ -54,7 +54,7 @@ impl AppTrait for App {
         .with_context(|| "Failed to connect to bitcoind")
     }
 
-    async fn send_payjoin(&self, bip21: &str, fee_rate: &f32) -> Result<()> {
+    async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()> {
         use payjoin::UriExt;
         let uri =
             Uri::try_from(bip21).map_err(|e| anyhow!("Failed to create URI from BIP21: {}", e))?;
@@ -66,8 +66,6 @@ impl AppTrait for App {
             Some(send_session) => send_session,
             None => {
                 let psbt = self.create_original_psbt(&uri, fee_rate)?;
-                let fee_rate_sat_per_kwu = fee_rate * 250.0_f32;
-                let fee_rate = FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu.ceil() as u64);
                 let mut req_ctx = SenderBuilder::new(psbt, uri.clone())
                     .build_recommended(fee_rate)
                     .with_context(|| "Failed to build payjoin request")?;
@@ -78,9 +76,8 @@ impl AppTrait for App {
         self.spawn_payjoin_sender(req_ctx).await
     }
 
-    async fn receive_payjoin(self, amount_arg: &str) -> Result<()> {
+    async fn receive_payjoin(self, amount: Amount) -> Result<()> {
         let address = self.bitcoind()?.get_new_address(None, None)?.assume_checked();
-        let amount = Amount::from_sat(amount_arg.parse()?);
         let ohttp_keys = unwrap_ohttp_keys_or_else_fetch(&self.config).await?;
         let session =
             Receiver::new(address, self.config.pj_directory.clone(), ohttp_keys.clone(), None);

payjoin-cli/src/main.rs

@@ -2,6 +2,8 @@ use anyhow::{Context, Result};
 use app::config::AppConfig;
 use app::App as AppTrait;
 use clap::{arg, value_parser, Arg, ArgMatches, Command};
+use payjoin::bitcoin::amount::ParseAmountError;
+use payjoin::bitcoin::{Amount, FeeRate};
 use url::Url;
 
 mod app;
@@ -23,14 +25,15 @@ async fn main() -> Result<()> {
     match matches.subcommand() {
         Some(("send", sub_matches)) => {
             let bip21 = sub_matches.get_one::<String>("BIP21").context("Missing BIP21 argument")?;
-            let fee_rate_sat_per_vb =
-                sub_matches.get_one::<f32>("fee_rate").context("Missing --fee-rate argument")?;
-            app.send_payjoin(bip21, fee_rate_sat_per_vb).await?;
+            let fee_rate = sub_matches
+                .get_one::<FeeRate>("fee_rate")
+                .context("Missing --fee-rate argument")?;
+            app.send_payjoin(bip21, *fee_rate).await?;
         }
         Some(("receive", sub_matches)) => {
             let amount =
-                sub_matches.get_one::<String>("AMOUNT").context("Missing AMOUNT argument")?;
-            app.receive_payjoin(amount).await?;
+                sub_matches.get_one::<Amount>("AMOUNT").context("Missing AMOUNT argument")?;
+            app.receive_payjoin(*amount).await?;
         }
         #[cfg(feature = "v2")]
         Some(("resume", _)) => {
@@ -98,13 +101,13 @@ fn cli() -> ArgMatches {
                     .long("fee-rate")
                     .value_name("FEE_SAT_PER_VB")
                     .help("Fee rate in sat/vB")
-                    .value_parser(value_parser!(f32)),
+                    .value_parser(parse_feerate_in_sat_per_vb),
             ),
     );
 
     let mut receive_cmd = Command::new("receive")
         .arg_required_else_help(true)
-        .arg(arg!(<AMOUNT> "The amount to receive in satoshis"))
+        .arg(arg!(<AMOUNT> "The amount to receive in satoshis").value_parser(parse_amount_in_sat))
         .arg_required_else_help(true);
 
     #[cfg(feature = "v2")]
@@ -152,3 +155,13 @@ fn cli() -> ArgMatches {
     cmd = cmd.subcommand(receive_cmd);
     cmd.get_matches()
 }
+
+fn parse_amount_in_sat(s: &str) -> Result<Amount, ParseAmountError> {
+    Amount::from_str_in(s, payjoin::bitcoin::Denomination::Satoshi)
+}
+
+fn parse_feerate_in_sat_per_vb(s: &str) -> Result<FeeRate, std::num::ParseFloatError> {
+    let fee_rate_sat_per_vb: f32 = s.parse()?;
+    let fee_rate_sat_per_kwu = fee_rate_sat_per_vb * 250.0_f32;
+    Ok(FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu.ceil() as u64))
+}