GenericSchemeMultiCall : Simplification. + Schemeconstraint (#795)

orenyodfat · ben-kaufman · web-flow · commit 19682c213667 · 2020-10-14T15:05:42.000+03:00
* simplify contract

* tests

* test approval

* check spender is white listed

* clean

* poc

* Add SchemeConstraints interface

* add eth constraint as an example for DxDaoSchemeConstraint

* +

* more ..

* fix dxdao constraints

* check schemeConstraint exist

* + comments

* coverage

* remove unused var

* more tests

* Bump version

Co-authored-by: benk10 &lt;ben.kaufman10@gmail.com&gt;
diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol
@@ -0,0 +1,148 @@
+pragma solidity 0.5.17;
+pragma experimental ABIEncoderV2;
+
+import "./SchemeConstraints.sol";
+
+
+contract DxDaoSchemeConstraints is SchemeConstraints {
+    using SafeMath for uint256;
+
+    uint256 public initialTimestamp;
+    uint256 public periodSize;
+    uint256 public periodLimitWei;
+
+    mapping(address=>uint256) public periodLimitToken;
+    mapping (uint256 => mapping(address => uint256)) public periodSpendingToken;
+    mapping(uint256=>uint256) public periodSpendingWei;
+    mapping(address=>bool) public contractsWhiteListMap;
+    bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256)
+
+    /* @dev initialize
+     * @param _periodSize the time period to limit the tokens and eth spending
+     * @param _periodLimitWei the limit of eth which can be sent per period
+     * @param _periodLimitTokensAddresses tokens to limit
+     * @param _periodLimitTokensAmounts the limit of token which can be sent per period
+     * @param _contractsWhiteList the contracts the scheme is allowed to interact with
+     */
+    function initialize(
+        uint256 _periodSize,
+        uint256 _periodLimitWei,
+        address[] calldata _periodLimitTokensAddresses,
+        uint256[] calldata _periodLimitTokensAmounts,
+        address[] calldata _contractsWhiteList
+    )
+    external {
+        require(initialTimestamp == 0, "cannot initialize twice");
+        require(_periodSize > 0, "preriod size should be greater than 0");
+        require(_periodLimitTokensAddresses.length == _periodLimitTokensAmounts.length,
+        "invalid length _periodLimitTokensAddresses");
+        periodSize = _periodSize;
+        periodLimitWei = _periodLimitWei;
+        // solhint-disable-next-line not-rely-on-time
+        initialTimestamp = block.timestamp;
+        for (uint i = 0; i < _contractsWhiteList.length; i++) {
+            contractsWhiteListMap[_contractsWhiteList[i]] = true;
+        }
+        for (uint i = 0; i < _periodLimitTokensAmounts.length; i++) {
+            periodLimitToken[_periodLimitTokensAddresses[i]] = _periodLimitTokensAmounts[i];
+        }
+        contractsWhiteList = _contractsWhiteList;
+    }
+
+    /*
+     * @dev isAllowedToCall should be called upon a proposal execution.
+     *  - check that the total spending of tokens within a 'periodSize' does not exceed the periodLimit per token
+     *  - check that the total sending of eth within a 'periodSize' does not exceed the periodLimit
+     * @param _contractsToCall the contracts to be called
+     * @param _callsData - The abi encode data for the calls
+     * @param _values value(ETH) to transfer with the calls
+     * @param _avatar avatar
+     * @return bool value true-allowed false not allowed
+     */
+    function isAllowedToCall(
+        address[] calldata _contractsToCall,
+        bytes[] calldata _callsData,
+        uint256[] calldata _values,
+        Avatar
+    )
+    external
+    returns(bool)
+    {
+
+        uint256 observervationIndex = observationIndex();
+        uint256 totalPeriodSpendingInWei;
+        for (uint i = 0; i < _contractsToCall.length; i++) {
+        // constraint eth transfer
+            totalPeriodSpendingInWei = totalPeriodSpendingInWei.add(_values[i]);
+            bytes memory callData = _callsData[i];
+        // constraint approve calls
+            if (callData[0] == APPROVE_SIGNATURE[0] &&
+                callData[1] == APPROVE_SIGNATURE[1] &&
+                callData[2] == APPROVE_SIGNATURE[2] &&
+                callData[3] == APPROVE_SIGNATURE[3]) {
+                uint256 amount;
+                address contractToCall = _contractsToCall[i];
+                // solhint-disable-next-line no-inline-assembly
+                assembly {
+                    amount := mload(add(callData, 68))
+                }
+                periodSpendingToken[observervationIndex][contractToCall] =
+                periodSpendingToken[observervationIndex][contractToCall].add(amount);
+                require(
+                periodSpendingToken[observervationIndex][contractToCall] <= periodLimitToken[contractToCall],
+                "periodSpendingTokensExceeded");
+            }
+
+        }
+        periodSpendingWei[observervationIndex] =
+        periodSpendingWei[observervationIndex].add(totalPeriodSpendingInWei);
+        require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded");
+        return true;
+    }
+
+    /*
+     * @dev isAllowedToPropose should be called upon a proposal submition.
+     * allow only whitelisted target contracts or 'approve' calls which the 'spender' is whitelisted
+     * @param _contractsToCall the contracts to be called
+     * @param _callsData - The abi encode data for the calls
+     * @param _values value(ETH) to transfer with the calls
+     * @param _avatar avatar
+     * @return bool value true-allowed false not allowed
+     */
+    function isAllowedToPropose(
+        address[] calldata _contractsToCall,
+        bytes[] calldata _callsData,
+        uint256[] calldata,
+        Avatar)
+    external
+    returns(bool)
+    {
+        for (uint i = 0; i < _contractsToCall.length; i++) {
+            if (!contractsWhiteListMap[_contractsToCall[i]]) {
+                address spender;
+                bytes memory callData = _callsData[i];
+                require(
+                    callData[0] == APPROVE_SIGNATURE[0] &&
+                    callData[1] == APPROVE_SIGNATURE[1] &&
+                    callData[2] == APPROVE_SIGNATURE[2] &&
+                    callData[3] == APPROVE_SIGNATURE[3],
+                "allow only approve call for none whitelistedContracts");
+                //in solidity > 6 this can be replaced by:
+                //(spender,) = abi.descode(callData[4:], (address, uint));
+                // see https://github.com/ethereum/solidity/issues/9439
+                // solhint-disable-next-line no-inline-assembly
+                assembly {
+                    spender := mload(add(callData, 36))
+                }
+                require(contractsWhiteListMap[spender], "spender contract not whitelisted");
+            }
+        }
+        return true;
+    }
+
+    function observationIndex() public view returns (uint256) {
+        // solhint-disable-next-line not-rely-on-time
+        return ((block.timestamp - initialTimestamp) / periodSize);
+    }
+
+}
diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol
@@ -4,6 +4,7 @@ pragma experimental ABIEncoderV2;
 import "@daostack/infra/contracts/votingMachines/IntVoteInterface.sol";
 import "@daostack/infra/contracts/votingMachines/ProposalExecuteInterface.sol";
 import "../votingMachines/VotingMachineCallbacks.sol";
+import "./SchemeConstraints.sol";
 
 
 /**
@@ -24,12 +25,10 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
     }
 
     mapping(bytes32=>MultiCallProposal) public proposals;
-
     IntVoteInterface public votingMachine;
     bytes32 public voteParams;
-    mapping(address=>bool) internal contractWhitelist;
-    address[] public whitelistedContracts;
     Avatar public avatar;
+    SchemeConstraints public schemeConstraints;
 
     event NewMultiCallProposal(
         address indexed _avatar,
@@ -61,37 +60,26 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
 
     event ProposalDeleted(address indexed _avatar, bytes32 indexed _proposalId);
 
-    /**
-     * @dev initialize
+    /* @dev initialize
      * @param _avatar the avatar to mint reputation from
      * @param _votingMachine the voting machines address to
      * @param _voteParams voting machine parameters.
-     * @param _contractWhitelist the contracts the scheme is allowed to interact with
-     *
+     * @param _schemeConstraints the schemeConstraints contracts.
      */
     function initialize(
         Avatar _avatar,
         IntVoteInterface _votingMachine,
         bytes32 _voteParams,
-        address[] calldata _contractWhitelist
+        SchemeConstraints _schemeConstraints
     )
     external
     {
         require(avatar == Avatar(0), "can be called only one time");
         require(_avatar != Avatar(0), "avatar cannot be zero");
-        require(_contractWhitelist.length > 0, "contractWhitelist cannot be empty");
         avatar = _avatar;
         votingMachine = _votingMachine;
         voteParams = _voteParams;
-        /* Whitelist controller by default*/
-        Controller controller = Controller(_avatar.owner());
-        whitelistedContracts.push(address(controller));
-        contractWhitelist[address(controller)] = true;
-
-        for (uint i = 0; i < _contractWhitelist.length; i++) {
-            contractWhitelist[_contractWhitelist[i]] = true;
-            whitelistedContracts.push(_contractWhitelist[i]);
-        }
+        schemeConstraints = _schemeConstraints;
     }
 
     /**
@@ -127,28 +115,23 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
         MultiCallProposal storage proposal = proposals[_proposalId];
         require(proposal.exist, "must be a live proposal");
         require(proposal.passed, "proposal must passed by voting machine");
+        if (schemeConstraints != SchemeConstraints(0)) {
+            require(
+            schemeConstraints.isAllowedToCall(
+            proposal.contractsToCall,
+            proposal.callsData,
+            proposal.values,
+            avatar),
+            "call is not allowed");
+        }
         proposal.exist = false;
         bytes memory genericCallReturnValue;
         bool success;
-        Controller controller = Controller(whitelistedContracts[0]);
-
+        Controller controller = Controller(avatar.owner());
         for (uint i = 0; i < proposal.contractsToCall.length; i++) {
             bytes memory callData = proposal.callsData[i];
-            if (proposal.contractsToCall[i] == address(controller)) {
-                (IERC20 extToken,
-                address spender,
-                uint256 valueToSpend
-                ) =
-                abi.decode(
-                    callData,
-                    (IERC20, address, uint256)
-                );
-                success = controller.externalTokenApproval(extToken, spender, valueToSpend, avatar);
-            } else {
-                (success, genericCallReturnValue) =
-                controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]);
-            }
-
+            (success, genericCallReturnValue) =
+            controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]);
             /* Whole transaction will be reverted if at least one call fails*/
             require(success, "Proposal call failed");
             emit ProposalCallExecuted(
@@ -190,19 +173,14 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf
             (_contractsToCall.length == _callsData.length) && (_contractsToCall.length == _values.length),
             "Wrong length of _contractsToCall, _callsDataLens or _values arrays"
         );
-        for (uint i = 0; i < _contractsToCall.length; i++) {
+        if (schemeConstraints != SchemeConstraints(0)) {
             require(
-                contractWhitelist[_contractsToCall[i]], "contractToCall is not whitelisted"
-            );
-            if (_contractsToCall[i] == whitelistedContracts[0]) {
-
-                (, address spender,) =
-                abi.decode(
-                    _callsData[i],
-                    (IERC20, address, uint256)
-                );
-                require(contractWhitelist[spender], "spender contract not whitelisted");
-            }
+            schemeConstraints.isAllowedToPropose(
+            _contractsToCall,
+            _callsData,
+            _values,
+            avatar),
+            "propose is not allowed");
         }
 
         proposalId = votingMachine.propose(2, voteParams, msg.sender, address(avatar));
diff --git a/contracts/schemes/SchemeConstraints.sol b/contracts/schemes/SchemeConstraints.sol
@@ -0,0 +1,44 @@
+pragma solidity 0.5.17;
+pragma experimental ABIEncoderV2;
+import "../controller/Avatar.sol";
+
+
+contract SchemeConstraints {
+
+    address[] public contractsWhiteList;
+
+   /*
+    * @dev isAllowedToCall should be called upon a proposal execution.
+    * @param _contractsToCall the contracts to be called
+    * @param _callsData - The abi encode data for the calls
+    * @param _values value(ETH) to transfer with the calls
+    * @param _avatar avatar
+    * @return bool value true-allowed false not allowed
+    */
+    function isAllowedToCall(
+        address[] calldata _contractsToCall,
+        bytes[] calldata _callsData,
+        uint256[] calldata _values,
+        Avatar _avatar)
+    external returns(bool);
+
+    /*
+     * @dev isAllowedToPropose should be called upon a proposal submition.
+     * @param _contractsToCall the contracts to be called
+     * @param _callsData - The abi encode data for the calls
+     * @param _values value(ETH) to transfer with the calls
+     * @param _avatar avatar
+     * @return bool value true-allowed false not allowed
+     */
+    function isAllowedToPropose(
+        address[] calldata _contractsToCall,
+        bytes[] calldata _callsData,
+        uint256[] calldata _values,
+        Avatar _avatar)
+    external returns(bool);
+
+    function getContractsWhiteList() external view returns(address[] memory) {
+        return contractsWhiteList;
+    }
+
+}
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@daostack/arc",
-  "version": "0.0.1-rc.45",
+  "version": "0.0.1-rc.46",
   "description": "A platform for building DAOs",
   "files": [
     "contracts/",
diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@daostack/arc",`
`3`		`- "version": "0.0.1-rc.45",`
	`3`	`+ "version": "0.0.1-rc.46",`
`4`	`4`	`"description": "A platform for building DAOs",`
`5`	`5`	`"files": [`
`6`	`6`	`"contracts/",`