From 4dfbd90b8b63ba92971faae48b7701d01e8222d1 Mon Sep 17 00:00:00 2001 From: Yuval Kogman Date: Wed, 20 Aug 2025 22:57:25 +0200 Subject: [PATCH 1/4] Use reply key for replyable errors to v2 senders Before some relatively recent spec changes, two messages were written to the same mailbox. The error path was not updated to reflect this, so the sender will not receive it (as it is polling a different mailbox), and the receiver further relied on the directory allowing mailbox contents to be overwritable (or at least accepting the POST request). With this change a replyable error to a v2 sender will be sent to the reply key, assuming one was obtained from the first message. If no key can be recovered from the message then there is no point in sending a reply as the peer is not adhering to the protocol. --- payjoin/src/core/receive/v2/mod.rs | 88 +++++++++++++++++--------- payjoin/src/core/receive/v2/session.rs | 66 ++++++++++++++----- 2 files changed, 109 insertions(+), 45 deletions(-) diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index f6eb3ab84..5edbd64b4 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -72,8 +72,8 @@ pub struct SessionContext { ohttp_keys: OhttpKeys, expiry: SystemTime, amount: Option, - s: HpkeKeyPair, - e: Option, + receiver_key: HpkeKeyPair, + reply_key: Option, } impl SessionContext { @@ -90,8 +90,21 @@ impl SessionContext { .map_err(|e| InternalSessionError::ParseUrl(e.into())) } - /// The per-session identifier - pub(crate) fn id(&self) -> ShortId { short_id_from_pubkey(self.s.public_key()) } + /// The mailbox ID where the receiver expects the sender's Original PSBT. + pub(crate) fn proposal_mailbox_id(&self) -> ShortId { + short_id_from_pubkey(self.receiver_key.public_key()) + } + + /// The mailbox ID where replies (the Proposal PSBT or errors) should + /// be sent. For V1 requests this is the same as the proposal mailbox ID. + // FIXME before the UncheckedProposal typestate is reached, this returns the + // proposal mailbox ID. It doesn't make sense to reply before receiving + // anything from the sender and at that point it's ambiguous whether it's a + // v2 or v1 sender anyway. Ideally this should be impossible leveraging the + // typestate machinery + pub(crate) fn reply_mailbox_id(&self) -> ShortId { + short_id_from_pubkey(self.reply_key.as_ref().unwrap_or(self.receiver_key.public_key())) + } } fn deserialize_address_assume_checked<'de, D>(deserializer: D) -> Result @@ -239,7 +252,7 @@ fn extract_err_req( if SystemTime::now() > session_context.expiry { return Err(InternalSessionError::Expired(session_context.expiry).into()); } - let mailbox = mailbox_endpoint(&session_context.directory, &session_context.id()); + let mailbox = mailbox_endpoint(&session_context.directory, &session_context.reply_mailbox_id()); let (body, ohttp_ctx) = ohttp_encapsulate( &mut session_context.ohttp_keys.0.clone(), "POST", @@ -282,11 +295,11 @@ impl ReceiverBuilder { address, directory, ohttp_keys, - s: HpkeKeyPair::gen_keypair(), + receiver_key: HpkeKeyPair::gen_keypair(), expiry: SystemTime::now() + TWENTY_FOUR_HOURS_DEFAULT_EXPIRY, amount: None, mailbox: None, - e: None, + reply_key: None, }; Ok(Self(session_context)) } @@ -353,13 +366,13 @@ impl Receiver { ), }; - if let Some(proposal) = proposal { + if let Some((proposal, reply_key)) = proposal { MaybeFatalTransitionWithNoResults::success( - SessionEvent::UncheckedProposal((proposal.clone(), self.context.e.clone())), + SessionEvent::UncheckedProposal((proposal.clone(), reply_key.clone())), Receiver { state: UncheckedProposal { original: proposal, - session_context: self.state.context.clone(), + session_context: SessionContext { reply_key, ..self.state.context.clone() }, }, }, ) @@ -372,7 +385,7 @@ impl Receiver { &mut self, body: &[u8], context: ohttp::ClientResponse, - ) -> Result, Error> { + ) -> Result)>, Error> { let body = match process_get_res(body, context) .map_err(InternalSessionError::DirectoryResponse)? { @@ -381,9 +394,13 @@ impl Receiver { }; match std::str::from_utf8(&body) { // V1 response bodies are utf8 plaintext - Ok(response) => Ok(Some(self.extract_proposal_from_v1(response)?)), + Ok(response) => + Ok(Some(self.extract_proposal_from_v1(response).map(|original| (original, None))?)), // V2 response bodies are encrypted binary - Err(_) => Ok(Some(self.extract_proposal_from_v2(body)?)), + Err(_) => Ok(Some( + self.extract_proposal_from_v2(body) + .map(|(original, reply_key)| (original, Some(reply_key)))?, + )), } } @@ -393,7 +410,8 @@ impl Receiver { ([u8; crate::directory::ENCAPSULATED_MESSAGE_BYTES], ohttp::ClientResponse), OhttpEncapsulationError, > { - let fallback_target = mailbox_endpoint(&self.context.directory, &self.context.id()); + let fallback_target = + mailbox_endpoint(&self.context.directory, &self.context.proposal_mailbox_id()); ohttp_encapsulate(&mut self.context.ohttp_keys, "GET", fallback_target.as_str(), None) } @@ -401,12 +419,15 @@ impl Receiver { self.unchecked_from_payload(response) } - fn extract_proposal_from_v2(&mut self, response: Vec) -> Result { - let (payload_bytes, e) = decrypt_message_a(&response, self.context.s.secret_key().clone())?; - self.context.e = Some(e); + fn extract_proposal_from_v2( + &mut self, + response: Vec, + ) -> Result<(Original, HpkePublicKey), Error> { + let (payload_bytes, reply_key) = + decrypt_message_a(&response, self.context.receiver_key.secret_key().clone())?; let payload = std::str::from_utf8(&payload_bytes) .map_err(|e| Error::ReplyToSender(InternalPayloadError::Utf8(e).into()))?; - self.unchecked_from_payload(payload).map_err(Error::ReplyToSender) + self.unchecked_from_payload(payload).map_err(Error::ReplyToSender).map(|p| (p, reply_key)) } fn unchecked_from_payload(&mut self, payload: &str) -> Result { @@ -450,7 +471,7 @@ impl Receiver { let new_state = Receiver { state: UncheckedProposal { original: event, - session_context: SessionContext { e: reply_key, ..self.state.context }, + session_context: SessionContext { reply_key, ..self.state.context }, }, }; @@ -997,7 +1018,7 @@ impl Receiver { let body: Vec; let method: &str; - if let Some(e) = &self.session_context.e { + if let Some(e) = &self.session_context.reply_key { // Prepare v2 payload let payjoin_bytes = self.psbt.serialize(); let sender_mailbox = short_id_from_pubkey(e); @@ -1006,12 +1027,13 @@ impl Receiver { .directory .join(&sender_mailbox.to_string()) .map_err(|e| ReplyableError::Implementation(ImplementationError::new(e)))?; - body = encrypt_message_b(payjoin_bytes, &self.session_context.s, e)?; + body = encrypt_message_b(payjoin_bytes, &self.session_context.receiver_key, e)?; method = "POST"; } else { // Prepare v2 wrapped and backwards-compatible v1 payload body = self.psbt.to_string().as_bytes().to_vec(); - let receiver_mailbox = short_id_from_pubkey(self.session_context.s.public_key()); + let receiver_mailbox = + short_id_from_pubkey(self.session_context.receiver_key.public_key()); target_resource = self .session_context .directory @@ -1072,10 +1094,10 @@ pub(crate) fn pj_uri<'a>( use crate::uri::PayjoinExtras; let pj_param = crate::uri::PjParam::V2(crate::uri::v2::PjParam::new( session_context.directory.clone(), - session_context.id(), + session_context.proposal_mailbox_id(), session_context.expiry, session_context.ohttp_keys.clone(), - session_context.s.public_key().clone(), + session_context.receiver_key.public_key().clone(), )); let extras = PayjoinExtras { pj_param, output_substitution }; let mut uri = bitcoin_uri::Uri::with_extras(session_context.address.clone(), extras); @@ -1112,8 +1134,8 @@ pub mod test { ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"), ), expiry: SystemTime::now() + Duration::from_secs(60), - s: HpkeKeyPair::gen_keypair(), - e: None, + receiver_key: HpkeKeyPair::gen_keypair(), + reply_key: None, amount: None, }); @@ -1123,7 +1145,10 @@ pub mod test { .expect("Test utils query params should not fail"); UncheckedProposal { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, - session_context: SHARED_CONTEXT.clone(), + session_context: SessionContext { + reply_key: Some(HpkeKeyPair::gen_keypair().public_key().clone()), + ..SHARED_CONTEXT.clone() + }, } } @@ -1133,7 +1158,10 @@ pub mod test { .expect("Test utils query params should not fail"); MaybeInputsOwned { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, - session_context: SHARED_CONTEXT.clone(), + session_context: SessionContext { + reply_key: Some(HpkeKeyPair::gen_keypair().public_key().clone()), + ..SHARED_CONTEXT.clone() + }, } } @@ -1303,11 +1331,11 @@ pub mod test { let actual_json = JsonReply::from(&res); assert_eq!(actual_json.to_json(), expected_json); - let (_req, _ctx) = extract_err_req(&actual_json, &*EXAMPLE_URL, &SHARED_CONTEXT)?; + let (_req, _ctx) = extract_err_req(&actual_json, &*EXAMPLE_URL, &receiver.session_context)?; let internal_error: ReplyableError = InternalPayloadError::MissingPayment.into(); let (_req, _ctx) = - extract_err_req(&(&internal_error).into(), &*EXAMPLE_URL, &SHARED_CONTEXT)?; + extract_err_req(&(&internal_error).into(), &*EXAMPLE_URL, &receiver.session_context)?; Ok(()) } diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index d6d093127..11bf170d6 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -131,6 +131,14 @@ impl SessionHistory { &self, ohttp_relay: impl IntoUrl, ) -> Result, SessionError> { + // FIXME ideally this should be more like a method of + // Receiver and subsequent states instead of the + // history as a whole since it doesn't make sense to call it before, + // reaching that state. + if !self.received_sender_proposal() { + return Ok(None); + } + let session_context = match self.session_context() { Some(session_context) => session_context, None => return Ok(None), @@ -139,15 +147,26 @@ impl SessionHistory { Some((_, Some(json_reply))) => json_reply, _ => return Ok(None), }; - let (req, ctx) = extract_err_req(&json_reply, ohttp_relay, session_context)?; + let (req, ctx) = extract_err_req(&json_reply, ohttp_relay, &session_context)?; Ok(Some((req, ctx))) } - fn session_context(&self) -> Option<&SessionContext> { - self.events.iter().find_map(|event| match event { - SessionEvent::Created(session_context) => Some(session_context), + fn received_sender_proposal(&self) -> bool { + self.events.iter().any(|event| matches!(event, SessionEvent::UncheckedProposal(_))) + } + + fn session_context(&self) -> Option { + let mut initial_session_context = self.events.iter().find_map(|event| match event { + SessionEvent::Created(session_context) => Some(session_context.clone()), _ => None, - }) + })?; + + initial_session_context.reply_key = self.events.iter().find_map(|event| match event { + SessionEvent::UncheckedProposal((_proposal, reply_key)) => reply_key.clone(), + _ => None, + }); + + Some(initial_session_context) } } @@ -305,18 +324,22 @@ mod tests { fn test_replaying_unchecked_proposal() -> Result<(), BoxError> { let session_context = SHARED_CONTEXT.clone(); let original = original_from_test_vector(); + let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); let test = SessionHistoryTest { events: vec![ SessionEvent::Created(session_context.clone()), - SessionEvent::UncheckedProposal((original.clone(), None)), + SessionEvent::UncheckedProposal((original.clone(), reply_key.clone())), ], expected_session_history: SessionHistoryExpectedOutcome { psbt_with_fee_contributions: None, fallback_tx: None, }, expected_receiver_state: ReceiveSession::UncheckedProposal(Receiver { - state: UncheckedProposal { original, session_context }, + state: UncheckedProposal { + original, + session_context: SessionContext { reply_key, ..session_context }, + }, }), }; run_session_history_test(test) @@ -356,18 +379,22 @@ mod tests { fn test_replaying_unchecked_proposal_with_reply_key() -> Result<(), BoxError> { let session_context = SHARED_CONTEXT.clone(); let original = original_from_test_vector(); + let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); let test = SessionHistoryTest { events: vec![ SessionEvent::Created(session_context.clone()), - SessionEvent::UncheckedProposal((original.clone(), session_context.e.clone())), + SessionEvent::UncheckedProposal((original.clone(), reply_key.clone())), ], expected_session_history: SessionHistoryExpectedOutcome { psbt_with_fee_contributions: None, fallback_tx: None, }, expected_receiver_state: ReceiveSession::UncheckedProposal(Receiver { - state: UncheckedProposal { original, session_context }, + state: UncheckedProposal { + original, + session_context: SessionContext { reply_key, ..session_context }, + }, }), }; run_session_history_test(test) @@ -384,9 +411,10 @@ mod tests { .save(&persister) .unwrap(); let expected_fallback = maybe_inputs_owned.extract_tx_to_schedule_broadcast(); + let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); events.push(SessionEvent::Created(session_context.clone())); - events.push(SessionEvent::UncheckedProposal((original.clone(), None))); + events.push(SessionEvent::UncheckedProposal((original.clone(), reply_key.clone()))); events.push(SessionEvent::MaybeInputsOwned()); let test = SessionHistoryTest { @@ -396,7 +424,10 @@ mod tests { fallback_tx: Some(expected_fallback), }, expected_receiver_state: ReceiveSession::MaybeInputsOwned(Receiver { - state: MaybeInputsOwned { original, session_context }, + state: MaybeInputsOwned { + original, + session_context: SessionContext { reply_key, ..session_context }, + }, }), }; run_session_history_test(test) @@ -438,9 +469,10 @@ mod tests { .save(&persister) .expect("Contributed inputs should be valid"); let expected_fallback = maybe_inputs_owned.extract_tx_to_schedule_broadcast(); + let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); events.push(SessionEvent::Created(session_context.clone())); - events.push(SessionEvent::UncheckedProposal((original.clone(), None))); + events.push(SessionEvent::UncheckedProposal((original.clone(), reply_key.clone()))); events.push(SessionEvent::MaybeInputsOwned()); events.push(SessionEvent::MaybeInputsSeen()); events.push(SessionEvent::OutputsUnknown()); @@ -462,7 +494,7 @@ mod tests { expected_receiver_state: ReceiveSession::ProvisionalProposal(Receiver { state: ProvisionalProposal { psbt_context: provisional_proposal.state.psbt_context.clone(), - session_context, + session_context: SessionContext { reply_key, ..session_context }, }, }), }; @@ -510,9 +542,10 @@ mod tests { .save(&persister) .expect("Payjoin proposal should be finalized"); let expected_fallback = maybe_inputs_owned.extract_tx_to_schedule_broadcast(); + let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); events.push(SessionEvent::Created(session_context.clone())); - events.push(SessionEvent::UncheckedProposal((original.clone(), None))); + events.push(SessionEvent::UncheckedProposal((original.clone(), reply_key.clone()))); events.push(SessionEvent::MaybeInputsOwned()); events.push(SessionEvent::MaybeInputsSeen()); events.push(SessionEvent::OutputsUnknown()); @@ -533,7 +566,10 @@ mod tests { fallback_tx: Some(expected_fallback), }, expected_receiver_state: ReceiveSession::PayjoinProposal(Receiver { - state: PayjoinProposal { psbt: payjoin_proposal.psbt().clone(), session_context }, + state: PayjoinProposal { + psbt: payjoin_proposal.psbt().clone(), + session_context: SessionContext { reply_key, ..session_context }, + }, }), }; run_session_history_test(test) From 94063ce66bc1f3f690b82fe3a9a070ed91667412 Mon Sep 17 00:00:00 2001 From: Yuval Kogman Date: Fri, 22 Aug 2025 16:44:18 +0200 Subject: [PATCH 2/4] Rename Unchecked{Proposal,OriginalPayload} --- payjoin-cli/src/app/v1.rs | 6 +- payjoin-cli/src/app/v2/mod.rs | 10 ++-- .../test/test_payjoin_integration_test.dart | 6 +- .../test/test_payjoin_integration_test.py | 2 +- payjoin-ffi/src/receive/mod.rs | 44 +++++++-------- payjoin/src/core/receive/v1/mod.rs | 22 ++++---- payjoin/src/core/receive/v2/mod.rs | 38 ++++++------- payjoin/src/core/receive/v2/session.rs | 55 +++++++++++-------- payjoin/tests/integration.rs | 8 +-- 9 files changed, 100 insertions(+), 91 deletions(-) diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index f2b6829e7..871b3ac5c 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -13,7 +13,7 @@ use hyper::{Method, Request, Response, StatusCode}; use hyper_util::rt::TokioIo; use payjoin::bitcoin::psbt::Psbt; use payjoin::bitcoin::{Amount, FeeRate}; -use payjoin::receive::v1::{PayjoinProposal, UncheckedProposal}; +use payjoin::receive::v1::{PayjoinProposal, UncheckedOriginalPayload}; use payjoin::receive::ReplyableError::{self, Implementation, V1}; use payjoin::send::v1::SenderBuilder; use payjoin::{ImplementationError, IntoUrl, Uri, UriExt}; @@ -301,7 +301,7 @@ impl App { .await .map_err(|e| Implementation(ImplementationError::new(e)))? .to_bytes(); - let proposal = UncheckedProposal::from_request(&body, query_string, headers)?; + let proposal = UncheckedOriginalPayload::from_request(&body, query_string, headers)?; let payjoin_proposal = self.process_v1_proposal(proposal)?; let psbt = payjoin_proposal.psbt(); @@ -315,7 +315,7 @@ impl App { fn process_v1_proposal( &self, - proposal: UncheckedProposal, + proposal: UncheckedOriginalPayload, ) -> Result { let wallet = self.wallet(); diff --git a/payjoin-cli/src/app/v2/mod.rs b/payjoin-cli/src/app/v2/mod.rs index d51266958..55e92314a 100644 --- a/payjoin-cli/src/app/v2/mod.rs +++ b/payjoin-cli/src/app/v2/mod.rs @@ -7,8 +7,8 @@ use payjoin::persist::OptionalTransitionOutcome; use payjoin::receive::v2::{ process_err_res, replay_event_log as replay_receiver_event_log, Initialized, MaybeInputsOwned, MaybeInputsSeen, OutputsUnknown, PayjoinProposal, ProvisionalProposal, ReceiveSession, - Receiver, ReceiverBuilder, SessionHistory, UncheckedProposal, WantsFeeRange, WantsInputs, - WantsOutputs, + Receiver, ReceiverBuilder, SessionHistory, UncheckedOriginalPayload, WantsFeeRange, + WantsInputs, WantsOutputs, }; use payjoin::send::v2::{ replay_event_log as replay_sender_event_log, SendSession, Sender, SenderBuilder, V2GetContext, @@ -297,7 +297,7 @@ impl App { &self, session: Receiver, persister: &ReceiverPersister, - ) -> Result> { + ) -> Result> { let ohttp_relay = self .unwrap_relay_or_else_fetch(Some(session.pj_uri().extras.endpoint().clone())) .await?; @@ -333,7 +333,7 @@ impl App { match session { ReceiveSession::Initialized(proposal) => self.read_from_directory(proposal, persister).await, - ReceiveSession::UncheckedProposal(proposal) => + ReceiveSession::UncheckedOriginalPayload(proposal) => self.check_proposal(proposal, persister).await, ReceiveSession::MaybeInputsOwned(proposal) => self.check_inputs_not_owned(proposal, persister).await, @@ -393,7 +393,7 @@ impl App { async fn check_proposal( &self, - proposal: Receiver, + proposal: Receiver, persister: &ReceiverPersister, ) -> Result<()> { let wallet = self.wallet(); diff --git a/payjoin-ffi/dart/test/test_payjoin_integration_test.dart b/payjoin-ffi/dart/test/test_payjoin_integration_test.dart index ad86d24c7..5fd58f2c7 100644 --- a/payjoin-ffi/dart/test/test_payjoin_integration_test.dart +++ b/payjoin-ffi/dart/test/test_payjoin_integration_test.dart @@ -233,7 +233,7 @@ Future process_maybe_inputs_owned( } Future process_unchecked_proposal( - payjoin.UncheckedProposal proposal, + payjoin.UncheckedOriginalPayload proposal, InMemoryReceiverPersister recv_persister) async { final unchecked_proposal = proposal .checkBroadcastSuitability(null, MempoolAcceptanceCallback(receiver)) @@ -258,7 +258,7 @@ Future retrieve_receiver_proposal( } var proposal = res.success(); return await process_unchecked_proposal( - proposal as payjoin.UncheckedProposal, recv_persister); + proposal as payjoin.UncheckedOriginalPayload, recv_persister); } Future process_receiver_proposal( @@ -274,7 +274,7 @@ Future process_receiver_proposal( return res; } - if (receiver is payjoin.UncheckedProposalReceiveSession) { + if (receiver is payjoin.UncheckedOriginalPayloadReceiveSession) { return await process_unchecked_proposal(receiver.inner, recv_persister); } if (receiver is payjoin.MaybeInputsOwnedReceiveSession) { diff --git a/payjoin-ffi/python/test/test_payjoin_integration_test.py b/payjoin-ffi/python/test/test_payjoin_integration_test.py index c5bfb4128..833263f85 100644 --- a/payjoin-ffi/python/test/test_payjoin_integration_test.py +++ b/payjoin-ffi/python/test/test_payjoin_integration_test.py @@ -101,7 +101,7 @@ async def retrieve_receiver_proposal(self, receiver: Initialized, recv_persister proposal = res.success() return await self.process_unchecked_proposal(proposal, recv_persister) - async def process_unchecked_proposal(self, proposal: UncheckedProposal, recv_persister: InMemoryReceiverSessionEventLog) : + async def process_unchecked_proposal(self, proposal: UncheckedOriginalPayload, recv_persister: InMemoryReceiverSessionEventLog) : receiver = proposal.check_broadcast_suitability(None, MempoolAcceptanceCallback(self.receiver)).save(recv_persister) return await self.process_maybe_inputs_owned(receiver, recv_persister) diff --git a/payjoin-ffi/src/receive/mod.rs b/payjoin-ffi/src/receive/mod.rs index 5c2b0563d..7ebd9c977 100644 --- a/payjoin-ffi/src/receive/mod.rs +++ b/payjoin-ffi/src/receive/mod.rs @@ -77,7 +77,7 @@ impl ReceiverSessionEvent { pub enum ReceiveSession { Uninitialized, Initialized { inner: Arc }, - UncheckedProposal { inner: Arc }, + UncheckedOriginalPayload { inner: Arc }, MaybeInputsOwned { inner: Arc }, MaybeInputsSeen { inner: Arc }, OutputsUnknown { inner: Arc }, @@ -96,8 +96,8 @@ impl From for ReceiveSession { ReceiveSession::Uninitialized => Self::Uninitialized, ReceiveSession::Initialized(inner) => Self::Initialized { inner: Arc::new(inner.into()) }, - ReceiveSession::UncheckedProposal(inner) => - Self::UncheckedProposal { inner: Arc::new(inner.into()) }, + ReceiveSession::UncheckedOriginalPayload(inner) => + Self::UncheckedOriginalPayload { inner: Arc::new(inner.into()) }, ReceiveSession::MaybeInputsOwned(inner) => Self::MaybeInputsOwned { inner: Arc::new(inner.into()) }, ReceiveSession::MaybeInputsSeen(inner) => @@ -308,7 +308,7 @@ pub struct InitializedTransition( Option< payjoin::persist::MaybeFatalTransitionWithNoResults< payjoin::receive::v2::SessionEvent, - payjoin::receive::v2::Receiver, + payjoin::receive::v2::Receiver, payjoin::receive::v2::Receiver, payjoin::receive::Error, >, @@ -339,7 +339,7 @@ impl InitializedTransition { #[derive(uniffi::Object)] pub struct InitializedTransitionOutcome( payjoin::persist::OptionalTransitionOutcome< - payjoin::receive::v2::Receiver, + payjoin::receive::v2::Receiver, payjoin::receive::v2::Receiver, >, ); @@ -350,7 +350,7 @@ impl InitializedTransitionOutcome { pub fn is_success(&self) -> bool { self.0.is_success() } - pub fn success(&self) -> Option> { + pub fn success(&self) -> Option> { self.0.success().map(|r| Arc::new(r.clone().into())) } } @@ -358,14 +358,14 @@ impl InitializedTransitionOutcome { impl From< payjoin::persist::OptionalTransitionOutcome< - payjoin::receive::v2::Receiver, + payjoin::receive::v2::Receiver, payjoin::receive::v2::Receiver, >, > for InitializedTransitionOutcome { fn from( value: payjoin::persist::OptionalTransitionOutcome< - payjoin::receive::v2::Receiver, + payjoin::receive::v2::Receiver, payjoin::receive::v2::Receiver, >, ) -> Self { @@ -396,7 +396,7 @@ impl Initialized { .map_err(Into::into) } - /// The response can either be an UncheckedProposal or an ACCEPTED message indicating no UncheckedProposal is available yet. + /// The response can either be an UncheckedOriginalPayload or an ACCEPTED message indicating no UncheckedOriginalPayload is available 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()), @@ -414,29 +414,29 @@ impl Initialized { } #[derive(Clone, uniffi::Object)] -pub struct UncheckedProposal( - payjoin::receive::v2::Receiver, +pub struct UncheckedOriginalPayload( + payjoin::receive::v2::Receiver, ); -impl From> - for UncheckedProposal +impl From> + for UncheckedOriginalPayload { fn from( - value: payjoin::receive::v2::Receiver, + value: payjoin::receive::v2::Receiver, ) -> Self { Self(value) } } -impl From - for payjoin::receive::v2::Receiver +impl From + for payjoin::receive::v2::Receiver { - fn from(value: UncheckedProposal) -> Self { value.0 } + fn from(value: UncheckedOriginalPayload) -> Self { value.0 } } #[derive(uniffi::Object)] #[allow(clippy::type_complexity)] -pub struct UncheckedProposalTransition( +pub struct UncheckedOriginalPayloadTransition( Arc< RwLock< Option< @@ -450,7 +450,7 @@ pub struct UncheckedProposalTransition( >, ); -impl_save_for_transition!(UncheckedProposalTransition, MaybeInputsOwned); +impl_save_for_transition!(UncheckedOriginalPayloadTransition, MaybeInputsOwned); #[derive(uniffi::Object)] #[allow(clippy::type_complexity)] @@ -475,13 +475,13 @@ pub trait CanBroadcast: Send + Sync { } #[uniffi::export] -impl UncheckedProposal { +impl UncheckedOriginalPayload { pub fn check_broadcast_suitability( &self, min_fee_rate: Option, can_broadcast: Arc, - ) -> UncheckedProposalTransition { - UncheckedProposalTransition(Arc::new(RwLock::new(Some( + ) -> UncheckedOriginalPayloadTransition { + UncheckedOriginalPayloadTransition(Arc::new(RwLock::new(Some( self.0.clone().check_broadcast_suitability( min_fee_rate.map(FeeRate::from_sat_per_kwu), |transaction| { diff --git a/payjoin/src/core/receive/v1/mod.rs b/payjoin/src/core/receive/v1/mod.rs index a21c920e6..57267ff07 100644 --- a/payjoin/src/core/receive/v1/mod.rs +++ b/payjoin/src/core/receive/v1/mod.rs @@ -7,7 +7,7 @@ //! using [`build_v1_pj_uri`] //! 2. Listen for a sender's request on the `pj` endpoint //! 3. Parse the request using -//! [`UncheckedProposal::from_request()`] +//! [`UncheckedOriginalPayload::from_request()`] //! 4. Validate the proposal using the `check` methods to guide you. //! 5. Assuming the proposal is valid, augment it into a payjoin with the available //! `try_preserving_privacy` and `contribute` methods @@ -58,7 +58,7 @@ pub fn build_v1_pj_uri<'a>( Ok(bitcoin_uri::Uri::with_extras(address.clone(), extras)) } -impl UncheckedProposal { +impl UncheckedOriginalPayload { pub fn from_request( body: &[u8], query: &str, @@ -71,7 +71,7 @@ impl UncheckedProposal { let (psbt, params) = crate::receive::parse_payload(base64, query, SUPPORTED_VERSIONS) .map_err(ReplyableError::Payload)?; - Ok(UncheckedProposal { original: Original { psbt, params } }) + Ok(Self { original: Original { psbt, params } }) } } @@ -93,11 +93,11 @@ impl UncheckedProposal { /// If you are implementing an interactive payment receiver, then such checks are not necessary, and you /// can go ahead with calling [`Self::assume_interactive_receiver`] to move on to the next typestate. #[derive(Debug, Clone)] -pub struct UncheckedProposal { +pub struct UncheckedOriginalPayload { original: Original, } -impl UncheckedProposal { +impl UncheckedOriginalPayload { /// Checks that the original PSBT in the proposal can be broadcasted. /// /// If the receiver is a non-interactive payment processor (ex. a donation page which generates @@ -387,7 +387,7 @@ mod tests { let validated_request = validate_body(headers.clone(), body); assert!(validated_request.is_ok()); - let proposal = UncheckedProposal::from_request(body, QUERY_PARAMS, headers)?; + let proposal = UncheckedOriginalPayload::from_request(body, QUERY_PARAMS, headers)?; let witness_utxo = proposal.original.psbt.inputs[0] .witness_utxo @@ -405,11 +405,11 @@ mod tests { Ok(()) } - fn unchecked_proposal_from_test_vector() -> UncheckedProposal { + fn unchecked_proposal_from_test_vector() -> UncheckedOriginalPayload { let pairs = url::form_urlencoded::parse(QUERY_PARAMS.as_bytes()); let params = Params::from_query_pairs(pairs, &[Version::One]) .expect("Could not parse params from query pairs"); - UncheckedProposal { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params } } + UncheckedOriginalPayload { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params } } } fn maybe_inputs_owned_from_test_vector() -> MaybeInputsOwned { @@ -419,7 +419,7 @@ mod tests { MaybeInputsOwned { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params } } } - fn wants_outputs_from_test_vector(proposal: UncheckedProposal) -> WantsOutputs { + fn wants_outputs_from_test_vector(proposal: UncheckedOriginalPayload) -> WantsOutputs { proposal .assume_interactive_receiver() .check_inputs_not_owned(&mut |_| Ok(false)) @@ -437,7 +437,9 @@ mod tests { .expect("Receiver output should be identified") } - fn provisional_proposal_from_test_vector(proposal: UncheckedProposal) -> ProvisionalProposal { + fn provisional_proposal_from_test_vector( + proposal: UncheckedOriginalPayload, + ) -> ProvisionalProposal { wants_outputs_from_test_vector(proposal) .commit_outputs() .commit_inputs() diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index 5edbd64b4..50212d1ec 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -97,7 +97,7 @@ impl SessionContext { /// The mailbox ID where replies (the Proposal PSBT or errors) should /// be sent. For V1 requests this is the same as the proposal mailbox ID. - // FIXME before the UncheckedProposal typestate is reached, this returns the + // FIXME before the UncheckedOriginalPayload typestate is reached, this returns the // proposal mailbox ID. It doesn't make sense to reply before receiving // anything from the sender and at that point it's ambiguous whether it's a // v2 or v1 sender anyway. Ideally this should be impossible leveraging the @@ -130,7 +130,7 @@ fn short_id_from_pubkey(pubkey: &HpkePublicKey) -> ShortId { pub enum ReceiveSession { Uninitialized, Initialized(Receiver), - UncheckedProposal(Receiver), + UncheckedOriginalPayload(Receiver), MaybeInputsOwned(Receiver), MaybeInputsSeen(Receiver), OutputsUnknown(Receiver), @@ -150,10 +150,10 @@ impl ReceiveSession { ( ReceiveSession::Initialized(state), - SessionEvent::UncheckedProposal((proposal, reply_key)), + SessionEvent::UncheckedOriginalPayload((proposal, reply_key)), ) => Ok(state.apply_unchecked_from_payload(proposal, reply_key)?), - (ReceiveSession::UncheckedProposal(state), SessionEvent::MaybeInputsOwned()) => + (ReceiveSession::UncheckedOriginalPayload(state), SessionEvent::MaybeInputsOwned()) => Ok(state.apply_maybe_inputs_owned()), (ReceiveSession::MaybeInputsOwned(state), SessionEvent::MaybeInputsSeen()) => @@ -195,7 +195,7 @@ mod sealed { pub trait State {} impl State for super::Initialized {} - impl State for super::UncheckedProposal {} + impl State for super::UncheckedOriginalPayload {} impl State for super::MaybeInputsOwned {} impl State for super::MaybeInputsSeen {} impl State for super::OutputsUnknown {} @@ -344,15 +344,15 @@ impl Receiver { Ok((req, ohttp_ctx)) } - /// The response can either be an UncheckedProposal or an ACCEPTED message - /// indicating no UncheckedProposal is available yet. + /// The response can either be an UncheckedOriginalPayload or an ACCEPTED message + /// indicating no UncheckedOriginalPayload is available yet. pub fn process_response( &mut self, body: &[u8], context: ohttp::ClientResponse, ) -> MaybeFatalTransitionWithNoResults< SessionEvent, - Receiver, + Receiver, Receiver, Error, > { @@ -368,9 +368,9 @@ impl Receiver { if let Some((proposal, reply_key)) = proposal { MaybeFatalTransitionWithNoResults::success( - SessionEvent::UncheckedProposal((proposal.clone(), reply_key.clone())), + SessionEvent::UncheckedOriginalPayload((proposal.clone(), reply_key.clone())), Receiver { - state: UncheckedProposal { + state: UncheckedOriginalPayload { original: proposal, session_context: SessionContext { reply_key, ..self.state.context.clone() }, }, @@ -469,13 +469,13 @@ impl Receiver { } let new_state = Receiver { - state: UncheckedProposal { + state: UncheckedOriginalPayload { original: event, session_context: SessionContext { reply_key, ..self.state.context }, }, }; - Ok(ReceiveSession::UncheckedProposal(new_state)) + Ok(ReceiveSession::UncheckedOriginalPayload(new_state)) } } @@ -485,7 +485,7 @@ impl Receiver { /// [`Receiver::process_response()`]. /// #[derive(Debug, Clone, PartialEq)] -pub struct UncheckedProposal { +pub struct UncheckedOriginalPayload { pub(crate) original: Original, pub(crate) session_context: SessionContext, } @@ -499,15 +499,15 @@ pub struct UncheckedProposal { /// The recommended usage of this typestate differs based on whether you are implementing an /// interactive (where the receiver takes manual actions to respond to the /// payjoin proposal) or a non-interactive (ex. a donation page which automatically generates a new QR code -/// for each visit) payment receiver. For the latter, you should call [`Receiver::check_broadcast_suitability`] to check +/// for each visit) payment receiver. For the latter, you should call [`Receiver::check_broadcast_suitability`] to check /// that the proposal is actually broadcastable (and, optionally, whether the fee rate is above the /// minimum limit you have set). These mechanisms protect the receiver against probing attacks, where /// a malicious sender can repeatedly send proposals to have the non-interactive receiver reveal the UTXOs /// it owns with the proposals it modifies. /// /// If you are implementing an interactive payment receiver, then such checks are not necessary, and you -/// can go ahead with calling [`Receiver::assume_interactive_receiver`] to move on to the next typestate. -impl Receiver { +/// can go ahead with calling [`Receiver::assume_interactive_receiver`] to move on to the next typestate. +impl Receiver { /// Checks that the original PSBT in the proposal can be broadcasted. /// /// If the receiver is a non-interactive payment processor (ex. a donation page which generates @@ -1139,11 +1139,11 @@ pub mod test { amount: None, }); - pub(crate) fn unchecked_proposal_v2_from_test_vector() -> UncheckedProposal { + pub(crate) fn unchecked_proposal_v2_from_test_vector() -> UncheckedOriginalPayload { let pairs = url::form_urlencoded::parse(QUERY_PARAMS.as_bytes()); let params = Params::from_query_pairs(pairs, &[Version::Two]) .expect("Test utils query params should not fail"); - UncheckedProposal { + UncheckedOriginalPayload { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, session_context: SessionContext { reply_key: Some(HpkeKeyPair::gen_keypair().public_key().clone()), @@ -1345,7 +1345,7 @@ pub mod test { let noop_persister = NoopSessionPersister::default(); let context = SessionContext { expiry: now, ..SHARED_CONTEXT.clone() }; let receiver = Receiver { - state: UncheckedProposal { + state: UncheckedOriginalPayload { original: crate::receive::tests::original_from_test_vector(), session_context: context.clone(), }, diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index 11bf170d6..59685fdce 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -90,7 +90,7 @@ impl SessionHistory { fn get_unchecked_proposal(&self) -> Option { self.events.iter().find_map(|event| match event { - SessionEvent::UncheckedProposal(proposal) => Some(proposal.0.clone()), + SessionEvent::UncheckedOriginalPayload(proposal) => Some(proposal.0.clone()), _ => None, }) } @@ -132,7 +132,7 @@ impl SessionHistory { ohttp_relay: impl IntoUrl, ) -> Result, SessionError> { // FIXME ideally this should be more like a method of - // Receiver and subsequent states instead of the + // Receiver and subsequent states instead of the // history as a whole since it doesn't make sense to call it before, // reaching that state. if !self.received_sender_proposal() { @@ -152,7 +152,7 @@ impl SessionHistory { } fn received_sender_proposal(&self) -> bool { - self.events.iter().any(|event| matches!(event, SessionEvent::UncheckedProposal(_))) + self.events.iter().any(|event| matches!(event, SessionEvent::UncheckedOriginalPayload(_))) } fn session_context(&self) -> Option { @@ -162,7 +162,7 @@ impl SessionHistory { })?; initial_session_context.reply_key = self.events.iter().find_map(|event| match event { - SessionEvent::UncheckedProposal((_proposal, reply_key)) => reply_key.clone(), + SessionEvent::UncheckedOriginalPayload((_proposal, reply_key)) => reply_key.clone(), _ => None, }); @@ -175,7 +175,7 @@ impl SessionHistory { /// Each event can be used to transition the receiver state machine to a new state pub enum SessionEvent { Created(SessionContext), - UncheckedProposal((Original, Option)), + UncheckedOriginalPayload((Original, Option)), MaybeInputsOwned(), MaybeInputsSeen(), OutputsUnknown(), @@ -202,12 +202,12 @@ mod tests { use crate::receive::v2::test::SHARED_CONTEXT; use crate::receive::v2::{ Initialized, MaybeInputsOwned, PayjoinProposal, ProvisionalProposal, Receiver, - UncheckedProposal, + UncheckedOriginalPayload, }; - fn unchecked_receiver_from_test_vector() -> Receiver { + fn unchecked_receiver_from_test_vector() -> Receiver { Receiver { - state: UncheckedProposal { + state: UncheckedOriginalPayload { original: original_from_test_vector(), session_context: SHARED_CONTEXT.clone(), }, @@ -257,8 +257,11 @@ mod tests { let test_cases = vec![ SessionEvent::Created(SHARED_CONTEXT.clone()), - SessionEvent::UncheckedProposal((original.clone(), None)), - SessionEvent::UncheckedProposal((original, Some(crate::HpkeKeyPair::gen_keypair().1))), + SessionEvent::UncheckedOriginalPayload((original.clone(), None)), + SessionEvent::UncheckedOriginalPayload(( + original, + Some(crate::HpkeKeyPair::gen_keypair().1), + )), SessionEvent::MaybeInputsOwned(), SessionEvent::MaybeInputsSeen(), SessionEvent::OutputsUnknown(), @@ -329,14 +332,14 @@ mod tests { let test = SessionHistoryTest { events: vec![ SessionEvent::Created(session_context.clone()), - SessionEvent::UncheckedProposal((original.clone(), reply_key.clone())), + SessionEvent::UncheckedOriginalPayload((original.clone(), reply_key.clone())), ], expected_session_history: SessionHistoryExpectedOutcome { psbt_with_fee_contributions: None, fallback_tx: None, }, - expected_receiver_state: ReceiveSession::UncheckedProposal(Receiver { - state: UncheckedProposal { + expected_receiver_state: ReceiveSession::UncheckedOriginalPayload(Receiver { + state: UncheckedOriginalPayload { original, session_context: SessionContext { reply_key, ..session_context }, }, @@ -348,20 +351,24 @@ mod tests { #[test] fn test_replaying_unchecked_proposal_expiry() { let now = SystemTime::now(); - let context = SessionContext { expiry: now, ..SHARED_CONTEXT.clone() }; + let session_context = SessionContext { expiry: now, ..SHARED_CONTEXT.clone() }; let original = original_from_test_vector(); + let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); let test = SessionHistoryTest { events: vec![ - SessionEvent::Created(context.clone()), - SessionEvent::UncheckedProposal((original.clone(), None)), + SessionEvent::Created(session_context.clone()), + SessionEvent::UncheckedOriginalPayload((original.clone(), reply_key.clone())), ], expected_session_history: SessionHistoryExpectedOutcome { psbt_with_fee_contributions: None, fallback_tx: None, }, - expected_receiver_state: ReceiveSession::UncheckedProposal(Receiver { - state: UncheckedProposal { original, session_context: context }, + expected_receiver_state: ReceiveSession::UncheckedOriginalPayload(Receiver { + state: UncheckedOriginalPayload { + original, + session_context: SessionContext { reply_key, ..session_context }, + }, }), }; let session_history = run_session_history_test(test); @@ -384,14 +391,14 @@ mod tests { let test = SessionHistoryTest { events: vec![ SessionEvent::Created(session_context.clone()), - SessionEvent::UncheckedProposal((original.clone(), reply_key.clone())), + SessionEvent::UncheckedOriginalPayload((original.clone(), reply_key.clone())), ], expected_session_history: SessionHistoryExpectedOutcome { psbt_with_fee_contributions: None, fallback_tx: None, }, - expected_receiver_state: ReceiveSession::UncheckedProposal(Receiver { - state: UncheckedProposal { + expected_receiver_state: ReceiveSession::UncheckedOriginalPayload(Receiver { + state: UncheckedOriginalPayload { original, session_context: SessionContext { reply_key, ..session_context }, }, @@ -414,7 +421,7 @@ mod tests { let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); events.push(SessionEvent::Created(session_context.clone())); - events.push(SessionEvent::UncheckedProposal((original.clone(), reply_key.clone()))); + events.push(SessionEvent::UncheckedOriginalPayload((original.clone(), reply_key.clone()))); events.push(SessionEvent::MaybeInputsOwned()); let test = SessionHistoryTest { @@ -472,7 +479,7 @@ mod tests { let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); events.push(SessionEvent::Created(session_context.clone())); - events.push(SessionEvent::UncheckedProposal((original.clone(), reply_key.clone()))); + events.push(SessionEvent::UncheckedOriginalPayload((original.clone(), reply_key.clone()))); events.push(SessionEvent::MaybeInputsOwned()); events.push(SessionEvent::MaybeInputsSeen()); events.push(SessionEvent::OutputsUnknown()); @@ -545,7 +552,7 @@ mod tests { let reply_key = Some(crate::HpkeKeyPair::gen_keypair().1); events.push(SessionEvent::Created(session_context.clone())); - events.push(SessionEvent::UncheckedProposal((original.clone(), reply_key.clone()))); + events.push(SessionEvent::UncheckedOriginalPayload((original.clone(), reply_key.clone()))); events.push(SessionEvent::MaybeInputsOwned()); events.push(SessionEvent::MaybeInputsSeen()); events.push(SessionEvent::OutputsUnknown()); diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 3094e077f..c7fe85a03 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -173,7 +173,7 @@ mod integration { use payjoin::persist::NoopSessionPersister; use payjoin::receive::v2::{ replay_event_log as replay_receiver_event_log, PayjoinProposal, Receiver, - ReceiverBuilder, UncheckedProposal, + ReceiverBuilder, UncheckedOriginalPayload, }; use payjoin::send::v2::SenderBuilder; use payjoin::{OhttpKeys, PjUri, UriExt}; @@ -724,7 +724,7 @@ mod integration { fn handle_directory_proposal( receiver: &bitcoincore_rpc::Client, - proposal: Receiver, + proposal: Receiver, custom_inputs: Option>, ) -> Result, BoxError> { let noop_persister = NoopSessionPersister::default(); @@ -1069,7 +1069,7 @@ mod integration { custom_inputs: Option>, ) -> Result { // Receiver receive payjoin proposal, IRL it will be an HTTP request (over ssl or onion) - let proposal = payjoin::receive::v1::UncheckedProposal::from_request( + let proposal = payjoin::receive::v1::UncheckedOriginalPayload::from_request( req.body.as_slice(), req.url.query().unwrap_or(""), headers, @@ -1082,7 +1082,7 @@ mod integration { } fn handle_proposal( - proposal: payjoin::receive::v1::UncheckedProposal, + proposal: payjoin::receive::v1::UncheckedOriginalPayload, receiver: &bitcoincore_rpc::Client, custom_outputs: Option>, drain_script: Option<&bitcoin::Script>, From 41332a8c59e65b040daec25e760b3b317f95f955 Mon Sep 17 00:00:00 2001 From: Yuval Kogman Date: Fri, 22 Aug 2025 16:56:40 +0200 Subject: [PATCH 3/4] Rename Original to OriginalPayload --- payjoin/src/core/receive/common/mod.rs | 8 +++---- payjoin/src/core/receive/mod.rs | 8 +++---- payjoin/src/core/receive/v1/mod.rs | 18 +++++++++------- payjoin/src/core/receive/v2/mod.rs | 29 ++++++++++++++------------ payjoin/src/core/receive/v2/session.rs | 6 +++--- 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/payjoin/src/core/receive/common/mod.rs b/payjoin/src/core/receive/common/mod.rs index 4f51cb413..ac9cf4f20 100644 --- a/payjoin/src/core/receive/common/mod.rs +++ b/payjoin/src/core/receive/common/mod.rs @@ -18,7 +18,7 @@ use super::optional_parameters::Params; use super::{InputPair, OutputSubstitutionError, ReplyableError, SelectionError}; use crate::output_substitution::OutputSubstitution; use crate::psbt::PsbtExt; -use crate::receive::{InternalPayloadError, Original, PsbtContext}; +use crate::receive::{InternalPayloadError, OriginalPayload, PsbtContext}; /// Typestate which the receiver may substitute or add outputs to. /// @@ -38,11 +38,11 @@ pub struct WantsOutputs { } impl WantsOutputs { - /// Create a new [`WantsOutputs`] typestate from an [`Original`] typestate and a list of + /// Create a new [`WantsOutputs`] typestate from an [`OriginalPayload`] typestate and a list of /// owned outputs. /// /// The first output in the `owned_vouts` list is used as the `change_vout`. - pub(crate) fn new(original: Original, owned_vouts: Vec) -> Self { + pub(crate) fn new(original: OriginalPayload, owned_vouts: Vec) -> Self { Self { original_psbt: original.psbt.clone(), payjoin_psbt: original.psbt, @@ -670,7 +670,7 @@ mod tests { assert_eq!(original.psbt_fee_rate().unwrap().to_sat_per_vb_floor(), 2); // Specify excessive fee rate in sender params original_params.min_fee_rate = FeeRate::from_sat_per_vb_unchecked(1000); - let updated_original = Original { psbt: original.psbt, params: original_params }; + let updated_original = OriginalPayload { psbt: original.psbt, params: original_params }; let proposal_psbt = Psbt::from_str(RECEIVER_INPUT_CONTRIBUTION).unwrap(); let input = InputPair::new( diff --git a/payjoin/src/core/receive/mod.rs b/payjoin/src/core/receive/mod.rs index 5e9198968..9d5ebf139 100644 --- a/payjoin/src/core/receive/mod.rs +++ b/payjoin/src/core/receive/mod.rs @@ -336,12 +336,12 @@ impl PsbtContext { } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct Original { +pub struct OriginalPayload { psbt: Psbt, params: Params, } -impl Original { +impl OriginalPayload { // Calculates the fee rate of the original proposal PSBT. fn psbt_fee_rate(&self) -> Result { let original_psbt_fee = self.psbt.fee().map_err(|e| { @@ -475,11 +475,11 @@ pub(crate) mod tests { // We should pub(crate) it and moved to a common place. const NON_WITNESS_DATA_WEIGHT: Weight = Weight::from_non_witness_data_size(32 + 4 + 4); - pub(crate) fn original_from_test_vector() -> Original { + pub(crate) fn original_from_test_vector() -> OriginalPayload { let pairs = url::form_urlencoded::parse(QUERY_PARAMS.as_bytes()); let params = Params::from_query_pairs(pairs, &[Version::One]) .expect("Could not parse params from query pairs"); - Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params } + OriginalPayload { psbt: PARSED_ORIGINAL_PSBT.clone(), params } } #[test] diff --git a/payjoin/src/core/receive/v1/mod.rs b/payjoin/src/core/receive/v1/mod.rs index 57267ff07..1f5f392f2 100644 --- a/payjoin/src/core/receive/v1/mod.rs +++ b/payjoin/src/core/receive/v1/mod.rs @@ -71,7 +71,7 @@ impl UncheckedOriginalPayload { let (psbt, params) = crate::receive::parse_payload(base64, query, SUPPORTED_VERSIONS) .map_err(ReplyableError::Payload)?; - Ok(Self { original: Original { psbt, params } }) + Ok(Self { original: OriginalPayload { psbt, params } }) } } @@ -94,7 +94,7 @@ impl UncheckedOriginalPayload { /// can go ahead with calling [`Self::assume_interactive_receiver`] to move on to the next typestate. #[derive(Debug, Clone)] pub struct UncheckedOriginalPayload { - original: Original, + original: OriginalPayload, } impl UncheckedOriginalPayload { @@ -136,7 +136,7 @@ impl UncheckedOriginalPayload { /// Call [`Self::check_inputs_not_owned`] to proceed. #[derive(Debug, Clone)] pub struct MaybeInputsOwned { - pub(crate) original: Original, + pub(crate) original: OriginalPayload, } impl MaybeInputsOwned { @@ -166,7 +166,7 @@ impl MaybeInputsOwned { /// Call [`Self::check_no_inputs_seen_before`] to proceed. #[derive(Debug, Clone)] pub struct MaybeInputsSeen { - original: Original, + original: OriginalPayload, } impl MaybeInputsSeen { /// Check that the receiver has never seen the inputs in the original proposal before. @@ -194,7 +194,7 @@ impl MaybeInputsSeen { /// Call [`Self::identify_receiver_outputs`] to proceed. #[derive(Debug, Clone)] pub struct OutputsUnknown { - original: Original, + original: OriginalPayload, } impl OutputsUnknown { @@ -409,14 +409,18 @@ mod tests { let pairs = url::form_urlencoded::parse(QUERY_PARAMS.as_bytes()); let params = Params::from_query_pairs(pairs, &[Version::One]) .expect("Could not parse params from query pairs"); - UncheckedOriginalPayload { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params } } + UncheckedOriginalPayload { + original: OriginalPayload { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, + } } fn maybe_inputs_owned_from_test_vector() -> MaybeInputsOwned { let pairs = url::form_urlencoded::parse(QUERY_PARAMS.as_bytes()); let params = Params::from_query_pairs(pairs, &[Version::One]) .expect("Could not parse params from query pairs"); - MaybeInputsOwned { original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params } } + MaybeInputsOwned { + original: OriginalPayload { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, + } } fn wants_outputs_from_test_vector(proposal: UncheckedOriginalPayload) -> WantsOutputs { diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index 50212d1ec..35d698a1e 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -52,7 +52,7 @@ use crate::persist::{ MaybeFatalTransition, MaybeFatalTransitionWithNoResults, MaybeSuccessTransition, MaybeTransientTransition, NextStateTransition, }; -use crate::receive::{parse_payload, InputPair, Original, PsbtContext}; +use crate::receive::{parse_payload, InputPair, OriginalPayload, PsbtContext}; use crate::uri::ShortId; use crate::{ImplementationError, IntoUrl, IntoUrlError, Request, Version}; @@ -385,7 +385,7 @@ impl Receiver { &mut self, body: &[u8], context: ohttp::ClientResponse, - ) -> Result)>, Error> { + ) -> Result)>, Error> { let body = match process_get_res(body, context) .map_err(InternalSessionError::DirectoryResponse)? { @@ -415,14 +415,17 @@ impl Receiver { ohttp_encapsulate(&mut self.context.ohttp_keys, "GET", fallback_target.as_str(), None) } - fn extract_proposal_from_v1(&mut self, response: &str) -> Result { + fn extract_proposal_from_v1( + &mut self, + response: &str, + ) -> Result { self.unchecked_from_payload(response) } fn extract_proposal_from_v2( &mut self, response: Vec, - ) -> Result<(Original, HpkePublicKey), Error> { + ) -> Result<(OriginalPayload, HpkePublicKey), Error> { let (payload_bytes, reply_key) = decrypt_message_a(&response, self.context.receiver_key.secret_key().clone())?; let payload = std::str::from_utf8(&payload_bytes) @@ -430,7 +433,7 @@ impl Receiver { self.unchecked_from_payload(payload).map_err(Error::ReplyToSender).map(|p| (p, reply_key)) } - fn unchecked_from_payload(&mut self, payload: &str) -> Result { + fn unchecked_from_payload(&mut self, payload: &str) -> Result { let (base64, padded_query) = payload.split_once('\n').unwrap_or_default(); let query = padded_query.trim_matches('\0'); log::trace!("Received query: {query}, base64: {base64}"); // my guess is no \n so default is wrong @@ -449,7 +452,7 @@ impl Receiver { params.output_substitution = OutputSubstitution::Disabled; } - let inner = Original { psbt, params }; + let inner = OriginalPayload { psbt, params }; Ok(inner) } @@ -460,7 +463,7 @@ impl Receiver { pub(crate) fn apply_unchecked_from_payload( self, - event: Original, + event: OriginalPayload, reply_key: Option, ) -> Result { if self.state.context.expiry < SystemTime::now() { @@ -486,7 +489,7 @@ impl Receiver { /// #[derive(Debug, Clone, PartialEq)] pub struct UncheckedOriginalPayload { - pub(crate) original: Original, + pub(crate) original: OriginalPayload, pub(crate) session_context: SessionContext, } @@ -577,7 +580,7 @@ impl Receiver { #[derive(Debug, Clone, PartialEq)] pub struct MaybeInputsOwned { - original: Original, + original: OriginalPayload, session_context: SessionContext, } @@ -643,7 +646,7 @@ impl Receiver { #[derive(Debug, Clone, PartialEq)] pub struct MaybeInputsSeen { - original: Original, + original: OriginalPayload, session_context: SessionContext, } @@ -701,7 +704,7 @@ impl Receiver { #[derive(Debug, Clone, PartialEq)] pub struct OutputsUnknown { - original: Original, + original: OriginalPayload, session_context: SessionContext, } @@ -1144,7 +1147,7 @@ pub mod test { let params = Params::from_query_pairs(pairs, &[Version::Two]) .expect("Test utils query params should not fail"); UncheckedOriginalPayload { - original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, + original: OriginalPayload { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, session_context: SessionContext { reply_key: Some(HpkeKeyPair::gen_keypair().public_key().clone()), ..SHARED_CONTEXT.clone() @@ -1157,7 +1160,7 @@ pub mod test { let params = Params::from_query_pairs(pairs, &[Version::Two]) .expect("Test utils query params should not fail"); MaybeInputsOwned { - original: Original { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, + original: OriginalPayload { psbt: PARSED_ORIGINAL_PSBT.clone(), params }, session_context: SessionContext { reply_key: Some(HpkeKeyPair::gen_keypair().public_key().clone()), ..SHARED_CONTEXT.clone() diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index 59685fdce..6036b510c 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -6,7 +6,7 @@ use super::{ReceiveSession, SessionContext}; use crate::output_substitution::OutputSubstitution; use crate::persist::SessionPersister; use crate::receive::v2::{extract_err_req, SessionError}; -use crate::receive::{common, JsonReply, Original, PsbtContext}; +use crate::receive::{common, JsonReply, OriginalPayload, PsbtContext}; use crate::{ImplementationError, IntoUrl, PjUri, Request}; /// Errors that can occur when replaying a receiver event log @@ -88,7 +88,7 @@ impl SessionHistory { }) } - fn get_unchecked_proposal(&self) -> Option { + fn get_unchecked_proposal(&self) -> Option { self.events.iter().find_map(|event| match event { SessionEvent::UncheckedOriginalPayload(proposal) => Some(proposal.0.clone()), _ => None, @@ -175,7 +175,7 @@ impl SessionHistory { /// Each event can be used to transition the receiver state machine to a new state pub enum SessionEvent { Created(SessionContext), - UncheckedOriginalPayload((Original, Option)), + UncheckedOriginalPayload((OriginalPayload, Option)), MaybeInputsOwned(), MaybeInputsSeen(), OutputsUnknown(), From 68608bf6ee79e7a2cb6a17a64833a7de06c46ad5 Mon Sep 17 00:00:00 2001 From: Yuval Kogman Date: Thu, 28 Aug 2025 22:10:41 +0200 Subject: [PATCH 4/4] mutation coverage for replyable errs w/ reply key Co-authored-by: Ben Allen --- payjoin/src/core/receive/v2/mod.rs | 33 +++++++----- payjoin/src/core/receive/v2/session.rs | 75 +++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 15 deletions(-) diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index 35d698a1e..500d93724 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -1168,6 +1168,22 @@ pub mod test { } } + pub(crate) fn mock_err() -> (String, JsonReply) { + let noop_persister = NoopSessionPersister::default(); + let receiver = Receiver { state: unchecked_proposal_v2_from_test_vector() }; + let server_error = || { + receiver + .clone() + .check_broadcast_suitability(None, |_| Err("mock error".into())) + .save(&noop_persister) + }; + + let error = server_error().expect_err("Server error should be populated with mock error"); + let res = error.api_error().expect("check_broadcast error should propagate to api error"); + let actual_json = JsonReply::from(&res); + (res.to_string(), actual_json) + } + #[test] fn test_v2_mutable_receiver_state_closures() { let persister = NoopSessionPersister::default(); @@ -1314,27 +1330,16 @@ pub mod test { #[test] fn test_extract_err_req() -> Result<(), BoxError> { - let noop_persister = NoopSessionPersister::default(); let receiver = Receiver { state: unchecked_proposal_v2_from_test_vector() }; - - let server_error = || { - receiver - .clone() - .check_broadcast_suitability(None, |_| Err("mock error".into())) - .save(&noop_persister) - }; - + let mock_err = mock_err(); let expected_json = serde_json::json!({ "errorCode": "unavailable", "message": "Receiver error" }); - let error = server_error().expect_err("Server error should be populated with mock error"); - let res = error.api_error().expect("check_broadcast error should propagate to api error"); - let actual_json = JsonReply::from(&res); - assert_eq!(actual_json.to_json(), expected_json); + assert_eq!(mock_err.1.to_json(), expected_json); - let (_req, _ctx) = extract_err_req(&actual_json, &*EXAMPLE_URL, &receiver.session_context)?; + let (_req, _ctx) = extract_err_req(&mock_err.1, &*EXAMPLE_URL, &receiver.session_context)?; let internal_error: ReplyableError = InternalPayloadError::MissingPayment.into(); let (_req, _ctx) = diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index 6036b510c..ef6f54ca7 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -199,7 +199,7 @@ mod tests { use crate::persist::test_utils::InMemoryTestPersister; use crate::persist::NoopSessionPersister; use crate::receive::tests::original_from_test_vector; - use crate::receive::v2::test::SHARED_CONTEXT; + use crate::receive::v2::test::{mock_err, SHARED_CONTEXT}; use crate::receive::v2::{ Initialized, MaybeInputsOwned, PayjoinProposal, ProvisionalProposal, Receiver, UncheckedOriginalPayload, @@ -595,4 +595,77 @@ mod tests { Ok(()) } + + #[test] + fn test_skipped_session_extract_err_request() -> Result<(), BoxError> { + let ohttp_relay = EXAMPLE_URL.clone(); + let mock_err = mock_err(); + + let session_history = SessionHistory { events: vec![SessionEvent::MaybeInputsOwned()] }; + let err_req = session_history.extract_err_req(&ohttp_relay)?; + assert!(err_req.is_none()); + + let session_history = SessionHistory { + events: vec![ + SessionEvent::MaybeInputsOwned(), + SessionEvent::SessionInvalid(mock_err.0.clone(), Some(mock_err.1.clone())), + ], + }; + + let err_req = session_history.extract_err_req(&ohttp_relay)?; + assert!(err_req.is_none()); + + let session_history = SessionHistory { + events: vec![ + SessionEvent::Created(SHARED_CONTEXT.clone()), + SessionEvent::MaybeInputsOwned(), + SessionEvent::SessionInvalid(mock_err.0.clone(), Some(mock_err.1.clone())), + ], + }; + + let err_req = session_history.extract_err_req(&ohttp_relay)?; + assert!(err_req.is_none()); + Ok(()) + } + + #[test] + fn test_session_extract_err_req_reply_key() -> Result<(), BoxError> { + let proposal = original_from_test_vector(); + let ohttp_relay = EXAMPLE_URL.clone(); + let mock_err = mock_err(); + + let session_history_one = SessionHistory { + events: vec![ + SessionEvent::Created(SHARED_CONTEXT.clone()), + SessionEvent::UncheckedOriginalPayload(( + proposal.clone(), + Some(crate::HpkeKeyPair::gen_keypair().1), + )), + SessionEvent::SessionInvalid(mock_err.0.clone(), Some(mock_err.1.clone())), + ], + }; + + let err_req_one = session_history_one.extract_err_req(&ohttp_relay)?; + assert!(err_req_one.is_some()); + + let session_history_two = SessionHistory { + events: vec![ + SessionEvent::Created(SHARED_CONTEXT.clone()), + SessionEvent::UncheckedOriginalPayload(( + proposal.clone(), + Some(crate::HpkeKeyPair::gen_keypair().1), + )), + SessionEvent::SessionInvalid(mock_err.0, Some(mock_err.1)), + ], + }; + + let err_req_two = session_history_two.extract_err_req(ohttp_relay)?; + assert!(err_req_two.is_some()); + assert_ne!( + session_history_one.session_context().unwrap().reply_key, + session_history_two.session_context().unwrap().reply_key + ); + + Ok(()) + } }