Non deterministic mutation in `check_outputs`

[benalleng]

payjoin/src/send/mod.rs:288:24: replace match guard with true

creates a mutation on this code block

rust-payjoin/payjoin/src/send/mod.rs

Lines 241 to 303 in 7cf3df0

	fn check_outputs(&self, proposal: &Psbt) -> InternalResult<Amount> {
	let mut original_outputs =
	self.original_psbt.unsigned_tx.output.iter().enumerate().peekable();
	let mut contributed_fee = Amount::ZERO;
	for (proposed_txout, proposed_psbtout) in
	proposal.unsigned_tx.output.iter().zip(&proposal.outputs)
	{
	ensure(
	proposed_psbtout.bip32_derivation.is_empty(),
	InternalProposalError::TxOutContainsKeyPaths,
	)?;
	match (original_outputs.peek(), self.fee_contribution) {
	// fee output
	(
	Some((original_output_index, original_output)),
	Some(AdditionalFeeContribution {
	max_amount: max_fee_contrib,
	vout: fee_contrib_idx,
	}),
	) if proposed_txout.script_pubkey == original_output.script_pubkey
	&& *original_output_index == fee_contrib_idx =>
	{
	if proposed_txout.value < original_output.value {
	contributed_fee = original_output.value - proposed_txout.value;
	ensure(
	contributed_fee <= max_fee_contrib,
	InternalProposalError::FeeContributionExceedsMaximum,
	)?;
	// The remaining fee checks are done in later in `check_fees`
	}
	original_outputs.next();
	}
	// payee output
	(Some((_original_output_index, original_output)), _)
	if original_output.script_pubkey == self.payee =>
	{
	ensure(
	self.output_substitution == OutputSubstitution::Enabled
	|| (proposed_txout.script_pubkey == original_output.script_pubkey
	&& proposed_txout.value >= original_output.value),
	InternalProposalError::DisallowedOutputSubstitution,
	)?;
	original_outputs.next();
	}
	// our output
	(Some((_original_output_index, original_output)), _)
	if proposed_txout.script_pubkey == original_output.script_pubkey =>
	{
	ensure(
	proposed_txout.value >= original_output.value,
	InternalProposalError::OutputValueDecreased,
	)?;
	original_outputs.next();
	}
	// additional output
	_ => (),
	}
	}
	ensure(original_outputs.peek().is_none(), InternalProposalError::MissingOrShuffledOutputs)?;
	Ok(contributed_fee)
	}

Namely creating this mutation.

(Some((_original_output_index, original_output)), _) 
    if true => 
{ 
    ensure( 
        proposed_txout.value >= original_output.value, 
        InternalProposalError::OutputValueDecreased, 
    )?; 
    original_outputs.next(); 
} 

The issue arises that this mutation is only caught sometimes by the test integration::batching::receiver_forwards_payment
https://github.com/payjoin/rust-payjoin/blob/master/payjoin/tests/integration.rs#L1208

It is able to catch the mutation when the receiver output "Our output" is early in the iteration but if it occurs last of the 3 outputs in this test it is seemingly skipped and the test passes with this mutation incorrectly.

N.B. This is my understanding of how this match block works, It is possible I am misunderstanding the logic as the non-deterministic nature is causing some confusion.

I have 2 questions about the approach trying to solve this mutation.

1. Can we force some deterministic result to prevent this mutation being inconsistent, possibly in some new unit-test?
2. Is there some simple assert I am missing in the integration test to catch this change.

[spacebear21]

This specific mutation is now skipped as of 1f39170, so this issue is still alive until a better solution is found.