diff --git a/payjoin-cli/src/app.rs b/payjoin-cli/src/app.rs index 8531fa046..8293b7fa6 100644 --- a/payjoin-cli/src/app.rs +++ b/payjoin-cli/src/app.rs @@ -273,7 +273,7 @@ impl App { let bitcoind = self.bitcoind().map_err(|e| Error::Server(e.into()))?; // in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx - let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast(); + let _to_broadcast_in_failure_case = proposal.psbt_transaction(); // The network is used for checks later let network = @@ -282,7 +282,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 ffc913605..f58c09560 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -65,6 +65,13 @@ pub(crate) enum InternalRequestError { /// Original PSBT input has been seen before. Only automatic receivers, aka "interactive" in the spec /// look out for these to prevent probing attacks. InputSeen(bitcoin::OutPoint), + /// Original PSBT fee rate is below minimum fee rate set by the receiver. + /// + /// First argument is the calculated fee rate of the original PSBT. + /// [`UncheckedProposal::psbt_fee_rate`] + /// + /// Second argument is the minimum fee rate optionaly set by the receiver. + PsbtBelowFeeRate(bitcoin::FeeRate, bitcoin::FeeRate), } impl From for RequestError { @@ -125,6 +132,17 @@ impl fmt::Display for RequestError { write_error(f, "original-psbt-rejected", &format!("Input Type Error: {}.", e)), InternalRequestError::InputSeen(_) => write_error(f, "original-psbt-rejected", "The receiver rejected the original PSBT."), + 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 3de4b852c..932e8c22c 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 @@ -295,7 +295,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 { @@ -344,15 +344,25 @@ impl UncheckedProposal { } /// The Sender's Original PSBT - pub fn extract_tx_to_schedule_broadcast(&self) -> bitcoin::Transaction { - self.psbt.clone().extract_tx() + pub fn psbt_transaction(&self) -> bitcoin::Transaction { self.psbt.clone().extract_tx() } + + /// The Sender's Original PSBT fee rate. + pub fn psbt_fee_rate(&self) -> Result { + if let Ok(original_psbt_fee) = self.psbt.fee() { + Ok(original_psbt_fee / self.psbt_transaction().weight()) + } else { + Err(Error::BadRequest(InternalRequestError::OriginalPsbtNotBroadcastable.into())) + } } - /// Call after checking that the Original PSBT can be broadcast. + /// 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. @@ -360,10 +370,20 @@ 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(Error::BadRequest( + 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 { @@ -891,4 +911,10 @@ mod test { assert!(payjoin.is_ok(), "Payjoin should be a valid PSBT"); } + + #[test] + fn psbt_fee_rate() { + let proposal = proposal_from_test_vector().unwrap(); + assert_eq!(proposal.psbt_fee_rate().unwrap().to_sat_per_vb_floor(), 2); + } } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index dd4cc545b..918c9f82b 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -12,7 +12,7 @@ mod integration { use payjoin::bitcoin::base64; use payjoin::receive::Headers; use payjoin::send::{Request, RequestBuilder}; - use payjoin::{bitcoin, Error, Uri}; + use payjoin::{bitcoin, Uri}; #[test] fn integration_test() { @@ -143,8 +143,24 @@ mod integration { let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast(); // Receive Check 1: Can Broadcast + // Here we set receiver min feerate to be higher than the proposal's psbt feerate. + let original_psbt_fee_rate = proposal.psbt_fee_rate().unwrap(); + let receiver_min_feerate = original_psbt_fee_rate.checked_mul(10); + assert!(proposal + .clone() + .check_broadcast_suitability(receiver_min_feerate, |tx| { + Ok(receiver + .test_mempool_accept(&[bitcoin::consensus::encode::serialize_hex(&tx)]) + .unwrap() + .first() + .unwrap() + .allowed) + }) + .is_err()); + // Here we set receiver min feerate to be lower than the proposal's psbt feerate + let receiver_min_feerate = original_psbt_fee_rate.checked_div(10).unwrap(); let proposal = proposal - .check_can_broadcast(|tx| { + .check_broadcast_suitability(Some(receiver_min_feerate), |tx| { Ok(receiver .test_mempool_accept(&[bitcoin::consensus::encode::serialize_hex(&tx)]) .unwrap()