Skip to content

Commit

Permalink
Allow receiver to set minimum fee_rate
Browse files Browse the repository at this point in the history
    Payjoin receiver can set minimum `fee_rate`
    in order to reject payjoin requests that contain
    a psbt which doesnt meet the minimum `fee_rate`
    threshold.

    We add this functionality to `check_broadcast_suitability`
    function which previously was called `check_can_broadcast`.
  • Loading branch information
jbesraa committed Dec 8, 2023
1 parent 08ee414 commit 1764e1c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 9 deletions.
2 changes: 1 addition & 1 deletion payjoin-cli/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))?;
Expand Down
13 changes: 13 additions & 0 deletions payjoin/src/receive/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ 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 provided by the sender
/// 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<InternalRequestError> for RequestError {
Expand Down Expand Up @@ -125,6 +133,11 @@ 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!("Fee too low: {} < {}.", original_psbt_fee_rate, receiver_min_fee_rate),
),
}
}
}
Expand Down
40 changes: 34 additions & 6 deletions payjoin/src/receive/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -348,22 +348,44 @@ impl UncheckedProposal {
self.psbt.clone().extract_tx()
}

/// Call after checking that the Original PSBT can be broadcast.
/// The Sender's Original PSBT fee rate.
pub fn psbt_fee_rate(&self) -> Result<FeeRate, Error> {
if let Ok(original_psbt_fee) = self.psbt.fee() {
let original_transaction = self.psbt.clone().extract_tx();
Ok(original_psbt_fee / original_transaction.weight())
} else {
Err(Error::BadRequest(InternalRequestError::OriginalPsbtNotBroadcastable.into()))
}
}

/// 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.
/// If a sender can make requests at will, they can learn which bitcoin the receiver owns at no cost.
/// 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_feerate: Option<FeeRate>,
can_broadcast: impl Fn(&bitcoin::Transaction) -> Result<bool, Error>,
) -> Result<MaybeInputsOwned, Error> {
let original_psbt_fee_rate = self.psbt_fee_rate()?;
if let Some(min_feerate) = min_feerate {
if original_psbt_fee_rate < min_feerate {
return Err(Error::BadRequest(
InternalRequestError::PsbtBelowFeeRate(original_psbt_fee_rate, min_feerate).into(),
));
}
}
if can_broadcast(&self.psbt.clone().extract_tx())? {
Ok(MaybeInputsOwned { psbt: self.psbt, params: self.params })
} else {
Expand Down Expand Up @@ -894,4 +916,10 @@ mod test {

assert!(payjoin.is_ok(), "Payjoin should be a valid PSBT");
}

#[test]
fn psbt_fee_rate() {
let proposal = proposal_from_test_vector(None).unwrap();
assert_eq!(proposal.psbt_fee_rate().unwrap().to_sat_per_vb_floor(), 2);
}
}
20 changes: 18 additions & 2 deletions payjoin/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 = bitcoin::FeeRate::from_sat_per_kwu(281);
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()
Expand Down

0 comments on commit 1764e1c

Please sign in to comment.