Skip to content

refactor(cli): consolidate relay logic in RelayManager#1635

Merged
benalleng merged 4 commits into
payjoin:masterfrom
va-an:refactor/relay-manager
Jun 10, 2026
Merged

refactor(cli): consolidate relay logic in RelayManager#1635
benalleng merged 4 commits into
payjoin:masterfrom
va-an:refactor/relay-manager

Conversation

@va-an

@va-an va-an commented Jun 10, 2026

Copy link
Copy Markdown

Relates to #1582

Done per the plan from #1582 (comment):

  • Drop the Arc<Mutex<>> wrapper around RelayManager
  • Put Arc<Mutex<>> on the failed_relays field instead
  • Move the functions into &self methods on RelayManager

The original plan mentioned three functions, but App::unwrap_relay_or_else_fetch was turned into App::post_via_relay in #1628, so only two methods were moved here. It could be worth moving post_via_relay into RelayManager as well, but I'd suggest leaving that out of the scope of this PR.

Commands to verify the function moves (bash/zsh; strips the method's extra impl indent so only the signature/self-receiver changes show up). b28a1d8e is the master tip this branch is based on, pinned so the line ranges stay valid:

fetch_ohttp_keys:

diff -u \
  <(git show b28a1d8e:payjoin-cli/src/app/v2/ohttp.rs | sed -n '59,106p') \
  <(git show HEAD:payjoin-cli/src/app/v2/ohttp.rs      | sed -n '56,97p' | sed 's/^    //')

unwrap_ohttp_keys_or_else_fetch:

diff -u \
  <(git show b28a1d8e:payjoin-cli/src/app/v2/ohttp.rs | sed -n '49,57p') \
  <(git show HEAD:payjoin-cli/src/app/v2/ohttp.rs      | sed -n '49,54p' | sed 's/^    //')

Disclosure: code written by hand, Claude was used for review and discussion.

Pull Request Checklist

Please confirm the following before requesting review:

va-an added 3 commits June 10, 2026 13:05
Preparatory commit before moving the relay-selection functions into
RelayManager methods.

The `Arc<Mutex<>>` wrapper around `RelayManager` only exists to synchronize
`failed_relays` and gets in the way of that refactor, so move the
`Arc<Mutex<>>` inside `RelayManager` onto the `failed_relays` field. As a
bonus, `RelayManager` becomes simpler to use - callers no longer lock
on every method call.
Move the free `fetch_ohttp_keys` from ohttp.rs into an `&self` method
on `RelayManager`, so relay selection lives in the type that owns the
relay state. The method reads `config` from `self` instead of taking
it as an argument.

The relay functions are relocated bottom-up through the call graph
(`fetch_ohttp_keys` <- `unwrap_ohttp_keys_or_else_fetch` <-
`App::unwrap_relay_or_else_fetch`); this is the first. Its only caller
now invokes the method on its `relay_manager` argument, which becomes
`self` once that caller is moved too.
Move the free `unwrap_ohttp_keys_or_else_fetch` from ohttp.rs into an
`&self` method on `RelayManager`, reading `config` from `self`. Second
step of relocating relay selection into the type that owns the relay
state.

Its caller in `receive_payjoin` now invokes the method directly on
`self.relay_manager`, dropping the per-call `relay_manager.clone()`
and the `config` argument.
@coveralls

coveralls commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 27272110652

Coverage increased (+0.05%) to 85.244%

Details

  • Coverage increased (+0.05%) from the base build.
  • Patch coverage: 30 uncovered changes across 2 files (10 of 40 lines covered, 25.0%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
payjoin-cli/src/app/v2/ohttp.rs 38 9 23.68%
payjoin-cli/src/app/v2/mod.rs 2 1 50.0%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
payjoin-cli/src/app/v2/ohttp.rs 1 38.0%

Coverage Stats

Coverage Status
Relevant Lines: 14699
Covered Lines: 12530
Line Coverage: 85.24%
Coverage Strength: 371.39 hits per line

💛 - Coveralls

Comment thread payjoin-cli/src/app/v2/ohttp.rs Outdated
@va-an
va-an requested a review from benalleng June 10, 2026 11:10

@benalleng benalleng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

TACK f130191

ran the cli with 5 active sessions and was able to see that once one relay hit the example.com relay it dropped it from the list and was not tried again during the lifetime of the resume process for any of the sessions

@benalleng
benalleng merged commit 53861db into payjoin:master Jun 10, 2026
14 checks passed
@va-an
va-an deleted the refactor/relay-manager branch June 10, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants