From 37c7a5216f8903629c20cfa4bdfa0203541c12a9 Mon Sep 17 00:00:00 2001 From: Gonzalo Othacehe Date: Thu, 10 Jul 2025 16:53:03 -0300 Subject: [PATCH 01/10] up rules --- scripts/solhint-custom/index.js | 64 +++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 9625027eefe..28d5ebf4fef 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -23,11 +23,11 @@ class Base { 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' && node.visibility !== 'external') { + this.error(node, 'Interface functions must be external'); } } }, @@ -47,20 +47,36 @@ module.exports = [ static ruleId = 'leading-underscore'; VariableDeclaration(node) { - 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'); + if (node.isDeclaredConst || node.isImmutable) { + if (/^_/.test(node.name)) { + this.error(node, 'Constant and immutable variables should not have leading underscore'); + } + } + else { + if (node.isStateVar) { + if (node.visibility === 'private' || node.visibility === 'internal') { + if (!/^_/.test(node.name)) { + this.error(node, 'Private and internal state variables must have leading underscore'); + } + } + else { + if (/^_/.test(node.name)) { + this.error(node, 'Public state variables should not have leading underscore'); + } + } } - } 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 (node.visibility === 'private') { if (!/^_/.test(node.name)) { - this.error(node, 'Private and internal functions must have leading underscore'); + this.error(node, 'Private functions must have leading underscore'); + } + } + if (node.visibility === 'internal' && node.parent.kind !== 'library') { + if (!/^_/.test(node.name)) { + this.error(node, 'Non-library internal functions must have leading underscore'); } } if (node.visibility === 'internal' && node.parent.kind === 'library') { @@ -68,17 +84,21 @@ module.exports = [ this.error(node, 'Library internal functions should not have leading underscore'); } } + if (node.visibility === 'public' || node.visibility === 'external') { + if (/^_/.test(node.name)) { + this.error(node, 'Public and external functions should not have leading underscore'); + } + } } }, - // 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' && node.isVirtual) { + this.error(node, 'Functions should not be external and virtual'); + } + } + }, ]; From 6e13ca783a60367f7d23b93b719a548b96586c41 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 23:38:29 +0200 Subject: [PATCH 02/10] Update index.js --- scripts/solhint-custom/index.js | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 28d5ebf4fef..7f6cb5d5b9a 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -47,23 +47,19 @@ module.exports = [ static ruleId = 'leading-underscore'; VariableDeclaration(node) { - if (node.isDeclaredConst || node.isImmutable) { - if (/^_/.test(node.name)) { - this.error(node, 'Constant and immutable variables should not have leading underscore'); - } + if (node.isDeclaredConst || node.isImmutable) { + if (/^_/.test(node.name)) { + this.error(node, 'Constant and immutable variables should not have leading underscore'); } - else { - if (node.isStateVar) { - if (node.visibility === 'private' || node.visibility === 'internal') { - if (!/^_/.test(node.name)) { - this.error(node, 'Private and internal state variables must have leading underscore'); - } - } - else { - if (/^_/.test(node.name)) { - this.error(node, 'Public state variables should not have leading underscore'); - } - } + } else if (node.isStateVar) { + if (node.visibility === 'private' || node.visibility === 'internal') { + if (!/^_/.test(node.name)) { + this.error(node, 'Private and internal state variables must have leading underscore'); + } + } else { + if (/^_/.test(node.name)) { + this.error(node, 'Public state variables should not have leading underscore'); + } } } } From 56b120f607f79a30afa37091e83edf527f724fe3 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 23:40:26 +0200 Subject: [PATCH 03/10] Update index.js --- scripts/solhint-custom/index.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 7f6cb5d5b9a..03a33779e8e 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -65,25 +65,17 @@ module.exports = [ } FunctionDefinition(node) { - if (node.visibility === 'private') { - if (!/^_/.test(node.name)) { - this.error(node, 'Private functions must have leading underscore'); - } + if (node.visibility === 'private' && !/^_/.test(node.name)) { + this.error(node, 'Private functions must have leading underscore'); } - if (node.visibility === 'internal' && node.parent.kind !== 'library') { - if (!/^_/.test(node.name)) { - this.error(node, 'Non-library internal functions must have leading underscore'); - } + if (node.visibility === 'internal' && node.parent.kind !== 'library' && !/^_/.test(node.name)) { + this.error(node, 'Non-library 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'); - } + if (node.visibility === 'internal' && node.parent.kind === 'library' && /^_/.test(node.name)) { + this.error(node, 'Library internal functions should not have leading underscore'); } - if (node.visibility === 'public' || node.visibility === 'external') { - if (/^_/.test(node.name)) { - this.error(node, 'Public and external functions should not have leading underscore'); - } + if (node.visibility === 'public' || node.visibility === 'external' && /^_/.test(node.name)) { + this.error(node, 'Public and external functions should not have leading underscore'); } } }, From ba351f3211ed3d990aa1c822426966c41d13c7bd Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Jul 2025 23:41:44 +0200 Subject: [PATCH 04/10] Update index.js --- scripts/solhint-custom/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 03a33779e8e..a586790aadd 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -74,8 +74,11 @@ module.exports = [ if (node.visibility === 'internal' && node.parent.kind === 'library' && /^_/.test(node.name)) { this.error(node, 'Library internal functions should not have leading underscore'); } - if (node.visibility === 'public' || node.visibility === 'external' && /^_/.test(node.name)) { - this.error(node, 'Public and external functions should not have leading underscore'); + if (node.visibility === 'public' && /^_/.test(node.name)) { + this.error(node, 'Public functions should not have leading underscore'); + } + if (node.visibility === 'external' && /^_/.test(node.name)) { + this.error(node, 'External functions should not have leading underscore'); } } }, From bee7bc3b497857e2a182b379900c8b396b8d80bc Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 16 Jul 2025 22:31:26 +0200 Subject: [PATCH 05/10] Update index.js --- scripts/solhint-custom/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index a586790aadd..6718ab7b858 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -52,14 +52,14 @@ module.exports = [ this.error(node, 'Constant and immutable variables should not have leading underscore'); } } else if (node.isStateVar) { - if (node.visibility === 'private' || node.visibility === 'internal') { - if (!/^_/.test(node.name)) { - this.error(node, 'Private and internal state variables must have leading underscore'); - } - } else { - if (/^_/.test(node.name)) { - this.error(node, 'Public state variables should not have leading underscore'); - } + if (node.visibility === 'private' && !/^_/.test(node.name)) + this.error(node, 'Private state variables must have leading underscore'); + } + if (node.visibility === 'internal' && !/^_/.test(node.name)) { + this.error(node, 'Internal state variables must have leading underscore'); + } + if (node.visibility === 'public' && /^_/.test(node.name)) { + this.error(node, 'Public state variables should not have leading underscore'); } } } From f859dd7933f7a1e6fc18ac6c7c129e24ef2497b2 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 16 Jul 2025 22:33:10 +0200 Subject: [PATCH 06/10] Update index.js --- scripts/solhint-custom/index.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 6718ab7b858..060ee81d6a3 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -47,20 +47,20 @@ module.exports = [ static ruleId = 'leading-underscore'; VariableDeclaration(node) { - if (node.isDeclaredConst || node.isImmutable) { - if (/^_/.test(node.name)) { - this.error(node, 'Constant and immutable variables should not have leading underscore'); - } - } else if (node.isStateVar) { - if (node.visibility === 'private' && !/^_/.test(node.name)) - this.error(node, 'Private state variables must have leading underscore'); - } - if (node.visibility === 'internal' && !/^_/.test(node.name)) { - this.error(node, 'Internal state variables must have leading underscore'); - } - if (node.visibility === 'public' && /^_/.test(node.name)) { - this.error(node, 'Public state variables should not have leading underscore'); - } + if (node.isDeclaredConst && /^_/.test(node.name)) { + this.error(node, 'Constant variables should not have leading underscore'); + } + if (node.isImmutable && /^_/.test(node.name)) { + this.error(node, 'Immutable variables should not have leading underscore'); + } + if (node.isStateVar && node.visibility === 'private' && !/^_/.test(node.name)) { + this.error(node, 'Private state variables must have leading underscore'); + } + if (node.isStateVar && node.visibility === 'internal' && !/^_/.test(node.name)) { + this.error(node, 'Internal state variables must have leading underscore'); + } + if (node.isStateVar && node.visibility === 'public' && /^_/.test(node.name)) { + this.error(node, 'Public state variables should not have leading underscore'); } } From d807d85c14ea35ac533bfc46d5453d65c9ee9d79 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 16 Jul 2025 22:36:28 +0200 Subject: [PATCH 07/10] Update index.js --- scripts/solhint-custom/index.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 060ee81d6a3..5fe96ee1b3b 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -36,8 +36,7 @@ module.exports = [ static ruleId = 'private-variables'; VariableDeclaration(node) { - const constantOrImmutable = node.isDeclaredConst || node.isImmutable; - if (node.isStateVar && !constantOrImmutable && node.visibility !== 'private') { + if (node.isStateVar && !node.isDeclaredConst && !node.isImmutable && node.visibility !== 'private') { this.error(node, 'State variables must be private'); } } @@ -47,37 +46,37 @@ module.exports = [ static ruleId = 'leading-underscore'; VariableDeclaration(node) { - if (node.isDeclaredConst && /^_/.test(node.name)) { + if (node.isDeclaredConst && node.name.startsWith('_')) { this.error(node, 'Constant variables should not have leading underscore'); } - if (node.isImmutable && /^_/.test(node.name)) { + if (node.isImmutable && node.name.startsWith('_')) { this.error(node, 'Immutable variables should not have leading underscore'); } - if (node.isStateVar && node.visibility === 'private' && !/^_/.test(node.name)) { + if (node.isStateVar && node.visibility === 'private' && !node.name.startsWith('_')) { this.error(node, 'Private state variables must have leading underscore'); } - if (node.isStateVar && node.visibility === 'internal' && !/^_/.test(node.name)) { + if (node.isStateVar && node.visibility === 'internal' && !node.name.startsWith('_')) { this.error(node, 'Internal state variables must have leading underscore'); } - if (node.isStateVar && node.visibility === 'public' && /^_/.test(node.name)) { + if (node.isStateVar && node.visibility === 'public' && node.name.startsWith('_')) { this.error(node, 'Public state variables should not have leading underscore'); } } FunctionDefinition(node) { - if (node.visibility === 'private' && !/^_/.test(node.name)) { + if (node.visibility === 'private' && !node.name.startsWith('_')) { this.error(node, 'Private functions must have leading underscore'); } - if (node.visibility === 'internal' && node.parent.kind !== 'library' && !/^_/.test(node.name)) { + if (node.visibility === 'internal' && node.parent.kind !== 'library' && !node.name.startsWith('_')) { this.error(node, 'Non-library internal functions must have leading underscore'); } - if (node.visibility === 'internal' && node.parent.kind === 'library' && /^_/.test(node.name)) { + if (node.visibility === 'internal' && node.parent.kind === 'library' && node.name.startsWith('_')) { this.error(node, 'Library internal functions should not have leading underscore'); } - if (node.visibility === 'public' && /^_/.test(node.name)) { + if (node.visibility === 'public' && node.name.startsWith('_')) { this.error(node, 'Public functions should not have leading underscore'); } - if (node.visibility === 'external' && /^_/.test(node.name)) { + if (node.visibility === 'external' && node.name.startsWith('_')) { this.error(node, 'External functions should not have leading underscore'); } } From add0ff0fbd91a087855f5ff1d84cf11b21faf6e1 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 17 Jul 2025 21:37:05 +0200 Subject: [PATCH 08/10] fix lint --- scripts/solhint-custom/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index 5fe96ee1b3b..cfd6dc994a1 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -84,7 +84,7 @@ module.exports = [ 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'); From 101f119aa4e6a6c4258c09a9f78653ea41d0e21b Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 24 Jul 2025 16:08:52 +0200 Subject: [PATCH 09/10] improve --- scripts/solhint-custom/index.js | 87 +++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js index cfd6dc994a1..39b99be5086 100644 --- a/scripts/solhint-custom/index.js +++ b/scripts/solhint-custom/index.js @@ -14,8 +14,8 @@ class Base { } } - error(node, message) { - if (!this.ignored) { + require(condition, node, message) { + if (!condition && !this.ignored) { this.reporter.error(node, this.ruleId, message); } } @@ -26,8 +26,8 @@ module.exports = [ static ruleId = 'interface-only-external-functions'; FunctionDefinition(node) { - if (node.parent.kind === 'interface' && node.visibility !== 'external') { - this.error(node, 'Interface functions must be external'); + if (node.parent.kind === 'interface') { + this.require(node.visibility === 'external', node, 'Interface functions must be external'); } } }, @@ -36,8 +36,12 @@ module.exports = [ static ruleId = 'private-variables'; VariableDeclaration(node) { - if (node.isStateVar && !node.isDeclaredConst && !node.isImmutable && 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', + ); } } }, @@ -46,38 +50,45 @@ module.exports = [ static ruleId = 'leading-underscore'; VariableDeclaration(node) { - if (node.isDeclaredConst && node.name.startsWith('_')) { - this.error(node, 'Constant variables should not have leading underscore'); - } - if (node.isImmutable && node.name.startsWith('_')) { - this.error(node, 'Immutable variables should not have leading underscore'); - } - if (node.isStateVar && node.visibility === 'private' && !node.name.startsWith('_')) { - this.error(node, 'Private state variables must have leading underscore'); - } - if (node.isStateVar && node.visibility === 'internal' && !node.name.startsWith('_')) { - this.error(node, 'Internal state variables must have leading underscore'); - } - if (node.isStateVar && node.visibility === 'public' && node.name.startsWith('_')) { - this.error(node, 'Public state 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': + 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; + } } } FunctionDefinition(node) { - if (node.visibility === 'private' && !node.name.startsWith('_')) { - this.error(node, 'Private functions must have leading underscore'); - } - if (node.visibility === 'internal' && node.parent.kind !== 'library' && !node.name.startsWith('_')) { - this.error(node, 'Non-library internal functions must have leading underscore'); - } - if (node.visibility === 'internal' && node.parent.kind === 'library' && node.name.startsWith('_')) { - this.error(node, 'Library internal functions should not have leading underscore'); - } - if (node.visibility === 'public' && node.name.startsWith('_')) { - this.error(node, 'Public functions should not have leading underscore'); - } - if (node.visibility === 'external' && node.name.startsWith('_')) { - this.error(node, 'External 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; } } }, @@ -86,8 +97,12 @@ module.exports = [ static ruleId = 'no-external-virtual'; FunctionDefinition(node) { - if (node.visibility == 'external' && node.isVirtual) { - this.error(node, 'Functions should not be external and virtual'); + if (node.visibility == 'external') { + this.require( + node.isReceiveEther || node.isFallback || !node.isVirtual, + node, + 'Functions should not be external and virtual', + ); } } }, From ca766d7a161ddabdeaa07eda67d1aa57657d7eab Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 24 Jul 2025 16:18:47 +0200 Subject: [PATCH 10/10] fix code to match new rules --- contracts/governance/Governor.sol | 2 +- contracts/governance/TimelockController.sol | 8 ++++---- .../governance/extensions/GovernorTimelockCompound.sol | 4 ++-- .../governance/extensions/GovernorTimelockControl.sol | 2 +- .../governance/extensions/GovernorVotesQuorumFraction.sol | 2 +- contracts/metatx/ERC2771Forwarder.sol | 4 ++-- contracts/mocks/proxy/UUPSUpgradeableMock.sol | 2 +- contracts/proxy/utils/UUPSUpgradeable.sol | 2 +- contracts/token/ERC20/extensions/ERC20Permit.sol | 2 +- contracts/utils/Base64.sol | 8 ++++---- contracts/utils/Multicall.sol | 2 +- scripts/solhint-custom/index.js | 7 +++++-- test/metatx/ERC2771Forwarder.t.sol | 2 +- 13 files changed, 25 insertions(+), 22 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 5a7b1617041..c80a82f5b0b 100644 --- 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 index e608d393d98..93996cc9735 100644 --- 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 index ea705a76e43..e8572e219b6 100644 --- 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 index 6aab59c9470..ff963adffaa 100644 --- 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 index 5abd7c129b0..c225fe85fa9 100644 --- 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 index a463e70e019..b1448acf084 100644 --- 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 index 8c5641e6ca3..b5e8f506ce4 100644 --- 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 index d0f58427f9b..7717637fd1b 100644 --- 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 index d593191966a..fc213415112 100644 --- 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 index c6ee6a524aa..4f3e493859e 100644 --- a/contracts/utils/Base64.sol +++ b/contracts/utils/Base64.sol @@ -11,14 +11,14 @@ 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); } /** @@ -26,7 +26,7 @@ library Base64 { * 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 index 94222feb0f4..c986c3c7cde 100644 --- 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 index 39b99be5086..8a86f9aa320 100644 --- 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 index 605ae62097e..224367377ff 100644 --- 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,