3.3 Scopes Can Be Added but Not Removed (#61)

* Add max scope length for assignedScopes * Add delete scope function
voteagora · Nov 25, 2024 · 1ac1177 · 1ac1177
1 parent 5e49671
commit 1ac1177
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 0 deletions.
diff --git a/src/ProposalTypesConfigurator.sol b/src/ProposalTypesConfigurator.sol
@@ -16,6 +16,7 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
 
     event ScopeCreated(uint8 indexed proposalTypeId, bytes24 indexed scopeKey, bytes4 selector, string description);
     event ScopeDisabled(uint8 indexed proposalTypeId, bytes24 indexed scopeKey);
+    event ScopeDeleted(uint8 indexed proposalTypeId, bytes24 indexed scopeKey);
 
     /*//////////////////////////////////////////////////////////////
                            IMMUTABLE STORAGE
@@ -25,6 +26,8 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
 
     /// @notice Max value of `quorum` and `approvalThreshold` in `ProposalType`
     uint16 public constant PERCENT_DIVISOR = 10_000;
+    // @notice Max length of the `assignedScopes` array
+    uint8 public constant MAX_SCOPE_LENGTH = 5;
 
     /*//////////////////////////////////////////////////////////////
                                 STORAGE
@@ -122,6 +125,7 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
     ) external override onlyAdminOrTimelock {
         if (!_proposalTypes[proposalTypeId].exists) revert InvalidProposalType();
         if (parameters.length != comparators.length) revert InvalidParameterConditions();
+        if (_assignedScopes[proposalTypeId][key].length == MAX_SCOPE_LENGTH) revert MaxScopeLengthReached();
 
         Scope memory scope = Scope(key, selector, parameters, comparators, types, proposalTypeId, description, true);
         _assignedScopes[proposalTypeId][key].push(scope);
@@ -178,6 +182,7 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
     {
         if (!_proposalTypes[proposalTypeId].exists) revert InvalidProposalType();
         if (scope.parameters.length != scope.comparators.length) revert InvalidParameterConditions();
+        if (_assignedScopes[proposalTypeId][scope.key].length == MAX_SCOPE_LENGTH) revert MaxScopeLengthReached();
 
         _scopeExists[scope.key] = true;
         _assignedScopes[proposalTypeId][scope.key].push(scope);
@@ -207,6 +212,17 @@ contract ProposalTypesConfigurator is IProposalTypesConfigurator {
         emit ScopeDisabled(proposalTypeId, scopeKey);
     }
 
+    /**
+     * @notice Deletes a scope inside assignedScopes for a proposal type.
+     * @param proposalTypeId the proposal type ID that has the assigned scope.
+     * @param scopeKey the contract and function signature representing the scope key
+     * @param idx the index of the assigned scope.
+     */
+    function deleteScope(uint8 proposalTypeId, bytes24 scopeKey, uint8 idx) external override onlyAdminOrTimelock {
+        delete _assignedScopes[proposalTypeId][scopeKey][idx];
+        emit ScopeDeleted(proposalTypeId, scopeKey);
+    }
+
     /**
      * @notice Validates that a proposed transaction conforms to the scope defined in a given proposal type. Note: This
      *   version only supports functions that have for each parameter 32-byte abi encodings, please see the ABI

diff --git a/src/interfaces/IProposalTypesConfigurator.sol b/src/interfaces/IProposalTypesConfigurator.sol
@@ -18,6 +18,7 @@ interface IProposalTypesConfigurator {
     error InvalidParamNotEqual();
     error InvalidParamRange();
     error InvalidProposedTxForType();
+    error MaxScopeLengthReached();
     error InvalidCalldatasLength();
     error InvalidCalldata();
 
@@ -107,6 +108,7 @@ interface IProposalTypesConfigurator {
     function getSelector(uint8 proposalTypeId, bytes24 key) external returns (bytes4);
     function addScopeForProposalType(uint8 proposalTypeId, Scope calldata scope) external;
     function disableScope(uint8 proposalTypeId, bytes24 scopeKey, uint8 idx) external;
+    function deleteScope(uint8 proposalTypeId, bytes24 scopeKey, uint8 idx) external;
     function validateProposedTx(bytes calldata proposedTx, uint8 proposalTypeId, bytes24 key) external;
     function validateProposalData(address[] memory targets, bytes[] memory calldatas, uint8 proposalTypeId) external;
 }
diff --git a/test/ProposalTypesConfigurator.t.sol b/test/ProposalTypesConfigurator.t.sol
@@ -281,6 +281,30 @@ contract SetProposalType is ProposalTypesConfiguratorTest {
         );
         vm.stopPrank();
     }
+
+    function testRevert_setScopeForProposalType_MaxScopeLengthReached() public {
+        vm.startPrank(admin);
+
+        bytes32 txTypeHash = keccak256("transfer(address,address,uint)");
+        address contractAddress = makeAddr("contract");
+        bytes24 scopeKey = _pack(contractAddress, bytes4(txTypeHash));
+        bytes4 txEncoded = bytes4(abi.encode("transfer(address,address,uint)", 0xdeadbeef, 0xdeadbeef, 10));
+        bytes[] memory parameters = new bytes[](1);
+        IProposalTypesConfigurator.Comparators[] memory comparators = new IProposalTypesConfigurator.Comparators[](1);
+        IProposalTypesConfigurator.SupportedTypes[] memory types = new IProposalTypesConfigurator.SupportedTypes[](1);
+
+        for (uint8 i = 0; i < proposalTypesConfigurator.MAX_SCOPE_LENGTH(); i++) {
+            proposalTypesConfigurator.setScopeForProposalType(
+                0, scopeKey, txEncoded, parameters, comparators, types, "Lorem Ipsum"
+            );
+        }
+
+        vm.expectRevert(IProposalTypesConfigurator.MaxScopeLengthReached.selector);
+        proposalTypesConfigurator.setScopeForProposalType(
+            0, scopeKey, txEncoded, parameters, comparators, types, "Lorem Ipsum"
+        );
+        vm.stopPrank();
+    }
 }
 
 contract AddScopeForProposalType is ProposalTypesConfiguratorTest {
@@ -376,6 +400,28 @@ contract AddScopeForProposalType is ProposalTypesConfiguratorTest {
         proposalTypesConfigurator.addScopeForProposalType(0, scope);
         vm.stopPrank();
     }
+
+    function testRevert_addScopeForProposalType_MaxScopeLengthReached() public {
+        vm.startPrank(admin);
+        bytes32 txTypeHash = keccak256("transfer(address,address,uint)");
+        address contractAddress = makeAddr("contract");
+        bytes24 scopeKey = _pack(contractAddress, bytes4(txTypeHash));
+        bytes4 txEncoded = bytes4(abi.encode("transfer(address,address,uint)", 0xdeadbeef, 0xdeadbeef, 10));
+        bytes[] memory parameters = new bytes[](1);
+        IProposalTypesConfigurator.Comparators[] memory comparators = new IProposalTypesConfigurator.Comparators[](1);
+        IProposalTypesConfigurator.SupportedTypes[] memory types = new IProposalTypesConfigurator.SupportedTypes[](1);
+
+        IProposalTypesConfigurator.Scope memory scope =
+            IProposalTypesConfigurator.Scope(scopeKey, txEncoded, parameters, comparators, types, 0, "Lorem", true);
+
+        for (uint8 i = 0; i < proposalTypesConfigurator.MAX_SCOPE_LENGTH(); i++) {
+            proposalTypesConfigurator.addScopeForProposalType(0, scope);
+        }
+
+        vm.expectRevert(IProposalTypesConfigurator.MaxScopeLengthReached.selector);
+        proposalTypesConfigurator.addScopeForProposalType(0, scope);
+        vm.stopPrank();
+    }
 }
 
 contract ValidateProposedTx is ProposalTypesConfiguratorTest {