diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs index c60baa82d..a929920b8 100644 --- a/payjoin/src/receive/error.rs +++ b/payjoin/src/receive/error.rs @@ -337,12 +337,6 @@ pub struct InputContributionError(InternalInputContributionError); #[derive(Debug)] pub(crate) enum InternalInputContributionError { - /// The address type could not be determined - AddressType(crate::psbt::AddressTypeError), - /// The original PSBT has no inputs - NoSenderInputs, - /// The proposed receiver inputs would introduce mixed input script types - MixedInputScripts(bitcoin::AddressType, bitcoin::AddressType), /// Total input value is not enough to cover additional output value ValueTooLow, } @@ -350,15 +344,6 @@ pub(crate) enum InternalInputContributionError { impl fmt::Display for InputContributionError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self.0 { - InternalInputContributionError::AddressType(e) => - write!(f, "The address type could not be determined: {}", e), - InternalInputContributionError::NoSenderInputs => - write!(f, "The original PSBT has no inputs"), - InternalInputContributionError::MixedInputScripts(type_a, type_b) => write!( - f, - "The proposed receiver inputs would introduce mixed input script types: {}; {}.", - type_a, type_b - ), InternalInputContributionError::ValueTooLow => write!(f, "Total input value is not enough to cover additional output value"), } diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs index 430a8faec..99a46c357 100644 --- a/payjoin/src/receive/mod.rs +++ b/payjoin/src/receive/mod.rs @@ -36,12 +36,6 @@ impl InputPair { Ok(input_pair) } - pub(crate) fn address_type(&self) -> AddressType { - InternalInputPair::from(self) - .address_type() - .expect("address type should have been validated in InputPair::new") - } - pub(crate) fn previous_txout(&self) -> TxOut { InternalInputPair::from(self) .previous_txout() diff --git a/payjoin/src/receive/v1/mod.rs b/payjoin/src/receive/v1/mod.rs index 9ae0593ec..5cabdd2c1 100644 --- a/payjoin/src/receive/v1/mod.rs +++ b/payjoin/src/receive/v1/mod.rs @@ -495,18 +495,11 @@ impl WantsInputs { .first() .map(|input| input.sequence) .unwrap_or_default(); - let uniform_sender_input_type = self.uniform_sender_input_type()?; // Insert contributions at random indices for privacy let mut rng = rand::thread_rng(); let mut receiver_input_amount = Amount::ZERO; for input_pair in inputs.into_iter() { - let input_type = input_pair.address_type(); - if self.params.v == 1 { - // v1 payjoin proposals must not introduce mixed input script types - self.check_mixed_input_types(input_type, uniform_sender_input_type)?; - } - receiver_input_amount += input_pair.previous_txout().value; let index = rng.gen_range(0..=self.payjoin_psbt.unsigned_tx.input.len()); payjoin_psbt.inputs.insert(index, input_pair.psbtin); @@ -533,46 +526,6 @@ impl WantsInputs { }) } - /// Check for mixed input types and throw an error if conditions are met - fn check_mixed_input_types( - &self, - receiver_input_type: bitcoin::AddressType, - uniform_sender_input_type: Option, - ) -> Result<(), InputContributionError> { - if let Some(uniform_sender_input_type) = uniform_sender_input_type { - if receiver_input_type != uniform_sender_input_type { - return Err(InternalInputContributionError::MixedInputScripts( - receiver_input_type, - uniform_sender_input_type, - ) - .into()); - } - } - Ok(()) - } - - /// Check if the sender's inputs are all of the same type - /// - /// Returns `None` if the sender inputs are not all of the same type - fn uniform_sender_input_type( - &self, - ) -> Result, InputContributionError> { - let mut sender_inputs = self.original_psbt.input_pairs(); - let first_input_type = sender_inputs - .next() - .ok_or(InternalInputContributionError::NoSenderInputs)? - .address_type() - .map_err(InternalInputContributionError::AddressType)?; - for input in sender_inputs { - if input.address_type().map_err(InternalInputContributionError::AddressType)? - != first_input_type - { - return Ok(None); - } - } - Ok(Some(first_input_type)) - } - // Compute the minimum amount that the receiver must contribute to the transaction as input fn receiver_min_input_amount(&self) -> Amount { let output_amount = self diff --git a/payjoin/src/send/error.rs b/payjoin/src/send/error.rs index ae68c4904..f01e49e2b 100644 --- a/payjoin/src/send/error.rs +++ b/payjoin/src/send/error.rs @@ -2,7 +2,7 @@ use std::fmt::{self, Display}; use bitcoin::locktime::absolute::LockTime; use bitcoin::transaction::Version; -use bitcoin::{AddressType, Sequence}; +use bitcoin::Sequence; /// Error building a Sender from a SenderBuilder. /// @@ -147,8 +147,6 @@ pub(crate) enum InternalProposalError { VersionsDontMatch { proposed: Version, original: Version }, LockTimesDontMatch { proposed: LockTime, original: LockTime }, SenderTxinSequenceChanged { proposed: Sequence, original: Sequence }, - SenderTxinContainsNonWitnessUtxo, - SenderTxinContainsWitnessUtxo, SenderTxinContainsFinalScriptSig, SenderTxinContainsFinalScriptWitness, TxInContainsKeyPaths, @@ -156,7 +154,6 @@ pub(crate) enum InternalProposalError { ReceiverTxinNotFinalized, ReceiverTxinMissingUtxoInfo, MixedSequence, - MixedInputTypes { proposed: AddressType, original: AddressType }, MissingOrShuffledInputs, TxOutContainsKeyPaths, FeeContributionExceedsMaximum, @@ -188,8 +185,6 @@ impl fmt::Display for InternalProposalError { VersionsDontMatch { proposed, original, } => write!(f, "proposed transaction version {} doesn't match the original {}", proposed, original), LockTimesDontMatch { proposed, original, } => write!(f, "proposed transaction lock time {} doesn't match the original {}", proposed, original), SenderTxinSequenceChanged { proposed, original, } => write!(f, "proposed transaction sequence number {} doesn't match the original {}", proposed, original), - SenderTxinContainsNonWitnessUtxo => write!(f, "an input in proposed transaction belonging to the sender contains non-witness UTXO information"), - SenderTxinContainsWitnessUtxo => write!(f, "an input in proposed transaction belonging to the sender contains witness UTXO information"), SenderTxinContainsFinalScriptSig => write!(f, "an input in proposed transaction belonging to the sender contains finalized non-witness signature"), SenderTxinContainsFinalScriptWitness => write!(f, "an input in proposed transaction belonging to the sender contains finalized witness signature"), TxInContainsKeyPaths => write!(f, "proposed transaction inputs contain key paths"), @@ -197,7 +192,6 @@ impl fmt::Display for InternalProposalError { ReceiverTxinNotFinalized => write!(f, "an input in proposed transaction belonging to the receiver is not finalized"), ReceiverTxinMissingUtxoInfo => write!(f, "an input in proposed transaction belonging to the receiver is missing UTXO information"), MixedSequence => write!(f, "inputs of proposed transaction contain mixed sequence numbers"), - MixedInputTypes { proposed, original, } => write!(f, "proposed transaction contains input of type {:?} while original contains inputs of type {:?}", proposed, original), MissingOrShuffledInputs => write!(f, "proposed transaction is missing inputs of the sender or they are shuffled"), TxOutContainsKeyPaths => write!(f, "proposed transaction outputs contain key paths"), FeeContributionExceedsMaximum => write!(f, "fee contribution exceeds allowed maximum"), @@ -225,8 +219,6 @@ impl std::error::Error for InternalProposalError { VersionsDontMatch { proposed: _, original: _ } => None, LockTimesDontMatch { proposed: _, original: _ } => None, SenderTxinSequenceChanged { proposed: _, original: _ } => None, - SenderTxinContainsNonWitnessUtxo => None, - SenderTxinContainsWitnessUtxo => None, SenderTxinContainsFinalScriptSig => None, SenderTxinContainsFinalScriptWitness => None, TxInContainsKeyPaths => None, @@ -234,7 +226,6 @@ impl std::error::Error for InternalProposalError { ReceiverTxinNotFinalized => None, ReceiverTxinMissingUtxoInfo => None, MixedSequence => None, - MixedInputTypes { .. } => None, MissingOrShuffledInputs => None, TxOutContainsKeyPaths => None, FeeContributionExceedsMaximum => None, diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index f45f8f30f..04f27a8d3 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -38,7 +38,6 @@ pub struct PsbtContext { fee_contribution: Option<(bitcoin::Amount, usize)>, min_fee_rate: FeeRate, payee: ScriptBuf, - allow_mixed_input_scripts: bool, } macro_rules! check_eq { @@ -145,11 +144,6 @@ impl PsbtContext { original.txin.sequence, SenderTxinSequenceChanged ); - ensure!( - proposed.psbtin.non_witness_utxo.is_none(), - SenderTxinContainsNonWitnessUtxo - ); - ensure!(proposed.psbtin.witness_utxo.is_none(), SenderTxinContainsWitnessUtxo); ensure!( proposed.psbtin.final_script_sig.is_none(), SenderTxinContainsFinalScriptSig @@ -180,13 +174,6 @@ impl PsbtContext { ReceiverTxinMissingUtxoInfo ); ensure!(proposed.txin.sequence == original.txin.sequence, MixedSequence); - if !self.allow_mixed_input_scripts { - check_eq!( - proposed.address_type()?, - original.address_type()?, - MixedInputTypes - ); - } } } } @@ -448,7 +435,6 @@ pub(crate) mod test { fee_contribution: Some((bitcoin::Amount::from_sat(182), 0)), min_fee_rate: FeeRate::ZERO, payee, - allow_mixed_input_scripts: false, } } diff --git a/payjoin/src/send/v1.rs b/payjoin/src/send/v1.rs index 5febac1f9..a20fc7025 100644 --- a/payjoin/src/send/v1.rs +++ b/payjoin/src/send/v1.rs @@ -245,7 +245,6 @@ impl Sender { fee_contribution: self.fee_contribution, payee: self.payee.clone(), min_fee_rate: self.min_fee_rate, - allow_mixed_input_scripts: false, }, }, )) diff --git a/payjoin/src/send/v2/mod.rs b/payjoin/src/send/v2/mod.rs index 33d0faada..48359a99b 100644 --- a/payjoin/src/send/v2/mod.rs +++ b/payjoin/src/send/v2/mod.rs @@ -177,7 +177,6 @@ impl Sender { fee_contribution: self.v1.fee_contribution, payee: self.v1.payee.clone(), min_fee_rate: self.v1.min_fee_rate, - allow_mixed_input_scripts: true, }, hpke_ctx, ohttp_ctx, diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index e81b44ede..d7510c6e2 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -130,7 +130,7 @@ mod integration { } #[test] - fn disallow_mixed_input_scripts() -> Result<(), BoxError> { + fn allow_mixed_input_scripts() -> Result<(), BoxError> { init_tracing(); let (_bitcoind, sender, receiver) = init_bitcoind_sender_receiver( Some(AddressType::Bech32), @@ -159,8 +159,8 @@ mod integration { // ********************** // Inside the Receiver: - // This should error because the receiver is attempting to introduce mixed input script types - assert!(handle_v1_pj_request(req, headers, &receiver, None, None, None).is_err()); + // This should NOT error because the receiver is attempting to introduce mixed input script types + assert!(handle_v1_pj_request(req, headers, &receiver, None, None, None).is_ok()); Ok(()) } } @@ -393,157 +393,6 @@ mod integration { } } - #[tokio::test] - async fn v2_to_v2_mixed_input_script_types() { - init_tracing(); - let docker: Cli = Cli::default(); - let db = docker.run(Redis); - let db_host = format!("127.0.0.1:{}", db.get_host_port_ipv4(6379)); - - let mut services = TestServices::initialize(db_host).await.unwrap(); - tokio::select!( - err = services.take_ohttp_relay_handle().unwrap() => panic!("Ohttp relay exited early: {:?}", err), - err = services.take_directory_handle().unwrap() => panic!("Directory server exited early: {:?}", err), - res = do_v2_send_receive(&services) => assert!(res.is_ok(), "v2 send receive failed: {:#?}", res) - ); - - async fn do_v2_send_receive(services: &TestServices) -> Result<(), BoxError> { - let (bitcoind, sender, receiver) = init_bitcoind_sender_receiver(None, None)?; - let agent = services.http_agent(); - services.wait_for_services_ready().await?; - let directory = services.directory_url(); - let ohttp_keys = services.fetch_ohttp_keys().await?; - // ********************** - // Inside the Receiver: - // make utxos with different script types - - let legacy_address = - receiver.get_new_address(None, Some(AddressType::Legacy))?.assume_checked(); - let nested_segwit_address = - receiver.get_new_address(None, Some(AddressType::P2shSegwit))?.assume_checked(); - let segwit_address = - receiver.get_new_address(None, Some(AddressType::Bech32))?.assume_checked(); - // TODO: - //let taproot_address = - // receiver.get_new_address(None, Some(AddressType::Bech32m))?.assume_checked(); - bitcoind.client.generate_to_address(1, &legacy_address)?; - bitcoind.client.generate_to_address(1, &nested_segwit_address)?; - bitcoind.client.generate_to_address(101, &segwit_address)?; - let receiver_utxos = receiver - .list_unspent( - None, - None, - Some(&[&legacy_address, &nested_segwit_address, &segwit_address]), - None, - None, - ) - .unwrap(); - assert_eq!(3, receiver_utxos.len(), "receiver doesn't have enough UTXOs"); - assert_eq!( - Amount::from_btc(150.0)?, - receiver_utxos.iter().fold(Amount::ZERO, |acc, txo| acc + txo.amount), - "receiver doesn't have enough bitcoin" - ); - - let address = receiver.get_new_address(None, None)?.assume_checked(); - - // test session with expiry in the future - let mut session = - Receiver::new(address.clone(), directory.clone(), ohttp_keys.clone(), None); - println!("session: {:#?}", &session); - let pj_uri_string = session.pj_uri().to_string(); - // Poll receive request - let mock_ohttp_relay = directory.clone(); - let (req, ctx) = session.extract_req(&mock_ohttp_relay)?; - let response = agent.post(req.url).body(req.body).send().await?; - assert!(response.status().is_success()); - let response_body = session.process_res(&response.bytes().await?, ctx).unwrap(); - // No proposal yet since sender has not responded - assert!(response_body.is_none()); - - // ********************** - // Inside the Sender: - // Create a funded PSBT (not broadcasted) to address with amount given in the pj_uri - let pj_uri = Uri::from_str(&pj_uri_string) - .unwrap() - .assume_checked() - .check_pj_supported() - .unwrap(); - let psbt = build_sweep_psbt(&sender, &pj_uri)?; - let req_ctx = SenderBuilder::new(psbt.clone(), pj_uri.clone()) - .build_recommended(FeeRate::BROADCAST_MIN)?; - let (Request { url, body, content_type, .. }, post_ctx) = - req_ctx.extract_v2(mock_ohttp_relay.to_owned())?; - let response = agent - .post(url.clone()) - .header("Content-Type", content_type) - .body(body.clone()) - .send() - .await - .unwrap(); - log::info!("Response: {:#?}", &response); - assert!(response.status().is_success()); - let get_ctx = post_ctx.process_response(&response.bytes().await?)?; - let (Request { url, body, content_type, .. }, ohttp_ctx) = - get_ctx.extract_req(mock_ohttp_relay.to_owned())?; - let response = agent - .post(url.clone()) - .header("Content-Type", content_type) - .body(body.clone()) - .send() - .await?; - // No response body yet since we are async and pushed fallback_psbt to the buffer - assert!(get_ctx.process_response(&response.bytes().await?, ohttp_ctx)?.is_none()); - - // ********************** - // Inside the Receiver: - - // GET fallback psbt - let (req, ctx) = session.extract_req(&mock_ohttp_relay)?; - let response = agent.post(req.url).body(req.body).send().await?; - // POST payjoin - let proposal = - session.process_res(response.bytes().await?.to_vec().as_slice(), ctx)?.unwrap(); - let inputs = receiver_utxos.into_iter().map(input_pair_from_list_unspent).collect(); - let mut payjoin_proposal = - handle_directory_proposal(&receiver, proposal, Some(inputs)); - assert!(!payjoin_proposal.is_output_substitution_disabled()); - let (req, ctx) = payjoin_proposal.extract_v2_req(&mock_ohttp_relay)?; - let response = agent.post(req.url).body(req.body).send().await?; - payjoin_proposal.process_res(&response.bytes().await?, ctx)?; - - // ********************** - // Inside the Sender: - // Sender checks, signs, finalizes, extracts, and broadcasts - // Replay post fallback to get the response - let (Request { url, body, content_type, .. }, ohttp_ctx) = - get_ctx.extract_req(mock_ohttp_relay.to_owned())?; - let response = agent - .post(url.clone()) - .header("Content-Type", content_type) - .body(body.clone()) - .send() - .await?; - let checked_payjoin_proposal_psbt = - get_ctx.process_response(&response.bytes().await?, ohttp_ctx)?.unwrap(); - let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?; - sender.send_raw_transaction(&payjoin_tx)?; - log::info!("sent"); - - // Check resulting transaction and balances - let network_fees = predicted_tx_weight(&payjoin_tx) * FeeRate::BROADCAST_MIN; - // Sender sent the entire value of their utxo to receiver (minus fees) - assert_eq!(payjoin_tx.input.len(), 4); - assert_eq!(payjoin_tx.output.len(), 1); - assert_eq!( - receiver.get_balances()?.mine.untrusted_pending, - Amount::from_btc(200.0)? - network_fees - ); - assert_eq!(sender.get_balances()?.mine.untrusted_pending, Amount::from_btc(0.0)?); - Ok(()) - } - } - #[test] fn v2_to_v1() -> Result<(), BoxError> { init_tracing();