Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encode &ohttp= and &exp= parameters in the &pj= URL as a fragment instead of top-level bip21 params. #298

Closed
DanGould opened this issue Jun 24, 2024 · 0 comments · Fixed by #300
Labels

Comments

@DanGould
Copy link
Contributor

DanGould commented Jun 24, 2024

Right now Payjoin V2 encodes &ohttp= and &exp= as bip21 URI query keys since that seemed natural as bip78 used them for &pjos=. However, this may present a few problems down the line.

Instead of bip21 URI query keys like this

bitcoin:tb1qamy39l62neydk8hmvcfhzewpp3n906d2y335rd?pjos=0&pj=https://payjo.in/AoMhJgAzb-5vB7XspWSTbCaa2vkpucHZ6n2ZixU7vwY8&ohttp=AQAgbLIT3-hgHP5dAGB574bdCyTeJvJR4n7hLJ4B8dxgYDMABAABAAM&exp=1719258352

They should be percent-encoded as fragment paramerters in the &pj= url:

bitcoin:tb1qamy39l62neydk8hmvcfhzewpp3n906d2y335rd?pjos=0&pj=https://payjo.in/AoMhJgAzb-5vB7XspWSTbCaa2vkpucHZ6n2ZixU7vwY8%23ohttp%3DAQAgbLIT3-hgHP5dAGB574bdCyTeJvJR4n7hLJ4B8dxgYDMABAABAAM%26exp%3D1719258352

This solves problems.

  1. BIP21 parsers would otherwise need to update to support the new ohttp and exp parameters. URL parsers already parse the fragment out of a URL.
  2. QR encoders can encode everything in the &pj= parameter in alphanumeric mode which is more dense but excludes '?', '=', '&'. Each time bip21 uri params switch therefore, the QR encoded incurs overhead to switch between alphanumeric and bytes mode which can be more costly than the advantage.
  3. This stops introducing a precedence problem where multiple bip21 params could be used by conflicting protocols

To be done as part of #216

@DanGould DanGould added the api label Jun 24, 2024
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 24, 2024
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 25, 2024
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 25, 2024
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 26, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 26, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 26, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 27, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jun 27, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
@DanGould DanGould added this to the payjoin-0.19.0 milestone Jun 27, 2024
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 8, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 8, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 8, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 8, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 8, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 9, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 9, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit to DanGould/rust-payjoin that referenced this issue Jul 11, 2024
This extension trait defines functions to parse and set the ohttp parameter
in the fragment of a `pj=` URL.

Close payjoin#298
DanGould added a commit that referenced this issue Jul 11, 2024
Close #298

This introduces a `Url` extension trait called `payjoin::uri::UrlExt`
which can set and get `ohttp=` as a fragment rather than a URI
parameter. Doing so allows BIP 77 payjoin v2 parameters to be compliant
with any BIP 21 bitcoin URI parser that already supports &pj= without
introducing new params. This seems to simplify the v1/v2 feature gates
too.

This change also handles subdirectory parsing differently since the URL
no longer ends with the subdirectory, but also the fragment. This
introduces new V2 errors. On my first attempt I added subdirectory
parsing and setting to `UrlExt`, but opted to remove that since it's out
of scope for fragment handling and it would probably need to be
considered to be feature gated behind `send`/`receive` which could make
the `uri` module messier. (But also perhaps not since `UriExt` is
private and `send`/`receive` gates can be enforced in those higher
module abstractions where `UriExt` is depended upon).

The `UriExt` is unit tested.

I'm not sure how to best handle the difference between Ul paths with or
without a trailing slash. Technically the latter is a true subdirectory
but for our purposes, we're actually using HTTP endpoints that don't
differentiate. Our parser should probably prefer one to another. I'm
inclined to prefer fewer characters for simplicity.

e.g.

```
https://localhost:53429/A8xaMow612_1aiU15nyhQz0RorgIegqftrT7Yo19xKWc#ohttp=AQAgJLHcnSjkWH3Qc6ix2GOSPdyPH3OSu0-YL3NPbLd2Dx8ABAABAAM
```

vs

```
https://localhost:53429/A8xaMow612_1aiU15nyhQz0RorgIegqftrT7Yo19xKWc/#ohttp=AQAgJLHcnSjkWH3Qc6ix2GOSPdyPH3OSu0-YL3NPbLd2Dx8ABAABAAM
```

This was done as prerequisite to #299 since I believe `exp` should be a
payjoin-specific fragment parameter as well that gets added to `UriExt`
This was referenced Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant