Support HTTPS OHTTP relays in Dart

[chavic]

I directed codex to reimplement HTTPS OHTTP relay support in Dart using
HttpClient.connectionFactory, opening TLS to HTTPS relays while leaving
CONNECT, response parsing, TLS to the directory, and chunked framing to
Dart's HttpClient.

This avoids owning a custom CONNECT implementation and HTTP response parser
in the Dart bindings. The change also adds timeout handling, URL validation,
relay certificate pinning for local test setups, a response body cap, and
focused tests for HTTP relays, HTTPS relays, chunk extensions, and invalid
relay schemes.

Disclosure: co-authored by Codex

[coveralls]

Coverage Report for CI Build 25516441401

Coverage remained the same at 85.169%

Details

• Coverage remained the same as the base build.
• Patch coverage: No coverable lines changed in this PR.
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	13519
Covered Lines:	11514
Line Coverage:	85.17%
Coverage Strength:	400.02 hits per line

---

💛 - Coveralls

[spacebear21]

I tested this against the bull bitcoin mobile integration test and it seems to work! At first glance this looks wayyy more sane than #1500

[chavic]

I tested this against the bull bitcoin mobile integration test and it seems to work! At first glance this looks wayyy more sane than #1500

Yeah, digging a little deep into why the SDK fails was the trick. The proposed solution works well on our version of the SDK

[spacebear21]

tACK 4b6e603

Tested locally against both SatoshiPortal/bullbitcoin-mobile#2041 and against the regression tests I had previously vibecoded for #1500. I feel comfortable not porting all of those tests here because it relies on the dart SDK under the hood which I trust more than the slop in #1500.

@chavic thanks for coming in clutch and approaching this from first principles 🫡

One thing: I wonder if we really need the additional validation in _validateUrl ? It seems more restrictive than the other implementations e.g. fetch_ohttp_keys's IntoUrl in the rust library. Maybe fetchOhttpKeys should just take Uri parameters directly instead of Strings.

[chavic]

@spacebear21 Changing fetchOhttpKeys from Strings to Uri would be cleaner, i can revise that in this PR

[DanGould]

I am so grateful @chavic surfaced this design after looking at the actual SDK! I felt some hesitation from both of us to merge.

Regarding Strings to Uri, I actually prefer to keep it taking String since we've got the ~IntoUrl/manual Uri parsing hidden by taking String params for the rest of the dart bindings. I don't really think this one function should take Uri, but it does make sense to match rust-payjoin's IntoUrl semantics

I wonder if we really need the additional validation in _validateUrl ? It seems more restrictive than the other implementations e.g. fetch_ohttp_keys's IntoUrl in the rust library

It's actually slightly less restrictive afaict compared to our payjoin::Url struct contract if the validation is done on _hostPattern = RegExp(r'^[A-Za-z0-9._\-:]+$'); as of 4b6e603 and our rust implementation is:

rust-payjoin/payjoin/src/core/url.rs

Lines 433 to 435 in 676ede9

	if !host_str.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '.') {
	return Err(ParseError::InvalidHost);
	}

the Dart allowlist's _and : ARE a real divergence from rust-payjoin's parser. If we want strict parity at the host-charset level (I think we do), the cleanest move is either:

• Drop _ from Dart to match rust-payjoin's RFC-1123-style strictness (imo this strict is easiest & my preference, tho we just gotta update bip77 spec to match eventually) OR
• Add _ to rust-payjoin's parser to match RFC 3986 reg-name (which is what the URI spec actually says and I think technically what BIP21 expects URLs to be)

[chavic]

I am so grateful @chavic surfaced this design after looking at the actual SDK! I felt some hesitation from both of us to merge.

Regarding Strings to Uri, I actually prefer to keep it taking String since we've got the ~IntoUrl/manual Uri parsing hidden by taking String params for the rest of the dart bindings. I don't really think this one function should take Uri, but it does make sense to match rust-payjoin's IntoUrl semantics

I wonder if we really need the additional validation in _validateUrl ? It seems more restrictive than the other implementations e.g. fetch_ohttp_keys's IntoUrl in the rust library

It's actually slightly less restrictive afaict compared to our payjoin::Url struct contract if the validation is done on _hostPattern = RegExp(r'^[A-Za-z0-9._\-:]+$'); as of 4b6e603 and our rust implementation is:

rust-payjoin/payjoin/src/core/url.rs

Lines 433 to 435 in 676ede9

	if !host_str.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '.') {
	return Err(ParseError::InvalidHost);
	}

the Dart allowlist's _and : ARE a real divergence from rust-payjoin's parser. If we want strict parity at the host-charset level (I think we do), the cleanest move is either:

• Drop _ from Dart to match rust-payjoin's RFC-1123-style strictness (imo this strict is easiest & my preference, tho we just gotta update bip77 spec to match eventually) OR
• Add _ to rust-payjoin's parser to match RFC 3986 reg-name (which is what the URI spec actually says and I think technically what BIP21 expects URLs to be)

Dropping _ will make things consistent fast. The easier option of dropping _ leads to a minor update to bip77, but BIP21 expects _ is how I'm reading this... Is it correct to assume bip77 should be mostly compatible with BIP21, may need a few more minutes to gather additional context and think about implications, if it's still in the air 🤔

[DanGould]

dropping _ (and :) is still consistent with BIP21, complete compatibility because 77 depends on 21 and 77 just narrows what it makes acceptable

[benalleng]

TACK 4b6e603

tested against my bb mobile branch SatoshiPortal/bullbitcoin-mobile#1978 using the payjoin cli from this PR with both sender and receiver flows.

[chavic]

@DanGould I checked the failing CI against #1536, and this looks like the same macOS Python/Nix environment issue rather than something introduced by the Dart changes here, I think all of those are covered.

Build and test python (macos-latest) and the failure is during the corepc-node build step:

  ld: library not found for -lbz2
  clang: error: linker command failed with exit code 1
  error: could not compile `corepc-node` (build script)

PR #1536 appears to address this failure

So I think this CI failure is unrelated to the Dart OHTTP relay changes in this PR. Once #1536 lands, rebasing this branch should pick up the fix

[spacebear21]

tACK b0da3a4 with my BBM upgrade branch again.

I don't want to let this PR get blocked from getting merged because of an unrelated CI issue introduced separately. Great work on this @chavic 🫡