test: flaky test for DamgardEtAl

[SimonRastikian]

Closes #3398

[claude]

PR title type suggestion: This PR changes only test files, so the type prefix should probably be test: instead of fix:.
Suggested title: test: fix flaky test for DamgardEtAl

[claude]

Pull request overview

This PR addresses flaky test test_onboarding_jobs (issue #3398) by enforcing a participant-count floor of 3 in gen_valid_params_proposal. Previously the function could sample n_old_participants = 2 with max_added = 0, producing parameters that violate DamgardEtAl's n >= 2t - 1 constraint. The fix introduces a min_added floor so the resulting participant count is always >= 3, mirroring what gen_threshold_params already does.

Changes:

• Compute min_added from n_old_participants so the proposed n is at least 3
• Update sampling of max_added to respect the new floor

Reviewed changes

Per-file summary

File	Description
crates/contract/src/state/test_utils.rs	Adds a min_added floor to gen_valid_params_proposal so generated proposals always have n >= 3.

Findings

Blocking (must fix before merge):

• crates/contract/src/state/test_utils.rs:42-65 — The file contains unresolved git merge conflict markers (<<<<<<< Updated upstream, =======, >>>>>>> Stashed changes). The file as committed will not compile. You need to pick one of the two alternatives and remove all conflict markers. The two variants differ in approach:

  • Upstream variant: sample max_added from min_added..min_added + 10 (shifts the whole range up).
  • Stashed variant: sample max_added from 0..10 then clamp via .max(min_added) (preserves the original [0, 10) range and only nudges the lowest values).

  The PR description / comment text suggests the "Stashed" variant was the intended one ("Clamping after sampling keeps the original [0, 10) range and only nudges the lowest values up when they'd produce an invalid n"), but either is functionally adequate — please resolve and force-push a clean diff. After resolution the diff should be ~5–10 lines, not 23.

Non-blocking (nits, follow-ups, suggestions):

• crates/contract/src/state/test_utils.rs:56-60 — Minor: min_added could be written 3usize.saturating_sub(n_old_participants) for terser intent, though the current form is clear enough.
• Consider adding a focused unit test that exercises the n_old_participants == 2 path (e.g., by constructing params with current_k = 2, current_n = 2) to lock in the regression and avoid relying on the probabilistic test_onboarding_jobs to catch a future reintroduction.

⚠️ Issues found

[claude]

PR title type suggestion: This PR changes only test files, so the type prefix should be test: instead of fix:.

Suggested title: test: fix flaky test for DamgardEtAl

[claude]

PR title type suggestion: Since this PR only changes test files, the type prefix should be test: instead of fix:.

Suggested title: test: fix flaky test for DamgardEtAl

[claude]

PR title type suggestion: This PR changes only test utilities, so the type prefix should probably be test: instead of fix:.
Suggested title: test: fix flaky test for DamgardEtAl

[claude]

PR title type suggestion: This PR changes only test utilities, so the type prefix should probably be test: instead of fix:.

Suggested title: test: fix flaky test for DamgardEtAl

[DSharifi]

Thanks!

[gilcu3]

I think we should start by making the test generator deterministic, and then ensure that it generates a valid set of parameters for all our schemes more naturally, without having to patch values after they were generated

[gilcu3]

if DamgardEtAl requires $n \geq 3$ then how can we have n_old_participants < 3? I think the fix proposed is very convoluted and does not really solve part of the issue, which is that this test is not deterministic

[SimonRastikian]

n_old_participants is computed as follows.
Where

   let current_k = params.threshold().value() as usize;
   let current_n = params.participants().len();
  let n_old_participants: usize = rng.gen_range(current_k..current_n + 1);

It could happen that
current_k = t = 2 and current_n = n = 3. As you see this can indeed imply that n_old_participants = 2.

[gilcu3]

Thank you!