Update OZ solhint custom rules

contracts/governance/Governor.sol

@@ -653,7 +653,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
      * in a governance proposal to recover tokens or Ether that was sent to the governor contract by mistake.
      * Note that if the executor is simply the governor itself, use of `relay` is redundant.
      */
-    function relay(address target, uint256 value, bytes calldata data) external payable virtual onlyGovernance {
+    function relay(address target, uint256 value, bytes calldata data) public payable virtual onlyGovernance {
         (bool success, bytes memory returndata) = target.call{value: value}(data);
         Address.verifyCallResult(success, returndata);
     }

contracts/governance/TimelockController.sol

@@ -26,7 +26,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
     bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
     bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
     bytes32 public constant CANCELLER_ROLE = keccak256("CANCELLER_ROLE");
-    uint256 internal constant _DONE_TIMESTAMP = uint256(1);
+    uint256 internal constant DONE_TIMESTAMP = uint256(1);
 
     mapping(bytes32 id => uint256) private _timestamps;
     uint256 private _minDelay;
@@ -207,7 +207,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
         uint256 timestamp = getTimestamp(id);
         if (timestamp == 0) {
             return OperationState.Unset;
-        } else if (timestamp == _DONE_TIMESTAMP) {
+        } else if (timestamp == DONE_TIMESTAMP) {
             return OperationState.Done;
         } else if (timestamp > block.timestamp) {
             return OperationState.Waiting;
@@ -432,7 +432,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
         if (!isOperationReady(id)) {
             revert TimelockUnexpectedOperationState(id, _encodeStateBitmap(OperationState.Ready));
         }
-        _timestamps[id] = _DONE_TIMESTAMP;
+        _timestamps[id] = DONE_TIMESTAMP;
     }
 
     /**
@@ -445,7 +445,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
      * - the caller must be the timelock itself. This can only be achieved by scheduling and later executing
      * an operation where the timelock is the target and the data is the ABI-encoded call to this function.
      */
-    function updateDelay(uint256 newDelay) external virtual {
+    function updateDelay(uint256 newDelay) public virtual {
         address sender = _msgSender();
         if (sender != address(this)) {
             revert TimelockUnauthorizedCaller(sender);

contracts/governance/extensions/GovernorTimelockCompound.sol

@@ -136,7 +136,7 @@ abstract contract GovernorTimelockCompound is Governor {
     /**
      * @dev Accept admin right over the timelock.
      */
-    // solhint-disable-next-line private-vars-leading-underscore
+    // solhint-disable-next-line openzeppelin/leading-underscore
     function __acceptAdmin() public {
         _timelock.acceptAdmin();
     }
@@ -154,7 +154,7 @@ abstract contract GovernorTimelockCompound is Governor {
 
      * CAUTION: It is not recommended to change the timelock while there are other queued governance proposals.
      */
-    function updateTimelock(ICompoundTimelock newTimelock) external virtual onlyGovernance {
+    function updateTimelock(ICompoundTimelock newTimelock) public virtual onlyGovernance {
         _updateTimelock(newTimelock);
     }
 

contracts/governance/extensions/GovernorTimelockControl.sol

@@ -146,7 +146,7 @@ abstract contract GovernorTimelockControl is Governor {
      *
      * CAUTION: It is not recommended to change the timelock while there are other queued governance proposals.
      */
-    function updateTimelock(TimelockController newTimelock) external virtual onlyGovernance {
+    function updateTimelock(TimelockController newTimelock) public virtual onlyGovernance {
         _updateTimelock(newTimelock);
     }
 

contracts/governance/extensions/GovernorVotesQuorumFraction.sol

@@ -73,7 +73,7 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
      * - Must be called through a governance proposal.
      * - New numerator must be smaller or equal to the denominator.
      */
-    function updateQuorumNumerator(uint256 newQuorumNumerator) external virtual onlyGovernance {
+    function updateQuorumNumerator(uint256 newQuorumNumerator) public virtual onlyGovernance {
         _updateQuorumNumerator(newQuorumNumerator);
     }
 

contracts/metatx/ERC2771Forwarder.sol

@@ -61,7 +61,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         bytes signature;
     }
 
-    bytes32 internal constant _FORWARD_REQUEST_TYPEHASH =
+    bytes32 internal constant FORWARD_REQUEST_TYPEHASH =
         keccak256(
             "ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,uint48 deadline,bytes data)"
         );
@@ -222,7 +222,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         (address recovered, ECDSA.RecoverError err, ) = _hashTypedDataV4(
             keccak256(
                 abi.encode(
-                    _FORWARD_REQUEST_TYPEHASH,
+                    FORWARD_REQUEST_TYPEHASH,
                     request.from,
                     request.to,
                     request.value,

contracts/mocks/proxy/UUPSUpgradeableMock.sol

@@ -29,7 +29,7 @@ contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {
 }
 
 contract UUPSUnsupportedProxiableUUID is UUPSUpgradeableMock {
-    function proxiableUUID() external pure override returns (bytes32) {
+    function proxiableUUID() public pure override returns (bytes32) {
         return keccak256("invalid UUID");
     }
 }

contracts/proxy/utils/UUPSUpgradeable.sol

@@ -69,7 +69,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
      * bricking a proxy that upgrades to it, by delegating to itself until out of gas. Thus it is critical that this
      * function revert if invoked through a proxy. This is guaranteed by the `notDelegated` modifier.
      */
-    function proxiableUUID() external view virtual notDelegated returns (bytes32) {
+    function proxiableUUID() public view virtual notDelegated returns (bytes32) {
         return ERC1967Utils.IMPLEMENTATION_SLOT;
     }
 

contracts/token/ERC20/extensions/ERC20Permit.sol

@@ -71,7 +71,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
 
     /// @inheritdoc IERC20Permit
     // solhint-disable-next-line func-name-mixedcase
-    function DOMAIN_SEPARATOR() external view virtual returns (bytes32) {
+    function DOMAIN_SEPARATOR() public view virtual returns (bytes32) {
         return _domainSeparatorV4();
     }
 }

contracts/utils/Base64.sol

@@ -11,22 +11,22 @@ library Base64 {
      * @dev Base64 Encoding/Decoding Table
      * See sections 4 and 5 of https://datatracker.ietf.org/doc/html/rfc4648
      */
-    string internal constant _TABLE = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
-    string internal constant _TABLE_URL = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
+    string internal constant TABLE = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+    string internal constant TABLE_URL = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
 
     /**
      * @dev Converts a `bytes` to its Bytes64 `string` representation.
      */
     function encode(bytes memory data) internal pure returns (string memory) {
-        return _encode(data, _TABLE, true);
+        return _encode(data, TABLE, true);
     }
 
     /**
      * @dev Converts a `bytes` to its Bytes64Url `string` representation.
      * Output is not padded with `=` as specified in https://www.rfc-editor.org/rfc/rfc4648[rfc4648].
      */
     function encodeURL(bytes memory data) internal pure returns (string memory) {
-        return _encode(data, _TABLE_URL, false);
+        return _encode(data, TABLE_URL, false);
     }
 
     /**

contracts/utils/Multicall.sol

@@ -23,7 +23,7 @@ abstract contract Multicall is Context {
      * @dev Receives and executes a batch of function calls on this contract.
      * @custom:oz-upgrades-unsafe-allow-reachable delegatecall
      */
-    function multicall(bytes[] calldata data) external virtual returns (bytes[] memory results) {
+    function multicall(bytes[] calldata data) public virtual returns (bytes[] memory results) {
         bytes memory context = msg.sender == _msgSender()
             ? new bytes(0)
             : msg.data[msg.data.length - _contextSuffixLength():];

scripts/solhint-custom/index.js

@@ -14,20 +14,20 @@ class Base {
     }
   }
 
-  error(node, message) {
-    if (!this.ignored) {
+  require(condition, node, message) {
+    if (!condition && !this.ignored) {
       this.reporter.error(node, this.ruleId, message);
     }
   }
 }
 
 module.exports = [
   class extends Base {
-    static ruleId = 'interface-names';
+    static ruleId = 'interface-only-external-functions';
 
-    ContractDefinition(node) {
-      if (node.kind === 'interface' && !/^I[A-Z]/.test(node.name)) {
-        this.error(node, 'Interface names should have a capital I prefix');
+    FunctionDefinition(node) {
+      if (node.parent.kind === 'interface') {
+        this.require(node.visibility === 'external', node, 'Interface functions must be external');
       }
     }
   },
@@ -36,9 +36,12 @@ module.exports = [
     static ruleId = 'private-variables';
 
     VariableDeclaration(node) {
-      const constantOrImmutable = node.isDeclaredConst || node.isImmutable;
-      if (node.isStateVar && !constantOrImmutable && node.visibility !== 'private') {
-        this.error(node, 'State variables must be private');
+      if (node.isStateVar) {
+        this.require(
+          node.isDeclaredConst || node.isImmutable || node.visibility === 'private',
+          node,
+          'State variables must be private',
+        );
       }
     }
   },
@@ -47,38 +50,63 @@ module.exports = [
     static ruleId = 'leading-underscore';
 
     VariableDeclaration(node) {
+      // TODO: do we want that rule ? Should no immutable variable have a prefix regardless of visibility ?
+      //
+      // else if (node.isImmutable) {
+      //   this.require(!node.name.startsWith('_'), node, 'Immutable variables should not have leading underscore');
+      // }
       if (node.isDeclaredConst) {
-        // TODO: expand visibility and fix
-        if (node.visibility === 'private' && /^_/.test(node.name)) {
-          this.error(node, 'Constant variables should not have leading underscore');
+        this.require(!node.name.startsWith('_'), node, 'Constant variables should not have leading underscore');
+      } else if (node.isStateVar) {
+        switch (node.visibility) {
+          case 'private':
+            this.require(node.name.startsWith('_'), node, 'Private state variables must have leading underscore');
+            break;
+          case 'internal':
+            this.require(node.name.startsWith('_'), node, 'Internal state variables must have leading underscore');
+            break;
+          case 'public':
+            this.require(!node.name.startsWith('_'), node, 'Public state variables should not have leading underscore');
+            break;
         }
-      } else if (node.visibility === 'private' && !/^_/.test(node.name)) {
-        this.error(node, 'Non-constant private variables must have leading underscore');
       }
     }
 
     FunctionDefinition(node) {
-      if (node.visibility === 'private' || (node.visibility === 'internal' && node.parent.kind !== 'library')) {
-        if (!/^_/.test(node.name)) {
-          this.error(node, 'Private and internal functions must have leading underscore');
-        }
-      }
-      if (node.visibility === 'internal' && node.parent.kind === 'library') {
-        if (/^_/.test(node.name)) {
-          this.error(node, 'Library internal functions should not have leading underscore');
-        }
+      switch (node.visibility) {
+        case 'external':
+          this.require(!node.name.startsWith('_'), node, 'External functions should not have leading underscore');
+          break;
+        case 'public':
+          this.require(!node.name.startsWith('_'), node, 'Public functions should not have leading underscore');
+          break;
+        case 'internal':
+          this.require(
+            node.name.startsWith('_') !== (node.parent.kind === 'library'),
+            node,
+            node.parent.kind === 'library'
+              ? 'Library internal functions should not have leading underscore'
+              : 'Non-library internal functions must have leading underscore',
+          );
+          break;
+        case 'private':
+          this.require(node.name.startsWith('_'), node, 'Private functions must have leading underscore');
+          break;
       }
     }
   },
 
-  // TODO: re-enable and fix
-  // class extends Base {
-  //   static ruleId = 'no-external-virtual';
-  //
-  //   FunctionDefinition(node) {
-  //     if (node.visibility == 'external' && node.isVirtual) {
-  //       this.error(node, 'Functions should not be external and virtual');
-  //     }
-  //   }
-  // },
+  class extends Base {
+    static ruleId = 'no-external-virtual';
+
+    FunctionDefinition(node) {
+      if (node.visibility == 'external') {
+        this.require(
+          node.isReceiveEther || node.isFallback || !node.isVirtual,
+          node,
+          'Functions should not be external and virtual',
+        );
+      }
+    }
+  },
 ];

test/metatx/ERC2771Forwarder.t.sol

@@ -27,7 +27,7 @@ contract ERC2771ForwarderMock is ERC2771Forwarder {
             _hashTypedDataV4(
                 keccak256(
                     abi.encode(
-                        _FORWARD_REQUEST_TYPEHASH,
+                        FORWARD_REQUEST_TYPEHASH,
                         request.from,
                         request.to,
                         request.value,