Fixes DAO Final Audit Report

contracts/dao/DAOController.sol

@@ -17,7 +17,6 @@ contract DAOController is Initializable {
 
     struct Scheme {
         bytes32 paramsHash; // a hash voting parameters of the scheme
-        bool isRegistered;
         bool canManageSchemes;
         bool canMakeAvatarCalls;
         bool canChangeReputation;
@@ -61,19 +60,15 @@ contract DAOController is Initializable {
     /// @notice Cannot unregister last scheme with manage schemes permission
     error DAOController__CannotUnregisterLastSchemeWithManageSchemesPermission();
 
+    /// @notice Cannot register a scheme with paramsHash 0
+    error DAOController__CannotRegisterSchemeWithNullParamsHash();
+
     /// @notice Sender is not the scheme that originally started the proposal
     error DAOController__SenderIsNotTheProposer();
 
     /// @notice Sender is not a registered scheme or proposal is not active
     error DAOController__SenderIsNotRegisteredOrProposalIsInactive();
 
-    /// @dev Verify if scheme is registered
-    modifier onlyRegisteredScheme() {
-        if (!schemes[msg.sender].isRegistered) {
-            revert DAOController__SenderNotRegistered();
-        }
-        _;
-    }
     /// @dev Verify if scheme can manage schemes
     modifier onlyRegisteringSchemes() {
         if (!schemes[msg.sender].canManageSchemes) {
@@ -111,7 +106,6 @@ contract DAOController is Initializable {
     ) public initializer {
         schemes[scheme] = Scheme({
             paramsHash: paramsHash,
-            isRegistered: true,
             canManageSchemes: true,
             canMakeAvatarCalls: true,
             canChangeReputation: true
@@ -135,11 +129,15 @@ contract DAOController is Initializable {
         bool canManageSchemes,
         bool canMakeAvatarCalls,
         bool canChangeReputation
-    ) external onlyRegisteredScheme onlyRegisteringSchemes returns (bool success) {
+    ) external onlyRegisteringSchemes returns (bool success) {
         Scheme memory scheme = schemes[schemeAddress];
 
+        if (paramsHash == bytes32(0)) {
+            revert DAOController__CannotRegisterSchemeWithNullParamsHash();
+        }
+
         // Add or change the scheme:
-        if ((!scheme.isRegistered || !scheme.canManageSchemes) && canManageSchemes) {
+        if (!scheme.canManageSchemes && canManageSchemes) {
             schemesWithManageSchemesPermission = schemesWithManageSchemesPermission + 1;
         } else if (scheme.canManageSchemes && !canManageSchemes) {
             if (schemesWithManageSchemesPermission <= 1) {
@@ -150,7 +148,6 @@ contract DAOController is Initializable {
 
         schemes[schemeAddress] = Scheme({
             paramsHash: paramsHash,
-            isRegistered: true,
             canManageSchemes: canManageSchemes,
             canMakeAvatarCalls: canMakeAvatarCalls,
             canChangeReputation: canChangeReputation
@@ -166,16 +163,11 @@ contract DAOController is Initializable {
      * @param schemeAddress The address of the scheme to unregister/delete from `schemes` mapping
      * @return success Success of the operation
      */
-    function unregisterScheme(address schemeAddress)
-        external
-        onlyRegisteredScheme
-        onlyRegisteringSchemes
-        returns (bool success)
-    {
+    function unregisterScheme(address schemeAddress) external onlyRegisteringSchemes returns (bool success) {
         Scheme memory scheme = schemes[schemeAddress];
 
         //check if the scheme is registered
-        if (_isSchemeRegistered(schemeAddress) == false) {
+        if (scheme.paramsHash == bytes32(0)) {
             return false;
         }
 
@@ -206,7 +198,7 @@ contract DAOController is Initializable {
         bytes calldata data,
         DAOAvatar avatar,
         uint256 value
-    ) external onlyRegisteredScheme onlyAvatarCallScheme returns (bool callSuccess, bytes memory callData) {
+    ) external onlyAvatarCallScheme returns (bool callSuccess, bytes memory callData) {
         return avatar.executeCall(to, data, value);
     }
 
@@ -243,15 +235,6 @@ contract DAOController is Initializable {
         reputationToken.transferOwnership(newOwner);
     }
 
-    /**
-     * @dev Returns whether a scheme is registered or not
-     * @param scheme The address of the scheme
-     * @return isRegistered Whether a scheme is registered or not
-     */
-    function isSchemeRegistered(address scheme) external view returns (bool isRegistered) {
-        return _isSchemeRegistered(scheme);
-    }
-
     /**
      * @dev Returns scheme paramsHash
      * @param scheme The address of the scheme
@@ -300,10 +283,6 @@ contract DAOController is Initializable {
         return schemesWithManageSchemesPermission;
     }
 
-    function _isSchemeRegistered(address scheme) private view returns (bool) {
-        return (schemes[scheme].isRegistered);
-    }
-
     /**
      * @dev Function to get reputation token
      * @return tokenAddress The reputation token set on controller.initialize

contracts/dao/schemes/Scheme.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.17;
 
 import "@openzeppelin/contracts/utils/Address.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "../../utils/PermissionRegistry.sol";
 import "../DAOReputation.sol";
 import "../DAOAvatar.sol";
@@ -25,7 +26,7 @@ import "../votingMachine/VotingMachineCallbacks.sol";
  * Once the governance process ends on the voting machine the voting machine can execute the proposal winning option.
  * If the wining option cant be executed successfully, it can be finished without execution once the maxTimesForExecution time passes.
  */
-abstract contract Scheme is VotingMachineCallbacks {
+abstract contract Scheme is Initializable, VotingMachineCallbacks {
     using Address for address;
 
     enum ProposalState {
@@ -59,9 +60,6 @@ abstract contract Scheme is VotingMachineCallbacks {
 
     event ProposalStateChange(bytes32 indexed proposalId, uint256 indexed state);
 
-    /// @notice Emitted when its initialized twice
-    error Scheme__CannotInitTwice();
-
     /// @notice Emitted if avatar address is zero
     error Scheme__AvatarAddressCannotBeZero();
 
@@ -105,11 +103,7 @@ abstract contract Scheme is VotingMachineCallbacks {
         address permissionRegistryAddress,
         string calldata _schemeName,
         uint256 _maxRepPercentageChange
-    ) external {
-        if (address(avatar) != address(0)) {
-            revert Scheme__CannotInitTwice();
-        }
-
+    ) external initializer {
         if (avatarAddress == address(0)) {
             revert Scheme__AvatarAddressCannotBeZero();
         }
@@ -176,7 +170,8 @@ abstract contract Scheme is VotingMachineCallbacks {
     }
 
     /**
-     * @dev Execution of proposals, can only be called by the voting machine in which the vote is held.
+     * @dev Execute the proposal calls for the winning option,
+     * can only be called by the voting machine in which the vote is held.
      * @param proposalId The ID of the voting in the voting machine
      * @param winningOption The winning option in the voting machine
      * @return success Success of the execution
@@ -193,13 +188,18 @@ abstract contract Scheme is VotingMachineCallbacks {
         }
         executingProposal = true;
 
-        Proposal memory proposal = proposals[proposalId];
+        Proposal storage proposal = proposals[proposalId];
 
         if (proposal.state != ProposalState.Submitted) {
             revert Scheme__ProposalMustBeSubmitted();
         }
 
-        if (winningOption > 1) {
+        if (winningOption == 1) {
+            proposal.state = ProposalState.Rejected;
+            emit ProposalStateChange(proposalId, uint256(ProposalState.Rejected));
+        } else {
+            proposal.state = ProposalState.Passed;
+            emit ProposalStateChange(proposalId, uint256(ProposalState.Passed));
             uint256 oldRepSupply = getNativeReputationTotalSupply();
 
             permissionRegistry.setERC20Balances();
@@ -253,7 +253,8 @@ abstract contract Scheme is VotingMachineCallbacks {
     }
 
     /**
-     * @dev Finish a proposal and set the final state in storage
+     * @dev Finish a proposal and set the final state in storage without any execution.
+     * The only thing done here is a change in the proposal state in case the proposal was not executed.
      * @param proposalId The ID of the voting in the voting machine
      * @param winningOption The winning option in the voting machine
      * @return success Proposal finish successfully
@@ -265,18 +266,18 @@ abstract contract Scheme is VotingMachineCallbacks {
         returns (bool success)
     {
         Proposal storage proposal = proposals[proposalId];
-        if (proposal.state != ProposalState.Submitted) {
-            revert Scheme__ProposalMustBeSubmitted();
-        }
-
-        if (winningOption == 1) {
-            proposal.state = ProposalState.Rejected;
-            emit ProposalStateChange(proposalId, uint256(ProposalState.Rejected));
+        if (proposal.state == ProposalState.Submitted) {
+            if (winningOption == 1) {
+                proposal.state = ProposalState.Rejected;
+                emit ProposalStateChange(proposalId, uint256(ProposalState.Rejected));
+            } else {
+                proposal.state = ProposalState.Passed;
+                emit ProposalStateChange(proposalId, uint256(ProposalState.Passed));
+            }
+            return true;
         } else {
-            proposal.state = ProposalState.Passed;
-            emit ProposalStateChange(proposalId, uint256(ProposalState.Passed));
+            return false;
         }
-        return true;
     }
 
     /**