Merge parallel feature flag local certificate logic in cli ohttp

[benalleng]

These ohttp functions had parallel methods when running tests vs running in production. This made it difficult to read as there were two functions that appeared to do completely different things when a feature flag was present.

Because localhost relays are not actually able to be proxies here we still need to opportunistically select them from the config when running our tests instead of just allowing local cert validation.

[2025-05-30T17:17:53Z DEBUG payjoin_cli::app::v2::ohttp] Failed to connect to relay: http://localhost:34391/, Internal(InternalError(Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("payjo.in")), port: None, path: "/.well-known/ohttp-gateway", query: None, fragment: None }, source: hyper_util::client::legacy::Error(Connect, "unsuccessful tunnel") })))

[coveralls]

Pull Request Test Coverage Report for Build 15476179133

Details

• 36 of 36 (100.0%) changed or added relevant lines in 2 files are covered.
• 12 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.1%) to 85.159%

---

Files with Coverage Reduction	New Missed Lines	%
payjoin-cli/src/app/v2/ohttp.rs	12	81.25%

Totals
Change from base Build 15449709719:	0.1%
Covered Lines:	6811
Relevant Lines:	7998

---

💛 - Coveralls

[benalleng]

This earlier comment was placed in an incorrect place based on my misunderstanding of where the logic was failing

[benalleng]

Ok resolved, as it turns out the problem was that the subsequent steps in the test when initializing were falling back to the default payjoin directory payjo.in and not the localhost directory, by passing --pj-directory as a global arg we are enforcing that we are using the same directory through the e2e test.

This was not a problem before as the ohttp-keys flag was overriding any checks and skipping this logic in the test.

[DanGould]

The simplification is much greater than the diff count suggests. Great change.

3 things

1. needs rebase
2. validate_relay and fetch_keys are almost exact now. Can they be deduplicated while you're rebase
3. It seems completely incorrect to require --pj-directory for a sender. The BIP21 defines the directory. If we're always defaulting to payjo.in that's a bug.

[benalleng]

It seems completely incorrect to require --pj-directory for a sender. The BIP21 defines the directory. If we're always defaulting to payjo.in that's a bug.

I think you're right that its actually a bug. The sender would just seemingly ignore the directory listed in the bip21 and use the payjo.in directory as a fallback. I'd like to make the directory based on the provided bip21 if possible at least in the send command, that still leaves an open question for me about doing that for the resume command, but I assume the bip21 is still accessible somehow.

[benalleng]

I opted to keep the --pj-directory as a flag for the resume command. Is that ok or is there another place where I could access the directory from within the saved state as initialization is happening?

[DanGould]

It doesn't make much sense for resume to take a --pj-directory flag at all because senders will use the directory from the bitcoin URI they're sending to and receivers ought to have saved a directory as part of their initialization since a session is tied to a specific directory. extract_req should encapsulate the directory url inside the OHTTP request.

[benalleng]

Ok so I created a way for the directory to be correctly chosen across each payjoin-cli command.

1. receive still uses the --pj-directory flag as the primary way to select what directory to use and will ultimately be what creates the bip21 to be used in the sender
2. send checks the bip21 arg already present and parses out the pj-directory from there
3. resume initializes without any flags or args so the initialization occurs and defaults are set before the resume session has access to any information about what directory to use so I modified the pj_directory to be mutable at the step where the session is established.

[DanGould]

resume initializes without any flags or args so the initialization occurs and defaults are set before the resume session has access to any information about what directory to use so I modified the pj_directory to be mutable at the step where the session is established.

An existing session should have its own pj_directory that overrides any configured payjoin directory. I can't think of any instance where application code should mutate the config once it is initialized.

[benalleng]

The issue I ran into is that the session isn't accessed until after the config defaults for that session are already written

[benalleng]

If I make a clone db at the config builder step how should I make sure that I am choosing the right session for the config with the get_recv_sessions() returning a Vec<Receiver>

[DanGould]

you shouldn't need to edit the config or apply defaults even on resume. Each session should already have an associated pj_directory that can be used, no? The config should be irrelevant.

[benalleng]

Ok, I can just go direct for the session directory in the fetch function I wasn't sure if we wanted some consistency with he config but it makes sense to skip over it for this 👍

I definitely wasn't aware you could do this for anything beyond resume

[spacebear21]

These clones are unnecessary and we can use references instead when calling fetch_ohttp_keys:

Suggested change

	payjoin::io::fetch_ohttp_keys_with_cert(
	selected_relay.clone(),
	payjoin_directory.clone(),
	cert_der,
	)
	payjoin::io::fetch_ohttp_keys_with_cert(
	&selected_relay,
	&payjoin_directory,
	cert_der,
	)

[spacebear21]

Minor optimization: we shouldn't have to fetch_ohttp_keys if the keys are going to be overridden by the config anyways.

[spacebear21]

Also, isn't this error-prone if the config ohttp_keys doesn't correspond to the randomly selected relay in fetch_ohttp_keys?

[benalleng]

Ok, I guess that makes sense, I still need to return the relay but in those instances I think it is safe to just skip the fetching and use the first default relay

[spacebear21]

tACK 6e03ab1

Ran the cli receiver and sender and interrupted both sides to continue with the resume command. My v2 config only includes a single relay fwiw:

[v2]
ohttp_relays=["https://pj.bobspacebkk.com"]