From 460851f33ed6abe1fe33f506b7eabada4a8ccc0f Mon Sep 17 00:00:00 2001
From: kingster-will <83567446+kingster-will@users.noreply.github.com>
Date: Mon, 29 Jan 2024 13:07:57 -0800
Subject: [PATCH] Encode Permission Parameters as byte32 keys in Permissions
 Mapping  (#45)

Encode all the permission parameters into a bytes32 key using the keccak256 hashing function and the abi.encodePacked function.
---
 contracts/AccessController.sol | 64 +++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/contracts/AccessController.sol b/contracts/AccessController.sol
index fc11863cc..351159bb9 100644
--- a/contracts/AccessController.sol
+++ b/contracts/AccessController.sol
@@ -33,7 +33,9 @@ contract AccessController is IAccessController, Governable {
     address public IP_ACCOUNT_REGISTRY;
     address public MODULE_REGISTRY;
 
-    mapping(address => mapping(address => mapping(address => mapping(bytes4 => uint8)))) public permissions;
+    /// @dev encoded permission => permission
+    /// encoded permission = keccak256(abi.encodePacked(ipAccount, signer, to, func))
+    mapping(bytes32 => uint8) public permissions;
 
     constructor(address governance) Governable(governance) {}
 
@@ -56,7 +58,8 @@ contract AccessController is IAccessController, Governable {
         if (permission_ > 2) {
             revert Errors.AccessController__PermissionIsNotValid();
         }
-        permissions[address(0)][signer_][to_][func_] = permission_;
+        _setPermission(address(0), signer_, to_, func_, permission_);
+        emit PermissionSet(address(0), signer_, to_, func_, permission_);
     }
 
     /// @notice Sets the permission for a specific function call
@@ -95,26 +98,11 @@ contract AccessController is IAccessController, Governable {
         if (!IModuleRegistry(MODULE_REGISTRY).isRegistered(msg.sender) && ipAccount_ != msg.sender) {
             revert Errors.AccessController__CallerIsNotIPAccount();
         }
-        permissions[ipAccount_][signer_][to_][func_] = permission_;
+        _setPermission(ipAccount_, signer_, to_, func_, permission_);
 
         emit PermissionSet(ipAccount_, signer_, to_, func_, permission_);
     }
 
-    /// @notice Returns the permission level for a specific function call.
-    /// @param ipAccount_ The account that owns the IP.
-    /// @param signer_ The account that signs the transaction.
-    /// @param to_ The recipient of the transaction.
-    /// @param func_ The function selector.
-    /// @return The permission level for the specific function call.
-    function getPermission(
-        address ipAccount_,
-        address signer_,
-        address to_,
-        bytes4 func_
-    ) external view returns (uint8) {
-        return permissions[ipAccount_][signer_][to_][func_];
-    }
-
     /// @notice Checks if a specific function call is allowed.
     /// @dev This function checks the permission level for a specific function call.
     /// If a specific permission is set, it overrides the general (wildcard) permission.
@@ -143,29 +131,55 @@ contract AccessController is IAccessController, Governable {
         if (IIPAccount(payable(ipAccount_)).owner() == signer_) {
             return true;
         }
-
+        uint functionPermission = getPermission(ipAccount_, signer_, to_, func_);
         // Specific function permission overrides wildcard/general permission
-        if (permissions[ipAccount_][signer_][to_][func_] == AccessPermission.ALLOW) {
+        if (functionPermission == AccessPermission.ALLOW) {
             return true;
         }
 
         // If specific function permission is ABSTAIN, check module level permission
-        if (permissions[ipAccount_][signer_][to_][func_] == AccessPermission.ABSTAIN) {
+        if (functionPermission == AccessPermission.ABSTAIN) {
+            uint8 modulePermission = getPermission(ipAccount_, signer_, to_, bytes4(0));
             // Return true if allow to call all functions of the module
-            if (permissions[ipAccount_][signer_][to_][bytes4(0)] == AccessPermission.ALLOW) {
+            if (modulePermission == AccessPermission.ALLOW) {
                 return true;
             }
             // If module level permission is ABSTAIN, check transaction signer level permission
-            if (permissions[ipAccount_][signer_][to_][bytes4(0)] == AccessPermission.ABSTAIN) {
-                if (permissions[address(0)][signer_][to_][func_] == AccessPermission.ALLOW) {
+            if (modulePermission == AccessPermission.ABSTAIN) {
+                if (getPermission(address(0), signer_, to_, func_) == AccessPermission.ALLOW) {
                     return true;
                 }
                 // Return true if the ipAccount allow the signer can call all functions of all modules
                 // Otherwise, return false
-                return permissions[ipAccount_][signer_][address(0)][bytes4(0)] == AccessPermission.ALLOW;
+                return getPermission(ipAccount_, signer_, address(0), bytes4(0)) == AccessPermission.ALLOW;
             }
             return false;
         }
         return false;
     }
+
+    /// @notice Returns the permission level for a specific function call.
+    /// @param ipAccount The account that owns the IP.
+    /// @param signer The account that signs the transaction.
+    /// @param to The recipient of the transaction.
+    /// @param func The function selector.
+    /// @return The permission level for the specific function call.
+    function getPermission(address ipAccount, address signer, address to, bytes4 func) public view returns (uint8) {
+        return permissions[_encodePermission(ipAccount, signer, to, func)];
+    }
+
+    /// @dev the permission parameters will be encoded into bytes32 as key in the permissions mapping to save storage
+    function _setPermission(address ipAccount, address signer, address to, bytes4 func, uint8 permission) internal {
+        permissions[_encodePermission(ipAccount, signer, to, func)] = permission;
+    }
+
+    /// @dev encode permission to hash (bytes32)
+    function _encodePermission(
+        address ipAccount,
+        address signer,
+        address to,
+        bytes4 func
+    ) internal pure returns (bytes32) {
+        return keccak256(abi.encodePacked(ipAccount, signer, to, func));
+    }
 }