From e187ebf4b92cdee841ad6cf77ece5ecef2316c07 Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 09:54:25 -0500 Subject: [PATCH 1/8] refactor: process_response &mut impl std::io::Read -> &str --- payjoin-cli/src/app/v1.rs | 12 ++++++------ payjoin/src/send/v1.rs | 19 +++++++------------ payjoin/tests/integration.rs | 6 +++--- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 075fa73dd..2cbf07933 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -89,6 +89,8 @@ impl AppTrait for App { .send() .await .with_context(|| "HTTP request failed")?; + let response_str = response.text().await.with_context(|| "Failed to read response")?; + let fallback_tx = Psbt::from_str(&body) .map_err(|e| anyhow!("Failed to load PSBT from base64: {}", e))? .extract_tx()?; @@ -97,12 +99,10 @@ impl AppTrait for App { "Sent fallback transaction hex: {:#}", payjoin::bitcoin::consensus::encode::serialize_hex(&fallback_tx) ); - let psbt = ctx.process_response(&mut response.bytes().await?.to_vec().as_slice()).map_err( - |e| { - log::debug!("Error processing response: {:?}", e); - anyhow!("Failed to process response {}", e) - }, - )?; + let psbt = ctx.process_response(&response_str).map_err(|e| { + log::debug!("Error processing response: {:?}", e); + anyhow!("Failed to process response {}", e) + })?; self.process_pj_response(psbt)?; Ok(()) diff --git a/payjoin/src/send/v1.rs b/payjoin/src/send/v1.rs index 5febac1f9..7c3802245 100644 --- a/payjoin/src/send/v1.rs +++ b/payjoin/src/send/v1.rs @@ -183,7 +183,7 @@ impl<'a> SenderBuilder<'a> { fn build(self) -> Result { let mut psbt = self.psbt.validate().map_err(InternalBuildSenderError::InconsistentOriginalPsbt)?; - psbt.validate_input_utxos(true).map_err(InternalBuildSenderError::InvalidOriginalInput)?; + psbt.validate_input_utxos().map_err(InternalBuildSenderError::InvalidOriginalInput)?; let endpoint = self.uri.extras.endpoint.clone(); let disable_output_substitution = self.uri.extras.disable_output_substitution || self.disable_output_substitution; @@ -229,7 +229,7 @@ impl Sender { /// Extract serialized V1 Request and Context from a Payjoin Proposal pub fn extract_v1(&self) -> Result<(Request, V1Context), url::ParseError> { let url = serialize_url( - self.endpoint.clone(), + &self.endpoint, self.disable_output_substitution, self.fee_contribution, self.min_fee_rate, @@ -237,7 +237,7 @@ impl Sender { )?; let body = self.psbt.to_string().as_bytes().to_vec(); Ok(( - Request::new_v1(url, body), + Request::new_v1(&url, &body), V1Context { psbt_context: PsbtContext { original_psbt: self.psbt.clone(), @@ -269,13 +269,8 @@ impl V1Context { /// Call this method with response from receiver to continue BIP78 flow. If the response is /// valid you will get appropriate PSBT that you should sign and broadcast. #[inline] - pub fn process_response( - self, - response: &mut impl std::io::Read, - ) -> Result { - let mut res_str = String::new(); - response.read_to_string(&mut res_str).map_err(InternalValidationError::Io)?; - let proposal = Psbt::from_str(&res_str).map_err(|_| ResponseError::parse(&res_str))?; + pub fn process_response(self, response: &str) -> Result { + let proposal = Psbt::from_str(response).map_err(|_| ResponseError::parse(response))?; self.psbt_context.process_proposal(proposal).map_err(Into::into) } } @@ -296,7 +291,7 @@ mod test { "message": "This version of payjoin is not supported." }) .to_string(); - match ctx.process_response(&mut known_json_error.as_bytes()) { + match ctx.process_response(&known_json_error) { Err(ResponseError::WellKnown(WellKnownError::VersionUnsupported { .. })) => (), _ => panic!("Expected WellKnownError"), } @@ -307,7 +302,7 @@ mod test { "message": "This version of payjoin is not supported." }) .to_string(); - match ctx.process_response(&mut invalid_json_error.as_bytes()) { + match ctx.process_response(&invalid_json_error) { Err(ResponseError::Validation(_)) => (), _ => panic!("Expected unrecognized JSON error"), } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 37cb6e06f..5c0e6a553 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -105,7 +105,7 @@ mod integration { // ********************** // Inside the Sender: // Sender checks, signs, finalizes, extracts, and broadcasts - let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?; + let checked_payjoin_proposal_psbt = ctx.process_response(&response)?; let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?; sender.send_raw_transaction(&payjoin_tx)?; @@ -1045,7 +1045,7 @@ mod integration { // ********************** // Inside the Sender: // Sender checks, signs, finalizes, extracts, and broadcasts - let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?; + let checked_payjoin_proposal_psbt = ctx.process_response(&response)?; let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?; sender.send_raw_transaction(&payjoin_tx)?; @@ -1130,7 +1130,7 @@ mod integration { // ********************** // Inside the Sender: // Sender checks, signs, finalizes, extracts, and broadcasts - let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?; + let checked_payjoin_proposal_psbt = ctx.process_response(&response)?; let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?; sender.send_raw_transaction(&payjoin_tx)?; From 6d02133b7c7809b80915140a8c2a8d1e97ce275a Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 09:56:49 -0500 Subject: [PATCH 2/8] refactor: hpke use refs where applicable hybrid keys should be refs here --- payjoin/src/hpke.rs | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/payjoin/src/hpke.rs b/payjoin/src/hpke.rs index 3cf360d02..114f0f378 100644 --- a/payjoin/src/hpke.rs +++ b/payjoin/src/hpke.rs @@ -1,8 +1,9 @@ use std::ops::Deref; use std::{error, fmt}; -use bitcoin::key::constants::{ELLSWIFT_ENCODING_SIZE, PUBLIC_KEY_SIZE}; -use bitcoin::secp256k1; +use bitcoin::key::constants::{ + ELLSWIFT_ENCODING_SIZE, PUBLIC_KEY_SIZE, UNCOMPRESSED_PUBLIC_KEY_SIZE, +}; use bitcoin::secp256k1::ellswift::ElligatorSwift; use hpke::aead::ChaCha20Poly1305; use hpke::kdf::HkdfSha256; @@ -120,8 +121,8 @@ impl<'de> serde::Deserialize<'de> for HpkeSecretKey { pub struct HpkePublicKey(pub PublicKey); impl HpkePublicKey { - pub fn to_compressed_bytes(&self) -> [u8; 33] { - let compressed_key = secp256k1::PublicKey::from_slice(&self.0.to_bytes()) + pub fn to_compressed_bytes(&self) -> [u8; PUBLIC_KEY_SIZE] { + let compressed_key = bitcoin::secp256k1::PublicKey::from_slice(&self.0.to_bytes()) .expect("Invalid public key from known valid bytes"); compressed_key.serialize() } @@ -170,7 +171,7 @@ impl<'de> serde::Deserialize<'de> for HpkePublicKey { /// Message A is sent from the sender to the receiver containing an Original PSBT payload pub fn encrypt_message_a( - body: Vec, + body: &[u8], reply_pk: &HpkePublicKey, receiver_pk: &HpkePublicKey, ) -> Result, HpkeError> { @@ -193,7 +194,7 @@ pub fn encrypt_message_a( pub fn decrypt_message_a( message_a: &[u8], - receiver_sk: HpkeSecretKey, + receiver_sk: &HpkeSecretKey, ) -> Result<(Vec, HpkePublicKey), HpkeError> { use std::io::{Cursor, Read}; @@ -222,7 +223,7 @@ pub fn decrypt_message_a( /// Message B is sent from the receiver to the sender containing a Payjoin PSBT payload or an error pub fn encrypt_message_b( - mut plaintext: Vec, + mut plaintext: &[u8], receiver_keypair: &HpkeKeyPair, sender_pk: &HpkePublicKey, ) -> Result, HpkeError> { @@ -245,8 +246,8 @@ pub fn encrypt_message_b( pub fn decrypt_message_b( message_b: &[u8], - receiver_pk: HpkePublicKey, - sender_sk: HpkeSecretKey, + receiver_pk: &HpkePublicKey, + sender_sk: &HpkeSecretKey, ) -> Result, HpkeError> { let enc = message_b.get(..ELLSWIFT_ENCODING_SIZE).ok_or(HpkeError::PayloadTooShort)?; let enc = encapped_key_from_ellswift_bytes(enc)?; @@ -260,14 +261,6 @@ pub fn decrypt_message_b( Ok(plaintext) } -fn pad_plaintext(msg: &mut Vec, padded_length: usize) -> Result<&[u8], HpkeError> { - if msg.len() > padded_length { - return Err(HpkeError::PayloadTooLarge { actual: msg.len(), max: padded_length }); - } - msg.resize(padded_length, 0); - Ok(msg) -} - /// Error from de/encrypting a v2 Hybrid Public Key Encryption payload. #[derive(Debug, PartialEq)] pub enum HpkeError { From 6cb2cd4a8d0ee07062f2162123d2146b4396dd7d Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 09:58:17 -0500 Subject: [PATCH 3/8] feat: ohttp use crates for ohttp capsulation --- payjoin/src/ohttp.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/payjoin/src/ohttp.rs b/payjoin/src/ohttp.rs index f62ee6fed..84e645302 100644 --- a/payjoin/src/ohttp.rs +++ b/payjoin/src/ohttp.rs @@ -11,7 +11,7 @@ const OHTTP_REQ_HEADER_BYTES: usize = 7; pub const PADDED_BHTTP_REQ_BYTES: usize = ENCAPSULATED_MESSAGE_BYTES - (N_ENC + N_T + OHTTP_REQ_HEADER_BYTES); -pub fn ohttp_encapsulate( +pub(crate) fn ohttp_encapsulate( ohttp_keys: &mut ohttp::KeyConfig, method: &str, target_resource: &str, @@ -50,7 +50,7 @@ pub fn ohttp_encapsulate( } /// decapsulate ohttp, bhttp response and return http response body and status code -pub fn ohttp_decapsulate( +pub(crate) fn ohttp_decapsulate( res_ctx: ohttp::ClientResponse, ohttp_body: &[u8; ENCAPSULATED_MESSAGE_BYTES], ) -> Result>, OhttpEncapsulationError> { @@ -69,7 +69,7 @@ pub fn ohttp_decapsulate( /// Error from de/encapsulating an Oblivious HTTP request or response. #[derive(Debug)] -pub enum OhttpEncapsulationError { +pub(crate) enum OhttpEncapsulationError { Http(http::Error), Ohttp(ohttp::Error), Bhttp(bhttp::Error), @@ -249,8 +249,9 @@ impl std::fmt::Display for ParseOhttpKeysError { ParseOhttpKeysError::InvalidFormat => write!(f, "Invalid format"), ParseOhttpKeysError::InvalidPublicKey => write!(f, "Invalid public key"), ParseOhttpKeysError::DecodeBech32(e) => write!(f, "Failed to decode base64: {}", e), - ParseOhttpKeysError::DecodeKeyConfig(e) => - write!(f, "Failed to decode KeyConfig: {}", e), + ParseOhttpKeysError::DecodeKeyConfig(e) => { + write!(f, "Failed to decode KeyConfig: {}", e) + } } } } From f0f47a1911314494ffe4f02416fa37dfe0e08408 Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 10:01:29 -0500 Subject: [PATCH 4/8] fix: psbt MissingUtxoInformation is always treated as error --- payjoin/src/psbt.rs | 16 +++++----------- payjoin/src/receive/mod.rs | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/payjoin/src/psbt.rs b/payjoin/src/psbt.rs index a051fa800..694e6b801 100644 --- a/payjoin/src/psbt.rs +++ b/payjoin/src/psbt.rs @@ -38,7 +38,7 @@ pub(crate) trait PsbtExt: Sized { // guarantees that length of psbt input matches that of unsigned_tx inputs and same /// thing for outputs. fn validate(self) -> Result; - fn validate_input_utxos(&self, treat_missing_as_error: bool) -> Result<(), PsbtInputsError>; + fn validate_input_utxos(&self) -> Result<(), PsbtInputsError>; } impl PsbtExt for Psbt { @@ -83,11 +83,9 @@ impl PsbtExt for Psbt { } } - fn validate_input_utxos(&self, treat_missing_as_error: bool) -> Result<(), PsbtInputsError> { + fn validate_input_utxos(&self) -> Result<(), PsbtInputsError> { self.input_pairs().enumerate().try_for_each(|(index, input)| { - input - .validate_utxo(treat_missing_as_error) - .map_err(|error| PsbtInputsError { index, error }) + input.validate_utxo().map_err(|error| PsbtInputsError { index, error }) }) } } @@ -124,14 +122,10 @@ impl InternalInputPair<'_> { } } - pub fn validate_utxo( - &self, - treat_missing_as_error: bool, - ) -> Result<(), InternalPsbtInputError> { + pub fn validate_utxo(&self) -> Result<(), InternalPsbtInputError> { match (&self.psbtin.non_witness_utxo, &self.psbtin.witness_utxo) { - (None, None) if treat_missing_as_error => + (None, None) => Err(InternalPsbtInputError::PrevTxOut(PrevTxOutError::MissingUtxoInformation)), - (None, None) => Ok(()), (Some(tx), None) if tx.compute_txid() == self.txin.previous_output.txid => tx .output .get::(self.txin.previous_output.vout.try_into().map_err(|_| { diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index 24f41da26..7a48dd76c 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -23,7 +23,7 @@ impl InputPair { pub fn new(txin: TxIn, psbtin: psbt::Input) -> Result { let input_pair = Self { txin, psbtin }; let raw = InternalInputPair::from(&input_pair); - raw.validate_utxo(true)?; + raw.validate_utxo()?; let address_type = raw.address_type().map_err(InternalPsbtInputError::AddressType)?; if address_type == AddressType::P2sh && input_pair.psbtin.redeem_script.is_none() { return Err(InternalPsbtInputError::NoRedeemScript.into()); From ee2ddfbd917254e570ac43d33e5c52ad567fbfb3 Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 10:03:32 -0500 Subject: [PATCH 5/8] refactor: request new_v# url and body args as refs --- payjoin/src/request.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/payjoin/src/request.rs b/payjoin/src/request.rs index 8dd83f9e1..2059d7651 100644 --- a/payjoin/src/request.rs +++ b/payjoin/src/request.rs @@ -28,13 +28,13 @@ pub struct Request { impl Request { /// Construct a new v1 request. - pub(crate) fn new_v1(url: Url, body: Vec) -> Self { - Self { url, content_type: V1_REQ_CONTENT_TYPE, body } + pub(crate) fn new_v1(url: &Url, body: &[u8]) -> Self { + Self { url: url.clone(), content_type: V1_REQ_CONTENT_TYPE, body: body.to_vec() } } /// Construct a new v2 request. #[cfg(feature = "v2")] - pub(crate) fn new_v2(url: Url, body: [u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES]) -> Self { - Self { url, content_type: V2_REQ_CONTENT_TYPE, body: body.to_vec() } + pub(crate) fn new_v2(url: &Url, body: [u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES]) -> Self { + Self { url: url.clone(), content_type: V2_REQ_CONTENT_TYPE, body: body.to_vec() } } } From 97b0bc4caebc6b68132df3f8e26e1e86d1cb7adb Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 10:04:39 -0500 Subject: [PATCH 6/8] remove: unused std:io:Error --- payjoin/src/send/error.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index ae68c4904..cc619fa73 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -91,7 +91,6 @@ pub struct ValidationError(InternalValidationError); #[derive(Debug)] pub(crate) enum InternalValidationError { Parse, - Io(std::io::Error), Proposal(InternalProposalError), #[cfg(feature = "v2")] V2Encapsulation(crate::send::v2::EncapsulationError), @@ -115,7 +114,6 @@ impl fmt::Display for ValidationError { match &self.0 { Parse => write!(f, "couldn't decode as PSBT or JSON",), - Io(e) => write!(f, "couldn't read PSBT: {}", e), Proposal(e) => write!(f, "proposal PSBT error: {}", e), #[cfg(feature = "v2")] V2Encapsulation(e) => write!(f, "v2 encapsulation error: {}", e), @@ -129,7 +127,6 @@ impl std::error::Error for ValidationError { match &self.0 { Parse => None, - Io(error) => Some(error), Proposal(e) => Some(e), #[cfg(feature = "v2")] V2Encapsulation(e) => Some(e), From 2599eb1a17b5efe71f62884faa4eed7720d43c4f Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 10:05:51 -0500 Subject: [PATCH 7/8] refactor: send module serialize_url endpoint arg as ref --- payjoin/src/send/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index f45f8f30f..2e2f16777 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -14,7 +14,7 @@ use std::str::FromStr; use bitcoin::psbt::Psbt; use bitcoin::{Amount, FeeRate, Script, ScriptBuf, TxOut, Weight}; pub use error::{BuildSenderError, ResponseError, ValidationError}; -pub(crate) use error::{InternalBuildSenderError, InternalProposalError, InternalValidationError}; +pub(crate) use error::{InternalBuildSenderError, InternalProposalError}; use url::Url; use crate::psbt::PsbtExt; @@ -400,13 +400,13 @@ fn determine_fee_contribution( } fn serialize_url( - endpoint: Url, + endpoint: &Url, disable_output_substitution: bool, fee_contribution: Option<(bitcoin::Amount, usize)>, min_fee_rate: FeeRate, version: &str, ) -> Result { - let mut url = endpoint; + let mut url = endpoint.clone(); url.query_pairs_mut().append_pair("v", version); if disable_output_substitution { url.query_pairs_mut().append_pair("disableoutputsubstitution", "true"); From 85aaf0b6310b3827cfb4b4d567c3cb3df34de5b2 Mon Sep 17 00:00:00 2001 From: benalleng Date: Wed, 22 Jan 2025 10:07:08 -0500 Subject: [PATCH 8/8] refactor: send module v2 extract functions ohttp_relay args as refs --- payjoin/src/hpke.rs | 5 +++-- payjoin/src/send/v2/mod.rs | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/payjoin/src/hpke.rs b/payjoin/src/hpke.rs index 114f0f378..be30fe2ea 100644 --- a/payjoin/src/hpke.rs +++ b/payjoin/src/hpke.rs @@ -4,6 +4,7 @@ use std::{error, fmt}; use bitcoin::key::constants::{ ELLSWIFT_ENCODING_SIZE, PUBLIC_KEY_SIZE, UNCOMPRESSED_PUBLIC_KEY_SIZE, }; +use bitcoin::secp256k1; use bitcoin::secp256k1::ellswift::ElligatorSwift; use hpke::aead::ChaCha20Poly1305; use hpke::kdf::HkdfSha256; @@ -121,8 +122,8 @@ impl<'de> serde::Deserialize<'de> for HpkeSecretKey { pub struct HpkePublicKey(pub PublicKey); impl HpkePublicKey { - pub fn to_compressed_bytes(&self) -> [u8; PUBLIC_KEY_SIZE] { - let compressed_key = bitcoin::secp256k1::PublicKey::from_slice(&self.0.to_bytes()) + pub fn to_compressed_bytes(&self) -> [u8; 33] { + let compressed_key = secp256k1::PublicKey::from_slice(&self.0.to_bytes()) .expect("Invalid public key from known valid bytes"); compressed_key.serialize() } diff --git a/payjoin/src/send/v2/mod.rs b/payjoin/src/send/v2/mod.rs index 57cf8232e..526354510 100644 --- a/payjoin/src/send/v2/mod.rs +++ b/payjoin/src/send/v2/mod.rs @@ -136,7 +136,7 @@ impl Sender { /// and has no fallback to v1. pub fn extract_v2( &self, - ohttp_relay: Url, + ohttp_relay: &Url, ) -> Result<(Request, V2PostContext), CreateRequestError> { use crate::hpke::encrypt_message_a; use crate::ohttp::ohttp_encapsulate; @@ -255,7 +255,7 @@ pub struct V2GetContext { impl V2GetContext { pub fn extract_req( &self, - ohttp_relay: Url, + ohttp_relay: &Url, ) -> Result<(Request, ohttp::ClientResponse), CreateRequestError> { use crate::uri::UrlExt; let base_url = self.endpoint.clone(); @@ -317,7 +317,7 @@ struct HpkeContext { #[cfg(feature = "v2")] impl HpkeContext { - pub fn new(receiver: HpkePublicKey, reply_key: &HpkeSecretKey) -> Self { + fn new(receiver: HpkePublicKey, reply_key: &HpkeSecretKey) -> Self { Self { receiver, reply_pair: HpkeKeyPair::from_secret_key(reply_key) } } }