rust-payjoin icon indicating copy to clipboard operation
rust-payjoin copied to clipboard

synchronize BIP 77 with code

Open nothingmuch opened this issue 10 months ago • 15 comments

The BIP 77 spec is not fully up to date with these changes:

  • [x] short IDs
  • [x] uppercase bech32 fragment parameters
  • [x] RK1 fragment parameter
  • [x] ellswift encoding in DHKEM, bit uniformity of payload
  • [x] ensure DHKEM is sufficiently documented in BIP 77 in an unambiguous spec, since it's a draft RFC now
  • [x] padding rules for inner and outer messages
  • [x] incl. pub const INFO_A: &[u8; 8] = b"PjV2MsgA";, pub const INFO_B: &[u8; 8] = b"PjV2MsgB";

Additionally the document can be improved:

  • [ ] UI/UX and privacy tradeoffs of relay and directory choices
  • [x] diagrams for byte representations of the payloads
  • [x] ascii sequence diagram to plantuml?
  • [x] Describe bip21 as a potential bootstrap mechanism where TLS is unavailable

nothingmuch avatar Jan 03 '25 17:01 nothingmuch

I started pushing these changes to a secondary branch so I don't interrupt the reviewers until we're ready for a review. FYI

Note that Short IDs likely also need to be uppercase since they're parsed as having a case-sensitive bech32 HRP in our code, too

DanGould avatar Jan 06 '25 02:01 DanGould

Note that Short IDs likely also need to be uppercase since they're parsed as having a case-sensitive bech32 HRP in our code, too

Yes, with the same rationale as fragment parameter, although the # character is not in the QR alphanumeric set, since it's % escaped according to RFC 3986 reccomendation to make the hex encoding uppercase, the run of alphanumeric QR code characters potentially starts at the beginning of the entire parameter, and lowercase shortid would break that

nothingmuch avatar Jan 06 '25 15:01 nothingmuch

ensure DHKEM is sufficiently documented in BIP 77 in an unambiguous spec, since it's a draft RFC now

77 links to the draft RFC. What more would make it sufficiently unambiguous?

====Secp256k1-based DHKEM====

[[https://www.ietf.org/archive/id/draft-wahby-cfrg-hpke-kem-secp256k1-01.html| Secp256k1-based DHKEM for HPKE]] is most appropriate because of secp256k1's availability in bitcoin contexts.

DanGould avatar Jan 10 '25 04:01 DanGould

Pushed changes to the secondary branch

DanGould avatar Jan 10 '25 05:01 DanGould

#480 must be addressed for our implementation to be in sync with spec, which BIP 77 relies on, as must all existing Payjoin implementations

DanGould avatar Jan 14 '25 15:01 DanGould

Was it a mistake to use different INFO strings for HPKE?

All the algorithms also take an info parameter that can be used to influence the generation of keys (e.g., to fold in identity information) and an aad parameter that provides additional authenticated data to the AEAD algorithm in use.

The default info value is "", did we make a mistake by using something non-default? Further, was it a mistake to use TWO separate info strings pub const INFO_A: &[u8; 8] = b"PjV2MsgA";, pub const INFO_B: &[u8; 8] = b"PjV2MsgB";

According to RFC 9180, the info parameter serves as application-supplied information that can be used to influence key derivation. It's used in both the key schedule context and in the key derivation process. Do we even need it? Is it worth removing it to reduce complexity?

DanGould avatar Jan 23 '25 17:01 DanGould

I'm going to mark DHKEM on the draft RFC as unambiguous unless you raise a further concern.

DanGould avatar Jan 23 '25 18:01 DanGould

I believe these additional strings are appropriate, both in general (domain separators considered a good practice) and in particular, because although this might be superfluous in BIP 77 due to the use of ephemeral keys in both messages, in the context of a backwards compatible extension where the same pubkey might be used for two different protocols (e.g. the multiparty payjoin experiments) this can help clients reliably distinguish versions.

nothingmuch avatar Jan 23 '25 18:01 nothingmuch

Do you think it's also appropriate to have 2 separate info strings, one for a and one for b?

DanGould avatar Jan 23 '25 18:01 DanGould

Yes, again both in general and in particular, especially now that both messages appear in the same namespace (i.e. the directory is a key value store where short IDs are keys and values are the union of both message types, and potentially arbitrary future message types)

nothingmuch avatar Jan 23 '25 18:01 nothingmuch

I pushed to the bips repo. We're synced now, but missing the yet-to-be-defined-or-implemented ohttp relay payjoin directory relationship

re: https://github.com/orgs/payjoin/discussions/486

DanGould avatar Jan 24 '25 20:01 DanGould

Can this issue be closed now?

spacebear21 avatar May 05 '25 17:05 spacebear21

UI/UX and privacy tradeoffs of relay and directory choices

This is the only point that hasn't been addressed.

I think the spririt of the issue is also to take the vocab from BIP 77 and use it in here (e.g. reply key, mailbox, etc) as well as to line up the sequences where possible

DanGould avatar May 05 '25 17:05 DanGould

  • PUT for v1 backwards compatible replies is not described but should be.
  • #1025
  • HTTP status codes

nothingmuch avatar Sep 17 '25 00:09 nothingmuch

  • req-pj - in spec but not implemented
  • &pjv2= (receiver opt out of bip 78 compatibility)

nothingmuch avatar Sep 17 '25 00:09 nothingmuch