Refactor ohttp keys (#317)

Refactor `OhttpKeys` so that #300 can just focus on semantics.

- First Commit removes a clippy warning
- 2nd reduces unnecessary base64 verbosity. base64 dependency was being
used where psbt.to_string was fine, and the custom configs were being
built where confs could do
- 3rd removes a vestigial PartialEq implementation
- 4th disambiguates `Display` and `Serialize` roles. [OHTTP KeyConfigs
encoding](https://www.ietf.org/rfc/rfc9458.html#name-key-configuration-encoding)
for serialization is well specified by the rfc. We're using string
representations in URLs and configs just for Payjoin V2 which is a
separate thing

payjoin-cli/src/app/config.rs

@@ -99,7 +99,14 @@ impl AppConfig {
                         )?
                         .set_override_option(
                             "ohttp_keys",
-                            matches.get_one::<String>("ohttp_keys").map(|s| s.as_str()),
+                            matches.get_one::<String>("ohttp_keys").and_then(|s| {
+                                payjoin::base64::decode_config(s, payjoin::base64::URL_SAFE_NO_PAD)
+                                    .map_err(|e| {
+                                        log::error!("Failed to decode ohttp_keys: {}", e);
+                                        ConfigError::Message(format!("Invalid ohttp_keys: {}", e))
+                                    })
+                                    .ok()
+                            }),
                         )?
                 };
 

payjoin-cli/src/app/mod.rs

@@ -5,9 +5,8 @@ use anyhow::{anyhow, Context, Result};
 use bitcoincore_rpc::bitcoin::Amount;
 use bitcoincore_rpc::RpcApi;
 use payjoin::bitcoin::psbt::Psbt;
-use payjoin::bitcoin::{self, base64};
 use payjoin::send::RequestContext;
-use payjoin::PjUri;
+use payjoin::{bitcoin, PjUri};
 
 pub mod config;
 use crate::app::config::AppConfig;
@@ -77,7 +76,7 @@ pub trait App {
         log::debug!("Proposed psbt: {:#?}", psbt);
         let psbt = self
             .bitcoind()?
-            .wallet_process_psbt(&serialize_psbt(&psbt), None, None, None)
+            .wallet_process_psbt(&psbt.to_string(), None, None, None)
             .with_context(|| "Failed to process PSBT")?
             .psbt;
         let tx = self
@@ -134,8 +133,6 @@ impl payjoin::receive::Headers for Headers<'_> {
     }
 }
 
-fn serialize_psbt(psbt: &Psbt) -> String { base64::encode(psbt.serialize()) }
-
 #[cfg(feature = "danger-local-https")]
 fn http_agent() -> Result<reqwest::Client> { Ok(http_agent_builder()?.build()?) }
 

payjoin-cli/src/app/v1.rs

@@ -11,7 +11,7 @@ use hyper::{Body, Method, Request, Response, Server, StatusCode};
 use payjoin::bitcoin::psbt::Psbt;
 use payjoin::bitcoin::{self};
 use payjoin::receive::{PayjoinProposal, UncheckedProposal};
-use payjoin::{base64, Error, PjUriBuilder, Uri, UriExt};
+use payjoin::{Error, PjUriBuilder, Uri, UriExt};
 
 use super::config::AppConfig;
 use super::App as AppTrait;
@@ -242,7 +242,7 @@ impl App {
 
         let payjoin_proposal = self.process_v1_proposal(proposal)?;
         let psbt = payjoin_proposal.psbt();
-        let body = base64::encode(psbt.serialize());
+        let body = psbt.to_string();
         println!("Responded with Payjoin proposal {}", psbt.clone().extract_tx().txid());
         Ok(Response::new(Body::from(body)))
     }
@@ -325,7 +325,7 @@ impl App {
         let payjoin_proposal = provisional_payjoin.finalize_proposal(
             |psbt: &Psbt| {
                 bitcoind
-                    .wallet_process_psbt(&base64::encode(psbt.serialize()), None, None, Some(false))
+                    .wallet_process_psbt(&psbt.to_string(), None, None, Some(false))
                     .map(|res| Psbt::from_str(&res.psbt).map_err(|e| Error::Server(e.into())))
                     .map_err(|e| Error::Server(e.into()))?
             },

payjoin-cli/src/app/v2.rs

@@ -8,7 +8,7 @@ use payjoin::bitcoin::psbt::Psbt;
 use payjoin::bitcoin::Amount;
 use payjoin::receive::v2::ActiveSession;
 use payjoin::send::RequestContext;
-use payjoin::{base64, bitcoin, Error, Uri};
+use payjoin::{bitcoin, Error, Uri};
 use tokio::signal;
 use tokio::sync::watch;
 
@@ -341,7 +341,7 @@ impl App {
         let payjoin_proposal = provisional_payjoin.finalize_proposal(
             |psbt: &Psbt| {
                 bitcoind
-                    .wallet_process_psbt(&base64::encode(psbt.serialize()), None, None, Some(false))
+                    .wallet_process_psbt(&psbt.to_string(), None, None, Some(false))
                     .map(|res| Psbt::from_str(&res.psbt).map_err(|e| Error::Server(e.into())))
                     .map_err(|e| Error::Server(e.into()))?
             },

payjoin-cli/tests/e2e.rs

@@ -148,7 +148,6 @@ mod e2e {
 
         use http::StatusCode;
         use once_cell::sync::{Lazy, OnceCell};
-        use payjoin::base64;
         use reqwest::{Client, ClientBuilder};
         use testcontainers::clients::Cli;
         use testcontainers_modules::redis::Redis;
@@ -220,8 +219,6 @@ mod e2e {
             let ohttp_keys =
                 payjoin::io::fetch_ohttp_keys(ohttp_relay.clone(), directory.clone(), cert.clone())
                     .await?;
-            let bytes = ohttp_keys.0.encode()?;
-            let ohttp_keys = base64::encode_config(bytes, base64::URL_SAFE);
 
             let receiver_rpchost = format!("http://{}/wallet/receiver", bitcoind.params.rpc_socket);
             let sender_rpchost = format!("http://{}/wallet/sender", bitcoind.params.rpc_socket);
@@ -250,7 +247,7 @@ mod e2e {
                 .arg("--pj-directory")
                 .arg(&directory)
                 .arg("--ohttp-keys")
-                .arg(&ohttp_keys)
+                .arg(&ohttp_keys.to_string())
                 .stdout(Stdio::piped())
                 .stderr(Stdio::inherit())
                 .spawn()

payjoin-directory/src/lib.rs

@@ -102,10 +102,7 @@ fn init_ohttp() -> Result<ohttp::Server> {
     // create or read from file
     let server_config = ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC))?;
     let encoded_config = server_config.encode()?;
-    let b64_config = base64::encode_config(
-        encoded_config,
-        base64::Config::new(base64::CharacterSet::UrlSafe, false),
-    );
+    let b64_config = base64::encode_config(encoded_config, base64::URL_SAFE_NO_PAD);
     info!("ohttp-keys server config base64 UrlSafe: {:?}", b64_config);
     Ok(ohttp::Server::new(server_config)?)
 }
@@ -242,12 +239,11 @@ impl From<hyper::http::Error> for HandlerError {
 }
 
 async fn post_session(body: Body) -> Result<Response<Body>, HandlerError> {
-    let b64_config = base64::Config::new(base64::CharacterSet::UrlSafe, false);
     let bytes =
         hyper::body::to_bytes(body).await.map_err(|e| HandlerError::BadRequest(e.into()))?;
     let base64_id =
         String::from_utf8(bytes.to_vec()).map_err(|e| HandlerError::BadRequest(e.into()))?;
-    let pubkey_bytes: Vec<u8> = base64::decode_config(base64_id, b64_config)
+    let pubkey_bytes: Vec<u8> = base64::decode_config(base64_id, base64::URL_SAFE_NO_PAD)
         .map_err(|e| HandlerError::BadRequest(e.into()))?;
     let pubkey = bitcoin::secp256k1::PublicKey::from_slice(&pubkey_bytes)
         .map_err(|e| HandlerError::BadRequest(e.into()))?;

payjoin/src/receive/v2.rs

@@ -98,19 +98,15 @@ impl SessionInitializer {
         let _ = res.read_to_end(&mut buf);
         let response = crate::v2::ohttp_decapsulate(ctx, &buf)?;
         if !response.status().is_success() {
-            return Err(Error::Server(
-                format!("Enrollment failed, expected success status",).into(),
-            ));
+            return Err(Error::Server("Enrollment failed, expected success status".into()));
         }
 
         Ok(ActiveSession { context: self.context.clone() })
     }
 }
 
 fn subdir_path_from_pubkey(pubkey: &bitcoin::secp256k1::PublicKey) -> String {
-    let pubkey = pubkey.serialize();
-    let b64_config = base64::Config::new(base64::CharacterSet::UrlSafe, false);
-    base64::encode_config(pubkey, b64_config)
+    base64::encode_config(pubkey.serialize(), base64::URL_SAFE_NO_PAD)
 }
 
 /// An active payjoin V2 session, allowing for polled requests to the
@@ -445,7 +441,7 @@ impl PayjoinProposal {
 
     pub fn psbt(&self) -> &Psbt { self.inner.psbt() }
 
-    pub fn extract_v1_req(&self) -> String { base64::encode(self.inner.payjoin_psbt.serialize()) }
+    pub fn extract_v1_req(&self) -> String { self.inner.payjoin_psbt.to_string() }
 
     #[cfg(feature = "v2")]
     pub fn extract_v2_req(&mut self) -> Result<(Request, ohttp::ClientResponse), Error> {

payjoin/src/send/mod.rs

@@ -248,7 +248,7 @@ impl<'a> RequestBuilder<'a> {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, PartialEq)]
 pub struct RequestContext {
     psbt: Psbt,
     endpoint: Url,
@@ -264,23 +264,6 @@ pub struct RequestContext {
     e: bitcoin::secp256k1::SecretKey,
 }
 
-#[cfg(feature = "v2")]
-impl PartialEq for RequestContext {
-    fn eq(&self, other: &Self) -> bool {
-        self.psbt == other.psbt
-            && self.endpoint == other.endpoint
-            // KeyConfig is not yet PartialEq
-            && self.ohttp_keys.as_ref().map(|cfg| cfg.encode().unwrap_or_default()) == other.ohttp_keys.as_ref().map(|cfg| cfg.encode().unwrap_or_default())
-            && self.disable_output_substitution == other.disable_output_substitution
-            && self.fee_contribution == other.fee_contribution
-            && self.min_fee_rate == other.min_fee_rate
-            && self.input_type == other.input_type
-            && self.sequence == other.sequence
-            && self.payee == other.payee
-            && self.e == other.e
-    }
-}
-
 #[cfg(feature = "v2")]
 impl Eq for RequestContext {}
 
@@ -375,10 +358,9 @@ impl RequestContext {
             subdirectory = path_and_query;
         }
 
-        let b64_config =
-            bitcoin::base64::Config::new(bitcoin::base64::CharacterSet::UrlSafe, false);
-        let pubkey_bytes = bitcoin::base64::decode_config(subdirectory, b64_config)
-            .map_err(InternalCreateRequestError::SubdirectoryNotBase64)?;
+        let pubkey_bytes =
+            bitcoin::base64::decode_config(subdirectory, bitcoin::base64::URL_SAFE_NO_PAD)
+                .map_err(InternalCreateRequestError::SubdirectoryNotBase64)?;
         Ok(bitcoin::secp256k1::PublicKey::from_slice(&pubkey_bytes)
             .map_err(InternalCreateRequestError::SubdirectoryInvalidPubkey)?)
     }
@@ -401,13 +383,7 @@ impl Serialize for RequestContext {
         let mut state = serializer.serialize_struct("RequestContext", 8)?;
         state.serialize_field("psbt", &self.psbt.to_string())?;
         state.serialize_field("endpoint", &self.endpoint.as_str())?;
-        let ohttp_string = self.ohttp_keys.as_ref().map_or(Ok("".to_string()), |config| {
-            config
-                .encode()
-                .map_err(|e| serde::ser::Error::custom(format!("ohttp-keys encoding error: {}", e)))
-                .map(bitcoin::base64::encode)
-        })?;
-        state.serialize_field("ohttp_keys", &ohttp_string)?;
+        state.serialize_field("ohttp_keys", &self.ohttp_keys)?;
         state.serialize_field("disable_output_substitution", &self.disable_output_substitution)?;
         state.serialize_field(
             "fee_contribution",
@@ -476,21 +452,7 @@ impl<'de> Deserialize<'de> for RequestContext {
                                 url::Url::from_str(&map.next_value::<String>()?)
                                     .map_err(de::Error::custom)?,
                             ),
-                        "ohttp_keys" => {
-                            let ohttp_base64: String = map.next_value()?;
-                            ohttp_keys = if ohttp_base64.is_empty() {
-                                None
-                            } else {
-                                Some(
-                                    crate::v2::OhttpKeys::decode(
-                                        bitcoin::base64::decode(&ohttp_base64)
-                                            .map_err(de::Error::custom)?
-                                            .as_slice(),
-                                    )
-                                    .map_err(de::Error::custom)?,
-                                )
-                            };
-                        }
+                        "ohttp_keys" => ohttp_keys = Some(map.next_value()?),
                         "disable_output_substitution" =>
                             disable_output_substitution = Some(map.next_value()?),
                         "fee_contribution" => {
@@ -516,7 +478,7 @@ impl<'de> Deserialize<'de> for RequestContext {
                 Ok(RequestContext {
                     psbt: psbt.ok_or_else(|| de::Error::missing_field("psbt"))?,
                     endpoint: endpoint.ok_or_else(|| de::Error::missing_field("endpoint"))?,
-                    ohttp_keys,
+                    ohttp_keys: ohttp_keys.ok_or_else(|| de::Error::missing_field("ohttp_keys"))?,
                     disable_output_substitution: disable_output_substitution
                         .ok_or_else(|| de::Error::missing_field("disable_output_substitution"))?,
                     fee_contribution,

payjoin/src/uri.rs

@@ -1,9 +1,13 @@
 use std::borrow::Cow;
+#[cfg(feature = "v2")]
+use std::str::FromStr;
 
 use bitcoin::address::NetworkChecked;
 use bitcoin::{Address, Amount};
 use url::Url;
 
+#[cfg(feature = "v2")]
+use crate::v2::ParseOhttpKeysError;
 #[cfg(feature = "v2")]
 use crate::OhttpKeys;
 
@@ -212,11 +216,8 @@ impl<'a> bip21::SerializeParams for &'a PayjoinExtras {
             ("pjos", if self.disable_output_substitution { "1" } else { "0" }.to_string()),
         ];
         #[cfg(feature = "v2")]
-        if let Some(ohttp_keys) = self.ohttp_keys.clone().and_then(|c| c.encode().ok()) {
-            let config =
-                bitcoin::base64::Config::new(bitcoin::base64::CharacterSet::UrlSafe, false);
-            let base64_ohttp_keys = bitcoin::base64::encode_config(ohttp_keys, config);
-            params.push(("ohttp", base64_ohttp_keys));
+        if let Some(ohttp_keys) = &self.ohttp_keys {
+            params.push(("ohttp", ohttp_keys.to_string()));
         } else {
             log::warn!("Failed to encode ohttp config, ignoring");
         }
@@ -242,11 +243,8 @@ impl<'a> bip21::de::DeserializationState<'a> for DeserializationState {
             "ohttp" if self.ohttp.is_none() => {
                 let base64_config =
                     Cow::try_from(value).map_err(|_| InternalPjParseError::NotUtf8)?;
-                let config_bytes =
-                    bitcoin::base64::decode_config(&*base64_config, bitcoin::base64::URL_SAFE)
-                        .map_err(|_| InternalPjParseError::NotBase64)?;
-                let config = OhttpKeys::decode(&config_bytes)
-                    .map_err(|_| InternalPjParseError::DecodeOhttpKeys)?;
+                let config = OhttpKeys::from_str(&base64_config)
+                    .map_err(InternalPjParseError::ParseOhttpKeys)?;
                 self.ohttp = Some(config);
                 Ok(bip21::de::ParamKind::Known)
             }
@@ -307,11 +305,9 @@ impl std::fmt::Display for PjParseError {
             }
             InternalPjParseError::MissingEndpoint => write!(f, "Missing payjoin endpoint"),
             InternalPjParseError::NotUtf8 => write!(f, "Endpoint is not valid UTF-8"),
-            #[cfg(feature = "v2")]
-            InternalPjParseError::NotBase64 => write!(f, "ohttp config is not valid base64"),
             InternalPjParseError::BadEndpoint => write!(f, "Endpoint is not valid"),
             #[cfg(feature = "v2")]
-            InternalPjParseError::DecodeOhttpKeys => write!(f, "ohttp config is not valid"),
+            InternalPjParseError::ParseOhttpKeys(e) => write!(f, "OHTTP keys are not valid: {}", e),
             InternalPjParseError::UnsecureEndpoint => {
                 write!(f, "Endpoint scheme is not secure (https or onion)")
             }
@@ -325,11 +321,9 @@ enum InternalPjParseError {
     MultipleParams(&'static str),
     MissingEndpoint,
     NotUtf8,
-    #[cfg(feature = "v2")]
-    NotBase64,
     BadEndpoint,
     #[cfg(feature = "v2")]
-    DecodeOhttpKeys,
+    ParseOhttpKeys(ParseOhttpKeysError),
     UnsecureEndpoint,
 }