Skip to content

Gate V1 protocol behind runtime feature flag#1336

Merged
spacebear21 merged 2 commits intopayjoin:masterfrom
DanGould:v1-feature-flag
Feb 19, 2026
Merged

Gate V1 protocol behind runtime feature flag#1336
spacebear21 merged 2 commits intopayjoin:masterfrom
DanGould:v1-feature-flag

Conversation

@DanGould
Copy link
Copy Markdown
Contributor

@DanGould DanGould commented Feb 18, 2026

Summary

  • Adds a v1 Cargo feature to payjoin-directory that gates all V1 protocol support (fallback endpoint, PSBT screening, database operations, error variants)
  • Without v1, the directory only supports V2 and returns a version-unsupported BIP78 error for V1 requests
  • Forwards the feature in payjoin-mailroom as v1 = ["payjoin-directory/v1"]

Stack

This is part of a stacked PR series (merge in order):

  1. Clear V1 payload from memory after first read #1335 - Clear V1 payload from memory after first read
  2. This PR - Gate V1 behind compile-time feature flag
  3. Next: GeoIP region filtering + address blocklist

This PR was developed with AI assistance (Claude)

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Feb 18, 2026

Pull Request Test Coverage Report for Build 22189613184

Details

  • 86 of 89 (96.63%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 83.319%

Changes Missing Coverage Covered Lines Changed/Added Lines %
payjoin-directory/src/config.rs 0 1 0.0%
payjoin-directory/src/main.rs 0 1 0.0%
payjoin-mailroom/src/config.rs 7 8 87.5%
Totals Coverage Status
Change from base Build 22188799265: 0.1%
Covered Lines: 10344
Relevant Lines: 12415

💛 - Coveralls

@DanGould DanGould force-pushed the v1-feature-flag branch 3 times, most recently from d71f39a to 1aebe2c Compare February 18, 2026 15:02
Comment thread payjoin-directory/src/lib.rs Outdated
Comment thread flake.nix Outdated
Comment thread payjoin-directory/src/lib.rs Outdated
Comment thread payjoin-directory/src/lib.rs Outdated
Comment on lines +458 to +460
ScreenResult::ParseError(e) => {
warn!("Could not screen V1 payload: {e}");
// fail-open: unparsable PSBTs can't complete transactions
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.

Shouldn't this return an error response?

Comment thread payjoin-directory/src/db/files.rs Outdated
Comment thread payjoin-directory/src/lib.rs Outdated
@DanGould DanGould force-pushed the v1-feature-flag branch 3 times, most recently from 90799c0 to 51198a0 Compare February 19, 2026 08:38
@DanGould DanGould changed the title Gate V1 protocol behind compile-time feature flag Gate V1 protocol behind runtime feature flag Feb 19, 2026
@DanGould DanGould requested a review from spacebear21 February 19, 2026 13:25
Comment thread payjoin-directory/src/lib.rs Outdated
DanGould and others added 2 commits February 19, 2026 10:55
Add an `enable_v1` runtime option to both payjoin-directory and
payjoin-mailroom that controls whether V1 protocol requests are
accepted. Defaults to false (V2-only) and must be explicitly enabled
in the operator's configuration.

When disabled, V1 POST requests return a `version-unsupported` BIP78
error and V1 PUT responses via OHTTP are silently dropped.
Copy link
Copy Markdown
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

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

self-ACK on force push: I updated the error response per my earlier review comment and added a commit that tests expected error responses.


use super::*;

async fn test_service(enable_v1: bool) -> Service<FilesDb> {
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.

Does this have any value to be pulled out into payjoin-test-utils to be used more generally?

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.

in theory we could move these tests to the payjoin-mailroom which already has a similar utility, but for now I think it's inoffensive enough until payjoin-directory gets folded in

@benalleng
Copy link
Copy Markdown
Collaborator

benalleng commented Feb 19, 2026

Test plan

* [x]  `cargo test -p payjoin-directory --lib` passes (6 tests, no v1)

* [x]  `cargo test -p payjoin-directory --features v1 --lib` passes (14 tests)

This is beginning to balloon in the misrepresentation of what is happening here.

Copy link
Copy Markdown
Collaborator

@benalleng benalleng left a comment

Choose a reason for hiding this comment

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

TACK 96b9edc

The code in this PR looks good though claude is definitely misrepresenting what is actually happening in the PR title and description. I assume this happened after some iteration maybe? For our sanity I think that we should clarify what is actually happening as it seemed to properly correct the commit message.

@spacebear21
Copy link
Copy Markdown
Collaborator

I deleted the "test plan" part of the PR description. This is an area where I personally still don't feel comfortable letting the robot write commit messages/PR descriptions. I let it plan and write the code, then review and write commits/descriptions myself.

@spacebear21 spacebear21 merged commit 1908139 into payjoin:master Feb 19, 2026
16 checks passed
if self.enable_v1 {
self.post_fallback_v1(id, query, body).await
} else {
let _ = (id, query, body);
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.

Suggested change
let _ = (id, query, body);

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.

5 participants