From e8220af0fa1161f06a92048737b9f7e10cf89df8 Mon Sep 17 00:00:00 2001 From: Yuval Kogman Date: Thu, 28 Nov 2024 02:47:47 +0100 Subject: [PATCH] Remove Option wrapper from fragment param setters Clearing of the values is not used anywhere, and this simplifies their type signatures. PjUriBuilder left unmodified, there is a ticket for it already. --- payjoin/src/uri/mod.rs | 12 +++++-- payjoin/src/uri/url_ext.rs | 64 ++++++++++++++++---------------------- 2 files changed, 35 insertions(+), 41 deletions(-) diff --git a/payjoin/src/uri/mod.rs b/payjoin/src/uri/mod.rs index a790d019..62250f8e 100644 --- a/payjoin/src/uri/mod.rs +++ b/payjoin/src/uri/mod.rs @@ -181,11 +181,17 @@ impl PjUriBuilder { #[allow(unused_mut)] let mut pj = origin; #[cfg(feature = "v2")] - pj.set_receiver_pubkey(receiver_pubkey); + if let Some(receiver_pubkey) = receiver_pubkey { + pj.set_receiver_pubkey(receiver_pubkey); + } #[cfg(feature = "v2")] - pj.set_ohttp(ohttp_keys); + if let Some(ohttp_keys) = ohttp_keys { + pj.set_ohttp(ohttp_keys); + } #[cfg(feature = "v2")] - pj.set_exp(expiry); + if let Some(expiry) = expiry { + pj.set_exp(expiry); + } Self { address, amount: None, message: None, label: None, pj, pjos: false } } /// Set the amount you want to receive. diff --git a/payjoin/src/uri/url_ext.rs b/payjoin/src/uri/url_ext.rs index 3f1cebfa..2a164068 100644 --- a/payjoin/src/uri/url_ext.rs +++ b/payjoin/src/uri/url_ext.rs @@ -11,11 +11,11 @@ use crate::OhttpKeys; /// Parse and set fragment parameters from `&pj=` URI parameter URLs pub(crate) trait UrlExt { fn receiver_pubkey(&self) -> Result; - fn set_receiver_pubkey(&mut self, exp: Option); + fn set_receiver_pubkey(&mut self, exp: HpkePublicKey); fn ohttp(&self) -> Option; - fn set_ohttp(&mut self, ohttp: Option); + fn set_ohttp(&mut self, ohttp: OhttpKeys); fn exp(&self) -> Option; - fn set_exp(&mut self, exp: Option); + fn set_exp(&mut self, exp: std::time::SystemTime); } impl UrlExt for Url { @@ -37,16 +37,14 @@ impl UrlExt for Url { } /// Set the receiver's public key in the URL fragment - fn set_receiver_pubkey(&mut self, pubkey: Option) { + fn set_receiver_pubkey(&mut self, pubkey: HpkePublicKey) { let rk_hrp: bech32::Hrp = bech32::Hrp::parse("RK").unwrap(); set_param( self, "RK1", - pubkey.map(|k| { - crate::bech32::nochecksum::encode(rk_hrp, &k.to_compressed_bytes()) - .expect("encoding compressed pubkey bytes should never fail") - }), + &crate::bech32::nochecksum::encode(rk_hrp, &pubkey.to_compressed_bytes()) + .expect("encoding compressed pubkey bytes should never fail"), ) } @@ -56,9 +54,7 @@ impl UrlExt for Url { } /// Set the ohttp parameter in the URL fragment - fn set_ohttp(&mut self, ohttp: Option) { - set_param(self, "OH1", ohttp.map(|o| o.to_string())) - } + fn set_ohttp(&mut self, ohttp: OhttpKeys) { set_param(self, "OH1", &ohttp.to_string()) } /// Retrieve the exp parameter from the URL fragment fn exp(&self) -> Option { @@ -80,21 +76,21 @@ impl UrlExt for Url { } /// Set the exp parameter in the URL fragment - fn set_exp(&mut self, exp: Option) { - let exp_str = exp.map(|e| { - 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, - }; + fn set_exp(&mut self, exp: std::time::SystemTime) { + let t = match exp.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 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, "EX1", exp_str) + let ex_hrp: bech32::Hrp = bech32::Hrp::parse("EX").unwrap(); + + let exp_str = crate::bech32::nochecksum::encode(ex_hrp, &buf) + .expect("encoding u32 timestamp should never fail"); + + set_param(self, "EX1", &exp_str) } } @@ -112,7 +108,7 @@ where None } -fn set_param(url: &mut Url, prefix: &str, param: Option) { +fn set_param(url: &mut Url, prefix: &str, param: &str) { let fragment = url.fragment().unwrap_or(""); let mut fragment = fragment.to_string(); if let Some(start) = fragment.find(prefix) { @@ -123,12 +119,10 @@ fn set_param(url: &mut Url, prefix: &str, param: Option) { } } - if let Some(param) = param { - if !fragment.is_empty() { - fragment.push('+'); - } - fragment.push_str(¶m); + if !fragment.is_empty() { + fragment.push('+'); } + fragment.push_str(param); url.set_fragment(if fragment.is_empty() { None } else { Some(&fragment) }); } @@ -144,13 +138,10 @@ mod tests { let serialized = "OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC"; let ohttp_keys = OhttpKeys::from_str(serialized).unwrap(); - url.set_ohttp(Some(ohttp_keys.clone())); + url.set_ohttp(ohttp_keys.clone()); assert_eq!(url.fragment(), Some(serialized)); assert_eq!(url.ohttp(), Some(ohttp_keys)); - - url.set_ohttp(None); - assert_eq!(url.fragment(), None); } #[test] @@ -159,13 +150,10 @@ mod tests { let exp_time = std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1720547781); - url.set_exp(Some(exp_time)); + url.set_exp(exp_time); assert_eq!(url.fragment(), Some("EX1C4UC6ES")); assert_eq!(url.exp(), Some(exp_time)); - - url.set_exp(None); - assert_eq!(url.fragment(), None); } #[test]