Demonstrate failure handling with fallback TX broadcast by zealsham · Pull Request #1164 · payjoin/rust-payjoin

zealsham · 2025-10-16T11:54:39Z

As a refrence implementation , payjoin-cli needs to demonstrate proper fallback transaction handling in the case of payjon failure. From what i see , both sender and receiver can broadcast the fall back transaction , This pr starts by handling the sender side of things and would eventually progress to the receiver .

This is still a work in progress

this addresses #1156 .

handle v1 sender broadcasting fallback
handle v2 sender broadcasting fallback
handle v2 receiver broadcasting fallback
check that transaction is broadcastable before broadasting

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-16T11:58:37Z

Coverage Report for CI Build 24482436745

Warning

No base build found for commit 592f914 on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 84.48%

Details

Patch coverage: 12 uncovered changes across 1 file (76 of 88 lines covered, 86.36%).

Uncovered Changes

File	Changed	Covered	%
payjoin-cli/src/app/v2/mod.rs	88	76	86.36%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	12835
Covered Lines:	10843
Line Coverage:	84.48%
Coverage Strength:	411.48 hits per line

💛 - Coveralls

zealsham · 2025-11-06T20:14:26Z

@arminsabouri i implemented error downcast method for v2 in this PR, the downside of this method is having to repeat the same block of code for the the different state of the senders session. I only did it for the "pooling proposal" phase of the sender session, i'll have to do it for the rest of the phases but just want to know your input on this before i head that direction.

i'm also simutaneously working on the "session outcome" based solution with the draft PR from you .

arminsabouri · 2026-01-22T22:25:39Z

+                        self.process_pj_response(psbt)?;
+                        Ok(())
+                    }
+                    Err(e) => {


I'm not sure we should be falling back on any Error. We should be doing something similar to what you did below and only fallback on a FatalError -- an irrecoverable protocol deviation.

which of the error types/variant is safe for fallback broadcast .

these two ResponseError::WellKnown() | ResponseError::Unrecognized are the ideal ones IMO. however from https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#user-content-Receivers_well_known_errors , i don't think every variant needs fallback broadcasted

i don't think every variant needs fallback broadcasted

Why not? You could argue unavailable doesn't require a fallback to be broadcasted.

Still think we should this @zealsham . Lets double check this against the spec

arminsabouri · 2026-01-22T22:48:52Z

+                    Err(e) => {
+                        // Check if it's a PersistedError with an API error
+                        if let Some(persisted_error) = e.downcast_ref::<PersistedError<
+                            EncapsulationError,


Why downcast specifically to a EncapsulationError? Are there not other Fatal error types?
Regardless, if we want to go this route, we should explore the changes neccecary in the API to make this more ergonomic. and idiomatic
e.is_fatal() or e.should_fallback().

Alternatively we could explore integrating this change into the typestate it self. I.e the error statemachine scrutinizes the error type and decides what to do. Leaving the developer entirely out of it.

If we want to developer to decide how to react then we should explore including Outcome in close() #1181

I would like to talk through the options a bit more before commiting to this route. Let me know your thoughts @zealsham

cc @spacebear21

i think the developer should ultimately decide what to do if a fatal error occurs. from the BIP

At any point, either party may choose to broadcast the fallback transaction described by the Original PSBT instead of proceeding

I believe some implementers might want the sender doing that or the receiver doing that, exposing outcome in close() handles that properly, automatically broadcasting fallback TX doesn't seem like what most devs will want.

Also i'll have to go through #1181 again , as i've lost most of the context.

arminsabouri · 2026-03-25T15:45:30Z

                self.process_pj_response(proposal)?;
-                return Ok(());
+                Ok(())
            }


Can we just have a another match here for a failure case? Instead of scrutinizing the error types above

arminsabouri · 2026-03-25T15:54:02Z

@zealsham take a look at lightningdevkit/ldk-node#746.
They are doing something similar

bc1cindy · 2026-04-02T12:15:59Z

-            }
-            _ => return Err(anyhow!("Unexpected sender state")),
+            SendSession::Closed(outcome) => match outcome {
+                SenderSessionOutcome::Failure => {


could Failure and Cancel be collapsed with different log messages?

Thanks for catching this .
"Payjoin session was canceled, broadcasting fallback" sounds appropirate for the cancel outcome ?

arminsabouri

Approach ACK. This is moving in the right direction! We should think through how we respond the unknown response errors from the reciever (arguably that could be its own PR if you want).

arminsabouri · 2026-04-02T15:46:43Z

+                        self.process_pj_response(psbt)?;
+                        Ok(())
+                    }
+                    Err(e) => {


Still think we should this @zealsham . Lets double check this against the spec

arminsabouri · 2026-04-02T15:49:56Z

                let receiver_pubkey = pj_param.receiver_pubkey();
-                let sender_state =
-                    self.db.get_send_session_ids()?.into_iter().find_map(|session_id| {
+                let (sender_state, persister, fallback_tx) =


Do we have to pass fallback_tx around the app. IIRC with the persister + session id you can always replay the session and obtain the fallback via the session history.

Still think we should this @zealsham . Lets double check this against the spec

Yes, This handles V1 sender flow, How much of a reward it is depends on how much V1 payjoin transaction are occuring. Since payjoin-cli is a reference example, we should demonstrate all cases even if a bulk of payjoin-TX would be v2

I think you answered a question from a different thread. Would still appreaciate an answer on this one

ohh, my mistake @arminsabouri . The fallback is gotten from replay_event just once. i could avoid that in the None branch but that would require me running the replay_session_event again. so as a trade off i kept the fallback in hand to avoid another redundant replay_event_ call.

Replaying is not a computationally heavy task (there are bounded number of events emitted from the API) and if it reduces code complexity I think this is worth it. To some extent we dont even need a persister object either -- thats a seperate problem.

Something like #1197 would also help in this situtation. i.e the fallback is always available on the top most sender struct. Sender.fallback_tx -> bitcoin::Transaction . For now I think its fine to just replay the events when we need to fallback.

arminsabouri · 2026-04-13T14:00:14Z

                let receiver_pubkey = pj_param.receiver_pubkey();
-                let sender_state =
-                    self.db.get_send_session_ids()?.into_iter().find_map(|session_id| {
+                let (sender_state, persister, fallback_tx) =


I think you answered a question from a different thread. Would still appreaciate an answer on this one

arminsabouri · 2026-04-13T14:02:54Z

+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+    async fn process_sender_session_broadcasts_fallback_on_failure() {


Suggested change

async fn process_sender_session_broadcasts_fallback_on_failure() {

async fn sender_processes_fallback_on_failure() {

arminsabouri · 2026-04-13T14:05:56Z

+                        "closed without success"
+                    } else {
+                        "canceled"
+                    };


This label seems unnecessary. Is this log read anywhere?

The label isn't logged , just printed. it should be logged, the label basically tells why the fallback tx was broadcasted, it exist to seperate cancel scenario from failure scenario

Ok in that case I would just remove "success" from the closed message.Also as a developer I wouldn't really know the difference between closed / cancled from this message. A better word may be "failed". Or event better mention why it failed. e.g display the error message.

arminsabouri · 2026-04-13T14:09:55Z

+            tokio::spawn(async move {
+                let mut stdout = tokio::io::stdout();
+                while let Some(line) =
+                    lines.next_line().await.expect("Failed to read line from stdout")
+                {
+                    stdout
+                        .write_all(format!("{line}\n").as_bytes())
+                        .await
+                        .expect("Failed to write to stdout");
+                    if line.contains("Fallback transaction broadcasted") {
+                        let _ = tx.send(true).await;
+                        break;
+                    }
+                }


Nit: Noticing some duplicate code. The test above has pretty much the same structure. Can we have a common method? Marked as nit. We can postpone for a follow up PR.

okay, i'll do the DRY in followup PR

arminsabouri · 2026-04-13T14:10:49Z

+                .arg("--port")
+                .arg("0")
+                .arg("--pj-endpoint")
+                .arg("https://dfdkfkdj.codm")


The other tests set this to let pj_endpoint = "https://localhost";

I used a random domain there, i'll set to localhost as anyone can just register that domain in theory

arminsabouri · 2026-04-13T14:11:26Z


+    #[cfg(feature = "v1")]
+    #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+    async fn send_receive_payjoin_v1_fallback_transaction_handling() -> Result<(), BoxError> {


Suggested change

async fn send_receive_payjoin_v1_fallback_transaction_handling() -> Result<(), BoxError> {

async fn payjoin_v1_fallback_transaction_handling() -> Result<(), BoxError> {

arminsabouri · 2026-04-13T14:20:35Z

+        })
+        .await?;
+
+        assert!(payjoin_sent, "Payjoin send was not detected");


How does the sender decide to actually fallback ?

so two error type triggers fallback for v1, the reciever pj-ednpoint being an unreachable host triggers the fallback on the sender part

If the http call succeeds but the receiver returns an error response or an invalid psbt ctx.process_response returns Err, and the same fallback broadcast happens.

As a refrence implementation , payjoin-cli needs to demonstrate proper fallback transaction handling in the case of payjon failure. From what i see , both sender and receiver can broadcast the fall back transaction , This pr starts by handling the sender side of things and would eventually progress to the receiver . this addresses payjoin#1156 This is mostly sender fallback handling. Broadcast on Sessionoutcome

zealsham marked this pull request as draft October 16, 2025 11:59

zealsham force-pushed the handle-fallback-tx branch from 9597ed0 to b25f2a6 Compare October 16, 2025 12:07

arminsabouri mentioned this pull request Oct 28, 2025

WIP - Include outcome in close #1181

Closed

2 tasks

zealsham force-pushed the handle-fallback-tx branch from 3d44e43 to 44354f1 Compare November 9, 2025 16:44

zealsham force-pushed the handle-fallback-tx branch from 59fd8f2 to 1c4175e Compare December 1, 2025 03:54

arminsabouri mentioned this pull request Dec 4, 2025

Failure to contribute inputs should result in reciverer fallingback #1213

Open

zealsham force-pushed the handle-fallback-tx branch from 1c4175e to 069fa34 Compare January 21, 2026 23:05

zealsham marked this pull request as ready for review January 22, 2026 12:16

zealsham requested review from DanGould, arminsabouri and spacebear21 January 22, 2026 12:16

zealsham changed the title ~~WIP:Demonstrate failure handling with fallback TX broadcast~~ Demonstrate failure handling with fallback TX broadcast Jan 22, 2026

arminsabouri requested changes Jan 22, 2026

View reviewed changes

arminsabouri requested changes Mar 25, 2026

View reviewed changes

zealsham force-pushed the handle-fallback-tx branch 2 times, most recently from 48b8064 to 154a49b Compare March 27, 2026 10:43

zealsham requested a review from arminsabouri March 30, 2026 11:16

DanGould removed their request for review March 31, 2026 13:33

bc1cindy reviewed Apr 2, 2026

View reviewed changes

arminsabouri reviewed Apr 2, 2026

View reviewed changes

zealsham force-pushed the handle-fallback-tx branch from 154a49b to a011f7a Compare April 5, 2026 20:18

zealsham requested review from arminsabouri and bc1cindy April 6, 2026 10:57

arminsabouri reviewed Apr 13, 2026

View reviewed changes

DanGould mentioned this pull request Apr 14, 2026

Receive cancel() #1470

Merged

2 tasks

zealsham force-pushed the handle-fallback-tx branch from a011f7a to e15bbe5 Compare April 15, 2026 22:20

zealsham force-pushed the handle-fallback-tx branch from e15bbe5 to 745a1f4 Compare April 15, 2026 22:50

	async fn process_sender_session_broadcasts_fallback_on_failure() {
	async fn sender_processes_fallback_on_failure() {

	async fn send_receive_payjoin_v1_fallback_transaction_handling() -> Result<(), BoxError> {
	async fn payjoin_v1_fallback_transaction_handling() -> Result<(), BoxError> {

Conversation

zealsham commented Oct 16, 2025 • edited by arminsabouri Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Coverage Report for CI Build 24482436745

Coverage: 84.48%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

zealsham commented Nov 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arminsabouri Mar 26, 2026 • 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!

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!

arminsabouri commented Mar 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

zealsham commented Oct 16, 2025 •

edited by arminsabouri

Loading

coveralls commented Oct 16, 2025 •

edited

Loading

arminsabouri Mar 26, 2026 •

edited

Loading