test: lint for functions that are virtual and override (#7086)

Co-authored-by: nambrot <[email protected]>

Author: yorhodes

.changeset/many-stingrays-invite.md

@@ -0,0 +1,5 @@
+---
+"@hyperlane-xyz/core": patch
+---
+
+Remove majority of virtual override functions

package.json

@@ -40,6 +40,7 @@
     "postinstall": "husky install"
   },
   "workspaces": [
+    "solhint-plugin",
     "solidity",
     "typescript/*",
     "starknet"

solhint-plugin/index.js

@@ -0,0 +1,48 @@
+// https://protofire.github.io/solhint/docs/writing-plugins.html
+class NoVirtualOverrideAllowed {
+  constructor(reporter, config) {
+    this.ruleId = 'no-virtual-override';
+
+    this.reporter = reporter;
+    this.config = config;
+  }
+
+  FunctionDefinition(ctx) {
+    const isVirtual = ctx.isVirtual;
+    const hasOverride = ctx.override !== null;
+
+    if (isVirtual && hasOverride) {
+      this.reporter.error(
+        ctx,
+        this.ruleId,
+        'Functions cannot be "virtual" and "override" at the same time',
+      );
+    }
+  }
+}
+
+class NoVirtualInitializerAllowed {
+  constructor(reporter, config) {
+    this.ruleId = 'no-virtual-initializer';
+
+    this.reporter = reporter;
+    this.config = config;
+  }
+
+  FunctionDefinition(ctx) {
+    const isVirtual = ctx.isVirtual;
+    const hasInitializer = ctx.modifiers.some(
+      (modifier) => modifier.name === 'initializer',
+    );
+
+    if (isVirtual && hasInitializer) {
+      this.reporter.error(
+        ctx,
+        this.ruleId,
+        'Functions cannot be "virtual" and "initializer" at the same time',
+      );
+    }
+  }
+}
+
+module.exports = [NoVirtualOverrideAllowed, NoVirtualInitializerAllowed];

solhint-plugin/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "solhint-plugin-hyperlane",
+  "private": true,
+  "version": "0.0.0",
+  "description": "",
+  "license": "Apache-2.0",
+  "type": "commonjs",
+  "main": "index.js"
+}

solidity/.solhint.json

@@ -12,7 +12,9 @@
     "reason-string": ["warn", { "maxLength": 64 }],
     "prettier/prettier": "error",
     "gas-custom-errors": "off",
-    "named-parameters-mapping": "error"
+    "named-parameters-mapping": "error",
+    "hyperlane/no-virtual-override": "error",
+    "hyperlane/no-virtual-initializer": "error"
   },
-  "plugins": ["prettier"]
+  "plugins": ["prettier", "hyperlane"]
 }

solidity/contracts/avs/HyperlaneServiceManager.sol

@@ -286,9 +286,7 @@ contract HyperlaneServiceManager is ECDSAServiceManagerBase, PackageVersioned {
     }
 
     /// @inheritdoc ECDSAServiceManagerBase
-    function _deregisterOperatorFromAVS(
-        address operator
-    ) internal virtual override {
+    function _deregisterOperatorFromAVS(address operator) internal override {
         address[] memory challengers = getOperatorChallengers(operator);
         _completeUnenrollment(operator, challengers);
 

solidity/contracts/client/Router.sol

@@ -93,6 +93,7 @@ abstract contract Router is MailboxClient, IMessageRecipient {
      * @param _sender The sender address
      * @param _message The message
      */
+    // solhint-disable-next-line hyperlane/no-virtual-override
     function handle(
         uint32 _origin,
         bytes32 _sender,

solidity/contracts/hooks/DefaultHook.sol

@@ -24,14 +24,14 @@ contract DefaultHook is AbstractPostDispatchHook, MailboxClient {
     function _quoteDispatch(
         bytes calldata metadata,
         bytes calldata message
-    ) internal view virtual override returns (uint256) {
+    ) internal view override returns (uint256) {
         return _hook().quoteDispatch(metadata, message);
     }
 
     function _postDispatch(
         bytes calldata metadata,
         bytes calldata message
-    ) internal virtual override {
+    ) internal override {
         _hook().postDispatch{value: msg.value}(metadata, message);
     }
 }

solidity/contracts/hooks/aggregation/StaticAggregationHookFactory.sol

@@ -18,12 +18,7 @@ import {StaticAggregationHook} from "./StaticAggregationHook.sol";
 import {StaticAddressSetFactory} from "../../libs/StaticAddressSetFactory.sol";
 
 contract StaticAggregationHookFactory is StaticAddressSetFactory {
-    function _deployImplementation()
-        internal
-        virtual
-        override
-        returns (address)
-    {
+    function _deployImplementation() internal override returns (address) {
         return address(new StaticAggregationHook());
     }
 }

solidity/contracts/hooks/igp/InterchainGasPaymaster.sol

@@ -192,6 +192,7 @@ contract InterchainGasPaymaster is
      * @param _gasLimit The amount of destination gas to pay for.
      * @return The amount of native tokens required to pay for interchain gas.
      */
+    // solhint-disable-next-line hyperlane/no-virtual-override
     function quoteGasPayment(
         uint32 _destinationDomain,
         uint256 _gasLimit