From a6dc62a8ff8d52f10f292d44b490a47b6f111845 Mon Sep 17 00:00:00 2001 From: spacebear Date: Wed, 22 Jul 2026 00:27:57 +0000 Subject: [PATCH 1/2] Insulate bitcoin_uri from the public API Part of the 1.0 API hardening: remove the foreign bitcoin_uri types from payjoin's public API so that a breaking release of bitcoin_uri no longer forces a breaking release of payjoin. This follows the same approach as the bitcoin-ohttp and bitcoin-hpke insulation in #1702. Previously `Uri` and `PjUri` were type aliases to `bitcoin_uri::Uri`, so the wrapped type, its public fields (including `bitcoin_uri::Param` labels and messages), the `bitcoin_uri::de::Error` parse error, and the `bitcoin_uri::Uri` returned by `check_pj_supported` were all reachable through the public API. The parser traits were also implemented directly on the public `MaybePayjoinExtras` and `PayjoinExtras` types. The crate now only names `bitcoin_uri` in private newtype fields and on the private adapters. No `bitcoin_uri` type or trait impl is reachable through the public API. --- Cargo-minimal.lock | 1 - Cargo-recent.lock | 1 - fuzz/Cargo.toml | 1 - fuzz/fuzz_targets/uri/deserialize_pjuri.rs | 23 +- payjoin-cli/src/app/v1.rs | 8 +- payjoin-cli/src/app/v2/mod.rs | 7 +- payjoin-ffi/src/uri/mod.rs | 37 ++-- payjoin/Cargo.toml | 1 + payjoin/src/core/mod.rs | 2 +- payjoin/src/core/receive/v1/mod.rs | 6 +- payjoin/src/core/receive/v2/mod.rs | 16 +- payjoin/src/core/receive/v2/session.rs | 6 +- payjoin/src/core/send/v1.rs | 20 +- payjoin/src/core/send/v2/mod.rs | 4 +- payjoin/src/core/send/v2/session.rs | 2 +- payjoin/src/core/uri/error.rs | 60 +++++ payjoin/src/core/uri/mod.rs | 243 ++++++++++++++++----- payjoin/src/core/uri/v1.rs | 34 ++- payjoin/src/core/uri/v2.rs | 24 +- payjoin/tests/integration.rs | 24 +- payjoin/tests/uri_api.rs | 103 +++++++++ 21 files changed, 454 insertions(+), 169 deletions(-) create mode 100644 payjoin/tests/uri_api.rs diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index eafd6b958..ae887d999 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -2648,7 +2648,6 @@ dependencies = [ name = "payjoin-fuzz" version = "0.0.1" dependencies = [ - "bitcoin_uri", "home", "libfuzzer-sys", "payjoin", diff --git a/Cargo-recent.lock b/Cargo-recent.lock index 46667aac0..f2f39df53 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -2779,7 +2779,6 @@ dependencies = [ name = "payjoin-fuzz" version = "0.0.1" dependencies = [ - "bitcoin_uri", "home", "libfuzzer-sys", "payjoin", diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 312c18a0d..14e7962ad 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -11,7 +11,6 @@ cargo-fuzz = true default = [] [dependencies] -bitcoin_uri = { version = "0.1.0" } home = "=0.5.11" libfuzzer-sys = { version = "0.4.10" } payjoin = { path = "../payjoin", default-features = false, features = [ diff --git a/fuzz/fuzz_targets/uri/deserialize_pjuri.rs b/fuzz/fuzz_targets/uri/deserialize_pjuri.rs index 99e539219..2d15acc28 100644 --- a/fuzz/fuzz_targets/uri/deserialize_pjuri.rs +++ b/fuzz/fuzz_targets/uri/deserialize_pjuri.rs @@ -2,9 +2,8 @@ use std::any::{Any, TypeId}; -use bitcoin_uri::Param; use libfuzzer_sys::fuzz_target; -use payjoin::{Uri, UriExt}; +use payjoin::Uri; fn do_test(data: &[u8]) { if let Ok(uri_str) = std::str::from_utf8(data) { @@ -12,29 +11,29 @@ fn do_test(data: &[u8]) { Ok(uri) => uri.assume_checked(), Err(_) => return, }; - let address = pj_uri.address.is_spend_standard(); - if !address { + if !pj_uri.address().is_spend_standard() { return; } - if let Some(label) = pj_uri.clone().label { - if TypeId::of::() != label.type_id() { + if let Some(label) = pj_uri.label() { + if TypeId::of::() != label.type_id() { return; } }; - if let Some(message) = pj_uri.clone().message { - if TypeId::of::() != message.type_id() { + if let Some(message) = pj_uri.message() { + if TypeId::of::() != message.type_id() { return; } }; - let extras = match pj_uri.clone().check_pj_supported() { - Ok(res) => res.extras, + let extras = match pj_uri.check_pj_supported() { + Ok(res) => res, Err(_) => return, }; assert!( - TypeId::of::() == extras.output_substitution().type_id() + TypeId::of::() + == extras.extras().output_substitution().type_id() ); - assert!(TypeId::of::() == extras.endpoint().type_id()) + assert!(TypeId::of::() == extras.extras().endpoint().type_id()) } } diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 4dd654477..34418b206 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -15,7 +15,7 @@ use payjoin::bitcoin::{Amount, FeeRate}; use payjoin::receive::v1::{PayjoinProposal, UncheckedOriginalPayload}; use payjoin::receive::Error; use payjoin::send::v1::SenderBuilder; -use payjoin::{ImplementationError, IntoUrl, Uri, UriExt}; +use payjoin::{ImplementationError, IntoUrl, Uri}; use tokio::net::TcpListener; use tokio::sync::watch; @@ -61,8 +61,8 @@ impl AppTrait for App { Uri::try_from(bip21).map_err(|e| anyhow!("Failed to create URI from BIP21: {}", e))?; let uri = uri.assume_checked(); let uri = uri.check_pj_supported().map_err(|_| anyhow!("URI does not support Payjoin"))?; - let amount = uri.amount.ok_or_else(|| anyhow!("please specify the amount in the Uri"))?; - let psbt = self.create_original_psbt(&uri.address, amount, fee_rate)?; + let amount = uri.amount().ok_or_else(|| anyhow!("please specify the amount in the Uri"))?; + let psbt = self.create_original_psbt(uri.address(), amount, fee_rate)?; let fallback_tx = psbt.clone().extract_tx()?; let (req, ctx) = SenderBuilder::new(psbt, uri.clone()) .build_recommended(fee_rate) @@ -148,7 +148,7 @@ impl App { endpoint, payjoin::OutputSubstitution::Enabled, )?; - pj_uri.amount = Some(amount); + pj_uri.set_amount(amount); Ok(pj_uri.to_string()) } diff --git a/payjoin-cli/src/app/v2/mod.rs b/payjoin-cli/src/app/v2/mod.rs index a127b1fe4..0c7225c7e 100644 --- a/payjoin-cli/src/app/v2/mod.rs +++ b/payjoin-cli/src/app/v2/mod.rs @@ -210,15 +210,14 @@ impl AppTrait for App { fn wallet(&self) -> BitcoindWallet { self.wallet.clone() } async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()> { - use payjoin::UriExt; let uri = Uri::try_from(bip21) .map_err(|e| anyhow!("Failed to create URI from BIP21: {}", e))? .assume_checked() .check_pj_supported() .map_err(|_| anyhow!("URI does not support Payjoin"))?; - let address = uri.address; - let amount = uri.amount.ok_or_else(|| anyhow!("please specify the amount in the Uri"))?; - match uri.extras.pj_param() { + let address = uri.address().clone(); + let amount = uri.amount().ok_or_else(|| anyhow!("please specify the amount in the Uri"))?; + match uri.extras().pj_param() { #[cfg(feature = "v1")] PjParam::V1(pj_param) => { let psbt = self.create_original_psbt(&address, amount, fee_rate)?; diff --git a/payjoin-ffi/src/uri/mod.rs b/payjoin-ffi/src/uri/mod.rs index 9898743a4..c6a36f8de 100644 --- a/payjoin-ffi/src/uri/mod.rs +++ b/payjoin-ffi/src/uri/mod.rs @@ -3,20 +3,19 @@ use std::sync::Arc; pub use error::{PjNotSupported, PjParseError, UrlParseError}; use payjoin::bitcoin::address::NetworkChecked; -use payjoin::UriExt; use crate::error::FfiValidationError; use crate::validation::validate_amount_sat; pub mod error; #[derive(Clone, uniffi::Object)] -pub struct Uri(payjoin::Uri<'static, NetworkChecked>); -impl From for payjoin::Uri<'static, NetworkChecked> { +pub struct Uri(payjoin::Uri); +impl From for payjoin::Uri { fn from(value: Uri) -> Self { value.0 } } -impl From> for Uri { - fn from(value: payjoin::Uri<'static, NetworkChecked>) -> Self { Uri(value) } +impl From> for Uri { + fn from(value: payjoin::Uri) -> Self { Uri(value) } } #[uniffi::export] @@ -27,15 +26,11 @@ impl Uri { .map(|e| e.assume_checked().into()) .map_err(PjParseError::from_err) } - pub fn address(&self) -> String { self.clone().0.address.to_string() } + pub fn address(&self) -> String { self.0.address().to_string() } /// Gets the amount in satoshis. - pub fn amount_sats(&self) -> Option { self.0.amount.map(|x| x.to_sat()) } - pub fn label(&self) -> Option { - self.0.label.clone().and_then(|x| String::try_from(x).ok()) - } - pub fn message(&self) -> Option { - self.0.message.clone().and_then(|x| String::try_from(x).ok()) - } + pub fn amount_sats(&self) -> Option { self.0.amount().map(|x| x.to_sat()) } + pub fn label(&self) -> Option { self.0.label() } + pub fn message(&self) -> Option { self.0.message() } pub fn check_pj_supported(&self) -> Result, PjNotSupported> { self.0 @@ -47,32 +42,32 @@ impl Uri { pub fn as_string(&self) -> String { self.0.clone().to_string() } } -impl From> for PjUri { - fn from(value: payjoin::PjUri<'static>) -> Self { Self(value) } +impl From for PjUri { + fn from(value: payjoin::PjUri) -> Self { Self(value) } } -impl From for payjoin::PjUri<'_> { +impl From for payjoin::PjUri { fn from(value: PjUri) -> Self { value.0 } } #[derive(Clone, uniffi::Object)] -pub struct PjUri(pub payjoin::PjUri<'static>); +pub struct PjUri(pub payjoin::PjUri); #[uniffi::export] impl PjUri { - pub fn address(&self) -> String { self.0.clone().address.to_string() } + pub fn address(&self) -> String { self.0.address().to_string() } /// Number of sats requested as payment - pub fn amount_sats(&self) -> Option { self.0.clone().amount.map(|e| e.to_sat()) } + pub fn amount_sats(&self) -> Option { self.0.amount().map(|e| e.to_sat()) } /// Sets the amount in sats and returns a new PjUri pub fn set_amount_sats(&self, amount_sats: u64) -> Result { let mut uri = self.0.clone(); let amount = validate_amount_sat(amount_sats)?; - uri.amount = Some(amount); + uri.set_amount(amount); Ok(uri.into()) } - pub fn pj_endpoint(&self) -> String { self.0.extras.endpoint().to_string() } + pub fn pj_endpoint(&self) -> String { self.0.extras().endpoint().to_string() } pub fn as_string(&self) -> String { self.0.clone().to_string() } } diff --git a/payjoin/Cargo.toml b/payjoin/Cargo.toml index 26abadbbb..01c0e4213 100644 --- a/payjoin/Cargo.toml +++ b/payjoin/Cargo.toml @@ -27,6 +27,7 @@ _core = [ "serde_json", "dep:percent-encoding-rfc3986", "bitcoin_uri", + "bitcoin_uri/std", "serde", "bitcoin/serde", ] diff --git a/payjoin/src/core/mod.rs b/payjoin/src/core/mod.rs index ff1fdb2af..ce01b7c6e 100644 --- a/payjoin/src/core/mod.rs +++ b/payjoin/src/core/mod.rs @@ -21,7 +21,7 @@ pub use url::{ParseError as UrlParseError, Url}; #[cfg(feature = "v2")] pub mod time; pub mod uri; -pub use uri::{PjParam, PjParseError, PjUri, Uri, UriExt}; +pub use uri::{PjParam, PjParseError, PjUri, Uri, UriParseError}; pub(crate) mod error_codes; pub(crate) mod output_substitution; diff --git a/payjoin/src/core/receive/v1/mod.rs b/payjoin/src/core/receive/v1/mod.rs index a47cbd67b..474458c42 100644 --- a/payjoin/src/core/receive/v1/mod.rs +++ b/payjoin/src/core/receive/v1/mod.rs @@ -47,14 +47,14 @@ pub trait Headers { fn get_header(&self, key: &str) -> Option<&str>; } -pub fn build_v1_pj_uri<'a>( +pub fn build_v1_pj_uri( address: &bitcoin::Address, endpoint: impl IntoUrl, output_substitution: OutputSubstitution, -) -> Result, PjParseError> { +) -> Result { let pj_param = PjParam::parse(endpoint)?; let extras = crate::uri::PayjoinExtras { pj_param, output_substitution }; - Ok(bitcoin_uri::Uri::with_extras(address.clone(), extras)) + Ok(crate::uri::PjUri::from_extras(address.clone(), extras)) } impl UncheckedOriginalPayload { diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index 0e536afad..8f4dd1b9d 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -719,7 +719,7 @@ impl Receiver { } /// Build a V2 Payjoin URI from the receiver's context - pub fn pj_uri<'a>(&self) -> crate::PjUri<'a> { + pub fn pj_uri(&self) -> crate::PjUri { pj_uri(&self.session_context, OutputSubstitution::Disabled) } @@ -1629,10 +1629,10 @@ fn mailbox_endpoint(directory: &Url, id: &ShortId) -> Url { } /// Gets the Payjoin URI from a session context -pub(crate) fn pj_uri<'a>( +pub(crate) fn pj_uri( session_context: &SessionContext, output_substitution: OutputSubstitution, -) -> crate::PjUri<'a> { +) -> crate::PjUri { use crate::uri::PayjoinExtras; let pj_param = crate::uri::PjParam::V2(crate::uri::v2::PjParam::new( session_context.directory.clone(), @@ -1642,8 +1642,10 @@ pub(crate) fn pj_uri<'a>( session_context.receiver_key.public_key().clone(), )); let extras = PayjoinExtras { pj_param, output_substitution }; - let mut uri = bitcoin_uri::Uri::with_extras(session_context.address.clone(), extras); - uri.amount = session_context.amount; + let mut uri = crate::uri::PjUri::from_extras(session_context.address.clone(), extras); + if let Some(amount) = session_context.amount { + uri.set_amount(amount); + } uri } @@ -2504,8 +2506,8 @@ pub mod test { fn test_v2_pj_uri() { let uri = Receiver { state: Initialized {}, session_context: SHARED_CONTEXT.clone() }.pj_uri(); - assert_ne!(uri.extras.pj_param.endpoint().as_str(), EXAMPLE_URL); - assert_eq!(uri.extras.output_substitution, OutputSubstitution::Disabled); + assert_ne!(uri.extras().pj_param().endpoint().as_str(), EXAMPLE_URL); + assert_eq!(uri.extras().output_substitution(), OutputSubstitution::Disabled); } #[test] diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index 24e929931..63fd3c3da 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -109,7 +109,7 @@ impl SessionHistory { } /// Receiver session Payjoin URI - pub fn pj_uri<'a>(&self) -> PjUri<'a> { + pub fn pj_uri(&self) -> PjUri { self.events .iter() .find_map(|event| match event { @@ -1207,8 +1207,8 @@ mod tests { let uri = SessionHistory { events }.pj_uri(); - assert_ne!(uri.extras.pj_param.endpoint().as_str(), EXAMPLE_URL); - assert_eq!(uri.extras.output_substitution, OutputSubstitution::Disabled); + assert_ne!(uri.extras().pj_param().endpoint().as_str(), EXAMPLE_URL); + assert_eq!(uri.extras().output_substitution(), OutputSubstitution::Disabled); Ok(()) } diff --git a/payjoin/src/core/send/v1.rs b/payjoin/src/core/send/v1.rs index 830148133..0e6293dea 100644 --- a/payjoin/src/core/send/v1.rs +++ b/payjoin/src/core/send/v1.rs @@ -47,13 +47,13 @@ impl SenderBuilder { /// to create a [`Sender`] pub fn new(psbt: Psbt, uri: PjUri) -> Self { Self { - endpoint: uri.extras.pj_param.endpoint_url(), + endpoint: uri.extras().pj_param().endpoint_url(), // Adopt the output substitution preference from the URI - output_substitution: uri.extras.output_substitution, + output_substitution: uri.extras().output_substitution(), psbt_ctx_builder: PsbtContextBuilder::new( psbt, - uri.address.script_pubkey(), - uri.amount, + uri.address().script_pubkey(), + uri.amount(), ), } } @@ -247,12 +247,12 @@ mod test { use crate::error_codes::ErrorCode; use crate::send::error::{ResponseError, WellKnownError}; use crate::send::test::create_psbt_context; - use crate::{Uri, UriExt, MAX_CONTENT_LENGTH}; + use crate::{Uri, MAX_CONTENT_LENGTH}; const PJ_URI: &str = "bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?amount=0.02&pjos=0&pj=HTTPS://EXAMPLE.COM/"; - fn pj_uri<'a>() -> PjUri<'a> { + fn pj_uri() -> PjUri { Uri::try_from(PJ_URI) .expect("uri should succeed") .assume_checked() @@ -404,7 +404,7 @@ mod test { ) .expect("sender should succeed"); assert_eq!(sender.psbt_ctx.output_substitution, OutputSubstitution::Disabled); - assert_eq!(&sender.psbt_ctx.payee, &pj_uri().address.script_pubkey()); + assert_eq!(&sender.psbt_ctx.payee, &pj_uri().address().script_pubkey()); let fee_contribution = sender.psbt_ctx.fee_contribution.expect("sender should contribute fees"); assert_eq!(fee_contribution.max_amount, psbt.unsigned_tx.output[0].value); @@ -418,7 +418,7 @@ mod test { .build_recommended(FeeRate::BROADCAST_MIN) .expect("sender should succeed"); assert_eq!(sender.psbt_ctx.output_substitution, OutputSubstitution::Disabled); - assert_eq!(&sender.psbt_ctx.payee, &pj_uri().address.script_pubkey()); + assert_eq!(&sender.psbt_ctx.payee, &pj_uri().address().script_pubkey()); let fee_contribution = sender.psbt_ctx.fee_contribution.expect("sender should contribute fees"); assert_eq!(fee_contribution.max_amount, Amount::from_sat(91)); @@ -426,7 +426,7 @@ mod test { assert_eq!(sender.psbt_ctx.min_fee_rate, FeeRate::from_sat_per_kwu(250)); // Ensure the receiver's output substitution preference is respected either way let mut pj_uri = pj_uri(); - pj_uri.extras.output_substitution = OutputSubstitution::Enabled; + pj_uri.set_output_substitution(OutputSubstitution::Enabled); let sender = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri) .build_recommended(FeeRate::from_sat_per_vb_u32(1)) .expect("sender should succeed"); @@ -436,7 +436,7 @@ mod test { #[test] fn test_always_disable_output_substitution() { let mut pj_uri = pj_uri(); - pj_uri.extras.output_substitution = OutputSubstitution::Enabled; + pj_uri.set_output_substitution(OutputSubstitution::Enabled); let sender = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri) .always_disable_output_substitution() .build_recommended(FeeRate::BROADCAST_MIN) diff --git a/payjoin/src/core/send/v2/mod.rs b/payjoin/src/core/send/v2/mod.rs index 403bc0fbb..221e26fb0 100644 --- a/payjoin/src/core/send/v2/mod.rs +++ b/payjoin/src/core/send/v2/mod.rs @@ -74,11 +74,11 @@ impl SenderBuilder { /// Call [`SenderBuilder::build_recommended()`] or other `build` methods /// to create a [`Sender`] pub fn new(psbt: Psbt, uri: PjUri) -> Self { - match uri.extras.pj_param { + match uri.extras().pj_param() { #[cfg(feature = "v1")] crate::uri::PjParam::V1(_) => unimplemented!("V2 SenderBuilder only supports v2 URLs"), crate::uri::PjParam::V2(pj_param) => - Self::from_parts(psbt, &pj_param, &uri.address, uri.amount), + Self::from_parts(psbt, pj_param, uri.address(), uri.amount()), } } diff --git a/payjoin/src/core/send/v2/session.rs b/payjoin/src/core/send/v2/session.rs index 3adda473e..78126bced 100644 --- a/payjoin/src/core/send/v2/session.rs +++ b/payjoin/src/core/send/v2/session.rs @@ -190,7 +190,7 @@ mod tests { use crate::send::v2::{Sender, SenderBuilder, SessionContext, WithReplyKey}; use crate::send::PsbtContext; use crate::time::Time; - use crate::{HpkeKeyPair, Uri, UriExt}; + use crate::{HpkeKeyPair, Uri}; /// Expired V2 Payjoin URI without Amount inspired by BIP 77 test vector const PJ_URI: &str = "bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?pjos=0&pj=HTTPS://PAYJO.IN/TXJCGKTKXLUUZ%23EX1WKV8CEC-OH1QYPM59NK2LXXS4890SUAXXYT25Z2VAPHP0X7YEYCJXGWAG6UG9ZU6NQ-RK1Q0DJS3VVDXWQQTLQ8022QGXSX7ML9PHZ6EDSF6AKEWQG758JPS2EV"; diff --git a/payjoin/src/core/uri/error.rs b/payjoin/src/core/uri/error.rs index 93c103c47..b60f7d77f 100644 --- a/payjoin/src/core/uri/error.rs +++ b/payjoin/src/core/uri/error.rs @@ -1,6 +1,66 @@ #[derive(Debug)] pub struct PjParseError(pub(super) InternalPjParseError); +/// Error parsing a BIP21 URI into a payjoin [`Uri`](super::Uri). +/// +/// This wraps the underlying `bitcoin_uri` parse error so that a breaking change +/// in that crate does not force a breaking change in this crate's public API. +#[derive(Debug)] +pub struct UriParseError(InternalUriParseError); + +#[derive(Debug)] +enum InternalUriParseError { + /// The BIP21 URI itself (address, amount, or standard parameters) is invalid. + /// + /// The foreign error is held in a private variant so that it does not appear + /// in the public API, while preserving the `source()` chain. + Bip21(bitcoin_uri::de::UriError), + /// The payjoin parameters are invalid. + PayjoinParams(PjParseError), +} + +impl UriParseError { + /// Erases the foreign `bitcoin_uri` parse error into this opaque type. + /// + /// This is an inherent constructor rather than a `From` impl so that + /// `bitcoin_uri` types stay out of the public API. + pub(super) fn from_bip21_error(value: bitcoin_uri::de::Error) -> Self { + match value { + bitcoin_uri::de::Error::Uri(e) => UriParseError(InternalUriParseError::Bip21(e)), + bitcoin_uri::de::Error::Extras(e) => + UriParseError(InternalUriParseError::PayjoinParams(e)), + } + } + + /// The payjoin parameter parse error, if parsing failed because the payjoin + /// parameters were invalid. + #[cfg(test)] + pub(crate) fn payjoin_params(&self) -> Option<&PjParseError> { + match &self.0 { + InternalUriParseError::PayjoinParams(e) => Some(e), + InternalUriParseError::Bip21(_) => None, + } + } +} + +impl std::fmt::Display for UriParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self.0 { + InternalUriParseError::Bip21(e) => write!(f, "Invalid BIP21 URI: {e}"), + InternalUriParseError::PayjoinParams(e) => write!(f, "Invalid payjoin parameters: {e}"), + } + } +} + +impl std::error::Error for UriParseError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self.0 { + InternalUriParseError::Bip21(e) => Some(e), + InternalUriParseError::PayjoinParams(e) => Some(e), + } + } +} + #[derive(Debug)] pub(super) enum InternalPjParseError { BadPjOs, diff --git a/payjoin/src/core/uri/mod.rs b/payjoin/src/core/uri/mod.rs index f120b4baa..0e6ae3a20 100644 --- a/payjoin/src/core/uri/mod.rs +++ b/payjoin/src/core/uri/mod.rs @@ -1,9 +1,12 @@ //! Payjoin URI parsing and validation use std::borrow::Cow; +use std::fmt; +use std::str::FromStr; -use bitcoin::address::NetworkChecked; -pub use error::PjParseError; +use bitcoin::address::{NetworkChecked, NetworkUnchecked, NetworkValidation}; +use bitcoin::{Address, Amount}; +pub use error::{PjParseError, UriParseError}; #[cfg(feature = "v2")] pub(crate) use crate::directory::ShortId; @@ -106,91 +109,217 @@ impl PayjoinExtras { pub fn output_substitution(&self) -> OutputSubstitution { self.output_substitution } } -pub type Uri<'a, NetworkValidation> = bitcoin_uri::Uri<'a, NetworkValidation, MaybePayjoinExtras>; -pub type PjUri<'a> = bitcoin_uri::Uri<'a, NetworkChecked, PayjoinExtras>; - -mod sealed { - use bitcoin::address::NetworkChecked; +/// A BIP21 URI that may or may not request payjoin. +/// +/// This newtype wraps [`bitcoin_uri::Uri`] so that a breaking change in that +/// crate does not force a breaking change in this crate's public API. Parse one +/// with [`Uri::try_from`] or [`str::parse`], validate the address network with +/// [`assume_checked`](Self::assume_checked) or +/// [`require_network`](Self::require_network), then check for payjoin support +/// with [`check_pj_supported`](Self::check_pj_supported). +/// +/// The URI is always owned, so it carries no lifetime parameter. +#[derive(Clone, Debug)] +pub struct Uri( + bitcoin_uri::Uri<'static, NetVal, MaybePayjoinExtrasAdapter>, +); + +impl Uri { + /// The address the URI pays to. + pub fn address(&self) -> &Address { &self.0.address } + + /// The amount the URI requests, if any. + pub fn amount(&self) -> Option { self.0.amount } + + /// The label describing the URI, if present and valid UTF-8. + pub fn label(&self) -> Option { + self.0.label.clone().and_then(|label| String::try_from(label).ok()) + } - pub trait UriExt: Sized {} + /// The message describing the URI, if present and valid UTF-8. + pub fn message(&self) -> Option { + self.0.message.clone().and_then(|message| String::try_from(message).ok()) + } - impl UriExt for super::Uri<'_, NetworkChecked> {} - impl UriExt for super::PjUri<'_> {} + /// The payjoin parameters carried by the URI. + pub fn extras(&self) -> &MaybePayjoinExtras { &self.0.extras.0 } } -pub trait UriExt<'a>: sealed::UriExt { - // Error type is boxed to reduce the size of the Result - // (See https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err) - fn check_pj_supported(self) -> Result, Box>>; +impl Uri { + /// Marks the URI's address as validated without checking the network. + pub fn assume_checked(self) -> Uri { Uri(self.0.assume_checked()) } + + /// Validates that the URI's address is valid for the given network. + pub fn require_network( + self, + network: bitcoin::Network, + ) -> Result, UriParseError> { + self.0.require_network(network).map(Uri).map_err(UriParseError::from_bip21_error) + } } -impl<'a> UriExt<'a> for Uri<'a, NetworkChecked> { - fn check_pj_supported(self) -> Result, Box>> { - match self.extras { +impl Uri { + /// Converts this URI into a [`PjUri`] if it supports payjoin. + /// + /// If payjoin is unsupported the URI is handed back unchanged in the error + /// variant. It is boxed to reduce the size of the `Result` (see + /// ). + pub fn check_pj_supported(self) -> Result> { + match self.0.extras.0 { MaybePayjoinExtras::Supported(payjoin) => { - let mut uri = bitcoin_uri::Uri::with_extras(self.address, payjoin); - uri.amount = self.amount; - uri.label = self.label; - uri.message = self.message; + let mut uri = + bitcoin_uri::Uri::with_extras(self.0.address, PayjoinExtrasAdapter(payjoin)); + uri.amount = self.0.amount; + uri.label = self.0.label; + uri.message = self.0.message; - Ok(uri) + Ok(PjUri(uri)) } MaybePayjoinExtras::Unsupported => { - let mut uri = bitcoin_uri::Uri::new(self.address); - uri.amount = self.amount; - uri.label = self.label; - uri.message = self.message; - - Err(Box::new(uri)) + let mut uri = bitcoin_uri::Uri::with_extras( + self.0.address, + MaybePayjoinExtrasAdapter(MaybePayjoinExtras::Unsupported), + ); + uri.amount = self.0.amount; + uri.label = self.0.label; + uri.message = self.0.message; + + Err(Box::new(Uri(uri))) } } } } -impl bitcoin_uri::de::DeserializationError for MaybePayjoinExtras { +impl FromStr for Uri { + type Err = UriParseError; + + fn from_str(s: &str) -> Result { + let uri: bitcoin_uri::Uri<'static, NetworkUnchecked, MaybePayjoinExtrasAdapter> = + s.parse().map_err(UriParseError::from_bip21_error)?; + Ok(Uri(uri)) + } +} + +impl TryFrom<&str> for Uri { + type Error = UriParseError; + + fn try_from(s: &str) -> Result { s.parse() } +} + +impl TryFrom for Uri { + type Error = UriParseError; + + fn try_from(s: String) -> Result { s.parse() } +} + +impl fmt::Display for Uri { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(f) } +} + +/// A BIP21 URI that is known to request payjoin, with validated payjoin parameters. +/// +/// Obtained from [`Uri::check_pj_supported`]. Like [`Uri`], this newtype +/// insulates the public API from [`bitcoin_uri`] and is always owned. +#[derive(Clone, Debug)] +pub struct PjUri(bitcoin_uri::Uri<'static, NetworkChecked, PayjoinExtrasAdapter>); + +impl PjUri { + /// Builds a payjoin URI from a checked address and validated payjoin parameters. + pub(crate) fn from_extras(address: Address, extras: PayjoinExtras) -> Self { + PjUri(bitcoin_uri::Uri::with_extras(address, PayjoinExtrasAdapter(extras))) + } + + /// The address the URI pays to. + pub fn address(&self) -> &Address { &self.0.address } + + /// The amount the URI requests, if any. + pub fn amount(&self) -> Option { self.0.amount } + + /// Sets the amount the URI requests. + pub fn set_amount(&mut self, amount: Amount) { self.0.amount = Some(amount); } + + /// The label describing the URI, if present and valid UTF-8. + pub fn label(&self) -> Option { + self.0.label.clone().and_then(|label| String::try_from(label).ok()) + } + + /// The message describing the URI, if present and valid UTF-8. + pub fn message(&self) -> Option { + self.0.message.clone().and_then(|message| String::try_from(message).ok()) + } + + /// The validated payjoin parameters carried by the URI. + pub fn extras(&self) -> &PayjoinExtras { &self.0.extras.0 } + + /// Overrides the output substitution preference carried by the URI. + #[cfg(test)] + pub(crate) fn set_output_substitution(&mut self, output_substitution: OutputSubstitution) { + self.0.extras.0.output_substitution = output_substitution; + } +} + +impl fmt::Display for PjUri { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(f) } +} + +/// Private adapter that carries the `bitcoin_uri` parsing and serialization +/// trait impls, keeping them off the public [`MaybePayjoinExtras`] type so that +/// `bitcoin_uri` stays out of this crate's semver surface. +#[derive(Clone, Debug)] +pub(crate) struct MaybePayjoinExtrasAdapter(pub(crate) MaybePayjoinExtras); + +/// Private adapter that carries the `bitcoin_uri` serialization trait impl for +/// [`PayjoinExtras`], keeping it off the public type. +#[derive(Clone, Debug)] +pub(crate) struct PayjoinExtrasAdapter(pub(crate) PayjoinExtras); + +/// Serializes the payjoin BIP21 query parameters (`pj` and optional `pjos`). +fn serialize_payjoin_params(extras: &PayjoinExtras) -> Vec<(&'static str, String)> { + let mut params = Vec::with_capacity(2); + if extras.output_substitution == OutputSubstitution::Disabled { + params.push(("pjos", String::from("0"))); + } + params.push(("pj", extras.pj_param.to_string())); + params +} + +impl bitcoin_uri::de::DeserializationError for MaybePayjoinExtrasAdapter { type Error = PjParseError; } -impl bitcoin_uri::de::DeserializeParams<'_> for MaybePayjoinExtras { +impl bitcoin_uri::de::DeserializeParams<'_> for MaybePayjoinExtrasAdapter { type DeserializationState = DeserializationState; } #[derive(Default)] -pub struct DeserializationState { +pub(crate) struct DeserializationState { pj: Option, pjos: Option, } -impl bitcoin_uri::SerializeParams for &MaybePayjoinExtras { +impl bitcoin_uri::SerializeParams for &MaybePayjoinExtrasAdapter { type Key = &'static str; type Value = String; type Iterator = std::vec::IntoIter<(Self::Key, Self::Value)>; fn serialize_params(self) -> Self::Iterator { - match self { - MaybePayjoinExtras::Supported(extras) => extras.serialize_params(), - MaybePayjoinExtras::Unsupported => vec![].into_iter(), + match &self.0 { + MaybePayjoinExtras::Supported(extras) => serialize_payjoin_params(extras).into_iter(), + MaybePayjoinExtras::Unsupported => Vec::new().into_iter(), } } } -impl bitcoin_uri::SerializeParams for &PayjoinExtras { +impl bitcoin_uri::SerializeParams for &PayjoinExtrasAdapter { type Key = &'static str; type Value = String; type Iterator = std::vec::IntoIter<(Self::Key, Self::Value)>; - fn serialize_params(self) -> Self::Iterator { - let mut params = Vec::with_capacity(2); - if self.output_substitution == OutputSubstitution::Disabled { - params.push(("pjos", String::from("0"))); - } - params.push(("pj", self.pj_param.to_string())); - params.into_iter() - } + fn serialize_params(self) -> Self::Iterator { serialize_payjoin_params(&self.0).into_iter() } } impl bitcoin_uri::de::DeserializationState<'_> for DeserializationState { - type Value = MaybePayjoinExtras; + type Value = MaybePayjoinExtrasAdapter; fn is_param_known(&self, param: &str) -> bool { matches!(param, "pj" | "pjos") } @@ -228,14 +357,15 @@ impl bitcoin_uri::de::DeserializationState<'_> for DeserializationState { self, ) -> std::result::Result::Error> { - match (self.pj, self.pjos) { - (None, None) => Ok(MaybePayjoinExtras::Unsupported), - (None, Some(_)) => Err(InternalPjParseError::MissingEndpoint.into()), - (Some(pj_param), pjos) => Ok(MaybePayjoinExtras::Supported(PayjoinExtras { + let extras = match (self.pj, self.pjos) { + (None, None) => MaybePayjoinExtras::Unsupported, + (None, Some(_)) => return Err(InternalPjParseError::MissingEndpoint.into()), + (Some(pj_param), pjos) => MaybePayjoinExtras::Supported(PayjoinExtras { pj_param, output_substitution: pjos.unwrap_or(OutputSubstitution::Enabled), - })), - } + }), + }; + Ok(MaybePayjoinExtrasAdapter(extras)) } } @@ -243,8 +373,6 @@ impl bitcoin_uri::de::DeserializationState<'_> for DeserializationState { mod tests { use std::convert::TryFrom; - use bitcoin_uri::SerializeParams; - use super::*; #[test] @@ -281,7 +409,7 @@ mod tests { assert!( !Uri::try_from("bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX") .unwrap() - .extras + .extras() .pj_is_supported(), "Uri expected a failure with missing pj extras, but it succeeded" ); @@ -292,13 +420,12 @@ mod tests { use bitcoin_uri::de::DeserializationState as _; let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pjos=1&pj=HTTPS://EXAMPLE.COM/TXJCGKTKXLUUZ%23EX1C4UC6ES-OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC-RK1Q0DJS3VVDXWQQTLQ8022QGXSX7ML9PHZ6EDSF6AKEWQG758JPS2EV"; let pjuri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap(); - let serialized_params = pjuri.extras.serialize_params(); - let pjos_key = serialized_params.clone().next().expect("Missing pjos key").0; - let pj_key = serialized_params.clone().next().expect("Missing pj key").0; + let serialized_params = serialize_payjoin_params(pjuri.extras()); + let pj_key = serialized_params.first().expect("Missing pj key").0; let state = DeserializationState::default(); - assert!(state.is_param_known(pjos_key), "The pjos key should match 'pjos', but it failed"); + assert!(state.is_param_known("pjos"), "The pjos key should match 'pjos', but it failed"); assert!(state.is_param_known(pj_key), "The pj key should match 'pj', but it failed"); assert!( !state.is_param_known("unknown_param"), diff --git a/payjoin/src/core/uri/v1.rs b/payjoin/src/core/uri/v1.rs index dc566b1e7..1677b5904 100644 --- a/payjoin/src/core/uri/v1.rs +++ b/payjoin/src/core/uri/v1.rs @@ -39,7 +39,7 @@ mod tests { use super::*; use crate::uri::MaybePayjoinExtras; - use crate::{OutputSubstitution, PjParam, Uri, UriExt}; + use crate::{OutputSubstitution, PjParam, Uri}; #[test] fn test_missing_amount() { @@ -79,7 +79,7 @@ mod tests { %23OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC" ) .unwrap() - .extras + .extras() .pj_is_supported(), "Uri expected a success with a well formatted pj extras, but it failed" ); @@ -90,8 +90,8 @@ mod tests { let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ &pjos=0&pj=HTTPS://EXAMPLE.COM/missing_short_id\ %23oh1qypm5jxyns754y4r45qwe336qfx6zr8dqgvqculvztv20tfveydmfqc"; - let extras = Uri::try_from(uri).unwrap().extras; - match extras { + let uri = Uri::try_from(uri).unwrap(); + match uri.extras() { crate::uri::MaybePayjoinExtras::Supported(extras) => { assert!(matches!(extras.pj_param, crate::uri::PjParam::V1(_))); } @@ -105,23 +105,19 @@ mod tests { let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pjos=1&pjos=1&pj=HTTPS://EXAMPLE.COM/\ %23OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC"; - let pjuri = Uri::try_from(uri); + let err = Uri::try_from(uri).expect_err("duplicate pjos param should fail to parse"); assert!(matches!( - pjuri, - Err(bitcoin_uri::de::Error::Extras(PjParseError( - InternalPjParseError::DuplicateParams("pjos") - ))) + err.payjoin_params(), + Some(PjParseError(InternalPjParseError::DuplicateParams("pjos"))) )); let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pjos=1&pj=HTTPS://EXAMPLE.COM/\ %23OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC&pj=HTTPS://EXAMPLE.COM/\ %23OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC"; - let pjuri = Uri::try_from(uri); + let err = Uri::try_from(uri).expect_err("duplicate pj param should fail to parse"); assert!(matches!( - pjuri, - Err(bitcoin_uri::de::Error::Extras(PjParseError( - InternalPjParseError::DuplicateParams("pj") - ))) + err.payjoin_params(), + Some(PjParseError(InternalPjParseError::DuplicateParams("pj"))) )); } @@ -136,13 +132,13 @@ mod tests { .check_pj_supported() .expect("Could not parse pj extras"); - pjuri.extras.output_substitution = OutputSubstitution::Disabled; + pjuri.set_output_substitution(OutputSubstitution::Disabled); assert!( pjuri.to_string().contains(expected_is_disabled), "Pj uri should contain param: {expected_is_disabled}, but it did not" ); - pjuri.extras.output_substitution = OutputSubstitution::Enabled; + pjuri.set_output_substitution(OutputSubstitution::Enabled); assert!( !pjuri.to_string().contains(expected_is_enabled), "Pj uri should elide param: {expected_is_enabled}, but it did not" @@ -154,7 +150,7 @@ mod tests { // pjos=0 should disable output substitution let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pj=https://example.com&pjos=0"; let parsed = Uri::try_from(uri).unwrap(); - match parsed.extras { + match parsed.extras() { MaybePayjoinExtras::Supported(extras) => assert_eq!(extras.output_substitution, OutputSubstitution::Disabled), _ => panic!("Expected Supported PayjoinExtras"), @@ -163,7 +159,7 @@ mod tests { // pjos=1 should allow output substitution let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pj=https://example.com&pjos=1"; let parsed = Uri::try_from(uri).unwrap(); - match parsed.extras { + match parsed.extras() { MaybePayjoinExtras::Supported(extras) => assert_eq!(extras.output_substitution, OutputSubstitution::Enabled), _ => panic!("Expected Supported PayjoinExtras"), @@ -172,7 +168,7 @@ mod tests { // Elided pjos=1 should allow output substitution let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pj=https://example.com"; let parsed = Uri::try_from(uri).unwrap(); - match parsed.extras { + match parsed.extras() { MaybePayjoinExtras::Supported(extras) => assert_eq!(extras.output_substitution, OutputSubstitution::Enabled), _ => panic!("Expected Supported PayjoinExtras"), diff --git a/payjoin/src/core/uri/v2.rs b/payjoin/src/core/uri/v2.rs index 9b9da5a5f..052cc59a8 100644 --- a/payjoin/src/core/uri/v2.rs +++ b/payjoin/src/core/uri/v2.rs @@ -394,7 +394,7 @@ mod tests { use payjoin_test_utils::{BoxError, EXAMPLE_URL}; use super::*; - use crate::{Uri, UriExt}; + use crate::Uri; #[test] fn test_ohttp_get_set() { @@ -548,7 +548,9 @@ mod tests { &pjos=0&pj=HTTPS://EXAMPLE.COM/TXJCGKTKXLUUZ\ %23EX1C4UC6ES-OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC-RK1Q0DJS3VVDXWQQTLQ8022QGXSX7ML9PHZ6EDSF6AKEWQG758JPS2EV"; let pjuri = Uri::try_from(uri).unwrap().assume_checked().check_pj_supported().unwrap(); - assert!(ohttp(&Url::parse(&pjuri.extras.endpoint()).expect("Could not parse url")).is_ok()); + assert!( + ohttp(&Url::parse(&pjuri.extras().endpoint()).expect("Could not parse url")).is_ok() + ); assert_eq!(format!("{pjuri}"), uri); let reordered = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ @@ -557,7 +559,9 @@ mod tests { &pjos=0"; let pjuri = Uri::try_from(reordered).unwrap().assume_checked().check_pj_supported().unwrap(); - assert!(ohttp(&Url::parse(&pjuri.extras.endpoint()).expect("Could not parse url")).is_ok()); + assert!( + ohttp(&Url::parse(&pjuri.extras().endpoint()).expect("Could not parse url")).is_ok() + ); assert_eq!(format!("{pjuri}"), uri); } @@ -566,20 +570,22 @@ mod tests { let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ &pjos=0&pj=HTTPS://EXAMPLE.COM/TXJCGKTKXLUUZ\ %23ex1c4uc6es-oh1qypm5jxyns754y4r45qwe336qfx6zr8dqgvqculvztv20tfveydmfqc-rk1q0djs3vvdxwqqtlq8022qgxsx7ml9phz6edsf6akewqg758jps2ev"; + let err = Uri::try_from(uri).expect_err("lowercase fragment should fail to parse"); assert!(matches!( - Uri::try_from(uri), - Err(bitcoin_uri::de::Error::Extras(crate::uri::PjParseError( - crate::uri::InternalPjParseError::V2(PjParseError::LowercaseFragment) + err.payjoin_params(), + Some(crate::uri::PjParseError(crate::uri::InternalPjParseError::V2( + PjParseError::LowercaseFragment ))) )); let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ &pjos=0&pj=HTTPS://EXAMPLE.COM/TXJCGKTKXLUUZ\ %23EX1C4UC6ES-OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC-RK1Q0DJS3VVDXWQQTLQ8022QGXSX7ML9PHZ6EDSF6AKEWQG758JPS2Ev"; + let err = Uri::try_from(uri).expect_err("trailing lowercase fragment should fail to parse"); assert!(matches!( - Uri::try_from(uri), - Err(bitcoin_uri::de::Error::Extras(crate::uri::PjParseError( - crate::uri::InternalPjParseError::V2(PjParseError::LowercaseFragment) + err.payjoin_params(), + Some(crate::uri::PjParseError(crate::uri::InternalPjParseError::V2( + PjParseError::LowercaseFragment ))) )); Ok(()) diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 18f5b1447..a6e1e26ae 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -36,7 +36,6 @@ mod integration { #[cfg(feature = "v1")] mod v1 { use payjoin::send::v1::SenderBuilder; - use payjoin::UriExt; use tracing::debug; use super::*; @@ -111,7 +110,7 @@ mod integration { let pj_receiver_address = receiver.new_address()?; let mut pj_uri = build_v1_pj_uri(&pj_receiver_address, EXAMPLE_URL, OutputSubstitution::Enabled)?; - pj_uri.amount = Some(Amount::ONE_BTC); + pj_uri.set_amount(Amount::ONE_BTC); // ********************** // Inside the Sender: @@ -170,7 +169,7 @@ mod integration { let pj_receiver_address = receiver.new_address()?; let mut pj_uri = build_v1_pj_uri(&pj_receiver_address, EXAMPLE_URL, OutputSubstitution::Enabled)?; - pj_uri.amount = Some(Amount::ONE_BTC); + pj_uri.set_amount(Amount::ONE_BTC); // ********************** // Inside the Sender: @@ -211,7 +210,7 @@ mod integration { }; use payjoin::send::v2::{replay_event_log as replay_sender_event_log, SenderBuilder}; use payjoin::send::ResponseError; - use payjoin::{OhttpKeys, PjUri, UriExt}; + use payjoin::{OhttpKeys, PjUri}; use payjoin_test_utils::{ BoxSendSyncError, InMemoryPersister, SessionPersister, TestServices, }; @@ -930,7 +929,7 @@ mod integration { let pj_receiver_address = receiver.new_address()?; let mut pj_uri = build_v1_pj_uri(&pj_receiver_address, EXAMPLE_URL, OutputSubstitution::Enabled)?; - pj_uri.amount = Some(Amount::ONE_BTC); + pj_uri.set_amount(Amount::ONE_BTC); // ********************** // Inside the Sender: @@ -943,7 +942,7 @@ mod integration { // FIXME this test no longer sends v2 to v1 because that concept is gone and should now be // Handled by the implementation. Therefore, the e2e test should now test v2-capable sender // successfully sending to v1. - assert!(matches!(pj_uri.extras.pj_param(), payjoin::PjParam::V1(_))); + assert!(matches!(pj_uri.extras().pj_param(), payjoin::PjParam::V1(_))); let psbt = build_original_psbt(&sender, &pj_uri)?; let req_ctx = payjoin::send::v1::SenderBuilder::new(psbt, pj_uri) .build_recommended(FeeRate::BROADCAST_MIN)?; @@ -1242,7 +1241,7 @@ mod integration { pj_uri: &PjUri, ) -> Result { let mut outputs = HashMap::with_capacity(1); - outputs.insert(pj_uri.address.to_string(), Amount::from_btc(50.0)?.to_btc()); + outputs.insert(pj_uri.address().to_string(), Amount::from_btc(50.0)?.to_btc()); let options = serde_json::json!({ "lockUnspents": true, // The minimum relay feerate ensures that tests fail if the receiver would add inputs/outputs @@ -1271,7 +1270,6 @@ mod integration { #[cfg(feature = "v1")] mod batching { use payjoin::send::v1::SenderBuilder; - use payjoin::UriExt; use super::*; @@ -1295,7 +1293,7 @@ mod integration { let pj_receiver_address = receiver.new_address()?; let mut pj_uri = build_v1_pj_uri(&pj_receiver_address, EXAMPLE_URL, OutputSubstitution::Enabled)?; - pj_uri.amount = Some(Amount::ONE_BTC); + pj_uri.set_amount(Amount::ONE_BTC); // ********************** // Inside the Sender: @@ -1375,7 +1373,7 @@ mod integration { let pj_receiver_address = receiver.new_address()?; let mut pj_uri = build_v1_pj_uri(&pj_receiver_address, EXAMPLE_URL, OutputSubstitution::Enabled)?; - pj_uri.amount = Some(Amount::ONE_BTC); + pj_uri.set_amount(Amount::ONE_BTC); // ********************** // Inside the Sender: @@ -1458,8 +1456,10 @@ mod integration { fn build_original_psbt(sender: &corepc_node::Client, pj_uri: &PjUri) -> Result { let mut outputs = HashMap::with_capacity(1); - outputs - .insert(pj_uri.address.to_string(), pj_uri.amount.unwrap_or(Amount::ONE_BTC).to_btc()); + outputs.insert( + pj_uri.address().to_string(), + pj_uri.amount().unwrap_or(Amount::ONE_BTC).to_btc(), + ); let options = json!({ "lockUnspents": true, // The minimum relay feerate ensures that tests fail if the receiver would add inputs/outputs diff --git a/payjoin/tests/uri_api.rs b/payjoin/tests/uri_api.rs new file mode 100644 index 000000000..30daf28c5 --- /dev/null +++ b/payjoin/tests/uri_api.rs @@ -0,0 +1,103 @@ +//! Assertions on payjoin's public BIP 21 URI surface. +//! +//! Every type named in these tests is payjoin's own. No `bitcoin_uri` type may appear in a +//! signature a downstream crate can observe, so that the BIP 21 parser can be upgraded or +//! swapped without forcing a payjoin major release. This is the downstream-visible +//! counterpart to the in-crate unit tests. + +use std::error::Error; + +use payjoin::bitcoin::address::{NetworkChecked, NetworkUnchecked}; +use payjoin::bitcoin::{Amount, Network}; +use payjoin::{PjParseError, Uri, UriParseError}; + +const NO_PJ: &str = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=1"; + +#[test] +fn parse_error_is_a_payjoin_type() { + // The type annotation is the assertion: a malformed BIP 21 URI fails with payjoin's own + // error, not with the parser's. The underlying cause stays reachable via the source chain. + let err: UriParseError = Uri::try_from("bitcoin:this is not a valid uri &&&").unwrap_err(); + assert!(!err.to_string().is_empty()); + assert!(err.source().is_some(), "the underlying cause should stay reachable"); +} + +#[test] +fn payjoin_param_error_is_recoverable_from_the_source_chain() { + // A well-formed BIP 21 URI with a bad `pjos` value fails in payjoin's own extras parser. + // Downstream recovers the concrete payjoin error by downcasting the source, without + // naming or matching on any foreign type. + let err: UriParseError = + Uri::try_from("bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?pjos=2").unwrap_err(); + let source = err.source().expect("payjoin parameter errors have a source"); + assert!( + source.downcast_ref::().is_some(), + "expected a payjoin::PjParseError, got: {source}" + ); +} + +#[test] +fn accessors_replace_public_fields() { + let uri: Uri = NO_PJ.parse().expect("valid BIP 21 uri"); + let uri: Uri = uri.assume_checked(); + + assert_eq!(uri.address().to_string(), "12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX"); + assert_eq!(uri.amount(), Some(Amount::ONE_BTC)); + assert_eq!(uri.label(), None); + assert_eq!(uri.message(), None); + assert!(!uri.extras().pj_is_supported()); +} + +#[test] +fn label_and_message_decode_to_strings() { + let uri = Uri::try_from( + "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?label=Luke-Jr&message=Donation%20for%20xyz", + ) + .expect("valid BIP 21 uri") + .assume_checked(); + + assert_eq!(uri.label().as_deref(), Some("Luke-Jr")); + assert_eq!(uri.message().as_deref(), Some("Donation for xyz")); +} + +#[test] +fn payjoin_uri_exposes_bip21_fields() { + // A payjoin URI carries the same BIP 21 fields as a plain one. The accessors read them + // back through payjoin's own types, and `set_amount` overrides the requested amount. + let mut pjuri = Uri::try_from( + "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=1&label=Luke-Jr&message=Donation%20for%20xyz&pjos=1&pj=HTTPS://EXAMPLE.COM/TXJCGKTKXLUUZ%23EX1C4UC6ES-OH1QYPM5JXYNS754Y4R45QWE336QFX6ZR8DQGVQCULVZTV20TFVEYDMFQC-RK1Q0DJS3VVDXWQQTLQ8022QGXSX7ML9PHZ6EDSF6AKEWQG758JPS2EV", + ) + .expect("valid payjoin uri") + .assume_checked() + .check_pj_supported() + .expect("this uri requests payjoin"); + + assert_eq!(pjuri.address().to_string(), "12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX"); + assert_eq!(pjuri.amount(), Some(Amount::ONE_BTC)); + assert_eq!(pjuri.label().as_deref(), Some("Luke-Jr")); + assert_eq!(pjuri.message().as_deref(), Some("Donation for xyz")); + + pjuri.set_amount(Amount::from_sat(10_000)); + assert_eq!(pjuri.amount(), Some(Amount::from_sat(10_000))); +} + +#[test] +fn require_network_reports_a_payjoin_error() { + let err: UriParseError = Uri::try_from(NO_PJ) + .expect("valid BIP 21 uri") + .require_network(Network::Testnet) + .expect_err("mainnet address must not satisfy testnet"); + assert!(!err.to_string().is_empty()); +} + +#[test] +fn check_pj_supported_hands_back_a_payjoin_uri() { + // The unsupported branch returns payjoin's own URI, boxed, so the caller keeps a usable + // value without ever naming a foreign type. It round-trips back to the input. + let returned: Box> = Uri::try_from(NO_PJ) + .expect("valid BIP 21 uri") + .assume_checked() + .check_pj_supported() + .expect_err("this uri has no pj parameter"); + assert_eq!(returned.to_string(), NO_PJ); +} From 35a107c81c9a907b8f3825982808b5036f8e221f Mon Sep 17 00:00:00 2001 From: spacebear Date: Wed, 22 Jul 2026 02:09:56 +0000 Subject: [PATCH 2/2] Simplify FFI UriParseError wrapper The FFI `PjParseError` flattened the parse error into a `String` because `payjoin::Uri`'s parse error used to be the foreign `bitcoin_uri::de::Error`, whose type could not be carried across the FFI boundary. Now that parsing returns payjoin's own `UriParseError`, which implements `std::error::Error`, it can be wrapped directly with `#[error(transparent)]` and `#[from]`. As a result it is also renamed to `UriParseError` to match the core library naming convention. `Eq` cannot be derived anymore because the `Bip21` variant of `payjoin::UriParseError` holds a `bitcoin_uri::de::UriError`, which doesn't implement `Eq`. --- payjoin-ffi/src/uri/error.rs | 14 ++++---------- payjoin-ffi/src/uri/mod.rs | 9 ++++----- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/payjoin-ffi/src/uri/error.rs b/payjoin-ffi/src/uri/error.rs index e5e437cd9..19af6378c 100644 --- a/payjoin-ffi/src/uri/error.rs +++ b/payjoin-ffi/src/uri/error.rs @@ -1,13 +1,7 @@ -#[derive(Debug, PartialEq, Eq, thiserror::Error, uniffi::Object)] -#[uniffi::export(Debug, Display, Eq)] -#[error("Error parsing the payjoin URI: {msg}")] -pub struct PjParseError { - msg: String, -} - -impl PjParseError { - pub(crate) fn from_err(err: impl std::fmt::Display) -> Self { Self { msg: err.to_string() } } -} +#[derive(Debug, thiserror::Error, uniffi::Object)] +#[uniffi::export(Debug, Display)] +#[error(transparent)] +pub struct UriParseError(#[from] payjoin::UriParseError); #[derive(Debug, PartialEq, Eq, thiserror::Error, uniffi::Object)] #[uniffi::export(Debug, Display, Eq)] diff --git a/payjoin-ffi/src/uri/mod.rs b/payjoin-ffi/src/uri/mod.rs index c6a36f8de..0e6d8c439 100644 --- a/payjoin-ffi/src/uri/mod.rs +++ b/payjoin-ffi/src/uri/mod.rs @@ -1,7 +1,7 @@ use std::str::FromStr; use std::sync::Arc; -pub use error::{PjNotSupported, PjParseError, UrlParseError}; +pub use error::{PjNotSupported, UriParseError, UrlParseError}; use payjoin::bitcoin::address::NetworkChecked; use crate::error::FfiValidationError; @@ -21,10 +21,9 @@ impl From> for Uri { #[uniffi::export] impl Uri { #[uniffi::constructor] - pub fn parse(uri: String) -> Result { - payjoin::Uri::from_str(uri.as_str()) - .map(|e| e.assume_checked().into()) - .map_err(PjParseError::from_err) + pub fn parse(uri: String) -> Result { + let uri = payjoin::Uri::from_str(uri.as_str())?; + Ok(uri.assume_checked().into()) } pub fn address(&self) -> String { self.0.address().to_string() } /// Gets the amount in satoshis.