fix code to match new rules

Amxx · Amxx · commit ca766d7a161d · 2025-07-24T16:51:03.000+02:00
diff --git a/contracts/governance/Governor.sol b/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);
     }
diff --git a/contracts/governance/TimelockController.sol b/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);
diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/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);
     }
 
diff --git a/contracts/governance/extensions/GovernorTimelockControl.sol b/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);
     }
 
diff --git a/contracts/governance/extensions/GovernorVotesQuorumFraction.sol b/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);
     }
 
diff --git a/contracts/metatx/ERC2771Forwarder.sol b/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,
diff --git a/contracts/mocks/proxy/UUPSUpgradeableMock.sol b/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");
     }
 }
diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/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;
     }
 
diff --git a/contracts/token/ERC20/extensions/ERC20Permit.sol b/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();
     }
 }
diff --git a/contracts/utils/Base64.sol b/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);
     }
 
     /**
diff --git a/contracts/utils/Multicall.sol b/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():];
diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js
@@ -50,10 +50,13 @@ 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) {
         this.require(!node.name.startsWith('_'), node, 'Constant variables should not have leading underscore');
-      } else if (node.isImmutable) {
-        this.require(!node.name.startsWith('_'), node, 'Immutable variables should not have leading underscore');
       } else if (node.isStateVar) {
         switch (node.visibility) {
           case 'private':
diff --git a/test/metatx/ERC2771Forwarder.t.sol b/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,

Original file line number	Diff line number	Diff line change
`@@ -653,7 +653,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72`
`653`	`653`	`* in a governance proposal to recover tokens or Ether that was sent to the governor contract by mistake.`
`654`	`654`	* Note that if the executor is simply the governor itself, use of `relay` is redundant.
`655`	`655`	`*/`
`656`		`- function relay(address target, uint256 value, bytes calldata data) external payable virtual onlyGovernance {`
	`656`	`+ function relay(address target, uint256 value, bytes calldata data) public payable virtual onlyGovernance {`
`657`	`657`	`(bool success, bytes memory returndata) = target.call{value: value}(data);`
`658`	`658`	`Address.verifyCallResult(success, returndata);`
`659`	`659`	`}`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,7 @@ abstract contract GovernorTimelockControl is Governor {`
`146`	`146`	`*`
`147`	`147`	`* CAUTION: It is not recommended to change the timelock while there are other queued governance proposals.`
`148`	`148`	`*/`
`149`		`- function updateTimelock(TimelockController newTimelock) external virtual onlyGovernance {`
	`149`	`+ function updateTimelock(TimelockController newTimelock) public virtual onlyGovernance {`
`150`	`150`	`_updateTimelock(newTimelock);`
`151`	`151`	`}`
`152`	`152`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {`
`73`	`73`	`* - Must be called through a governance proposal.`
`74`	`74`	`* - New numerator must be smaller or equal to the denominator.`
`75`	`75`	`*/`
`76`		`- function updateQuorumNumerator(uint256 newQuorumNumerator) external virtual onlyGovernance {`
	`76`	`+ function updateQuorumNumerator(uint256 newQuorumNumerator) public virtual onlyGovernance {`
`77`	`77`	`_updateQuorumNumerator(newQuorumNumerator);`
`78`	`78`	`}`
`79`	`79`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`contract UUPSUnsupportedProxiableUUID is UUPSUpgradeableMock {`
`32`		`- function proxiableUUID() external pure override returns (bytes32) {`
	`32`	`+ function proxiableUUID() public pure override returns (bytes32) {`
`33`	`33`	`return keccak256("invalid UUID");`
`34`	`34`	`}`
`35`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {`
`69`	`69`	`* bricking a proxy that upgrades to it, by delegating to itself until out of gas. Thus it is critical that this`
`70`	`70`	* function revert if invoked through a proxy. This is guaranteed by the `notDelegated` modifier.
`71`	`71`	`*/`
`72`		`- function proxiableUUID() external view virtual notDelegated returns (bytes32) {`
	`72`	`+ function proxiableUUID() public view virtual notDelegated returns (bytes32) {`
`73`	`73`	`return ERC1967Utils.IMPLEMENTATION_SLOT;`
`74`	`74`	`}`
`75`	`75`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {`
`71`	`71`
`72`	`72`	`/// @inheritdoc IERC20Permit`
`73`	`73`	`// solhint-disable-next-line func-name-mixedcase`
`74`		`- function DOMAIN_SEPARATOR() external view virtual returns (bytes32) {`
	`74`	`+ function DOMAIN_SEPARATOR() public view virtual returns (bytes32) {`
`75`	`75`	`return _domainSeparatorV4();`
`76`	`76`	`}`
`77`	`77`	`}`