Encode fragment parameters as bech32

These changes are to facilitate more efficient QR encoding, by exploiting the alphanumeric encoding mode. Use `+` instead of `&` as a parameter delimiter, as only the former is in the QR alphanumeric character set. Instead of encoding keys and values using `=` as a delimiter (which also not in alphanumeric set), the key is the human readable part of a bech32 string with no checksum. This effectively makes the `1` a key/value delimiter. The expiry time is serialized as a bitcoin consensus u32 value (little endian), and also encoded as a bech32 string instead of a decimal value.
payjoin · Dec 2, 2024 · 8dfda80 · 8dfda80
1 parent ebb85c3
commit 8dfda80
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 63 deletions.
diff --git a/payjoin/src/ohttp.rs b/payjoin/src/ohttp.rs
@@ -1,8 +1,7 @@
 use std::ops::{Deref, DerefMut};
 use std::{error, fmt};
 
-use bitcoin::base64::prelude::BASE64_URL_SAFE_NO_PAD;
-use bitcoin::base64::Engine;
+use bech32::{self, EncodeError};
 use bitcoin::key::constants::UNCOMPRESSED_PUBLIC_KEY_SIZE;
 
 pub const ENCAPSULATED_MESSAGE_BYTES: usize = 8192;
@@ -145,19 +144,19 @@ impl fmt::Display for OhttpKeys {
         let mut buf = vec![key_id];
         buf.extend_from_slice(&compressed_pubkey);
 
-        let encoded = BASE64_URL_SAFE_NO_PAD.encode(buf);
-        write!(f, "{}", encoded)
+        let oh_hrp: bech32::Hrp = bech32::Hrp::parse("OH").unwrap();
+
+        crate::bech32::nochecksum::encode_to_fmt(f, oh_hrp, &buf).map_err(|e| match e {
+            EncodeError::Fmt(e) => e,
+            _ => fmt::Error,
+        })
     }
 }
 
-impl std::str::FromStr for OhttpKeys {
-    type Err = ParseOhttpKeysError;
-
-    /// Parses a base64URL-encoded string into OhttpKeys.
-    /// The string format is: key_id || compressed_public_key
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let bytes = BASE64_URL_SAFE_NO_PAD.decode(s).map_err(ParseOhttpKeysError::DecodeBase64)?;
+impl TryFrom<&[u8]> for OhttpKeys {
+    type Error = ParseOhttpKeysError;
 
+    fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let key_id = *bytes.first().ok_or(ParseOhttpKeysError::InvalidFormat)?;
         let compressed_pk = bytes.get(1..34).ok_or(ParseOhttpKeysError::InvalidFormat)?;
 
@@ -174,6 +173,26 @@ impl std::str::FromStr for OhttpKeys {
     }
 }
 
+impl std::str::FromStr for OhttpKeys {
+    type Err = ParseOhttpKeysError;
+
+    /// Parses a base64URL-encoded string into OhttpKeys.
+    /// The string format is: key_id || compressed_public_key
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        // TODO extract to utility function
+        let oh_hrp: bech32::Hrp = bech32::Hrp::parse("OH").unwrap();
+
+        let (hrp, bytes) =
+            crate::bech32::nochecksum::decode(s).map_err(ParseOhttpKeysError::DecodeBech32)?;
+
+        if hrp != oh_hrp {
+            return Err(ParseOhttpKeysError::InvalidFormat);
+        }
+
+        Self::try_from(&bytes[..])
+    }
+}
+
 impl PartialEq for OhttpKeys {
     fn eq(&self, other: &Self) -> bool {
         match (self.encode(), other.encode()) {
@@ -220,7 +239,7 @@ impl serde::Serialize for OhttpKeys {
 pub enum ParseOhttpKeysError {
     InvalidFormat,
     InvalidPublicKey,
-    DecodeBase64(bitcoin::base64::DecodeError),
+    DecodeBech32(bech32::primitives::decode::CheckedHrpstringError),
     DecodeKeyConfig(ohttp::Error),
 }
 
@@ -229,7 +248,7 @@ impl std::fmt::Display for ParseOhttpKeysError {
         match self {
             ParseOhttpKeysError::InvalidFormat => write!(f, "Invalid format"),
             ParseOhttpKeysError::InvalidPublicKey => write!(f, "Invalid public key"),
-            ParseOhttpKeysError::DecodeBase64(e) => write!(f, "Failed to decode base64: {}", e),
+            ParseOhttpKeysError::DecodeBech32(e) => write!(f, "Failed to decode base64: {}", e),
             ParseOhttpKeysError::DecodeKeyConfig(e) =>
                 write!(f, "Failed to decode KeyConfig: {}", e),
         }
@@ -239,7 +258,7 @@ impl std::fmt::Display for ParseOhttpKeysError {
 impl std::error::Error for ParseOhttpKeysError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
-            ParseOhttpKeysError::DecodeBase64(e) => Some(e),
+            ParseOhttpKeysError::DecodeBech32(e) => Some(e),
             ParseOhttpKeysError::DecodeKeyConfig(e) => Some(e),
             ParseOhttpKeysError::InvalidFormat | ParseOhttpKeysError::InvalidPublicKey => None,
         }

diff --git a/payjoin/src/uri/error.rs b/payjoin/src/uri/error.rs
@@ -15,7 +15,8 @@ pub(crate) enum InternalPjParseError {
 #[derive(Debug)]
 pub(crate) enum ParseReceiverPubkeyError {
     MissingPubkey,
-    PubkeyNotBase64(bitcoin::base64::DecodeError),
+    InvalidHrp(bech32::Hrp),
+    DecodeBech32(bech32::primitives::decode::CheckedHrpstringError),
     InvalidPubkey(crate::hpke::HpkeError),
 }
 
@@ -26,7 +27,8 @@ impl std::fmt::Display for ParseReceiverPubkeyError {
 
         match &self {
             MissingPubkey => write!(f, "receiver public key is missing"),
-            PubkeyNotBase64(e) => write!(f, "receiver public is not valid base64: {}", e),
+            InvalidHrp(h) => write!(f, "incorrect hrp for receiver key: {}", h),
+            DecodeBech32(e) => write!(f, "receiver public is not valid base64: {}", e),
             InvalidPubkey(e) =>
                 write!(f, "receiver public key does not represent a valid pubkey: {}", e),
         }
@@ -40,7 +42,8 @@ impl std::error::Error for ParseReceiverPubkeyError {
 
         match &self {
             MissingPubkey => None,
-            PubkeyNotBase64(error) => Some(error),
+            InvalidHrp(_) => None,
+            DecodeBech32(error) => Some(error),
             InvalidPubkey(error) => Some(error),
         }
     }

diff --git a/payjoin/src/uri/url_ext.rs b/payjoin/src/uri/url_ext.rs
@@ -1,7 +1,7 @@
 use std::str::FromStr;
 
-use bitcoin::base64::prelude::BASE64_URL_SAFE_NO_PAD;
-use bitcoin::base64::Engine;
+use bitcoin::consensus::encode::Decodable;
+use bitcoin::consensus::Encodable;
 use url::Url;
 
 use super::error::ParseReceiverPubkeyError;
@@ -21,55 +21,80 @@ pub(crate) trait UrlExt {
 impl UrlExt for Url {
     /// Retrieve the receiver's public key from the URL fragment
     fn receiver_pubkey(&self) -> Result<HpkePublicKey, ParseReceiverPubkeyError> {
-        let value = get_param(self, "rk=", |v| Some(v.to_owned()))
+        let value = get_param(self, "RK1", |v| Some(v.to_owned()))
             .ok_or(ParseReceiverPubkeyError::MissingPubkey)?;
 
-        let decoded = BASE64_URL_SAFE_NO_PAD
-            .decode(&value)
-            .map_err(ParseReceiverPubkeyError::PubkeyNotBase64)?;
+        let (hrp, bytes) = crate::bech32::nochecksum::decode(&value)
+            .map_err(ParseReceiverPubkeyError::DecodeBech32)?;
 
-        HpkePublicKey::from_compressed_bytes(&decoded)
+        let rk_hrp: bech32::Hrp = bech32::Hrp::parse("RK").unwrap();
+        if hrp != rk_hrp {
+            return Err(ParseReceiverPubkeyError::InvalidHrp(hrp));
+        }
+
+        HpkePublicKey::from_compressed_bytes(&bytes[..])
             .map_err(ParseReceiverPubkeyError::InvalidPubkey)
     }
 
     /// Set the receiver's public key in the URL fragment
     fn set_receiver_pubkey(&mut self, pubkey: Option<HpkePublicKey>) {
+        let rk_hrp: bech32::Hrp = bech32::Hrp::parse("RK").unwrap();
+
         set_param(
             self,
-            "rk=",
-            pubkey.map(|k| BASE64_URL_SAFE_NO_PAD.encode(k.to_compressed_bytes())),
+            "RK1",
+            pubkey.map(|k| {
+                crate::bech32::nochecksum::encode(rk_hrp, &k.to_compressed_bytes())
+                    .expect("encoding compressed pubkey bytes should never fail")
+            }),
         )
     }
 
     /// Retrieve the ohttp parameter from the URL fragment
     fn ohttp(&self) -> Option<OhttpKeys> {
-        get_param(self, "ohttp=", |value| OhttpKeys::from_str(value).ok())
+        get_param(self, "OH1", |value| OhttpKeys::from_str(value).ok())
     }
 
     /// Set the ohttp parameter in the URL fragment
     fn set_ohttp(&mut self, ohttp: Option<OhttpKeys>) {
-        set_param(self, "ohttp=", ohttp.map(|o| o.to_string()))
+        set_param(self, "OH1", ohttp.map(|o| o.to_string()))
     }
 
     /// Retrieve the exp parameter from the URL fragment
     fn exp(&self) -> Option<std::time::SystemTime> {
-        get_param(self, "exp=", |value| {
-            value
-                .parse::<u64>()
+        get_param(self, "EX1", |value| {
+            let (hrp, bytes) = crate::bech32::nochecksum::decode(value).ok()?;
+
+            let ex_hrp: bech32::Hrp = bech32::Hrp::parse("EX").unwrap();
+            if hrp != ex_hrp {
+                return None;
+            }
+
+            let mut cursor = &bytes[..];
+            u32::consensus_decode(&mut cursor)
+                .map(|timestamp| {
+                    std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp as u64)
+                })
                 .ok()
-                .map(|timestamp| std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp))
         })
     }
 
     /// Set the exp parameter in the URL fragment
     fn set_exp(&mut self, exp: Option<std::time::SystemTime>) {
         let exp_str = exp.map(|e| {
-            match e.duration_since(std::time::UNIX_EPOCH) {
-                Ok(duration) => duration.as_secs().to_string(),
-                Err(_) => "0".to_string(), // Handle times before Unix epoch by setting to "0"
-            }
+            let t = match e.duration_since(std::time::UNIX_EPOCH) {
+                Ok(duration) => duration.as_secs().try_into().unwrap(), // TODO Result type instead of Option & unwrap
+                Err(_) => 0u32,
+            };
+
+            let mut buf = [0u8; 4];
+            t.consensus_encode(&mut &mut buf[..]).unwrap(); // TODO no unwrap
+
+            let ex_hrp: bech32::Hrp = bech32::Hrp::parse("EX").unwrap();
+            crate::bech32::nochecksum::encode(ex_hrp, &buf)
+                .expect("encoding u32 timestamp should never fail")
         });
-        set_param(self, "exp=", exp_str)
+        set_param(self, "EX1", exp_str)
     }
 }
 
@@ -78,32 +103,31 @@ where
     F: Fn(&str) -> Option<T>,
 {
     if let Some(fragment) = url.fragment() {
-        for param in fragment.split('&') {
-            if let Some(value) = param.strip_prefix(prefix) {
-                return parse(value);
+        for param in fragment.split('+') {
+            if param.starts_with(prefix) {
+                return parse(param);
             }
         }
     }
     None
 }
 
-fn set_param(url: &mut Url, prefix: &str, value: Option<String>) {
+fn set_param(url: &mut Url, prefix: &str, param: Option<String>) {
     let fragment = url.fragment().unwrap_or("");
     let mut fragment = fragment.to_string();
     if let Some(start) = fragment.find(prefix) {
-        let end = fragment[start..].find('&').map_or(fragment.len(), |i| start + i);
+        let end = fragment[start..].find('+').map_or(fragment.len(), |i| start + i);
         fragment.replace_range(start..end, "");
-        if fragment.ends_with('&') {
+        if fragment.ends_with('+') {
             fragment.pop();
         }
     }
 
-    if let Some(value) = value {
-        let new_param = format!("{}{}", prefix, value);
+    if let Some(param) = param {
         if !fragment.is_empty() {
-            fragment.push('&');
+            fragment.push('+');
         }
-        fragment.push_str(&new_param);
+        fragment.push_str(&param);
     }
 
     url.set_fragment(if fragment.is_empty() { None } else { Some(&fragment) });
@@ -118,11 +142,11 @@ mod tests {
     fn test_ohttp_get_set() {
         let mut url = Url::parse("https://example.com").unwrap();
 
-        let ohttp_keys =
-            OhttpKeys::from_str("AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw").unwrap();
+        let serialized = "OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC";
+        let ohttp_keys = OhttpKeys::from_str(serialized).unwrap();
         url.set_ohttp(Some(ohttp_keys.clone()));
-        assert_eq!(url.fragment(), Some("ohttp=AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw"));
 
+        assert_eq!(url.fragment(), Some(serialized));
         assert_eq!(url.ohttp(), Some(ohttp_keys));
 
         url.set_ohttp(None);
@@ -136,29 +160,19 @@ mod tests {
         let exp_time =
             std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1720547781);
         url.set_exp(Some(exp_time));
-        assert_eq!(url.fragment(), Some("exp=1720547781"));
+        assert_eq!(url.fragment(), Some("EX1C4UC6ES"));
 
         assert_eq!(url.exp(), Some(exp_time));
 
         url.set_exp(None);
         assert_eq!(url.fragment(), None);
     }
 
-    #[test]
-    fn test_invalid_v2_url_fragment_on_bip21() {
-        // fragment is not percent encoded so `&ohttp=` is parsed as a query parameter, not a fragment parameter
-        let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\
-                   &pj=https://example.com\
-                   #exp=1720547781&ohttp=AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw";
-        let uri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap();
-        assert!(uri.extras.endpoint().ohttp().is_none());
-    }
-
     #[test]
     fn test_valid_v2_url_fragment_on_bip21() {
         let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\
                    &pj=https://example.com\
-                   #ohttp%3DAQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw";
+                   #OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC";
         let uri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap();
         assert!(uri.extras.endpoint().ohttp().is_some());
     }

diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs
@@ -193,7 +193,7 @@ mod integration {
         #[tokio::test]
         async fn test_bad_ohttp_keys() {
             let bad_ohttp_keys =
-                OhttpKeys::from_str("AQO6SMScPUqSo60A7MY6Ak2hDO0CGAxz7BLYp60syRu0gw")
+                OhttpKeys::from_str("OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC")
                     .expect("Invalid OhttpKeys");
 
             let (cert, key) = local_cert_key();