add condition (#108)

* add condition * rename * fix linter * fix: executor 63/64 rule tests for coverage * feat: add tests for powerful condition contract * ci: undo changes * ci: rmv not used import * fix: prettier * feat: add tests for conditions that checks block number and timestamp * ci: prettier * ci: lint fix * Feat/rule condition clean up (#109) * cd: rename powerfulCondtion to RuledCondition * add natspecs * remove redandant imports * extension alwaystruecondition * extension folder * rename, fix and add tests * add rules updated event * Feat: add missing tests (#111) * feat: add test for always true condition * feat: modify mock code to allow sending the compare list on the data * feat: add tests for rule condition * feat: simplify ruled condition tests * fix lint * add supportsinterface on executor * change const name --------- Co-authored-by: Claudia <[email protected]> Co-authored-by: Rekard0 <[email protected]>
aragon · Oct 30, 2024 · 3d484e5 · 3d484e5
1 parent 2016e23
commit 3d484e5
Show file tree

Hide file tree

Showing 17 changed files with 1,512 additions and 26 deletions.
diff --git a/contracts/src/executors/Executor.sol b/contracts/src/executors/Executor.sol
@@ -2,6 +2,8 @@
 
 pragma solidity ^0.8.8;
 
+import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+
 import {IExecutor, Action} from "./IExecutor.sol";
 import {flipBit, hasBit} from "../utils/math/BitMap.sol";
 
@@ -10,12 +12,12 @@ import {flipBit, hasBit} from "../utils/math/BitMap.sol";
 /// Most useful use-case is to deploy as non-upgradeable and call from another contract via delegatecall.
 /// If used with delegatecall, DO NOT add state variables in sequential slots, otherwise this will overwrite
 /// the storage of the calling contract.
-contract Executor is IExecutor {
+contract Executor is IExecutor, ERC165 {
     /// @notice The internal constant storing the maximal action array length.
     uint256 internal constant MAX_ACTIONS = 256;
 
     // keccak256("osx-commons.storage.Executor")
-    bytes32 private constant ReentrancyGuardStorageLocation =
+    bytes32 private constant REENTRANCY_GUARD_STORAGE_LOCATION =
         0x4d6542319dfb3f7c8adbb488d7b4d7cf849381f14faf4b64de3ac05d08c0bdec;
 
     /// @notice The first out of two values to which the `_reentrancyStatus` state variable (used by the `nonReentrant` modifier) can be set indicating that a function was not entered.
@@ -53,6 +55,13 @@ contract Executor is IExecutor {
         _storeReentrancyStatus(_NOT_ENTERED);
     }
 
+    /// @notice Checks if this or the parent contract supports an interface by its ID.
+    /// @param _interfaceId The ID of the interface.
+    /// @return Returns `true` if the interface is supported.
+    function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) {
+        return _interfaceId == type(IExecutor).interfaceId || super.supportsInterface(_interfaceId);
+    }
+
     /// @inheritdoc IExecutor
     function execute(
         bytes32 _callId,
@@ -120,15 +129,17 @@ contract Executor is IExecutor {
     /// @notice Gets the current reentrancy status.
     /// @return status This returns the current reentrancy status.
     function _getReentrancyStatus() private view returns (uint256 status) {
+        // solhint-disable-next-line no-inline-assembly
         assembly {
-            status := sload(ReentrancyGuardStorageLocation)
+            status := sload(REENTRANCY_GUARD_STORAGE_LOCATION)
         }
     }
 
     /// @notice Stores the reentrancy status on a specific slot.
     function _storeReentrancyStatus(uint256 _status) private {
+        // solhint-disable-next-line no-inline-assembly
         assembly {
-            sstore(ReentrancyGuardStorageLocation, _status)
+            sstore(REENTRANCY_GUARD_STORAGE_LOCATION, _status)
         }
     }
 }
diff --git a/contracts/src/mocks/executors/ActionExecute.sol b/contracts/src/mocks/executors/ActionExecute.sol
@@ -3,18 +3,18 @@
 pragma solidity ^0.8.8;
 
 import {IExecutor, Action} from "../../executors/Executor.sol";
-import "hardhat/console.sol";
 
 /// @notice A dummy contract to test if Executor can successfully execute an action.
 contract ActionExecute {
-    uint num = 10;
+    uint256 internal _num = 10;
 
-    function setTest(uint newNum) public returns (uint) {
-        num = newNum;
-        return num;
+    function setTest(uint256 newNum) public returns (uint256) {
+        _num = newNum;
+        return _num;
     }
 
     function fail() public pure {
+        // solhint-disable-next-line reason-string, custom-errors
         revert("ActionExecute:Revert");
     }
 

diff --git a/contracts/src/mocks/executors/GasConsumer.sol b/contracts/src/mocks/executors/GasConsumer.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.8;
 
 /// @notice This contract is used for testing to consume gas.
 contract GasConsumer {
+    // solhint-disable-next-line named-parameters-mapping
     mapping(uint256 => uint256) public store;
 
     function consumeGas(uint256 count) external {

diff --git a/contracts/src/mocks/permission/condition/extensions/RuledConditionMock.sol b/contracts/src/mocks/permission/condition/extensions/RuledConditionMock.sol
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity ^0.8.8;
+import {RuledCondition} from "../../../../permission/condition/extensions/RuledCondition.sol";
+import {DaoAuthorizableUpgradeable} from "../../../../permission/auth/DaoAuthorizableUpgradeable.sol";
+
+/// @notice A mock powerful condition to expose internal functions
+/// @dev DO NOT USE IN PRODUCTION!
+contract RuledConditionMock is DaoAuthorizableUpgradeable, RuledCondition {
+    function updateRules(Rule[] memory _rules) public virtual {
+        _updateRules(_rules);
+    }
+
+    function isGranted(
+        address _where,
+        address _who,
+        bytes32 _permissionId,
+        bytes calldata data
+    ) external view override returns (bool isPermitted) {
+        uint256[] memory _compareList = data.length == 0
+            ? new uint256[](0)
+            : abi.decode(data, (uint256[]));
+        return _evalRule(0, _where, _who, _permissionId, _compareList);
+    }
+}
diff --git a/contracts/src/mocks/plugin/CustomExecutorMock.sol b/contracts/src/mocks/plugin/CustomExecutorMock.sol
@@ -2,7 +2,6 @@
 
 pragma solidity ^0.8.8;
 
-import {IDAO} from "../../dao/IDAO.sol";
 import {IExecutor, Action} from "../../executors/IExecutor.sol";
 
 /// @notice A mock DAO that anyone can set permissions in.

diff --git a/contracts/src/mocks/plugin/PluginCloneableMock.sol b/contracts/src/mocks/plugin/PluginCloneableMock.sol
@@ -5,7 +5,7 @@ pragma solidity ^0.8.8;
 
 import {PluginCloneable} from "../../plugin/PluginCloneable.sol";
 import {IDAO} from "../../dao/IDAO.sol";
-import {IExecutor, Action} from "../../executors/IExecutor.sol";
+import {Action} from "../../executors/IExecutor.sol";
 
 /// @notice A mock cloneable plugin to be deployed via the minimal proxy pattern.
 /// v1.1 (Release 1, Build 1)

diff --git a/contracts/src/mocks/plugin/PluginMock.sol b/contracts/src/mocks/plugin/PluginMock.sol
@@ -4,7 +4,7 @@ pragma solidity ^0.8.8;
 
 import {Plugin} from "../../plugin/Plugin.sol";
 import {IDAO} from "../../dao/IDAO.sol";
-import {IExecutor, Action} from "../../executors/IExecutor.sol";
+import {Action} from "../../executors/IExecutor.sol";
 
 /// @notice A mock plugin to be deployed via the `new` keyword.
 /// v1.1 (Release 1, Build 1)

diff --git a/contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol b/contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol
@@ -5,7 +5,7 @@ pragma solidity ^0.8.8;
 
 import {PluginUUPSUpgradeable} from "../../plugin/PluginUUPSUpgradeable.sol";
 import {IDAO} from "../../dao/IDAO.sol";
-import {IExecutor, Action} from "../../executors/IExecutor.sol";
+import {Action} from "../../executors/IExecutor.sol";
 
 /// @notice A mock upgradeable plugin to be deployed via the UUPS proxy pattern.
 /// v1.1 (Release 1, Build 1)

diff --git a/contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol b/contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol
@@ -3,7 +3,7 @@
 pragma solidity ^0.8.8;
 
 import {Proposal} from "../../../../plugin/extensions/proposal/Proposal.sol";
-import {IExecutor, Action} from "../../../../executors/IExecutor.sol";
+import {Action} from "../../../../executors/IExecutor.sol";
 
 /// @notice A mock contract.
 /// @dev DO NOT USE IN PRODUCTION!

diff --git a/contracts/src/mocks/plugin/extensions/proposal/ProposalUpgradeableMock.sol b/contracts/src/mocks/plugin/extensions/proposal/ProposalUpgradeableMock.sol
@@ -3,7 +3,7 @@
 pragma solidity ^0.8.8;
 
 import {ProposalUpgradeable} from "../../../../plugin/extensions/proposal/ProposalUpgradeable.sol";
-import {IExecutor, Action} from "../../../../executors/IExecutor.sol";
+import {Action} from "../../../../executors/IExecutor.sol";
 
 /// @notice A mock contract.
 /// @dev DO NOT USE IN PRODUCTION!

diff --git a/contracts/src/permission/condition/extensions/AlwaysTrueCondition.sol b/contracts/src/permission/condition/extensions/AlwaysTrueCondition.sol
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.8;
+
+import {PermissionCondition} from "../PermissionCondition.sol";
+
+contract AlwaysTrueCondition is PermissionCondition {
+    function isGranted(
+        address _where,
+        address _who,
+        bytes32 _permissionId,
+        bytes calldata _data
+    ) public pure override returns (bool) {
+        (_where, _who, _permissionId, _data);
+        return true;
+    }
+}