compacting comments

Author: SimonRastikian

crates/contract/src/state/running.rs

@@ -281,8 +281,7 @@ pub mod running_tests {
         DomainConfig, DomainId, DomainPurpose, Protocol, ReconstructionThreshold,
     };
 
-    /// Returns the state plus an [`Environment`] already signing as the first
-    /// current participant.
+    /// Returns the state plus an [`Environment`] signing as the first participant.
     fn setup(num_domains: usize) -> (RunningContractState, Environment) {
         with_first_participant_signer(gen_running_state(num_domains))
     }
@@ -308,12 +307,10 @@ pub mod running_tests {
         (state, env)
     }
 
-    /// Returns a participant present in BOTH the current and proposed sets:
+    /// Returns a participant present in both the current and proposed sets:
     /// [`gen_valid_params_proposal`] keeps only a random subset of the current
-    /// participants, so an arbitrary current participant may be absent from the
-    /// proposal (rejected as a non-participant) and a freshly added one would
-    /// be deferred as a pending newcomer. The retained overlap is non-empty
-    /// (at least `threshold` current participants are kept).
+    /// participants, so an arbitrary current one may be absent from the proposal
+    /// and a freshly added one would be deferred as a pending newcomer.
     fn overlapping_signer(
         state: &RunningContractState,
         proposal: &ProposedGovernanceThresholdParameters,
@@ -332,8 +329,7 @@ pub mod running_tests {
             .expect("proposal must retain at least one current participant")
     }
 
-    /// Builds a [`DomainConfig`] for the next domain id with the given protocol,
-    /// purpose, and reconstruction threshold.
+    /// Builds a one-domain proposal for the next domain id.
     fn single_domain_proposal(
         state: &RunningContractState,
         protocol: Protocol,
@@ -514,7 +510,7 @@ pub mod running_tests {
 
     #[test]
     fn vote_add_domains__should_reject_threshold_below_two() {
-        // Given a running state and a proposal carrying t = 1
+        // Given
         let (mut state, _env) = setup(1);
         let proposal = single_domain_proposal(
             &state,
@@ -523,10 +519,10 @@ pub mod running_tests {
             ReconstructionThreshold::new(1),
         );
 
-        // When voting to add the domain
+        // When
         let err = state.vote_add_domains(proposal).unwrap_err();
 
-        // Then the universal lower bound is enforced
+        // Then
         assert!(
             err.to_string()
                 .contains("Reconstruction threshold must be at least 2"),
@@ -536,7 +532,7 @@ pub mod running_tests {
 
     #[test]
     fn vote_add_domains__should_reject_threshold_exceeding_participants() {
-        // Given a running state and a proposal whose threshold > n
+        // Given
         let (mut state, _env) = setup(1);
         let n = state.parameters.participants().len() as u64;
         let proposal = single_domain_proposal(
@@ -546,10 +542,10 @@ pub mod running_tests {
             ReconstructionThreshold::new(n + 1),
         );
 
-        // When voting to add the domain
+        // When
         let err = state.vote_add_domains(proposal).unwrap_err();
 
-        // Then the upper bound is enforced
+        // Then
         assert!(
             err.to_string().contains("exceeds participant count"),
             "Expected ReconstructionThresholdExceedsParticipants, got: {err}"
@@ -558,10 +554,8 @@ pub mod running_tests {
 
     #[test]
     fn vote_add_domains__should_reject_reconstruction_threshold_above_governance() {
-        // Given a Frost proposal whose ReconstructionThreshold exceeds the
-        // GovernanceThreshold (but is still <= participant count).
-        // GovernanceThreshold 4 < participant count 5, so `governance + 1 <= n`:
-        // the rejection comes from the threshold relation, not the n ceiling.
+        // Given governance 4 < n = 5, so `governance + 1 <= n`: the rejection
+        // comes from the threshold relation, not the n ceiling.
         let (mut state, _env) = setup_with_params(1, 5, 4);
         let governance = state.parameters.threshold().value();
         let proposal = single_domain_proposal(
@@ -574,8 +568,7 @@ pub mod running_tests {
         // When
         let err = state.vote_add_domains(proposal).unwrap_err();
 
-        // Then the GovernanceThreshold/ReconstructionThreshold relation is enforced via the
-        // canonical validate_governance_against_reconstruction helper.
+        // Then
         assert!(
             matches!(
                 err,
@@ -587,9 +580,7 @@ pub mod running_tests {
 
     #[test]
     fn vote_add_domains__should_reject_damgard_etal_threshold_violating_honest_majority() {
-        // Given a running state and a DamgardEtAl proposal with `2t - 1 > n`.
-        // gen_threshold_params produces n in [3, 30]; pick t = n so that
-        // 2t - 1 > n holds (universally true for n >= 2).
+        // Given a DamgardEtAl proposal with t = n, so `2t - 1 > n` for any n >= 2.
         let (mut state, _env) = setup(1);
         let n = state.parameters.participants().len() as u64;
         let proposal = single_domain_proposal(
@@ -599,10 +590,10 @@ pub mod running_tests {
             ReconstructionThreshold::new(n),
         );
 
-        // When voting to add the domain
+        // When
         let err = state.vote_add_domains(proposal).unwrap_err();
 
-        // Then the DamgardEtAl-specific bound is enforced
+        // Then
         assert!(
             err.to_string().contains("requires at least"),
             "Expected InsufficientParticipantsForProtocol, got: {err}"
@@ -611,16 +602,15 @@ pub mod running_tests {
 
     #[test]
     fn process_new_parameters_proposal__should_accept_empty_per_domain_threshold_updates() {
-        // Given a running state where existing thresholds are valid under the
-        // proposed participant count
+        // Given
         let (mut state, mut env) = setup(1);
         let proposal = gen_valid_params_proposal(&state.parameters);
         env.set_signer(&overlapping_signer(&state, &proposal));
 
         // When voting with an empty per_domain_thresholds map (legacy shape)
         let res = state.vote_new_parameters(state.keyset.epoch_id.next(), &proposal);
 
-        // Then the vote is recorded without error
+        // Then
         assert!(
             res.is_ok(),
             "Expected accept with empty threshold updates: {res:?}"
@@ -629,11 +619,11 @@ pub mod running_tests {
 
     #[test]
     fn process_new_parameters_proposal__should_reject_threshold_update_with_unknown_domain_id() {
-        // Given a running state with one domain
+        // Given
         let (mut state, _env) = setup(1);
         let proposal = gen_valid_params_proposal(&state.parameters);
 
-        // When voting with a threshold update referencing a non-existent domain ID
+        // When voting with a threshold update for a non-existent domain ID
         let proposal = proposal.with_per_domain_thresholds(BTreeMap::from([(
             DomainId(9999),
             ReconstructionThreshold::new(2),
@@ -642,17 +632,15 @@ pub mod running_tests {
             .vote_new_parameters(state.keyset.epoch_id.next(), &proposal)
             .unwrap_err();
 
-        // Then the unknown-domain guard rejects it
+        // Then
         assert!(
             err.to_string().contains("not in the current registry"),
             "Expected UnknownDomainInProposal, got: {err}"
         );
     }
 
-    /// Each case proposes a single valid domain; `reconstruction_threshold` of
-    /// `None` means "use the GovernanceThreshold", the maximum allowed value
-    /// (the upper boundary case). `pinned_params` of `None` uses the randomized
-    /// fixture defaults.
+    /// `reconstruction_threshold: None` means the GovernanceThreshold (the
+    /// maximum allowed); `pinned_params: None` uses the randomized fixture.
     #[rstest]
     #[case::caitsith_threshold_differing_from_existing(1, Some((5, 5)), Protocol::CaitSith, Some(3))]
     #[case::caitsith_threshold_matching_existing(1, None, Protocol::CaitSith, Some(2))]
@@ -682,16 +670,13 @@ pub mod running_tests {
         // When
         let res = state.vote_add_domains(proposal);
 
-        // Then the vote is recorded without error (single voter is below
-        // quorum, so no transition).
+        // Then the vote is recorded (single voter is below quorum, so no transition)
         assert!(res.is_ok(), "Expected acceptance: {res:?}");
     }
 
     #[test]
     fn vote_add_domains__should_accept_two_new_caitsith_with_differing_thresholds() {
-        // Given a Running state with no existing CaitSith and a proposal
-        // adding two CaitSith domains at different thresholds.
-        // GovernanceThreshold 5 so reconstruction thresholds 2 and 3 are allowed.
+        // Given a proposal adding two CaitSith domains at different thresholds
         let (mut state, _env) = setup_with_params(0, 5, 5);
         let next_id = state.domains.next_domain_id();
         let proposal = vec![
@@ -718,13 +703,11 @@ pub mod running_tests {
 
     #[test]
     fn process_new_parameters_proposal__should_apply_threshold_update_to_validation() {
-        // Given a running state with one domain whose existing threshold would
-        // remain valid under the new participants, but the threshold update
-        // swaps it for an invalid (too-low) value.
+        // Given
         let (mut state, _env) = setup(1);
         let proposal = gen_valid_params_proposal(&state.parameters);
 
-        // When voting with a threshold update that violates the universal lower bound
+        // When a threshold update swaps a valid stored value for a too-low one
         let domain_id = state.domains.domains()[0].id;
         let proposal = proposal.with_per_domain_thresholds(BTreeMap::from([(
             domain_id,
@@ -734,7 +717,7 @@ pub mod running_tests {
             .vote_new_parameters(state.keyset.epoch_id.next(), &proposal)
             .unwrap_err();
 
-        // Then the updated value (not the stored value) is validated and rejected
+        // Then the updated value, not the stored one, is validated
         assert!(
             err.to_string()
                 .contains("Reconstruction threshold must be at least"),
@@ -744,23 +727,20 @@ pub mod running_tests {
 
     #[test]
     fn process_new_parameters_proposal__should_accept_valid_per_domain_threshold_update() {
-        // Given a running state with one CaitSith domain at the fixture default
-        // t = 2.
-        // GovernanceThreshold 4 so the proposal's ReconstructionThreshold (3) fits.
+        // Given governance 4 so a ReconstructionThreshold of 3 fits
         let (mut state, mut env) = setup_with_params(1, 5, 4);
         let proposal = gen_valid_params_proposal(&state.parameters);
         env.set_signer(&overlapping_signer(&state, &proposal));
 
-        // When voting with an update raising t to 3, which stays within both the proposed
-        // participant count and the GovernanceThreshold (>= 3 by the pinned params above).
+        // When voting with an update raising t from the fixture default 2 to 3
         let domain_id = state.domains.domains()[0].id;
         let proposal = proposal.with_per_domain_thresholds(BTreeMap::from([(
             domain_id,
             ReconstructionThreshold::new(3),
         )]));
         let res = state.vote_new_parameters(state.keyset.epoch_id.next(), &proposal);
 
-        // Then the vote is recorded without error
+        // Then
         assert!(
             res.is_ok(),
             "Expected accept with valid threshold update: {res:?}"
@@ -770,7 +750,7 @@ pub mod running_tests {
     #[test]
     fn process_new_parameters_proposal__should_reject_threshold_update_exceeding_participant_count()
     {
-        // Given a running state with one domain
+        // Given
         let (mut state, _env) = setup(1);
         let proposal = gen_valid_params_proposal(&state.parameters);
 
@@ -786,7 +766,6 @@ pub mod running_tests {
             .unwrap_err();
 
         // Then the updated value is validated against the proposed participants
-        // and rejected.
         assert!(
             err.to_string().contains("exceeds participant count"),
             "Expected ReconstructionThresholdExceedsParticipants, got: {err}"
@@ -795,9 +774,8 @@ pub mod running_tests {
 
     #[test]
     fn process_new_parameters_proposal__should_accept_threshold_update_diverging_caitsith() {
-        // Given a running state with two CaitSith domains, both at the fixture
-        // default t = 2 (the protocols cycle, so 5 domains yields two CaitSith).
-        // GovernanceThreshold 4 so the proposal's ReconstructionThreshold (3) fits.
+        // Given two CaitSith domains at the fixture default t = 2 (the
+        // protocols cycle, so 5 domains yields two CaitSith)
         let (mut state, mut env) = setup_with_params(5, 5, 4);
         assert!(
             state
@@ -812,8 +790,7 @@ pub mod running_tests {
         let proposal = gen_valid_params_proposal(&state.parameters);
         env.set_signer(&overlapping_signer(&state, &proposal));
 
-        // When an update raises only one CaitSith domain's threshold, leaving the
-        // CaitSith domains non-uniform.
+        // When an update raises only one CaitSith domain's threshold
         let caitsith_id = state
             .domains
             .domains()