diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index 58acd60e..794fa793 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -348,11 +348,8 @@ impl ProvisionalProposal { /// Proper coin selection allows payjoin to resemble ordinary transactions. /// To ensure the resemblance, a number of heuristics must be avoided. /// - /// UIH "Unnecessary input heuristic" is one class of them to avoid. We define - /// UIH1 and UIH2 according to the BlockSci practice - /// BlockSci UIH1 and UIH2: - // if min(out) < min(in) then UIH1 else UIH2 - // https://eprint.iacr.org/2022/589.pdf + /// UIH "Unnecessary input heuristic" is avoided for multi-output transactions. + /// A simple consolidation is otherwise chosen if available. pub fn try_preserving_privacy( &self, candidate_inputs: HashMap, @@ -361,11 +358,28 @@ impl ProvisionalProposal { return Err(SelectionError::from(InternalSelectionError::Empty)); } - if self.payjoin_psbt.outputs.len() != 2 { - // Current UIH techniques only support many-input, two-output transactions. + if self.payjoin_psbt.outputs.len() > 2 { + // This UIH avoidance function supports only + // many-input, n-output transactions such that n <= 2 for now return Err(SelectionError::from(InternalSelectionError::TooManyOutputs)); } + if self.payjoin_psbt.outputs.len() == 2 { + self.avoid_uih(candidate_inputs) + } else { + self.select_first_candidate(candidate_inputs) + } + } + + /// UIH "Unnecessary input heuristic" is one class of heuristics to avoid. We define + /// UIH1 and UIH2 according to the BlockSci practice + /// BlockSci UIH1 and UIH2: + // if min(in) > min(out) then UIH1 else UIH2 + // https://eprint.iacr.org/2022/589.pdf + fn avoid_uih( + &self, + candidate_inputs: HashMap, + ) -> Result { let min_original_out_sats = self .payjoin_psbt .unsigned_tx @@ -382,23 +396,17 @@ impl ProvisionalProposal { .min() .unwrap_or_else(|| Amount::MAX_MONEY.to_sat()); - // Assume many-input, two output to select the vout for now let prior_payment_sats = self.payjoin_psbt.unsigned_tx.output[self.owned_vouts[0]].value; - for candidate in candidate_inputs { - // TODO bound loop by timeout / iterations + for candidate in candidate_inputs { let candidate_sats = candidate.0.to_sat(); let candidate_min_out = min(min_original_out_sats, prior_payment_sats + candidate_sats); let candidate_min_in = min(min_original_in_sats, candidate_sats); - if candidate_min_out < candidate_min_in { + if candidate_min_in > candidate_min_out { // The candidate avoids UIH2 but conforms to UIH1: Optimal change heuristic. // It implies the smallest output is the sender's change address. return Ok(candidate.1); - } else { - // The candidate conforms to UIH2: Unnecessary input - // and could be identified as a potential payjoin - continue; } } @@ -406,6 +414,17 @@ impl ProvisionalProposal { Err(SelectionError::from(InternalSelectionError::NotFound)) } + fn select_first_candidate( + &self, + candidate_inputs: HashMap, + ) -> Result { + candidate_inputs + .values() + .next() + .cloned() + .ok_or_else(|| SelectionError::from(InternalSelectionError::NotFound)) + } + pub fn contribute_witness_input(&mut self, txo: TxOut, outpoint: OutPoint) { // The payjoin proposal must not introduce mixed input sequence numbers let original_sequence = self diff --git a/payjoin/src/receive/v2.rs b/payjoin/src/receive/v2.rs index 88dfa555..f2add385 100644 --- a/payjoin/src/receive/v2.rs +++ b/payjoin/src/receive/v2.rs @@ -382,7 +382,7 @@ impl ProvisionalProposal { /// UIH "Unnecessary input heuristic" is one class of them to avoid. We define /// UIH1 and UIH2 according to the BlockSci practice /// BlockSci UIH1 and UIH2: - // if min(out) < min(in) then UIH1 else UIH2 + // if min(in) > min(out) then UIH1 else UIH2 // https://eprint.iacr.org/2022/589.pdf pub fn try_preserving_privacy( &self, diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 1f247bfc..84c14e23 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -62,6 +62,11 @@ pub struct RequestBuilder<'a> { uri: PjUri<'a>, disable_output_substitution: bool, fee_contribution: Option<(bitcoin::Amount, Option)>, + /// Decreases the fee contribution instead of erroring. + /// + /// If this option is true and a transaction with change amount lower than fee + /// contribution is provided then instead of returning error the fee contribution will + /// be just lowered in the request to match the change amount. clamp_fee_contribution: bool, min_fee_rate: FeeRate, } @@ -108,6 +113,14 @@ impl<'a> RequestBuilder<'a> { // TODO support optional batched payout scripts. This would require a change to // build() which now checks for a single payee. let mut payout_scripts = std::iter::once(self.uri.address.script_pubkey()); + + // Check if the PSBT is a sweep transaction with only one output that's a payout script and no change + if self.psbt.unsigned_tx.output.len() == 1 + && payout_scripts.all(|script| script == self.psbt.unsigned_tx.output[0].script_pubkey) + { + return self.build_non_incentivizing(min_fee_rate); + } + if let Some((additional_fee_index, fee_available)) = self .psbt .unsigned_tx @@ -154,7 +167,7 @@ impl<'a> RequestBuilder<'a> { false, ); } - self.build_non_incentivizing() + self.build_non_incentivizing(min_fee_rate) } /// Offer the receiver contribution to pay for his input. @@ -187,12 +200,15 @@ impl<'a> RequestBuilder<'a> { /// /// While it's generally better to offer some contribution some users may wish not to. /// This function disables contribution. - pub fn build_non_incentivizing(mut self) -> Result { + pub fn build_non_incentivizing( + mut self, + min_fee_rate: FeeRate, + ) -> Result { // since this is a builder, these should already be cleared // but we'll reset them to be sure self.fee_contribution = None; self.clamp_fee_contribution = false; - self.min_fee_rate = FeeRate::ZERO; + self.min_fee_rate = min_fee_rate; self.build() } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 4397816d..aae2a96f 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -402,14 +402,9 @@ mod integration { .assume_checked() .check_pj_supported() .unwrap(); - let psbt = build_original_psbt(&sender, &pj_uri)?; + let psbt = build_sweep_psbt(&sender, &pj_uri)?; let mut req_ctx = RequestBuilder::from_psbt_and_uri(psbt.clone(), pj_uri.clone())? - .build_with_additional_fee( - Amount::from_sat(10000), - None, - FeeRate::ZERO, - false, - )?; + .build_recommended(payjoin::bitcoin::FeeRate::BROADCAST_MIN)?; let (Request { url, body, .. }, send_ctx) = req_ctx.extract_v2(directory.to_owned())?; let response = agent @@ -826,6 +821,31 @@ mod integration { Ok(Psbt::from_str(&psbt)?) } + fn build_sweep_psbt( + sender: &bitcoincore_rpc::Client, + pj_uri: &PjUri, + ) -> Result { + let mut outputs = HashMap::with_capacity(1); + outputs.insert(pj_uri.address.to_string(), Amount::from_btc(50.0)?); + let options = bitcoincore_rpc::json::WalletCreateFundedPsbtOptions { + lock_unspent: Some(true), + fee_rate: Some(Amount::from_sat(2000)), + subtract_fee_from_outputs: vec![0], + ..Default::default() + }; + let psbt = sender + .wallet_create_funded_psbt( + &[], // inputs + &outputs, + None, // locktime + Some(options), + Some(true), // check that the sender properly clears keypaths + )? + .psbt; + let psbt = sender.wallet_process_psbt(&psbt, None, None, None)?.psbt; + Ok(Psbt::from_str(&psbt)?) + } + fn extract_pj_tx( sender: &bitcoincore_rpc::Client, psbt: Psbt,