From 85052cf8f0d06509697308f45cb7204e6ab46abf Mon Sep 17 00:00:00 2001 From: DanGould Date: Sun, 12 Jan 2025 22:11:22 -0500 Subject: [PATCH 1/5] Handle receiver error with POST Recoverable errors are be shared with the Sender so that they might recover rather than necessarily waiting for the session to expire. --- payjoin-cli/src/app/v2.rs | 43 ++++++++++++++++++++---- payjoin/src/receive/error.rs | 13 +++++++- payjoin/src/receive/v2/error.rs | 5 +++ payjoin/src/receive/v2/mod.rs | 58 +++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 7 deletions(-) diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index b0f979cc4..790e297fa 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -6,7 +6,7 @@ use bitcoincore_rpc::RpcApi; use payjoin::bitcoin::consensus::encode::serialize_hex; use payjoin::bitcoin::psbt::Psbt; use payjoin::bitcoin::{Amount, FeeRate}; -use payjoin::receive::v2::Receiver; +use payjoin::receive::v2::{Receiver, UncheckedProposal}; use payjoin::send::v2::{Sender, SenderBuilder}; use payjoin::{bitcoin, Error, Uri}; use tokio::signal; @@ -119,7 +119,7 @@ impl App { println!("{}", pj_uri); let mut interrupt = self.interrupt.clone(); - let res = tokio::select! { + let receiver = tokio::select! { res = self.long_poll_fallback(&mut session) => res, _ = interrupt.changed() => { println!("Interrupted. Call the `resume` command to resume all sessions."); @@ -128,10 +128,13 @@ impl App { }?; println!("Fallback transaction received. Consider broadcasting this to get paid if the Payjoin fails:"); - println!("{}", serialize_hex(&res.extract_tx_to_schedule_broadcast())); - let mut payjoin_proposal = self - .process_v2_proposal(res) - .map_err(|e| anyhow!("Failed to process proposal {}", e))?; + println!("{}", serialize_hex(&receiver.extract_tx_to_schedule_broadcast())); + let mut payjoin_proposal = match self.process_v2_proposal(receiver.clone()) { + Ok(proposal) => proposal, + Err(e) => { + return Err(handle_request_error(e, receiver, &self.config.ohttp_relay).await); + } + }; let (req, ohttp_ctx) = payjoin_proposal .extract_v2_req(&self.config.ohttp_relay) .map_err(|e| anyhow!("v2 req extraction failed {}", e))?; @@ -328,6 +331,34 @@ impl App { } } +/// Handle request error by sending an error response over the directory +async fn handle_request_error( + e: Error, + mut receiver: UncheckedProposal, + ohttp_relay: &payjoin::Url, +) -> anyhow::Error { + let (err_req, err_ctx) = match receiver.extract_err_req(&e, ohttp_relay) { + Ok(req_ctx) => req_ctx, + Err(e) => return anyhow!("Failed to extract error request: {}", e), + }; + + let err_response = match post_request(err_req).await { + Ok(response) => response, + Err(e) => return anyhow!("Failed to post error request: {}", e), + }; + + let err_bytes = match err_response.bytes().await { + Ok(bytes) => bytes, + Err(e) => return anyhow!("Failed to get error response bytes: {}", e), + }; + + if let Err(e) = receiver.process_err_res(&err_bytes, err_ctx) { + return anyhow!("Failed to process error response: {}", e); + } + + e.into() +} + fn try_contributing_inputs( payjoin: payjoin::receive::v2::WantsInputs, bitcoind: &bitcoincore_rpc::Client, diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index d6271c7fc..665ae681e 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -6,7 +6,18 @@ pub enum Error { /// To be returned as HTTP 400 BadRequest(RequestError), // To be returned as HTTP 500 - Server(Box), + Server(Box), +} + +impl Error { + pub fn to_json(&self) -> String { + match self { + Self::BadRequest(e) => e.to_string(), + Self::Server(_) => + "{{ \"errorCode\": \"server-error\", \"message\": \"Internal server error\" }}" + .to_string(), + } + } } impl fmt::Display for Error { diff --git a/payjoin/src/receive/v2/error.rs b/payjoin/src/receive/v2/error.rs index 22785819d..858e143dc 100644 --- a/payjoin/src/receive/v2/error.rs +++ b/payjoin/src/receive/v2/error.rs @@ -14,6 +14,8 @@ pub(crate) enum InternalSessionError { OhttpEncapsulation(OhttpEncapsulationError), /// Unexpected response size UnexpectedResponseSize(usize), + /// Unexpected status code + UnexpectedStatusCode(http::StatusCode), } impl fmt::Display for SessionError { @@ -28,6 +30,8 @@ impl fmt::Display for SessionError { size, crate::ohttp::ENCAPSULATED_MESSAGE_BYTES ), + InternalSessionError::UnexpectedStatusCode(status) => + write!(f, "Unexpected status code: {}", status), } } } @@ -38,6 +42,7 @@ impl error::Error for SessionError { InternalSessionError::Expired(_) => None, InternalSessionError::OhttpEncapsulation(e) => Some(e), InternalSessionError::UnexpectedResponseSize(_) => None, + InternalSessionError::UnexpectedStatusCode(_) => None, } } } diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 8a090e0f0..998248f92 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -269,6 +269,64 @@ impl UncheckedProposal { let inner = self.v1.assume_interactive_receiver(); MaybeInputsOwned { v1: inner, context: self.context } } + + /// Extract an OHTTP Encapsulated HTTP POST request to return + /// a Receiver Error Response + pub fn extract_err_req( + &mut self, + err: &Error, + ohttp_relay: &Url, + ) -> Result<(Request, ohttp::ClientResponse), SessionError> { + let subdir = self.subdir(); + let (body, ohttp_ctx) = ohttp_encapsulate( + &mut self.context.ohttp_keys, + "POST", + subdir.as_str(), + Some(err.to_json().as_bytes()), + ) + .map_err(InternalSessionError::OhttpEncapsulation)?; + + let req = Request::new_v2(ohttp_relay.clone(), body); + Ok((req, ohttp_ctx)) + } + + /// Process an OHTTP Encapsulated HTTP POST Error response + /// to ensure it has been posted properly + pub fn process_err_res( + &mut self, + body: &[u8], + context: ohttp::ClientResponse, + ) -> Result<(), SessionError> { + let response_array: &[u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES] = + body.try_into().map_err(|_| { + SessionError::from(InternalSessionError::UnexpectedResponseSize(body.len())) + })?; + let response = ohttp_decapsulate(context, response_array)?; + + match response.status() { + http::StatusCode::OK => Ok(()), + _ => Err(SessionError::from(InternalSessionError::UnexpectedStatusCode( + response.status(), + ))), + } + } + + /// The subdirectory for this Payjoin receiver session. + /// It consists of a directory URL and the session ShortID in the path. + pub fn subdir(&self) -> Url { + let mut url = self.context.directory.clone(); + { + let mut path_segments = + url.path_segments_mut().expect("Payjoin Directory URL cannot be a base"); + path_segments.push(&self.id().to_string()); + } + url + } + + /// The per-session identifier + pub fn id(&self) -> ShortId { + sha256::Hash::hash(&self.context.s.public_key().to_compressed_bytes()).into() + } } /// Typestate to validate that the Original PSBT has no receiver-owned inputs. From 3c39eaf0aed4303e3612fd8dc2867febc6216585 Mon Sep 17 00:00:00 2001 From: DanGould Date: Sun, 12 Jan 2025 22:20:08 -0500 Subject: [PATCH 2/5] Abstract common Receiver subdir and id functions Only `Receiver.id()` needs to be public. Subdir can be a private pure function. --- payjoin/src/receive/v2/mod.rs | 56 +++++++++++++---------------------- 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 998248f92..8db8c8846 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -141,7 +141,7 @@ impl Receiver { ([u8; crate::ohttp::ENCAPSULATED_MESSAGE_BYTES], ohttp::ClientResponse), OhttpEncapsulationError, > { - let fallback_target = self.subdir(); + let fallback_target = subdir(&self.context.directory, &self.id()); ohttp_encapsulate(&mut self.context.ohttp_keys, "GET", fallback_target.as_str(), None) } @@ -192,7 +192,7 @@ impl Receiver { /// Build a V2 Payjoin URI from the receiver's context pub fn pj_uri<'a>(&self) -> crate::PjUri<'a> { use crate::uri::{PayjoinExtras, UrlExt}; - let mut pj = self.subdir().clone(); + let mut pj = subdir(&self.context.directory, &self.id()).clone(); pj.set_receiver_pubkey(self.context.s.public_key().clone()); pj.set_ohttp(self.context.ohttp_keys.clone()); pj.set_exp(self.context.expiry); @@ -200,22 +200,8 @@ impl Receiver { bitcoin_uri::Uri::with_extras(self.context.address.clone(), extras) } - /// The subdirectory for this Payjoin receiver session. - /// It consists of a directory URL and the session ShortID in the path. - pub fn subdir(&self) -> Url { - let mut url = self.context.directory.clone(); - { - let mut path_segments = - url.path_segments_mut().expect("Payjoin Directory URL cannot be a base"); - path_segments.push(&self.id().to_string()); - } - url - } - /// The per-session identifier - pub fn id(&self) -> ShortId { - sha256::Hash::hash(&self.context.s.public_key().to_compressed_bytes()).into() - } + pub fn id(&self) -> ShortId { id(&self.context.s) } } /// The sender's original PSBT and optional parameters @@ -277,7 +263,7 @@ impl UncheckedProposal { err: &Error, ohttp_relay: &Url, ) -> Result<(Request, ohttp::ClientResponse), SessionError> { - let subdir = self.subdir(); + let subdir = subdir(&self.context.directory, &id(&self.context.s)); let (body, ohttp_ctx) = ohttp_encapsulate( &mut self.context.ohttp_keys, "POST", @@ -310,23 +296,6 @@ impl UncheckedProposal { ))), } } - - /// The subdirectory for this Payjoin receiver session. - /// It consists of a directory URL and the session ShortID in the path. - pub fn subdir(&self) -> Url { - let mut url = self.context.directory.clone(); - { - let mut path_segments = - url.path_segments_mut().expect("Payjoin Directory URL cannot be a base"); - path_segments.push(&self.id().to_string()); - } - url - } - - /// The per-session identifier - pub fn id(&self) -> ShortId { - sha256::Hash::hash(&self.context.s.public_key().to_compressed_bytes()).into() - } } /// Typestate to validate that the Original PSBT has no receiver-owned inputs. @@ -597,6 +566,23 @@ impl PayjoinProposal { } } +/// The subdirectory for this Payjoin receiver session. +/// It consists of a directory URL and the session ShortID in the path. +fn subdir(directory: &Url, id: &ShortId) -> Url { + let mut url = directory.clone(); + { + let mut path_segments = + url.path_segments_mut().expect("Payjoin Directory URL cannot be a base"); + path_segments.push(&id.to_string()); + } + url +} + +/// The per-session identifier +fn id(s: &HpkeKeyPair) -> ShortId { + sha256::Hash::hash(&s.public_key().to_compressed_bytes()).into() +} + #[cfg(test)] mod test { use super::*; From 0cd9a5d651e36f947eac815eb11e8d1f679003d7 Mon Sep 17 00:00:00 2001 From: DanGould Date: Mon, 13 Jan 2025 16:15:44 -0500 Subject: [PATCH 3/5] Abstract common receive/v2 static test fields --- payjoin/src/receive/v2/mod.rs | 71 ++++++++++++++--------------------- 1 file changed, 28 insertions(+), 43 deletions(-) diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 8db8c8846..680d7451b 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -585,33 +585,37 @@ fn id(s: &HpkeKeyPair) -> ShortId { #[cfg(test)] mod test { + use std::str::FromStr; + + use ohttp::hpke::{Aead, Kdf, Kem}; + use ohttp::{KeyId, SymmetricSuite}; + use once_cell::sync::Lazy; + use super::*; + const KEY_ID: KeyId = 1; + const KEM: Kem = Kem::K256Sha256; + const SYMMETRIC: &[SymmetricSuite] = + &[ohttp::SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)]; + static EXAMPLE_DIRECTORY_URL: Lazy = + Lazy::new(|| Url::parse("https://directory.com").unwrap()); + + static SHARED_CONTEXT: Lazy = Lazy::new(|| SessionContext { + address: Address::from_str("tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4") + .unwrap() + .assume_checked(), + directory: EXAMPLE_DIRECTORY_URL.clone(), + subdirectory: None, + ohttp_keys: OhttpKeys(ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).unwrap()), + expiry: SystemTime::now() + Duration::from_secs(60), + s: HpkeKeyPair::gen_keypair(), + e: None, + }); + #[test] #[cfg(feature = "v2")] fn receiver_ser_de_roundtrip() { - use ohttp::hpke::{Aead, Kdf, Kem}; - use ohttp::{KeyId, SymmetricSuite}; - const KEY_ID: KeyId = 1; - const KEM: Kem = Kem::K256Sha256; - const SYMMETRIC: &[SymmetricSuite] = - &[ohttp::SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)]; - - let session = Receiver { - context: SessionContext { - address: Address::from_str("tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4") - .unwrap() - .assume_checked(), - directory: url::Url::parse("https://directory.com").unwrap(), - subdirectory: None, - ohttp_keys: OhttpKeys( - ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).unwrap(), - ), - expiry: SystemTime::now() + Duration::from_secs(60), - s: HpkeKeyPair::gen_keypair(), - e: None, - }, - }; + let session = Receiver { context: SHARED_CONTEXT.clone() }; let serialized = serde_json::to_string(&session).unwrap(); let deserialized: Receiver = serde_json::from_str(&serialized).unwrap(); assert_eq!(session, deserialized); @@ -619,27 +623,8 @@ mod test { #[test] fn test_v2_pj_uri() { - let address = bitcoin::Address::from_str("12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX") - .unwrap() - .assume_checked(); - let receiver_keys = crate::hpke::HpkeKeyPair::gen_keypair(); - let ohttp_keys = - OhttpKeys::from_str("OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC") - .expect("Invalid OhttpKeys"); - let arbitrary_url = Url::parse("https://example.com").unwrap(); - let uri = Receiver { - context: SessionContext { - address, - directory: arbitrary_url.clone(), - subdirectory: None, - ohttp_keys, - expiry: SystemTime::now() + Duration::from_secs(60), - s: receiver_keys, - e: None, - }, - } - .pj_uri(); - assert_ne!(uri.extras.endpoint, arbitrary_url); + let uri = Receiver { context: SHARED_CONTEXT.clone() }.pj_uri(); + assert_ne!(uri.extras.endpoint, EXAMPLE_DIRECTORY_URL.clone()); assert!(!uri.extras.disable_output_substitution); } } From 2e2300df80d3afa484d0ce2bc28812b415ced996 Mon Sep 17 00:00:00 2001 From: DanGould Date: Mon, 13 Jan 2025 16:16:11 -0500 Subject: [PATCH 4/5] Remove redundant feature flag --- payjoin/src/receive/v2/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 680d7451b..341609cc6 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -613,7 +613,6 @@ mod test { }); #[test] - #[cfg(feature = "v2")] fn receiver_ser_de_roundtrip() { let session = Receiver { context: SHARED_CONTEXT.clone() }; let serialized = serde_json::to_string(&session).unwrap(); From cdefa562fcbbba697e594cb1a81fb31ea0cf48e2 Mon Sep 17 00:00:00 2001 From: DanGould Date: Mon, 13 Jan 2025 16:26:54 -0500 Subject: [PATCH 5/5] Unit test extract_err_req Ensure a request is produced from the v2 receiver state machine error extract function. --- payjoin/src/receive/error.rs | 2 +- payjoin/src/receive/v1.rs | 5 ++--- payjoin/src/receive/v2/mod.rs | 25 +++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index 665ae681e..99a69dab8 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -61,7 +61,7 @@ impl From for Error { /// This is currently opaque type because we aren't sure which variants will stay. /// You can only display it. #[derive(Debug)] -pub struct RequestError(InternalRequestError); +pub struct RequestError(pub(crate) InternalRequestError); #[derive(Debug)] pub(crate) enum InternalRequestError { diff --git a/payjoin/src/receive/v1.rs b/payjoin/src/receive/v1.rs index ec16412f2..e2d3c94b6 100644 --- a/payjoin/src/receive/v1.rs +++ b/payjoin/src/receive/v1.rs @@ -877,7 +877,7 @@ impl PayjoinProposal { } #[cfg(test)] -mod test { +pub(crate) mod test { use std::str::FromStr; use bitcoin::{Address, Network}; @@ -891,7 +891,6 @@ mod test { } impl MockHeaders { - #[cfg(test)] fn new(length: u64) -> MockHeaders { MockHeaders { length: length.to_string() } } } @@ -905,7 +904,7 @@ mod test { } } - fn proposal_from_test_vector() -> Result { + pub fn proposal_from_test_vector() -> Result { // OriginalPSBT Test Vector from BIP // | InputScriptType | Orginal PSBT Fee rate | maxadditionalfeecontribution | additionalfeeoutputindex| // |-----------------|-----------------------|------------------------------|-------------------------| diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs index 341609cc6..eb4379c2a 100644 --- a/payjoin/src/receive/v2/mod.rs +++ b/payjoin/src/receive/v2/mod.rs @@ -600,6 +600,8 @@ mod test { static EXAMPLE_DIRECTORY_URL: Lazy = Lazy::new(|| Url::parse("https://directory.com").unwrap()); + static EXAMPLE_OHTTP_RELAY: Lazy = Lazy::new(|| Url::parse("https://relay.com").unwrap()); + static SHARED_CONTEXT: Lazy = Lazy::new(|| SessionContext { address: Address::from_str("tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4") .unwrap() @@ -612,6 +614,29 @@ mod test { e: None, }); + #[test] + fn extract_err_req() -> Result<(), Box> { + let mut proposal = UncheckedProposal { + v1: crate::receive::v1::test::proposal_from_test_vector().unwrap(), + context: SHARED_CONTEXT.clone(), + }; + + let server_error = proposal + .clone() + .check_broadcast_suitability(None, |_| Err(Error::Server("mock error".into()))) + .err() + .unwrap(); + assert_eq!( + server_error.to_json(), + "{{ \"errorCode\": \"server-error\", \"message\": \"Internal server error\" }}" + ); + let (_req, _ctx) = proposal.clone().extract_err_req(&server_error, &EXAMPLE_OHTTP_RELAY)?; + + let internal_error = Error::BadRequest(RequestError(InternalRequestError::MissingPayment)); + let (_req, _ctx) = proposal.extract_err_req(&internal_error, &EXAMPLE_OHTTP_RELAY)?; + Ok(()) + } + #[test] fn receiver_ser_de_roundtrip() { let session = Receiver { context: SHARED_CONTEXT.clone() };