chores: fix compiler warnings; fix solhint warnings; add build to pre…

…commit (#93)

* chore: remove global import warnings

* chore: fix 'Explicit mark state visibility' warnings

* chore: use capslock for constant names

* chore: Use camelCase for Contract name

* chore: use mixedCase for function name

* chore: fix unused variables

* chore: fix avoid low level calls

* chore: fix solhint no-inline-assembly

* chore: fix solhint no empty blocks. Could be better once protofire/solhint#418 gets merged

* chore: fix solhint max-states-count

* chore: fix compilation warnings

* chore: add forge build to pre-commit

* chore: add --deny-warnings to CI

* fix: remove --deny-warnings from husky

---------

Co-authored-by: Orland0x <37511817+Orland0x@users.noreply.github.com>

.forge-snapshots/ProposeWithSig.snap

@@ -1 +1 @@
-272375
+272388

.forge-snapshots/ProposeWithTx.snap

@@ -1 +1 @@
-262066
+262042

src/Space.sol

@@ -1,13 +1,13 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.18;
 
-import "@openzeppelin/contracts/access/Ownable.sol";
-import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
+import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
+import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 
-import "src/interfaces/ISpace.sol";
-import "src/types.sol";
-import "src/interfaces/IVotingStrategy.sol";
-import "src/interfaces/IExecutionStrategy.sol";
+import { ISpace } from "src/interfaces/ISpace.sol";
+import { Choice, FinalizationStatus, IndexedStrategy, Proposal, ProposalStatus, Strategy, Vote } from "src/types.sol";
+import { IVotingStrategy } from "src/interfaces/IVotingStrategy.sol";
+import { IExecutionStrategy } from "src/interfaces/IExecutionStrategy.sol";
 
 /**
  * @author  SnapshotLabs
@@ -211,7 +211,7 @@ contract Space is ISpace, Ownable, ReentrancyGuard {
      * @dev     it has already been set. Time complexity is O(n).
      * @param   strats  Array to check for duplicates.
      */
-    function _assertNoDuplicateIndices(IndexedStrategy[] memory strats) internal {
+    function _assertNoDuplicateIndices(IndexedStrategy[] memory strats) internal pure {
         if (strats.length < 2) {
             return;
         }

src/SpaceFactory.sol

@@ -2,9 +2,9 @@
 
 pragma solidity ^0.8.18;
 
-import "./Space.sol";
-import "./interfaces/ISpaceFactory.sol";
-import "./types.sol";
+import { Space } from "./Space.sol";
+import { ISpaceFactory } from "./interfaces/ISpaceFactory.sol";
+import { IndexedStrategy, Strategy } from "./types.sol";
 
 /**
  * @title   Space Factory

src/authenticators/Authenticator.sol

@@ -5,14 +5,16 @@ pragma solidity ^0.8.18;
 abstract contract Authenticator {
     bytes4 internal constant PROPOSE_SELECTOR =
         bytes4(keccak256("propose(address,string,(uint8,bytes),(uint8,bytes)[])"));
-    bytes4 constant VOTE_SELECTOR = bytes4(keccak256("vote(address,uint256,uint8,(uint8,bytes)[],string)"));
-    bytes4 constant UPDATE_PROPOSAL_SELECTOR =
+    bytes4 internal constant VOTE_SELECTOR = bytes4(keccak256("vote(address,uint256,uint8,(uint8,bytes)[],string)"));
+    bytes4 internal constant UPDATE_PROPOSAL_SELECTOR =
         bytes4(keccak256("updateProposal(address,uint256,(uint8,bytes),string)"));
 
     function _call(address target, bytes4 functionSelector, bytes memory data) internal {
+        // solhint-disable-next-line avoid-low-level-calls
         (bool success, ) = target.call(abi.encodePacked(functionSelector, data));
         if (!success) {
             // If the call failed, we revert with the propagated error message.
+            // solhint-disable-next-line no-inline-assembly
             assembly {
                 let returnDataSize := returndatasize()
                 returndatacopy(0, 0, returnDataSize)

src/authenticators/EthSigAuthenticator.sol

@@ -2,12 +2,13 @@
 
 pragma solidity ^0.8.18;
 
-import "./Authenticator.sol";
-import "../utils/SignatureVerifier.sol";
+import { Authenticator } from "./Authenticator.sol";
+import { SignatureVerifier } from "../utils/SignatureVerifier.sol";
 
 contract EthSigAuthenticator is Authenticator, SignatureVerifier {
     error InvalidFunctionSelector();
 
+    // solhint-disable-next-line no-empty-blocks
     constructor(string memory name, string memory version) SignatureVerifier(name, version) {}
 
     function authenticate(

src/authenticators/EthTxAuthenticator.sol

@@ -2,8 +2,8 @@
 
 pragma solidity ^0.8.18;
 
-import "./Authenticator.sol";
-import "../types.sol";
+import { Authenticator } from "./Authenticator.sol";
+import { Choice, IndexedStrategy, Strategy } from "../types.sol";
 
 /**
  * @author  SnapshotLabs

src/authenticators/VanillaAuthenticator.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "./Authenticator.sol";
+import { Authenticator } from "./Authenticator.sol";
 
 contract VanillaAuthenticator is Authenticator {
     function authenticate(address target, bytes4 functionSelector, bytes memory data) external {

src/execution-strategies/AvatarExecutionStrategy.sol

@@ -2,9 +2,10 @@
 
 pragma solidity ^0.8.18;
 
-import "@zodiac/interfaces/IAvatar.sol";
-import "./SimpleQuorumExecutionStrategy.sol";
-import "../utils/SpaceManager.sol";
+import { IAvatar } from "@zodiac/interfaces/IAvatar.sol";
+import { SimpleQuorumExecutionStrategy } from "./SimpleQuorumExecutionStrategy.sol";
+import { SpaceManager } from "../utils/SpaceManager.sol";
+import { MetaTransaction, Proposal, ProposalStatus } from "../types.sol";
 
 /// @title Avatar Execution Strategy - An Execution strategy that executes transactions on an Avatar contract
 /// @dev An Avatar contract is any contract that implements the IAvatar interface, eg a Gnosis Safe.

src/execution-strategies/SimpleQuorumExecutionStrategy.sol

@@ -2,7 +2,8 @@
 
 pragma solidity ^0.8.18;
 
-import "../interfaces/IExecutionStrategy.sol";
+import { IExecutionStrategy } from "../interfaces/IExecutionStrategy.sol";
+import { FinalizationStatus, Proposal, ProposalStatus } from "../types.sol";
 
 abstract contract SimpleQuorumExecutionStrategy is IExecutionStrategy {
     function execute(

src/execution-strategies/VanillaExecutionStrategy.sol

@@ -2,10 +2,11 @@
 
 pragma solidity ^0.8.18;
 
-import "./SimpleQuorumExecutionStrategy.sol";
+import { SimpleQuorumExecutionStrategy } from "./SimpleQuorumExecutionStrategy.sol";
+import { Proposal, ProposalStatus } from "../types.sol";
 
 contract VanillaExecutionStrategy is SimpleQuorumExecutionStrategy {
-    uint256 numExecuted;
+    uint256 internal numExecuted;
 
     function execute(
         Proposal memory proposal,

src/interfaces/IExecutionStrategy.sol

@@ -2,8 +2,8 @@
 
 pragma solidity ^0.8.18;
 
-import "../types.sol";
-import "./execution-strategies/IExecutionStrategyErrors.sol";
+import { IndexedStrategy, Proposal, ProposalStatus } from "../types.sol";
+import { IExecutionStrategyErrors } from "./execution-strategies/IExecutionStrategyErrors.sol";
 
 interface IExecutionStrategy is IExecutionStrategyErrors {
     function execute(

src/interfaces/ISpace.sol

@@ -2,10 +2,13 @@
 
 pragma solidity ^0.8.18;
 
-import "./space/ISpaceState.sol";
-import "./space/ISpaceActions.sol";
-import "./space/ISpaceOwnerActions.sol";
-import "./space/ISpaceEvents.sol";
-import "./space/ISpaceErrors.sol";
+import { ISpaceState } from "./space/ISpaceState.sol";
+import { ISpaceActions } from "./space/ISpaceActions.sol";
+import { ISpaceOwnerActions } from "./space/ISpaceOwnerActions.sol";
+import { ISpaceEvents } from "./space/ISpaceEvents.sol";
+import { ISpaceErrors } from "./space/ISpaceErrors.sol";
 
-interface ISpace is ISpaceState, ISpaceActions, ISpaceOwnerActions, ISpaceEvents, ISpaceErrors {}
+// solhint-disable-next-line no-empty-blocks
+interface ISpace is ISpaceState, ISpaceActions, ISpaceOwnerActions, ISpaceEvents, ISpaceErrors {
+
+}

src/interfaces/ISpaceFactory.sol

@@ -2,10 +2,10 @@
 
 pragma solidity ^0.8.18;
 
-import "./space-factory/ISpaceFactoryErrors.sol";
-import "./space-factory/ISpaceFactoryEvents.sol";
+import { ISpaceFactoryErrors } from "./space-factory/ISpaceFactoryErrors.sol";
+import { ISpaceFactoryEvents } from "./space-factory/ISpaceFactoryEvents.sol";
 
-import "../types.sol";
+import { Strategy } from "../types.sol";
 
 interface ISpaceFactory is ISpaceFactoryErrors, ISpaceFactoryEvents {
     function createSpace(

src/interfaces/execution-strategies/IExecutionStrategyErrors.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "../../types.sol";
+import { ProposalStatus } from "../../types.sol";
 
 interface IExecutionStrategyErrors {
     /// @notice Thrown when the current status of a proposal does not allow the desired action.

src/interfaces/space-factory/ISpaceFactoryEvents.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "../../types.sol";
+import { Strategy } from "../../types.sol";
 
 interface ISpaceFactoryEvents {
     event SpaceCreated(

src/interfaces/space/ISpaceActions.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "src/types.sol";
+import { Choice, IndexedStrategy } from "src/types.sol";
 
 interface ISpaceActions {
     function propose(

src/interfaces/space/ISpaceEvents.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.18;
 
-import "src/types.sol";
+import { IndexedStrategy, Proposal, Strategy, Vote } from "src/types.sol";
 
 interface ISpaceEvents {
     event ProposalCreated(uint256 nextProposalId, address author, Proposal proposal, string metadataUri, bytes payload);

src/interfaces/space/ISpaceOwnerActions.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "../../types.sol";
+import { Strategy } from "../../types.sol";
 
 interface ISpaceOwnerActions {
     function cancel(uint256 proposalId) external;

src/interfaces/space/ISpaceState.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "src/types.sol";
+import { Proposal, ProposalStatus } from "src/types.sol";
 
 interface ISpaceState {
     function getController() external view returns (address);

src/types.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "@gnosis.pm/safe-contracts/contracts/common/Enum.sol";
+import { Enum } from "@gnosis.pm/safe-contracts/contracts/common/Enum.sol";
 
 struct Proposal {
     // notice: `uint32::max` corresponds to year ~2106.

src/utils/SXHash.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "src/types.sol";
+import { IndexedStrategy, Strategy } from "src/types.sol";
 
 /// @title SX Types Hashing Library
 /// @notice This library contains functions for hashing SX types for use in eip712 signatures.

src/utils/SignatureVerifier.sol

@@ -2,9 +2,9 @@
 
 pragma solidity ^0.8.18;
 
-import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
-import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
-import "src/types.sol";
+import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
+import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
+import { Choice, IndexedStrategy } from "src/types.sol";
 import { SXHash } from "src/utils/SXHash.sol";
 
 abstract contract SignatureVerifier is EIP712 {
@@ -35,6 +35,7 @@ abstract contract SignatureVerifier is EIP712 {
 
     mapping(address author => mapping(uint256 salt => bool used)) private usedSalts;
 
+    // solhint-disable-next-line no-empty-blocks
     constructor(string memory name, string memory version) EIP712(name, version) {}
 
     function _verifyProposeSig(uint8 v, bytes32 r, bytes32 s, uint256 salt, address space, bytes memory data) internal {
@@ -103,7 +104,7 @@ abstract contract SignatureVerifier is EIP712 {
         if (recoveredAddress != voter) revert InvalidSignature();
     }
 
-    function _verifyUpdateProposalSig(uint8 v, bytes32 r, bytes32 s, address space, bytes memory data) internal {
+    function _verifyUpdateProposalSig(uint8 v, bytes32 r, bytes32 s, address space, bytes memory data) internal view {
         (address author, uint256 proposalId, IndexedStrategy memory executionStrategy, string memory metadataUri) = abi
             .decode(data, (address, uint256, IndexedStrategy, string));
 

src/utils/SpaceManager.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 
 /// @title Space Manager - A contract that manages spaces that are able to execute transactions via this contract
 /// @author Snapshot Labs
@@ -19,6 +19,7 @@ contract SpaceManager is OwnableUpgradeable {
 
     /// @notice Initialize the contract with a list of spaces. Called only once.
     /// @param _spaces List of spaces.
+    // solhint-disable-next-line func-name-mixedcase
     function __SpaceManager_init(address[] memory _spaces) internal initializer {
         for (uint256 i = 0; i < _spaces.length; i++) {
             spaces[_spaces[i]] = true;

src/voting-strategies/CompVotingStrategy.sol

@@ -2,9 +2,9 @@
 
 pragma solidity ^0.8.18;
 
-import "../interfaces/IVotingStrategy.sol";
-import "../interfaces/IComp.sol";
-import "../utils/TimestampResolver.sol";
+import { IVotingStrategy } from "../interfaces/IVotingStrategy.sol";
+import { IComp } from "../interfaces/IComp.sol";
+import { TimestampResolver } from "../utils/TimestampResolver.sol";
 
 contract CompVotingStrategy is IVotingStrategy, TimestampResolver {
     error InvalidByteArray();
@@ -15,7 +15,7 @@ contract CompVotingStrategy is IVotingStrategy, TimestampResolver {
         bytes calldata params,
         bytes calldata /* userParams */
     ) external override returns (uint256) {
-        address tokenAddress = BytesToAddress(params, 0);
+        address tokenAddress = bytesToAddress(params, 0);
         uint256 blockNumber = resolveSnapshotTimestamp(timestamp);
         return uint256(IComp(tokenAddress).getPriorVotes(voterAddress, blockNumber));
     }
@@ -25,10 +25,11 @@ contract CompVotingStrategy is IVotingStrategy, TimestampResolver {
     /// @param _start The index to start extracting the address from
     /// @dev Function from the library, with the require switched for a revert statement:
     /// https://github.com/GNSPS/solidity-bytes-utils/blob/master/contracts/BytesLib.sol
-    function BytesToAddress(bytes memory _bytes, uint256 _start) internal pure returns (address) {
+    function bytesToAddress(bytes memory _bytes, uint256 _start) internal pure returns (address) {
         if (_bytes.length < _start + 20) revert InvalidByteArray();
         address tempAddress;
 
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             tempAddress := div(mload(add(add(_bytes, 0x20), _start)), 0x1000000000000000000000000)
         }

src/voting-strategies/VanillaVotingStrategy.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "../interfaces/IVotingStrategy.sol";
+import { IVotingStrategy } from "../interfaces/IVotingStrategy.sol";
 
 contract VanillaVotingStrategy is IVotingStrategy {
     function getVotingPower(

src/voting-strategies/WhitelistStrategy.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import "../interfaces/IVotingStrategy.sol";
+import { IVotingStrategy } from "../interfaces/IVotingStrategy.sol";
 
 contract WhitelistStrategy is IVotingStrategy {
     struct Member {
@@ -16,7 +16,7 @@ contract WhitelistStrategy is IVotingStrategy {
      * @param   params  The list of members. Needs to be sorted in ascending `addy` order
      * @return  uint256  The voting power of `voterAddress` if it exists: else 0
      */
-    function _getVotingPower(address voterAddress, bytes calldata params) internal returns (uint256) {
+    function _getVotingPower(address voterAddress, bytes calldata params) internal pure returns (uint256) {
         Member[] memory members = abi.decode(params, (Member[]));
 
         uint256 high = members.length - 1;
@@ -48,7 +48,7 @@ contract WhitelistStrategy is IVotingStrategy {
         address voterAddress,
         bytes calldata params, // Need to be sorted by ascending `addy`s
         bytes calldata /* userParams */
-    ) external override returns (uint256) {
+    ) external pure override returns (uint256) {
         return _getVotingPower(voterAddress, params);
     }
 }