Skip to content

refactor: clean up partial_signature.rs validation helpers and tests #1039

Description

@shane-moore

Context

PR #1033 (commit 4d98a50) introduced two pure-function validation helpers in anchor/message_validator/src/partial_signature.rs:

  • validate_validator_index_occurrence_limit(messages, limit)
  • validate_ptc_committee_message_count(count, validator_count)

The PTC arm uses both and the PTC tests are now ~10-20 LOC pure-function asserts. The Committee and AggregatorCommittee arms still inline equivalent loops, and their parallel tests still build full pipelines (~56 LOC and ~152 LOC). Many other tests across the file repeat the same 4-line fixture setup and 5-line validation-call boilerplate. Consolidate while we're already in the file.

Scope

1. Migrate Committee + AggregatorCommittee arms to the occurrence-limit helper

  • Role::Committee arm: replace inline loop (limit > 2) with validate_validator_index_occurrence_limit(..., 2)?.
  • Role::AggregatorCommittee arm: same with limit 5.

Keep per-role rationale comments.

2. Shrink the two heavy occurrence-limit tests

  • test_aggregator_committee_validator_index_occurrence_limit (~152 LOC, 5-vs-6).
  • test_committee_validator_index_exceeds_limit (~56 LOC, 2-vs-3).

Both collapse to ~10-15 LOC pure-function form mirroring the PR #1033 PTC test.

3. Extract a test-fixture bundle helper

The 4-line opening (create_committee_info + generate_test_key_pair + create_operator_pub_keys + create_signed_partial_sig_message) repeats in 18+ tests. Lift into:

struct TestFixture {
    committee_info: CommitteeInfo,
    operator_pub_keys: HashMap<OperatorId, Rsa<Public>>,
    signed_msg: SignedSSVMessage,
    private_key: Rsa<Private>,
}
fn build_test_fixture(role: Role, kind: PartialSignatureKind) -> TestFixture { ... }

Tests opening with the 4-line dance reduce to one line. Shape (struct vs tuple, builder vs direct fn) is implementer's call.

4. Extract a run-validation helper for the common defaults

The 5-line invocation appears in 22 tests:

let result = validate_partial_signature_message(
    validation_context,
    &mut DutyState::new(N),
    Arc::new(MockDutiesProvider { voluntary_exit_duty_count: M }),
);

N = 2 and M = 0 are the common defaults. Extract:

fn run_validation_default<C: SlotClock>(context: ValidationContext<'_, C>) -> Result<...> { /* defaults */ }
fn run_validation<C: SlotClock>(context: ValidationContext<'_, C>, duty_count: u64, voluntary_exit_duty_count: u64) -> Result<...> { /* explicit */ }

TTL tests (use DutyState::new(64)) and the few with non-zero exit count use the explicit version.

5. (Optional) Pull validate_role_for_fork to a pure function

Currently takes &ValidationContext; only reads role, slots_per_epoch, fork_schedule. Refactor to:

pub(crate) fn validate_role_for_fork(
    role: Role,
    slot: Slot,
    slots_per_epoch: u64,
    fork_schedule: &ForkSchedule,
) -> Result<(), ValidationFailure>

Single production caller passes fields explicitly. Fork-gate tests then call it pure-function-style. Skip if scope creep.

Acceptance

  • Committee + AggregatorCommittee arms call validate_validator_index_occurrence_limit.
  • Both heavy occurrence tests collapsed to pure-function form.
  • Test-fixture bundle helper extracted; 18+ sites adopt it.
  • Run-validation helper extracted (default + explicit); 22 sites adopt.
  • make cargo-fmt && make cargo-fmt-check && make lint && cargo test -p message_validator clean.
  • (Optional) validate_role_for_fork pulled pure; fork-gate tests simplified.

Split into multiple PRs if the implementer prefers.

Out of scope

  • Message-count rule generalization (formulas are per-role; no shared body worth extracting).
  • Per-arm rationale comments.
  • Any other validation rule.
  • TTL bucketing for PTC.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions