Return an error when a ClientResponse is reused

payjoin-ffi/src/ohttp.rs

@@ -27,13 +27,29 @@ impl OhttpKeys {
 
 use std::sync::Mutex;
 
+/// The OHTTP context needed to decapsulate the response to one request.
+///
+/// A context can process exactly one response. Passing it to a second
+/// `process_response` call returns [`ClientResponseError::AlreadyUsed`].
 #[derive(uniffi::Object)]
 pub struct ClientResponse(Mutex<Option<payjoin::OhttpResponse>>);
 
-impl From<&ClientResponse> for payjoin::OhttpResponse {
-    fn from(value: &ClientResponse) -> Self {
+/// Error returned when a [`ClientResponse`] is passed to more than one
+/// `process_response` call.
+#[derive(Debug, thiserror::Error, uniffi::Error)]
+pub enum ClientResponseError {
+    /// The OHTTP context was already used to process a response. Create a
+    /// new request to get a fresh context before retrying.
+    #[error("OHTTP response context was already used")]
+    AlreadyUsed,
+}
+
+impl TryFrom<&ClientResponse> for payjoin::OhttpResponse {
+    type Error = ClientResponseError;
+
+    fn try_from(value: &ClientResponse) -> Result<Self, Self::Error> {
         let mut data_guard = value.0.lock().unwrap();
-        Option::take(&mut *data_guard).expect("ClientResponse moved out of memory")
+        Option::take(&mut *data_guard).ok_or(ClientResponseError::AlreadyUsed)
     }
 }
 

payjoin-ffi/src/receive/mod.rs

@@ -21,7 +21,7 @@ use crate::validation::{
     validate_fee_rate_sat_per_vb_opt, validate_optional_script, validate_script_bytes,
     validate_script_vec, validate_weight_units, validate_witness_stack,
 };
-use crate::{ClientResponse, OutputSubstitution, Request};
+use crate::{ClientResponse, ClientResponseError, OutputSubstitution, Request};
 
 pub mod error;
 
@@ -714,10 +714,17 @@ impl Initialized {
     /// Returns an [`InitializedTransition`] that, once persisted, yields either
     /// an [`UncheckedOriginalPayload`] if the sender's Original PSBT is available,
     /// or [`Initialized`] if no proposal has arrived yet.
-    pub fn process_response(&self, body: &[u8], ctx: &ClientResponse) -> InitializedTransition {
-        InitializedTransition(Arc::new(RwLock::new(Some(
-            self.0.clone().process_response(body, ctx.into()),
-        ))))
+    ///
+    /// Returns [`ClientResponseError::AlreadyUsed`] if `ctx` was already used
+    /// to process a response.
+    pub fn process_response(
+        &self,
+        body: &[u8],
+        ctx: &ClientResponse,
+    ) -> Result<InitializedTransition, ClientResponseError> {
+        Ok(InitializedTransition(Arc::new(RwLock::new(Some(
+            self.0.clone().process_response(body, ctx.try_into()?),
+        )))))
     }
 
     /// Build a V2 Payjoin URI from the receiver's context
@@ -1408,14 +1415,17 @@ impl PayjoinProposal {
     /// This function decapsulates the response using the provided OHTTP context. If the response status is successful, it indicates that the Payjoin proposal has been accepted. Otherwise, it returns an error with the status code.
     ///
     /// After this function is called, the receiver can either wait for the Payjoin transaction to be broadcast or choose to broadcast the original PSBT.
+    ///
+    /// Returns [`ClientResponseError::AlreadyUsed`] if `ohttp_context` was
+    /// already used to process a response.
     pub fn process_response(
         &self,
         body: &[u8],
         ohttp_context: &ClientResponse,
-    ) -> PayjoinProposalTransition {
-        PayjoinProposalTransition(Arc::new(RwLock::new(Some(
-            self.0.clone().process_response(body, ohttp_context.into()),
-        ))))
+    ) -> Result<PayjoinProposalTransition, ClientResponseError> {
+        Ok(PayjoinProposalTransition(Arc::new(RwLock::new(Some(
+            self.0.clone().process_response(body, ohttp_context.try_into()?),
+        )))))
     }
 }
 
@@ -1508,14 +1518,17 @@ impl HasReplyableError {
     /// completes the error reporting and either yields a
     /// [`ReceiverPendingFallback`] if current session has validated fallback tx,
     /// or otherwise closes the session.
+    ///
+    /// Returns [`ClientResponseError::AlreadyUsed`] if `ohttp_context` was
+    /// already used to process a response.
     pub fn process_error_response(
         &self,
         body: &[u8],
         ohttp_context: &ClientResponse,
-    ) -> HasReplyableErrorTransition {
-        HasReplyableErrorTransition(Arc::new(RwLock::new(Some(
-            self.0.clone().process_error_response(body, ohttp_context.into()),
-        ))))
+    ) -> Result<HasReplyableErrorTransition, ClientResponseError> {
+        Ok(HasReplyableErrorTransition(Arc::new(RwLock::new(Some(
+            self.0.clone().process_error_response(body, ohttp_context.try_into()?),
+        )))))
     }
 }
 
@@ -1833,3 +1846,37 @@ impl payjoin::persist::AsyncSessionPersister for AsyncCallbackPersisterAdapter {
         async move { persister.close().await }
     }
 }
+
+#[cfg(all(test, feature = "_test-utils"))]
+mod tests {
+    use payjoin::persist::InMemoryPersister;
+    use payjoin::receive::v2::ReceiverBuilder;
+    use payjoin_test_utils::EXAMPLE_URL;
+
+    use super::*;
+
+    #[test]
+    fn reusing_ohttp_context_returns_error() {
+        let address =
+            payjoin::bitcoin::Address::from_str("tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4")
+                .expect("valid address")
+                .assume_checked();
+        let ohttp_keys = payjoin::OhttpKeys::decode(&payjoin_test_utils::ohttp_key_config_bytes())
+            .expect("valid ohttp keys");
+        let receiver: Initialized = ReceiverBuilder::new(address, EXAMPLE_URL, ohttp_keys)
+            .expect("valid receiver builder")
+            .build()
+            .save(&InMemoryPersister::default())
+            .expect("in-memory persister is infallible")
+            .into();
+
+        let ctx = receiver.create_poll_request(EXAMPLE_URL.to_string()).expect("valid request");
+        // An undersized body is a transient failure, so a caller may retry.
+        // Retrying with the same, now consumed, context must return an error.
+        receiver
+            .process_response(&[0u8; 1], &ctx.client_response)
+            .expect("first use of the context");
+        let reused = receiver.process_response(&[0u8; 1], &ctx.client_response);
+        assert!(matches!(reused, Err(ClientResponseError::AlreadyUsed)));
+    }
+}

payjoin-ffi/src/send/mod.rs

@@ -8,7 +8,7 @@ pub use error::{
 
 use crate::error::ForeignError;
 pub use crate::error::{ImplementationError, SerdeJsonError};
-use crate::ohttp::ClientResponse;
+use crate::ohttp::{ClientResponse, ClientResponseError};
 use crate::request::Request;
 use crate::send::error::{SenderPersistedError, SenderReplayError};
 use crate::uri::PjUri;
@@ -539,14 +539,17 @@ impl WithReplyKey {
     /// A successful response can either be `None` if the relay has no response yet,
     /// or `Some(Psbt)`.
     /// If the response is a valid PSBT you should sign and broadcast it.
+    ///
+    /// Returns [`ClientResponseError::AlreadyUsed`] if `post_ctx` was already
+    /// used to process a response.
     pub fn process_response(
         &self,
         response: &[u8],
         post_ctx: &ClientResponse,
-    ) -> WithReplyKeyTransition {
-        WithReplyKeyTransition(Arc::new(RwLock::new(Some(
-            self.0.clone().process_response(response, post_ctx.into()),
-        ))))
+    ) -> Result<WithReplyKeyTransition, ClientResponseError> {
+        Ok(WithReplyKeyTransition(Arc::new(RwLock::new(Some(
+            self.0.clone().process_response(response, post_ctx.try_into()?),
+        )))))
     }
 }
 
@@ -659,14 +662,17 @@ impl PollingForProposal {
     /// A successful response can either be `None` if the relay has no response yet,
     /// or `Some(Psbt)`.
     /// If the response is a valid PSBT you should sign and broadcast it.
+    ///
+    /// Returns [`ClientResponseError::AlreadyUsed`] if `ohttp_ctx` was already
+    /// used to process a response.
     pub fn process_response(
         &self,
         response: &[u8],
         ohttp_ctx: &ClientResponse,
-    ) -> PollingForProposalTransition {
-        PollingForProposalTransition(Arc::new(RwLock::new(Some(
-            self.0.clone().process_response(response, ohttp_ctx.into()),
-        ))))
+    ) -> Result<PollingForProposalTransition, ClientResponseError> {
+        Ok(PollingForProposalTransition(Arc::new(RwLock::new(Some(
+            self.0.clone().process_response(response, ohttp_ctx.try_into()?),
+        )))))
     }
 }
 
@@ -882,4 +888,44 @@ mod tests {
         SenderBuilder::new(ORIGINAL_PSBT.to_string(), pj_uri(V2_PJ_URI))
             .expect("v2 URI must be accepted");
     }
+
+    #[test]
+    fn reusing_ohttp_context_returns_error() {
+        use payjoin::persist::InMemoryPersister;
+        use payjoin::receive::v2::ReceiverBuilder;
+        use payjoin_test_utils::{EXAMPLE_URL, PARSED_ORIGINAL_PSBT};
+
+        let address = payjoin::bitcoin::Address::from_str("2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7")
+            .expect("valid address")
+            .assume_checked();
+        let ohttp_keys = payjoin::OhttpKeys::decode(&payjoin_test_utils::ohttp_key_config_bytes())
+            .expect("valid ohttp keys");
+        let pj_uri = ReceiverBuilder::new(address, EXAMPLE_URL, ohttp_keys)
+            .expect("valid receiver builder")
+            .build()
+            .save(&InMemoryPersister::default())
+            .expect("in-memory persister is infallible")
+            .pj_uri();
+        let payjoin::PjParam::V2(pj_param) = pj_uri.extras().pj_param() else {
+            panic!("receiver URI must carry a v2 pj param");
+        };
+        let sender: WithReplyKey = payjoin::send::v2::SenderBuilder::from_parts(
+            PARSED_ORIGINAL_PSBT.clone(),
+            pj_param,
+            pj_uri.address(),
+            pj_uri.amount(),
+        )
+        .build_recommended(payjoin::bitcoin::FeeRate::BROADCAST_MIN)
+        .expect("valid sender builder")
+        .save(&InMemoryPersister::default())
+        .expect("in-memory persister is infallible")
+        .into();
+
+        let ctx = sender.create_v2_post_request(EXAMPLE_URL.to_string()).expect("valid request");
+        // An undersized body is a transient failure, so a caller may retry.
+        // Retrying with the same, now consumed, context must return an error.
+        sender.process_response(&[0u8; 1], &ctx.ohttp_ctx).expect("first use of the context");
+        let reused = sender.process_response(&[0u8; 1], &ctx.ohttp_ctx);
+        assert!(matches!(reused, Err(ClientResponseError::AlreadyUsed)));
+    }
 }