Skip to content

refactor(message_validator): make ValidationFailure -> MessageAcceptance classification exhaustive #1211

Description

@shane-moore

Context

From<&ValidationFailure> for MessageAcceptance (anchor/message_validator/src/lib.rs) lists the Ignore-class variants explicitly and sends everything else through a catch-all:

    | ValidationFailure::TooManyDistinctSigningRoots { .. } => MessageAcceptance::Ignore,
    _ => MessageAcceptance::Reject,
}

ValidationFailure has ~75 variants and the Ignore arm lists 20, so ~55 variants reach Reject via _, and so does every future variant whose author forgets to classify it. That default is fail-punitive: Reject applies the gossipsub P4 invalid-message penalty to the delivering peer, and the P4 weight is sized against the graylist threshold (anchor/network/src/scoring/topic_score_config.rs), so a small number of misclassified deliveries graylists an honest peer. The cost asymmetry is one-sided: a wrong Ignore drops one message the mesh re-delivers; a wrong Reject debits honest peers and can partition us from them.

This is not theoretical. #1203 adds RelayedDuplicateMessage and had to remember to extend the Ignore arm two hunks away from the variant declaration; omitting that one line would have compiled, passed every variant-asserting test, and reproduced exactly the unwarranted-peer-penalty bug #1131 exists to fix. #1197 is a shipped instance of the same failure mode.

The From impl is the single source of truth for peer punishment: nothing outside anchor/message_validator/src matches ValidationFailure::, and no metrics or labels key on the discriminant.

Proposed change

Remove the _ arm and enumerate the Reject variants explicitly, so the match is exhaustive over ValidationFailure. Every new variant then fails to compile until its author states the acceptance decision, turning a silent absence into a visible line in review. Keep the two arms visually separated with a comment naming the rule (Reject debits peer score; Ignore does not).

Acceptance criteria

  • The From<&ValidationFailure> for MessageAcceptance match has no wildcard arm and compiles exhaustively over ValidationFailure.
  • Every variant currently reaching Reject through the catch-all keeps its existing classification (no behavior change in this PR).
  • A comment on the match states the classification rule so future variants are sorted deliberately.

Notes

  • Base on unstable; epbs picks it up on the next backmerge. The epbs branch adds role-8 variants to the same enum, so landing this before further ePBS validator work maximizes the benefit.

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