From 33a58f779ae827d9ba8d3125c57a8399eaabe170 Mon Sep 17 00:00:00 2001 From: DanGould Date: Wed, 26 Mar 2025 13:59:15 -0400 Subject: [PATCH 1/6] Add `enum ErrorCodes` Well-known error codes are used on both sides of the protocol. Having a strong type will let us serialize an error Reply across a foreign language boundary. We were returning a non-spec sender-params-error which amounts to a rejection of the original PSBT for unparseable feerate. Instead, I decided to return the well-known original-psbt-rejected error code and to include the parse error in the debug message in order to comply more closely with existing spec. --- payjoin/src/error_codes.rs | 47 +++++++++++++++++++---- payjoin/src/receive/error.rs | 36 ++++++++--------- payjoin/src/receive/v1/exclusive/error.rs | 11 +++--- payjoin/src/send/error.rs | 27 +++++++------ 4 files changed, 76 insertions(+), 45 deletions(-) diff --git a/payjoin/src/error_codes.rs b/payjoin/src/error_codes.rs index 340d73eb5..4a1069dc2 100644 --- a/payjoin/src/error_codes.rs +++ b/payjoin/src/error_codes.rs @@ -1,14 +1,45 @@ //! Well-known error codes as defined in BIP-78 //! See: -/// The payjoin endpoint is not available for now. -pub const UNAVAILABLE: &str = "unavailable"; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ErrorCode { + /// The payjoin endpoint is not available for now. + Unavailable, + /// The receiver added some inputs but could not bump the fee of the payjoin proposal. + NotEnoughMoney, + /// This version of payjoin is not supported. + VersionUnsupported, + /// The receiver rejected the original PSBT. + OriginalPsbtRejected, +} -/// The receiver added some inputs but could not bump the fee of the payjoin proposal. -pub const NOT_ENOUGH_MONEY: &str = "not-enough-money"; +impl ErrorCode { + pub const fn as_str(&self) -> &'static str { + match self { + Self::Unavailable => "unavailable", + Self::NotEnoughMoney => "not-enough-money", + Self::VersionUnsupported => "version-unsupported", + Self::OriginalPsbtRejected => "original-psbt-rejected", + } + } +} -/// This version of payjoin is not supported. -pub const VERSION_UNSUPPORTED: &str = "version-unsupported"; +impl core::str::FromStr for ErrorCode { + type Err = (); -/// The receiver rejected the original PSBT. -pub const ORIGINAL_PSBT_REJECTED: &str = "original-psbt-rejected"; + fn from_str(s: &str) -> Result { + match s { + "unavailable" => Ok(Self::Unavailable), + "not-enough-money" => Ok(Self::NotEnoughMoney), + "version-unsupported" => Ok(Self::VersionUnsupported), + "original-psbt-rejected" => Ok(Self::OriginalPsbtRejected), + _ => Err(()), + } + } +} + +impl core::fmt::Display for ErrorCode { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str(self.as_str()) + } +} diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 47f5aa586..b44960fd6 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -1,7 +1,7 @@ use std::{error, fmt}; -use crate::error_codes::{ - NOT_ENOUGH_MONEY, ORIGINAL_PSBT_REJECTED, UNAVAILABLE, VERSION_UNSUPPORTED, +use crate::error_codes::ErrorCode::{ + self, NotEnoughMoney, OriginalPsbtRejected, Unavailable, VersionUnsupported, }; pub type ImplementationError = Box; @@ -82,17 +82,17 @@ impl JsonError for ReplyableError { Self::Payload(e) => e.to_json(), #[cfg(feature = "v1")] Self::V1(e) => e.to_json(), - Self::Implementation(_) => serialize_json_error(UNAVAILABLE, "Receiver error"), + Self::Implementation(_) => serialize_json_error(Unavailable, "Receiver error"), } } } -pub(crate) fn serialize_json_error(code: &str, message: impl fmt::Display) -> String { +pub(crate) fn serialize_json_error(code: ErrorCode, message: impl fmt::Display) -> String { format!(r#"{{ "errorCode": "{}", "message": "{}" }}"#, code, message) } pub(crate) fn serialize_json_plus_fields( - code: &str, + code: ErrorCode, message: impl fmt::Display, additional_fields: &str, ) -> String { @@ -185,29 +185,29 @@ impl JsonError for PayloadError { use InternalPayloadError::*; match &self.0 { - Utf8(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - ParsePsbt(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), + Utf8(_) => serialize_json_error(OriginalPsbtRejected, self), + ParsePsbt(_) => serialize_json_error(OriginalPsbtRejected, self), SenderParams(e) => match e { super::optional_parameters::Error::UnknownVersion { supported_versions } => { let supported_versions_json = serde_json::to_string(supported_versions).unwrap_or_default(); serialize_json_plus_fields( - VERSION_UNSUPPORTED, + VersionUnsupported, "This version of payjoin is not supported.", &format!(r#""supported": {}"#, supported_versions_json), ) } - _ => serialize_json_error("sender-params-error", self), + _ => serialize_json_error(OriginalPsbtRejected, self), }, - InconsistentPsbt(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - PrevTxOut(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - MissingPayment => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - OriginalPsbtNotBroadcastable => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - InputOwned(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - InputWeight(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - InputSeen(_) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - PsbtBelowFeeRate(_, _) => serialize_json_error(ORIGINAL_PSBT_REJECTED, self), - FeeTooHigh(_, _) => serialize_json_error(NOT_ENOUGH_MONEY, self), + InconsistentPsbt(_) => serialize_json_error(OriginalPsbtRejected, self), + PrevTxOut(_) => serialize_json_error(OriginalPsbtRejected, self), + MissingPayment => serialize_json_error(OriginalPsbtRejected, self), + OriginalPsbtNotBroadcastable => serialize_json_error(OriginalPsbtRejected, self), + InputOwned(_) => serialize_json_error(OriginalPsbtRejected, self), + InputWeight(_) => serialize_json_error(OriginalPsbtRejected, self), + InputSeen(_) => serialize_json_error(OriginalPsbtRejected, self), + PsbtBelowFeeRate(_, _) => serialize_json_error(OriginalPsbtRejected, self), + FeeTooHigh(_, _) => serialize_json_error(NotEnoughMoney, self), } } } diff --git a/payjoin/src/receive/v1/exclusive/error.rs b/payjoin/src/receive/v1/exclusive/error.rs index 73a155ecc..afb5de6f7 100644 --- a/payjoin/src/receive/v1/exclusive/error.rs +++ b/payjoin/src/receive/v1/exclusive/error.rs @@ -42,13 +42,14 @@ impl JsonError for RequestError { fn to_json(&self) -> String { use InternalRequestError::*; + use crate::error_codes::ErrorCode::OriginalPsbtRejected; use crate::receive::error::serialize_json_error; match &self.0 { - Io(_) => serialize_json_error("original-psbt-rejected", self), - MissingHeader(_) => serialize_json_error("original-psbt-rejected", self), - InvalidContentType(_) => serialize_json_error("original-psbt-rejected", self), - InvalidContentLength(_) => serialize_json_error("original-psbt-rejected", self), - ContentLengthTooLarge(_) => serialize_json_error("original-psbt-rejected", self), + Io(_) => serialize_json_error(OriginalPsbtRejected, self), + MissingHeader(_) => serialize_json_error(OriginalPsbtRejected, self), + InvalidContentType(_) => serialize_json_error(OriginalPsbtRejected, self), + InvalidContentLength(_) => serialize_json_error(OriginalPsbtRejected, self), + ContentLengthTooLarge(_) => serialize_json_error(OriginalPsbtRejected, self), } } } diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index a11cca37d..0136374a9 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -1,12 +1,11 @@ use std::fmt::{self, Display}; +use std::str::FromStr; use bitcoin::locktime::absolute::LockTime; use bitcoin::transaction::Version; use bitcoin::Sequence; -use crate::error_codes::{ - NOT_ENOUGH_MONEY, ORIGINAL_PSBT_REJECTED, UNAVAILABLE, VERSION_UNSUPPORTED, -}; +use crate::error_codes::ErrorCode; /// Error building a Sender from a SenderBuilder. /// @@ -282,8 +281,8 @@ impl ResponseError { if let Some(error_code) = json.as_object().and_then(|v| v.get("errorCode")).and_then(|v| v.as_str()) { - match error_code { - code if code == VERSION_UNSUPPORTED => { + match ErrorCode::from_str(error_code) { + Ok(ErrorCode::VersionUnsupported) => { let supported = json .as_object() .and_then(|v| v.get("supported")) @@ -292,9 +291,9 @@ impl ResponseError { .unwrap_or_default(); WellKnownError::VersionUnsupported { message, supported }.into() } - code if code == UNAVAILABLE => WellKnownError::Unavailable(message).into(), - code if code == NOT_ENOUGH_MONEY => WellKnownError::NotEnoughMoney(message).into(), - code if code == ORIGINAL_PSBT_REJECTED => + Ok(ErrorCode::Unavailable) => WellKnownError::Unavailable(message).into(), + Ok(ErrorCode::NotEnoughMoney) => WellKnownError::NotEnoughMoney(message).into(), + Ok(ErrorCode::OriginalPsbtRejected) => WellKnownError::OriginalPsbtRejected(message).into(), _ => Self::Unrecognized { error_code: error_code.to_string(), message }, } @@ -373,12 +372,12 @@ pub enum WellKnownError { } impl WellKnownError { - pub fn error_code(&self) -> &str { + pub fn error_code(&self) -> ErrorCode { match self { - WellKnownError::Unavailable(_) => UNAVAILABLE, - WellKnownError::NotEnoughMoney(_) => NOT_ENOUGH_MONEY, - WellKnownError::VersionUnsupported { .. } => VERSION_UNSUPPORTED, - WellKnownError::OriginalPsbtRejected(_) => ORIGINAL_PSBT_REJECTED, + WellKnownError::Unavailable(_) => ErrorCode::Unavailable, + WellKnownError::NotEnoughMoney(_) => ErrorCode::NotEnoughMoney, + WellKnownError::VersionUnsupported { .. } => ErrorCode::VersionUnsupported, + WellKnownError::OriginalPsbtRejected(_) => ErrorCode::OriginalPsbtRejected, } } pub fn message(&self) -> &str { @@ -413,7 +412,7 @@ mod tests { let known_str_error = r#"{"errorCode":"version-unsupported", "message":"custom message here", "supported": [1, 2]}"#; match ResponseError::parse(known_str_error) { ResponseError::WellKnown(e) => { - assert_eq!(e.error_code(), "version-unsupported"); + assert_eq!(e.error_code(), ErrorCode::VersionUnsupported); assert_eq!(e.message(), "custom message here"); assert_eq!( e.to_string(), From 2de6e18203b9fc970b69f9903a9ee2482a0aae02 Mon Sep 17 00:00:00 2001 From: DanGould Date: Wed, 26 Mar 2025 14:25:40 -0400 Subject: [PATCH 2/6] Replace JsonError trait with JsonReply struct This concrete simple struct can more easily cross the foreign language boundaries for bindings than the errors with complex internal variants that implement the JsonError trait. --- payjoin/src/receive/error.rs | 103 +++++++++++++--------- payjoin/src/receive/mod.rs | 2 +- payjoin/src/receive/v1/exclusive/error.rs | 19 ++-- payjoin/src/receive/v2/mod.rs | 6 +- 4 files changed, 73 insertions(+), 57 deletions(-) diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index b44960fd6..1e4bb9573 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -48,7 +48,8 @@ impl error::Error for Error { /// 1. Provide structured error responses for protocol-level failures /// 2. Hide implementation details of external errors for security /// 3. Support proper error propagation through the receiver stack -/// 4. Provide errors according to BIP-78 JSON error specifications for return using [`JsonError::to_json`] +/// 4. Provide errors according to BIP-78 JSON error specifications for return +/// after conversion into [`JsonReply`] #[derive(Debug)] pub enum ReplyableError { /// Error arising from validation of the original PSBT payload @@ -62,41 +63,54 @@ pub enum ReplyableError { Implementation(ImplementationError), } -/// A trait for errors that can be serialized to JSON in a standardized format. +/// The standard format for errors that can be replied as JSON. /// -/// The JSON output follows the structure: +/// The JSON output includes the following fields: /// ```json /// { /// "errorCode": "specific-error-code", /// "message": "Human readable error message" /// } /// ``` -pub trait JsonError { - /// Converts the error into a JSON string representation. - fn to_json(&self) -> String; +pub struct JsonReply { + /// The error code + error_code: ErrorCode, + /// The error message to be displayed only in debug logs + message: String, + /// Additional fields to be added to the JSON + additional_fields: Vec, } -impl JsonError for ReplyableError { - fn to_json(&self) -> String { - match self { - Self::Payload(e) => e.to_json(), - #[cfg(feature = "v1")] - Self::V1(e) => e.to_json(), - Self::Implementation(_) => serialize_json_error(Unavailable, "Receiver error"), - } +impl JsonReply { + /// Create a new Reply + pub fn new(error_code: ErrorCode, message: impl fmt::Display) -> Self { + Self { error_code, message: message.to_string(), additional_fields: vec![] } } -} -pub(crate) fn serialize_json_error(code: ErrorCode, message: impl fmt::Display) -> String { - format!(r#"{{ "errorCode": "{}", "message": "{}" }}"#, code, message) + /// Serialize the Reply to a JSON string + pub fn to_json(&self) -> String { + if self.additional_fields.is_empty() { + format!(r#"{{ "errorCode": "{}", "message": "{}" }}"#, self.error_code, self.message) + } else { + format!( + r#"{{ "errorCode": "{}", "message": "{}", {} }}"#, + self.error_code, + self.message, + self.additional_fields.join(", ") + ) + } + } } -pub(crate) fn serialize_json_plus_fields( - code: ErrorCode, - message: impl fmt::Display, - additional_fields: &str, -) -> String { - format!(r#"{{ "errorCode": "{}", "message": "{}", {} }}"#, code, message, additional_fields) +impl From<&ReplyableError> for JsonReply { + fn from(e: &ReplyableError) -> Self { + match e { + ReplyableError::Payload(e) => e.into(), + #[cfg(feature = "v1")] + ReplyableError::V1(e) => e.into(), + ReplyableError::Implementation(_) => JsonReply::new(Unavailable, "Receiver error"), + } + } } impl fmt::Display for ReplyableError { @@ -180,34 +194,37 @@ pub(crate) enum InternalPayloadError { FeeTooHigh(bitcoin::FeeRate, bitcoin::FeeRate), } -impl JsonError for PayloadError { - fn to_json(&self) -> String { +impl From<&PayloadError> for JsonReply { + fn from(e: &PayloadError) -> Self { use InternalPayloadError::*; - match &self.0 { - Utf8(_) => serialize_json_error(OriginalPsbtRejected, self), - ParsePsbt(_) => serialize_json_error(OriginalPsbtRejected, self), + match &e.0 { + Utf8(_) => JsonReply::new(OriginalPsbtRejected, e), + ParsePsbt(_) => JsonReply::new(OriginalPsbtRejected, e), SenderParams(e) => match e { super::optional_parameters::Error::UnknownVersion { supported_versions } => { let supported_versions_json = serde_json::to_string(supported_versions).unwrap_or_default(); - serialize_json_plus_fields( - VersionUnsupported, - "This version of payjoin is not supported.", - &format!(r#""supported": {}"#, supported_versions_json), - ) + JsonReply { + error_code: VersionUnsupported, + message: "This version of payjoin is not supported.".to_string(), + additional_fields: vec![format!( + r#""supported": {}"#, + supported_versions_json + )], + } } - _ => serialize_json_error(OriginalPsbtRejected, self), + _ => JsonReply::new(OriginalPsbtRejected, e), }, - InconsistentPsbt(_) => serialize_json_error(OriginalPsbtRejected, self), - PrevTxOut(_) => serialize_json_error(OriginalPsbtRejected, self), - MissingPayment => serialize_json_error(OriginalPsbtRejected, self), - OriginalPsbtNotBroadcastable => serialize_json_error(OriginalPsbtRejected, self), - InputOwned(_) => serialize_json_error(OriginalPsbtRejected, self), - InputWeight(_) => serialize_json_error(OriginalPsbtRejected, self), - InputSeen(_) => serialize_json_error(OriginalPsbtRejected, self), - PsbtBelowFeeRate(_, _) => serialize_json_error(OriginalPsbtRejected, self), - FeeTooHigh(_, _) => serialize_json_error(NotEnoughMoney, self), + InconsistentPsbt(_) => JsonReply::new(OriginalPsbtRejected, e), + PrevTxOut(_) => JsonReply::new(OriginalPsbtRejected, e), + MissingPayment => JsonReply::new(OriginalPsbtRejected, e), + OriginalPsbtNotBroadcastable => JsonReply::new(OriginalPsbtRejected, e), + InputOwned(_) => JsonReply::new(OriginalPsbtRejected, e), + InputWeight(_) => JsonReply::new(OriginalPsbtRejected, e), + InputSeen(_) => JsonReply::new(OriginalPsbtRejected, e), + PsbtBelowFeeRate(_, _) => JsonReply::new(OriginalPsbtRejected, e), + FeeTooHigh(_, _) => JsonReply::new(NotEnoughMoney, e), } } } diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index fa3151b5d..2eb981573 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -14,7 +14,7 @@ use std::str::FromStr; use bitcoin::{psbt, AddressType, Psbt, TxIn, TxOut}; pub(crate) use error::InternalPayloadError; pub use error::{ - Error, ImplementationError, InputContributionError, JsonError, OutputSubstitutionError, + Error, ImplementationError, InputContributionError, JsonReply, OutputSubstitutionError, PayloadError, ReplyableError, SelectionError, }; use optional_parameters::Params; diff --git a/payjoin/src/receive/v1/exclusive/error.rs b/payjoin/src/receive/v1/exclusive/error.rs index afb5de6f7..3e3bc65ad 100644 --- a/payjoin/src/receive/v1/exclusive/error.rs +++ b/payjoin/src/receive/v1/exclusive/error.rs @@ -1,7 +1,7 @@ use core::fmt; use std::error; -use crate::receive::error::JsonError; +use crate::receive::JsonReply; /// Error that occurs during validation of an incoming v1 payjoin request. /// @@ -38,18 +38,17 @@ impl From for super::ReplyableError { fn from(e: InternalRequestError) -> Self { super::ReplyableError::V1(e.into()) } } -impl JsonError for RequestError { - fn to_json(&self) -> String { +impl From<&RequestError> for JsonReply { + fn from(e: &RequestError) -> Self { use InternalRequestError::*; use crate::error_codes::ErrorCode::OriginalPsbtRejected; - use crate::receive::error::serialize_json_error; - match &self.0 { - Io(_) => serialize_json_error(OriginalPsbtRejected, self), - MissingHeader(_) => serialize_json_error(OriginalPsbtRejected, self), - InvalidContentType(_) => serialize_json_error(OriginalPsbtRejected, self), - InvalidContentLength(_) => serialize_json_error(OriginalPsbtRejected, self), - ContentLengthTooLarge(_) => serialize_json_error(OriginalPsbtRejected, self), + match &e.0 { + Io(_) => JsonReply::new(OriginalPsbtRejected, e), + MissingHeader(_) => JsonReply::new(OriginalPsbtRejected, e), + InvalidContentType(_) => JsonReply::new(OriginalPsbtRejected, e), + InvalidContentLength(_) => JsonReply::new(OriginalPsbtRejected, e), + ContentLengthTooLarge(_) => JsonReply::new(OriginalPsbtRejected, e), } } } diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 050d89aa1..4be28cf57 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -13,7 +13,7 @@ use url::Url; use super::error::{Error, InputContributionError}; use super::{ - v1, ImplementationError, InternalPayloadError, JsonError, OutputSubstitutionError, + v1, ImplementationError, InternalPayloadError, JsonReply, OutputSubstitutionError, ReplyableError, SelectionError, }; use crate::hpke::{decrypt_message_a, encrypt_message_b, HpkeKeyPair, HpkePublicKey}; @@ -286,7 +286,7 @@ impl UncheckedProposal { &mut self.context.ohttp_keys, "POST", subdir.as_str(), - Some(err.to_json().as_bytes()), + Some(JsonReply::from(err).to_json().as_bytes()), ) .map_err(InternalSessionError::OhttpEncapsulation)?; let req = Request::new_v2(&self.context.full_relay_url(ohttp_relay)?, &body); @@ -626,7 +626,7 @@ mod test { .err() .ok_or("expected error but got success")?; assert_eq!( - server_error.to_json(), + JsonReply::from(&server_error).to_json(), r#"{ "errorCode": "unavailable", "message": "Receiver error" }"# ); let (_req, _ctx) = proposal.clone().extract_err_req(&server_error, &*EXAMPLE_URL)?; From 66a52cbc074189cb4dec52d9642416351a1b292e Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 27 Mar 2025 11:34:16 -0400 Subject: [PATCH 3/6] Use strong json Value types with serde_json Hand rolling strings is simple, but serde_json is already a dependency and is more robust. --- payjoin/src/receive/error.rs | 40 ++++++++++++++++------------------- payjoin/src/receive/v2/mod.rs | 12 ++++++----- payjoin/src/send/error.rs | 28 ++++++++++++------------ 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 1e4bb9573..50b444a52 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -77,28 +77,30 @@ pub struct JsonReply { error_code: ErrorCode, /// The error message to be displayed only in debug logs message: String, - /// Additional fields to be added to the JSON - additional_fields: Vec, + /// Additional fields to be included in the JSON response + extra: serde_json::Map, } impl JsonReply { /// Create a new Reply pub fn new(error_code: ErrorCode, message: impl fmt::Display) -> Self { - Self { error_code, message: message.to_string(), additional_fields: vec![] } + Self { error_code, message: message.to_string(), extra: serde_json::Map::new() } + } + + /// Add an additional field to the JSON response + pub fn with_extra(mut self, key: &str, value: impl Into) -> Self { + self.extra.insert(key.to_string(), value.into()); + self } /// Serialize the Reply to a JSON string - pub fn to_json(&self) -> String { - if self.additional_fields.is_empty() { - format!(r#"{{ "errorCode": "{}", "message": "{}" }}"#, self.error_code, self.message) - } else { - format!( - r#"{{ "errorCode": "{}", "message": "{}", {} }}"#, - self.error_code, - self.message, - self.additional_fields.join(", ") - ) - } + pub fn to_json(&self) -> serde_json::Value { + let mut map = serde_json::Map::new(); + map.insert("errorCode".to_string(), self.error_code.to_string().into()); + map.insert("message".to_string(), self.message.clone().into()); + map.extend(self.extra.clone()); + + serde_json::Value::Object(map) } } @@ -205,14 +207,8 @@ impl From<&PayloadError> for JsonReply { super::optional_parameters::Error::UnknownVersion { supported_versions } => { let supported_versions_json = serde_json::to_string(supported_versions).unwrap_or_default(); - JsonReply { - error_code: VersionUnsupported, - message: "This version of payjoin is not supported.".to_string(), - additional_fields: vec![format!( - r#""supported": {}"#, - supported_versions_json - )], - } + JsonReply::new(VersionUnsupported, "This version of payjoin is not supported.") + .with_extra("supported", supported_versions_json) } _ => JsonReply::new(OriginalPsbtRejected, e), }, diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 4be28cf57..e640b8df4 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -286,7 +286,7 @@ impl UncheckedProposal { &mut self.context.ohttp_keys, "POST", subdir.as_str(), - Some(JsonReply::from(err).to_json().as_bytes()), + Some(JsonReply::from(err).to_json().to_string().as_bytes()), ) .map_err(InternalSessionError::OhttpEncapsulation)?; let req = Request::new_v2(&self.context.full_relay_url(ohttp_relay)?, &body); @@ -625,10 +625,12 @@ mod test { .check_broadcast_suitability(None, |_| Err("mock error".into())) .err() .ok_or("expected error but got success")?; - assert_eq!( - JsonReply::from(&server_error).to_json(), - r#"{ "errorCode": "unavailable", "message": "Receiver error" }"# - ); + let expected_json = serde_json::json!({ + "errorCode": "unavailable", + "message": "Receiver error" + }); + let actual_json = JsonReply::from(&server_error).to_json(); + assert_eq!(actual_json, expected_json); let (_req, _ctx) = proposal.clone().extract_err_req(&server_error, &*EXAMPLE_URL)?; let internal_error = InternalPayloadError::MissingPayment.into(); diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 0136374a9..27099f1c8 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -344,20 +344,22 @@ impl Display for ResponseError { impl fmt::Debug for ResponseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Self::WellKnown(e) => write!( - f, - r#"Well known error: {{ "errorCode": "{}", - "message": "{}" }}"#, - e.error_code(), - e.message() - ), + Self::WellKnown(e) => { + let json = serde_json::json!({ + "errorCode": e.error_code().to_string(), + "message": e.message() + }); + write!(f, "Well known error: {}", json) + } Self::Validation(e) => write!(f, "Validation({:?})", e), - Self::Unrecognized { error_code, message } => write!( - f, - r#"Unrecognized error: {{ "errorCode": "{}", "message": "{}" }}"#, - error_code, message - ), + Self::Unrecognized { error_code, message } => { + let json = serde_json::json!({ + "errorCode": error_code, + "message": message + }); + write!(f, "Unrecognized error: {}", json) + } } } } @@ -403,7 +405,7 @@ impl Display for WellKnownError { #[cfg(test)] mod tests { - use bitcoind::bitcoincore_rpc::jsonrpc::serde_json::json; + use serde_json::json; use super::*; From c8dd1024f6f13f6d05b7295c1e9e2da401b1b041 Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 27 Mar 2025 12:25:22 -0400 Subject: [PATCH 4/6] DRY up JsonReply for legibility --- payjoin/src/receive/error.rs | 27 +++++++++++++---------- payjoin/src/receive/v1/exclusive/error.rs | 12 +++++----- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 50b444a52..26f9c7431 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -201,8 +201,19 @@ impl From<&PayloadError> for JsonReply { use InternalPayloadError::*; match &e.0 { - Utf8(_) => JsonReply::new(OriginalPsbtRejected, e), - ParsePsbt(_) => JsonReply::new(OriginalPsbtRejected, e), + Utf8(_) + | ParsePsbt(_) + | InconsistentPsbt(_) + | PrevTxOut(_) + | MissingPayment + | OriginalPsbtNotBroadcastable + | InputOwned(_) + | InputWeight(_) + | InputSeen(_) + | PsbtBelowFeeRate(_, _) => JsonReply::new(OriginalPsbtRejected, e), + + FeeTooHigh(_, _) => JsonReply::new(NotEnoughMoney, e), + SenderParams(e) => match e { super::optional_parameters::Error::UnknownVersion { supported_versions } => { let supported_versions_json = @@ -210,17 +221,9 @@ impl From<&PayloadError> for JsonReply { JsonReply::new(VersionUnsupported, "This version of payjoin is not supported.") .with_extra("supported", supported_versions_json) } - _ => JsonReply::new(OriginalPsbtRejected, e), + super::optional_parameters::Error::FeeRate => + JsonReply::new(OriginalPsbtRejected, e), }, - InconsistentPsbt(_) => JsonReply::new(OriginalPsbtRejected, e), - PrevTxOut(_) => JsonReply::new(OriginalPsbtRejected, e), - MissingPayment => JsonReply::new(OriginalPsbtRejected, e), - OriginalPsbtNotBroadcastable => JsonReply::new(OriginalPsbtRejected, e), - InputOwned(_) => JsonReply::new(OriginalPsbtRejected, e), - InputWeight(_) => JsonReply::new(OriginalPsbtRejected, e), - InputSeen(_) => JsonReply::new(OriginalPsbtRejected, e), - PsbtBelowFeeRate(_, _) => JsonReply::new(OriginalPsbtRejected, e), - FeeTooHigh(_, _) => JsonReply::new(NotEnoughMoney, e), } } } diff --git a/payjoin/src/receive/v1/exclusive/error.rs b/payjoin/src/receive/v1/exclusive/error.rs index 3e3bc65ad..f6e682c14 100644 --- a/payjoin/src/receive/v1/exclusive/error.rs +++ b/payjoin/src/receive/v1/exclusive/error.rs @@ -42,13 +42,13 @@ impl From<&RequestError> for JsonReply { fn from(e: &RequestError) -> Self { use InternalRequestError::*; - use crate::error_codes::ErrorCode::OriginalPsbtRejected; match &e.0 { - Io(_) => JsonReply::new(OriginalPsbtRejected, e), - MissingHeader(_) => JsonReply::new(OriginalPsbtRejected, e), - InvalidContentType(_) => JsonReply::new(OriginalPsbtRejected, e), - InvalidContentLength(_) => JsonReply::new(OriginalPsbtRejected, e), - ContentLengthTooLarge(_) => JsonReply::new(OriginalPsbtRejected, e), + Io(_) + | MissingHeader(_) + | InvalidContentType(_) + | InvalidContentLength(_) + | ContentLengthTooLarge(_) => + JsonReply::new(crate::error_codes::ErrorCode::OriginalPsbtRejected, e), } } } From 28f41b35009e44cc2bfe4ae88990928a7776f0fa Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 27 Mar 2025 12:26:02 -0400 Subject: [PATCH 5/6] Take JsonReply in extract_err_req This simple type can more easily cross foreign language boundaries. Fix #605 --- payjoin-cli/src/app/v2.rs | 5 +++-- payjoin/src/receive/error.rs | 15 +++++++------ payjoin/src/receive/v1/exclusive/error.rs | 4 ++-- payjoin/src/receive/v2/mod.rs | 27 ++++++++++++++--------- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index bfc49ace3..e716a41c2 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -287,7 +287,8 @@ async fn handle_recoverable_error( mut receiver: UncheckedProposal, ohttp_relay: &payjoin::Url, ) -> anyhow::Error { - let (err_req, err_ctx) = match receiver.extract_err_req(&e, ohttp_relay) { + let to_return = anyhow!("Replied with error: {}", e); + let (err_req, err_ctx) = match receiver.extract_err_req(&e.into(), ohttp_relay) { Ok(req_ctx) => req_ctx, Err(e) => return anyhow!("Failed to extract error request: {}", e), }; @@ -306,7 +307,7 @@ async fn handle_recoverable_error( return anyhow!("Failed to process error response: {}", e); } - e.into() + to_return } fn try_contributing_inputs( diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 26f9c7431..a508ae25b 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -104,13 +104,14 @@ impl JsonReply { } } -impl From<&ReplyableError> for JsonReply { - fn from(e: &ReplyableError) -> Self { +impl From for JsonReply { + fn from(e: ReplyableError) -> Self { + use ReplyableError::*; match e { - ReplyableError::Payload(e) => e.into(), + Payload(e) => e.into(), #[cfg(feature = "v1")] - ReplyableError::V1(e) => e.into(), - ReplyableError::Implementation(_) => JsonReply::new(Unavailable, "Receiver error"), + V1(e) => e.into(), + Implementation(_) => JsonReply::new(Unavailable, "Receiver error"), } } } @@ -196,8 +197,8 @@ pub(crate) enum InternalPayloadError { FeeTooHigh(bitcoin::FeeRate, bitcoin::FeeRate), } -impl From<&PayloadError> for JsonReply { - fn from(e: &PayloadError) -> Self { +impl From for JsonReply { + fn from(e: PayloadError) -> Self { use InternalPayloadError::*; match &e.0 { diff --git a/payjoin/src/receive/v1/exclusive/error.rs b/payjoin/src/receive/v1/exclusive/error.rs index f6e682c14..77cba76fe 100644 --- a/payjoin/src/receive/v1/exclusive/error.rs +++ b/payjoin/src/receive/v1/exclusive/error.rs @@ -38,8 +38,8 @@ impl From for super::ReplyableError { fn from(e: InternalRequestError) -> Self { super::ReplyableError::V1(e.into()) } } -impl From<&RequestError> for JsonReply { - fn from(e: &RequestError) -> Self { +impl From for JsonReply { + fn from(e: RequestError) -> Self { use InternalRequestError::*; match &e.0 { diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index e640b8df4..6544e6004 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -278,7 +278,7 @@ impl UncheckedProposal { /// a Receiver Error Response pub fn extract_err_req( &mut self, - err: &ReplyableError, + err: &JsonReply, ohttp_relay: impl IntoUrl, ) -> Result<(Request, ohttp::ClientResponse), SessionError> { let subdir = subdir(&self.context.directory, &id(&self.context.s)); @@ -286,7 +286,7 @@ impl UncheckedProposal { &mut self.context.ohttp_keys, "POST", subdir.as_str(), - Some(JsonReply::from(err).to_json().to_string().as_bytes()), + Some(err.to_json().to_string().as_bytes()), ) .map_err(InternalSessionError::OhttpEncapsulation)?; let req = Request::new_v2(&self.context.full_relay_url(ohttp_relay)?, &body); @@ -620,21 +620,26 @@ mod test { context: SHARED_CONTEXT.clone(), }; - let server_error = proposal - .clone() - .check_broadcast_suitability(None, |_| Err("mock error".into())) - .err() - .ok_or("expected error but got success")?; + let server_error = || { + proposal + .clone() + .check_broadcast_suitability(None, |_| Err("mock error".into())) + .expect_err("expected broadcast suitability check to fail") + }; + let expected_json = serde_json::json!({ "errorCode": "unavailable", "message": "Receiver error" }); - let actual_json = JsonReply::from(&server_error).to_json(); + + let actual_json = JsonReply::from(server_error()).to_json().clone(); assert_eq!(actual_json, expected_json); - let (_req, _ctx) = proposal.clone().extract_err_req(&server_error, &*EXAMPLE_URL)?; - let internal_error = InternalPayloadError::MissingPayment.into(); - let (_req, _ctx) = proposal.extract_err_req(&internal_error, &*EXAMPLE_URL)?; + let (_req, _ctx) = + proposal.clone().extract_err_req(&server_error().into(), &*EXAMPLE_URL)?; + + let internal_error: ReplyableError = InternalPayloadError::MissingPayment.into(); + let (_req, _ctx) = proposal.extract_err_req(&internal_error.into(), &*EXAMPLE_URL)?; Ok(()) } From 2041f54efb2558c8565266e5dd07847939439217 Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 27 Mar 2025 15:15:50 -0400 Subject: [PATCH 6/6] Refactor send::WellKnownError to use ErrorCode The types are directly related and the send/receive separation is no longer part of our feature distinctions so it makes sense to combine them. --- payjoin/src/send/error.rs | 92 +++++++++++++++++---------------------- payjoin/src/send/v1.rs | 6 ++- 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 27099f1c8..4e4290c7b 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -266,22 +266,18 @@ pub enum ResponseError { } impl ResponseError { - fn from_json(json: serde_json::Value) -> Self { - // we try to find the errorCode field and - // if it exists we try to parse it as a well known error - // if its an unknown error we return the error code and message - // from original response - // if errorCode field doesn't exist we return parse error + pub(crate) fn from_json(json: serde_json::Value) -> Self { let message = json .as_object() .and_then(|v| v.get("message")) .and_then(|v| v.as_str()) .unwrap_or_default() .to_string(); - if let Some(error_code) = - json.as_object().and_then(|v| v.get("errorCode")).and_then(|v| v.as_str()) - { - match ErrorCode::from_str(error_code) { + + let error_code = json.as_object().and_then(|v| v.get("errorCode")).and_then(|v| v.as_str()); + + match error_code { + Some(code) => match ErrorCode::from_str(code) { Ok(ErrorCode::VersionUnsupported) => { let supported = json .as_object() @@ -289,23 +285,19 @@ impl ResponseError { .and_then(|v| v.as_array()) .map(|array| array.iter().filter_map(|v| v.as_u64()).collect::>()) .unwrap_or_default(); - WellKnownError::VersionUnsupported { message, supported }.into() + WellKnownError::version_unsupported(message, supported).into() } - Ok(ErrorCode::Unavailable) => WellKnownError::Unavailable(message).into(), - Ok(ErrorCode::NotEnoughMoney) => WellKnownError::NotEnoughMoney(message).into(), - Ok(ErrorCode::OriginalPsbtRejected) => - WellKnownError::OriginalPsbtRejected(message).into(), - _ => Self::Unrecognized { error_code: error_code.to_string(), message }, - } - } else { - InternalValidationError::Parse.into() + Ok(code) => WellKnownError::new(code, message).into(), + Err(_) => Self::Unrecognized { error_code: code.to_string(), message }, + }, + None => InternalValidationError::Parse.into(), } } /// Parse a response from the receiver. /// /// response must be valid JSON string. - pub fn parse(response: &str) -> Self { + pub(crate) fn parse(response: &str) -> Self { match serde_json::from_str(response) { Ok(json) => Self::from_json(json), Err(_) => InternalValidationError::Parse.into(), @@ -346,8 +338,8 @@ impl fmt::Debug for ResponseError { match self { Self::WellKnown(e) => { let json = serde_json::json!({ - "errorCode": e.error_code().to_string(), - "message": e.message() + "errorCode": e.code.to_string(), + "message": e.message }); write!(f, "Well known error: {}", json) } @@ -364,41 +356,39 @@ impl fmt::Debug for ResponseError { } } +/// A well-known error that can be safely displayed to end users. #[derive(Debug, Clone, PartialEq, Eq)] -#[non_exhaustive] -pub enum WellKnownError { - Unavailable(String), - NotEnoughMoney(String), - VersionUnsupported { message: String, supported: Vec }, - OriginalPsbtRejected(String), +pub struct WellKnownError { + pub(crate) code: ErrorCode, + pub(crate) message: String, + pub(crate) supported_versions: Option>, } impl WellKnownError { - pub fn error_code(&self) -> ErrorCode { - match self { - WellKnownError::Unavailable(_) => ErrorCode::Unavailable, - WellKnownError::NotEnoughMoney(_) => ErrorCode::NotEnoughMoney, - WellKnownError::VersionUnsupported { .. } => ErrorCode::VersionUnsupported, - WellKnownError::OriginalPsbtRejected(_) => ErrorCode::OriginalPsbtRejected, - } + /// Create a new well-known error with the given code and message. + pub(crate) fn new(code: ErrorCode, message: String) -> Self { + Self { code, message, supported_versions: None } } - pub fn message(&self) -> &str { - match self { - WellKnownError::Unavailable(m) => m, - WellKnownError::NotEnoughMoney(m) => m, - WellKnownError::VersionUnsupported { message: m, .. } => m, - WellKnownError::OriginalPsbtRejected(m) => m, - } + + /// Create a version unsupported error with the given message and supported versions. + pub(crate) fn version_unsupported(message: String, supported: Vec) -> Self { + Self { code: ErrorCode::VersionUnsupported, message, supported_versions: Some(supported) } } } -impl Display for WellKnownError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Self::Unavailable(_) => write!(f, "The payjoin endpoint is not available for now."), - Self::NotEnoughMoney(_) => write!(f, "The receiver added some inputs but could not bump the fee of the payjoin proposal."), - Self::VersionUnsupported { supported: v, .. }=> write!(f, "This version of payjoin is not supported. Use version {:?}.", v), - Self::OriginalPsbtRejected(_) => write!(f, "The receiver rejected the original PSBT."), +impl core::fmt::Display for WellKnownError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self.code { + ErrorCode::Unavailable => write!(f, "The payjoin endpoint is not available for now."), + ErrorCode::NotEnoughMoney => write!(f, "The receiver added some inputs but could not bump the fee of the payjoin proposal."), + ErrorCode::VersionUnsupported => { + if let Some(supported) = &self.supported_versions { + write!(f, "This version of payjoin is not supported. Use version {:?}.", supported) + } else { + write!(f, "This version of payjoin is not supported.") + } + } + ErrorCode::OriginalPsbtRejected => write!(f, "The receiver rejected the original PSBT."), } } } @@ -414,8 +404,8 @@ mod tests { let known_str_error = r#"{"errorCode":"version-unsupported", "message":"custom message here", "supported": [1, 2]}"#; match ResponseError::parse(known_str_error) { ResponseError::WellKnown(e) => { - assert_eq!(e.error_code(), ErrorCode::VersionUnsupported); - assert_eq!(e.message(), "custom message here"); + assert_eq!(e.code, ErrorCode::VersionUnsupported); + assert_eq!(e.message, "custom message here"); assert_eq!( e.to_string(), "This version of payjoin is not supported. Use version [1, 2]." diff --git a/payjoin/src/send/v1.rs b/payjoin/src/send/v1.rs index 5a763b72b..958ac4dd2 100644 --- a/payjoin/src/send/v1.rs +++ b/payjoin/src/send/v1.rs @@ -286,6 +286,7 @@ mod test { use payjoin_test_utils::{BoxError, PARSED_ORIGINAL_PSBT}; use super::SenderBuilder; + use crate::error_codes::ErrorCode; use crate::send::error::{ResponseError, WellKnownError}; use crate::send::test::create_psbt_context; use crate::{Uri, UriExt}; @@ -322,7 +323,10 @@ mod test { }) .to_string(); match ctx.process_response(&mut known_json_error.as_bytes()) { - Err(ResponseError::WellKnown(WellKnownError::VersionUnsupported { .. })) => (), + Err(ResponseError::WellKnown(WellKnownError { + code: ErrorCode::VersionUnsupported, + .. + })) => (), _ => panic!("Expected WellKnownError"), }