Merge pull request #34 from dl-solidity-library/dev

Dev

Author: Arvolear

README.md

@@ -7,12 +7,13 @@
 
 **Elaborate solidity development modules library by DL.**
 
-The library consist of modules and utilities that are built on top of [Openzeppelin Contracts](https://github.com/OpenZeppelin/openzeppelin-contracts) but goes far beyond the mediocre solidity. 
+The library consists of modules and utilities that are built on top of [Openzeppelin Contracts](https://github.com/OpenZeppelin/openzeppelin-contracts) but go far beyond mediocre solidity.
 
 - Implementation of [**Contracts Registry**](https://eips.ethereum.org/EIPS/eip-6224) pattern
 - Versatile **RBAC** smart contract
 - Enhanced and simplified [**Diamond**](https://eips.ethereum.org/EIPS/eip-2535) pattern
-- Utilities to ease work with ERC20 decimals, arrays and sets
+- Heap based priority queue library
+- Utilities to ease work with ERC20 decimals, arrays, and sets
 
 ## Overview
 
@@ -26,7 +27,7 @@ The latest stable version is always in the `master` branch.
 
 ## Usage
 
-You will find the smart contracts in the `/contracts` directory. Fell free to play around and check the source code, it is rather descriptive.
+You will find the smart contracts in the `/contracts` directory. Feel free to play around and check the source code, it is rather descriptive.
 
 Once the [npm package](https://www.npmjs.com/package/@dlsl/dev-modules) is installed, one can use the modules just like that:
 

contracts/access-control/MerkleWhitelisted.sol

@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.4;
+
+import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
+
+/**
+ *  @notice The Whitelist Access Control module
+ *
+ *  This is a simple abstract contract that implements whitelisting logic.
+ *
+ *  The contract is based on a Merkle tree, which allows for the huge whitelists to be cheaply validated courtesy of
+ *  O(log(n)) tree complexity. The whitelist itself is stored in the tree leaves and only the root of the tree is saved on-chain.
+ *
+ *  To validate the whitelist belonging, the tree leaf (the whitelist element) has to be computed and passed to the
+ *  "root-construction" function together with the corresponding tree branches. The function will then check the
+ *  roots equality. If the roots match, the element belongs to the whitelist.
+ *
+ *  Note: the branch nodes are sorted numerically.
+ */
+abstract contract MerkleWhitelisted {
+    using MerkleProof for bytes32[];
+
+    bytes32 private _merkleRoot;
+
+    modifier onlyWhitelisted(bytes memory data_, bytes32[] calldata merkleProof_) {
+        require(
+            isWhitelisted(keccak256(data_), merkleProof_),
+            "MerkleWhitelisted: not whitelisted"
+        );
+        _;
+    }
+
+    modifier onlyWhitelistedUser(address user_, bytes32[] calldata merkleProof_) {
+        require(isWhitelistedUser(user_, merkleProof_), "MerkleWhitelisted: not whitelisted");
+        _;
+    }
+
+    /**
+     *  @notice The function to check if the leaf belongs to the Merkle tree
+     *  @param leaf_ the leaf to be checked
+     *  @param merkleProof_ the path from the leaf to the Merkle tree root
+     *  @return true if the leaf belongs to the Merkle tree, false otherwise
+     */
+    function isWhitelisted(
+        bytes32 leaf_,
+        bytes32[] calldata merkleProof_
+    ) public view returns (bool) {
+        return merkleProof_.verifyCalldata(_merkleRoot, leaf_);
+    }
+
+    /**
+     *  @notice The function to check if the user belongs to the Merkle tree
+     *  @param user_ the user to be checked
+     *  @param merkleProof_ the path from the user to the Merkle tree root
+     *  @return true if the user belongs to the Merkle tree, false otherwise
+     */
+    function isWhitelistedUser(
+        address user_,
+        bytes32[] calldata merkleProof_
+    ) public view returns (bool) {
+        return isWhitelisted(keccak256(abi.encodePacked(user_)), merkleProof_);
+    }
+
+    /**
+     *  @notice The function to get the current Merkle root
+     *  @return the current Merkle root or zero bytes if it has not been set
+     */
+    function getMerkleRoot() public view returns (bytes32) {
+        return _merkleRoot;
+    }
+
+    /**
+     *  @notice The function that should be called from the child contract to set the Merkle root
+     *  @param merkleRoot_ the Merkle root to be set
+     */
+    function _setMerkleRoot(bytes32 merkleRoot_) internal {
+        _merkleRoot = merkleRoot_;
+    }
+}

contracts/access-control/RBAC.sol

@@ -133,7 +133,7 @@ abstract contract RBAC is IRBAC, Initializable {
     /**
      *  @notice The function to get the list of user roles
      *  @param who_ the user
-     *  @return roles_ the roes of the user
+     *  @return roles_ the roles of the user
      */
     function getUserRoles(address who_) public view override returns (string[] memory roles_) {
         return _userRoles[who_].values();
@@ -181,51 +181,30 @@ abstract contract RBAC is IRBAC, Initializable {
     }
 
     /**
-     *  @notice The function to check the user's possesion of the role
+     *  @dev DO NOT call `super.hasPermission(...)` in derived contracts, because this method
+     *  handles not 2 but 3 states: NO PERMISSION, ALLOWED, DISALLOWED
+     *  @notice The function to check the user's possession of the role
      *  @param who_ the user
      *  @param resource_ the resource the user has to have the permission of
      *  @param permission_ the permission the user has to have
-     *  @return true_ if user has the permission, false otherwise
+     *  @return isAllowed_ true if the user has the permission, false otherwise
      */
     function hasPermission(
         address who_,
         string memory resource_,
         string memory permission_
-    ) public view override returns (bool) {
-        StringSet.Set storage _roles = _userRoles[who_];
+    ) public view virtual override returns (bool isAllowed_) {
+        string[] memory roles_ = getUserRoles(who_);
 
-        uint256 length_ = _roles.length();
-        bool isAllowed_;
+        for (uint256 i = 0; i < roles_.length; i++) {
+            string memory role_ = roles_[i];
 
-        for (uint256 i = 0; i < length_; i++) {
-            string memory role_ = _roles.at(i);
-
-            StringSet.Set storage _allDisallowed = _rolePermissions[role_][false][ALL_RESOURCE];
-            StringSet.Set storage _allAllowed = _rolePermissions[role_][true][ALL_RESOURCE];
-
-            StringSet.Set storage _disallowed = _rolePermissions[role_][false][resource_];
-            StringSet.Set storage _allowed = _rolePermissions[role_][true][resource_];
-
-            if (
-                _allDisallowed.contains(ALL_PERMISSION) ||
-                _allDisallowed.contains(permission_) ||
-                _disallowed.contains(ALL_PERMISSION) ||
-                _disallowed.contains(permission_)
-            ) {
+            if (_isDisallowed(role_, resource_, permission_)) {
                 return false;
             }
 
-            if (
-                _allAllowed.contains(ALL_PERMISSION) ||
-                _allAllowed.contains(permission_) ||
-                _allowed.contains(ALL_PERMISSION) ||
-                _allowed.contains(permission_)
-            ) {
-                isAllowed_ = true;
-            }
+            isAllowed_ = isAllowed_ || _isAllowed(role_, resource_, permission_);
         }
-
-        return isAllowed_;
     }
 
     /**
@@ -296,4 +275,50 @@ abstract contract RBAC is IRBAC, Initializable {
 
         emit RemovedPermissions(role_, resourceToRemove_, permissionsToRemove_, allowed_);
     }
+
+    /**
+     *  @notice The function to check if the role has the permission
+     *  @param role_ the role to search the permission in
+     *  @param resource_ the role resource to search the permission in
+     *  @param permission_ the permission to search
+     *  @return true_ if the role has the permission, false otherwise
+     */
+    function _isAllowed(
+        string memory role_,
+        string memory resource_,
+        string memory permission_
+    ) internal view returns (bool) {
+        mapping(string => StringSet.Set) storage _resources = _rolePermissions[role_][true];
+
+        StringSet.Set storage _allAllowed = _resources[ALL_RESOURCE];
+        StringSet.Set storage _allowed = _resources[resource_];
+
+        return (_allAllowed.contains(ALL_PERMISSION) ||
+            _allAllowed.contains(permission_) ||
+            _allowed.contains(ALL_PERMISSION) ||
+            _allowed.contains(permission_));
+    }
+
+    /**
+     *  @notice The function to check if the role has the antipermission
+     *  @param role_ the role to search the antipermission in
+     *  @param resource_ the role resource to search the antipermission in
+     *  @param permission_ the antipermission to search
+     *  @return true_ if the role has the antipermission, false otherwise
+     */
+    function _isDisallowed(
+        string memory role_,
+        string memory resource_,
+        string memory permission_
+    ) internal view returns (bool) {
+        mapping(string => StringSet.Set) storage _resources = _rolePermissions[role_][false];
+
+        StringSet.Set storage _allDisallowed = _resources[ALL_RESOURCE];
+        StringSet.Set storage _disallowed = _resources[resource_];
+
+        return (_allDisallowed.contains(ALL_PERMISSION) ||
+            _allDisallowed.contains(permission_) ||
+            _disallowed.contains(ALL_PERMISSION) ||
+            _disallowed.contains(permission_));
+    }
 }