Skip to content

Validate sender input UTXO data #1836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jolah1 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Validate sender input UTXO data #1836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2922a5b
Test receiver checks against the receiver input
Jolah1 Aug 25, 2026
df46ca8
Reject proposals missing original sender inputs
Jolah1 Aug 25, 2026
6db2b1d
Reject modified sender input UTXO information
Jolah1 Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions payjoin/src/core/send/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ pub(crate) enum InternalProposalError {
SenderTxinContainsFinalScriptSig,
SenderTxinContainsFinalScriptWitness,
SenderTxinNonAllSighashType,
SenderTxinUtxoInfoChanged,
TxInContainsKeyPaths,
ContainsPartialSigs,
ReceiverTxinNotFinalized,
Expand Down Expand Up @@ -201,6 +202,7 @@ impl fmt::Display for InternalProposalError {
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"),
SenderTxinNonAllSighashType => write!(f, "an input in proposed transaction belonging to the sender requests a sighash type other than SIGHASH_ALL"),
SenderTxinUtxoInfoChanged => write!(f, "an input in proposed transaction belonging to the sender contains modified UTXO information"),
TxInContainsKeyPaths => write!(f, "proposed transaction inputs contain key paths"),
ContainsPartialSigs => write!(f, "an input in proposed transaction belonging to the sender contains partial signatures"),
ReceiverTxinNotFinalized => write!(f, "an input in proposed transaction belonging to the receiver is not finalized"),
Expand Down Expand Up @@ -236,6 +238,7 @@ impl std::error::Error for InternalProposalError {
SenderTxinContainsFinalScriptSig => None,
SenderTxinContainsFinalScriptWitness => None,
SenderTxinNonAllSighashType => None,
SenderTxinUtxoInfoChanged => None,
TxInContainsKeyPaths => None,
ContainsPartialSigs => None,
ReceiverTxinNotFinalized => None,
Expand Down
179 changes: 151 additions & 28 deletions payjoin/src/core/send/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

use bitcoin::psbt::{Psbt, PsbtSighashType};
use bitcoin::sighash::TapSighashType;
use bitcoin::{Amount, FeeRate, Script, ScriptBuf, TxOut, Weight};
use bitcoin::{Amount, FeeRate, Script, ScriptBuf, TxIn, TxOut, Weight};
pub use error::{BuildSenderError, ResponseError, ValidationError, WellKnownError};
pub(crate) use error::{InternalBuildSenderError, InternalProposalError, InternalValidationError};

Expand Down Expand Up @@ -285,6 +285,21 @@ fn commits_to_all_inputs_and_outputs(sighash_type: PsbtSighashType) -> bool {
matches!(sighash_type.taproot_hash_ty(), Ok(TapSighashType::Default | TapSighashType::All))
}

/// Whether `originals` appear among `proposed` in the same relative order.
///
/// BIP 78 forbids the receiver from shuffling: its additional inputs "must be inserted at a
/// random index", so the sender's inputs must survive as an ordered subsequence of the
/// proposal's.
fn is_ordered_subsequence<'a>(
originals: impl IntoIterator<Item = &'a TxIn>,
proposed: impl IntoIterator<Item = &'a TxIn>,
) -> bool {
let mut proposed = proposed.into_iter();
originals
.into_iter()
.all(|original| proposed.any(|p| p.previous_output == original.previous_output))
}

impl PsbtContext {
fn process_proposal(self, mut proposal: Psbt) -> InternalResult<Psbt> {
self.basic_checks(&proposal)?;
Expand Down Expand Up @@ -367,6 +382,14 @@ impl PsbtContext {
proposal: &Psbt,
ensure_receiver_input_finalized: bool,
) -> InternalResult<()> {
ensure(
is_ordered_subsequence(
&self.original_psbt.unsigned_tx.input,
&proposal.unsigned_tx.input,
),
InternalProposalError::MissingOrShuffledInputs,
)?;

let mut original_inputs = self.original_psbt.input_pairs().peekable();

for proposed in proposal.input_pairs() {
Expand Down Expand Up @@ -404,6 +427,26 @@ impl PsbtContext {
InternalProposalError::SenderTxinNonAllSighashType,
)?;
}
// BIP 78 says nothing about the receiver altering UTXO data on the
// sender's inputs. Its reference implementation overwrites these
// fields with the sender's own values without ever reading them,
// and its proposal vectors return them unchanged. Rejecting a
// mismatch is our policy, not a spec requirement; `is_none_or`
// stays lenient toward receivers that strip what they do not need.
ensure(
proposed
.psbtin
.witness_utxo
.as_ref()
.is_none_or(|utxo| Some(utxo) == original.psbtin.witness_utxo.as_ref()),
InternalProposalError::SenderTxinUtxoInfoChanged,
)?;
ensure(
proposed.psbtin.non_witness_utxo.as_ref().is_none_or(|utxo| {
Some(utxo) == original.psbtin.non_witness_utxo.as_ref()
}),
InternalProposalError::SenderTxinUtxoInfoChanged,
)?;
Comment thread
xstoicunicornx marked this conversation as resolved.
original_inputs.next();
}
// theirs (receiver)
Expand Down Expand Up @@ -717,10 +760,11 @@ mod test {
use bitcoin::absolute::LockTime;
use bitcoin::bip32::{DerivationPath, Fingerprint};
use bitcoin::ecdsa::Signature;
use bitcoin::hashes::Hash;
use bitcoin::hex::FromHex;
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, SecretKey, SECP256K1};
use bitcoin::taproot::TaprootBuilder;
use bitcoin::{Amount, FeeRate, OutPoint, Script, ScriptBuf, Sequence, Witness};
use bitcoin::{Amount, FeeRate, OutPoint, Script, ScriptBuf, Sequence, Txid, Witness};
use payjoin_test_utils::{
BoxError, ADDITIONAL_FEE_OUTPUT_INDEX, MAX_ADDITIONAL_FEE_CONTRIBUTION,
PARSED_ORIGINAL_PSBT, PARSED_PAYJOIN_PROPOSAL, PARSED_PAYJOIN_PROPOSAL_WITH_SENDER_INFO,
Expand All @@ -747,6 +791,29 @@ mod test {
})
}

#[test]
fn ordered_subsequence_requires_original_order() {
let txin = |vout| TxIn {
previous_output: OutPoint::new(Txid::all_zeros(), vout),
..Default::default()
};
let originals = vec![txin(0), txin(1)];

assert!(is_ordered_subsequence(&originals, &vec![txin(0), txin(1)]));
// Receiver inputs may be inserted before, between, and after the sender's.
assert!(is_ordered_subsequence(&originals, &vec![txin(0), txin(9), txin(1)]));
Comment thread
xstoicunicornx marked this conversation as resolved.
assert!(is_ordered_subsequence(
&originals,
&vec![txin(3), txin(0), txin(9), txin(1), txin(3)]
));
assert!(!is_ordered_subsequence(
&originals,
&vec![txin(3), txin(1), txin(9), txin(0), txin(3)]
));
assert!(!is_ordered_subsequence(&originals, &vec![txin(1), txin(0)]));
assert!(!is_ordered_subsequence(&originals, &vec![txin(0)]));
}

#[test]
fn test_restore_original_utxos() -> Result<(), BoxError> {
let mut original_psbt = PARSED_ORIGINAL_PSBT.clone();
Expand Down Expand Up @@ -1241,6 +1308,81 @@ mod test {
Ok(())
}

#[test]
fn test_sender_input_outpoint_changed() -> Result<(), BoxError> {
let ctx = create_psbt_context()?;
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();
proposal.unsigned_tx.input[0].previous_output.vout += 1;

assert_eq!(
ctx.process_proposal(proposal).unwrap_err().to_string(),
InternalProposalError::MissingOrShuffledInputs.to_string()
);
Ok(())
}

#[test]
fn test_sender_input_witness_utxo_changed() -> Result<(), BoxError> {
let ctx = create_psbt_context()?;
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();
proposal.inputs[0]
.witness_utxo
.as_mut()
.expect("test sender input should have witness UTXO information")
.value += Amount::from_sat(1);

assert_eq!(
ctx.process_proposal(proposal).unwrap_err().to_string(),
InternalProposalError::SenderTxinUtxoInfoChanged.to_string()
);
Ok(())
}

#[test]
fn test_sender_input_non_witness_utxo_changed() -> Result<(), BoxError> {
let mut ctx = create_psbt_context()?;
let previous_tx = PARSED_ORIGINAL_PSBT.clone().extract_tx_unchecked_fee_rate();
ctx.original_psbt.inputs[0].non_witness_utxo = Some(previous_tx.clone());
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();
proposal.inputs[0].non_witness_utxo = Some(previous_tx);
proposal.inputs[0]
.non_witness_utxo
.as_mut()
.expect("test sender input should have non-witness UTXO information")
.output[0]
.value += Amount::from_sat(1);

assert_eq!(
ctx.process_proposal(proposal).unwrap_err().to_string(),
InternalProposalError::SenderTxinUtxoInfoChanged.to_string()
);
Ok(())
}

#[test]
fn test_sender_input_utxo_info_unchanged_or_stripped() -> Result<(), BoxError> {
let mut ctx = create_psbt_context()?;
let previous_tx = PARSED_ORIGINAL_PSBT.clone().extract_tx_unchecked_fee_rate();
ctx.original_psbt.inputs[0].non_witness_utxo = Some(previous_tx.clone());
Comment thread
xstoicunicornx marked this conversation as resolved.
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();
proposal.inputs[0].non_witness_utxo = Some(previous_tx);

// Both fields must be present and matching, or the unchanged case below
// passes vacuously through `is_none_or`.
assert!(proposal.inputs[0].witness_utxo.is_some());
assert_eq!(proposal.inputs[0].witness_utxo, ctx.original_psbt.inputs[0].witness_utxo);
assert!(proposal.inputs[0].non_witness_utxo.is_some());

assert!(ctx.clone().process_proposal(proposal.clone()).is_ok());

// Tolerate receivers that strip UTXO data they do not need.
proposal.inputs[0].witness_utxo = None;
proposal.inputs[0].non_witness_utxo = None;

assert!(ctx.process_proposal(proposal).is_ok());
Ok(())
}

#[test]
fn test_sender_input_final_script_sig_is_present() -> Result<(), BoxError> {
let ctx = create_psbt_context()?;
Expand Down Expand Up @@ -1397,14 +1539,9 @@ mod test {
let ctx = create_psbt_context()?;
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();

// If the outpoints are different, they are considered a receiver input and will be checked as such
let proposed_outpoint = proposal.unsigned_tx.input.first().unwrap().previous_output;
proposal.unsigned_tx.input.get_mut(0).unwrap().previous_output =
OutPoint::new(proposed_outpoint.txid, proposed_outpoint.vout + 1);

// Make the receiver's input un-finalized
proposal.inputs.get_mut(0).unwrap().final_script_sig = None;
proposal.inputs.get_mut(0).unwrap().final_script_witness = None;
// Make the receiver's input unfinalized.
proposal.inputs.get_mut(1).unwrap().final_script_sig = None;
proposal.inputs.get_mut(1).unwrap().final_script_witness = None;

assert_eq!(
ctx.process_proposal(proposal).unwrap_err().to_string(),
Expand All @@ -1419,16 +1556,9 @@ mod test {
let ctx = create_psbt_context()?;
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();

// If the outpoints are different, they are considered a receiver input and will be checked as such
let proposed_outpoint = proposal.unsigned_tx.input.first().unwrap().previous_output;
proposal.unsigned_tx.input.get_mut(0).unwrap().previous_output =
OutPoint::new(proposed_outpoint.txid, proposed_outpoint.vout + 1);
proposal.inputs.get_mut(0).unwrap().final_script_sig = Some(ScriptBuf::new());
proposal.inputs.get_mut(0).unwrap().final_script_witness = Some(Witness::new());

// Make the receiver's input un-finalized
proposal.inputs.get_mut(0).unwrap().witness_utxo = None;
proposal.inputs.get_mut(0).unwrap().non_witness_utxo = None;
// Remove the receiver's UTXO information.
proposal.inputs.get_mut(1).unwrap().witness_utxo = None;
proposal.inputs.get_mut(1).unwrap().non_witness_utxo = None;

assert_eq!(
ctx.process_proposal(proposal).unwrap_err().to_string(),
Expand All @@ -1443,16 +1573,9 @@ mod test {
let mut ctx = create_psbt_context()?;
let mut proposal: bitcoin::Psbt = PARSED_PAYJOIN_PROPOSAL.clone();

// If the outpoints are different, they are considered a receiver input and will be checked as such
let proposed_outpoint = proposal.unsigned_tx.input.first().unwrap().previous_output;
proposal.unsigned_tx.input.get_mut(0).unwrap().previous_output =
OutPoint::new(proposed_outpoint.txid, proposed_outpoint.vout + 1);
proposal.inputs.get_mut(0).unwrap().final_script_sig = Some(ScriptBuf::new());
proposal.inputs.get_mut(0).unwrap().final_script_witness = Some(Witness::new());

// Ensure the sequence is different
let sequence = ctx.original_psbt.unsigned_tx.input.get_mut(0).unwrap().sequence;
proposal.unsigned_tx.input.get_mut(0).unwrap().sequence =
proposal.unsigned_tx.input.get_mut(1).unwrap().sequence =
Sequence::from_consensus(sequence.to_consensus_u32() + 1);

assert_eq!(
Expand Down
Loading