Allow receiver to specify a max feerate

This ensures that the receiver does not pay more in fees than they would
by building a separate transaction at max_feerate instead. That prevents
a scenario where the sender specifies a `minfeerate` param that would
require the receiver to pay more than they are willing to spend in fees.

Privacy-conscious receivers should err on the side of indulgence with
their preferred max feerate in order to satisfy their preference for
privacy.

This commit also adds a `max-feerate` receive option to payjoin-cli.

payjoin-cli/src/app/config.rs

@@ -15,6 +15,8 @@ pub struct AppConfig {
     pub bitcoind_rpcuser: String,
     pub bitcoind_rpcpassword: String,
     pub db_path: PathBuf,
+    // receive-only
+    pub max_feerate: Option<u64>,
 
     // v2 only
     #[cfg(feature = "v2")]
@@ -84,10 +86,22 @@ impl AppConfig {
                         .map_err(|_| {
                             ConfigError::Message("\"port\" must be a valid number".to_string())
                         })?;
-                    builder.set_override_option("port", port)?.set_override_option(
-                        "pj_endpoint",
-                        matches.get_one::<Url>("pj_endpoint").map(|s| s.as_str()),
-                    )?
+                    let max_feerate = matches
+                        .get_one::<String>("max_feerate")
+                        .map(|max_feerate| max_feerate.parse::<u64>())
+                        .transpose()
+                        .map_err(|_| {
+                            ConfigError::Message(
+                                "\"max_feerate\" must be a valid amount".to_string(),
+                            )
+                        })?;
+                    builder
+                        .set_override_option("port", port)?
+                        .set_override_option(
+                            "pj_endpoint",
+                            matches.get_one::<Url>("pj_endpoint").map(|s| s.as_str()),
+                        )?
+                        .set_override_option("max_feerate", max_feerate)?
                 };
 
                 #[cfg(feature = "v2")]

payjoin-cli/src/app/v1.rs

@@ -14,7 +14,7 @@ use hyper::service::service_fn;
 use hyper::{Method, Request, Response, StatusCode};
 use hyper_util::rt::TokioIo;
 use payjoin::bitcoin::psbt::Psbt;
-use payjoin::bitcoin::{self};
+use payjoin::bitcoin::{self, FeeRate};
 use payjoin::receive::{PayjoinProposal, UncheckedProposal};
 use payjoin::{Error, PjUriBuilder, Uri, UriExt};
 use tokio::net::TcpListener;
@@ -366,6 +366,7 @@ impl App {
                     .map_err(|e| Error::Server(e.into()))?
             },
             Some(bitcoin::FeeRate::MIN),
+            self.config.max_feerate.and_then(FeeRate::from_sat_per_vb),
         )?;
         Ok(payjoin_proposal)
     }

payjoin-cli/src/app/v2.rs

@@ -6,7 +6,7 @@ use anyhow::{anyhow, Context, Result};
 use bitcoincore_rpc::RpcApi;
 use payjoin::bitcoin::consensus::encode::serialize_hex;
 use payjoin::bitcoin::psbt::Psbt;
-use payjoin::bitcoin::Amount;
+use payjoin::bitcoin::{Amount, FeeRate};
 use payjoin::receive::v2::ActiveSession;
 use payjoin::send::RequestContext;
 use payjoin::{bitcoin, Error, Uri};
@@ -343,6 +343,7 @@ impl App {
                     .map_err(|e| Error::Server(e.into()))?
             },
             Some(bitcoin::FeeRate::MIN),
+            self.config.max_feerate.and_then(FeeRate::from_sat_per_vb),
         )?;
         let payjoin_proposal_psbt = payjoin_proposal.psbt();
         log::debug!("Receiver's Payjoin proposal PSBT Rsponse: {:#?}", payjoin_proposal_psbt);

payjoin-cli/src/main.rs

@@ -111,6 +111,12 @@ fn cli() -> ArgMatches {
     let mut cmd = cmd.subcommand(Command::new("resume"));
 
     // Conditional arguments based on features for the receive subcommand
+    receive_cmd = receive_cmd.arg(
+        Arg::new("max_feerate")
+            .long("max-feerate")
+            .num_args(1)
+            .help("The maximum effective feerate the receiver is willing to pay (sat/vB)"),
+    );
     #[cfg(not(feature = "v2"))]
     {
         receive_cmd = receive_cmd.arg(

payjoin/src/receive/error.rs

@@ -90,6 +90,8 @@ pub(crate) enum InternalRequestError {
     ///
     /// Second argument is the minimum fee rate optionaly set by the receiver.
     PsbtBelowFeeRate(bitcoin::FeeRate, bitcoin::FeeRate),
+    /// Receiver fee exceeds maximum allowed fee
+    FeeTooHigh(bitcoin::Amount, bitcoin::Amount),
 }
 
 impl From<InternalRequestError> for RequestError {
@@ -172,6 +174,14 @@ impl fmt::Display for RequestError {
                     original_psbt_fee_rate, receiver_min_fee_rate
                 ),
             ),
+            InternalRequestError::FeeTooHigh(proposed_fee, max_fee) => write_error(
+                f,
+                "original-psbt-rejected",
+                &format!(
+                    "Receiver fee exceeds maximum allowed fee: {} > {}",
+                    proposed_fee, max_fee
+                ),
+            ),
         }
     }
 }

payjoin/src/receive/mod.rs

@@ -672,7 +672,11 @@ pub struct ProvisionalProposal {
 impl ProvisionalProposal {
     /// Apply additional fee contribution now that the receiver has contributed input
     /// this is kind of a "build_proposal" step before we sign and finalize and extract
-    fn apply_fee(&mut self, min_feerate: Option<FeeRate>) -> Result<&Psbt, RequestError> {
+    fn apply_fee(
+        &mut self,
+        min_feerate: Option<FeeRate>,
+        max_feerate: Option<FeeRate>,
+    ) -> Result<&Psbt, RequestError> {
         let min_feerate = min_feerate.unwrap_or(FeeRate::MIN);
         log::trace!("min_feerate: {:?}", min_feerate);
         log::trace!("params.min_feerate: {:?}", self.params.min_feerate);
@@ -682,13 +686,15 @@ impl ProvisionalProposal {
         // If the sender specified a fee contribution, the receiver is allowed to decrease the
         // sender's fee output to pay for additional input fees. Any fees in excess of
         // `max_additional_fee_contribution` must be covered by the receiver.
-        let additional_fee = self.additional_input_fee(min_feerate)?;
+        let input_contribution_weight = self.additional_input_weight()?;
+        let additional_fee = input_contribution_weight * min_feerate;
+        log::trace!("additional_fee: {}", additional_fee);
+        let mut receiver_additional_fee = additional_fee;
         if additional_fee > Amount::ZERO {
             log::trace!(
                 "self.params.additional_fee_contribution: {:?}",
                 self.params.additional_fee_contribution
             );
-            let mut receiver_additional_fee = additional_fee;
             if let Some((max_additional_fee_contribution, additional_fee_output_index)) =
                 self.params.additional_fee_contribution
             {
@@ -713,28 +719,33 @@ impl ProvisionalProposal {
                     sender_additional_fee;
                 receiver_additional_fee -= sender_additional_fee;
             }
-
-            // The receiver covers any additional fees if applicable
-            if receiver_additional_fee > Amount::ZERO {
-                log::trace!("receiver_additional_fee: {}", receiver_additional_fee);
-                // Remove additional miner fee from the receiver's specified output
-                self.payjoin_psbt.unsigned_tx.output[self.change_vout].value -=
-                    receiver_additional_fee;
-            }
         }
 
         // The sender's fee contribution can only be used to pay for additional input weight, so
         // any additional outputs must be paid for by the receiver.
-        let additional_receiver_fee = self.additional_output_fee(min_feerate);
-        if additional_receiver_fee > Amount::ZERO {
+        let output_contribution_weight = self.additional_output_weight();
+        receiver_additional_fee += output_contribution_weight * min_feerate;
+        log::trace!("receiver_additional_fee: {}", receiver_additional_fee);
+        if let Some(feerate) = max_feerate {
+            // Ensure that the receiver does not pay more in fees
+            // than they would by building a separate transaction at max_feerate instead.
+            let max_fee = (input_contribution_weight + output_contribution_weight) * feerate;
+            log::trace!("max_fee: {}", max_fee);
+            if receiver_additional_fee > max_fee {
+                return Err(
+                    InternalRequestError::FeeTooHigh(receiver_additional_fee, max_fee).into()
+                );
+            }
+        }
+        if receiver_additional_fee > Amount::ZERO {
             // Remove additional miner fee from the receiver's specified output
-            self.payjoin_psbt.unsigned_tx.output[self.change_vout].value -= additional_receiver_fee;
+            self.payjoin_psbt.unsigned_tx.output[self.change_vout].value -= receiver_additional_fee;
         }
         Ok(&self.payjoin_psbt)
     }
 
-    /// Calculate the additional fee required to pay for any additional inputs in the payjoin tx
-    fn additional_input_fee(&self, min_feerate: FeeRate) -> Result<Amount, RequestError> {
+    /// Calculate the additional input weight contributed by the receiver
+    fn additional_input_weight(&self) -> Result<Weight, RequestError> {
         // This error should never happen. We check for at least one input in the constructor
         let input_pair = self
             .payjoin_psbt
@@ -751,13 +762,11 @@ impl ProvisionalProposal {
         log::trace!("weight_per_input : {}", weight_per_input);
         let contribution_weight = weight_per_input * input_count as u64;
         log::trace!("contribution_weight: {}", contribution_weight);
-        let additional_fee = contribution_weight * min_feerate;
-        log::trace!("additional_fee: {}", additional_fee);
-        Ok(additional_fee)
+        Ok(contribution_weight)
     }
 
-    /// Calculate the additional fee required to pay for any additional outputs in the payjoin tx
-    fn additional_output_fee(&self, min_feerate: FeeRate) -> Amount {
+    /// Calculate the additional output weight contributed by the receiver
+    fn additional_output_weight(&self) -> Weight {
         let payjoin_outputs_weight = self
             .payjoin_psbt
             .unsigned_tx
@@ -772,9 +781,7 @@ impl ProvisionalProposal {
             .fold(Weight::ZERO, |acc, txo| acc + txo.weight());
         let output_contribution_weight = payjoin_outputs_weight - original_outputs_weight;
         log::trace!("output_contribution_weight : {}", output_contribution_weight);
-        let additional_fee = output_contribution_weight * min_feerate;
-        log::trace!("additional_fee: {}", additional_fee);
-        additional_fee
+        output_contribution_weight
     }
 
     /// Return a Payjoin Proposal PSBT that the sender will find acceptable.
@@ -833,14 +840,15 @@ impl ProvisionalProposal {
         mut self,
         wallet_process_psbt: impl Fn(&Psbt) -> Result<Psbt, Error>,
         min_feerate_sat_per_vb: Option<FeeRate>,
+        max_feerate_sat_per_vb: Option<FeeRate>,
     ) -> Result<PayjoinProposal, Error> {
         for i in self.sender_input_indexes() {
             log::trace!("Clearing sender script signatures for input {}", i);
             self.payjoin_psbt.inputs[i].final_script_sig = None;
             self.payjoin_psbt.inputs[i].final_script_witness = None;
             self.payjoin_psbt.inputs[i].tap_key_sig = None;
         }
-        let psbt = self.apply_fee(min_feerate_sat_per_vb)?;
+        let psbt = self.apply_fee(min_feerate_sat_per_vb, max_feerate_sat_per_vb)?;
         let psbt = wallet_process_psbt(psbt)?;
         let payjoin_proposal = self.prepare_psbt(psbt)?;
         Ok(payjoin_proposal)
@@ -868,6 +876,10 @@ impl PayjoinProposal {
 
 #[cfg(test)]
 mod test {
+    use std::str::FromStr;
+
+    use bitcoin::hashes::Hash;
+    use bitcoin::{Address, Network, ScriptBuf};
     use rand::rngs::StdRng;
     use rand::SeedableRng;
 
@@ -916,10 +928,6 @@ mod test {
 
     #[test]
     fn unchecked_proposal_unlocks_after_checks() {
-        use std::str::FromStr;
-
-        use bitcoin::{Address, Network};
-
         let proposal = proposal_from_test_vector().unwrap();
         assert_eq!(proposal.psbt_fee_rate().unwrap().to_sat_per_vb_floor(), 2);
         let mut payjoin = proposal
@@ -942,11 +950,63 @@ mod test {
             .commit_outputs()
             .commit_inputs();
 
-        let payjoin = payjoin.apply_fee(None);
+        let payjoin = payjoin.apply_fee(None, None);
 
         assert!(payjoin.is_ok(), "Payjoin should be a valid PSBT");
     }
 
+    #[test]
+    fn sender_specifies_excessive_feerate() {
+        let mut proposal = proposal_from_test_vector().unwrap();
+        assert_eq!(proposal.psbt_fee_rate().unwrap().to_sat_per_vb_floor(), 2);
+        // Specify excessive fee rate in sender params
+        proposal.params.min_feerate = FeeRate::from_sat_per_vb(1000).unwrap();
+        // Input contribution for the receiver, from the BIP78 test vector
+        let input: (OutPoint, TxOut) = (
+            OutPoint {
+                txid: "833b085de288cda6ff614c6e8655f61e7ae4f84604a2751998dc25a0d1ba278f"
+                    .parse()
+                    .unwrap(),
+                vout: 1,
+            },
+            TxOut {
+                value: Amount::from_sat(2000000),
+                // HACK: The script pubkey in the original test vector is a nested p2sh witness
+                // script, which is not correctly supported in our current weight calculations.
+                // To get around this limitation, this test uses a native segwit script instead.
+                script_pubkey: ScriptBuf::new_p2wpkh(&bitcoin::WPubkeyHash::hash(
+                    "00145f806655e5924c9204c2d51be5394f4bf9eda210".as_bytes(),
+                )),
+            },
+        );
+        let mut payjoin = proposal
+            .assume_interactive_receiver()
+            .check_inputs_not_owned(|_| Ok(false))
+            .expect("No inputs should be owned")
+            .check_no_mixed_input_scripts()
+            .expect("No mixed input scripts")
+            .check_no_inputs_seen_before(|_| Ok(false))
+            .expect("No inputs should be seen before")
+            .identify_receiver_outputs(|script| {
+                let network = Network::Bitcoin;
+                Ok(Address::from_script(script, network).unwrap()
+                    == Address::from_str(&"3CZZi7aWFugaCdUCS15dgrUUViupmB8bVM")
+                        .unwrap()
+                        .require_network(network)
+                        .unwrap())
+            })
+            .expect("Receiver output should be identified")
+            .commit_outputs()
+            .contribute_witness_inputs(vec![input])
+            .expect("Failed to contribute inputs")
+            .commit_inputs();
+        let mut payjoin_clone = payjoin.clone();
+        let psbt = payjoin.apply_fee(None, FeeRate::from_sat_per_vb(1000));
+        assert!(psbt.is_ok(), "Payjoin should be a valid PSBT");
+        let psbt = payjoin_clone.apply_fee(None, FeeRate::from_sat_per_vb(995));
+        assert!(psbt.is_err(), "Payjoin exceeds receiver fee preference and should error");
+    }
+
     #[test]
     fn test_interleave_shuffle() {
         let mut original1 = vec![1, 2, 3];

payjoin/src/receive/v2/mod.rs

@@ -492,8 +492,13 @@ impl ProvisionalProposal {
         self,
         wallet_process_psbt: impl Fn(&Psbt) -> Result<Psbt, Error>,
         min_feerate_sat_per_vb: Option<FeeRate>,
+        max_feerate_sat_per_vb: Option<FeeRate>,
     ) -> Result<PayjoinProposal, Error> {
-        let inner = self.inner.finalize_proposal(wallet_process_psbt, min_feerate_sat_per_vb)?;
+        let inner = self.inner.finalize_proposal(
+            wallet_process_psbt,
+            min_feerate_sat_per_vb,
+            max_feerate_sat_per_vb,
+        )?;
         Ok(PayjoinProposal { inner, context: self.context })
     }
 }

payjoin/tests/integration.rs

@@ -665,6 +665,7 @@ mod integration {
                             .unwrap())
                     },
                     Some(FeeRate::BROADCAST_MIN),
+                    FeeRate::from_sat_per_vb(2),
                 )
                 .unwrap();
             payjoin_proposal
@@ -1123,6 +1124,7 @@ mod integration {
                         .unwrap())
                 },
                 Some(FeeRate::BROADCAST_MIN),
+                FeeRate::from_sat_per_vb(2),
             )
             .unwrap();
         payjoin_proposal