From cfc818c9b0900e6e15cd7c56f9ecbd0781f0c72c Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 27 Jun 2024 00:07:24 -0400 Subject: [PATCH 1/4] Fix clippy --- payjoin/src/receive/v2.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/payjoin/src/receive/v2.rs b/payjoin/src/receive/v2.rs index 37d8c115..8470a51f 100644 --- a/payjoin/src/receive/v2.rs +++ b/payjoin/src/receive/v2.rs @@ -98,9 +98,7 @@ 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() }) From cc2dc5b4e563b70a8566be233f7234044cb4b196 Mon Sep 17 00:00:00 2001 From: DanGould Date: Tue, 25 Jun 2024 14:16:47 -0400 Subject: [PATCH 2/4] Separate OhttpKeys Display and serde::Serialize --- Cargo.lock | 202 ++++++++++++++++++++++++++++------ payjoin-cli/src/app/config.rs | 9 +- payjoin-cli/tests/e2e.rs | 5 +- payjoin/src/send/mod.rs | 26 +---- payjoin/src/uri.rs | 26 ++--- payjoin/src/v2.rs | 63 +++++++++-- payjoin/tests/integration.rs | 14 +-- 7 files changed, 245 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49963c69..d5816578 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -357,9 +357,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.101" +version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac367972e516d45567c7eafc73d24e1c193dcf200a8d94e9db7b3d38b349572d" +checksum = "779e6b7d17797c0b42023d417228c02889300190e700cb074c3438d9c541d332" [[package]] name = "cfg-if" @@ -466,9 +466,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "0.3.0" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d4198f73e42b4936b35b5bb248d81d2b595ecb170da0bac7655c54eedfa8da8" +checksum = "033f6b7a4acb1f358c742aaca805c939ee73b4c6209ae4318ec7aca81c42e646" dependencies = [ "os_str_bytes", ] @@ -695,6 +695,15 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e8566979429cf69b49a5c740c60791108e86440e8be149bbea4fe54d2c32d6e2" +[[package]] +name = "deranged" +version = "0.3.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b42b6fa04a440b495c8b04d0e71b707c585f83cb9cb28cf8cd0d976c315e31b4" +dependencies = [ + "powerfmt", +] + [[package]] name = "digest" version = "0.9.0" @@ -1065,6 +1074,15 @@ dependencies = [ "digest 0.10.7", ] +[[package]] +name = "home" +version = "0.5.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3d1354bf6b7235cb4a0576c2619fd4ed18183f689b12b006a0ee7329eeff9a5" +dependencies = [ + "windows-sys 0.52.0", +] + [[package]] name = "hpke" version = "0.11.0" @@ -1257,6 +1275,25 @@ dependencies = [ "webpki-roots 0.26.3", ] +[[package]] +name = "hyper-rustls" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ee4be2c948921a1a5320b629c4193916ed787a7f7f293fd3f7f5a6c9de74155" +dependencies = [ + "futures-util", + "http 1.1.0", + "hyper 1.3.1", + "hyper-util", + "rustls 0.23.10", + "rustls-native-certs 0.7.0", + "rustls-pki-types", + "tokio", + "tokio-rustls 0.26.0", + "tower-service", + "webpki-roots 0.26.3", +] + [[package]] name = "hyper-tungstenite" version = "0.13.0" @@ -1437,9 +1474,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.21" +version = "0.4.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" +checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" [[package]] name = "matchers" @@ -1521,6 +1558,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-conv" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" + [[package]] name = "num_cpus" version = "1.16.0" @@ -1533,9 +1576,9 @@ dependencies = [ [[package]] name = "object" -version = "0.36.0" +version = "0.36.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "576dfe1fc8f9df304abb159d767a29d0476f7750fbf8aa7ad07816004a207434" +checksum = "081b846d1d56ddfc18fdf1a922e4f6e07a11768ea1b92dec44e42b72712ccfce" dependencies = [ "memchr", ] @@ -1909,6 +1952,12 @@ dependencies = [ "universal-hash 0.5.1", ] +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.17" @@ -1948,6 +1997,53 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "quinn" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4ceeeeabace7857413798eb1ffa1e9c905a9946a57d81fb69b4b71c4d8eb3ad" +dependencies = [ + "bytes", + "pin-project-lite", + "quinn-proto", + "quinn-udp", + "rustc-hash", + "rustls 0.23.10", + "thiserror", + "tokio", + "tracing", +] + +[[package]] +name = "quinn-proto" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddf517c03a109db8100448a4be38d498df8a210a99fe0e1b9eaf39e78c640efe" +dependencies = [ + "bytes", + "rand", + "ring 0.17.8", + "rustc-hash", + "rustls 0.23.10", + "slab", + "thiserror", + "tinyvec", + "tracing", +] + +[[package]] +name = "quinn-udp" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9096629c45860fc7fb143e125eb826b5e721e10be3263160c7d60ca832cf8c46" +dependencies = [ + "libc", + "once_cell", + "socket2 0.5.7", + "tracing", + "windows-sys 0.52.0", +] + [[package]] name = "quote" version = "1.0.36" @@ -1995,7 +2091,7 @@ checksum = "52c4f3084aa3bc7dfbba4eff4fab2a54db4324965d8872ab933565e6fbd83bc6" dependencies = [ "pem", "ring 0.16.20", - "time 0.3.20", + "time 0.3.36", "yasna", ] @@ -2049,14 +2145,14 @@ dependencies = [ [[package]] name = "regex" -version = "1.9.6" +version = "1.10.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebee201405406dbf528b8b672104ae6d6d63e6d118cb10e4d51abbc7b58044ff" +checksum = "b91213439dad192326a0d7c6ee3955910425f441d7038e0d6933b0aec5c4517f" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.3.9", - "regex-syntax 0.7.5", + "regex-automata 0.4.7", + "regex-syntax 0.8.4", ] [[package]] @@ -2070,13 +2166,13 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.3.9" +version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59b23e92ee4318893fa3fe3e6fb365258efbfe6ac6ab30f090cdcbb7aa37efa9" +checksum = "38caf58cc5ef2fed281f89292ef23f6365465ed9a41b7a7754eb4e26496c92df" dependencies = [ "aho-corasick", "memchr", - "regex-syntax 0.7.5", + "regex-syntax 0.8.4", ] [[package]] @@ -2087,15 +2183,15 @@ checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" [[package]] name = "regex-syntax" -version = "0.7.5" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" +checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b" [[package]] name = "reqwest" -version = "0.12.4" +version = "0.12.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "566cafdd92868e0939d3fb961bd0dc25fcfaaed179291093b3d43e6b3150ea10" +checksum = "c7d6d2a27d57148378eb5e111173f4276ad26340ecc5c49a4a2152167a2d6a37" dependencies = [ "base64 0.22.1", "bytes", @@ -2105,7 +2201,7 @@ dependencies = [ "http-body 1.0.0", "http-body-util", "hyper 1.3.1", - "hyper-rustls 0.26.0", + "hyper-rustls 0.27.2", "hyper-util", "ipnet", "js-sys", @@ -2114,7 +2210,8 @@ dependencies = [ "once_cell", "percent-encoding", "pin-project-lite", - "rustls 0.22.4", + "quinn", + "rustls 0.23.10", "rustls-native-certs 0.7.0", "rustls-pemfile 2.1.2", "rustls-pki-types", @@ -2123,7 +2220,7 @@ dependencies = [ "serde_urlencoded", "sync_wrapper", "tokio", - "tokio-rustls 0.25.0", + "tokio-rustls 0.26.0", "tower-service", "url", "wasm-bindgen", @@ -2190,6 +2287,12 @@ version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "rustc_version" version = "0.4.0" @@ -2238,6 +2341,20 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rustls" +version = "0.23.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05cff451f60db80f490f3c182b77c35260baace73209e9cdbbe526bfe3a4d402" +dependencies = [ + "once_cell", + "ring 0.17.8", + "rustls-pki-types", + "rustls-webpki 0.102.4", + "subtle", + "zeroize", +] + [[package]] name = "rustls-native-certs" version = "0.6.3" @@ -2613,9 +2730,9 @@ dependencies = [ [[package]] name = "sync_wrapper" -version = "0.1.2" +version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" +checksum = "a7065abeca94b6a8a577f9bd45aa0867a2238b74e8eb67cf10d492bc39351394" [[package]] name = "tar" @@ -2718,19 +2835,22 @@ dependencies = [ [[package]] name = "time" -version = "0.3.20" +version = "0.3.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd0cbfecb4d19b5ea75bb31ad904eb5b9fa13f21079c3b92017ebdf4999a5890" +checksum = "5dfd88e563464686c916c7e46e623e520ddc6d79fa6641390f2e3fa86e83e885" dependencies = [ + "deranged", + "num-conv", + "powerfmt", "serde", "time-core", ] [[package]] name = "time-core" -version = "0.1.0" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2e153e1f1acaef8acc537e68b44906d2db6436e2b35ac2c6b42640fff91f00fd" +checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" [[package]] name = "tinyvec" @@ -2798,6 +2918,17 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-rustls" +version = "0.26.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c7bc40d0e5a97695bb96e27995cd3a08538541b0a846f65bba7a359f36700d4" +dependencies = [ + "rustls 0.23.10", + "rustls-pki-types", + "tokio", +] + [[package]] name = "tokio-tungstenite" version = "0.21.0" @@ -3012,9 +3143,9 @@ checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" [[package]] name = "url" -version = "2.5.0" +version = "2.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "31e6302e3bb753d46e83516cae55ae196fc0c309407cf11ab35cc51a4c2a4633" +checksum = "22784dbdf76fdde8af1aeda5622b546b422b6fc585325248a2bf9f5e41e94d6c" dependencies = [ "form_urlencoded", "idna", @@ -3154,13 +3285,14 @@ dependencies = [ [[package]] name = "which" -version = "4.4.0" +version = "4.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2441c784c52b289a054b7201fc93253e288f094e2f4be9058343127c4226a269" +checksum = "87ba24419a2078cd2b0f2ede2691b6c66d8e47836da3b6db8265ebad47afbfc7" dependencies = [ "either", - "libc", + "home", "once_cell", + "rustix", ] [[package]] @@ -3379,7 +3511,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e17bb3549cc1321ae1296b9cdc2698e2b6cb1992adfa19a8c72e5b7a738f44cd" dependencies = [ - "time 0.3.20", + "time 0.3.36", ] [[package]] diff --git a/payjoin-cli/src/app/config.rs b/payjoin-cli/src/app/config.rs index 63fc0a50..e335dce7 100644 --- a/payjoin-cli/src/app/config.rs +++ b/payjoin-cli/src/app/config.rs @@ -99,7 +99,14 @@ impl AppConfig { )? .set_override_option( "ohttp_keys", - matches.get_one::("ohttp_keys").map(|s| s.as_str()), + matches.get_one::("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() + }), )? }; diff --git a/payjoin-cli/tests/e2e.rs b/payjoin-cli/tests/e2e.rs index aa915f6e..900a93be 100644 --- a/payjoin-cli/tests/e2e.rs +++ b/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() diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 2cd942cc..ba532bb7 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -401,13 +401,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 +470,7 @@ impl<'de> Deserialize<'de> for RequestContext { url::Url::from_str(&map.next_value::()?) .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 +496,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, diff --git a/payjoin/src/uri.rs b/payjoin/src/uri.rs index e9aee45d..7d00a5d8 100644 --- a/payjoin/src/uri.rs +++ b/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, } diff --git a/payjoin/src/v2.rs b/payjoin/src/v2.rs index 7f130b21..cb30eac8 100644 --- a/payjoin/src/v2.rs +++ b/payjoin/src/v2.rs @@ -1,6 +1,7 @@ use std::ops::{Deref, DerefMut}; use std::{error, fmt}; +use bitcoin::base64; use bitcoin::secp256k1::ecdh::SharedSecret; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use chacha20poly1305::aead::{Aead, KeyInit, OsRng, Payload}; @@ -249,6 +250,24 @@ impl OhttpKeys { } } +impl fmt::Display for OhttpKeys { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let encoded = + base64::encode_config(self.encode().map_err(|_| fmt::Error)?, base64::URL_SAFE_NO_PAD); + write!(f, "{}", encoded) + } +} + +impl std::str::FromStr for OhttpKeys { + type Err = ParseOhttpKeysError; + + fn from_str(s: &str) -> Result { + let bytes = base64::decode_config(s, base64::URL_SAFE_NO_PAD) + .map_err(ParseOhttpKeysError::DecodeBase64)?; + OhttpKeys::decode(&bytes).map_err(ParseOhttpKeysError::DecodeKeyConfig) + } +} + impl PartialEq for OhttpKeys { fn eq(&self, other: &Self) -> bool { match (self.encode(), other.encode()) { @@ -276,12 +295,8 @@ impl<'de> serde::Deserialize<'de> for OhttpKeys { where D: serde::Deserializer<'de>, { - use bitcoin::base64; - - let base64_string = String::deserialize(deserializer)?; - let bytes = base64::decode_config(base64_string, base64::URL_SAFE) - .map_err(serde::de::Error::custom)?; - Ok(OhttpKeys(ohttp::KeyConfig::decode(&bytes).map_err(serde::de::Error::custom)?)) + let bytes = Vec::::deserialize(deserializer)?; + OhttpKeys::decode(&bytes).map_err(serde::de::Error::custom) } } @@ -290,11 +305,33 @@ impl serde::Serialize for OhttpKeys { where S: serde::Serializer, { - use bitcoin::base64; + let bytes = self.encode().map_err(serde::ser::Error::custom)?; + bytes.serialize(serializer) + } +} + +#[derive(Debug)] +pub enum ParseOhttpKeysError { + DecodeBase64(base64::DecodeError), + DecodeKeyConfig(ohttp::Error), +} + +impl std::fmt::Display for ParseOhttpKeysError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ParseOhttpKeysError::DecodeBase64(e) => write!(f, "Failed to decode base64: {}", e), + ParseOhttpKeysError::DecodeKeyConfig(e) => + write!(f, "Failed to decode KeyConfig: {}", e), + } + } +} - let bytes = self.0.encode().map_err(serde::ser::Error::custom)?; - let base64_string = base64::encode_config(bytes, base64::URL_SAFE); - base64_string.serialize(serializer) +impl std::error::Error for ParseOhttpKeysError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ParseOhttpKeysError::DecodeBase64(e) => Some(e), + ParseOhttpKeysError::DecodeKeyConfig(e) => Some(e), + } } } @@ -304,6 +341,8 @@ mod test { #[test] fn test_ohttp_keys_roundtrip() { + use std::str::FromStr; + use ohttp::hpke::{Aead, Kdf, Kem}; use ohttp::{KeyId, SymmetricSuite}; const KEY_ID: KeyId = 1; @@ -311,8 +350,8 @@ mod test { const SYMMETRIC: &[SymmetricSuite] = &[ohttp::SymmetricSuite::new(Kdf::HkdfSha256, Aead::ChaCha20Poly1305)]; let keys = OhttpKeys(ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).unwrap()); - let serialized = serde_json::to_string(&keys).unwrap(); - let deserialized: OhttpKeys = serde_json::from_str(&serialized).unwrap(); + let serialized = &keys.to_string(); + let deserialized = OhttpKeys::from_str(serialized).unwrap(); assert_eq!(keys.encode().unwrap(), deserialized.encode().unwrap()); } } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 119135b6..90bd4450 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -5,7 +5,7 @@ mod integration { use std::str::FromStr; use bitcoin::psbt::Psbt; - use bitcoin::{base64, Amount, FeeRate, OutPoint}; + use bitcoin::{Amount, FeeRate, OutPoint}; use bitcoind::bitcoincore_rpc::core_rpc_json::{AddressType, WalletProcessPsbtResult}; use bitcoind::bitcoincore_rpc::{self, RpcApi}; use log::{log_enabled, Level}; @@ -21,6 +21,7 @@ mod integration { #[cfg(not(feature = "v2"))] mod v1 { + use bitcoin::base64; use log::debug; use payjoin::receive::{Headers, PayjoinProposal, UncheckedProposal}; use payjoin::{PjUri, PjUriBuilder, UriExt}; @@ -303,14 +304,9 @@ mod integration { #[tokio::test] async fn test_bad_ohttp_keys() { - let bad_ohttp_keys = OhttpKeys::decode( - &base64::decode_config( - "AQAg3WpRjS0aqAxQUoLvpas2VYjT2oIg6-3XSiB-QiYI1BAABAABAAM", - base64::URL_SAFE, - ) - .expect("invalid base64"), - ) - .expect("Invalid OhttpKeys"); + let bad_ohttp_keys = + OhttpKeys::from_str("AQAg3WpRjS0aqAxQUoLvpas2VYjT2oIg6-3XSiB-QiYI1BAABAABAAM") + .expect("Invalid OhttpKeys"); std::env::set_var("RUST_LOG", "debug"); let (cert, key) = local_cert_key(); From af92319ae5a0d904ffcdb5c03e92dc7d0dd73cfa Mon Sep 17 00:00:00 2001 From: DanGould Date: Wed, 26 Jun 2024 12:35:52 -0400 Subject: [PATCH 3/4] Simplify base64 dependency usage - use psbt.to_string() to get base64 - use base64::encode/decode_config with const configs --- payjoin-cli/src/app/mod.rs | 7 ++----- payjoin-cli/src/app/v1.rs | 6 +++--- payjoin-cli/src/app/v2.rs | 4 ++-- payjoin-directory/src/lib.rs | 8 ++------ payjoin/src/receive/v2.rs | 6 ++---- payjoin/src/send/mod.rs | 7 +++---- payjoin/tests/integration.rs | 13 +++++-------- 7 files changed, 19 insertions(+), 32 deletions(-) diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs index 5cdf8712..2bc54f6e 100644 --- a/payjoin-cli/src/app/mod.rs +++ b/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 { Ok(http_agent_builder()?.build()?) } diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 998b6f3f..37a5eed0 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/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()))? }, diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index 56c6fb8c..a08cb922 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/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()))? }, diff --git a/payjoin-directory/src/lib.rs b/payjoin-directory/src/lib.rs index 97a36889..94c95806 100644 --- a/payjoin-directory/src/lib.rs +++ b/payjoin-directory/src/lib.rs @@ -102,10 +102,7 @@ fn init_ohttp() -> Result { // 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 for HandlerError { } async fn post_session(body: Body) -> Result, 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 = base64::decode_config(base64_id, b64_config) + let pubkey_bytes: Vec = 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()))?; diff --git a/payjoin/src/receive/v2.rs b/payjoin/src/receive/v2.rs index 8470a51f..88dfa555 100644 --- a/payjoin/src/receive/v2.rs +++ b/payjoin/src/receive/v2.rs @@ -106,9 +106,7 @@ impl SessionInitializer { } 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 @@ -443,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> { diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index ba532bb7..e175f831 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -375,10 +375,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)?) } diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 90bd4450..4397816d 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -21,7 +21,6 @@ mod integration { #[cfg(not(feature = "v2"))] mod v1 { - use bitcoin::base64; use log::debug; use payjoin::receive::{Headers, PayjoinProposal, UncheckedProposal}; use payjoin::{PjUri, PjUriBuilder, UriExt}; @@ -103,7 +102,7 @@ mod integration { let proposal = handle_proposal(proposal, receiver); let psbt = proposal.psbt(); debug!("Receiver's Payjoin proposal PSBT: {:#?}", &psbt); - base64::encode(&psbt.serialize()) + psbt.to_string() } fn handle_proposal( @@ -179,7 +178,7 @@ mod integration { |psbt: &Psbt| { Ok(receiver .wallet_process_psbt( - &bitcoin::base64::encode(psbt.serialize()), + &psbt.to_string(), None, None, Some(true), // check that the receiver properly clears keypaths @@ -270,9 +269,8 @@ mod integration { sender: &bitcoincore_rpc::Client, psbt: Psbt, ) -> Result> { - let payjoin_base64_string = base64::encode(&psbt.serialize()); let payjoin_psbt = - sender.wallet_process_psbt(&payjoin_base64_string, None, None, None)?.psbt; + sender.wallet_process_psbt(&psbt.to_string(), None, None, None)?.psbt; let payjoin_psbt = sender.finalize_psbt(&payjoin_psbt, Some(false))?.psbt.unwrap(); let payjoin_psbt = Psbt::from_str(&payjoin_psbt)?; debug!("Sender's Payjoin PSBT: {:#?}", payjoin_psbt); @@ -703,7 +701,7 @@ mod integration { |psbt: &Psbt| { Ok(receiver .wallet_process_psbt( - &bitcoin::base64::encode(psbt.serialize()), + &psbt.to_string(), None, None, Some(true), // check that the receiver properly clears keypaths @@ -832,9 +830,8 @@ mod integration { sender: &bitcoincore_rpc::Client, psbt: Psbt, ) -> Result> { - let payjoin_base64_string = bitcoin::base64::encode(&psbt.serialize()); let payjoin_psbt = - sender.wallet_process_psbt(&payjoin_base64_string, None, None, None)?.psbt; + sender.wallet_process_psbt(&psbt.to_string(), None, None, None)?.psbt; let payjoin_psbt = sender.finalize_psbt(&payjoin_psbt, Some(false))?.psbt.unwrap(); let payjoin_psbt = Psbt::from_str(&payjoin_psbt)?; From b7cf5e84d6ab9b7f6bb4c649150b8d7429312a5d Mon Sep 17 00:00:00 2001 From: DanGould Date: Tue, 25 Jun 2024 14:20:05 -0400 Subject: [PATCH 4/4] Remove redundant PartialEq implementation The `OhttpKeys` type is `Eq` so explicit impl can be replaced with a derive macro. --- payjoin/src/send/mod.rs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index e175f831..1f247bfc 100644 --- a/payjoin/src/send/mod.rs +++ b/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 {}