misc: Better error throwing during enable/disable rule selectors

lens-protocol · Jan 14, 2025 · f4b42d5 · f4b42d5
1 parent 283d303
commit f4b42d5
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 15 deletions.
diff --git a/contracts/core/libraries/RulesLib.sol b/contracts/core/libraries/RulesLib.sol
@@ -59,22 +59,27 @@ library RulesLib {
         bytes4 ruleSelector
     ) internal {
         require(rulesStorage.isConfigured[ruleAddress][configSalt], Errors.RuleNotConfigured());
-        require(
-            !_isSelectorAlreadyEnabled(rulesStorage, ruleSelector, ruleAddress, configSalt),
-            Errors.RedundantStateChange()
-        );
+        if (rulesStorage.ruleStates[ruleSelector][ruleAddress][configSalt].isEnabled) {
+            if (rulesStorage.ruleStates[ruleSelector][ruleAddress][configSalt].isRequired == isRequired) {
+                revert Errors.RedundantStateChange();
+            } else {
+                revert Errors.SelectorEnabledForDifferentRuleType();
+            }
+        }
         _addRuleSelectorToStorage(rulesStorage, ruleSelector, ruleAddress, configSalt, isRequired);
     }
 
     function disableRuleSelector(
         RulesStorage storage rulesStorage,
-        bool, /* isRequired */
+        bool isRequired,
         address ruleAddress,
         bytes32 configSalt,
         bytes4 ruleSelector
     ) internal {
+        require(rulesStorage.ruleStates[ruleSelector][ruleAddress][configSalt].isEnabled, Errors.RedundantStateChange());
         require(
-            _isSelectorAlreadyEnabled(rulesStorage, ruleSelector, ruleAddress, configSalt), Errors.RedundantStateChange()
+            rulesStorage.ruleStates[ruleSelector][ruleAddress][configSalt].isRequired == isRequired,
+            Errors.SelectorEnabledForDifferentRuleType()
         );
         _removeRuleSelectorFromStorage(rulesStorage, ruleSelector, ruleAddress, configSalt);
     }
@@ -148,13 +153,4 @@ library RulesLib {
         rules.pop();
         delete rulesStorage.ruleStates[ruleSelector][ruleAddress][configSalt];
     }
-
-    function _isSelectorAlreadyEnabled(
-        RulesStorage storage rulesStorage,
-        bytes4 ruleSelector,
-        address ruleAddress,
-        bytes32 configSalt
-    ) private view returns (bool) {
-        return rulesStorage.ruleStates[ruleSelector][ruleAddress][configSalt].isEnabled;
-    }
 }
diff --git a/contracts/core/types/Errors.sol b/contracts/core/types/Errors.sol
@@ -36,6 +36,7 @@ library Errors {
     error RedundantStateChange();
     error RequiredRuleReverted();
     error RuleNotConfigured();
+    error SelectorEnabledForDifferentRuleType();
     error Self();
     error SingleAnyOfRule();
     error UnexpectedContractImpl();