diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index dad80f701..e11ce4f36 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -389,7 +389,7 @@ impl Receiver { /// Build a V2 Payjoin URI from the receiver's context pub fn pj_uri<'a>(&self) -> crate::PjUri<'a> { - pj_uri(&self.context, OutputSubstitution::Enabled) + pj_uri(&self.context, OutputSubstitution::Disabled) } pub(crate) fn apply_unchecked_from_payload( @@ -1010,6 +1010,6 @@ pub mod test { fn test_v2_pj_uri() { let uri = Receiver { state: Initialized { context: SHARED_CONTEXT.clone() } }.pj_uri(); assert_ne!(uri.extras.endpoint, EXAMPLE_URL.clone()); - assert_eq!(uri.extras.output_substitution, OutputSubstitution::Enabled); + assert_eq!(uri.extras.output_substitution, OutputSubstitution::Disabled); } } diff --git a/payjoin/src/core/send/v1.rs b/payjoin/src/core/send/v1.rs index db62ede29..73077014e 100644 --- a/payjoin/src/core/send/v1.rs +++ b/payjoin/src/core/send/v1.rs @@ -305,6 +305,14 @@ mod test { const PJ_URI: &str = "bitcoin:2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7?amount=0.02&pjos=0&pj=HTTPS://EXAMPLE.COM/"; + fn pj_uri<'a>() -> PjUri<'a> { + Uri::try_from(PJ_URI) + .expect("uri should succeed") + .assume_checked() + .check_pj_supported() + .expect("uri should support payjoin") + } + fn create_v1_context() -> super::V1Context { let psbt_context = create_psbt_context().expect("failed to create context"); super::V1Context { psbt_context } @@ -381,40 +389,39 @@ mod test { } #[test] - fn test_build_recommended_fee_contribution() -> Result<(), BoxError> { + fn test_build_recommended_max_fee_contribution() { let psbt = PARSED_ORIGINAL_PSBT.clone(); - let sender = SenderBuilder::new( - psbt.clone(), - Uri::try_from(PJ_URI) - .map_err(|e| format!("{e}"))? - .assume_checked() - .check_pj_supported() - .map_err(|e| format!("{e}"))?, - ) - .build_recommended(FeeRate::from_sat_per_vb(2000000).expect("Could not determine feerate")); - assert!(sender.is_ok(), "{:#?}", sender.err()); - assert_eq!( - sender.unwrap().fee_contribution.unwrap().max_amount, - psbt.unsigned_tx.output[0].value - ); - Ok(()) + let sender = SenderBuilder::new(psbt.clone(), pj_uri()) + .build_recommended( + FeeRate::from_sat_per_vb(2000000).expect("Could not determine feerate"), + ) + .expect("sender should succeed"); + assert_eq!(sender.output_substitution, OutputSubstitution::Disabled); + assert_eq!(&sender.payee, &pj_uri().address.script_pubkey()); + let fee_contribution = sender.fee_contribution.expect("sender should contribute fees"); + assert_eq!(fee_contribution.max_amount, psbt.unsigned_tx.output[0].value); + assert_eq!(fee_contribution.vout, 0); + assert_eq!(sender.min_fee_rate, FeeRate::from_sat_per_kwu(500000000)); } #[test] - fn test_build_recommended() -> Result<(), BoxError> { - let sender = SenderBuilder::new( - PARSED_ORIGINAL_PSBT.clone(), - Uri::try_from(PJ_URI) - .map_err(|e| format!("{e}"))? - .assume_checked() - .check_pj_supported() - .map_err(|e| format!("{e}"))?, - ) - .build_recommended(FeeRate::MIN); - assert!(sender.is_ok(), "{:#?}", sender.err()); - assert_eq!(NON_WITNESS_INPUT_WEIGHT, bitcoin::Weight::from_wu(160)); - assert_eq!(sender.unwrap().fee_contribution.unwrap().max_amount, Amount::from_sat(0)); - Ok(()) + fn test_build_recommended() { + let sender = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri()) + .build_recommended(FeeRate::BROADCAST_MIN) + .expect("sender should succeed"); + assert_eq!(sender.output_substitution, OutputSubstitution::Disabled); + assert_eq!(&sender.payee, &pj_uri().address.script_pubkey()); + let fee_contribution = sender.fee_contribution.expect("sender should contribute fees"); + assert_eq!(fee_contribution.max_amount, Amount::from_sat(91)); + assert_eq!(fee_contribution.vout, 0); + assert_eq!(sender.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; + let sender = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri) + .build_recommended(FeeRate::from_sat_per_vb_unchecked(1)) + .expect("sender should succeed"); + assert_eq!(sender.output_substitution, OutputSubstitution::Enabled); } #[test] diff --git a/payjoin/src/core/send/v2/mod.rs b/payjoin/src/core/send/v2/mod.rs index 06d33eba5..4b5ca1be5 100644 --- a/payjoin/src/core/send/v2/mod.rs +++ b/payjoin/src/core/send/v2/mod.rs @@ -75,15 +75,7 @@ impl<'a> SenderBuilder<'a> { self, min_fee_rate: FeeRate, ) -> MaybeBadInitInputsTransition, BuildSenderError> { - let v1 = match self.0.build_recommended(min_fee_rate) { - Ok(inner) => inner, - Err(e) => return MaybeBadInitInputsTransition::bad_init_inputs(e), - }; - let with_reply_key = WithReplyKey { v1, reply_key: HpkeKeyPair::gen_keypair().0 }; - MaybeBadInitInputsTransition::success( - SessionEvent::CreatedReplyKey(with_reply_key.clone()), - Sender { state: with_reply_key }, - ) + self.v2_sender_from_v1(self.0.clone().build_recommended(min_fee_rate)) } /// Offer the receiver contribution to pay for his input. @@ -106,21 +98,12 @@ impl<'a> SenderBuilder<'a> { min_fee_rate: FeeRate, clamp_fee_contribution: bool, ) -> MaybeBadInitInputsTransition, BuildSenderError> { - let v1 = match self.0.build_with_additional_fee( + self.v2_sender_from_v1(self.0.clone().build_with_additional_fee( max_fee_contribution, change_index, min_fee_rate, clamp_fee_contribution, - ) { - Ok(inner) => inner, - Err(e) => return MaybeBadInitInputsTransition::bad_init_inputs(e), - }; - - let with_reply_key = WithReplyKey { v1, reply_key: HpkeKeyPair::gen_keypair().0 }; - MaybeBadInitInputsTransition::success( - SessionEvent::CreatedReplyKey(with_reply_key.clone()), - Sender { state: with_reply_key }, - ) + )) } /// Perform Payjoin without incentivizing the payee to cooperate. @@ -131,11 +114,26 @@ impl<'a> SenderBuilder<'a> { self, min_fee_rate: FeeRate, ) -> MaybeBadInitInputsTransition, BuildSenderError> { - let v1 = match self.0.build_non_incentivizing(min_fee_rate) { + self.v2_sender_from_v1(self.0.clone().build_non_incentivizing(min_fee_rate)) + } + + /// Helper function that takes a V1 sender build result and wraps it in a V2 Sender, + /// returning the appropriate state transition. + fn v2_sender_from_v1( + &self, + v1_result: Result, + ) -> MaybeBadInitInputsTransition, BuildSenderError> { + let mut v1 = match v1_result { Ok(inner) => inner, Err(e) => return MaybeBadInitInputsTransition::bad_init_inputs(e), }; + // V2 senders may always ignore the receiver's `pjos` output substitution preference, + // because all communications with the receiver are end-to-end authenticated. + if self.0.output_substitution == OutputSubstitution::Enabled { + v1.output_substitution = OutputSubstitution::Enabled; + } + let with_reply_key = WithReplyKey { v1, reply_key: HpkeKeyPair::gen_keypair().0 }; MaybeBadInitInputsTransition::success( SessionEvent::CreatedReplyKey(with_reply_key.clone()), @@ -495,9 +493,12 @@ mod test { use std::time::{Duration, SystemTime}; use bitcoin::hex::FromHex; + use bitcoin::Address; use payjoin_test_utils::{BoxError, EXAMPLE_URL, KEM, KEY_ID, PARSED_ORIGINAL_PSBT, SYMMETRIC}; use super::*; + use crate::persist::NoopSessionPersister; + use crate::receive::v2::Receiver; use crate::OhttpKeys; const SERIALIZED_BODY_V2: &str = "63484e696450384241484d43414141414159386e757447674a647959475857694245623435486f65396c5747626b78682f36624e694f4a6443447544414141414141442b2f2f2f2f41747956754155414141414146366b554865684a38476e536442554f4f7636756a584c72576d734a5244434867495165414141414141415871525233514a62627a30686e513849765130667074476e2b766f746e656f66544141414141414542494b6762317755414141414146366b55336b34656b47484b57524e6241317256357452356b455644564e4348415163584667415578347046636c4e56676f31575741644e3153594e583874706854414243477343527a424541694238512b41366465702b527a393276687932366c5430416a5a6e3450524c6938426639716f422f434d6b30774967502f526a3250575a3367456a556b546c6844524e415130675877544f3774396e2b563134705a366f6c6a554249514d566d7341616f4e5748564d5330324c6654536530653338384c4e697450613155515a794f6968592b464667414241425941464562324769753663344b4f35595730706677336c4770396a4d55554141413d0a763d32"; @@ -614,4 +615,49 @@ mod test { } Ok(()) } + + #[test] + fn test_v2_sender_builder() { + let address = Address::from_str("2N47mmrWXsNBvQR6k78hWJoTji57zXwNcU7") + .expect("valid address") + .assume_checked(); + let directory = EXAMPLE_URL.clone(); + let ohttp_keys = OhttpKeys( + ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"), + ); + let pj_uri = Receiver::create_session(address.clone(), directory, ohttp_keys, None) + .save(&NoopSessionPersister::default()) + .expect("receiver should succeed") + .pj_uri(); + let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri.clone()) + .build_recommended(FeeRate::BROADCAST_MIN) + .save(&NoopSessionPersister::default()) + .expect("sender should succeed"); + // v2 senders may always override the receiver's `pjos` parameter to enable output + // substitution + assert_eq!(req_ctx.v1.output_substitution, OutputSubstitution::Enabled); + assert_eq!(&req_ctx.v1.payee, &address.script_pubkey()); + let fee_contribution = req_ctx.v1.fee_contribution.expect("sender should contribute fees"); + assert_eq!(fee_contribution.max_amount, Amount::from_sat(91)); + assert_eq!(fee_contribution.vout, 0); + assert_eq!(req_ctx.v1.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()) + .build_non_incentivizing(FeeRate::BROADCAST_MIN) + .save(&NoopSessionPersister::default()) + .expect("sender should succeed"); + assert_eq!(req_ctx.v1.output_substitution, OutputSubstitution::Enabled); + let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri.clone()) + .build_with_additional_fee(Amount::ZERO, Some(0), FeeRate::BROADCAST_MIN, false) + .save(&NoopSessionPersister::default()) + .expect("sender should succeed"); + assert_eq!(req_ctx.v1.output_substitution, OutputSubstitution::Enabled); + // ensure that a v2 sender may still disable output substitution if they prefer. + let req_ctx = SenderBuilder::new(PARSED_ORIGINAL_PSBT.clone(), pj_uri) + .always_disable_output_substitution() + .build_recommended(FeeRate::BROADCAST_MIN) + .save(&NoopSessionPersister::default()) + .expect("sender should succeed"); + assert_eq!(req_ctx.v1.output_substitution, OutputSubstitution::Disabled); + } }