From 2fbc22c9acea0132ab38b616ebdc0740346e662e Mon Sep 17 00:00:00 2001 From: DanGould Date: Tue, 17 Oct 2023 18:47:14 -0400 Subject: [PATCH 1/3] Parse receiver response errors --- payjoin-cli/Cargo.lock | 66 +++++++++------------ payjoin-cli/src/app.rs | 9 ++- payjoin/Cargo.toml | 2 + payjoin/src/receive/error.rs | 2 +- payjoin/src/send/error.rs | 107 ++++++++++++++++++++++++++++++++++- payjoin/src/send/mod.rs | 15 +++-- 6 files changed, 151 insertions(+), 50 deletions(-) diff --git a/payjoin-cli/Cargo.lock b/payjoin-cli/Cargo.lock index 7bf944f4..f83fa5e1 100644 --- a/payjoin-cli/Cargo.lock +++ b/payjoin-cli/Cargo.lock @@ -36,9 +36,9 @@ dependencies = [ [[package]] name = "aho-corasick" -version = "1.0.2" +version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43f6cb1bf222025340178f382c426f13757b2960e89779dfcb319c32542a5a41" +checksum = "b2969dcb958b36655471fc61f7e416fa76033bdd4bfed0678d8fee1e2d07a1f0" dependencies = [ "memchr", ] @@ -75,30 +75,29 @@ dependencies = [ [[package]] name = "anstream" -version = "0.3.2" +version = "0.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ca84f3628370c59db74ee214b3263d58f9aadd9b4fe7e711fd87dc452b7f163" +checksum = "2ab91ebe16eb252986481c5b62f6098f3b698a45e34b5b98200cf20dd2484a44" dependencies = [ "anstyle", "anstyle-parse", "anstyle-query", "anstyle-wincon", "colorchoice", - "is-terminal", "utf8parse", ] [[package]] name = "anstyle" -version = "1.0.1" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a30da5c5f2d5e72842e00bcb57657162cdabef0931f40e2deb9b4140440cecd" +checksum = "7079075b41f533b8c61d2a4d073c4676e1f8b249ff94a393b0595db304e0dd87" [[package]] name = "anstyle-parse" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "938874ff5980b03a87c5524b3ae5b59cf99b1d6bc836848df7bc5ada9643c333" +checksum = "317b9a89c1868f5ea6ff1d9539a69f45dffc21ce321ac1fd1160dfa48c8e2140" dependencies = [ "utf8parse", ] @@ -114,9 +113,9 @@ dependencies = [ [[package]] name = "anstyle-wincon" -version = "1.0.1" +version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "180abfa45703aebe0093f79badacc01b8fd4ea2e35118747e5811127f926e188" +checksum = "f0699d10d2f4d628a98ee7b57b289abbc98ff3bad977cb3152709d4bf2330628" dependencies = [ "anstyle", "windows-sys", @@ -361,18 +360,18 @@ checksum = "cca491388666e04d7248af3f60f0c40cfb0991c72205595d7c396e3510207d1a" [[package]] name = "clap" -version = "4.3.19" +version = "4.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fd304a20bff958a57f04c4e96a2e7594cc4490a0e809cbd48bb6437edaa452d" +checksum = "d04704f56c2cde07f43e8e2c154b43f216dc5c92fc98ada720177362f953b956" dependencies = [ "clap_builder", ] [[package]] name = "clap_builder" -version = "4.3.19" +version = "4.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01c6a3f08f1fe5662a35cfe393aec09c4df95f60ee93b7556505260f75eee9e1" +checksum = "0e231faeaca65ebd1ea3c737966bf858971cd38c3849107aa3ea7de90a804e45" dependencies = [ "anstream", "anstyle", @@ -382,9 +381,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "0.5.0" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2da6da31387c7e4ef160ffab6d5e7f00c42626fe39aea70a7b0f1773f7dd6c1b" +checksum = "cd7cc57abe963c6d3b9d8be5b06ba7c8957a930305ca90304f24ef040aa6f961" [[package]] name = "colorchoice" @@ -837,17 +836,6 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28b29a3cd74f0f4598934efe3aeba42bae0eb4680554128851ebbecb02af14e6" -[[package]] -name = "is-terminal" -version = "0.4.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" -dependencies = [ - "hermit-abi 0.3.2", - "rustix", - "windows-sys", -] - [[package]] name = "itoa" version = "1.0.9" @@ -1130,6 +1118,8 @@ dependencies = [ "bitcoin", "log", "rand", + "serde", + "serde_json", "url", ] @@ -1153,9 +1143,9 @@ dependencies = [ [[package]] name = "pem" -version = "2.0.1" +version = "3.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b13fe415cdf3c8e44518e18a7c95a13431d9bdf6d15367d82b23c377fdd441a" +checksum = "3163d2912b7c3b52d651a055f2c7eec9ba5cd22d26ef75b8dd3a59980b185923" dependencies = [ "base64 0.21.2", "serde", @@ -1297,9 +1287,9 @@ dependencies = [ [[package]] name = "rcgen" -version = "0.11.1" +version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4954fbc00dcd4d8282c987710e50ba513d351400dbdd00e803a05172a90d8976" +checksum = "52c4f3084aa3bc7dfbba4eff4fab2a54db4324965d8872ab933565e6fbd83bc6" dependencies = [ "pem", "ring", @@ -1318,9 +1308,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.9.3" +version = "1.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81bc1d4caf89fac26a70747fe603c130093b53c773888797a6329091246d651a" +checksum = "12de2eff854e5fa4b1295edd650e227e9d8fb0c9e90b12e7f36d6a6811791a29" dependencies = [ "aho-corasick", "memchr", @@ -1330,9 +1320,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fed1ceff11a1dddaee50c9dc8e4938bd106e9d89ae372f192311e7da498e3b69" +checksum = "49530408a136e16e5b486e883fbb6ba058e8e4e8ae6621a77b048b314336e629" dependencies = [ "aho-corasick", "memchr", @@ -1341,9 +1331,9 @@ dependencies = [ [[package]] name = "regex-syntax" -version = "0.7.4" +version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5ea92a5b6195c6ef2a0295ea818b312502c6fc94dde986c5553242e18fd4ce2" +checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" [[package]] name = "reqwest" diff --git a/payjoin-cli/src/app.rs b/payjoin-cli/src/app.rs index dd786c37..6ed2900e 100644 --- a/payjoin-cli/src/app.rs +++ b/payjoin-cli/src/app.rs @@ -107,9 +107,12 @@ impl App { .header("Content-Type", "text/plain") .send() .with_context(|| "HTTP request failed")?; - // TODO display well-known errors and log::debug the rest - let psbt = - ctx.process_response(&mut response).with_context(|| "Failed to process response")?; + let psbt = ctx.process_response(&mut response).or_else(|e| { + // It is imperative to carefully display pre-defined messages to end users and the details in debug, otherwise receiver could craft a message to phish the sender. + log::error!("Response contains error: {}", e); + log::debug!("Response contains error: {:?}", e); + Err(e) + })?; log::debug!("Proposed psbt: {:#?}", psbt); let psbt = self .bitcoind diff --git a/payjoin/Cargo.toml b/payjoin/Cargo.toml index 254459ce..7f5ac3f1 100644 --- a/payjoin/Cargo.toml +++ b/payjoin/Cargo.toml @@ -22,6 +22,8 @@ bitcoin = { version = "0.30.0", features = ["base64"] } bip21 = "0.3.1" log = { version = "0.4.14"} rand = { version = "0.8.4", optional = true } +serde = { version = "1.0.107", features = ["derive"] } +serde_json = "1.0" url = "2.2.2" [dev-dependencies] diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index ffc91360..d481cbe8 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -13,7 +13,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { Self::BadRequest(e) => e.fmt(f), - Self::Server(e) => write!(f, "Internal Server Error: {}", e), + Self::Server(_) => crate::response_error::WellKnownError::Unavailable.fmt(f), } } } diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 1c7cdea5..0437d8fd 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -16,7 +16,7 @@ pub struct ValidationError { #[derive(Debug)] pub(crate) enum InternalValidationError { - Psbt(bitcoin::psbt::PsbtParseError), + Parse, Io(std::io::Error), InvalidInputType(InputTypeError), InvalidProposedInput(crate::psbt::PrevTxOutError), @@ -58,7 +58,6 @@ impl fmt::Display for ValidationError { use InternalValidationError::*; match &self.internal { - Psbt(e) => write!(f, "couldn't decode PSBT: {}", e), Io(e) => write!(f, "couldn't read PSBT: {}", e), InvalidInputType(e) => write!(f, "invalid transaction input type: {}", e), InvalidProposedInput(e) => write!(f, "invalid proposed transaction input: {}", e), @@ -86,6 +85,7 @@ impl fmt::Display for ValidationError { PayeeTookContributedFee => write!(f, "payee tried to take fee contribution for himself"), FeeContributionPaysOutputSizeIncrease => write!(f, "fee contribution pays for additional outputs"), FeeRateBelowMinimum => write!(f, "the fee rate of proposed transaction is below minimum"), + Parse => write!(f, "couldn't decode as PSBT or JSON",), } } } @@ -95,7 +95,6 @@ impl std::error::Error for ValidationError { use InternalValidationError::*; match &self.internal { - Psbt(error) => Some(error), Io(error) => Some(error), InvalidInputType(error) => Some(error), InvalidProposedInput(error) => Some(error), @@ -123,6 +122,7 @@ impl std::error::Error for ValidationError { PayeeTookContributedFee => None, FeeContributionPaysOutputSizeIncrease => None, FeeRateBelowMinimum => None, + Parse => None, } } } @@ -210,3 +210,104 @@ impl std::error::Error for CreateRequestError { impl From for CreateRequestError { fn from(value: InternalCreateRequestError) -> Self { CreateRequestError(value) } } + +use serde::Deserialize; + +pub enum ResponseError { + // Well known errors with internal message for logs as String + WellKnown(WellKnownError, String), + // Don't display unknowns to end users, only debug logs + Unrecognized(String, String), + + Validation(ValidationError), +} + +impl From for ResponseError { + fn from(value: InternalValidationError) -> Self { + Self::Validation(ValidationError { internal: value }) + } +} + +// It is imperative to carefully display pre-defined messages to end users and the details in debug. +impl fmt::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(e) => write!(f, "The receiver sent an invalid response: {}", e), + } + } +} + +impl fmt::Debug for ResponseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::WellKnown(e, msg) => + write!(f, r#"Well known error: {{ "errorCode": "{}", "message": "{}" }}"#, e, msg), + Self::Unrecognized(code, msg) => write!( + f, + r#"Unrecognized error: {{ "errorCode": "{}", "message": "{}" }}"#, + code, msg + ), + Self::Validation(e) => write!(f, "Validation({:?})", e), + } + } +} + +impl<'de> Deserialize<'de> for ResponseError { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let res = RawResponseError::deserialize(deserializer)?; + + let well_known_error = WellKnownError::from_str(&res.error_code); + + if let Some(wk_error) = well_known_error { + Ok(ResponseError::WellKnown(wk_error, res.message)) + } else { + Ok(ResponseError::Unrecognized(res.error_code, res.message)) + } + } +} + +impl std::error::Error for ResponseError {} + +#[derive(Debug, Deserialize)] +struct RawResponseError { + #[serde(rename = "errorCode")] + error_code: String, + message: String, +} + +#[derive(Debug)] +pub enum WellKnownError { + Unavailable, + NotEnoughMoney, + VersionUnsupported, + OriginalPsbtRejected, +} + +impl WellKnownError { + pub fn from_str(s: &str) -> Option { + match s { + "unavailable" => Some(WellKnownError::Unavailable), + "not-enough-money" => Some(WellKnownError::NotEnoughMoney), + "version-unsupported" => Some(WellKnownError::VersionUnsupported), + "original-psbt-rejected" => Some(WellKnownError::OriginalPsbtRejected), + _ => None, + } + } +} + +impl fmt::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 => write!(f, "This version of payjoin is not supported."), + Self::OriginalPsbtRejected => write!(f, "The receiver rejected the original PSBT."), + } + } +} diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 411ac002..68592581 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -145,7 +145,7 @@ use std::str::FromStr; use bitcoin::psbt::Psbt; use bitcoin::{FeeRate, Script, ScriptBuf, Sequence, TxOut, Weight}; -pub use error::{CreateRequestError, ValidationError}; +pub use error::{CreateRequestError, ResponseError, ValidationError}; pub(crate) use error::{InternalCreateRequestError, InternalValidationError}; use url::Url; @@ -351,12 +351,17 @@ impl Context { pub fn process_response( self, response: &mut impl std::io::Read, - ) -> Result { + ) -> 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(InternalValidationError::Psbt)?; - - // process in non-generic function + let proposal = Psbt::from_str(&res_str).or_else(|_| { + // That wasn't a valid PSBT. Maybe it's a valid error response? + serde_json::from_str::(&res_str) + // which isn't the Ok result, it's actually an error. + .map(|e| Err(e)) + // if not, we have an invalid response + .unwrap_or_else(|_| Err(InternalValidationError::Parse.into())) + })?; self.process_proposal(proposal).map(Into::into).map_err(Into::into) } From f535666af2ab54028f31d3b9b3a1893ea6d20e91 Mon Sep 17 00:00:00 2001 From: DanGould Date: Tue, 17 Oct 2023 20:02:21 -0400 Subject: [PATCH 2/3] Internal error returns Unavailable --- payjoin/src/receive/error.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index d481cbe8..4e52af72 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -13,7 +13,10 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { Self::BadRequest(e) => e.fmt(f), - Self::Server(_) => crate::response_error::WellKnownError::Unavailable.fmt(f), + Self::Server(_) => write!( + f, + r#"{{ "errorCode": "unavailable", "message": "The payjoin endpoint is not available for now." }}"# + ), } } } From 210ca401567f73f19024478cb8ae7843d349afb0 Mon Sep 17 00:00:00 2001 From: DanGould Date: Fri, 20 Oct 2023 14:53:36 -0400 Subject: [PATCH 3/3] [draft] tinyjson --- payjoin-cli/Cargo.lock | 9 +++++-- payjoin/Cargo.toml | 3 +-- payjoin/src/send/error.rs | 50 +++++++++++++++++++-------------------- payjoin/src/send/mod.rs | 8 ++----- 4 files changed, 34 insertions(+), 36 deletions(-) diff --git a/payjoin-cli/Cargo.lock b/payjoin-cli/Cargo.lock index f83fa5e1..05078967 100644 --- a/payjoin-cli/Cargo.lock +++ b/payjoin-cli/Cargo.lock @@ -1118,8 +1118,7 @@ dependencies = [ "bitcoin", "log", "rand", - "serde", - "serde_json", + "tinyjson", "url", ] @@ -1702,6 +1701,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "tinyjson" +version = "2.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ab95735ea2c8fd51154d01e39cf13912a78071c2d89abc49a7ef102a7dd725a" + [[package]] name = "tinyvec" version = "1.6.0" diff --git a/payjoin/Cargo.toml b/payjoin/Cargo.toml index 7f5ac3f1..efe03e55 100644 --- a/payjoin/Cargo.toml +++ b/payjoin/Cargo.toml @@ -22,8 +22,7 @@ bitcoin = { version = "0.30.0", features = ["base64"] } bip21 = "0.3.1" log = { version = "0.4.14"} rand = { version = "0.8.4", optional = true } -serde = { version = "1.0.107", features = ["derive"] } -serde_json = "1.0" +tinyjson = "2" url = "2.2.2" [dev-dependencies] diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index 0437d8fd..c738a681 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -211,8 +211,6 @@ impl From for CreateRequestError { fn from(value: InternalCreateRequestError) -> Self { CreateRequestError(value) } } -use serde::Deserialize; - pub enum ResponseError { // Well known errors with internal message for logs as String WellKnown(WellKnownError, String), @@ -222,6 +220,30 @@ pub enum ResponseError { Validation(ValidationError), } +impl ResponseError { + pub fn from_json(json: &str) -> Self { + use std::convert::TryInto; + + use tinyjson::{JsonParser, JsonValue}; + + let parsed: JsonValue = json.parse().unwrap(); + //.unwrap_or_else( |_| ResponseError::Validation(InternalValidationError::Parse.into())); + let maybe_code = parsed["errorCode"].get(); + let maybe_message = parsed["message"].get(); + if let (Some(error_code), Some(message)) = (maybe_code, maybe_message) { + let well_known_error = WellKnownError::from_str(&error_code); + + if let Some(wk_error) = well_known_error { + ResponseError::WellKnown(wk_error, message.to_string()) + } else { + ResponseError::Unrecognized(error_code.to_string(), message.to_string()) + } + } else { + ResponseError::Validation(InternalValidationError::Parse.into()) + } + } +} + impl From for ResponseError { fn from(value: InternalValidationError) -> Self { Self::Validation(ValidationError { internal: value }) @@ -255,32 +277,8 @@ impl fmt::Debug for ResponseError { } } -impl<'de> Deserialize<'de> for ResponseError { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let res = RawResponseError::deserialize(deserializer)?; - - let well_known_error = WellKnownError::from_str(&res.error_code); - - if let Some(wk_error) = well_known_error { - Ok(ResponseError::WellKnown(wk_error, res.message)) - } else { - Ok(ResponseError::Unrecognized(res.error_code, res.message)) - } - } -} - impl std::error::Error for ResponseError {} -#[derive(Debug, Deserialize)] -struct RawResponseError { - #[serde(rename = "errorCode")] - error_code: String, - message: String, -} - #[derive(Debug)] pub enum WellKnownError { Unavailable, diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 68592581..07f2adf8 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -354,13 +354,9 @@ impl Context { ) -> 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).or_else(|_| { + let proposal = Psbt::from_str(&res_str).map_err(|_| { // That wasn't a valid PSBT. Maybe it's a valid error response? - serde_json::from_str::(&res_str) - // which isn't the Ok result, it's actually an error. - .map(|e| Err(e)) - // if not, we have an invalid response - .unwrap_or_else(|_| Err(InternalValidationError::Parse.into())) + ResponseError::from_json(&res_str) })?; self.process_proposal(proposal).map(Into::into).map_err(Into::into) }