refactor: redundant code extermination in running.rs

[SimonRastikian]

While working on renaming the threshold into governance threshold, I realized a huge code redundancy in running.rs

[claude]

Pull request overview

Test-only refactor of the #[cfg(test)] mod running_tests module in running.rs: the repeated gen_running_state + Environment::new + set_signer preamble is extracted into setup/setup_with_params, the duplicated "find a participant present in both the current and proposed sets" block becomes overlapping_signer, and five near-identical happy-path vote_add_domains tests collapse into one #[rstest] with five named cases. No production code changes; net -98 lines.

I checked the five merged cases against the real bounds (governance_threshold_lower_relative_bound = ceil(3n/5), upper bound = n, validate_domain_reconstruction_threshold, validate_governance_against_reconstruction): (5,5) with t=3, (4,3) with t=3, and the seeded-fixture cases at t=2 or t=governance all remain valid, and none of the removed test names are referenced anywhere else in the repo. The refactor is behavior-preserving and the coverage of all five original tests is retained.

Changes:

• New test helpers setup, setup_with_params, with_first_participant_signer, overlapping_signer; single_domain_proposal moved up with the other helpers and proposal_with_threshold dropped in its favor.
• Five vote_add_domains acceptance tests merged into vote_add_domains__should_accept_valid_domain_proposal, encoding "use the GovernanceThreshold" as reconstruction_threshold: None and "use the seeded fixture params" as pinned_params: None.
• BTreeMap::new() + insert replaced with BTreeMap::from([...]); the mid-file use std::collections::BTreeMap; hoisted into the module import block.

Reviewed changes

Per-file summary

File	Description
crates/contract/src/state/running.rs	Test-module refactor: shared setup/signer/proposal helpers, five acceptance tests merged into one parameterized #[rstest], BTreeMap::from for threshold-update maps, imports tidied.

Findings

Blocking (must fix before merge):

• crates/contract/src/state/running.rs:317 — overlapping_signer panics on failure (.expect("proposal must retain at least one current participant"), line 332) and is precisely the case that docs/engineering-standards.md §"must_ prefix for panicking test helpers" describes: "extracting setup data from a known-good state". Such helpers must carry the must_ prefix so callers can see at the call site that the function will panic on failure. The repo already follows this for the directly analogous helper — must_get_domain (crates/e2e-tests/tests/common.rs:397), must_g1_point_outside_subgroup (crates/contract/src/primitives/ckd.rs:140). Suggest must_find_overlapping_signer; and since with_first_participant_signer (line 303) indexes participants()[0], the same rule covers setup / setup_with_params / with_first_participant_signer (e.g. must_setup, must_setup_with_params, must_sign_as_first_participant). Extracting these into named helpers is what makes the missing prefix visible, so this is the right PR to apply it in; scoping the rename to the new helpers and leaving the pre-existing gen_* fixtures untouched is fine.

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

• crates/contract/src/state/running.rs:657 and :661 — the merged cases lost the justification for their pinned params. Each original test spelled out why the numbers were chosen ("GovernanceThreshold 5 so a reconstruction threshold of 3 is allowed", "GovernanceThreshold 4 so the proposal ReconstructionThreshold (3) fits"); Some((5, 5)) and Some((4, 3)) are now opaque, so someone debugging a failure cannot tell whether the values are load-bearing. They are: for (4, 3) with t=3, governance == reconstruction, so that case silently also pins the upper boundary that validate_governance_against_reconstruction permits. A trailing // (n, k): ... per case, or named constants, would carry what the shared doc comment cannot say per-case.
• crates/contract/src/state/running.rs:335 — the doc comment "Builds a [DomainConfig] for the next domain id with the given protocol, purpose, and reconstruction threshold." restates the signature and adds nothing beyond the name and parameter list. The text is pre-existing, but this PR relocates it, which makes it a cheap moment to drop it per docs/engineering-standards.md §"Write helpful code comments" (paraphrasing the code). The overlapping_signer and #[rstest] doc comments, by contrast, explain non-obvious why and should stay.
• crates/contract/src/state/running.rs:284, :290, :311 — these doc comments use intra-doc links ([`Environment`], [`setup`], [`gen_valid_params_proposal`]) from inside a #[cfg(test)] module. Rustdoc never sees this module, so the links are neither rendered nor link-checked; per CLAUDE.md ("a cfg(test) item invisible to rustdoc stays a plain code span") plain backticks are the intended form here.

⚠️ Issues found