Merge branch 'main' into audit_comp_duplicate_metatx

src/Space.sol

@@ -16,7 +16,9 @@ import {
     ProposalStatus,
     Strategy,
     UpdateSettingsCalldata,
-    InitializeCalldata
+    InitializeCalldata,
+    TRUE,
+    FALSE
 } from "src/types.sol";
 import { IVotingStrategy } from "src/interfaces/IVotingStrategy.sol";
 import { IExecutionStrategy } from "src/interfaces/IExecutionStrategy.sol";
@@ -62,13 +64,13 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
     /// @inheritdoc ISpaceState
     Strategy public override proposalValidationStrategy;
     /// @inheritdoc ISpaceState
-    mapping(address auth => bool allowed) public override authenticators;
+    mapping(address auth => uint256 allowed) public override authenticators;
     /// @inheritdoc ISpaceState
     mapping(uint256 proposalId => Proposal proposal) public override proposals;
     // @inheritdoc ISpaceState
     mapping(uint256 proposalId => mapping(Choice choice => uint256 votePower)) public override votePower;
     /// @inheritdoc ISpaceState
-    mapping(uint256 proposalId => mapping(address voter => bool hasVoted)) public override voteRegistry;
+    mapping(uint256 proposalId => mapping(address voter => uint256 hasVoted)) public override voteRegistry;
 
     /// @inheritdoc ISpaceActions
     function initialize(InitializeCalldata calldata input) external override initializer {
@@ -168,7 +170,7 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
 
     /// @dev Gates access to whitelisted authenticators only.
     modifier onlyAuthenticator() {
-        if (authenticators[msg.sender] != true) revert AuthenticatorNotWhitelisted();
+        if (authenticators[msg.sender] != TRUE) revert AuthenticatorNotWhitelisted();
         _;
     }
 
@@ -250,9 +252,9 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
         if (block.number >= proposal.maxEndBlockNumber) revert VotingPeriodHasEnded();
         if (block.number < proposal.startBlockNumber) revert VotingPeriodHasNotStarted();
         if (proposal.finalizationStatus != FinalizationStatus.Pending) revert ProposalFinalized();
-        if (voteRegistry[proposalId][voter]) revert UserAlreadyVoted();
+        if (voteRegistry[proposalId][voter] == TRUE) revert UserAlreadyVoted();
 
-        voteRegistry[proposalId][voter] = true;
+        voteRegistry[proposalId][voter] = TRUE;
 
         uint256 votingPower = _getCumulativePower(
             voter,
@@ -384,14 +386,14 @@ contract Space is ISpace, Initializable, IERC4824, UUPSUpgradeable, OwnableUpgra
     /// @dev Adds an array of authenticators.
     function _addAuthenticators(address[] calldata _authenticators) internal {
         for (uint256 i = 0; i < _authenticators.length; i++) {
-            authenticators[_authenticators[i]] = true;
+            authenticators[_authenticators[i]] = TRUE;
         }
     }
 
     /// @dev Removes an array of authenticators.
     function _removeAuthenticators(address[] calldata _authenticators) internal {
         for (uint256 i = 0; i < _authenticators.length; i++) {
-            authenticators[_authenticators[i]] = false;
+            authenticators[_authenticators[i]] = FALSE;
         }
         // TODO: should we check that there are still authenticators left? same for other setters..
     }

src/interfaces/space/ISpaceState.sol

@@ -20,7 +20,7 @@ interface ISpaceState {
     function votingDelay() external view returns (uint32);
 
     /// @notice Returns whether a given address is a whitelisted authenticator.
-    function authenticators(address) external view returns (bool);
+    function authenticators(address) external view returns (uint256);
 
     /// @notice Returns the voting strategy at a given index.
     /// @param index The index of the voting strategy.
@@ -49,7 +49,7 @@ interface ISpaceState {
     /// @notice Returns whether a voter has voted on a proposal.
     /// @param proposalId The ID of the proposal.
     /// @param voter The address of the voter.
-    function voteRegistry(uint256 proposalId, address voter) external view returns (bool);
+    function voteRegistry(uint256 proposalId, address voter) external view returns (uint256);
 
     /// @notice Returns the proposal at a given ID.
     /// @dev Returns all zeros if the proposal does not exist.

src/types.sol

@@ -5,6 +5,10 @@ pragma solidity ^0.8.18;
 import { Enum } from "@gnosis.pm/safe-contracts/contracts/common/Enum.sol";
 import { IExecutionStrategy } from "src/interfaces/IExecutionStrategy.sol";
 
+/// @dev Constants used to replace the `bool` type in mappings for gas efficiency.
+uint256 constant TRUE = 1;
+uint256 constant FALSE = 0;
+
 /// @notice The data stored for each proposal when it is created.
 /// @dev Packed into 4 256-bit slots.
 struct Proposal {

src/utils/SignatureVerifier.sol

@@ -6,6 +6,7 @@ import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import { Choice, IndexedStrategy, Strategy } from "src/types.sol";
 import { SXHash } from "src/utils/SXHash.sol";
+import { TRUE, FALSE } from "../types.sol";
 
 /// @title EIP712 Signature Verifier
 /// @notice Verifies Signatures for Snapshot X actions.
@@ -39,7 +40,7 @@ abstract contract SignatureVerifier is EIP712 {
             "Strategy(address addr,bytes params)"
         );
 
-    mapping(address author => mapping(uint256 salt => bool used)) private usedSalts;
+    mapping(address author => mapping(uint256 salt => uint256 used)) private usedSalts;
 
     // solhint-disable-next-line no-empty-blocks
     constructor(string memory name, string memory version) EIP712(name, version) {}
@@ -54,7 +55,7 @@ abstract contract SignatureVerifier is EIP712 {
             bytes memory userProposalValidationParams
         ) = abi.decode(data, (address, string, Strategy, bytes));
 
-        if (usedSalts[author][salt]) revert SaltAlreadyUsed();
+        if (usedSalts[author][salt] == TRUE) revert SaltAlreadyUsed();
 
         address recoveredAddress = ECDSA.recover(
             _hashTypedDataV4(
@@ -78,7 +79,7 @@ abstract contract SignatureVerifier is EIP712 {
         if (recoveredAddress != author) revert InvalidSignature();
 
         // Mark salt as used to prevent replay attacks.
-        usedSalts[author][salt] = true;
+        usedSalts[author][salt] = TRUE;
     }
 
     /// @dev Verifies an EIP712 signature for a vote call.
@@ -128,7 +129,7 @@ abstract contract SignatureVerifier is EIP712 {
             (address, uint256, Strategy, string)
         );
 
-        if (usedSalts[author][salt]) revert SaltAlreadyUsed();
+        if (usedSalts[author][salt] == TRUE) revert SaltAlreadyUsed();
 
         address recoveredAddress = ECDSA.recover(
             _hashTypedDataV4(
@@ -152,6 +153,6 @@ abstract contract SignatureVerifier is EIP712 {
         if (recoveredAddress != author) revert InvalidSignature();
 
         // Mark salt as used to prevent replay attacks.
-        usedSalts[author][salt] = true;
+        usedSalts[author][salt] = TRUE;
     }
 }

src/utils/SpaceManager.sol

@@ -3,14 +3,15 @@
 pragma solidity ^0.8.18;
 
 import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import { TRUE, FALSE } from "../types.sol";
 
 /// @title Space Manager
 /// @notice Manages a whitelist of Spaces that are authorized to execute transactions via this contract.
 contract SpaceManager is OwnableUpgradeable {
     /// @notice Thrown if a space is not in the whitelist.
     error InvalidSpace();
 
-    mapping(address space => bool isEnabled) internal spaces;
+    mapping(address space => uint256 isEnabled) internal spaces;
 
     /// @notice Emitted when a space is enabled.
     event SpaceEnabled(address space);
@@ -23,35 +24,35 @@ contract SpaceManager is OwnableUpgradeable {
     // solhint-disable-next-line func-name-mixedcase
     function __SpaceManager_init(address[] memory _spaces) internal onlyInitializing {
         for (uint256 i = 0; i < _spaces.length; i++) {
-            spaces[_spaces[i]] = true;
+            spaces[_spaces[i]] = TRUE;
         }
     }
 
     /// @notice Enable a space.
     /// @param space Address of the space.
     function enableSpace(address space) external onlyOwner {
-        if (space == address(0) || spaces[space]) revert InvalidSpace();
-        spaces[space] = true;
+        if (space == address(0) || (spaces[space] == TRUE)) revert InvalidSpace();
+        spaces[space] = TRUE;
         emit SpaceEnabled(space);
     }
 
     /// @notice Disable a space.
     /// @param space Address of the space.
     function disableSpace(address space) external onlyOwner {
-        if (!spaces[space]) revert InvalidSpace();
-        spaces[space] = false;
+        if (spaces[space] == FALSE) revert InvalidSpace();
+        spaces[space] = FALSE;
         emit SpaceDisabled(space);
     }
 
     /// @notice Check if a space is enabled.
     /// @param space Address of the space.
-    /// @return bool whether the space is enabled.
-    function isSpaceEnabled(address space) external view returns (bool) {
+    /// @return uint256 whether the space is enabled.
+    function isSpaceEnabled(address space) external view returns (uint256) {
         return spaces[space];
     }
 
     modifier onlySpace() {
-        if (!spaces[msg.sender]) revert InvalidSpace();
+        if (spaces[msg.sender] == FALSE) revert InvalidSpace();
         _;
     }
 }

test/AvatarExecutionStrategy.t.sol

@@ -5,7 +5,16 @@ pragma solidity ^0.8.18;
 import { SpaceTest } from "./utils/Space.t.sol";
 import { Avatar } from "./mocks/Avatar.sol";
 import { AvatarExecutionStrategy } from "../src/execution-strategies/AvatarExecutionStrategy.sol";
-import { Choice, Enum, IndexedStrategy, MetaTransaction, ProposalStatus, Strategy } from "../src/types.sol";
+import {
+    Choice,
+    Enum,
+    IndexedStrategy,
+    MetaTransaction,
+    ProposalStatus,
+    Strategy,
+    TRUE,
+    FALSE
+} from "../src/types.sol";
 import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
 abstract contract AvatarExecutionStrategyTest is SpaceTest {
@@ -196,7 +205,7 @@ abstract contract AvatarExecutionStrategyTest is SpaceTest {
         vm.expectEmit(true, true, true, true);
         emit SpaceEnabled(space);
         avatarExecutionStrategy.enableSpace(space);
-        assertEq(avatarExecutionStrategy.isSpaceEnabled(space), true);
+        assertEq(avatarExecutionStrategy.isSpaceEnabled(space), TRUE);
     }
 
     function testEnableInvalidSpace() public {
@@ -225,7 +234,7 @@ abstract contract AvatarExecutionStrategyTest is SpaceTest {
         vm.expectEmit(true, true, true, true);
         emit SpaceDisabled(address(space));
         avatarExecutionStrategy.disableSpace(address(space));
-        assertEq(avatarExecutionStrategy.isSpaceEnabled(address(space)), false);
+        assertEq(avatarExecutionStrategy.isSpaceEnabled(address(space)), FALSE);
 
         // Check that proposals from the disabled space can't be executed
         MetaTransaction[] memory transactions = new MetaTransaction[](1);

test/CompTimelockExecutionStrategy.t.sol

@@ -3,7 +3,17 @@
 pragma solidity ^0.8.18;
 
 import { SpaceTest } from "./utils/Space.t.sol";
-import { Choice, Enum, IndexedStrategy, MetaTransaction, ProposalStatus, Strategy, Proposal } from "../src/types.sol";
+import {
+    Choice,
+    Enum,
+    IndexedStrategy,
+    MetaTransaction,
+    ProposalStatus,
+    Strategy,
+    Proposal,
+    TRUE,
+    FALSE
+} from "../src/types.sol";
 import {
     CompTimelockCompatibleExecutionStrategy
 } from "../src/execution-strategies/CompTimelockCompatibleExecutionStrategy.sol";
@@ -499,7 +509,7 @@ abstract contract CompTimelockExecutionStrategyTest is SpaceTest {
         assertEq(timelockExecutionStrategy.vetoGuardian(), vetoGuardian);
         assertEq(timelockExecutionStrategy.quorum(), quorum);
         assertEq(address(timelockExecutionStrategy.timelock()), address(timelock));
-        assertEq(timelockExecutionStrategy.isSpaceEnabled(address(space)), true);
+        assertEq(timelockExecutionStrategy.isSpaceEnabled(address(space)), TRUE);
     }
 }
 

test/SpaceOwnerActions.t.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.18;
 
 import { SpaceV2 } from "./mocks/SpaceV2.sol";
-import { SpaceTest } from "./utils/Space.t.sol";
+import { TRUE, FALSE, SpaceTest } from "./utils/Space.t.sol";
 import { Choice, IndexedStrategy, Strategy, UpdateSettingsCalldata } from "../src/types.sol";
 import { VanillaExecutionStrategy } from "../src/execution-strategies/VanillaExecutionStrategy.sol";
 import { BitPacker } from "../src/utils/BitPacker.sol";
@@ -590,9 +590,9 @@ contract SpaceOwnerActionsTest is SpaceTest {
         );
 
         // Ensure authenticators were correctly updated
-        assertEq(space.authenticators(newAuths[0]), true);
-        assertEq(space.authenticators(newAuths[1]), true);
-        assertEq(space.authenticators(authenticators[0]), false);
+        assertEq(space.authenticators(newAuths[0]), TRUE);
+        assertEq(space.authenticators(newAuths[1]), TRUE);
+        assertEq(space.authenticators(authenticators[0]), FALSE);
     }
 
     function testUpdateVotingStrategies() public {
@@ -701,9 +701,9 @@ contract SpaceOwnerActionsTest is SpaceTest {
         assertEq(params, _proposalValidationStrategy.params);
 
         // Ensure authenticators were correctly updated
-        assertEq(space.authenticators(newAuths[0]), true);
-        assertEq(space.authenticators(newAuths[1]), true);
-        assertEq(space.authenticators(authenticators[0]), false);
+        assertEq(space.authenticators(newAuths[0]), TRUE);
+        assertEq(space.authenticators(newAuths[1]), TRUE);
+        assertEq(space.authenticators(authenticators[0]), FALSE);
 
         // Ensure voting strategies were correctly updated
         assertEq(space.activeVotingStrategies().isBitSet(0), false);

test/TimelockExecutionStrategy.t.sol

@@ -3,7 +3,17 @@
 pragma solidity ^0.8.18;
 
 import { SpaceTest } from "./utils/Space.t.sol";
-import { Choice, Enum, IndexedStrategy, MetaTransaction, ProposalStatus, Strategy, Proposal } from "../src/types.sol";
+import {
+    Choice,
+    Enum,
+    IndexedStrategy,
+    MetaTransaction,
+    ProposalStatus,
+    Strategy,
+    Proposal,
+    TRUE,
+    FALSE
+} from "../src/types.sol";
 import { TimelockExecutionStrategy } from "../src/execution-strategies/TimelockExecutionStrategy.sol";
 import { MockImplementation } from "./mocks/MockImplementation.sol";
 import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
@@ -490,7 +500,7 @@ abstract contract TimelockExecutionStrategyTest is SpaceTest {
         assertEq(timelockExecutionStrategy.vetoGuardian(), vetoGuardian);
         assertEq(timelockExecutionStrategy.quorum(), quorum);
         assertEq(timelockExecutionStrategy.timelockDelay(), 1000);
-        assertEq(timelockExecutionStrategy.isSpaceEnabled(address(space)), true);
+        assertEq(timelockExecutionStrategy.isSpaceEnabled(address(space)), TRUE);
     }
 }
 

test/Vote.t.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.18;
 
-import { SpaceTest } from "./utils/Space.t.sol";
+import { TRUE, FALSE, SpaceTest } from "./utils/Space.t.sol";
 import { Choice, IndexedStrategy, Strategy, UpdateSettingsCalldata } from "../src/types.sol";
 import { VanillaVotingStrategy } from "../src/voting-strategies/VanillaVotingStrategy.sol";
 
@@ -20,7 +20,7 @@ contract VoteTest is SpaceTest {
             abi.encode(author, proposalId, Choice.For, userVotingStrategies, voteMetadataURI)
         );
 
-        assertTrue(space.voteRegistry(proposalId, author));
+        assertEq(space.voteRegistry(proposalId, author), TRUE);
     }
 
     function testVoteInvalidAuth() public {

test/utils/Space.t.sol

@@ -16,7 +16,7 @@ import {
 import { ISpaceEvents } from "../../src/interfaces/space/ISpaceEvents.sol";
 import { ISpaceErrors } from "../../src/interfaces/space/ISpaceErrors.sol";
 import { IExecutionStrategyErrors } from "../../src/interfaces/execution-strategies/IExecutionStrategyErrors.sol";
-import { Choice, Strategy, IndexedStrategy, InitializeCalldata } from "../../src/types.sol";
+import { Choice, Strategy, IndexedStrategy, InitializeCalldata, TRUE, FALSE } from "../../src/types.sol";
 
 // solhint-disable-next-line max-states-count
 abstract contract SpaceTest is Test, GasSnapshot, ISpaceEvents, ISpaceErrors, IExecutionStrategyErrors {