From 629b92431be1ef15371f6a104327cb86c131d2bf Mon Sep 17 00:00:00 2001 From: DanGould Date: Fri, 29 Jul 2022 12:37:51 +0800 Subject: [PATCH 1/5] Check proposal with type state machine --- bip78/src/receiver/mod.rs | 109 +++++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 13 deletions(-) diff --git a/bip78/src/receiver/mod.rs b/bip78/src/receiver/mod.rs index d929b8fc..9d338e93 100644 --- a/bip78/src/receiver/mod.rs +++ b/bip78/src/receiver/mod.rs @@ -1,10 +1,9 @@ -use bitcoin::util::psbt::PartiallySignedTransaction as Psbt; -use bitcoin::{Script, TxOut}; +use bitcoin::{util::psbt::PartiallySignedTransaction as Psbt, AddressType, Script, TxOut}; mod error; -pub use error::RequestError; use error::InternalRequestError; +pub use error::RequestError; pub trait Headers { fn get_header(&self, key: &str) -> Option<&str>; @@ -14,8 +13,24 @@ pub struct UncheckedProposal { psbt: Psbt, } +pub struct MaybeInputsOwned { + psbt: Psbt, +} + +pub struct MaybeMixedInputScripts { + psbt: Psbt, +} + +pub struct MaybeInputsSeen { + psbt: Psbt, +} + impl UncheckedProposal { - pub fn from_request(body: impl std::io::Read, query: &str, headers: impl Headers) -> Result { + pub fn from_request( + body: impl std::io::Read, + query: &str, + headers: impl Headers, + ) -> Result { use crate::bitcoin::consensus::Decodable; let content_type = headers.get_header("content-type").ok_or(InternalRequestError::MissingHeader("Content-Type"))?; @@ -42,20 +57,89 @@ impl UncheckedProposal { }) } + /// The Sender's Original PSBT pub fn get_transaction_to_check_broadcast(&self) -> bitcoin::Transaction { self.psbt.clone().extract_tx() } - pub fn assume_broadcastability_was_verified(self) -> UnlockedProposal { - UnlockedProposal { - psbt: self.psbt, - } + /// Call after checking that the Original PSBT can be broadcast. + /// + /// Receiver MUST check that the Original PSBT from the sender + /// can be broadcast, i.e. `testmempoolaccept` bitcoind rpc returns { "allowed": true,.. } + /// for `get_transaction_to_check_broadcast()` before calling this method. + /// + /// 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 assume_tested_and_scheduled_broadcast(self) -> MaybeInputsOwned { + MaybeInputsOwned { psbt: self.psbt } } - pub fn this_is_purely_interactive_wallet(self) -> UnlockedProposal { - UnlockedProposal { - psbt: self.psbt, - } + /// Call this method if the only way to initiate a PayJoin with this receiver + /// requires manual intervention, as in most consumer wallets. + /// + /// So-called "non-interactive" receivers, like payment processors, that allow arbitrary requests are otherwise vulnerable to probing attacks. + /// Those receivers call `get_transaction_to_check_broadcast()` and `attest_tested_and_scheduled_broadcast()` after making those checks downstream. + pub fn assume_interactive_receive_endpoint(self) -> MaybeInputsOwned { + MaybeInputsOwned { psbt: self.psbt } + } +} + +impl MaybeInputsOwned { + /// The receiver should not be able to sign for any of these Original PSBT inputs. + /// + /// Check that none of them are owned by the receiver downstream before proceeding. + pub fn iter_input_script_pubkeys(&self) -> Vec> { + todo!() // return impl '_ + Iterator> + } + + /// Check that the Original PSBT has no receiver-owned inputs. + /// Return original-psbt-rejected error or otherwise refuse to sign undesirable inputs. + /// + /// An attacker could try to spend receiver's own inputs. This check prevents that. + /// Call this after checking downstream. + pub fn assume_inputs_not_owned(self) -> MaybeMixedInputScripts { + MaybeMixedInputScripts { psbt: self.psbt } + } +} + +impl MaybeMixedInputScripts { + /// If there is only 1 input type, the receiver should be able to produce the same + /// type. + /// + /// Check downstream before proceeding. + pub fn iter_input_script_types(&self) -> Vec> { + todo!() // return Iterator> + } + + /// Verify the original transaction did not have mixed input types + /// Call this after checking downstream. + /// + /// Note: mixed spends do not necessarily indicate distinct wallet fingerprints. + /// This check is intended to prevent some types of wallet fingerprinting. + pub fn assume_no_mixed_input_scripts(self) -> MaybeInputsSeen { + MaybeInputsSeen { psbt: self.psbt } + } +} + +impl MaybeInputsSeen { + /// The receiver should not have sent to or received the Original PSBT's inputs before. + /// + /// Check that these are unknown, never before seen inputs before proceeding. + pub fn iter_input_outpoints(&self) -> impl '_ + Iterator { + self.psbt.global.unsigned_tx.input.iter().map(|input| &input.previous_output) + } + + /// Make sure that the original transaction inputs have never been seen before. + /// This prevents probing attacks. This prevents reentrant PayJoin, where a sender + /// proposes a PayJoin PSBT as a new Original PSBT for a new PayJoin. + /// + /// Call this after checking downstream. + pub fn assume_no_inputs_seen_before(self) -> UnlockedProposal { + UnlockedProposal { psbt: self.psbt } } } @@ -112,4 +196,3 @@ pub struct NewOutputOptions { set_as_fee_output: bool, subtract_fees_from_this: bool, } - From 541501df6f7b04c1c9a9b41cac7708fc64bff18a Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 7 Jul 2022 11:08:18 +0800 Subject: [PATCH 2/5] Test receiver checks --- bip78/src/receiver/mod.rs | 55 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/bip78/src/receiver/mod.rs b/bip78/src/receiver/mod.rs index 9d338e93..c1ccb165 100644 --- a/bip78/src/receiver/mod.rs +++ b/bip78/src/receiver/mod.rs @@ -196,3 +196,58 @@ pub struct NewOutputOptions { set_as_fee_output: bool, subtract_fees_from_this: bool, } + +#[cfg(test)] +mod test { + use super::*; + + struct MockHeaders { + length: String, + } + + impl MockHeaders { + #[cfg(test)] + fn new(length: u64) -> MockHeaders { + MockHeaders { length: length.to_string() } + } + } + + impl Headers for MockHeaders { + fn get_header(&self, key: &str) -> Option<&str> { + match key { + "content-length" => Some(&self.length), + "content-type" => Some("text/plain"), + _ => None, + } + } + } + + fn get_proposal_from_test_vector() -> Result { + + // OriginalPSBT Test Vector from BIP + // | InputScriptType | Orginal PSBT Fee rate | maxadditionalfeecontribution | additionalfeeoutputindex| + // |-----------------|-----------------------|------------------------------|-------------------------| + // | P2SH-P2WPKH | 2 sat/vbyte | 0.00000182 | 0 | + let original_psbt = "cHNidP8BAHMCAAAAAY8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////AtyVuAUAAAAAF6kUHehJ8GnSdBUOOv6ujXLrWmsJRDCHgIQeAAAAAAAXqRR3QJbbz0hnQ8IvQ0fptGn+votneofTAAAAAAEBIKgb1wUAAAAAF6kU3k4ekGHKWRNbA1rV5tR5kEVDVNCHAQcXFgAUx4pFclNVgo1WWAdN1SYNX8tphTABCGsCRzBEAiB8Q+A6dep+Rz92vhy26lT0AjZn4PRLi8Bf9qoB/CMk0wIgP/Rj2PWZ3gEjUkTlhDRNAQ0gXwTO7t9n+V14pZ6oljUBIQMVmsAaoNWHVMS02LfTSe0e388LNitPa1UQZyOihY+FFgABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUAAA="; + + let body = original_psbt.as_bytes(); + let headers = MockHeaders::new(body.len() as u64); + UncheckedProposal::from_request(body, "", headers) + } + + #[test] + fn can_get_proposal_from_request() { + let proposal = get_proposal_from_test_vector(); + assert!(proposal.is_ok(), "OriginalPSBT should be a valid request"); + } + + #[test] + fn unchecked_proposal_unlocks_after_checks() { + let proposal = get_proposal_from_test_vector().unwrap(); + let unlocked = proposal + .assume_tested_and_scheduled_broadcast() + .assume_inputs_not_owned() + .assume_no_mixed_input_scripts() + .assume_no_inputs_seen_before(); + } +} From a5f5b26bfab3792b30f56315f87b0655f36688d4 Mon Sep 17 00:00:00 2001 From: DanGould Date: Mon, 1 Aug 2022 15:06:34 +0800 Subject: [PATCH 3/5] Move sender::Params to Configuration --- bip78/src/lib.rs | 2 ++ bip78/src/sender/mod.rs | 12 ++++++------ bip78/src/uri.rs | 4 ++-- bip78/tests/integration.rs | 2 +- payjoin-client/src/main.rs | 2 +- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/bip78/src/lib.rs b/bip78/src/lib.rs index 118d2aae..19d5bb4f 100644 --- a/bip78/src/lib.rs +++ b/bip78/src/lib.rs @@ -32,6 +32,8 @@ pub(crate) mod weight; #[cfg(any(feature = "sender", feature = "receiver"))] pub(crate) mod fee_rate; #[cfg(any(feature = "sender", feature = "receiver"))] +pub(crate) mod params; +#[cfg(any(feature = "sender", feature = "receiver"))] pub(crate) mod psbt; pub use uri::{Uri, PjUri, UriExt, PjUriExt, PjParseError}; diff --git a/bip78/src/sender/mod.rs b/bip78/src/sender/mod.rs index 0806cbac..6beb1286 100644 --- a/bip78/src/sender/mod.rs +++ b/bip78/src/sender/mod.rs @@ -36,14 +36,14 @@ type InternalResult = Result; /// Builder for sender-side payjoin parameters /// /// These parameters define how client wants to handle PayJoin. -pub struct Params { +pub struct Configuration { disable_output_substitution: bool, fee_contribution: Option<(bitcoin::Amount, Option)>, clamp_fee_contribution: bool, min_fee_rate: FeeRate, } -impl Params { +impl Configuration { /// Offer the receiver contribution to pay for his input. /// /// These parameters will allow the receiver to take `max_fee_contribution` from given change @@ -52,7 +52,7 @@ impl Params { /// `change_index` specifies which output can be used to pay fee. I `None` is provided, then /// the output is auto-detected unless the supplied transaction has more than two outputs. pub fn with_fee_contribution(max_fee_contribution: bitcoin::Amount, change_index: Option) -> Self { - Params { + Configuration { disable_output_substitution: false, fee_contribution: Some((max_fee_contribution, change_index)), clamp_fee_contribution: false, @@ -65,7 +65,7 @@ impl Params { /// While it's generally better to offer some contribution some users may wish not to. /// This function disables contribution. pub fn non_incentivizing() -> Self { - Params { + Configuration { disable_output_substitution: false, fee_contribution: None, clamp_fee_contribution: false, @@ -432,7 +432,7 @@ fn check_change_index(psbt: &Psbt, payee: &Script, fee: bitcoin::Amount, index: Ok((check_fee_output_amount(output, fee, clamp_fee_contribution)?, index)) } -fn determine_fee_contribution(psbt: &Psbt, payee: &Script, params: &Params) -> Result, InternalCreateRequestError> { +fn determine_fee_contribution(psbt: &Psbt, payee: &Script, params: &Configuration) -> Result, InternalCreateRequestError> { Ok(match params.fee_contribution { Some((fee, None)) => find_change_index(psbt, payee, fee, params.clamp_fee_contribution)?, Some((fee, Some(index))) => Some(check_change_index(psbt, payee, fee, index, params.clamp_fee_contribution)?), @@ -475,7 +475,7 @@ fn serialize_psbt(psbt: &Psbt) -> Vec { .expect("Vec doesn't return errors in its write implementation") } -pub(crate) fn from_psbt_and_uri(mut psbt: Psbt, uri: crate::uri::PjUri<'_>, params: Params) -> Result<(Request, Context), CreateRequestError> { +pub(crate) fn from_psbt_and_uri(mut psbt: Psbt, uri: crate::uri::PjUri<'_>, params: Configuration) -> Result<(Request, Context), CreateRequestError> { psbt .validate_input_utxos(true) .map_err(InternalCreateRequestError::InvalidOriginalInput)?; diff --git a/bip78/src/uri.rs b/bip78/src/uri.rs index 49495a97..4b798515 100644 --- a/bip78/src/uri.rs +++ b/bip78/src/uri.rs @@ -40,7 +40,7 @@ pub trait PjUriExt: sealed::UriExt { fn create_pj_request( self, psbt: bitcoin::util::psbt::PartiallySignedTransaction, - params: sender::Params, + params: sender::Configuration, ) -> Result<(sender::Request, sender::Context), sender::CreateRequestError>; } @@ -53,7 +53,7 @@ impl<'a> PjUriExt for PjUri<'a> { fn create_pj_request( self, psbt: bitcoin::util::psbt::PartiallySignedTransaction, - params: sender::Params, + params: sender::Configuration, ) -> Result<(sender::Request, sender::Context), sender::CreateRequestError> { sender::from_psbt_and_uri(psbt.try_into().map_err(sender::InternalCreateRequestError::InconsistentOriginalPsbt)?, self, params) } diff --git a/bip78/tests/integration.rs b/bip78/tests/integration.rs index fd827db4..0ff0ef40 100644 --- a/bip78/tests/integration.rs +++ b/bip78/tests/integration.rs @@ -68,7 +68,7 @@ mod integration { .psbt; let psbt = load_psbt_from_base64(psbt.as_bytes()).unwrap(); debug!("Original psbt: {:#?}", psbt); - let pj_params = bip78::sender::Params::with_fee_contribution(bip78::bitcoin::Amount::from_sat(10000), None); + let pj_params = bip78::sender::Configuration::with_fee_contribution(bip78::bitcoin::Amount::from_sat(10000), None); let (req, ctx) = pj_uri.create_request(psbt, pj_params).unwrap(); let headers = HeaderMock::from_vec(&req.body); diff --git a/payjoin-client/src/main.rs b/payjoin-client/src/main.rs index e43f83a5..bac6aecd 100644 --- a/payjoin-client/src/main.rs +++ b/payjoin-client/src/main.rs @@ -57,7 +57,7 @@ fn main() { .psbt; let psbt = load_psbt_from_base64(psbt.as_bytes()).unwrap(); println!("Original psbt: {:#?}", psbt); - let pj_params = bip78::sender::Params::with_fee_contribution(bip78::bitcoin::Amount::from_sat(10000), None); + let pj_params = bip78::sender::Configuration::with_fee_contribution(bip78::bitcoin::Amount::from_sat(10000), None); let (req, ctx) = link.create_pj_request(psbt, pj_params).unwrap(); let response = reqwest::blocking::Client::new() .post(req.url) From 98fb17feee2e687b9605d69818d2e3dcc29d8ff8 Mon Sep 17 00:00:00 2001 From: DanGould Date: Mon, 1 Aug 2022 16:05:50 +0800 Subject: [PATCH 4/5] Parse optional parameters Params from query --- bip78/src/lib.rs | 2 +- bip78/src/params.rs | 94 +++++++++++++++++++++++++++++++++++++ bip78/src/receiver/error.rs | 1 + bip78/src/receiver/mock.rs | 18 +++++++ bip78/src/receiver/mod.rs | 36 +++++++++++--- bip78/tests/integration.rs | 2 +- 6 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 bip78/src/params.rs create mode 100644 bip78/src/receiver/mock.rs diff --git a/bip78/src/lib.rs b/bip78/src/lib.rs index 19d5bb4f..82798b34 100644 --- a/bip78/src/lib.rs +++ b/bip78/src/lib.rs @@ -31,7 +31,7 @@ mod uri; pub(crate) mod weight; #[cfg(any(feature = "sender", feature = "receiver"))] pub(crate) mod fee_rate; -#[cfg(any(feature = "sender", feature = "receiver"))] +#[cfg(any(feature = "receiver"))] pub(crate) mod params; #[cfg(any(feature = "sender", feature = "receiver"))] pub(crate) mod psbt; diff --git a/bip78/src/params.rs b/bip78/src/params.rs new file mode 100644 index 00000000..2a30de69 --- /dev/null +++ b/bip78/src/params.rs @@ -0,0 +1,94 @@ +use std::borrow::Borrow; +use std::fmt; + +use crate::fee_rate::FeeRate; + +#[derive(Debug)] +pub(crate) struct Params { + // version + // v: usize, + // disableoutputsubstitution + pub disable_output_substitution: bool, + // maxadditionalfeecontribution, additionalfeeoutputindex + pub additional_fee_contribution: Option<(bitcoin::Amount, usize)>, + // minfeerate + pub min_feerate: FeeRate, +} + +impl Default for Params { + fn default() -> Self { + Params { + disable_output_substitution: false, + additional_fee_contribution: None, + min_feerate: FeeRate::ZERO, + } + } +} + +impl Params { + #[cfg(feature = "receiver")] + pub fn from_query_pairs(pairs: I) -> Result + where + I: Iterator, + K: Borrow + Into, + V: Borrow + Into, + { + let mut params = Params::default(); + + let mut additional_fee_output_index = None; + let mut max_additional_fee_contribution = None; + + for (k, v) in pairs { + match (k.borrow(), v.borrow()) { + ("v", v) => if v != "1" { + return Err(ParamsError::UnknownVersion) + }, + ("additionalfeeoutputindex", index) => { + if let Ok(index) = index.parse::() { + additional_fee_output_index = Some(index); + } + }, + ("maxadditionalfeecontribution", fee) => { + max_additional_fee_contribution = bitcoin::Amount::from_str_in(&fee, bitcoin::Denomination::Bitcoin).ok(); + } + ("minfeerate", feerate) => { + params.min_feerate = match feerate.parse::() { + Ok(rate) => FeeRate::from_sat_per_vb(rate), + Err(e) => return Err(ParamsError::FeeRate(e)), + } + } + ("disableoutputsubstitution", v) => params.disable_output_substitution = v == "true", // existance is truthy + _ => (), + } + } + if let (Some(amount), Some(index)) = (max_additional_fee_contribution, additional_fee_output_index) { + params.additional_fee_contribution = Some((amount, index)); + } + + Ok(params) + } +} + +#[derive(Debug)] +pub(crate) enum ParamsError { + UnknownVersion, + FeeRate(std::num::ParseIntError), +} + +impl fmt::Display for ParamsError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ParamsError::UnknownVersion => write!(f, "unknown version"), + ParamsError::FeeRate(_) => write!(f, "could not parse feerate"), + } + } +} + +impl std::error::Error for ParamsError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ParamsError::FeeRate(error) => Some(error), + _ => None, + } + } +} diff --git a/bip78/src/receiver/error.rs b/bip78/src/receiver/error.rs index 36109ddb..56783045 100644 --- a/bip78/src/receiver/error.rs +++ b/bip78/src/receiver/error.rs @@ -8,6 +8,7 @@ pub(crate) enum InternalRequestError { InvalidContentType(String), InvalidContentLength(std::num::ParseIntError), ContentLengthTooLarge(u64), + SenderParams(crate::params::ParamsError) } impl From for RequestError { diff --git a/bip78/src/receiver/mock.rs b/bip78/src/receiver/mock.rs new file mode 100644 index 00000000..7d5e3989 --- /dev/null +++ b/bip78/src/receiver/mock.rs @@ -0,0 +1,18 @@ +use std::collections::HashMap; + +pub struct MockHeaders(HashMap); + +impl crate::receiver::Headers for MockHeaders { + fn get_header(&self, key: &str) -> Option<&str> { + self.0.get(key).map(|e| e.as_str()) + } +} + +impl MockHeaders { + pub fn from_slice(body: &[u8]) -> MockHeaders { + let mut h = HashMap::new(); + h.insert("content-type".to_string(), "text/plain".to_string()); + h.insert("content-length".to_string(), body.len().to_string()); + MockHeaders(h) + } +} \ No newline at end of file diff --git a/bip78/src/receiver/mod.rs b/bip78/src/receiver/mod.rs index c1ccb165..b199c78b 100644 --- a/bip78/src/receiver/mod.rs +++ b/bip78/src/receiver/mod.rs @@ -1,9 +1,11 @@ use bitcoin::{util::psbt::PartiallySignedTransaction as Psbt, AddressType, Script, TxOut}; +use crate::params::Params; mod error; -use error::InternalRequestError; pub use error::RequestError; +use error::InternalRequestError; + pub trait Headers { fn get_header(&self, key: &str) -> Option<&str>; @@ -11,18 +13,22 @@ pub trait Headers { pub struct UncheckedProposal { psbt: Psbt, + params: Params, } pub struct MaybeInputsOwned { psbt: Psbt, + params: Params, } pub struct MaybeMixedInputScripts { psbt: Psbt, + params: Params, } pub struct MaybeInputsSeen { psbt: Psbt, + params: Params, } impl UncheckedProposal { @@ -52,8 +58,14 @@ impl UncheckedProposal { let reader = base64::read::DecoderReader::new(&mut limited, base64::STANDARD); let psbt = Psbt::consensus_decode(reader).map_err(InternalRequestError::Decode)?; + let pairs = url::form_urlencoded::parse(query.as_bytes()); + let params = Params::from_query_pairs(pairs).map_err(InternalRequestError::SenderParams)?; + + // TODO check params are valid for request Origianl PSBT + Ok(UncheckedProposal { psbt, + params, }) } @@ -75,7 +87,10 @@ impl UncheckedProposal { /// /// Call this after checking downstream. pub fn assume_tested_and_scheduled_broadcast(self) -> MaybeInputsOwned { - MaybeInputsOwned { psbt: self.psbt } + MaybeInputsOwned { + psbt: self.psbt, + params: self.params, + } } /// Call this method if the only way to initiate a PayJoin with this receiver @@ -84,7 +99,10 @@ impl UncheckedProposal { /// So-called "non-interactive" receivers, like payment processors, that allow arbitrary requests are otherwise vulnerable to probing attacks. /// Those receivers call `get_transaction_to_check_broadcast()` and `attest_tested_and_scheduled_broadcast()` after making those checks downstream. pub fn assume_interactive_receive_endpoint(self) -> MaybeInputsOwned { - MaybeInputsOwned { psbt: self.psbt } + MaybeInputsOwned { + psbt: self.psbt, + params: self.params, + } } } @@ -102,7 +120,10 @@ impl MaybeInputsOwned { /// An attacker could try to spend receiver's own inputs. This check prevents that. /// Call this after checking downstream. pub fn assume_inputs_not_owned(self) -> MaybeMixedInputScripts { - MaybeMixedInputScripts { psbt: self.psbt } + MaybeMixedInputScripts { + psbt: self.psbt, + params: self.params, + } } } @@ -121,7 +142,10 @@ impl MaybeMixedInputScripts { /// Note: mixed spends do not necessarily indicate distinct wallet fingerprints. /// This check is intended to prevent some types of wallet fingerprinting. pub fn assume_no_mixed_input_scripts(self) -> MaybeInputsSeen { - MaybeInputsSeen { psbt: self.psbt } + MaybeInputsSeen { + psbt: self.psbt, + params: self.params, + } } } @@ -232,7 +256,7 @@ mod test { let body = original_psbt.as_bytes(); let headers = MockHeaders::new(body.len() as u64); - UncheckedProposal::from_request(body, "", headers) + UncheckedProposal::from_request(body,"?maxadditionalfeecontribution=0.00000182?additionalfeeoutputindex=0", headers) } #[test] diff --git a/bip78/tests/integration.rs b/bip78/tests/integration.rs index 0ff0ef40..a19764a2 100644 --- a/bip78/tests/integration.rs +++ b/bip78/tests/integration.rs @@ -73,7 +73,7 @@ mod integration { let headers = HeaderMock::from_vec(&req.body); // Receiver receive payjoin proposal, IRL it will be an HTTP request (over ssl or onion) - let proposal = bip78::receiver::UncheckedProposal::from_request(req.body.as_slice(), "", headers).unwrap(); + let proposal = bip78::receiver::UncheckedProposal::from_request(req.body.as_slice(), req.url.query().unwrap_or(""), headers).unwrap(); // TODO } From 850f0d9e3dea1b52336cb60a1f066505ebb0e280 Mon Sep 17 00:00:00 2001 From: DanGould Date: Tue, 16 Aug 2022 12:32:34 +0800 Subject: [PATCH 5/5] Log warnings in release --- bip78/Cargo.toml | 2 +- bip78/src/lib.rs | 12 ++++++++++++ bip78/src/params.rs | 26 ++++++++++++++++++++------ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/bip78/Cargo.toml b/bip78/Cargo.toml index 901fd8a2..ef9644ab 100644 --- a/bip78/Cargo.toml +++ b/bip78/Cargo.toml @@ -23,8 +23,8 @@ base64 = "0.13.0" rand = { version = "0.8.4", optional = true } bip21 = "0.1.1" url = "2.2.2" +log = { version = "0.4.14", optional = true } [dev-dependencies] bitcoind = { version = "0.18.0", features = ["0_21_1"] } env_logger = "0.9.0" -log = "0.4.14" diff --git a/bip78/src/lib.rs b/bip78/src/lib.rs index 82798b34..11cf6752 100644 --- a/bip78/src/lib.rs +++ b/bip78/src/lib.rs @@ -37,3 +37,15 @@ pub(crate) mod params; pub(crate) mod psbt; pub use uri::{Uri, PjUri, UriExt, PjUriExt, PjParseError}; + +#[cfg(feature = "log")] +macro_rules! log_warn { + ($($args:tt)*) => { log::warn!($($args)*) } +} + +#[cfg(not(feature = "log"))] +macro_rules! log_warn { + ($($args:tt)*) => {} +} +pub(crate) use log_warn; + diff --git a/bip78/src/params.rs b/bip78/src/params.rs index 2a30de69..01eb8baf 100644 --- a/bip78/src/params.rs +++ b/bip78/src/params.rs @@ -1,6 +1,7 @@ use std::borrow::Borrow; use std::fmt; +use crate::log_warn; use crate::fee_rate::FeeRate; #[derive(Debug)] @@ -44,12 +45,22 @@ impl Params { return Err(ParamsError::UnknownVersion) }, ("additionalfeeoutputindex", index) => { - if let Ok(index) = index.parse::() { - additional_fee_output_index = Some(index); + additional_fee_output_index = match index.parse::() { + Ok(index) => Some(index), + Err(_error) => { + log_warn!("bad `additionalfeeoutputindex` query value '{}': {}", index, _error); + None + } } }, ("maxadditionalfeecontribution", fee) => { - max_additional_fee_contribution = bitcoin::Amount::from_str_in(&fee, bitcoin::Denomination::Bitcoin).ok(); + max_additional_fee_contribution = match bitcoin::Amount::from_str_in(&fee, bitcoin::Denomination::Bitcoin) { + Ok(contribution) => Some(contribution), + Err(_error) => { + log_warn!("bad `maxadditionalfeecontribution` query value '{}': {}", fee, _error); + None + } + } } ("minfeerate", feerate) => { params.min_feerate = match feerate.parse::() { @@ -57,12 +68,15 @@ impl Params { Err(e) => return Err(ParamsError::FeeRate(e)), } } - ("disableoutputsubstitution", v) => params.disable_output_substitution = v == "true", // existance is truthy + ("disableoutputsubstitution", v) => params.disable_output_substitution = v == "true", _ => (), } } - if let (Some(amount), Some(index)) = (max_additional_fee_contribution, additional_fee_output_index) { - params.additional_fee_contribution = Some((amount, index)); + + match (max_additional_fee_contribution, additional_fee_output_index) { + (Some(amount), Some(index)) => params.additional_fee_contribution = Some((amount, index)), + (Some(_), None) | (None, Some(_)) => { log_warn!("only one additional-fee parameter specified: {:?}", params); }, + _ => () } Ok(params)