EVM Engineering: semgrep rule for require messages

[smartcontracts]

We're moving most of the codebase to custom error messages but there will still be plenty of places like scripts and tests where require gets used instead.

@jsvisa has a nice PR open (#12702) that does some of the work to make sure that all instances of require have a corresponding error message.

We should have a semgrep rule that enforces this condition so that we don't end up adding new require statements that don't have messages.

Tasks for this issue are:

• [ ] Add semgrep rule that enforces the format require(..., $MSG)
• [ ] Add tests for the semgrep rule
• [ ] Apply semgrep rule to all files and fix any open instances

When adding the semgrep rule, something along the lines of the following should work:

  - id: sol-style-enforce-require-msg
    languages: [solidity]
    severity: ERROR
    message: Require statement must have an error message
    patterns:
      - pattern: require($ERR);
      - pattern-not: require($ERR, $MSG);

Tests for the semgrep rule should be added in semgrep/sol-rules.t.sol.

For the sake of keeping PRs clean, I generally recommend creating the rule + tests in one PR while excluding files/folders where the rule is violated so that no changes to the contracts are actually required. Afterwards, a second PR can remove the exclusions and fix all of the violations.

[jsvisa]

Sounds good to me, I'll take this one

[smartcontracts]

Awesome, thank you!

[ControlCplusControlV]

I mentioned it in the design-doc for custom errors, but do we actually just want a semgrep rule to blanket outlaw require and then use assertEq and assert in tests/scripts

[smartcontracts]

I mentioned it in the design-doc for custom errors, but do we actually just want a semgrep rule to blanket outlaw require and then use assertEq and assert in tests/scripts

Yes