Fix rpc-error due to quick transaction lookup by zealsham · Pull Request #1159 · payjoin/rust-payjoin

zealsham · 2025-10-09T12:34:05Z

This pr addresses an error that occurs due to rpc client looking up the transaction right before it's on chain . Error: Transient error: Implementation error: Obtained failure status(500): Internal Server Error

The delay introduced allows the transaction to broadcasted on chain before the lookup

This closes #1154

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.

spacebear21 · 2025-10-09T15:15:54Z

Is this issue reproducible in the e2e tests? It would be nice to add an assertion that fails before this patch and succeeds with the patch - otherwise this timeout will get removed by someone else in a few months when we've forgotten why it was added.

zealsham · 2025-10-09T18:29:04Z

The time delay is making the test take forever as it blocks the entire tokio runtime . i'll figure out a diff approach

DanGould · 2025-10-09T18:36:19Z

"By default, the ZeroMQ feature is automatically compiled in if the necessary prerequisites are found" Are the necessary prereqs typically found? Waiting for an event is definitely the 'right' way to do this, but a much shorter timeout, even a function that polls on a much shorter timeout, would also be a hack I'm ok with merging.

coveralls · 2025-10-09T20:54:04Z

Pull Request Test Coverage Report for Build 18781657652

Details

22 of 32 (68.75%) changed or added relevant lines in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.04%) to 83.698%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin-cli/src/app/v2/mod.rs	22	32	68.75%

Files with Coverage Reduction	New Missed Lines	%
payjoin/src/core/receive/v2/mod.rs	2	90.84%

Totals
Change from base Build 18761146827:	-0.04%
Covered Lines:	8990
Relevant Lines:	10741

💛 - Coveralls

DanGould

A single timeout is gonna be too flaky and take a long time. IMO what you want is a separate method that loops on a small delay (~200ms) between making calls, and times out after looping for a longer time (~10s) and the timeout produces an error. A little polling code complexity will make a far more robust call for various platforms.

payjoin-cli/src/app/v2/mod.rs

zealsham · 2025-10-13T10:07:01Z

Is this issue reproducible in the e2e tests? It would be nice to add an assertion that fails before this patch and succeeds with the patch - otherwise this timeout will get removed by someone else in a few months when we've forgotten why it was added.

i've pondered on this and haven't been able to figure out how to trigger this in e2e

spacebear21

The quick-lookup error only occurs when the receiver is not interrupted prior to monitoring. In the e2e test, the receiver is interrupted then resumed after the sender has processed the payjoin tx. I think we should be able to reproduce the error in a e2e test that does not interrupt and resume the receiver and does a synchronous payjoin.

spacebear21 · 2025-10-22T15:53:49Z

payjoin-cli/src/app/v2/mod.rs

+            // Check if we've exceeded the wait time
+            // since the payment will eventually come through, wondering if i should just remove this check
+            if start.elapsed() >= timeout_duration {
+                return Err(anyhow!(
+                    "Timeout waiting for payment confirmation after {:?}",
+                    timeout_duration
+                ));
+            }
+


This check is necessary in a production implementation because the payment is not guaranteed to come through if the sender doesn't broadcast it. Instead of erroring out completely we could e.g. prompt the user on whether they want to broadcast the fallback transaction, or wait longer for the payjoin transaction. However we could do this in a follow-up PR after this bug is fixed.

spacebear21 · 2025-10-22T15:55:14Z

payjoin-cli/src/app/v2/mod.rs

+
+            match result {
+                Ok(_) => {
+                    println!("Payment confirmed!");


Suggested change

println!("Payment confirmed!");

println!("Payjoin transaction {txid} detected in the mempool!");

will a simple "payjoin transaction detected in the mempool" work ? , getting the txid outside checkpayment function scope will need changes to the api , making private fields public

payjoin-cli/src/app/v2/mod.rs

This pr addresses an error that occurs due to rpc client looking up the transaction right before it's on chain . Error: Transient error: Implementation error: Obtained failure status(500): Internal Server Error The delay introduced allows the transaction to broadcasted on chain before the lookup fix format issue fix long pooling test Implement pooling mechanism for transaction check Implement pooling mechanism for transaction check fix rpc error fix rpc error fix lint error

spacebear21

tACK dc9c4f4

spacebear21

tACK dc9c4f4

spacebear21 · 2025-10-24T15:10:59Z

payjoin-cli/src/app/v2/mod.rs

+                        // keep polling
+


nit: unnecessary lines

addressed feedback

zealsham closed this Oct 9, 2025

zealsham reopened this Oct 9, 2025

zealsham force-pushed the rpc-cli-error branch from 718e33f to 731ec24 Compare October 9, 2025 20:43

DanGould previously requested changes Oct 10, 2025

View reviewed changes

DanGould reviewed Oct 10, 2025

View reviewed changes

payjoin-cli/src/app/v2/mod.rs Outdated Show resolved Hide resolved

zealsham force-pushed the rpc-cli-error branch from ff92353 to bf7e789 Compare October 10, 2025 18:48

zealsham requested a review from DanGould October 10, 2025 20:55

spacebear21 reviewed Oct 22, 2025

View reviewed changes

zealsham force-pushed the rpc-cli-error branch from bf7e789 to 0944820 Compare October 24, 2025 13:00

zealsham force-pushed the rpc-cli-error branch from 0944820 to dc9c4f4 Compare October 24, 2025 13:44

spacebear21 approved these changes Oct 24, 2025

View reviewed changes

payjoin-cli/src/app/v2/mod.rs

Comment on lines +773 to +774

// keep polling

Copy link

Collaborator

spacebear21 Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary lines

spacebear21 merged commit 0284a9d into payjoin:master Oct 24, 2025
10 checks passed

	println!("Payment confirmed!");
	println!("Payjoin transaction {txid} detected in the mempool!");

Conversation

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

Uh oh!

Uh oh!

spacebear21 commented Oct 9, 2025

Uh oh!

zealsham commented Oct 9, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Pull Request Test Coverage Report for Build 18781657652

Details

💛 - Coveralls

Uh oh!

DanGould left a comment • 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!

Uh oh!

zealsham commented Oct 13, 2025

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

spacebear21 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

spacebear21 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

zealsham Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

spacebear21 Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zealsham commented Oct 9, 2025 •

edited

Loading

DanGould commented Oct 9, 2025 •

edited

Loading

coveralls commented Oct 9, 2025 •

edited

Loading

DanGould left a comment •

edited

Loading