From 209c1535d914c87e35d32dd0a23098e85fed8a07 Mon Sep 17 00:00:00 2001 From: jbesraa Date: Sun, 20 Aug 2023 18:23:50 +0300 Subject: [PATCH] Create ProvisionalPayjoin Struct Motivaiton is to create a struct that is used until we finalize the proposal and return PayjoinProposal so we dont leak data out of PayjoinProposal. Changed apply_fee and prepare_psbt to private functions and they are now invoked from finalize_proposal --- payjoin-cli/src/app.rs | 40 ++++++++++++++--------------- payjoin/src/receive/mod.rs | 49 +++++++++++++++++++++++++++++------- payjoin/tests/integration.rs | 41 +++++++++++++++++------------- 3 files changed, 84 insertions(+), 46 deletions(-) diff --git a/payjoin-cli/src/app.rs b/payjoin-cli/src/app.rs index 55691cb4..5a592088 100644 --- a/payjoin-cli/src/app.rs +++ b/payjoin-cli/src/app.rs @@ -10,7 +10,7 @@ use bitcoincore_rpc::RpcApi; use clap::ArgMatches; use config::{Config, File, FileFormat}; use payjoin::bitcoin::psbt::Psbt; -use payjoin::receive::{Error, PayjoinProposal}; +use payjoin::receive::{Error, ProvisionalProposal}; use payjoin::{bitcoin, PjUriExt, UriExt}; use rouille::{Request, Response}; use serde::{Deserialize, Serialize}; @@ -296,7 +296,7 @@ impl App { })?; log::trace!("check4"); - let mut payjoin = payjoin.identify_receiver_outputs(|output_script| { + let mut provisional_payjoin = payjoin.identify_receiver_outputs(|output_script| { if let Ok(address) = bitcoin::Address::from_script(output_script, network) { self.bitcoind .get_address_info(&address) @@ -309,7 +309,7 @@ impl App { if !self.config.sub_only { // Select receiver payjoin inputs. - _ = try_contributing_inputs(&mut payjoin, &self.bitcoind) + _ = try_contributing_inputs(&mut provisional_payjoin, &self.bitcoind) .map_err(|e| log::warn!("Failed to contribute inputs: {}", e)); } @@ -318,23 +318,23 @@ impl App { .get_new_address(None, None) .map_err(|e| Error::Server(e.into()))? .assume_checked(); - payjoin.substitute_output_address(receiver_substitute_address); + provisional_payjoin.substitute_output_address(receiver_substitute_address); - let payjoin_proposal_psbt = payjoin.apply_fee(Some(1))?; - - log::debug!("Extracted PSBT: {:#?}", payjoin_proposal_psbt); - // Sign payjoin psbt - let payjoin_base64_string = base64::encode(&payjoin_proposal_psbt.serialize()); - // `wallet_process_psbt` adds available utxo data and finalizes - let payjoin_proposal_psbt = self - .bitcoind - .wallet_process_psbt(&payjoin_base64_string, None, None, Some(false)) - .map_err(|e| Error::Server(e.into()))? - .psbt; - let payjoin_proposal_psbt = Psbt::from_str(&payjoin_proposal_psbt) - .context("Failed to parse PSBT") - .map_err(|e| Error::Server(e.into()))?; - let payjoin_proposal_psbt = payjoin.prepare_psbt(payjoin_proposal_psbt)?; + let payjoi_proposal = provisional_payjoin.finalize_proposal( + |psbt: &Psbt| { + self.bitcoind + .wallet_process_psbt( + &bitcoin::base64::encode(psbt.serialize()), + None, + None, + Some(false), + ) + .map(|res| Psbt::from_str(&res.psbt).map_err(|e| Error::Server(e.into()))) + .map_err(|e| Error::Server(e.into()))? + }, + Some(bitcoin::FeeRate::MIN), + )?; + let payjoin_proposal_psbt = payjoi_proposal.psbt(); log::debug!("Receiver's Payjoin proposal PSBT Rsponse: {:#?}", payjoin_proposal_psbt); let payload = base64::encode(&payjoin_proposal_psbt.serialize()); @@ -449,7 +449,7 @@ impl AppConfig { } fn try_contributing_inputs( - payjoin: &mut PayjoinProposal, + payjoin: &mut ProvisionalProposal, bitcoind: &bitcoincore_rpc::Client, ) -> Result<()> { use bitcoin::OutPoint; diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index 0f8a0748..b9c9a0ea 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -268,6 +268,7 @@ use std::cmp::{max, min}; use std::collections::{BTreeMap, HashMap}; +use std::str::FromStr; use bitcoin::psbt::Psbt; use bitcoin::{Amount, FeeRate, OutPoint, Script, TxOut}; @@ -522,7 +523,7 @@ impl OutputsUnknown { pub fn identify_receiver_outputs( self, is_receiver_output: impl Fn(&Script) -> Result, - ) -> Result { + ) -> Result { let owned_vouts: Vec = self .psbt .unsigned_tx @@ -540,7 +541,7 @@ impl OutputsUnknown { return Err(Error::BadRequest(InternalRequestError::MissingPayment.into())); } - Ok(PayjoinProposal { + Ok(ProvisionalProposal { original_psbt: self.psbt.clone(), payjoin_psbt: self.psbt, params: self.params, @@ -551,7 +552,6 @@ impl OutputsUnknown { /// A mutable checked proposal that the receiver may contribute inputs to to make a payjoin. pub struct PayjoinProposal { - original_psbt: Psbt, payjoin_psbt: Psbt, params: Params, owned_vouts: Vec, @@ -566,6 +566,20 @@ impl PayjoinProposal { self.params.disable_output_substitution } + pub fn get_owned_vouts(&self) -> &Vec { &self.owned_vouts } + + pub fn psbt(&self) -> &Psbt { &self.payjoin_psbt } +} + +/// A mutable checked proposal that the receiver may contribute inputs to to make a payjoin. +pub struct ProvisionalProposal { + original_psbt: Psbt, + payjoin_psbt: Psbt, + params: Params, + owned_vouts: Vec, +} + +impl ProvisionalProposal { /// Select receiver input such that the payjoin avoids surveillance. /// Return the input chosen that has been applied to the Proposal. /// @@ -707,12 +721,11 @@ impl PayjoinProposal { /// this is kind of a "build_proposal" step before we sign and finalize and extract /// /// WARNING: DO NOT ALTER INPUTS OR OUTPUTS AFTER THIS STEP - pub fn apply_fee( + fn apply_fee( &mut self, - min_feerate_sat_per_vb: Option, + min_feerate_sat_per_vb: Option, ) -> Result<&Psbt, RequestError> { - let min_feerate = - FeeRate::from_sat_per_vb_unchecked(min_feerate_sat_per_vb.unwrap_or_default()); + let min_feerate = min_feerate_sat_per_vb.unwrap_or(FeeRate::MIN); log::trace!("min_feerate: {:?}", min_feerate); log::trace!("params.min_feerate: {:?}", self.params.min_feerate); let min_feerate = max(min_feerate, self.params.min_feerate); @@ -760,7 +773,7 @@ impl PayjoinProposal { /// and return a PSBT that can produce a consensus-valid transaction that the sender will accept. /// /// wallet_process_psbt should sign and finalize receiver inputs - pub fn prepare_psbt(mut self, processed_psbt: Psbt) -> Result { + fn prepare_psbt(mut self, processed_psbt: Psbt) -> Result { self.payjoin_psbt = processed_psbt; log::trace!("Preparing PSBT {:#?}", self.payjoin_psbt); for input in self.payjoin_psbt.inputs_mut() { @@ -791,7 +804,25 @@ impl PayjoinProposal { self.payjoin_psbt.inputs[i].final_script_sig = None; self.payjoin_psbt.inputs[i].final_script_witness = None; } - Ok(self.payjoin_psbt) + Ok(PayjoinProposal { + payjoin_psbt: self.payjoin_psbt, + owned_vouts: self.owned_vouts, + params: self.params, + }) + } + + pub fn finalize_proposal( + mut self, + wallet_process_psbt: impl Fn(&Psbt) -> Result, + min_feerate_sat_per_vb: Option, + ) -> Result { + let psbt = self.apply_fee(min_feerate_sat_per_vb)?; + let psbt = wallet_process_psbt(psbt).map_err(|e| { + log::error!("wallet_process_psbt error"); + Error::from(e) + })?; + let payjoin_proposal = self.prepare_psbt(psbt).map_err(RequestError::from)?; + Ok(payjoin_proposal) } } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 349bd619..a1a5cfcd 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -6,13 +6,13 @@ mod integration { use bitcoin::psbt::Psbt; use bitcoin::{Amount, OutPoint}; use bitcoind::bitcoincore_rpc; - use bitcoind::bitcoincore_rpc::core_rpc_json::AddressType; + use bitcoind::bitcoincore_rpc::core_rpc_json::{AddressType, WalletProcessPsbtResult}; use bitcoind::bitcoincore_rpc::RpcApi; use log::{debug, log_enabled, Level}; use payjoin::bitcoin::base64; use payjoin::receive::Headers; use payjoin::send::Request; - use payjoin::{bitcoin, PjUriExt, Uri, UriExt}; + use payjoin::{bitcoin, Error, PjUriExt, Uri, UriExt}; #[test] fn integration_test() { @@ -200,20 +200,27 @@ mod integration { let receiver_substitute_address = receiver.get_new_address(None, None).unwrap().assume_checked(); payjoin.substitute_output_address(receiver_substitute_address); - - let payjoin_proposal_psbt = payjoin.apply_fee(None).unwrap(); - - // Sign payjoin psbt - let payjoin_base64_string = base64::encode(&payjoin_proposal_psbt.serialize()); - let payjoin_proposal_psbt = receiver - .wallet_process_psbt(&payjoin_base64_string, None, None, Some(false)) - .unwrap() - .psbt; - let payjoin_proposal_psbt = Psbt::from_str(&payjoin_proposal_psbt).unwrap(); - - let payjoin_proposal_psbt = payjoin.prepare_psbt(payjoin_proposal_psbt).unwrap(); - debug!("Receiver's Payjoin proposal PSBT: {:#?}", payjoin_proposal_psbt); - - base64::encode(&payjoin_proposal_psbt.serialize()) + let payjoin_proposal = payjoin + .finalize_proposal( + |psbt: &Psbt| { + Ok(receiver + .wallet_process_psbt( + &bitcoin::base64::encode(psbt.serialize()), + None, + None, + Some(false), + ) + .map(|res: WalletProcessPsbtResult| { + let psbt = Psbt::from_str(&res.psbt).unwrap(); + return psbt; + }) + .unwrap()) + }, + Some(bitcoin::FeeRate::MIN), + ) + .unwrap(); + let psbt = payjoin_proposal.psbt(); + debug!("Receiver's Payjoin proposal PSBT: {:#?}", &psbt); + base64::encode(&psbt.serialize()) } }