diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6cecbc92f..8a8f8f30a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -58,6 +58,12 @@ jobs: forge test -vvv --fork-url https://eth.drpc.org --fork-block-number 18613489 id: forge-test + - name: Run solhint + run: npx solhint contracts/**/*.sol + + - name: Run solhint + run: npx solhint contracts/*.sol + # - name: Gas Difference # run: # forge snapshot --gas-report --diff --desc diff --git a/.prettierignore b/.prettierignore index cf5eb8c1b..90ca9a19b 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,7 +1,7 @@ build coverage out -lib +/lib assets node_modules .next diff --git a/contracts/AccessController.sol b/contracts/AccessController.sol index 57f454421..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. @@ -124,6 +112,7 @@ contract AccessController is IAccessController, Governable { /// @param to_ The recipient of the transaction. /// @param func_ The function selector. /// @return True if the function call is allowed, false otherwise. + // solhint-disable code-complexity function checkPermission( address ipAccount_, address signer_, @@ -142,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)); + } }