Skip to content

WIP: Core mailroom manager; RequestHandler - #1960

Draft
benalleng wants to merge 4 commits into
payjoin:masterfrom
benalleng:core-mailroom-manager
Draft

WIP: Core mailroom manager; RequestHandler#1960
benalleng wants to merge 4 commits into
payjoin:masterfrom
benalleng:core-mailroom-manager

Conversation

@benalleng

@benalleng benalleng commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

This pulls the mailroom management logic into the core payjoin library to prevent footguns for implementers where possible.

To note this is a pretty big change in the sense that it adds quite a bit of io gated logic.

Coded and reviewed with help from GLM-5.3 the initial 2 commits are a direct grab from #1696, thanks for your contribution @bc1cindy!

Pull Request Checklist

Please confirm the following before requesting review:

@benalleng
benalleng force-pushed the core-mailroom-manager branch from 040bc40 to 8edde8a Compare October 9, 2026 16:03
@coveralls

coveralls commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 38776783695

Coverage increased (+0.5%) to 87.955%

Details

  • Coverage increased (+0.5%) from the base build.
  • Patch coverage: 46 uncovered changes across 2 files (687 of 733 lines covered, 93.72%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
payjoin/src/core/request_handler.rs 535 503 94.02%
payjoin-cli/src/app/v2/mod.rs 50 36 72.0%
Total (7 files) 733 687 93.72%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

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

Coverage Stats

Coverage Status
Relevant Lines: 18480
Covered Lines: 16254
Line Coverage: 87.95%
Coverage Strength: 310.72 hits per line
💛 - Coveralls

@benalleng
benalleng force-pushed the core-mailroom-manager branch from 8edde8a to bdf1a94 Compare October 9, 2026 16:26
@benalleng

benalleng commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

The TRANSIENT_RETRY_DELAY const still lives in the cli for now. @xstoicunicornx do you think we should go ahead and pull this out too to get started on the discussions we had in #440

@xstoicunicornx

xstoicunicornx commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

The TRANSIENT_RETRY_DELAY const still lives in the cli for now. @xstoicunicornx do you think we should go ahead and pull this out too to get started on the discussions we had in #440

I think this const is simply a guard to not retry too quickly and not related to the discussion in #440 . I think this should be left in the cli.

Also, not sure how I feel about naming this mailroom when we already have a payjoin-mailroom crate. It also does not represent a mailroom when using the established terms for OHTTP. It think something like OHTTP request handler or even mailroom handler would be better. IMO the name should intuitively convey that this is used to interface with the mailroom and isn't the mailroom itself.

@benalleng

benalleng commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

I think this const is simply a guard to not retry too quickly and not related to the discussion in #440 . I think this should be left in the cli.

Oh yeah I got it tied up with what that was all about. we can leave it alone. I was thinking about this old comment #1658 (comment) and just got all the timing stuff mixed up.

Noted about the naming

@benalleng
benalleng force-pushed the core-mailroom-manager branch from 787ae41 to 93e9ddf Compare October 9, 2026 20:20
@DanGould DanGould mentioned this pull request Oct 12, 2026
Open
This was referenced Oct 12, 2026
Open
Merged
@benalleng
benalleng force-pushed the core-mailroom-manager branch from 05c0871 to f712126 Compare October 12, 2026 20:03
@benalleng benalleng changed the title WIP: Core mailroom manager WIP: Core mailroom manager; RequestHandler Oct 12, 2026
@benalleng
benalleng force-pushed the core-mailroom-manager branch 11 times, most recently from 80a1ba4 to f0528e6 Compare October 14, 2026 20:00
@benalleng

This comment was marked as resolved.

Sign in to view
@DanGould

This comment was marked as resolved.

Sign in to view
@DanGould

This comment was marked as resolved.

Sign in to view
@benalleng
benalleng force-pushed the core-mailroom-manager branch 4 times, most recently from bbde7cf to efd47fb Compare October 15, 2026 15:57
@benalleng
Add payjoin::RequestHandler to allow for core library mailroom mana…
15b24e6 
…gement

This adds the mailroom management logic directly into the core library
allowing implementers to no longer rely on mimicing the reference
implementation as closely as possible and instead simply applying the
API methods directly from the payjoin library.

This feature is gated behind the `io` feature as this will inevitably
pull in the `reqwest` dep something that we want to avoid where
possible.
@benalleng
Move post_via_relay loop into core payjoin::RequestHandler
0a3a585
@benalleng
Replace the MailroomManager with the payjoin::RequestHandler in payjo…
940619e 
…in-cli

Instead of relying on direct implementation logic for core payjoin
success logic we have moved the mailroom manager logic into the payjoin
library to ensure that implementers have the least footguns possible.
@benalleng
Add RequestHandler for payjoin-ffi
44430a1
@benalleng
benalleng force-pushed the core-mailroom-manager branch from efd47fb to 44430a1 Compare October 15, 2026 17:22

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@benalleng @coveralls @xstoicunicornx @DanGould