diff --git a/payjoin-ffi/javascript/test/integration.test.ts b/payjoin-ffi/javascript/test/integration.test.ts index fb607b83f..277d4c477 100644 --- a/payjoin-ffi/javascript/test/integration.test.ts +++ b/payjoin-ffi/javascript/test/integration.test.ts @@ -504,18 +504,25 @@ function testFfiValidation(payjoin: PayjoinModule): void { assert.strictEqual(inner.tag, "WeightOutOfRange"); } - const pjUri = payjoin.Uri.parse( + const v1PjUri = payjoin.Uri.parse( "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=1&pj=https://example.com", ).checkPjSupported(); + const v2PjUri = payjoin.Uri.parse( + "bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?pjos=0&pj=HTTPS://PAYJO.IN/TXJCGKTKXLUUZ%23EX1WKV8CEC-OH1QYPM59NK2LXXS4890SUAXXYT25Z2VAPHP0X7YEYCJXGWAG6UG9ZU6NQ-RK1Q0DJS3VVDXWQQTLQ8022QGXSX7ML9PHZ6EDSF6AKEWQG758JPS2EV", + ).checkPjSupported(); const psbt = testUtils.originalPsbt(); assert.throws(() => { - new payjoin.SenderBuilder(psbt, pjUri).buildRecommended( + new payjoin.SenderBuilder(psbt, v2PjUri).buildRecommended( 18446744073709551615n, ); }, /RuntimeError/); assert.throws(() => { - pjUri.setAmountSats(tooLargeAmount); + new payjoin.SenderBuilder(psbt, v1PjUri); + }, /SenderInputError\.Build/); + + assert.throws(() => { + v1PjUri.setAmountSats(tooLargeAmount); }, /AmountOutOfRange/); } diff --git a/payjoin-ffi/src/send/mod.rs b/payjoin-ffi/src/send/mod.rs index bccbead8b..a3a949629 100644 --- a/payjoin-ffi/src/send/mod.rs +++ b/payjoin-ffi/src/send/mod.rs @@ -356,7 +356,8 @@ impl SenderBuilder { let psbt = payjoin::bitcoin::psbt::Psbt::from_str(psbt.as_str()) .map_err(PsbtParseError::from) .map_err(SenderInputError::Psbt)?; - let builder = payjoin::send::v2::SenderBuilder::new(psbt, Arc::unwrap_or_clone(uri).into()); + let builder = payjoin::send::v2::SenderBuilder::new(psbt, Arc::unwrap_or_clone(uri).into()) + .map_err(|e| SenderInputError::Build(Arc::new(e.into())))?; Ok(builder.into()) } diff --git a/payjoin/src/core/send/error.rs b/payjoin/src/core/send/error.rs index 7143cc9cb..319160f71 100644 --- a/payjoin/src/core/send/error.rs +++ b/payjoin/src/core/send/error.rs @@ -18,6 +18,8 @@ pub(crate) enum InternalBuildSenderError { InvalidOriginalInput(crate::psbt::PsbtInputsError), InconsistentOriginalPsbt(crate::psbt::InconsistentPsbt), OriginalTxinNonAllSighashType, + #[cfg(all(feature = "v1", feature = "v2"))] + UnsupportedVersion, NoInputs, PayeeValueNotEqual, NoOutputs, @@ -49,6 +51,8 @@ impl fmt::Display for BuildSenderError { InvalidOriginalInput(e) => write!(f, "an input in the original transaction is invalid: {e:#?}"), InconsistentOriginalPsbt(e) => write!(f, "the original transaction is inconsistent: {e:#?}"), OriginalTxinNonAllSighashType => write!(f, "an input in the original transaction requests a sighash type other than SIGHASH_ALL"), + #[cfg(all(feature = "v1", feature = "v2"))] + UnsupportedVersion => write!(f, "v2 sender does not support v1 payjoin URIs"), NoInputs => write!(f, "the original transaction has no inputs"), PayeeValueNotEqual => write!(f, "the value in original transaction doesn't equal value requested in the payment link"), NoOutputs => write!(f, "the original transaction has no outputs"), @@ -72,6 +76,8 @@ impl std::error::Error for BuildSenderError { InvalidOriginalInput(error) => Some(error), InconsistentOriginalPsbt(error) => Some(error), OriginalTxinNonAllSighashType => None, + #[cfg(all(feature = "v1", feature = "v2"))] + UnsupportedVersion => None, NoInputs => None, PayeeValueNotEqual => None, NoOutputs => None, diff --git a/payjoin/src/core/send/v2/mod.rs b/payjoin/src/core/send/v2/mod.rs index 982565a25..f33691c3f 100644 --- a/payjoin/src/core/send/v2/mod.rs +++ b/payjoin/src/core/send/v2/mod.rs @@ -71,12 +71,12 @@ impl SenderBuilder { /// /// Call [`SenderBuilder::build_recommended()`] or other `build` methods /// to create a [`Sender`] - pub fn new(psbt: Psbt, uri: PjUri) -> Self { + pub fn new(psbt: Psbt, uri: PjUri) -> Result { match uri.extras().pj_param() { #[cfg(feature = "v1")] - crate::uri::PjParam::V1(_) => unimplemented!("V2 SenderBuilder only supports v2 URLs"), + crate::uri::PjParam::V1(_) => Err(InternalBuildSenderError::UnsupportedVersion.into()), crate::uri::PjParam::V2(pj_param) => - Self::from_parts(psbt, pj_param, uri.address(), uri.amount()), + Ok(Self::from_parts(psbt, pj_param, uri.address(), uri.amount())), } } @@ -721,6 +721,24 @@ mod test { Ok(()) } + #[cfg(feature = "v1")] + #[test] + fn v1_uri_returns_error_instead_of_panicking() { + const V1_PJ_URI: &str = + "bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?amount=0.02&pjos=0&pj=HTTPS://EXAMPLE.COM/"; + let uri = crate::Uri::try_from(V1_PJ_URI) + .expect("valid URI") + .assume_checked() + .check_pj_supported() + .expect("payjoin should be supported"); + + let err = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), uri) + .err() + .expect("v1 URI should be rejected"); + + assert_eq!(err.to_string(), "v2 sender does not support v1 payjoin URIs"); + } + #[test] fn test_v2_sender_builder() { let address = Address::from_str("2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7") @@ -736,6 +754,7 @@ mod test { .expect("receiver should succeed") .pj_uri(); let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri.clone()) + .expect("v2 URI should be supported") .build_recommended(FeeRate::BROADCAST_MIN) .expect("build on test vector should succeed") .save(&InMemoryPersister::default()) @@ -757,6 +776,7 @@ mod test { assert_eq!(req_ctx.session_context.psbt_ctx.min_fee_rate, FeeRate::from_sat_per_kwu(250)); // ensure that the other builder methods also enable output substitution let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri.clone()) + .expect("v2 URI should be supported") .build_non_incentivizing(FeeRate::BROADCAST_MIN) .expect("build on test vector should succeed") .save(&InMemoryPersister::default()) @@ -766,6 +786,7 @@ mod test { OutputSubstitution::Enabled ); let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri.clone()) + .expect("v2 URI should be supported") .build_with_additional_fee(Amount::ZERO, Some(0), FeeRate::BROADCAST_MIN, false) .expect("build on test vector should succeed") .save(&InMemoryPersister::default()) @@ -776,6 +797,7 @@ mod test { ); // ensure that a v2 sender may still disable output substitution if they prefer. let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri) + .expect("v2 URI should be supported") .always_disable_output_substitution() .build_recommended(FeeRate::BROADCAST_MIN) .expect("build on test vector should succeed") diff --git a/payjoin/src/core/send/v2/session.rs b/payjoin/src/core/send/v2/session.rs index 78126bced..289189f98 100644 --- a/payjoin/src/core/send/v2/session.rs +++ b/payjoin/src/core/send/v2/session.rs @@ -320,6 +320,7 @@ mod tests { .check_pj_supported() .expect("Payjoin to be supported"), ) + .expect("v2 URI should be supported") .build_recommended(FeeRate::BROADCAST_MIN) .unwrap() .save(&InMemoryPersister::default()) @@ -354,6 +355,7 @@ mod tests { .check_pj_supported() .expect("Payjoin to be supported"), ) + .expect("v2 URI should be supported") .build_recommended(FeeRate::BROADCAST_MIN) .unwrap() .save(&InMemoryPersister::default()) @@ -390,6 +392,7 @@ mod tests { .check_pj_supported() .expect("Payjoin to be supported"), ) + .expect("v2 URI should be supported") .build_recommended(FeeRate::BROADCAST_MIN) .unwrap() .save(&InMemoryPersister::default()) @@ -437,6 +440,7 @@ mod tests { .check_pj_supported() .expect("Payjoin to be supported"), ) + .expect("v2 URI should be supported") .build_recommended(FeeRate::BROADCAST_MIN) .unwrap() .save(&InMemoryPersister::default()) @@ -479,6 +483,7 @@ mod tests { .check_pj_supported() .expect("Payjoin to be supported"), ) + .expect("v2 URI should be supported") .build_recommended(FeeRate::BROADCAST_MIN) .unwrap() .save(&InMemoryPersister::default()) diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 39f361cbd..e79beaf5d 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -317,7 +317,7 @@ mod integration { // Inside the Sender: let psbt = build_original_psbt(&sender, &expired_receiver.pj_uri())?; // Test that an expired pj_url errors - let expired_req_ctx = SenderBuilder::new(psbt, expired_receiver.pj_uri()) + let expired_req_ctx = SenderBuilder::new(psbt, expired_receiver.pj_uri())? .build_non_incentivizing(FeeRate::BROADCAST_MIN)? .save(&send_persister)?; @@ -390,7 +390,7 @@ mod integration { .check_pj_supported() .map_err(|e| e.to_string())?; let psbt = build_sweep_psbt(&sender, &pj_uri)?; - let req_ctx = SenderBuilder::new(psbt, pj_uri) + let req_ctx = SenderBuilder::new(psbt, pj_uri)? .build_recommended(FeeRate::BROADCAST_MIN)? .save(&sender_persister)?; let (Request { url, body, content_type, .. }, send_ctx) = @@ -841,7 +841,7 @@ mod integration { .check_pj_supported() .map_err(|e| e.to_string())?; let psbt = build_sweep_psbt(sender, &pj_uri)?; - let req_ctx = SenderBuilder::new(psbt, pj_uri) + let req_ctx = SenderBuilder::new(psbt, pj_uri)? .build_recommended(FeeRate::BROADCAST_MIN)? .save(send_persister)?; let (Request { url, body, content_type, .. }, send_ctx) =