Expose fail() and cancel() on Receiver session by spacebear21 · Pull Request #1134 · payjoin/rust-payjoin

spacebear21 · 2025-10-01T22:12:23Z

This gives implementers the ability to manually fail or cancel a receiver Payjoin session. A user might cancel a session for any reason, or the wallet might fail it if e.g. it detects a double-spend or cannot proceed past a transient error.

Note that the unit tests for closed sessions will need to be reworked once #1132 is merged, since the expected_receiver_state should be an error.

I used Claude sonnet 4.5 (it's quite good) to write the generic methods and unit tests.

Pull Request Checklist

Please confirm the following before requesting review:

I have disclosed my use of
AI
in the body of this PR.
I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

coveralls · 2025-10-01T22:16:27Z

Pull Request Test Coverage Report for Build 18177469540

Details

56 of 56 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.08%) to 84.648%

Totals
Change from base Build 18176477486:	0.08%
Covered Lines:	8756
Relevant Lines:	10344

💛 - Coveralls

This gives implementers the ability to manually fail or cancel a receiver Payjoin session. A user might cancel a session for any reason, or the wallet might fail it if e.g. it detects a double-spend or cannot proceed past a transient error.

arminsabouri

CodeAck

arminsabouri · 2025-10-02T13:34:59Z

+    ///
+    /// This method allows implementations to terminate the payjoin session when
+    /// the user decides to cancel the operation interactively.
+    pub fn cancel<P>(self, persister: &P) -> Result<(), P::InternalStorageError>


Do you think cancel is simpler than just calling close() on the persister?

The difference is that cancel() (and fail()) ensures that a Closed event is added to the log before closing.

arminsabouri · 2025-10-02T13:40:10Z

+    /// This method allows implementations to terminate the payjoin session when
+    /// they encounter errors that cannot be resolved, such as insufficient
+    /// funds or a double-spend detection.
+    pub fn fail<P>(self, persister: &P) -> Result<(), P::InternalStorageError>


Should we push a generic failure error reply? Otherwise the sender will be hanging until expiration.

Yes good point, we do need a way to communicate this (and probably cancel() too) back to the sender.

I started down an approach where they transition to HasReplyableError to enforce responding to the sender. However there is an outstanding issue in that it needs to differentiate between Cancel and Failure outcomes when replaying the event log, so I think GotReplyableError would need to carry this information somehow.

I'm not sure if we actually need to reply in case of Cancel, assuming the receiver would broadcast the fallback tx?

Cancel sounds like the unavailable wellKnownError to me, even if broadcast is sufficient

The only reason I can think of for a receiver to do this would be if the receiver wanted to refuse a naive tx.

A better solution for this example would be having the receiver use req-pj

So for now letting it expire is fine.

benalleng · 2025-10-03T20:27:04Z

I wanted to pose a question as to why we really need to have these 2 seperate instead of just having application devs use fail() OR cancel() and provide a reason instead of having both of these?

spacebear21 · 2026-03-25T15:25:31Z

+    /// Explicitly fail the session.
+    ///
+    /// This method allows implementations to terminate the payjoin session when
+    /// they encounter errors that cannot be resolved, such as insufficient
+    /// funds or a double-spend detection.
+    pub fn fail(self) -> NextStateTransition<SessionEvent, Receiver<HasReplyableError>> {
+        let err = JsonReply::new(crate::error_codes::ErrorCode::Unavailable, "Receiver error");
+        NextStateTransition::success(
+            SessionEvent::GotReplyableError(err.clone()),
+            Receiver {
+                state: HasReplyableError { error_reply: err, outcome: SessionOutcome::Failure },
+                session_context: self.session_context,
+            },
+        )
+    }
+


Since failure encapsulates "protocol violations" this method seems unnecessary, everything user-initiated would be a "cancel"

spacebear21 · 2026-03-25T15:26:21Z

+    pub fn cancel(self) -> NextStateTransition<SessionEvent, Receiver<HasReplyableError>> {
+        let err = JsonReply::new(crate::error_codes::ErrorCode::Unavailable, "Receiver error");
+        NextStateTransition::success(
+            SessionEvent::GotReplyableError(err.clone()),


This should transition to "Monitoring" for the fallback transaction

I think I've convinced myself that transitioning to Receiver<Monitor> on cancel() just for monitoring the fallback tx is overkill.

It would require refactoring the Monitor typestate to make PsbtContext optional, since we may not have a payjoin psbt when calling cancel().

It would also require a new SessionEvent type like CancelInitiated that transitions to Monitor "early" during replay.

It introduced ambiguity around the role of Monitor typestate. Currently this state is where we monitor for a set of possible transactions that the sender may broadcast. Monitoring on cancel() would mean we're now also monitoring a transaction broadcast by ourselves. The current SessionOutcome(FallbackBroadcasted) implies the session was closed by the sender. Now it might actually be the receiver who did? And shouldn't we be closing with SessionOutcome(Cancel) anyway?

The wallet already knows to track a tx that it broadcasted itself, and going through the monitor typestate for confirmation doesn't add any useful additional information that I can think of, especially considering all this additional complexity.

So I think cancel() should simply a) immediately close the session with SessionOutcome(Cancel) and b) return the fallback transaction to broadcast.

DanGould · 2026-04-21T15:02:16Z

superseded by #1470

spacebear21 requested a review from arminsabouri October 1, 2025 22:12

spacebear21 force-pushed the fail-cancel-session branch from e4008b1 to c373ec1 Compare October 1, 2025 22:13

spacebear21 force-pushed the fail-cancel-session branch from c373ec1 to 7c56df0 Compare October 1, 2025 22:29

arminsabouri approved these changes Oct 2, 2025

View reviewed changes

wip: transition to HasReplyableError

0bda385

spacebear21 marked this pull request as draft October 2, 2025 20:51

spacebear21 added this to the payjoin-1.0 milestone Oct 3, 2025

spacebear21 commented Mar 25, 2026

View reviewed changes

spacebear21 mentioned this pull request Apr 8, 2026

Receive cancel() #1470

Merged

2 tasks

DanGould closed this Apr 21, 2026

Conversation

spacebear21 commented Oct 1, 2025

Uh oh!

coveralls commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18177469540

Details

💛 - Coveralls

Uh oh!

arminsabouri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanGould Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benalleng commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanGould commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coveralls commented Oct 1, 2025 •

edited

Loading

DanGould Oct 3, 2025 •

edited

Loading

benalleng commented Oct 3, 2025 •

edited

Loading