diff --git a/payjoin-cli/src/app.rs b/payjoin-cli/src/app.rs index 768f5147..7e1d9647 100644 --- a/payjoin-cli/src/app.rs +++ b/payjoin-cli/src/app.rs @@ -447,7 +447,7 @@ impl App { )?; // Receive Check 1: Can Broadcast - let proposal = proposal.check_can_broadcast(|tx| { + let proposal = proposal.check_broadcast_suitability(None, |tx| { let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx); let mempool_results = bitcoind.test_mempool_accept(&[raw_tx]).map_err(|e| Error::Server(e.into()))?; diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 5de611bd..015d9c58 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -86,6 +86,12 @@ pub(crate) enum InternalRequestError { ParsePsbt(bitcoin::psbt::PsbtParseError), #[cfg(feature = "v2")] Utf8(std::string::FromUtf8Error), + /// Original PSBT fee rate is below minimum fee rate set by the receiver. + /// + /// First argument is the calculated fee rate of the original PSBT. + /// + /// Second argument is the minimum fee rate optionaly set by the receiver. + PsbtBelowFeeRate(bitcoin::FeeRate, bitcoin::FeeRate), } impl From for RequestError { @@ -150,6 +156,17 @@ impl fmt::Display for RequestError { InternalRequestError::ParsePsbt(e) => write_error(f, "Error parsing PSBT:", e), #[cfg(feature = "v2")] InternalRequestError::Utf8(e) => write_error(f, "Error parsing PSBT:", e), + InternalRequestError::PsbtBelowFeeRate( + original_psbt_fee_rate, + receiver_min_fee_rate, + ) => write_error( + f, + "original-psbt-rejected", + &format!( + "Original PSBT fee rate too low: {} < {}.", + original_psbt_fee_rate, receiver_min_fee_rate + ), + ), } } } diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index 699c5fb6..3fe79b1c 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -123,7 +123,7 @@ //! We need to know this transaction is consensus-valid. //! //! ``` -//! let checked_1 = proposal.check_can_broadcast(|tx| { +//! let checked_1 = proposal.check_broadcast_suitability(None, |tx| { //! let raw_tx = bitcoin::consensus::encode::serialize(&tx).to_hex(); //! let mempool_results = self //! .bitcoind @@ -297,7 +297,7 @@ pub trait Headers { /// /// If you are implementing an interactive payment processor, you should get extract the original /// transaction with extract_tx_to_schedule_broadcast() and schedule, followed by checking -/// that the transaction can be broadcast with check_can_broadcast. Otherwise it is safe to +/// that the transaction can be broadcast with check_broadcast_suitability. Otherwise it is safe to /// call assume_interactive_receive to proceed with validation. #[derive(Debug, Clone)] pub struct UncheckedProposal { @@ -345,16 +345,24 @@ impl UncheckedProposal { Ok(UncheckedProposal { psbt, params }) } - /// The Sender's Original PSBT + /// The Sender's Original PSBT transaction pub fn extract_tx_to_schedule_broadcast(&self) -> bitcoin::Transaction { self.psbt.clone().extract_tx() } - /// Call after checking that the Original PSBT can be broadcast. + fn psbt_fee_rate(&self) -> Result { + let original_psbt_fee = self.psbt.fee().map_err(InternalRequestError::Psbt)?; + Ok(original_psbt_fee / self.extract_tx_to_schedule_broadcast().weight()) + } + + /// Check that the Original PSBT can be broadcasted. /// /// Receiver MUST check that the Original PSBT from the sender - /// can be broadcast, i.e. `testmempoolaccept` bitcoind rpc returns { "allowed": true,.. } - /// for `extract_tx_to_sheculed_broadcast()` before calling this method. + /// can be broadcast, i.e. `testmempoolaccept` bitcoind rpc returns { "allowed": true,.. }. + /// + /// Receiver can optionaly set a minimum feerate that will be enforced on the Original PSBT. + /// This can be used to prevent probing attacks and make it easier to deal with + /// high feerate environments. /// /// Do this check if you generate bitcoin uri to receive Payjoin on sender request without manual human approval, like a payment processor. /// Such so called "non-interactive" receivers are otherwise vulnerable to probing attacks. @@ -362,14 +370,25 @@ impl UncheckedProposal { /// Broadcasting the Original PSBT after some time in the failure case makes incurs sender cost and prevents probing. /// /// Call this after checking downstream. - pub fn check_can_broadcast( + pub fn check_broadcast_suitability( self, + min_fee_rate: Option, can_broadcast: impl Fn(&bitcoin::Transaction) -> Result, ) -> Result { + let original_psbt_fee_rate = self.psbt_fee_rate()?; + if let Some(min_fee_rate) = min_fee_rate { + if original_psbt_fee_rate < min_fee_rate { + return Err(InternalRequestError::PsbtBelowFeeRate( + original_psbt_fee_rate, + min_fee_rate, + ) + .into()); + } + } if can_broadcast(&self.psbt.clone().extract_tx())? { Ok(MaybeInputsOwned { psbt: self.psbt, params: self.params }) } else { - Err(Error::BadRequest(InternalRequestError::OriginalPsbtNotBroadcastable.into())) + Err(InternalRequestError::OriginalPsbtNotBroadcastable.into()) } } @@ -874,6 +893,7 @@ mod test { 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 .assume_interactive_receiver() .check_inputs_not_owned(|_| Ok(false)) diff --git a/payjoin/src/receive/v2.rs b/payjoin/src/receive/v2.rs index 97850990..65720cf0 100644 --- a/payjoin/src/receive/v2.rs +++ b/payjoin/src/receive/v2.rs @@ -323,7 +323,7 @@ impl Enrolled { /// /// If you are implementing an interactive payment processor, you should get extract the original /// transaction with extract_tx_to_schedule_broadcast() and schedule, followed by checking -/// that the transaction can be broadcast with check_can_broadcast. Otherwise it is safe to +/// that the transaction can be broadcast with check_broadcast_suitability. Otherwise it is safe to /// call assume_interactive_receive to proceed with validation. pub struct UncheckedProposal { inner: super::UncheckedProposal, @@ -364,11 +364,12 @@ impl UncheckedProposal { /// Broadcasting the Original PSBT after some time in the failure case makes incurs sender cost and prevents probing. /// /// Call this after checking downstream. - pub fn check_can_broadcast( + pub fn check_broadcast_suitability( self, + min_fee_rate: Option, can_broadcast: impl Fn(&bitcoin::Transaction) -> Result, ) -> Result { - let inner = self.inner.check_can_broadcast(can_broadcast)?; + let inner = self.inner.check_broadcast_suitability(min_fee_rate, can_broadcast)?; Ok(MaybeInputsOwned { inner, context: self.context }) } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 7848408a..3083631d 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -101,7 +101,7 @@ mod integration { // Receive Check 1: Can Broadcast let proposal = proposal - .check_can_broadcast(|tx| { + .check_broadcast_suitability(None, |tx| { Ok(receiver .test_mempool_accept(&[bitcoin::consensus::encode::serialize_hex(&tx)]) .unwrap() @@ -483,7 +483,7 @@ mod integration { // Receive Check 1: Can Broadcast let proposal = proposal - .check_can_broadcast(|tx| { + .check_broadcast_suitability(None, |tx| { Ok(receiver .test_mempool_accept(&[bitcoin::consensus::encode::serialize_hex(&tx)]) .unwrap()