From fd093ecebb5c698b9afd95b044a86077c593c9be Mon Sep 17 00:00:00 2001 From: Leo Date: Sun, 27 Nov 2022 00:36:25 +0100 Subject: [PATCH 1/4] feat: add access basics --- src/access/AccessControl.huff | 162 +++++++++++++++++++++++++ src/access/IAccessControl.sol | 4 + src/access/Ownable.huff | 2 +- src/token/ERC20/ERC20.huff | 4 +- test/access/AccessControl.t.sol | 26 ++++ test/interfaces/ITestAccessControl.sol | 3 + 6 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 src/access/AccessControl.huff create mode 100644 src/access/IAccessControl.sol create mode 100644 test/access/AccessControl.t.sol create mode 100644 test/interfaces/ITestAccessControl.sol diff --git a/src/access/AccessControl.huff b/src/access/AccessControl.huff new file mode 100644 index 0000000..a3c6e9f --- /dev/null +++ b/src/access/AccessControl.huff @@ -0,0 +1,162 @@ +/// @title AccessControl +/// @author Yashiru +/// @dev Contract module that allows children to implement role-based access control +/// mechanisms. +/// +/// Roles are referred to by their `bytes32` identifier. These should be exposed +/// in the external API and be unique. The best way to achieve this is by +/// using `constant`: +/// +/// ``` +/// #define constant ADMIN_ROLE = 0xa49807205ce4d355092ef5a8a18f56e8913cf4a201fbe287825b095693c21775 +/// ``` +/// +/// Roles can be used to represent a set of permissions. To restrict access to a +/// function call, use hasRole() in your MAIN jumps: +/// +/// ``` +/// jumpDest1: +/// MACRO_ONE() +/// jumpDest2: +/// MACRO_TWO() +/// jumpDest3: +/// HAS_ROLE(caller, ADMIN_ROLE) +/// MACRO_THREE() +/// ``` +/// +/// Roles can be granted and revoked dynamically via the GRANT_ROLE_ROLE and +/// REVOKE_ROLE macros. Each role has an associated admin role, and only +/// accounts that have a role's admin role can call GRANT_ROLE_ROLE and REVOKE_ROLE. +/// +/// By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means +/// that only accounts with this role will be able to grant or revoke other +/// roles. More complex role relationships can be created by using +/// _SET_ROLE_ADMIN. +/// +/// WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to +/// grant and revoke this role. Extra precautions should be taken to secure +/// accounts that have been granted it. + +/* Imports */ +#include "../utils/Map.huff" +#include "../utils/Require.huff" + +#define function test() nonpayable returns () + +/// @dev Default admin role +#define constant DEFAULT_ADMIN_ROLE = 0x01 + +/* Custom errors */ +#define error CallerDoesNotHaveRequiredRole() + +/* CONSTRUCTOR */ +#define macro ACCESS_CONSTRUCTOR() = takes (0) returns (0) { + 0x01 [DEFAULT_ADMIN_ROLE] caller STORE_ELEMENT_FROM_KEYS(0x00) +} + +/// @dev Macro that checks that the caller has a specific role. +/// Reverts with a standardized message including the required role. +/// @param role The role that the caller must have +#define macro ONLY_ROLE(role) = takes(1) returns (0) { + caller + CHECK_ROLE() +} + +/// @dev Macro that grant a role to an account. The caller must be an admin +/// of the role to grant. +/// +/// @param role The role to grant to the target account +#define macro GRANT_ROLE(role) = takes(1) returns (0) { + // Input stack: [account] + _GET_ROLE_ADMIN() // [role_admin, account] + caller // [caller, role_admin, account] + CHECK_ROLE() // [account] + + // [role_to_grant, account] + 0x01 // [0x01, role_to_grant, account] + swap2 // [account, role_to_grant, 0x01] + + STORE_ELEMENT_FROM_KEYS(0x00) // [] +} + +/// @dev Macro that revoke a role to an account. The caller must be an admin +/// of the role to revoke. +/// +/// @param role The role to revoke to the target account +#define macro REVOKE_ROLE(role) = takes(1) returns (0) { + // Input stack: [account] + _GET_ROLE_ADMIN() // [role_admin, account] + caller // [caller, role_admin, account] + CHECK_ROLE() // [account] + + // [role_to_revoke, account] + 0x00 // [0x00, role_to_revoke, account] + swap2 // [account, role_to_revoke, 0x01] + + STORE_ELEMENT_FROM_KEYS(0x00) // [] +} + +/// @dev Macro that allow caller to renouce his role a role to an account. +/// The caller must have the role he want to renounce. +/// +/// @param role The role the caller want to renounce +#define macro RENOUNCE_ROLE(role) = takes(1) returns (0) { + // [role] + caller // [caller, role] + CHECK_ROLE() // [] + + 0x00 // [0x00] + // [role, 0x00] + caller // [caller, role, 0x00] + + STORE_ELEMENT_FROM_KEYS(0x00) // [] +} + +/// @dev Macro that checks that an account has a specific role. +/// Reverts with a standardized message including the required role. +/// +#define macro CHECK_ROLE() = takes(2) returns (0) { + /// Input stack: [account, role] + dup1 swap2 swap1 // [account, role, account] + LOAD_ELEMENT_FROM_KEYS(0x00) // [account_has_role, account] + + swap1 [DEFAULT_ADMIN_ROLE] swap1 // [account, default_admin_role, account_has_role] + LOAD_ELEMENT_FROM_KEYS(0x00) // [account_is_admin, account_has_role] + + or // [acount_is_admin_or_has_role] + + __ERROR(CallerDoesNotHaveRequiredRole) swap1 + REQUIRE() +} + +/// @dev Macro that push onto the stack 0 or 1 depending on if the account +/// has a specific role. +#define macro HAS_ROLE(role) = takes(1) returns (1) { + // Input stack: [account] + swap1 // [account, role] + LOAD_ELEMENT_FROM_KEYS(0x00) // [account_has_role] +} + +/// @dev Macro that push onto the the admin role of a role. +#define macro GET_ROLE_ADMIN(role) = takes(0) returns (1) { + // [role] + LOAD_ELEMENT(0x00) // [role_admin] +} + +/// @dev Macro that push onto the the admin role of a role. +#define macro SET_ROLE_ADMIN(role) = takes(0) returns (1) { + // Input stack: [role_admin] + [DEFAULT_ADMIN_ROLE] // [default_admin_role, role_admin] + caller // [caller, default_admin_role, role_admin] + CHECK_ROLE() // [, role_admin] + + // [role, role_admin] + STORE_ELEMENT(0x00) // [] +} + +#define macro MAIN() = takes(0) returns(0) { + ACCESS_CONSTRUCTOR() + 0x654131 + caller + CHECK_ROLE() +} \ No newline at end of file diff --git a/src/access/IAccessControl.sol b/src/access/IAccessControl.sol new file mode 100644 index 0000000..6cda0a2 --- /dev/null +++ b/src/access/IAccessControl.sol @@ -0,0 +1,4 @@ +interface IAccessControl { + error CallerDoesNotHaveRequiredRole(); + function test() external; +} \ No newline at end of file diff --git a/src/access/Ownable.huff b/src/access/Ownable.huff index e6ffb74..3a57751 100644 --- a/src/access/Ownable.huff +++ b/src/access/Ownable.huff @@ -3,7 +3,7 @@ /// @notice Basic Ownable contract /* Imports */ -#include "../utils/require.huff" +#include "../utils/Require.huff" /* Custom errors */ #define error CallerIsNotTheOwner() diff --git a/src/token/ERC20/ERC20.huff b/src/token/ERC20/ERC20.huff index 610b1ef..aaba73b 100644 --- a/src/token/ERC20/ERC20.huff +++ b/src/token/ERC20/ERC20.huff @@ -326,8 +326,8 @@ /// @dev ERC20 transferFrom from and account to an other /// -/// Expected calldata: `address, uint256` with the recipient first -/// and the amount in second +/// Expected calldata: `address, address, uint256` with the sender first, +/// the recipient in second and the amount in third #define macro TRANSFER_FROM() = takes(0) returns(1) { // Setup the stack for the transfer function. 0x24 calldataload // [to] diff --git a/test/access/AccessControl.t.sol b/test/access/AccessControl.t.sol new file mode 100644 index 0000000..96bd40a --- /dev/null +++ b/test/access/AccessControl.t.sol @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity ^0.8.16; + +import "foundry-huff/HuffDeployer.sol"; +import "forge-std/Test.sol"; +import "forge-std/console.sol"; +import "test/Constants.sol"; + +import {IAccessControl} from "../../src/access/IAccessControl.sol"; +import {ITestAccessControl} from "../interfaces/ITestAccessControl.sol"; + +contract AccessControl is Test, ITestAccessControl { + /// @dev Address of the SimpleStore contract. + IAccessControl public accessControl; + + /// @dev Setup the testing environment. + function setUp() public { + accessControl = IAccessControl(HuffDeployer.deploy( + "access/AccessControl" + )); + } + + function testAccess() public { + accessControl.test(); + } +} \ No newline at end of file diff --git a/test/interfaces/ITestAccessControl.sol b/test/interfaces/ITestAccessControl.sol new file mode 100644 index 0000000..1f93fdb --- /dev/null +++ b/test/interfaces/ITestAccessControl.sol @@ -0,0 +1,3 @@ +interface ITestAccessControl { + error CallerDoseNotHaveRequiredRole(); +} \ No newline at end of file From b3b4bb5abdabd9e2ebb6181d5cd64109f6cf8851 Mon Sep 17 00:00:00 2001 From: Leo Date: Sun, 27 Nov 2022 11:39:11 +0100 Subject: [PATCH 2/4] doc: improve comments --- src/access/AccessControl.huff | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/access/AccessControl.huff b/src/access/AccessControl.huff index a3c6e9f..def5b8d 100644 --- a/src/access/AccessControl.huff +++ b/src/access/AccessControl.huff @@ -12,7 +12,7 @@ /// ``` /// /// Roles can be used to represent a set of permissions. To restrict access to a -/// function call, use hasRole() in your MAIN jumps: +/// function call, use HAS_ROLE() in your MAIN jumps: /// /// ``` /// jumpDest1: @@ -24,9 +24,9 @@ /// MACRO_THREE() /// ``` /// -/// Roles can be granted and revoked dynamically via the GRANT_ROLE_ROLE and +/// Roles can be granted and revoked dynamically via the GRANT_ROLE and /// REVOKE_ROLE macros. Each role has an associated admin role, and only -/// accounts that have a role's admin role can call GRANT_ROLE_ROLE and REVOKE_ROLE. +/// accounts that have a role's admin role can call GRANT_ROLE and REVOKE_ROLE. /// /// By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means /// that only accounts with this role will be able to grant or revoke other @@ -50,12 +50,15 @@ #define error CallerDoesNotHaveRequiredRole() /* CONSTRUCTOR */ +/// @dev GRANT the DEFAULT_ADMIN_ROLE to the caller. +/// ⚠️ This macro should be caller only in the constructor of a child +/// contract. #define macro ACCESS_CONSTRUCTOR() = takes (0) returns (0) { 0x01 [DEFAULT_ADMIN_ROLE] caller STORE_ELEMENT_FROM_KEYS(0x00) } /// @dev Macro that checks that the caller has a specific role. -/// Reverts with a standardized message including the required role. +/// Reverts with a standardized Error. /// @param role The role that the caller must have #define macro ONLY_ROLE(role) = takes(1) returns (0) { caller @@ -129,15 +132,16 @@ REQUIRE() } -/// @dev Macro that push onto the stack 0 or 1 depending on if the account -/// has a specific role. +/// @dev Macro that check if an account as the specified role or not. +/// Push 0 or 1 onto the stack. #define macro HAS_ROLE(role) = takes(1) returns (1) { // Input stack: [account] swap1 // [account, role] LOAD_ELEMENT_FROM_KEYS(0x00) // [account_has_role] } -/// @dev Macro that push onto the the admin role of a role. +/// @dev Macro that pushes the administrator role of the provided role +/// onto the stack. #define macro GET_ROLE_ADMIN(role) = takes(0) returns (1) { // [role] LOAD_ELEMENT(0x00) // [role_admin] From 063a729407ae4471d0348d87fb64ad55ee5381b0 Mon Sep 17 00:00:00 2001 From: Leo Date: Sun, 27 Nov 2022 11:40:11 +0100 Subject: [PATCH 3/4] feat: replace the DEFAULT_ADMIN_ROLE --- src/access/AccessControl.huff | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/access/AccessControl.huff b/src/access/AccessControl.huff index def5b8d..6ca400a 100644 --- a/src/access/AccessControl.huff +++ b/src/access/AccessControl.huff @@ -44,7 +44,8 @@ #define function test() nonpayable returns () /// @dev Default admin role -#define constant DEFAULT_ADMIN_ROLE = 0x01 +/// keccak256("DEFAULT_ADMIN_ROLE") +#define constant DEFAULT_ADMIN_ROLE = 0x1effbbff9c66c5e59634f24fe842750c60d18891155c32dd155fc2d661a4c86d /* Custom errors */ #define error CallerDoesNotHaveRequiredRole() From e1818bbf89f955a92c52c7ca341f8886dcd5938f Mon Sep 17 00:00:00 2001 From: Leo Date: Sun, 27 Nov 2022 11:41:15 +0100 Subject: [PATCH 4/4] doc: fix comments --- src/access/AccessControl.huff | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/access/AccessControl.huff b/src/access/AccessControl.huff index 6ca400a..b85baad 100644 --- a/src/access/AccessControl.huff +++ b/src/access/AccessControl.huff @@ -72,7 +72,7 @@ /// @param role The role to grant to the target account #define macro GRANT_ROLE(role) = takes(1) returns (0) { // Input stack: [account] - _GET_ROLE_ADMIN() // [role_admin, account] + _GET_ROLE_ADMIN() // [role_admin, account] caller // [caller, role_admin, account] CHECK_ROLE() // [account] @@ -89,7 +89,7 @@ /// @param role The role to revoke to the target account #define macro REVOKE_ROLE(role) = takes(1) returns (0) { // Input stack: [account] - _GET_ROLE_ADMIN() // [role_admin, account] + _GET_ROLE_ADMIN() // [role_admin, account] caller // [caller, role_admin, account] CHECK_ROLE() // [account]