Qr optimizations (#417)

These commits:

1. encode fragment paramters as bech32 strings (with no checksum), with
  2 character param names in the human readable part, and `+` as a
  delimiter
  - expiry time parameter encoded as a little endian u32, in line with
    bitcoin header timestamp and unix time nLocktime encoding. `EX` hrp
  - ohttp has `OH` hrp, same byte representation as previously encoded in
    base64
  - receiver key has `RK` hrp, same byte representation as previously
    encoded in base64
2. use bech32 with no human readable part for the subdirectory IDs
3. uppercase the scheme and host of the URI
4. move the pj so it follows the pjos parameter

Once the `#` %-escaping bug is fixed (see #373), and as long as no path
elements between the host and the subdirectory ID, the entire `pj`
parameter of the bip21 URI can be encoded in a QR using the alphanumeric
mode. Closes #389.

The last commit seems sufficiently motivated. Since there is no purpose
to clearing these values it doesn't actually simplify the code that much.

Manually verified, with manual % escaping of `#`, using the `qrencode`
CLI encoder alphanumeric mode is indeed used for everything following
`pj=`.

Cargo-minimal.lock

@@ -217,9 +217,9 @@ dependencies = [
 
 [[package]]
 name = "bitcoin"
-version = "0.32.4"
+version = "0.32.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "788902099d47c8682efe6a7afb01c8d58b9794ba66c06affd81c3d6b560743eb"
+checksum = "ce6bc65742dea50536e35ad42492b234c27904a27f0abdcbce605015cb4ea026"
 dependencies = [
  "base58ck",
  "base64 0.21.7",

Cargo-recent.lock

@@ -217,9 +217,9 @@ dependencies = [
 
 [[package]]
 name = "bitcoin"
-version = "0.32.4"
+version = "0.32.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "788902099d47c8682efe6a7afb01c8d58b9794ba66c06affd81c3d6b560743eb"
+checksum = "ce6bc65742dea50536e35ad42492b234c27904a27f0abdcbce605015cb4ea026"
 dependencies = [
  "base58ck",
  "base64 0.21.7",

payjoin-directory/src/db.rs

@@ -7,19 +7,6 @@ use tracing::debug;
 const DEFAULT_COLUMN: &str = "";
 const PJ_V1_COLUMN: &str = "pjv1";
 
-// TODO move to payjoin crate as pub?
-// TODO impl From<HpkePublicKey> for ShortId
-// TODO impl Display for ShortId (Base64)
-// TODO impl TryFrom<&str> for ShortId (Base64)
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub(crate) struct ShortId(pub [u8; 8]);
-
-impl ShortId {
-    pub fn column_key(&self, column: &str) -> Vec<u8> {
-        self.0.iter().chain(column.as_bytes()).copied().collect()
-    }
-}
-
 #[derive(Debug, Clone)]
 pub(crate) struct DbPool {
     client: Client,
@@ -32,48 +19,48 @@ impl DbPool {
         Ok(Self { client, timeout })
     }
 
-    pub async fn push_default(&self, pubkey_id: &ShortId, data: Vec<u8>) -> RedisResult<()> {
-        self.push(pubkey_id, DEFAULT_COLUMN, data).await
+    pub async fn push_default(&self, subdirectory_id: &str, data: Vec<u8>) -> RedisResult<()> {
+        self.push(subdirectory_id, DEFAULT_COLUMN, data).await
     }
 
-    pub async fn peek_default(&self, pubkey_id: &ShortId) -> Option<RedisResult<Vec<u8>>> {
-        self.peek_with_timeout(pubkey_id, DEFAULT_COLUMN).await
+    pub async fn peek_default(&self, subdirectory_id: &str) -> Option<RedisResult<Vec<u8>>> {
+        self.peek_with_timeout(subdirectory_id, DEFAULT_COLUMN).await
     }
 
-    pub async fn push_v1(&self, pubkey_id: &ShortId, data: Vec<u8>) -> RedisResult<()> {
-        self.push(pubkey_id, PJ_V1_COLUMN, data).await
+    pub async fn push_v1(&self, subdirectory_id: &str, data: Vec<u8>) -> RedisResult<()> {
+        self.push(subdirectory_id, PJ_V1_COLUMN, data).await
     }
 
-    pub async fn peek_v1(&self, pubkey_id: &ShortId) -> Option<RedisResult<Vec<u8>>> {
-        self.peek_with_timeout(pubkey_id, PJ_V1_COLUMN).await
+    pub async fn peek_v1(&self, subdirectory_id: &str) -> Option<RedisResult<Vec<u8>>> {
+        self.peek_with_timeout(subdirectory_id, PJ_V1_COLUMN).await
     }
 
     async fn push(
         &self,
-        pubkey_id: &ShortId,
+        subdirectory_id: &str,
         channel_type: &str,
         data: Vec<u8>,
     ) -> RedisResult<()> {
         let mut conn = self.client.get_async_connection().await?;
-        let key = pubkey_id.column_key(channel_type);
+        let key = channel_name(subdirectory_id, channel_type);
         () = conn.set(&key, data.clone()).await?;
         () = conn.publish(&key, "updated").await?;
         Ok(())
     }
 
     async fn peek_with_timeout(
         &self,
-        pubkey_id: &ShortId,
+        subdirectory_id: &str,
         channel_type: &str,
     ) -> Option<RedisResult<Vec<u8>>> {
-        tokio::time::timeout(self.timeout, self.peek(pubkey_id, channel_type)).await.ok()
+        tokio::time::timeout(self.timeout, self.peek(subdirectory_id, channel_type)).await.ok()
     }
 
-    async fn peek(&self, pubkey_id: &ShortId, channel_type: &str) -> RedisResult<Vec<u8>> {
+    async fn peek(&self, subdirectory_id: &str, channel_type: &str) -> RedisResult<Vec<u8>> {
         let mut conn = self.client.get_async_connection().await?;
-        let key = pubkey_id.column_key(channel_type);
+        let key = channel_name(subdirectory_id, channel_type);
 
-        // Attempt to fetch existing content for the given pubkey_id and channel_type
+        // Attempt to fetch existing content for the given subdirectory_id and channel_type
         if let Ok(data) = conn.get::<_, Vec<u8>>(&key).await {
             if !data.is_empty() {
                 return Ok(data);
@@ -83,7 +70,7 @@ impl DbPool {
 
         // Set up a temporary listener for changes
         let mut pubsub_conn = self.client.get_async_connection().await?.into_pubsub();
-        let channel_name = pubkey_id.column_key(channel_type);
+        let channel_name = channel_name(subdirectory_id, channel_type);
         pubsub_conn.subscribe(&channel_name).await?;
 
         // Use a block to limit the scope of the mutable borrow
@@ -116,3 +103,7 @@ impl DbPool {
         Ok(data)
     }
 }
+
+fn channel_name(subdirectory_id: &str, channel_type: &str) -> Vec<u8> {
+    (subdirectory_id.to_owned() + channel_type).into_bytes()
+}

payjoin-directory/src/lib.rs

@@ -3,8 +3,6 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use anyhow::Result;
-use bitcoin::base64::prelude::BASE64_URL_SAFE_NO_PAD;
-use bitcoin::base64::Engine;
 use http_body_util::combinators::BoxBody;
 use http_body_util::{BodyExt, Empty, Full};
 use hyper::body::{Body, Bytes, Incoming};
@@ -17,8 +15,6 @@ use tokio::net::TcpListener;
 use tokio::sync::Mutex;
 use tracing::{debug, error, info, trace};
 
-use crate::db::ShortId;
-
 pub const DEFAULT_DIR_PORT: u16 = 8080;
 pub const DEFAULT_DB_HOST: &str = "localhost:6379";
 pub const DEFAULT_TIMEOUT_SECS: u64 = 30;
@@ -34,6 +30,9 @@ const V1_REJECT_RES_JSON: &str =
     r#"{{"errorCode": "original-psbt-rejected ", "message": "Body is not a string"}}"#;
 const V1_UNAVAILABLE_RES_JSON: &str = r#"{{"errorCode": "unavailable", "message": "V2 receiver offline. V1 sends require synchronous communications."}}"#;
 
+// 8 bytes as bech32 is 12.8 characters
+const ID_LENGTH: usize = 13;
+
 mod db;
 use crate::db::DbPool;
 
@@ -306,11 +305,11 @@ async fn post_fallback_v1(
     };
 
     let v2_compat_body = format!("{}\n{}", body_str, query);
-    let id = decode_short_id(id)?;
-    pool.push_default(&id, v2_compat_body.into())
+    let id = check_id_length(id)?;
+    pool.push_default(id, v2_compat_body.into())
         .await
         .map_err(|e| HandlerError::BadRequest(e.into()))?;
-    match pool.peek_v1(&id).await {
+    match pool.peek_v1(id).await {
         Some(result) => match result {
             Ok(buffered_req) => Ok(Response::new(full(buffered_req))),
             Err(e) => Err(HandlerError::BadRequest(e.into())),
@@ -327,19 +326,29 @@ async fn put_payjoin_v1(
     trace!("Put_payjoin_v1");
     let ok_response = Response::builder().status(StatusCode::OK).body(empty())?;
 
-    let id = decode_short_id(id)?;
+    let id = check_id_length(id)?;
     let req =
         body.collect().await.map_err(|e| HandlerError::InternalServerError(e.into()))?.to_bytes();
     if req.len() > V1_MAX_BUFFER_SIZE {
         return Err(HandlerError::PayloadTooLarge);
     }
 
-    match pool.push_v1(&id, req.into()).await {
+    match pool.push_v1(id, req.into()).await {
         Ok(_) => Ok(ok_response),
         Err(e) => Err(HandlerError::BadRequest(e.into())),
     }
 }
 
+fn check_id_length(id: &str) -> Result<&str, HandlerError> {
+    if id.len() != ID_LENGTH {
+        return Err(HandlerError::BadRequest(anyhow::anyhow!(
+            "subdirectory ID must be 13 bech32 characters",
+        )));
+    }
+
+    Ok(id)
+}
+
 async fn post_subdir(
     id: &str,
     body: BoxBody<Bytes, hyper::Error>,
@@ -348,14 +357,15 @@ async fn post_subdir(
     let none_response = Response::builder().status(StatusCode::OK).body(empty())?;
     trace!("post_subdir");
 
-    let id = decode_short_id(id)?;
+    let id = check_id_length(id)?;
+
     let req =
         body.collect().await.map_err(|e| HandlerError::InternalServerError(e.into()))?.to_bytes();
     if req.len() > V1_MAX_BUFFER_SIZE {
         return Err(HandlerError::PayloadTooLarge);
     }
 
-    match pool.push_default(&id, req.into()).await {
+    match pool.push_default(id, req.into()).await {
         Ok(_) => Ok(none_response),
         Err(e) => Err(HandlerError::BadRequest(e.into())),
     }
@@ -366,8 +376,8 @@ async fn get_subdir(
     pool: DbPool,
 ) -> Result<Response<BoxBody<Bytes, hyper::Error>>, HandlerError> {
     trace!("get_subdir");
-    let id = decode_short_id(id)?;
-    match pool.peek_default(&id).await {
+    let id = check_id_length(id)?;
+    match pool.peek_default(id).await {
         Some(result) => match result {
             Ok(buffered_req) => Ok(Response::new(full(buffered_req))),
             Err(e) => Err(HandlerError::BadRequest(e.into())),
@@ -396,16 +406,6 @@ async fn get_ohttp_keys(
     Ok(res)
 }
 
-fn decode_short_id(input: &str) -> Result<ShortId, HandlerError> {
-    let decoded =
-        BASE64_URL_SAFE_NO_PAD.decode(input).map_err(|e| HandlerError::BadRequest(e.into()))?;
-
-    decoded[..8]
-        .try_into()
-        .map_err(|_| HandlerError::BadRequest(anyhow::anyhow!("Invalid subdirectory ID")))
-        .map(ShortId)
-}
-
 fn empty() -> BoxBody<Bytes, hyper::Error> {
     Empty::<Bytes>::new().map_err(|never| match never {}).boxed()
 }

payjoin/Cargo.toml

@@ -19,12 +19,12 @@ exclude = ["tests"]
 send = []
 receive = ["bitcoin/rand"]
 base64 = ["bitcoin/base64"]
-v2 = ["bitcoin/rand", "bitcoin/serde", "hpke", "dep:http", "bhttp", "ohttp", "serde", "url/serde"]
+v2 = ["bitcoin/rand", "bitcoin/serde", "hpke", "dep:http", "bhttp", "ohttp", "serde", "url/serde" ]
 io = ["reqwest/rustls-tls"]
 danger-local-https = ["io", "reqwest/rustls-tls", "rustls"]
 
 [dependencies]
-bitcoin = { version = "0.32.4", features = ["base64"] }
+bitcoin = { version = "0.32.5", features = ["base64"] }
 bip21 = "0.5.0"
 hpke = { package = "bitcoin-hpke", version = "0.13.0", optional = true }
 log = { version = "0.4.14"}

payjoin/src/bech32.rs

@@ -0,0 +1,49 @@
+use std::fmt;
+
+use bitcoin::bech32::primitives::decode::{CheckedHrpstring, CheckedHrpstringError};
+use bitcoin::bech32::{self, EncodeError, Hrp, NoChecksum};
+
+pub mod nochecksum {
+    use super::*;
+
+    pub fn decode(encoded: &str) -> Result<(Hrp, Vec<u8>), CheckedHrpstringError> {
+        let hrp_string = CheckedHrpstring::new::<NoChecksum>(encoded)?;
+        Ok((hrp_string.hrp(), hrp_string.byte_iter().collect::<Vec<u8>>()))
+    }
+
+    pub fn encode(hrp: Hrp, data: &[u8]) -> Result<String, EncodeError> {
+        bech32::encode_upper::<NoChecksum>(hrp, data)
+    }
+
+    pub fn encode_to_fmt(f: &mut fmt::Formatter, hrp: Hrp, data: &[u8]) -> Result<(), EncodeError> {
+        bech32::encode_upper_to_fmt::<NoChecksum, fmt::Formatter>(f, hrp, data)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn bech32_for_qr() {
+        let bytes = vec![0u8, 1, 2, 3, 31, 32, 33, 95, 0, 96, 127, 128, 129, 254, 255, 0];
+        let hrp = Hrp::parse("STUFF").unwrap();
+        let encoded = nochecksum::encode(hrp, &bytes).unwrap();
+        let decoded = nochecksum::decode(&encoded).unwrap();
+        assert_eq!(decoded, (hrp, bytes.to_vec()));
+
+        // no checksum
+        assert_eq!(
+            encoded.len() as f32,
+            (hrp.as_str().len() + 1) as f32 + (bytes.len() as f32 * 8.0 / 5.0).ceil()
+        );
+
+        // TODO assert uppercase
+
+        // should not error
+        let corrupted = encoded + "QQPP";
+        let decoded = nochecksum::decode(&corrupted).unwrap();
+        assert_eq!(decoded.0, hrp);
+        assert_ne!(decoded, (hrp, bytes.to_vec()));
+    }
+}

payjoin/src/lib.rs

@@ -35,6 +35,8 @@ pub use crate::hpke::{HpkeKeyPair, HpkePublicKey};
 pub(crate) mod ohttp;
 #[cfg(feature = "v2")]
 pub use crate::ohttp::OhttpKeys;
+#[cfg(feature = "v2")]
+pub(crate) mod bech32;
 
 #[cfg(feature = "io")]
 pub mod io;