Skip to content

Commit

Permalink
Allow mixed inputs in v1 payjoin
Browse files Browse the repository at this point in the history 
BIP 78 was updated to allow mixed inputs.

> Disallowing mixed inputs was based on incorrect assumption that no
> wallet supports mixed inputs and thus mixed inputs imply PayJoin.
> However there are at least three wallets supporting mixed inputs.
> (Confirmed: Bitcoin Core, LND, Coinomi) Thus it makes sense to enable
> mixed inputs to avoid a payjoin-specific fingerptint. To avoid
> compatibility issues a grace period is suggested.

See: bitcoin/bips#1605
See also: payjoin#480
  • Loading branch information
@DanGould
DanGould committed Jan 23, 2025
1 parent 5d6460e commit ab12170
Show file tree
Hide file tree
Showing 8 changed files with 4 additions and 237 deletions.
15 changes: 0 additions & 15 deletions payjoin/src/receive/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,28 +337,13 @@ 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,
}

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"),
}
Expand Down
6 changes: 0 additions & 6 deletions payjoin/src/receive/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
47 changes: 0 additions & 47 deletions payjoin/src/receive/v1/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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<bitcoin::AddressType>,
) -> 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<Option<bitcoin::AddressType>, 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
Expand Down
5 changes: 1 addition & 4 deletions payjoin/src/send/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -154,7 +154,6 @@ pub(crate) enum InternalProposalError {
ReceiverTxinNotFinalized,
ReceiverTxinMissingUtxoInfo,
MixedSequence,
MixedInputTypes { proposed: AddressType, original: AddressType },
MissingOrShuffledInputs,
TxOutContainsKeyPaths,
FeeContributionExceedsMaximum,
Expand Down Expand Up @@ -193,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"),
Expand Down Expand Up @@ -228,7 +226,6 @@ impl std::error::Error for InternalProposalError {
ReceiverTxinNotFinalized => None,
ReceiverTxinMissingUtxoInfo => None,
MixedSequence => None,
MixedInputTypes { .. } => None,
MissingOrShuffledInputs => None,
TxOutContainsKeyPaths => None,
FeeContributionExceedsMaximum => None,
Expand Down
9 changes: 0 additions & 9 deletions payjoin/src/send/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -175,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
);
}
}
}
}
Expand Down Expand Up @@ -443,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,
}
}

Expand Down
1 change: 0 additions & 1 deletion payjoin/src/send/v1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
))
Expand Down
1 change: 0 additions & 1 deletion payjoin/src/send/v2/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
157 changes: 3 additions & 154 deletions payjoin/tests/integration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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(())
}
}
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit ab12170

Please sign in to comment.