Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supply & parse receiver response errors #120

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions payjoin-cli/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ impl App {
"Sent fallback transaction hex: {:#}",
payjoin::bitcoin::consensus::encode::serialize_hex(&fallback_tx)
);
let psbt = ctx
.process_response(&mut response.into_reader())
.map_err(|e| anyhow!("Failed to process response {}", e))?;
let psbt = ctx.process_response(&mut response.into_reader()).map_err(|e| {
log::debug!("Error processing response: {:?}", e);
anyhow!("Failed to process response {}", e)
})?;

self.process_pj_response(psbt)?;
Ok(())
Expand Down Expand Up @@ -293,7 +294,6 @@ impl App {
}

fn process_pj_response(&self, psbt: Psbt) -> Result<bitcoin::Txid> {
// TODO display well-known errors and log::debug the rest
log::debug!("Proposed psbt: {:#?}", psbt);
let psbt = self
.bitcoind()?
Expand Down
3 changes: 2 additions & 1 deletion payjoin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ bhttp = { version = "0.4.0", optional = true }
rand = { version = "0.8.4", optional = true }
serde = { version = "1.0.186", default-features = false, optional = true }
url = "2.2.2"
serde_json = "1.0.108"

[dev-dependencies]
bitcoind = { version = "0.31.1", features = ["0_21_2"] }
Expand All @@ -42,4 +43,4 @@ tokio = { version = "1.12.0", features = ["full"] }
ureq = "2.8.0"

[package.metadata.docs.rs]
features = ["send", "receive", "base64"]
features = ["send", "receive", "base64"]
196 changes: 192 additions & 4 deletions payjoin/src/send/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::fmt;
use std::fmt::{self, Display};

use bitcoin::locktime::absolute::LockTime;
use bitcoin::Sequence;
Expand All @@ -16,7 +16,7 @@ pub struct ValidationError {

#[derive(Debug)]
pub(crate) enum InternalValidationError {
PsbtParse(bitcoin::psbt::PsbtParseError),
Parse,
Io(std::io::Error),
InvalidInputType(InputTypeError),
InvalidProposedInput(crate::psbt::PrevTxOutError),
Expand Down Expand Up @@ -74,7 +74,7 @@ impl fmt::Display for ValidationError {
use InternalValidationError::*;

match &self.internal {
PsbtParse(e) => write!(f, "couldn't decode PSBT: {}", e),
Parse => write!(f, "couldn't decode as PSBT or JSON",),
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),
Expand Down Expand Up @@ -115,7 +115,7 @@ impl std::error::Error for ValidationError {
use InternalValidationError::*;

match &self.internal {
PsbtParse(error) => Some(error),
Parse => None,
Io(error) => Some(error),
InvalidInputType(error) => Some(error),
InvalidProposedInput(error) => Some(error),
Expand Down Expand Up @@ -235,3 +235,191 @@ impl std::error::Error for CreateRequestError {
impl From<InternalCreateRequestError> for CreateRequestError {
fn from(value: InternalCreateRequestError) -> Self { CreateRequestError(value) }
}

/// Represent an error returned by the receiver.
pub enum ResponseError {
/// `WellKnown` errors following the BIP78 spec
/// https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#user-content-Receivers_well_known_errors
/// These errors are displayed to end users.
///
/// The `WellKnownError` represents `errorCode` and `message`.
WellKnown(WellKnownError),
/// `Unrecognized` errors are errors that are not well known and are only displayed in debug logs.
/// They are not displayed to end users.
///
/// The first `String` is `errorCode`
/// The second `String` is `message`.
Unrecognized(String, String),
/// `Validation` errors are errors that are caused by malformed responses.
/// They are only displayed in debug logs.
Validation(ValidationError),
}

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
let message = json
.as_object()
.and_then(|v| v.get("message"))
.and_then(|v| v.as_str())
.ok_or(InternalValidationError::Parse)
.unwrap_or("");
match json
.as_object()
.and_then(|v| v.get("errorCode"))
.and_then(|v| v.as_str())
.ok_or(InternalValidationError::Parse)
{
Ok(str) =>
if let Ok(known_error) = WellKnownError::from_error_code(str, message.to_string()) {
return known_error.into();
} else {
return Self::Unrecognized(str.to_string(), message.to_string());
},
Err(e) => return e.into(),
}
}
/// Parse a response from the receiver.
///
/// response must be valid JSON string.
pub fn from_str(response: &str) -> Self {
if let Some(parsed) = serde_json::from_str(response).ok() {
Self::from_json(parsed)
} else {
InternalValidationError::Parse.into()
}
}
}

impl std::error::Error for ResponseError {}

impl From<WellKnownError> for ResponseError {
fn from(value: WellKnownError) -> Self { Self::WellKnown(value) }
}

impl From<InternalValidationError> 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 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) => write!(
f,
r#"Well known error: {{ "errorCode": "{}",
"message": "{}" }}"#,
e.error_code(),
e.message()
),
Self::Unrecognized(code, msg) => write!(
f,
r#"Unrecognized error: {{ "errorCode": "{}", "message": "{}" }}"#,
code, msg
),
Self::Validation(e) => write!(f, "Validation({:?})", e),
}
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum WellKnownError {
Unavailable(String),
NotEnoughMoney(String),
VersionUnsupported(String),
OriginalPsbtRejected(String),
}

impl WellKnownError {
pub fn error_code(&self) -> &str {
match self {
WellKnownError::Unavailable(_) => "unavailable",
WellKnownError::NotEnoughMoney(_) => "not-enough-money",
WellKnownError::VersionUnsupported(_) => "version-unsupported",
WellKnownError::OriginalPsbtRejected(_) => "original-psbt-rejected",
}
}
pub fn message(&self) -> &str {
match self {
WellKnownError::Unavailable(m) => m,
WellKnownError::NotEnoughMoney(m) => m,
WellKnownError::VersionUnsupported(m) => m,
WellKnownError::OriginalPsbtRejected(m) => m,
}
}
}

impl WellKnownError {
fn from_error_code(s: &str, message: String) -> Result<Self, ()> {
match s {
Copy link
Contributor

@DanGould DanGould Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this could shortcut return directly to an unrecognized ResponseError insted of Err(()). Just a note in case this gets refactored in the future.

"unavailable" => Ok(WellKnownError::Unavailable(message)),
"not-enough-money" => Ok(WellKnownError::NotEnoughMoney(message)),
"version-unsupported" => Ok(WellKnownError::VersionUnsupported(message)),
"original-psbt-rejected" => Ok(WellKnownError::OriginalPsbtRejected(message)),
_ => Err(()),
}
}
}

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(_) => write!(f, "This version of payjoin is not supported."),
Self::OriginalPsbtRejected(_) => write!(f, "The receiver rejected the original PSBT."),
}
}
}

#[cfg(test)]
mod tests {
use bitcoind::bitcoincore_rpc::jsonrpc::serde_json::json;

use super::*;

#[test]
fn test_parse_json() {
let known_str_error =
r#"{"errorCode":"version-unsupported", "message":"custom message here"}"#;
let error = ResponseError::from_str(known_str_error);
match error {
ResponseError::WellKnown(e) => {
assert_eq!(e.error_code(), "version-unsupported");
assert_eq!(e.message(), "custom message here");
assert_eq!(e.to_string(), "This version of payjoin is not supported.");
}
_ => panic!("Expected WellKnown error"),
};
let unrecognized_error = r#"{"errorCode":"random", "message":"random"}"#;
assert_eq!(
ResponseError::from_str(unrecognized_error).to_string(),
"The receiver sent an unrecognized error."
);
let invalid_json_error = json!({
"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: couldn't decode as PSBT or JSON"
);
}
}
60 changes: 47 additions & 13 deletions payjoin/src/send/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ use std::str::FromStr;
use bitcoin::address::NetworkChecked;
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};
#[cfg(feature = "v2")]
use serde::{
Expand Down Expand Up @@ -639,7 +639,7 @@ pub struct Request {
///
/// This type is used to process the response. Get it from [`RequestBuilder`](crate::send::RequestBuilder)'s build methods.
/// Then you only need to call [`.process_response()`](crate::send::Context::process_response()) on it to continue BIP78 flow.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct ContextV1 {
original_psbt: Psbt,
disable_output_substitution: bool,
Expand Down Expand Up @@ -712,12 +712,11 @@ impl ContextV1 {
pub fn process_response(
self,
response: &mut impl std::io::Read,
) -> Result<Psbt, ValidationError> {
) -> Result<Psbt, ResponseError> {
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::PsbtParse)?;

// process in non-generic function
let proposal =
Psbt::from_str(&res_str).or_else(|_| Err(ResponseError::from_str(&res_str)))?;
self.process_proposal(proposal).map(Into::into).map_err(Into::into)
}

Expand Down Expand Up @@ -1120,18 +1119,15 @@ fn serialize_url(
mod test {
const ORIGINAL_PSBT: &str = "cHNidP8BAHMCAAAAAY8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////AtyVuAUAAAAAF6kUHehJ8GnSdBUOOv6ujXLrWmsJRDCHgIQeAAAAAAAXqRR3QJbbz0hnQ8IvQ0fptGn+votneofTAAAAAAEBIKgb1wUAAAAAF6kU3k4ekGHKWRNbA1rV5tR5kEVDVNCHAQcXFgAUx4pFclNVgo1WWAdN1SYNX8tphTABCGsCRzBEAiB8Q+A6dep+Rz92vhy26lT0AjZn4PRLi8Bf9qoB/CMk0wIgP/Rj2PWZ3gEjUkTlhDRNAQ0gXwTO7t9n+V14pZ6oljUBIQMVmsAaoNWHVMS02LfTSe0e388LNitPa1UQZyOihY+FFgABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUAAA=";

#[test]
fn official_vectors() {
const PAYJOIN_PROPOSAL: &str = "cHNidP8BAJwCAAAAAo8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////jye60aAl3JgZdaIERvjkeh72VYZuTGH/ps2I4l0IO4MBAAAAAP7///8CJpW4BQAAAAAXqRQd6EnwadJ0FQ46/q6NcutaawlEMIcACT0AAAAAABepFHdAltvPSGdDwi9DR+m0af6+i2d6h9MAAAAAAQEgqBvXBQAAAAAXqRTeTh6QYcpZE1sDWtXm1HmQRUNU0IcBBBYAFMeKRXJTVYKNVlgHTdUmDV/LaYUwIgYDFZrAGqDVh1TEtNi300ntHt/PCzYrT2tVEGcjooWPhRYYSFzWUDEAAIABAACAAAAAgAEAAAAAAAAAAAEBIICEHgAAAAAAF6kUyPLL+cphRyyI5GTUazV0hF2R2NWHAQcXFgAUX4BmVeWSTJIEwtUb5TlPS/ntohABCGsCRzBEAiBnu3tA3yWlT0WBClsXXS9j69Bt+waCs9JcjWtNjtv7VgIge2VYAaBeLPDB6HGFlpqOENXMldsJezF9Gs5amvDQRDQBIQJl1jz1tBt8hNx2owTm+4Du4isx0pmdKNMNIjjaMHFfrQABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUIgICygvBWB5prpfx61y1HDAwo37kYP3YRJBvAjtunBAur3wYSFzWUDEAAIABAACAAAAAgAEAAAABAAAAAAA=";

fn create_v1_context() -> super::ContextV1 {
use std::str::FromStr;

use bitcoin::psbt::Psbt;
use bitcoin::FeeRate;

use crate::input_type::{InputType, SegWitV0Type};
use crate::psbt::PsbtExt;

let proposal = "cHNidP8BAJwCAAAAAo8nutGgJdyYGXWiBEb45Hoe9lWGbkxh/6bNiOJdCDuDAAAAAAD+////jye60aAl3JgZdaIERvjkeh72VYZuTGH/ps2I4l0IO4MBAAAAAP7///8CJpW4BQAAAAAXqRQd6EnwadJ0FQ46/q6NcutaawlEMIcACT0AAAAAABepFHdAltvPSGdDwi9DR+m0af6+i2d6h9MAAAAAAQEgqBvXBQAAAAAXqRTeTh6QYcpZE1sDWtXm1HmQRUNU0IcBBBYAFMeKRXJTVYKNVlgHTdUmDV/LaYUwIgYDFZrAGqDVh1TEtNi300ntHt/PCzYrT2tVEGcjooWPhRYYSFzWUDEAAIABAACAAAAAgAEAAAAAAAAAAAEBIICEHgAAAAAAF6kUyPLL+cphRyyI5GTUazV0hF2R2NWHAQcXFgAUX4BmVeWSTJIEwtUb5TlPS/ntohABCGsCRzBEAiBnu3tA3yWlT0WBClsXXS9j69Bt+waCs9JcjWtNjtv7VgIge2VYAaBeLPDB6HGFlpqOENXMldsJezF9Gs5amvDQRDQBIQJl1jz1tBt8hNx2owTm+4Du4isx0pmdKNMNIjjaMHFfrQABABYAFEb2Giu6c4KO5YW0pfw3lGp9jMUUIgICygvBWB5prpfx61y1HDAwo37kYP3YRJBvAjtunBAur3wYSFzWUDEAAIABAACAAAAAgAEAAAABAAAAAAA=";

let original_psbt = Psbt::from_str(ORIGINAL_PSBT).unwrap();
eprintln!("original: {:#?}", original_psbt);
let payee = original_psbt.unsigned_tx.output[1].script_pubkey.clone();
Expand All @@ -1145,7 +1141,21 @@ mod test {
input_type: InputType::SegWitV0 { ty: SegWitV0Type::Pubkey, nested: true },
sequence,
};
let mut proposal = Psbt::from_str(proposal).unwrap();
ctx
}

#[test]
fn official_vectors() {
use std::str::FromStr;

use bitcoin::psbt::Psbt;

use crate::psbt::PsbtExt;

let original_psbt = Psbt::from_str(ORIGINAL_PSBT).unwrap();
eprintln!("original: {:#?}", original_psbt);
let ctx = create_v1_context();
let mut proposal = Psbt::from_str(PAYJOIN_PROPOSAL).unwrap();
eprintln!("proposal: {:#?}", proposal);
for output in proposal.outputs_mut() {
output.bip32_derivation.clear();
Expand Down Expand Up @@ -1181,4 +1191,28 @@ mod test {
let deserialized = serde_json::from_str(&serialized).unwrap();
assert!(req_ctx == deserialized);
}

#[test]
fn handle_known_errors() {
let ctx = create_v1_context();
let known_json_error = serde_json::json!({
"errorCode": "version-unsupported",
"message": "This version of payjoin is not supported."
})
.to_string();
assert_eq!(
ctx.process_response(&mut known_json_error.as_bytes()).unwrap_err().to_string(),
"This version of payjoin is not supported."
);
let ctx = create_v1_context();
let invalid_json_error = serde_json::json!({
"err": "random",
"message": "This version of payjoin is not supported."
})
.to_string();
assert_eq!(
ctx.process_response(&mut invalid_json_error.as_bytes()).unwrap_err().to_string(),
"The receiver sent an invalid response: couldn't decode as PSBT or JSON"
)
Comment on lines +1213 to +1216
Copy link
Contributor

@DanGould DanGould Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might check Eq between error types instead of the magic string representation next time

}
}
Loading
Loading