Aragon Protocol contest - 0x6980's results

1. for modern and more readable code; update import usages

Context: All Contracts. Recommendation: import {contract1 , contract2} from "filename.sol";

2. Function writing that does not comply with the Solidity Style Guide

Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this. soliditylang-style-guide Functions should be grouped according to their visibility and ordered:

constructor

receive function (if exists)

fallback function (if exists)

external

public

internal

private

within a grouping, place the view and pure functions last

3. Empty blocks should be removed or Emit something

Code contains empty block

• DAO.sol#L136

136:   function _authorizeUpgrade(address) internal virtual override auth(UPGRADE_DAO_PERMISSION_ID) {}

• PermissionManager.sol#L320

320:        } catch {}

• Plugin.sol#L17

17:    constructor(IDAO _dao) DaoAuthorizable(_dao) {}

• PluginUUPSUpgradeable.sol#L63

61:    function _authorizeUpgrade(
62:        address
63:    ) internal virtual override auth(UPGRADE_PLUGIN_PERMISSION_ID) {}

• PluginRepo.sol#L259

257:    function _authorizeUpgrade(
258:        address
259:    ) internal virtual override auth(UPGRADE_REPO_PERMISSION_ID) {}

• CounterV1PluginSetup.sol#L27

        external
        virtual
        override
        returns (bytes memory initData, PreparedSetupData memory preparedSetupData)
27:     {}

• InterfaceBasedRegistry.sol#L56

56:    ) internal virtual override auth(UPGRADE_REGISTRY_PERMISSION_ID) {}

• ENSSubdomainRegistrar.sol#L76

76:    ) internal virtual override auth(UPGRADE_REGISTRAR_PERMISSION_ID) {}

• DAOMock.sol#L31
• DAOMock.sol#L33
• DAOMock.sol#L48
• DAOMock.sol#L50
• DAOMock.sol#L63

31:    function setTrustedForwarder(address /* _trustedForwarder */) external override {}

33:    function setMetadata(bytes calldata /* _metadata */) external override {}

48:    ) external payable override {}

50:    function setSignatureValidator(address /* _signatureValidator */) external override {}

63:    ) external override {}

• MajorityVotingMock.sol#L33

33    ) internal pure override {}

• TestERC721.sol#L8

8:    constructor(string memory name_, string memory symbol_) ERC721(name_, symbol_) {}

• TestERC1155.sol#L8

8:    constructor(string memory URI_) ERC1155(URI_) {}

Recommended Mitigation Steps

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

4. Use descriptive names for Contracts and Libraries

This codebase will be difficult to navigate, as there are no descriptive naming conventions that specify which files should contain meaningful logic.

Prefixes should be added like this by filing:

• Interface I_
• absctract contracts Abs_
• Libraries Lib_

We recommend that you implement this or a similar agreement.

5. Uppercase immutable variables

• DaoAuthorizable.sol#L15

15:    IDAO private immutable dao_;

• DAOFactory.sol#L19-L25, 3 instances

19:    address public immutable daoBase;

    /// @notice The DAO registry listing the `DAO` contracts created via this contract.
22:    DAORegistry public immutable daoRegistry;

    /// @notice The plugin setup processor for installing plugins on the newly created `DAO`s.
25:    PluginSetupProcessor public immutable pluginSetupProcessor;

• AdminSetup.sol#L20

20:    address private immutable implementation_;

• AddresslistVotingSetup.sol#L17

17:    AddresslistVoting private immutable addresslistVotingBase;

• TokenVotingSetup.sol#L30-L36

    /// @notice The address of the `TokenVoting` base contract.
30:    TokenVoting private immutable tokenVotingBase;

    /// @notice The address of the `GovernanceERC20` base contract.
33:    address public immutable governanceERC20Base;

    /// @notice The address of the `GovernanceWrappedERC20` base contract.
36:    address public immutable governanceWrappedERC20Base;

• MultisigSetup.sol#L16

16:    Multisig private immutable multisigBase;

• AdminCloneFactory.sol#L12

12:    address private immutable implementation;

6. Unsafe downcasting operation truncates user’s input.

• DAO.sol#L192
• DAO.sol#L198

192:    if (!hasBit(_allowFailureMap, uint8(i))) {

198:    failureMap = flipBit(failureMap, uint8(i));

We recommend the project handle downcasting and use safe casting library to make sure the downcast does not provide an unexpected truncate value. openzeppelin

7. Is better to use uin256 instead of uint

• ActionExecute.sol#L7-L9, 3 instances

contract ActionExecute {
    uint num = 10;

    function setTest(uint newNum) public returns (uint) {
        num = newNum;
        return num;
    }

    function fail() public pure {
        revert("ActionExecute:Revert");
    }
}

8. Generate perfect code headers every time.

Description: I recommend using header for Solidity code layout and readability

Headers

/*//////////////////////////////////////////////////////////////
                           TESTING 123
//////////////////////////////////////////////////////////////*/

9. Loss of precision due to rounding

• MerkleDistributor.sol#L111
• MerkleDistributor.sol#L121

111:        uint256 claimedWord_index = _index / 256;

121:        uint256 claimedWord_index = _index / 256;

• Ratio.sol#L24

24:       result = _value / RATIO_BASE;

1. Instead of calculating a statevar with keccak256() every time the contract is made pre calculate them before and only give the result to a constant

• DAO.sol#L40-L58, 6 instances

// @notice The ID of the permission required to call the `execute` function.
40:    bytes32 public constant EXECUTE_PERMISSION_ID = keccak256("EXECUTE_PERMISSION");

    /// @notice The ID of the permission required to call the `_authorizeUpgrade` function.
43:    bytes32 public constant UPGRADE_DAO_PERMISSION_ID = keccak256("UPGRADE_DAO_PERMISSION");

    /// @notice The ID of the permission required to call the `setMetadata` function.
46:    bytes32 public constant SET_METADATA_PERMISSION_ID = keccak256("SET_METADATA_PERMISSION");

    /// @notice The ID of the permission required to call the `setTrustedForwarder` function.
49:    bytes32 public constant SET_TRUSTED_FORWARDER_PERMISSION_ID =
        keccak256("SET_TRUSTED_FORWARDER_PERMISSION");

    /// @notice The ID of the permission required to call the `setSignatureValidator` function.
53:    bytes32 public constant SET_SIGNATURE_VALIDATOR_PERMISSION_ID =
        keccak256("SET_SIGNATURE_VALIDATOR_PERMISSION");

    /// @notice The ID of the permission required to call the `registerStandardCallback` function.
57:    bytes32 public constant REGISTER_STANDARD_CALLBACK_PERMISSION_ID =
        keccak256("REGISTER_STANDARD_CALLBACK_PERMISSION");

• PermissionManager.sol#L15, 1 instance

    /// @notice The ID of the permission required to call the `grant`, `grantWithCondition`, `revoke`, and `bulk` function.
15:    bytes32 public constant ROOT_PERMISSION_ID = keccak256("ROOT_PERMISSION");

• PluginUUPSUpgradeable.sol#L35, 1 instance

    /// @notice The ID of the permission required to call the `_authorizeUpgrade` function.
35:    bytes32 public constant UPGRADE_PLUGIN_PERMISSION_ID = keccak256("UPGRADE_PLUGIN_PERMISSION");

• DAORegistry.sol#L15, 1 instance

    /// @notice The ID of the permission required to call the `register` function.
15:    bytes32 public constant REGISTER_DAO_PERMISSION_ID = keccak256("REGISTER_DAO_PERMISSION");

• PluginRepo.sol#L49-L52, 2 instances

    /// @notice The ID of the permission required to call the `createVersion` function.
49:    bytes32 public constant MAINTAINER_PERMISSION_ID = keccak256("MAINTAINER_PERMISSION");

    /// @notice The ID of the permission required to call the `createVersion` function.
52:    bytes32 public constant UPGRADE_REPO_PERMISSION_ID = keccak256("UPGRADE_REPO_PERMISSION");

• PluginRepoRegistry.sol#L16, 1 instance

    /// @notice The ID of the permission required to call the `register` function.
16:    bytes32 public constant REGISTER_PLUGIN_REPO_PERMISSION_ID =
17:        keccak256("REGISTER_PLUGIN_REPO_PERMISSION");

• PluginSetupProcessor.sol#L27-L35, 3 instances

    /// @notice The ID of the permission required to call the `applyInstallation` function.
27:    bytes32 public constant APPLY_INSTALLATION_PERMISSION_ID =
28:        keccak256("APPLY_INSTALLATION_PERMISSION");

    /// @notice The ID of the permission required to call the `applyUpdate` function.
31:    bytes32 public constant APPLY_UPDATE_PERMISSION_ID = keccak256("APPLY_UPDATE_PERMISSION");

    /// @notice The ID of the permission required to call the `applyUninstallation` function.
34:    bytes32 public constant APPLY_UNINSTALLATION_PERMISSION_ID =
35:        keccak256("APPLY_UNINSTALLATION_PERMISSION");

• InterfaceBasedRegistry.sol#L18, 1 instance

17:     /// @notice The ID of the permission required to call the `_authorizeUpgrade` function.
18:    bytes32 public constant UPGRADE_REGISTRY_PERMISSION_ID =
19:        keccak256("UPGRADE_REGISTRY_PERMISSION");

• ENSSubdomainRegistrar.sol#L18-L23, 2 instances

    /// @notice The ID of the permission required to call the `_authorizeUpgrade` function.
18:    bytes32 public constant UPGRADE_REGISTRAR_PERMISSION_ID =
19:        keccak256("UPGRADE_REGISTRAR_PERMISSION");

    /// @notice The ID of the permission required to call the `registerSubnode` and `setDefaultResolver` function.
22:    bytes32 public constant REGISTER_ENS_SUBDOMAIN_PERMISSION_ID =
23:        keccak256("REGISTER_ENS_SUBDOMAIN_PERMISSION");

• MultiplyHelper.sol#L12, 1 instance

11:   /// @notice The ID of the permission required to call the `multiply` function.
12:    bytes32 public constant MULTIPLY_PERMISSION_ID = keccak256("MULTIPLY_PERMISSION");

• CounterV1.sol#L14, 1 instance

13:   /// @notice The ID of the permission required to call the `multiply` function.
14:    bytes32 public constant MULTIPLY_PERMISSION_ID = keccak256("MULTIPLY_PERMISSION");

• CounterV1PluginSetup.sol, 3 instances
• CounterV1PluginSetup.sol#L48
• CounterV1PluginSetup.sol#L69
• CounterV1PluginSetup.sol#L114

47:     bytes memory initData = abi.encodeWithSelector(
48:            bytes4(keccak256("initialize(address,address,uint256)")),
49:            _dao,
50:            multiplyHelper,
51:            _num
52:        );


64:        permissions[0] = PermissionLib.MultiTargetPermission(
65:            PermissionLib.Operation.Grant,
66:            _dao,
67:            plugin,
68:            PermissionLib.NO_CONDITION,
69:            keccak256("EXECUTE_PERMISSION")
70:        );


109:        permissions[0] = PermissionLib.MultiTargetPermission(
110:            PermissionLib.Operation.Revoke,
111:            _dao,
112:            _payload.plugin,
113:            PermissionLib.NO_CONDITION,
114:            keccak256("EXECUTE_PERMISSION")
115:        );

• CounterV2.sol#L14, 1 instance

13:   /// @notice The ID of the permission required to call the `multiply` function.
14:    bytes32 public constant MULTIPLY_PERMISSION_ID = keccak256("MULTIPLY_PERMISSION");

• CounterV2PluginSetup.sol, 3 instances
• CounterV2PluginSetup.sol#L49
• CounterV2PluginSetup.sol#L70
• CounterV2PluginSetup.sol#L116

48:     bytes memory initData = abi.encodeWithSelector(
49:            bytes4(keccak256("initialize(address,address,uint256)")),
50:            _dao,
51:            multiplyHelper,
52:            _num
53:        );


65:        permissions[0] = PermissionLib.MultiTargetPermission(
66:            PermissionLib.Operation.Grant,
67:            _dao,
68:            plugin,
69:            PermissionLib.NO_CONDITION,
70:            keccak256("EXECUTE_PERMISSION")
71:        );


113:        if (_currentBuild == 1) {
114:            (_newVariable) = abi.decode(_payload.data, (uint256));
115:            initData = abi.encodeWithSelector(
116:                bytes4(keccak256("setNewVariable(uint256)")),
117:                _newVariable
118:            );
119:        }

• Admin.sol#L23, 1 instance

22:    /// @notice The ID of the permission required to call the `executeProposal` function.
23:    bytes32 public constant EXECUTE_PROPOSAL_PERMISSION_ID =
24:        keccak256("EXECUTE_PROPOSAL_PERMISSION");

• MajorityVotingBase.sol#L184, 1 instance

182:    /// @notice The ID of the permission required to call the `updateVotingSettings` function.
183:    bytes32 public constant UPDATE_VOTING_SETTINGS_PERMISSION_ID =
184:        keccak256("UPDATE_VOTING_SETTINGS_PERMISSION");

• AddresslistVoting.sol#L27, 1 instance

26:    /// @notice The ID of the permission required to call the `addAddresses` and `removeAddresses` functions.
27:    bytes32 public constant UPDATE_ADDRESSES_PERMISSION_ID =
28:        keccak256("UPDATE_ADDRESSES_PERMISSION");

• Multisig.sol#L72, 1 instance

70:    /// @notice The ID of the permission required to call the `addAddresses` and `removeAddresses` functions.
71:    bytes32 public constant UPDATE_MULTISIG_SETTINGS_PERMISSION_ID =
72:        keccak256("UPDATE_MULTISIG_SETTINGS_PERMISSION");

• MerkleMinter.sol#L24-L28, 2 instances

23:    /// @notice The ID of the permission required to call the `merkleMint` function.
24:    bytes32 public constant MERKLE_MINT_PERMISSION_ID = keccak256("MERKLE_MINT_PERMISSION");
25:
26:    /// @notice The ID of the permission required to call the `changeDistributor` function.
27:    bytes32 public constant CHANGE_DISTRIBUTOR_PERMISSION_ID =
28:        keccak256("CHANGE_DISTRIBUTOR_PERMISSION");

• PermissionManagerTest.sol#L11-L12, 2 instances

10:    // Restricted permissionIds that shouldn't be allowed to grant for who = ANY_ADDR or where = ANY_ADDR
11:    bytes32 public constant TEST_PERMISSION_1_ID = keccak256("TEST_PERMISSION_1");
12:    bytes32 public constant TEST_PERMISSION_2_ID = keccak256("TEST_PERMISSION_2");

• PluginMockData.sol, 2 instances
• PluginMockData.sol#L24
• PluginMockData.sol#L41

18:    for (uint160 i = start; i < end; i++) {
19:        permissions[i - start] = PermissionLib.MultiTargetPermission(
20:           op,
21:            address(i),
22:            address(i),
23:            PermissionLib.NO_CONDITION,
24:            keccak256("MOCK_PERMISSION")
25:        );
26:    }


37:  function mockPluginProxy(address _pluginBase, address _dao) returns (address) {
38:    return
39:        createERC1967Proxy(
40:            _pluginBase,
41:            abi.encodeWithSelector(bytes4(keccak256("initialize(address)")), _dao)
42:        );
43:  }

• PluginTest.sol#L9, 1 instance

9:    bytes32 public constant DO_SOMETHING_PERMISSION_ID = keccak256("DO_SOMETHING_PERMISSION");

• SharedPluginTest.sol#L15, 1 instance

15:    bytes32 public constant ID_GATED_ACTION_PERMISSION_ID = keccak256("ID_GATED_ACTION_PERMISSION");

• InterfaceBasedRegistryMock.sol#L8, 1 instance

8:    bytes32 public constant REGISTER_PERMISSION_ID = keccak256("REGISTER_PERMISSION");

• GovernanceERC20.sol#L28, 1 instance

27:    /// @notice The permission identifier to mint new tokens
28:    bytes32 public constant MINT_PERMISSION_ID = keccak256("MINT_PERMISSION");

2. Use constants instead of type(uintx).max

type(uint160).max or type(uint16).max, etc. it uses more gas in the distribution process and also for each transaction than constant usage.

• PermissionManager.sol#L18, 1 instance

17:    /// @notice A special address encoding permissions that are valid for any address `who` or `where`.
18:    address internal constant ANY_ADDR = address(type(uint160).max);

-Multisig.sol#L159-L163, 2 instances

158:        // Check if the new address list length would be greater than `type(uint16).max`, the maximal number of approvals.
159:        if (newAddresslistLength > type(uint16).max) {
160:            revert AddresslistLengthOutOfBounds({
161:                limit: type(uint16).max,
162:                actual: newAddresslistLength
163:            });
164:        }

3. Add unchecked {} for subtractions where the operands cannot underflow because of a previous require or if statement

• PluginMockData.sol#L16, 2 instances
• PluginMockData.sol#L16
• PluginMockData.sol#L19

15:    require(end > start);
16:    permissions = new PermissionLib.MultiTargetPermission[](end - start);
17:
18:    for (uint160 i = start; i < end; i++) {
19:        permissions[i - start] = PermissionLib.MultiTargetPermission(
20:            op,
21:            address(i),
22:            address(i),
23:            PermissionLib.NO_CONDITION,
24:            keccak256("MOCK_PERMISSION")
25:        );
26:    }

• GovernanceERC20Mock.sol, 2 instances
• GovernanceERC20Mock.sol#L27-L28
• GovernanceERC20Mock.sol#L29-L30

25:    function setBalance(address to, uint256 amount) public {
26:        uint256 old = balanceOf(to);
27        if (old < amount) {
28            _mint(to, amount - old);
29        } else if (old > amount) {
30            _burn(to, old - amount);
31        }
32    }

4. Use assembly to write address storage values

• DAO.sol#L242
• DAO.sol#L281

239:    function setSignatureValidator(
240:        address _signatureValidator
241:    ) external override auth(SET_SIGNATURE_VALIDATOR_PERMISSION_ID) {
242:        signatureValidator = IERC1271(_signatureValidator);
243:
244:        emit SignatureValidatorSet({signatureValidator: _signatureValidator});
245:    }


281:    /// @notice Sets the trusted forwarder on the DAO and emits the associated event.
282:    /// @param _trustedForwarder The trusted forwarder address.
283:    function _setTrustedForwarder(address _trustedForwarder) internal {
284:        trustedForwarder = _trustedForwarder;
285:
286:        emit TrustedForwarderSet(_trustedForwarder);
287:    }

• DAOFactory.sol#L57

53:    constructor(DAORegistry _registry, PluginSetupProcessor _pluginSetupProcessor) {
54:        daoRegistry = _registry;
55:        pluginSetupProcessor = _pluginSetupProcessor;
56:
57:        daoBase = address(new DAO());
58:    }

• PluginRepoFactory.sol#L23-L25

22:    constructor(PluginRepoRegistry _pluginRepoRegistry) {
23:        pluginRepoRegistry = _pluginRepoRegistry;
24:
25:        pluginRepoBase = address(new PluginRepo());
26:    }

• TokenFactory.sol#L146
• TokenFactory.sol#L154

146:        governanceERC20Base = address(
147:            new GovernanceERC20(
148:                IDAO(address(0)),
149:                "baseName",
150:                "baseSymbol",
151:                GovernanceERC20.MintSettings(new address[](0), new uint256[](0))
152:            )
153:        );
154:        governanceWrappedERC20Base = address(
155:            new GovernanceWrappedERC20(IERC20Upgradeable(address(0)), "baseName", "baseSymbol")
156:        );

• ENSSubdomainRegistrar.sol#L69
• ENSSubdomainRegistrar.sol#L107

69:        resolver = nodeResolver;

107:       resolver = _resolver;

• AdminSetup.sol#L28

27:    constructor() {
28:        implementation_ = address(new Admin());
29:    }

• MajorityVotingMock.sol#L33

43:      votingToken = _token;

• TokenVotingSetup.sol, 2 instances
• TokenVotingSetup.sol#L62
• TokenVotingSetup.sol#L70

constructor() {
61:        governanceERC20Base = address(
62:            new GovernanceERC20(
63:                IDAO(address(0)),
64:                "",
65:                "",
66:                GovernanceERC20.MintSettings(new address[](0), new uint256[](0))
67:            )
68:        );
69:        governanceWrappedERC20Base = address(
70:            new GovernanceWrappedERC20(IERC20Upgradeable(address(0)), "", "")
71:        );
72:        tokenVotingBase = new TokenVoting();
73    }

• MerkleDistributor.sol#L52

46:    function initialize(
47:        IDAO _dao,
48:        IERC20Upgradeable _token,
49:        bytes32 _merkleRoot
50:    ) external initializer {
51:        __PluginUUPSUpgradeable_init(_dao);
52:        token = _token;
53:        merkleRoot = _merkleRoot;
54:    }

• MerkleMinter.sol#L48-L49, 2 instances
• MerkleMinter.sol#L56, 1 instance

41:    function initialize(
42:        IDAO _dao,
43:        IERC20MintableUpgradeable _token,
44:        IMerkleDistributor _distributorBase
45:    ) external initializer {
46:        __PluginUUPSUpgradeable_init(_dao);
47:
48:        token = _token;
49:        distributorBase = _distributorBase;
50:    }

53:    function changeDistributorBase(
54:        IMerkleDistributor _distributorBase
55:    ) external override auth(CHANGE_DISTRIBUTOR_PERMISSION_ID) {
56:        distributorBase = _distributorBase;
57:    }

• AdminCloneFactory.sol#L15

14:    constructor() {
15:        implementation = address(new Admin());
16:    }

• PluginUUPSUpgradeableSetupMock.sol#L16

15:    constructor() {
16:        pluginBase = address(new PluginUUPSUpgradeableV1Mock());
17:    }

Recommendation Code

    function changeDistributorBase(
        IMerkleDistributor _distributorBase
        ) external override auth(CHANGE_DISTRIBUTOR_PERMISSION_ID) {
        assembly {
                    sstore(distributorBase.slot, _distributorBase)
                }
    }

5. Optimize names to save gas

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.

Context

All Contracts

Recommendation

Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas. For example, the function IDs in the DAO.sol contract will be the most used; A lower method ID may be given.

Proof of Concept

POC

Sighash   |   Function Signature
eafb8b06  =>  initialize(bytes,address,address,string)
552b1863  =>  isPermissionRestrictedForAnyAddr(bytes32)
5ec29272  =>  _authorizeUpgrade(address)
da742228  =>  setTrustedForwarder(address)
ce1b815f  =>  getTrustedForwarder()
fdef9106  =>  hasPermission(address,address,bytes32,bytes)
ee57e36f  =>  setMetadata(bytes)
f01de670  =>  execute(bytes32,Action[],uint256)
bfe07da6  =>  deposit(address,uint256,string)
3e2ab0d9  =>  setSignatureValidator(address)
1626ba7e  =>  isValidSignature(bytes32,bytes)
4146c61e  =>  _setMetadata(bytes)
d292f98a  =>  _setTrustedForwarder(address)
f2e5d775  =>  _registerTokenInterfaces()
c4a50145  =>  registerStandardCallback(bytes4,bytes4,bytes4)
7034731b  =>  daoURI()
1080f99b  =>  setDaoURI(string)
f392bdc0  =>  _setDaoURI(string)

6. Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping’s value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key’s keccak256 hash (Gkeccak256 - 30 gas) and that calculation’s associated stack operations. Caching an array’s struct avoids recalculating the array offsets into memory/calldata.

• MajorityVotingBase.sol#L463-L464

460:        _executeProposal(
461:            dao(),
462:            _proposalId,
463:            proposals[_proposalId].actions,
464:            proposals[_proposalId].allowFailureMap
465:        );

• Multisig.sol#L356-L357

        _executeProposal(
            dao(),
            _proposalId,
356:        proposals[_proposalId].actions,
357:        proposals[_proposalId].allowFailureMap
        );

7. Using private rather than public for constants, saves gas.

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it’s used, and not adding another entry to the method ID table.

• TokenVotingSetup.sol#L33-L36, 2 instances

    /// notice The address of the GovernanceERC20 base contract.
33:    address public immutable governanceERC20Base;

    /// notice The address of the GovernanceWrappedERC20 base contract.
36:    address public immutable governanceWrappedERC20Base;

• DAOFactory.sol#L19-L25, 3 instances

    /// @notice The DAO base contract, to be used for creating new DAOs via createERC1967Proxy function.
19:    address public immutable daoBase;

    /// @notice The DAO registry listing the DAO contracts created via this contract.
22:    DAORegistry public immutable daoRegistry;

    /// @notice The plugin setup processor for installing plugins on the newly created DAOs.
25:    PluginSetupProcessor public immutable pluginSetupProcessor;

• DAOFactory.sol#L19-L25, 3 instances

    /// @notice The DAO base contract, to be used for creating new DAOs via createERC1967Proxy function.
19:    address public immutable daoBase;

    /// @notice The DAO registry listing the DAO contracts created via this contract.
22:    DAORegistry public immutable daoRegistry;

    /// @notice The plugin setup processor for installing plugins on the newly created DAOs.
25:    PluginSetupProcessor public immutable pluginSetupProcessor;

8. <array>.length should not be looked up in every loop of a for -loop

The overheads outlined below are PER LOOP, excluding the first loop

• storage arrays incur a Gwarmaccess (100 gas)

• memory arrays use MLOAD (3 gas)

• calldata arrays use CALLDATALOAD (3 gas) Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

• DAO.sol#L184

184:        for (uint256 i = 0; i < _actions.length; ) {

• PermissionManager.sol, 2 instances
• PermissionManager.sol#L150
• PermissionManager.sol#L170

150:        for (uint256 i; i < items.length; ) {

170:        for (uint256 i; i < _items.length; ) {

• DAOFactory.sol#L95

95:        for (uint256 i; i < _pluginSettings.length; ++i) {

• AddresslistVoting.sol#L130

130:        for (uint256 i; i < _actions.length; ) {

• TokenVoting.sol#L124

124:    for (uint256 i; i < _actions.length; ) {

• Multisig.sol#L252

252:    for (uint256 i; i < _actions.length; ) {

• Addresslist.sol#L60
• Addresslist.sol#L78

60:    for (uint256 i; i < _newAddresses.length; ) {

78:    for (uint256 i; i < _exitingAddresses.length; ) {

• GovernanceERC20.sol#L81

81:    for (uint256 i; i < _mintSettings.receivers.length; ) {

9. Inverting the condition of an if-else-statement wastes gas.

• PluginRepoRegistry.sol#L55 use bytes(subdomain).length == 0 instead of

55:     if (!(bytes(subdomain).length > 0)) {
56:            revert EmptyPluginRepoSubdomain();
57:      }

• CounterV1PluginSetup.sol#L105

105:    _payload.currentHelpers.length != 0 ? 3 : 2

• CounterV2PluginSetup.sol#L145

105:    _payload.currentHelpers.length != 0 ? 3 : 2

10. require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas*) in a function that may ultimately revert in the unhappy case.

• function call in LOC 310 can called after LOC 323 (after if statement) and before LOC 326.

308:    bytes32 pluginInstallationId = _getPluginInstallationId(_dao, plugin);

310:    bytes32 preparedSetupId = _getPreparedSetupId(
            _params.pluginSetupRef,
            hashPermissions(preparedSetupData.permissions),
            hashHelpers(preparedSetupData.helpers),
            bytes(""),
            PreparationType.Installation
        );

318:        PluginState storage pluginState = states[pluginInstallationId];

        // Check if this plugin is already installed.
321:        if (pluginState.currentAppliedSetupId != bytes32(0)) {
322:            revert PluginAlreadyInstalled();
323:        }

• change position of if statement in LOC 216 and put it after LOC 228 and before LOC 230.

216:        if (multisigSettings.onlyListed && !isListedAtBlock(_msgSender(), snapshotBlock)) {
217:            revert ProposalCreationForbidden(_msgSender());
218:        }

220:        if (_startDate == 0) {
221:            _startDate = block.timestamp.toUint64();
222:        } else if (_startDate < block.timestamp.toUint64()) {
223:            revert DateOutOfBounds({limit: block.timestamp.toUint64(), actual: _startDate});
224:        }

226:        if (_endDate < _startDate) {
227:            revert DateOutOfBounds({limit: _startDate, actual: _endDate});
228:        }

11. Use assembly to check for address(0)

Saves 6 gas per instance:

• DAO.sol#L225
• DAO.sol#L252

225:    if (_token == address(0)) {

252:    if (address(signatureValidator) == address(0)) {

• TokenFactory.sol#L86

86:     if (token != address(0)) {

• ENSSubdomainRegistrar.sol#L65
• ENSSubdomainRegistrar.sol#L89
• ENSSubdomainRegistrar.sol#L103

65:     if (nodeResolver == address(0)) {

89:     if (currentOwner != address(0)) {    

103:    if (_resolver == address(0)) {    

• CounterV1PluginSetup.sol#L43
• CounterV1PluginSetup.sol#L80

43:     if (_multiplyHelper == address(0)) {

80:     if (_multiplyHelper == address(0)) {

• CounterV2PluginSetup.sol#L44
• CounterV2PluginSetup.sol#L81

43:     if (_multiplyHelper == address(0)) {

80:     if (_multiplyHelper == address(0)) {

• AdminSetup.sol#L39

39:     if (admin == address(0)) {

• TokenVotingSetup.sol#L98
• TokenVotingSetup.sol#L181

98:     if (token != address(0)) {

181:    if (tokenSettings.addr == address(0)) {

• SharedPluginTest.sol#L24

24:     if (address(ownedIds[_id]) == address(0)) {

• GovernanceERC20.sol#L116

116:    if (to != address(0) && numCheckpoints(to) == 0 && delegates(to) == address(0)) {

• GovernanceWrappedERC20.sol#L106

106:    if (to != address(0) && numCheckpoints(to) == 0 && delegates(to) == address(0)) {