Add tests to catch mutants by benalleng · Pull Request #1622 · payjoin/rust-payjoin

benalleng · 2026-06-08T13:18:59Z

$ cargo mutants -j4
Found 633 mutants to test
ok       Unmutated baseline in 41s build + 7s test
 INFO Auto-set test timeout to 36s
633 mutants tested in 18m: 407 caught, 226 unviable

Closes #1689
Closes #778

Pull Request Checklist

Please confirm the following before requesting review:

I have disclosed my use of
AI
in the body of this PR.
I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

coveralls · 2026-06-08T13:23:22Z

Coverage Report for CI Build 28409851805

Coverage increased (+0.2%) to 85.673%

Details

Coverage increased (+0.2%) from the base build.
Patch coverage: 158 of 158 lines across 4 files are fully covered (100%).
No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	15272
Covered Lines:	13084
Line Coverage:	85.67%
Coverage Strength:	358.51 hits per line

💛 - Coveralls

arminsabouri · 2026-06-08T14:03:00Z

+            assert!(input.tap_key_sig.is_some(), "tap_key_sig should be preserved");
+            assert!(!input.tap_script_sigs.is_empty(), "tap_script_sigs should be preserved");
+            assert!(input.tap_merkle_root.is_some(), "tap_merkle_root should be preserved");
+        }


Should we check the sensative fields are removed as well? Same thing for the outputs?

arminsabouri · 2026-06-08T14:07:02Z

+            assert!(input.sighash_type.is_some(), "sighash_type should be preserved");
+            assert!(input.tap_key_sig.is_some(), "tap_key_sig should be preserved");
+            assert!(!input.tap_script_sigs.is_empty(), "tap_script_sigs should be preserved");
+            assert!(input.tap_merkle_root.is_some(), "tap_merkle_root should be preserved");


Missing the final input field types that are preserved

final_script_sig: input.final_script_sig.clone(), final_script_witness: input.final_script_witness.clone(),

Is it not odd that the receiver would keep anyting but these fields? Its essentially offloading the responsiblity of witness building to the sender. Motivating example: liana may fillout tap_merkle_root but the key spend is used. Here it would be preserved and your taptree strucuture leaked to the counter party

Just to be clear you think we should be stripping

sighash_type

tap_key_sig

tap_script_sigs

tap_merkle_root

And just preserve these fields?

for input in &processed_psbt.inputs { filtered_psbt.inputs.push(bitcoin::psbt::Input { witness_utxo: input.witness_utxo.clone(), non_witness_utxo: input.non_witness_utxo.clone(), final_script_sig: input.final_script_sig.clone(), final_script_witness: input.final_script_witness.clone(), ..Default::default() }); }

We have to do one or the other not both. The receiver either gives the sender the partial fields and the sender constructs witness OR the receiver provides fully constructed witness. The latter does break both bip174/370 definition of the finalizer role. i.e all the inputs should be finalizable. So its possible the rust-bitcoin ecosystem does not have support for this which is why we have the sender finalize today.

Thats why i found it odd that we preserve the final_ fields and the partial fields.

According to The BIP 78 spec protocol section receiver checklist subsection, which BIP 77 references

The payjoin proposal MUST:
...

Only finalize the inputs added by the receiver. (Referred later as additional inputs)

Additionally, in the sender checklist section it says:

For each input in the [payjoin] proposal:
...
Verify that no partial signature has been filled

So per the spec we want non_witness_utxo or witness_utxo and final_* fields.

This test catches the mutation that replaces the script_pubkey match guard in arm 3 ("our output") of PsbtContext::check_outputs with `true`. Arm 3 matches when the proposed output has the same script_pubkey as the original non-payee output. The mutation makes it match ANY output, consuming original outputs incorrectly and bypassing the script check.

Following the fix in f165562 the mutants tests here were previously skipped as I had not fully grasped the logic being tested. However these mutants can now much more easily be tested following that change.

DanGould

ACK 1e909d2 LGTM, one question for the follow up

DanGould · 2026-06-30T07:50:24Z

+
+    #[test]
+    fn try_preserving_privacy_falls_back_when_min_in_equals_min_out() {
+        let original = original_from_test_vector();
+        let wants_inputs = WantsOutputs::new(original, vec![0]).commit_outputs();
+        let small = candidate_input_from_test_vector(Amount::ONE_SAT);
+        let boundary = candidate_input_from_test_vector(Amount::from_sat(2_000_000));
+        let result = wants_inputs.try_preserving_privacy([small.clone(), boundary.clone()]);
+        assert_eq!(result.unwrap(), small);
+    }


I think this would have caught the bug BBMobile reported here. Almost on time!

Fix try_preserving_privacy returning Empty when candidates exist #1629

DanGould · 2026-06-30T07:56:44Z

+            ctx.output_substitution = OutputSubstitution::Enabled;
+
+            let mut proposal = PARSED_PAYJOIN_PROPOSAL.clone();
+            let original_change = &ctx.original_psbt.unsigned_tx.output[0];


Open to having the PAYJOIN_PROPOSAL_ADDITIONALFEEOUTPUTINDEX = 0 constant?
I knew this was reality but had to go hunt down the source to make sure the test made sense.

This is a followup on payjoin#1622 to add consts for the MAX_ADDITIONAL_FEE_CONTRIBUTION and ADDITIONAL_FEE_OUTPUT_INDEX to better understand why these values are included in unit tests across the codebase.

arminsabouri suggested changes Jun 8, 2026

View reviewed changes

benalleng force-pushed the cargo-mutants-06-08 branch 5 times, most recently from b86419e to fc33285 Compare June 11, 2026 13:32

benalleng force-pushed the cargo-mutants-06-08 branch from fc33285 to d88c366 Compare June 15, 2026 20:33

benalleng requested a review from arminsabouri June 29, 2026 14:08

Add tests to catch proper field input stripping

fd1f29e

benalleng force-pushed the cargo-mutants-06-08 branch from d88c366 to 22464e1 Compare June 29, 2026 21:27

benalleng requested a review from DanGould June 29, 2026 21:42

benalleng force-pushed the cargo-mutants-06-08 branch from 22464e1 to c8a09dd Compare June 29, 2026 21:49

benalleng added 2 commits June 29, 2026 19:37

Test that try_preserving_privacy checks that min_in > min_out

1e909d2

Following the fix in f165562 the mutants tests here were previously skipped as I had not fully grasped the logic being tested. However these mutants can now much more easily be tested following that change.

benalleng force-pushed the cargo-mutants-06-08 branch from c8a09dd to 1e909d2 Compare June 29, 2026 23:37

DanGould approved these changes Jun 30, 2026

View reviewed changes

benalleng merged commit d25ab88 into payjoin:master Jun 30, 2026
13 checks passed

benalleng mentioned this pull request Jun 30, 2026

Add Spec related consts for better readability in tests #1699

Merged

2 tasks

spacebear21 mentioned this pull request Jul 10, 2026

Bump payjoin version to 1.0.0-rc.4 #1726

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add tests to catch mutants#1622

Add tests to catch mutants#1622
benalleng merged 3 commits into
payjoin:masterfrom
benalleng:cargo-mutants-06-08

benalleng commented Jun 8, 2026 •

edited

Loading

Uh oh!

coveralls commented Jun 8, 2026 •

edited

Loading

Uh oh!

arminsabouri Jun 8, 2026

Uh oh!

arminsabouri Jun 8, 2026

Uh oh!

arminsabouri Jun 8, 2026

Uh oh!

benalleng Jun 8, 2026

Uh oh!

arminsabouri Jun 9, 2026

Uh oh!

DanGould Jun 9, 2026

Uh oh!

arminsabouri Jun 10, 2026

Uh oh!

DanGould left a comment

Uh oh!

DanGould Jun 30, 2026

Uh oh!

DanGould Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

benalleng commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 28409851805

Coverage increased (+0.2%) to 85.673%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanGould left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benalleng commented Jun 8, 2026 •

edited

Loading

coveralls commented Jun 8, 2026 •

edited

Loading