fix(payjoin): correct malformed receive BIP21 URI

[ethicnology]

Payjoin receive URI — fix + current limitation

What was wrong

Our receive payment request was rebuilt through dart:core Uri, which
percent-encoded the pj endpoint's :// (HTTPS%3A%2F%2F…) and dropped
pjos. The result was an out-of-spec, malformed BIP21 URI.

The fix

We now take the PDK-produced URI (already valid) and append amount/message
verbatim instead of re-encoding it. The pj endpoint stays byte-identical and
last (BIP-77 SHOULD), and we advertise pjos=0 when the PDK omits it — which
matches our receiver, since it never substitutes outputs. Covered by unit
tests: old-behavior characterization, new generator, and a decode round-trip.

• The over-encoding and missing pjos were our bug (BULL generation), not
  the bip21_uri package — the package's encoder keeps :// clean.
• The package's decoder does mutate the pj (force-uppercase, -→+). That's
  a separate, known issue, documented as skipped tests.

⚠️ Still locked on payjoin 0.23.0

We have not changed the payjoin engine. 0.23.0 uses the old + fragment
separator; final BIP-77 uses -. So:

• This fix makes our URI clean and in-spec for the 0.23 wire format — it does
  not fix cross-version interop.
• The bip21_uri decoder's -→+ rewrite is currently load-bearing: it's
  what lets us accept a final-BIP-77 (-) URI on the send path against the 0.23
  PDK. Removing it before upgrading would break sending.
• So the package cleanup and the PDK upgrade to a final-BIP-77 release must
  land together — not before.

[claude]

Claude finished @ethicnology's task in 2m 10s —— View job

---

Review: payjoin receive URI fix

• Read changed files & gather context
• Analyze URI generation logic for correctness
• Review tests
• Post review findings

Solid, well-scoped fix. Building the URI by string-appending to the PDK output instead of round-tripping through dart:core Uri is the right call — Uri percent-encodes :// and reorders/drops params. The pj endpoint stays byte-identical and last (BIP-77 SHOULD), pjos handling is correct, and the tests cover the important cases (byte-identity, pjos inject/dedup, ordering, decode round-trip). Nice characterization tests.

🟡 1. Small amounts render in scientific notation → out-of-spec BIP21

receive_state.dart:166

if (amountBtc > 0) 'amount=$amountBtc',

amountBtc is a double, and Dart's double.toString() switches to exponential notation once the exponent drops below -6:

amount (BTC)	sats	toString()
0.000001	100	0.000001 ✓
0.0000001	10	1e-7 ✗
0.00000001	1	1e-8 ✗

So a receive request for under 100 sats produces amount=1e-8, which is not a valid BIP21 amount. Amounts ≥ 100 sats coming from satsToBtc are fine (each is the shortest round-tripping decimal, so no 0.30000000000000004-style artifacts).

Note this is not a regression — the existing non-payjoin path has the identical bug at receive_state.dart:114 (confirmedAmountBtc.toString()). But since this PR's goal is spec-compliant URIs, it's worth fixing both in one spot, e.g. a helper that formats with 8 decimals and trims trailing zeros.

Fix this →

✅ Things I checked that are correct

• Query-param injection is prevented. note is the only user-controlled input and goes through Uri.encodeQueryComponent (receive_state.dart:167), which escapes &, =, #, etc. A note like &pj=evil becomes %26pj%3Devil — it can't inject a competing pj/pjos param. Good.
• pj stays last in all branches: extras are prepended, pdkQuery (which ends in pj=…) is appended last, even when the PDK already supplied pjos.
• No pjos duplication — contains('pjos=') guard works, and the extras.isEmpty → return pjUri short-circuit matches the "nothing to add" test.
• Space → + in message is consistent with what the bip21_uri decoder normalizes to (round-trip test confirms).

🔎 Minor — please confirm

The base (bitcoin:<address>) now comes from the PDK URI rather than bitcoinAddress!.address. That's almost certainly intended (the pj receiver address), but worth a sanity check that the address shown for a payjoin receive is meant to be the PDK's address and not necessarily equal to bitcoinAddress!.address.

---

Overall: correct and a clear improvement. The only actionable item is the small-amount formatting (low severity — sub-100-sat receives are uncommon — but cheap to fix while you're here).

[ethicnology]

Good catch @claude on the amount formatting. Added formatBtcAmount (toStringAsFixed(8) + trim) and applied it to all three receive paths (payjoin, bitcoin, liquid), so the pre-existing non-payjoin bug is covered too. New tests assert 1/10/100-sat amounts render as plain decimals.

Re: the base address — intended. The payjoin receiver is created with bitcoinAddress.address (createReceiver(address: ...)), so the PDK URI's base address is that same address. Taking it from the PDK URI just avoids re-assembling a string we already have in valid form.

Is it good enough now @claude ?

[claude]

Claude finished @ethicnology's task in 1m 19s —— View job

---

Re-review: amount formatting fix

• Read updated receive_state.dart & test diff
• Verify formatBtcAmount applied to all three paths
• Review the new tests
• Final verdict

Yes — this resolves the issue. 👍

formatBtcAmount is correct and applied everywhere. I traced all three call sites:

• Bitcoin (non-payjoin) — receive_state.dart:114 now formatBtcAmount(confirmedAmountBtc) ✓
• Liquid — receive_state.dart:128 ✓
• Payjoin — inside buildPayjoinPaymentRequest (amount=${formatBtcAmount(amountBtc)}) ✓

So the pre-existing non-payjoin bug is fixed in the same pass, as you said.

The implementation is sound. toStringAsFixed(8) always emits fixed-point (never scientific) and rounds to sat precision, so it also incidentally kills any 0.30000000000000004-style float artifact — a small bonus over the "shortest round-tripping decimal" reasoning from my last review. The trailing-zero trim is safe because toStringAsFixed(8) guarantees a . with 8 fractional digits, so 0+$ can only ever strip fractional zeros, and \.$ cleans up the integer case (1.0 → "1.00000000" → "1." → "1"). No special characters in the output, so passing it through dart:core Uri's queryParameters on the bitcoin/liquid paths doesn't re-encode it.

Tests look good. 1/10/100-sat cases assert plain decimals, and the trailing-zero trim is covered (1.0 → "1", 0.001 → "0.001"). Combined with the existing byte-identity / pjos / ordering / decode round-trip tests, the behavior is well pinned.

Base-address question — confirmed resolved. Your explanation matches: the receiver is created via createReceiver(address: bitcoinAddress.address), so the PDK URI's base is that same address; reusing it from the already-valid PDK output rather than re-assembling is the right call.

One housekeeping note (not blocking): this PR still targets develop and the body says you're intentionally locked on payjoin 0.23.0 — the bip21_uri decoder's -→+ rewrite stays load-bearing until the PDK upgrade lands together with the package cleanup. Just make sure that follow-up is tracked so the two don't drift apart.

Note: I couldn't execute flutter test here (sandbox permissions), so the above is by code inspection — but CI on the branch should confirm.

Nice work — LGTM. ✅

[ethicnology]

Hey @DanGould can you review that temporary fix until we prioritize #2041

[DanGould]

FYI there shouldn't be any wire format changes from 0.23 to 1.0.0-rc3 or cross version interop problems other than the URI. Technically 0.23 is out of spec but 1.0.0-rc3 iirc does accept the old URI for compatibility's sake for the time being so we don't kill clients' whose newest versions still aren't up to 0.25+. This will not be forever.

This fix makes our URI clean and in-spec for the 0.23 wire format — it does not fix cross-version interop.

There's no cross-version interop issues between 0.23,0.24,0.25, 1.0.0-rc3 afaiu beyond this issue if you consider it a wire format issue

[DanGould]

we advertise pjos=0 when the PDK omits it — which matches our receiver, since it never substitutes outputs

"Backwards-compatible receivers MUST disable output substitution by setting pjos=0 to prevent modification by a malicious directory" this is NOT about BBMobile substituting outputs, actually pjos=0 + output substitution is OK for BIP 77. it's for the BIP 78 backwards compatibility, where a malicious directory/proxy in the middle could actually STEAL FUNDS by malleating the plaintext if this isn't set properly