rust-payjoin icon indicating copy to clipboard operation
rust-payjoin copied to clipboard

Non deterministic mutation in `check_outputs`

Open benalleng opened this issue 6 months ago • 1 comments

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

creates a mutation on this code block

https://github.com/payjoin/rust-payjoin/blob/7cf3df0b860814c7e1b24d76584b11a996565e82/payjoin/src/send/mod.rs#L241-L303

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.

benalleng avatar Jun 16 '25 18:06 benalleng

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

spacebear21 avatar Jul 29 '25 18:07 spacebear21