send: return error for v1 URIs in v2 sender by vbrekher · Pull Request #1870 · payjoin/rust-payjoin

vbrekher · 2026-09-09T13:23:03Z

SenderBuilder::new in the v2 sender currently panics when it receives a v1 Payjoin URI.

Make the constructor fallible instead and return a normal build error for unsupported v1 URIs. The FFI constructor propagates the same failure through SenderInputError::Build.

This keeps the v1 sender behavior unchanged while making v2 safe to use with untrusted Payjoin URIs.

AI assistance

ChatGPT was used to inspect the codebase, develop the patch, add tests, run validation, and draft this PR description.

Closes #1849

coveralls · 2026-09-09T20:21:10Z

Coverage Report for CI Build 34613345334

Coverage increased (+0.02%) to 86.663%

Details

Coverage increased (+0.02%) from the base build.
Patch coverage: 1 uncovered change across 1 file (24 of 25 lines covered, 96.0%).
No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
payjoin/src/core/send/error.rs	2	1	50.0%
Total (3 files)	25	24	96.0%

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	16571
Covered Lines:	14361
Line Coverage:	86.66%
Coverage Strength:	341.4 hits per line

💛 - Coveralls

chavic · 2026-09-09T20:31:55Z

Hi @vbrekher , thanks for picking this up. The added type UnsupportedVersion probably needs to match the same gating as the arm that uses it. Without that properly gated, I expect clipy to fail when only v2 or v1 is enabled

vbrekher · 2026-09-09T20:36:22Z

Good catch, thanks. I gated UnsupportedVersion and the corresponding Display/Error::source match arms with #[cfg(feature = "v1")] so they match the constructor arm that uses the variant. Pushed the fix; the new Actions runs are currently waiting for approval.

chavic · 2026-09-09T20:39:31Z

@vbrekher, logs show another unrelated error; you'll need to update the JS assertion too

vbrekher · 2026-09-09T20:43:36Z

Thanks, found it in the JS integration log. The v1 URI now fails at SenderBuilder construction with the typed SenderInputError.Build, so the old /RuntimeError/ assertion was stale. I updated the assertion to expect SenderInputError.Build and removed the now-unreachable buildRecommended(...) call from that check. Pushed as d94e568.

chavic · 2026-09-09T20:45:39Z

Good catch, thanks. I gated UnsupportedVersion and the corresponding Display/Error::source match arms with #[cfg(feature = "v1")] so they match the constructor arm that uses the variant. Pushed the fix; the new Actions runs are currently waiting for approval.

And you'll actually want all(feature = "v1", feature = "v2"), since only having one should still fail. Can edit that commit rather than adding a new one. f85b369, which is the space removal, also belongs to an edit to a commit rather than being its own commit

vbrekher · 2026-09-09T22:26:42Z

Yep, fixed. The gate is now all(feature = "v1", feature = "v2"), and I cleaned up the branch history so the follow-up fixes are folded into the original commit. I also rebased it onto the current master, so the PR is back to one commit and no longer drifted. New Actions runs are waiting for approval.

benalleng · 2026-09-10T16:37:22Z

nix fmt is what we use as the general formatter across the repo. https://github.com/payjoin/rust-payjoin/blob/master/.github/CONTRIBUTING.md has some good pre-commit hooks to use for the future

xstoicunicornx

There is small formatting CI error that needs to be addressed.

This would also be a breaking change to interface now that we are on v1.0.0, not sure what the protocol is for handling/documenting that.

One other piece of feedback below but otherwise looks good overall.

xstoicunicornx · 2026-09-10T17:12:00Z

-        new payjoin.SenderBuilder(psbt, pjUri).buildRecommended(
-            18446744073709551615n,
-        );
-    }, /RuntimeError/);
+        new payjoin.SenderBuilder(psbt, pjUri);
+    }, /SenderInputError\.Build/);


This test was for oversized fee rate in SenderBuilder::build_recommended. We should probably update this test to use v2 URI instead to avoid test regression.

Maybe we want to add tests for handling the v1 URI error for all the bindings as well?

Good point. I restored the oversized fee-rate regression and switched that assertion to a v2 URI. The v1 constructor failure is now a separate assertion, so build_recommended overflow coverage is preserved. I also folded the follow-up back into the single PR commit and restored the final newline. Current head: 2fd412e.

Signed-off-by: v ₿ <valentin.brekher@gmail.com>

xstoicunicornx · 2026-09-11T19:27:35Z

Looks like integration test is failing, did you run tests locally? You can test by running bash contrib/test.sh from the javascript directory.

vbrekher requested review from DanGould, benalleng and spacebear21 as code owners September 9, 2026 13:23

vbrekher force-pushed the fix/v1-sender-uri-error branch from 798471f to 990ca19 Compare September 9, 2026 13:27

vbrekher force-pushed the fix/v1-sender-uri-error branch 2 times, most recently from ec30fce to 34cc3a4 Compare September 9, 2026 22:26

xstoicunicornx reviewed Sep 10, 2026

View reviewed changes

send: return error for v1 URIs in v2 sender

2fd412e

Signed-off-by: v ₿ <valentin.brekher@gmail.com>

vbrekher force-pushed the fix/v1-sender-uri-error branch from b2a0ed3 to 2fd412e Compare September 11, 2026 14:58

Navigation Menu

Uh oh!

send: return error for v1 URIs in v2 sender - #1870

send: return error for v1 URIs in v2 sender#1870
vbrekher wants to merge 1 commit into
payjoin:masterfrom
vbrekher:fix/v1-sender-uri-error

vbrekher commented Sep 9, 2026 •

edited

Loading

Uh oh!

coveralls commented Sep 9, 2026 •

edited

Loading

Uh oh!

chavic commented Sep 9, 2026

Uh oh!

vbrekher commented Sep 9, 2026

Uh oh!

chavic commented Sep 9, 2026

Uh oh!

vbrekher commented Sep 9, 2026

Uh oh!

chavic commented Sep 9, 2026 •

edited

Loading

Uh oh!

vbrekher commented Sep 9, 2026

Uh oh!

benalleng commented Sep 10, 2026

Uh oh!

xstoicunicornx left a comment

Uh oh!

xstoicunicornx Sep 10, 2026

Uh oh!

vbrekher Sep 11, 2026

Uh oh!

xstoicunicornx commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

vbrekher commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI assistance

Uh oh!

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

Uh oh!

Coverage Report for CI Build 34613345334

Coverage increased (+0.02%) to 86.663%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

chavic commented Sep 9, 2026

Uh oh!

vbrekher commented Sep 9, 2026

Uh oh!

chavic commented Sep 9, 2026

Uh oh!

vbrekher commented Sep 9, 2026

Uh oh!

chavic commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbrekher commented Sep 9, 2026

Uh oh!

benalleng commented Sep 10, 2026

Uh oh!

xstoicunicornx left a comment

Choose a reason for hiding this comment

Uh oh!

xstoicunicornx Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

vbrekher Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

xstoicunicornx commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vbrekher commented Sep 9, 2026 •

edited

Loading

coveralls commented Sep 9, 2026 •

edited

Loading

chavic commented Sep 9, 2026 •

edited

Loading