From 15aa7577ae72b1c5f43edccb7724d4bd8a74b1e3 Mon Sep 17 00:00:00 2001 From: DanGould Date: Sat, 4 Jan 2025 11:52:45 -0500 Subject: [PATCH 1/8] Remove redundant cfg feature expression --- payjoin/src/send/v2/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/payjoin/src/send/v2/mod.rs b/payjoin/src/send/v2/mod.rs index 7b2e34d7..97ec0655 100644 --- a/payjoin/src/send/v2/mod.rs +++ b/payjoin/src/send/v2/mod.rs @@ -324,7 +324,6 @@ impl HpkeContext { mod test { #[test] - #[cfg(feature = "v2")] fn req_ctx_ser_de_roundtrip() { use super::*; use crate::send::test::ORIGINAL_PSBT; From 363ed671515b864b7881579debcf3fc2894b0724 Mon Sep 17 00:00:00 2001 From: DanGould Date: Sat, 4 Jan 2025 13:00:39 -0500 Subject: [PATCH 2/8] Separate V2EncapsulationError These variants don't have to do with proposal validation. They are unique to payjoin v2 encapsulation and ought to be abstracted as such. --- payjoin/src/send/error.rs | 24 ++-------------- payjoin/src/send/v2/error.rs | 56 ++++++++++++++++++++++++++++++++++++ payjoin/src/send/v2/mod.rs | 20 ++++++------- 3 files changed, 69 insertions(+), 31 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 89d087be..94e0e0e5 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -135,13 +135,7 @@ pub(crate) enum InternalValidationError { FeeRateBelowMinimum, Psbt(bitcoin::psbt::Error), #[cfg(feature = "v2")] - Hpke(crate::hpke::HpkeError), - #[cfg(feature = "v2")] - OhttpEncapsulation(crate::ohttp::OhttpEncapsulationError), - #[cfg(feature = "v2")] - UnexpectedStatusCode, - #[cfg(feature = "v2")] - UnexpectedResponseSize(usize), + V2Encapsulation(crate::send::v2::EncapsulationError), } impl From for ValidationError { @@ -190,13 +184,7 @@ impl fmt::Display for ValidationError { FeeRateBelowMinimum => write!(f, "the fee rate of proposed transaction is below minimum"), Psbt(e) => write!(f, "psbt error: {}", e), #[cfg(feature = "v2")] - Hpke(e) => write!(f, "v2 error: {}", e), - #[cfg(feature = "v2")] - OhttpEncapsulation(e) => write!(f, "Ohttp encapsulation error: {}", e), - #[cfg(feature = "v2")] - UnexpectedStatusCode => write!(f, "unexpected status code"), - #[cfg(feature = "v2")] - UnexpectedResponseSize(size) => write!(f, "unexpected response size {}, expected {} bytes", size, crate::ohttp::ENCAPSULATED_MESSAGE_BYTES), + V2Encapsulation(e) => write!(f, "v2 encapsulation error: {}", e), } } } @@ -237,13 +225,7 @@ impl std::error::Error for ValidationError { FeeRateBelowMinimum => None, Psbt(error) => Some(error), #[cfg(feature = "v2")] - Hpke(error) => Some(error), - #[cfg(feature = "v2")] - OhttpEncapsulation(error) => Some(error), - #[cfg(feature = "v2")] - UnexpectedStatusCode => None, - #[cfg(feature = "v2")] - UnexpectedResponseSize(_) => None, + V2Encapsulation(e) => Some(e), } } } diff --git a/payjoin/src/send/v2/error.rs b/payjoin/src/send/v2/error.rs index a5c39593..002a605f 100644 --- a/payjoin/src/send/v2/error.rs +++ b/payjoin/src/send/v2/error.rs @@ -60,3 +60,59 @@ impl From for CreateRequestError { CreateRequestError(InternalCreateRequestError::ParseReceiverPubkey(value)) } } + +/// Error returned for v2-specific payload encapsulation errors. +#[derive(Debug)] +pub struct EncapsulationError { + internal: InternalEncapsulationError, +} + +#[derive(Debug)] +pub(crate) enum InternalEncapsulationError { + /// The response size is not the expected size. + InvalidSize(usize), + /// The status code is not the expected status code. + UnexpectedStatusCode(http::StatusCode), + /// The HPKE failed. + Hpke(crate::hpke::HpkeError), + /// The encapsulation failed. + Ohttp(crate::ohttp::OhttpEncapsulationError), +} + +impl fmt::Display for EncapsulationError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use InternalEncapsulationError::*; + + match &self.internal { + InvalidSize(size) => write!(f, "invalid size: {}", size), + UnexpectedStatusCode(status) => write!(f, "unexpected status code: {}", status), + Ohttp(error) => write!(f, "OHTTP encapsulation error: {}", error), + Hpke(error) => write!(f, "HPKE error: {}", error), + } + } +} + +impl std::error::Error for EncapsulationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use InternalEncapsulationError::*; + + match &self.internal { + InvalidSize(_) => None, + UnexpectedStatusCode(_) => None, + Ohttp(error) => Some(error), + Hpke(error) => Some(error), + } + } +} + +impl From for EncapsulationError { + fn from(value: InternalEncapsulationError) -> Self { EncapsulationError { internal: value } } +} + +impl From for super::ResponseError { + fn from(value: InternalEncapsulationError) -> Self { + super::ResponseError::Validation( + super::InternalValidationError::V2Encapsulation(value.into()).into(), + ) + } +} diff --git a/payjoin/src/send/v2/mod.rs b/payjoin/src/send/v2/mod.rs index 97ec0655..be5ea9bb 100644 --- a/payjoin/src/send/v2/mod.rs +++ b/payjoin/src/send/v2/mod.rs @@ -22,8 +22,8 @@ //! wallet and http client. use bitcoin::hashes::{sha256, Hash}; -pub use error::CreateRequestError; -use error::InternalCreateRequestError; +pub use error::{CreateRequestError, EncapsulationError}; +use error::{InternalCreateRequestError, InternalEncapsulationError}; use serde::{Deserialize, Serialize}; use url::Url; @@ -223,13 +223,13 @@ pub struct V2PostContext { } impl V2PostContext { - pub fn process_response(self, response: &[u8]) -> Result { + pub fn process_response(self, response: &[u8]) -> Result { let response_array: &[u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES] = response .try_into() - .map_err(|_| InternalValidationError::UnexpectedResponseSize(response.len()))?; + .map_err(|_| InternalEncapsulationError::InvalidSize(response.len()))?; let response = ohttp_decapsulate(self.ohttp_ctx, response_array) - .map_err(InternalValidationError::OhttpEncapsulation)?; + .map_err(InternalEncapsulationError::Ohttp)?; match response.status() { http::StatusCode::OK => { // return OK with new Typestate @@ -239,7 +239,7 @@ impl V2PostContext { hpke_ctx: self.hpke_ctx, }) } - _ => Err(InternalValidationError::UnexpectedStatusCode)?, + _ => Err(InternalEncapsulationError::UnexpectedStatusCode(response.status()))?, } } } @@ -286,21 +286,21 @@ impl V2GetContext { let response_array: &[u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES] = response .try_into() - .map_err(|_| InternalValidationError::UnexpectedResponseSize(response.len()))?; + .map_err(|_| InternalEncapsulationError::InvalidSize(response.len()))?; let response = ohttp_decapsulate(ohttp_ctx, response_array) - .map_err(InternalValidationError::OhttpEncapsulation)?; + .map_err(InternalEncapsulationError::Ohttp)?; let body = match response.status() { http::StatusCode::OK => response.body().to_vec(), http::StatusCode::ACCEPTED => return Ok(None), - _ => return Err(InternalValidationError::UnexpectedStatusCode)?, + _ => return Err(InternalEncapsulationError::UnexpectedStatusCode(response.status()))?, }; let psbt = decrypt_message_b( &body, self.hpke_ctx.receiver.clone(), self.hpke_ctx.reply_pair.secret_key().clone(), ) - .map_err(InternalValidationError::Hpke)?; + .map_err(InternalEncapsulationError::Hpke)?; let proposal = Psbt::deserialize(&psbt).map_err(InternalValidationError::Psbt)?; let processed_proposal = self.psbt_ctx.clone().process_proposal(proposal)?; From 5261e92eb59972bbf122874a150fa2367f5b9e8f Mon Sep 17 00:00:00 2001 From: DanGould Date: Sat, 4 Jan 2025 14:19:06 -0500 Subject: [PATCH 3/8] Separate ProposalError from Validation These variants all have to do with Proposal PSBT checks distinct from other encapsulation validation --- payjoin/src/send/error.rs | 108 ++++++++++++++++++++++++------------- payjoin/src/send/mod.rs | 20 +++---- payjoin/src/send/v2/mod.rs | 2 +- 3 files changed, 81 insertions(+), 49 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 94e0e0e5..c360e71f 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -94,22 +94,63 @@ pub struct ValidationError { pub(crate) enum InternalValidationError { Parse, Io(std::io::Error), + Proposal(InternalProposalError), + #[cfg(feature = "v2")] + V2Encapsulation(crate::send::v2::EncapsulationError), +} + +impl From for ValidationError { + fn from(value: InternalValidationError) -> Self { ValidationError { internal: value } } +} + +impl From for ValidationError { + fn from(value: crate::psbt::AddressTypeError) -> Self { + ValidationError { + internal: InternalValidationError::Proposal(InternalProposalError::InvalidAddressType( + value, + )), + } + } +} + +impl fmt::Display for ValidationError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use InternalValidationError::*; + + match &self.internal { + 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), + } + } +} + +impl std::error::Error for ValidationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use InternalValidationError::*; + + match &self.internal { + Parse => None, + Io(error) => Some(error), + Proposal(e) => Some(e), + #[cfg(feature = "v2")] + V2Encapsulation(e) => Some(e), + } + } +} + +/// Error that may occur when the proposal PSBT from receiver is malformed. +#[derive(Debug)] +pub(crate) enum InternalProposalError { InvalidAddressType(crate::psbt::AddressTypeError), NoInputs, PrevTxOut(crate::psbt::PrevTxOutError), InputWeight(crate::psbt::InputWeightError), - VersionsDontMatch { - proposed: Version, - original: Version, - }, - LockTimesDontMatch { - proposed: LockTime, - original: LockTime, - }, - SenderTxinSequenceChanged { - proposed: Sequence, - original: Sequence, - }, + VersionsDontMatch { proposed: Version, original: Version }, + LockTimesDontMatch { proposed: LockTime, original: LockTime }, + SenderTxinSequenceChanged { proposed: Sequence, original: Sequence }, SenderTxinContainsNonWitnessUtxo, SenderTxinContainsWitnessUtxo, SenderTxinContainsFinalScriptSig, @@ -119,10 +160,7 @@ pub(crate) enum InternalValidationError { ReceiverTxinNotFinalized, ReceiverTxinMissingUtxoInfo, MixedSequence, - MixedInputTypes { - proposed: AddressType, - original: AddressType, - }, + MixedInputTypes { proposed: AddressType, original: AddressType }, MissingOrShuffledInputs, TxOutContainsKeyPaths, FeeContributionExceedsMaximum, @@ -134,27 +172,19 @@ pub(crate) enum InternalValidationError { FeeContributionPaysOutputSizeIncrease, FeeRateBelowMinimum, Psbt(bitcoin::psbt::Error), - #[cfg(feature = "v2")] - V2Encapsulation(crate::send::v2::EncapsulationError), -} - -impl From for ValidationError { - fn from(value: InternalValidationError) -> Self { ValidationError { internal: value } } } -impl From for InternalValidationError { +impl From for InternalProposalError { fn from(value: crate::psbt::AddressTypeError) -> Self { - InternalValidationError::InvalidAddressType(value) + InternalProposalError::InvalidAddressType(value) } } -impl fmt::Display for ValidationError { +impl fmt::Display for InternalProposalError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use InternalValidationError::*; + use InternalProposalError::*; - match &self.internal { - Parse => write!(f, "couldn't decode as PSBT or JSON",), - Io(e) => write!(f, "couldn't read PSBT: {}", e), + match &self { InvalidAddressType(e) => write!(f, "invalid input address type: {}", e), NoInputs => write!(f, "PSBT doesn't have any inputs"), PrevTxOut(e) => write!(f, "missing previous txout information: {}", e), @@ -183,19 +213,15 @@ impl fmt::Display for ValidationError { FeeContributionPaysOutputSizeIncrease => write!(f, "fee contribution pays for additional outputs"), FeeRateBelowMinimum => write!(f, "the fee rate of proposed transaction is below minimum"), Psbt(e) => write!(f, "psbt error: {}", e), - #[cfg(feature = "v2")] - V2Encapsulation(e) => write!(f, "v2 encapsulation error: {}", e), } } } -impl std::error::Error for ValidationError { +impl std::error::Error for InternalProposalError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use InternalValidationError::*; + use InternalProposalError::*; - match &self.internal { - Parse => None, - Io(error) => Some(error), + match self { InvalidAddressType(error) => Some(error), NoInputs => None, PrevTxOut(error) => Some(error), @@ -224,8 +250,6 @@ impl std::error::Error for ValidationError { FeeContributionPaysOutputSizeIncrease => None, FeeRateBelowMinimum => None, Psbt(error) => Some(error), - #[cfg(feature = "v2")] - V2Encapsulation(e) => Some(e), } } } @@ -310,6 +334,14 @@ impl From for ResponseError { } } +impl From for ResponseError { + fn from(value: InternalProposalError) -> Self { + ResponseError::Validation(ValidationError { + internal: InternalValidationError::Proposal(value), + }) + } +} + // It is imperative to carefully display pre-defined messages to end users and the details in debug. impl Display for ResponseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index cf7f8df2..0f31b519 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -11,7 +11,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, InternalValidationError}; +pub(crate) use error::{InternalBuildSenderError, InternalProposalError, InternalValidationError}; use url::Url; use crate::psbt::PsbtExt; @@ -25,7 +25,7 @@ pub mod v1; #[cfg(feature = "v2")] pub mod v2; -type InternalResult = Result; +type InternalResult = Result; /// Data required to validate the response. /// @@ -60,7 +60,7 @@ macro_rules! check_eq { ($proposed:expr, $original:expr, $error:ident) => { match ($proposed, $original) { (proposed, original) if proposed != original => - return Err(InternalValidationError::$error { proposed, original }), + return Err(InternalProposalError::$error { proposed, original }), _ => (), } }; @@ -69,7 +69,7 @@ macro_rules! check_eq { macro_rules! ensure { ($cond:expr, $error:ident) => { if !($cond) { - return Err(InternalValidationError::$error); + return Err(InternalProposalError::$error); } }; } @@ -100,8 +100,8 @@ impl PsbtContext { } fn check_fees(&self, proposal: &Psbt, contributed_fee: Amount) -> InternalResult<()> { - let proposed_fee = proposal.fee().map_err(InternalValidationError::Psbt)?; - let original_fee = self.original_psbt.fee().map_err(InternalValidationError::Psbt)?; + let proposed_fee = proposal.fee().map_err(InternalProposalError::Psbt)?; + let original_fee = self.original_psbt.fee().map_err(InternalProposalError::Psbt)?; ensure!(original_fee <= proposed_fee, AbsoluteFeeDecreased); ensure!(contributed_fee <= proposed_fee - original_fee, PayeeTookContributedFee); let original_weight = self.original_psbt.clone().extract_tx_unchecked_fee_rate().weight(); @@ -112,7 +112,7 @@ impl PsbtContext { .map(|input_pair| { input_pair .previous_txout() - .map_err(InternalValidationError::PrevTxOut) + .map_err(InternalProposalError::PrevTxOut) .map(|txout| txout.script_pubkey.clone()) }) .collect::>>()?; @@ -121,14 +121,14 @@ impl PsbtContext { |acc, input_pair| -> InternalResult { let spk = &input_pair .previous_txout() - .map_err(InternalValidationError::PrevTxOut)? + .map_err(InternalProposalError::PrevTxOut)? .script_pubkey; if original_spks.contains(spk) { Ok(acc) } else { let weight = input_pair .expected_input_weight() - .map_err(InternalValidationError::InputWeight)?; + .map_err(InternalProposalError::InputWeight)?; Ok(acc + weight) } }, @@ -196,7 +196,7 @@ impl PsbtContext { .original_psbt .input_pairs() .next() - .ok_or(InternalValidationError::NoInputs)?; + .ok_or(InternalProposalError::NoInputs)?; // Verify the PSBT input is finalized ensure!( proposed.psbtin.final_script_sig.is_some() diff --git a/payjoin/src/send/v2/mod.rs b/payjoin/src/send/v2/mod.rs index be5ea9bb..490d9431 100644 --- a/payjoin/src/send/v2/mod.rs +++ b/payjoin/src/send/v2/mod.rs @@ -302,7 +302,7 @@ impl V2GetContext { ) .map_err(InternalEncapsulationError::Hpke)?; - let proposal = Psbt::deserialize(&psbt).map_err(InternalValidationError::Psbt)?; + let proposal = Psbt::deserialize(&psbt).map_err(InternalProposalError::Psbt)?; let processed_proposal = self.psbt_ctx.clone().process_proposal(proposal)?; Ok(Some(processed_proposal)) } From e26df285598bc340167a83fab8221b426ba9f95f Mon Sep 17 00:00:00 2001 From: DanGould Date: Sat, 4 Jan 2025 14:24:54 -0500 Subject: [PATCH 4/8] Fix ResponseError formatting This includes redundant documentation we don't want to get out of sync and the critical Unrecognized variant that requires special handling. --- payjoin/src/send/error.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index c360e71f..a2566025 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -262,17 +262,19 @@ pub enum ResponseError { /// /// [`BIP78::ReceiverWellKnownError`]: https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#user-content-Receivers_well_known_errors WellKnown(WellKnownError), + + /// Errors caused by malformed responses. + /// + /// These errors are only displayed in debug logs. + Validation(ValidationError), + /// `Unrecognized` Errors are NOT defined in the [`BIP78::ReceiverWellKnownError`] spec. /// - /// Its not safe to display `Unrecognized` errors to end users as they could be used + /// It is NOT safe to display `Unrecognized` errors to end users as they could be used /// maliciously to phish a non technical user. Only display them in debug logs. /// /// [`BIP78::ReceiverWellKnownError`]: https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#user-content-Receivers_well_known_errors Unrecognized { error_code: String, message: String }, - /// Errors caused by malformed responses. - /// - /// These errors are only displayed in debug logs. - Validation(ValidationError), } impl ResponseError { @@ -342,14 +344,14 @@ impl From for ResponseError { } } -// It is imperative to carefully display pre-defined messages to end users and the details in debug. impl Display for ResponseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { Self::WellKnown(e) => e.fmt(f), - // Don't display unknowns to end users, only debug logs - Self::Unrecognized { .. } => write!(f, "The receiver sent an unrecognized error."), Self::Validation(_) => write!(f, "The receiver sent an invalid response."), + + // Do NOT display unrecognized errors to end users, only debug logs + Self::Unrecognized { .. } => write!(f, "The receiver sent an unrecognized error."), } } } @@ -364,12 +366,13 @@ impl fmt::Debug for ResponseError { e.error_code(), e.message() ), + Self::Validation(e) => write!(f, "Validation({:?})", e), + Self::Unrecognized { error_code, message } => write!( f, r#"Unrecognized error: {{ "errorCode": "{}", "message": "{}" }}"#, error_code, message ), - Self::Validation(e) => write!(f, "Validation({:?})", e), } } } From 27f4cb9028f9f6e5a11175a8974a2eb52057dae2 Mon Sep 17 00:00:00 2001 From: DanGould Date: Sat, 4 Jan 2025 14:29:17 -0500 Subject: [PATCH 5/8] Display Validation ResponseErrors These errors are enumerated and not subject to the kinds of phishing attacks Unrecognized errors are. Downstream implementations may choose to display these in e.g. a CLI environment. Only the Unrecognized errors MUST be swallowed because their contents are defined by the counterparty. --- payjoin/src/send/error.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index a2566025..1586fc91 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -264,8 +264,6 @@ pub enum ResponseError { WellKnown(WellKnownError), /// Errors caused by malformed responses. - /// - /// These errors are only displayed in debug logs. Validation(ValidationError), /// `Unrecognized` Errors are NOT defined in the [`BIP78::ReceiverWellKnownError`] spec. @@ -348,7 +346,7 @@ impl Display for ResponseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { Self::WellKnown(e) => e.fmt(f), - Self::Validation(_) => write!(f, "The receiver sent an invalid response."), + Self::Validation(e) => write!(f, "The receiver sent an invalid response: {}", e), // Do NOT display unrecognized errors to end users, only debug logs Self::Unrecognized { .. } => write!(f, "The receiver sent an unrecognized error."), @@ -444,9 +442,9 @@ mod tests { "err": "random", "message": "This version of payjoin is not supported." }); - assert_eq!( - ResponseError::from_json(invalid_json_error).to_string(), - "The receiver sent an invalid response." - ); + assert!(matches!( + ResponseError::from_json(invalid_json_error), + ResponseError::Validation(_) + )); } } From 1a2273f6d2b362f30a4299882bc7be4a0d6debe2 Mon Sep 17 00:00:00 2001 From: DanGould Date: Sat, 4 Jan 2025 14:34:47 -0500 Subject: [PATCH 6/8] Test parse_json variant match, not string --- payjoin/src/send/error.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 1586fc91..f6a16e14 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -434,10 +434,10 @@ mod tests { _ => panic!("Expected WellKnown error"), }; let unrecognized_error = r#"{"errorCode":"random", "message":"random"}"#; - assert_eq!( - ResponseError::parse(unrecognized_error).to_string(), - "The receiver sent an unrecognized error." - ); + assert!(matches!( + ResponseError::parse(unrecognized_error), + ResponseError::Unrecognized { .. } + )); let invalid_json_error = json!({ "err": "random", "message": "This version of payjoin is not supported." From c2753296fb1dc69ff03090cfe60aa858a7843f2f Mon Sep 17 00:00:00 2001 From: DanGould Date: Sun, 5 Jan 2025 13:22:41 -0500 Subject: [PATCH 7/8] Move V1Context to v1 mod Make explicit that this type is only used in the v1 state path. Yes, it can be used in the v2 path, but only if extract_v1 is called to shift onto the v1 path explicitly. --- payjoin/src/send/mod.rs | 18 ------------------ payjoin/src/send/v1.rs | 18 ++++++++++++++++++ payjoin/src/send/v2/mod.rs | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 0f31b519..fad821de 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -27,24 +27,6 @@ pub mod v2; type InternalResult = Result; -/// Data required to validate the response. -/// -/// This type is used to process a BIP78 response. -/// Then call [`Self::process_response`] on it to continue BIP78 flow. -#[derive(Debug, Clone)] -pub struct V1Context { - psbt_context: PsbtContext, -} - -impl V1Context { - pub fn process_response( - self, - response: &mut impl std::io::Read, - ) -> Result { - self.psbt_context.process_response(response) - } -} - /// Data required to validate the response against the original PSBT. #[derive(Debug, Clone)] pub struct PsbtContext { diff --git a/payjoin/src/send/v1.rs b/payjoin/src/send/v1.rs index a3194bba..90b27b33 100644 --- a/payjoin/src/send/v1.rs +++ b/payjoin/src/send/v1.rs @@ -253,3 +253,21 @@ impl Sender { pub fn endpoint(&self) -> &Url { &self.endpoint } } + +/// Data required to validate the response. +/// +/// This type is used to process a BIP78 response. +/// Then call [`Self::process_response`] on it to continue BIP78 flow. +#[derive(Debug, Clone)] +pub struct V1Context { + psbt_context: PsbtContext, +} + +impl V1Context { + pub fn process_response( + self, + response: &mut impl std::io::Read, + ) -> Result { + self.psbt_context.process_response(response) + } +} diff --git a/payjoin/src/send/v2/mod.rs b/payjoin/src/send/v2/mod.rs index 490d9431..57cf8232 100644 --- a/payjoin/src/send/v2/mod.rs +++ b/payjoin/src/send/v2/mod.rs @@ -126,7 +126,7 @@ pub struct Sender { impl Sender { /// Extract serialized V1 Request and Context from a Payjoin Proposal - pub fn extract_v1(&self) -> Result<(Request, V1Context), url::ParseError> { + pub fn extract_v1(&self) -> Result<(Request, v1::V1Context), url::ParseError> { self.v1.extract_v1() } From be7df15881124272a47d5f6e020da80323ba29c4 Mon Sep 17 00:00:00 2001 From: DanGould Date: Mon, 6 Jan 2025 22:26:44 -0500 Subject: [PATCH 8/8] Encapsulate internal error variants in tuple No need for them to be named --- payjoin/src/send/error.rs | 26 +++++++++----------------- payjoin/src/send/v2/error.rs | 10 ++++------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index f6a16e14..ae68c490 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -86,9 +86,7 @@ impl std::error::Error for BuildSenderError { /// This is currently opaque type because we aren't sure which variants will stay. /// You can only display it. #[derive(Debug)] -pub struct ValidationError { - internal: InternalValidationError, -} +pub struct ValidationError(InternalValidationError); #[derive(Debug)] pub(crate) enum InternalValidationError { @@ -100,16 +98,14 @@ pub(crate) enum InternalValidationError { } impl From for ValidationError { - fn from(value: InternalValidationError) -> Self { ValidationError { internal: value } } + fn from(value: InternalValidationError) -> Self { ValidationError(value) } } impl From for ValidationError { fn from(value: crate::psbt::AddressTypeError) -> Self { - ValidationError { - internal: InternalValidationError::Proposal(InternalProposalError::InvalidAddressType( - value, - )), - } + ValidationError(InternalValidationError::Proposal( + InternalProposalError::InvalidAddressType(value), + )) } } @@ -117,7 +113,7 @@ impl fmt::Display for ValidationError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use InternalValidationError::*; - match &self.internal { + 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), @@ -131,7 +127,7 @@ impl std::error::Error for ValidationError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { use InternalValidationError::*; - match &self.internal { + match &self.0 { Parse => None, Io(error) => Some(error), Proposal(e) => Some(e), @@ -329,16 +325,12 @@ impl From for ResponseError { } impl From for ResponseError { - fn from(value: InternalValidationError) -> Self { - Self::Validation(ValidationError { internal: value }) - } + fn from(value: InternalValidationError) -> Self { Self::Validation(ValidationError(value)) } } impl From for ResponseError { fn from(value: InternalProposalError) -> Self { - ResponseError::Validation(ValidationError { - internal: InternalValidationError::Proposal(value), - }) + ResponseError::Validation(ValidationError(InternalValidationError::Proposal(value))) } } diff --git a/payjoin/src/send/v2/error.rs b/payjoin/src/send/v2/error.rs index 002a605f..663b6909 100644 --- a/payjoin/src/send/v2/error.rs +++ b/payjoin/src/send/v2/error.rs @@ -63,9 +63,7 @@ impl From for CreateRequestError { /// Error returned for v2-specific payload encapsulation errors. #[derive(Debug)] -pub struct EncapsulationError { - internal: InternalEncapsulationError, -} +pub struct EncapsulationError(InternalEncapsulationError); #[derive(Debug)] pub(crate) enum InternalEncapsulationError { @@ -83,7 +81,7 @@ impl fmt::Display for EncapsulationError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use InternalEncapsulationError::*; - match &self.internal { + match &self.0 { InvalidSize(size) => write!(f, "invalid size: {}", size), UnexpectedStatusCode(status) => write!(f, "unexpected status code: {}", status), Ohttp(error) => write!(f, "OHTTP encapsulation error: {}", error), @@ -96,7 +94,7 @@ impl std::error::Error for EncapsulationError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { use InternalEncapsulationError::*; - match &self.internal { + match &self.0 { InvalidSize(_) => None, UnexpectedStatusCode(_) => None, Ohttp(error) => Some(error), @@ -106,7 +104,7 @@ impl std::error::Error for EncapsulationError { } impl From for EncapsulationError { - fn from(value: InternalEncapsulationError) -> Self { EncapsulationError { internal: value } } + fn from(value: InternalEncapsulationError) -> Self { EncapsulationError(value) } } impl From for super::ResponseError {