smart contract work to override permissions by channel (#466)

TESTNET PROPOSAL: https://testnets.llama.xyz/orgs/river/base-sepolia/actions/158  ## Summary by CodeRabbit - **New Features** - Introduced channel permission management with new methods for setting, getting, and clearing channel permission overrides. - Added events for tracking changes in channel role permissions to enhance auditing capabilities. - Enhanced role management functionality by streamlining permission checks and introducing mappings for channel-specific permissions. - **Bug Fixes** - Improved internal checks to prevent operations on non-existent roles, enhancing security and reducing errors. - **Tests** - Added tests for new channel permission features to ensure proper functionality and error handling.  --------- Co-authored-by: Crystal Lemire <[email protected]>
river-build · Aug 10, 2024 · 6909753 · 6909753
1 parent 980e792
commit 6909753
Show file tree

Hide file tree

Showing 56 changed files with 1,461 additions and 162 deletions.
diff --git a/contracts/scripts/deployments/facets/DeployRoles.s.sol b/contracts/scripts/deployments/facets/DeployRoles.s.sol
@@ -22,6 +22,11 @@ contract DeployRoles is FacetHelper, Deployer {
     addSelector(Roles.getPermissionsByRoleId.selector);
     addSelector(Roles.addRoleToEntitlement.selector);
     addSelector(Roles.removeRoleFromEntitlement.selector);
+
+    // channel permission overrides
+    addSelector(Roles.setChannelPermissionOverrides.selector);
+    addSelector(Roles.getChannelPermissionOverrides.selector);
+    addSelector(Roles.clearChannelPermissionOverrides.selector);
   }
 
   function versionName() public pure override returns (string memory) {

diff --git a/contracts/src/spaces/facets/channels/ChannelService.sol b/contracts/src/spaces/facets/channels/ChannelService.sol
@@ -167,7 +167,6 @@ library ChannelService {
     bytes32 channelId
   ) internal view returns (uint256[] memory) {
     checkChannelExists(channelId);
-
     ChannelStorage.Layout storage channel = ChannelStorage.layout();
     return channel.rolesByChannelId[channelId].values();
   }

diff --git a/contracts/src/spaces/facets/entitlements/extensions/EntitlementDataQueryable.sol b/contracts/src/spaces/facets/entitlements/extensions/EntitlementDataQueryable.sol
@@ -8,6 +8,8 @@ import {IEntitlement} from "contracts/src/spaces/entitlements/IEntitlement.sol";
 import {IEntitlementGatedBase} from "contracts/src/spaces/facets/gated/IEntitlementGated.sol";
 
 // libraries
+import {StringSet} from "contracts/src/utils/StringSet.sol";
+import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 import {ChannelService} from "contracts/src/spaces/facets/channels/ChannelService.sol";
 import {EntitlementGatedStorage} from "contracts/src/spaces/facets/gated/EntitlementGatedStorage.sol";
 import {RolesStorage} from "contracts/src/spaces/facets/roles/RolesStorage.sol";
@@ -25,6 +27,7 @@ contract EntitlementDataQueryable is
   Facet
 {
   using StringSet for StringSet.Set;
+  using EnumerableSet for EnumerableSet.Bytes32Set;
 
   function getEntitlementDataByPermission(
     string calldata permission
@@ -67,38 +70,51 @@ contract EntitlementDataQueryable is
     bytes32 channelId,
     string calldata permission
   ) internal view returns (Role[] memory) {
+    // retrive the roles associated with the channel
     uint256[] memory channelRoles = ChannelService.getRolesByChannel(channelId);
     uint256 channelRolesLength = channelRoles.length;
 
-    uint256 roleCount = 0;
+    // initialize arrays to store the matching role IDs
     uint256[] memory matchedRoleIds = new uint256[](channelRolesLength);
+    uint256 matchedRoleCount = 0;
 
-    RolesStorage.Layout storage ds = RolesStorage.layout();
+    // access roles storage layout
+    RolesStorage.Layout storage rs = RolesStorage.layout();
 
-    // Count the number of roles that have the requested permission and record their ids.
+    // iterate through channel roles and check for the requested permission
     for (uint256 i; i < channelRolesLength; i++) {
       uint256 roleId = channelRoles[i];
 
-      RolesStorage.Role storage role = ds.roleById[roleId];
+      RolesStorage.Role storage role = rs.roleById[channelRoles[i]];
+
+      bool hasPermission = false;
 
-      if (role.isImmutable) {
-        continue;
+      // check if role is associated with the channel and has the requested permission
+      if (rs.channelOverridesByRole[roleId].contains(channelId)) {
+        StringSet.Set storage permissions = rs.permissionOverridesByRole[
+          roleId
+        ][channelId];
+        hasPermission = permissions.contains(permission);
+      }
+      // check the default permissions if this role didn't have a channel override.
+      else if (role.permissions.contains(permission)) {
+        hasPermission = true;
       }
 
-      // Check if the role has the requested permission.
-      if (role.permissions.contains(permission)) {
-        matchedRoleIds[roleCount] = roleId;
-        roleCount++;
+      // store the role ID if it has the requested permission
+      if (hasPermission) {
+        matchedRoleIds[matchedRoleCount] = roleId;
+        matchedRoleCount++;
       }
     }
 
-    // Assemble the roles that have the requested permission for the specified channel.
-    Role[] memory roles = new Role[](roleCount);
-    for (uint256 i; i < roleCount; i++) {
-      roles[i] = _getRoleById(matchedRoleIds[i]);
+    // create an array of roles with the matching IDs
+    Role[] memory rolesWithPermission = new Role[](matchedRoleCount);
+    for (uint256 i; i < matchedRoleCount; i++) {
+      rolesWithPermission[i] = _getRoleById(matchedRoleIds[i]);
     }
 
-    return roles;
+    return rolesWithPermission;
   }
 
   function _getEntitlements(
@@ -107,36 +123,33 @@ contract EntitlementDataQueryable is
     uint256 entitlementCount;
     uint256 rolesLength = roles.length;
 
-    for (uint256 i = 0; i < rolesLength; i++) {
-      Role memory role = roles[i];
-
-      if (!role.disabled) {
-        entitlementCount += role.entitlements.length;
+    for (uint256 i; i < rolesLength; i++) {
+      if (!roles[i].disabled) {
+        entitlementCount += roles[i].entitlements.length;
       }
     }
 
     EntitlementData[] memory entitlementData = new EntitlementData[](
       entitlementCount
     );
 
-    entitlementCount = 0;
+    uint256 currentIndex = 0;
 
     for (uint256 i; i < rolesLength; i++) {
-      Role memory role = roles[i];
-
-      if (!role.disabled) {
-        for (uint256 j; j < role.entitlements.length; j++) {
-          IEntitlement entitlement = IEntitlement(role.entitlements[j]);
+      if (!roles[i].disabled) {
+        for (uint256 j; j < roles[i].entitlements.length; j++) {
+          IEntitlement entitlement = IEntitlement(roles[i].entitlements[j]);
 
-          entitlementData[entitlementCount] = EntitlementData(
+          entitlementData[currentIndex] = EntitlementData(
             entitlement.moduleType(),
-            entitlement.getEntitlementDataByRoleId(role.id)
+            entitlement.getEntitlementDataByRoleId(roles[i].id)
           );
 
-          entitlementCount++;
+          currentIndex++;
         }
       }
     }
+
     return entitlementData;
   }
 }
diff --git a/contracts/src/spaces/facets/roles/IRoles.sol b/contracts/src/spaces/facets/roles/IRoles.sol
@@ -27,6 +27,24 @@ interface IRolesBase {
 
   event RoleRemoved(address indexed remover, uint256 indexed roleId);
 
+  event PermissionsAddedToChannelRole(
+    address indexed updater,
+    uint256 indexed roleId,
+    bytes32 indexed channelId
+  );
+
+  event PermissionsRemovedFromChannelRole(
+    address indexed updater,
+    uint256 indexed roleId,
+    bytes32 indexed channelId
+  );
+
+  event PermissionsUpdatedForChannelRole(
+    address indexed updater,
+    uint256 indexed roleId,
+    bytes32 indexed channelId
+  );
+
   // =============================================================
   //                           Errors
   // =============================================================

diff --git a/contracts/src/spaces/facets/roles/Roles.sol b/contracts/src/spaces/facets/roles/Roles.sol
@@ -39,24 +39,20 @@ contract Roles is IRoles, RolesBase, Entitled {
     CreateEntitlement[] memory entitlements
   ) external override {
     _validatePermission(Permissions.ModifyRoles);
-    _checkRoleExists(roleId);
     _updateRole(roleId, roleName, permissions, entitlements);
   }
 
   function removeRole(uint256 roleId) external override {
     _validatePermission(Permissions.ModifyRoles);
-    _checkRoleExists(roleId);
     _removeRole(roleId);
   }
 
   // permissions
-
   function addPermissionsToRole(
     uint256 roleId,
     string[] memory permissions
   ) external override {
     _validatePermission(Permissions.ModifyRoles);
-    _checkRoleExists(roleId);
     _addPermissionsToRole(roleId, permissions);
   }
 
@@ -65,7 +61,6 @@ contract Roles is IRoles, RolesBase, Entitled {
     string[] memory permissions
   ) external override {
     _validatePermission(Permissions.ModifyRoles);
-    _checkRoleExists(roleId);
     _removePermissionsFromRole(roleId, permissions);
   }
 
@@ -81,7 +76,6 @@ contract Roles is IRoles, RolesBase, Entitled {
     CreateEntitlement memory entitlement
   ) external {
     _validatePermission(Permissions.ModifyRoles);
-    _checkRoleExists(roleId);
     _addRoleToEntitlement(roleId, entitlement);
   }
 
@@ -90,7 +84,31 @@ contract Roles is IRoles, RolesBase, Entitled {
     CreateEntitlement memory entitlement
   ) external {
     _validatePermission(Permissions.ModifyRoles);
-    _checkRoleExists(roleId);
     _removeRoleFromEntitlement(roleId, entitlement);
   }
+
+  // custom channel permission overrides
+  function setChannelPermissionOverrides(
+    uint256 roleId,
+    bytes32 channelId,
+    string[] memory permissions
+  ) external {
+    _validatePermission(Permissions.ModifyRoles);
+    _setChannelPermissionOverrides(roleId, channelId, permissions);
+  }
+
+  function getChannelPermissionOverrides(
+    uint256 roleId,
+    bytes32 channelId
+  ) external view returns (string[] memory permissions) {
+    return _getChannelPermissionOverrides(roleId, channelId);
+  }
+
+  function clearChannelPermissionOverrides(
+    uint256 roleId,
+    bytes32 channelId
+  ) external {
+    _validatePermission(Permissions.ModifyRoles);
+    _clearChannelPermissionOverrides(roleId, channelId);
+  }
 }
diff --git a/contracts/src/spaces/facets/roles/RolesBase.sol b/contracts/src/spaces/facets/roles/RolesBase.sol
@@ -164,6 +164,9 @@ abstract contract RolesBase is IRolesBase {
     string[] memory permissions,
     CreateEntitlement[] memory entitlements
   ) internal {
+    // check role exists
+    _checkRoleExists(roleId);
+
     // get current entitlements before updating them
     IEntitlement[] memory currentEntitlements = _getEntitlementsByRole(roleId);
     uint256 currentEntitlementsLen = currentEntitlements.length;
@@ -259,6 +262,9 @@ abstract contract RolesBase is IRolesBase {
   }
 
   function _removeRole(uint256 roleId) internal {
+    // check role exists
+    _checkRoleExists(roleId);
+
     // get current entitlements
     IEntitlement[] memory currentEntitlements = _getEntitlementsByRole(roleId);
     uint256 currentEntitlementsLen = currentEntitlements.length;
@@ -318,6 +324,89 @@ abstract contract RolesBase is IRolesBase {
     emit RoleRemoved(msg.sender, roleId);
   }
 
+  // =============================================================
+  // Channel Permissions
+  // =============================================================
+  function _getChannelPermissionOverrides(
+    uint256 roleId,
+    bytes32 channelId
+  ) internal view returns (string[] memory permissions) {
+    // check role exists
+    _checkRoleExists(roleId);
+
+    // check channel exists
+    ChannelService.checkChannelExists(channelId);
+
+    return
+      RolesStorage
+      .layout()
+      .permissionOverridesByRole[roleId][channelId].values();
+  }
+
+  function _setChannelPermissionOverrides(
+    uint256 roleId,
+    bytes32 channelId,
+    string[] memory permissions
+  ) internal {
+    ChannelService.checkChannelExists(channelId);
+
+    // check role exists
+    _checkRoleExists(roleId);
+
+    RolesStorage.Layout storage rs = RolesStorage.layout();
+
+    rs.channelOverridesByRole[roleId].add(channelId);
+
+    StringSet.Set storage permissionsSet = rs.permissionOverridesByRole[roleId][
+      channelId
+    ];
+
+    // remove current channel permissions if any
+    if (permissionsSet.length() > 0) {
+      string[] memory currentPermissions = permissionsSet.values();
+      uint256 currentPermissionsLen = currentPermissions.length;
+      for (uint256 i = 0; i < currentPermissionsLen; i++) {
+        permissionsSet.remove(currentPermissions[i]);
+      }
+    }
+
+    // check if new permissions are not empty then add them
+    uint256 permissionsLen = permissions.length;
+    for (uint256 i = 0; i < permissionsLen; i++) {
+      _checkEmptyString(permissions[i]);
+      permissionsSet.add(permissions[i]);
+    }
+
+    emit PermissionsAddedToChannelRole(msg.sender, roleId, channelId);
+  }
+
+  function _clearChannelPermissionOverrides(
+    uint256 roleId,
+    bytes32 channelId
+  ) internal {
+    // check role exists
+    _checkRoleExists(roleId);
+
+    // check channel exists
+    ChannelService.checkChannelExists(channelId);
+
+    RolesStorage.Layout storage rs = RolesStorage.layout();
+    StringSet.Set storage permissionsSet = rs.permissionOverridesByRole[roleId][
+      channelId
+    ];
+
+    // get current permissions
+    string[] memory currentPermissions = permissionsSet.values();
+    uint256 currentPermissionsLen = currentPermissions.length;
+    for (uint256 i = 0; i < currentPermissionsLen; i++) {
+      permissionsSet.remove(currentPermissions[i]);
+    }
+
+    rs.channelOverridesByRole[roleId].remove(channelId);
+
+    emit PermissionsRemovedFromChannelRole(msg.sender, roleId, channelId);
+  }
+
   // =============================================================
   //                           Internals
   // =============================================================
@@ -413,6 +502,9 @@ abstract contract RolesBase is IRolesBase {
     uint256 roleId,
     string[] memory permissions
   ) internal {
+    // check role exists
+    _checkRoleExists(roleId);
+
     RolesStorage.Layout storage rs = RolesStorage.layout();
 
     uint256 permissionLen = permissions.length;
@@ -438,6 +530,9 @@ abstract contract RolesBase is IRolesBase {
     uint256 roleId,
     string[] memory permissions
   ) internal {
+    // check role exists
+    _checkRoleExists(roleId);
+
     // check permissions
     RolesStorage.Layout storage rs = RolesStorage.layout();
 
@@ -496,6 +591,9 @@ abstract contract RolesBase is IRolesBase {
     uint256 roleId,
     CreateEntitlement memory entitlement
   ) internal {
+    // check role exists
+    _checkRoleExists(roleId);
+
     // check entitlements exists
     EntitlementsManagerService.checkEntitlement(address(entitlement.module));