From 1c3fc1941349c3b2973525c51d83b79652920334 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Mon, 13 May 2024 14:35:37 +0200
Subject: [PATCH 01/10] feat(access): add events and errors for AccessControl

---
 contracts/Cargo.toml                   |   1 +
 contracts/src/access/access_control.rs | 102 +++++++++++++++++++++++++
 contracts/src/access/mod.rs            |   3 +
 3 files changed, 106 insertions(+)
 create mode 100644 contracts/src/access/access_control.rs

diff --git a/contracts/Cargo.toml b/contracts/Cargo.toml
index 3f6bbde02..58cd8d376 100644
--- a/contracts/Cargo.toml
+++ b/contracts/Cargo.toml
@@ -37,6 +37,7 @@ erc721_metadata = ["erc721"]
 
 access = []
 ownable = ["access"]
+access_control = ["access"]
 
 # Enables using the standard library. This is not included in the default
 # features, because this crate is meant to be used in a `no_std` environment.
diff --git a/contracts/src/access/access_control.rs b/contracts/src/access/access_control.rs
new file mode 100644
index 000000000..3961614f5
--- /dev/null
+++ b/contracts/src/access/access_control.rs
@@ -0,0 +1,102 @@
+//! Contract module that allows children to implement role-based access control
+//! mechanisms. This is a lightweight version that doesn't allow enumerating
+//! role members except through off-chain means by accessing the contract event
+//! logs. Some applications may benefit from on-chain enumerability, for those
+//! cases see [`AccessControlEnumberable`][enumerable ext].
+//!
+//! 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
+//! `public constant` hash digests:
+//!
+//! TODO: Update example to Rust.
+//!
+//! ```solidity
+//! bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
+//! ```
+//!
+//! Roles can be used to represent a set of permissions. To restrict access to a
+//! function call, use `has_role`:
+//!
+//! TODO: Update example to Rust.
+//!
+//! ```solidity
+//! function foo() public {
+//!     require(hasRole(MY_ROLE, msg.sender));
+//!     ...
+//! }
+//! ```
+//!
+//! Roles can be granted and revoked dynamically via the `grant_role` and
+//! `revoke_role` functions. Each role has an associated admin role, and only
+//! 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 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. We recommend using
+//! `AccessControlDefaultAdminRules` to enforce additional security measures for
+//! this role.
+//!
+//! [enumerable ext]: TBD
+use alloy_primitives::Address;
+use alloy_sol_types::sol;
+use stylus_proc::SolidityError;
+use stylus_sdk::{
+    evm, msg,
+    stylus_proc::{external, sol_storage},
+};
+
+sol! {
+    /// Emitted when `new_admin_role` is set as `role`'s admin role, replacing
+    /// `previous_admin_role`.
+    ///
+    /// `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite
+    /// `RoleAdminChanged` not being emitted signaling this.
+    #[allow(missing_docs)]
+    event RoleAdminChanged(bytes32 indexed role, bytes32 indexed previous_admin_role, bytes32 indexed new_admin_role);
+    /// Emitted when `account` is granted `role`.
+    ///
+    /// `sender` is the account that originated the contract call. This account
+    /// bears the admin role (for the granted role).
+    /// Expected in cases where the role was granted using the internal
+    /// [`AccessControl::grantRole`].
+    #[allow(missing_docs)]
+    event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
+    /// Emitted when `account` is revoked `role`.
+    ///
+    /// `sender` is the account that originated the contract call:
+    ///   - if using `revoke_role`, it is the admin role bearer.
+    ///   - if using `renounce_role`, it is the role bearer (i.e. `account`).
+    #[allow(missing_docs)]
+    event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
+}
+
+sol! {
+    /// The `account` is missing a role.
+    ///
+    /// * `account` - Account that was found to not be authorized.
+    #[derive(Debug)]
+    #[allow(missing_docs)]
+    error AccessControlUnauthorizedAccount(address account, bytes32 neededRole);
+    /// The caller of a function is not the expected one.
+    ///
+    /// NOTE: Don't confuse with [`AccessControlUnauthorizedAccount`].
+    #[derive(Debug)]
+    #[allow(missing_docs)]
+    error AccessControlBadConfirmation();
+}
+
+/// An error that occurred in the implementation of an [`AccessControl`]
+/// contract.
+#[derive(SolidityError, Debug)]
+pub enum Error {
+    /// The caller account is missing a role.
+    UnauthorizedAccount(AccessControlUnauthorizedAccount),
+    /// The caller of a afunction is not the expected one.
+    BadConfirmation(AccessControlBadConfirmation),
+}
diff --git a/contracts/src/access/mod.rs b/contracts/src/access/mod.rs
index ba842c57f..35420cdf4 100644
--- a/contracts/src/access/mod.rs
+++ b/contracts/src/access/mod.rs
@@ -2,3 +2,6 @@
 
 #[cfg(feature = "ownable")]
 pub mod ownable;
+
+#[cfg(feature = "access_control")]
+pub mod access_control;

From 970568c5b9ed448ca7af584fdc36cd437eb18832 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Mon, 13 May 2024 20:23:13 +0200
Subject: [PATCH 02/10] feat(access): add base AccessControl implementation

---
 contracts/src/access/access_control.rs | 301 +++++++++++++++++++++++--
 contracts/src/access/ownable.rs        |  10 +-
 2 files changed, 290 insertions(+), 21 deletions(-)

diff --git a/contracts/src/access/access_control.rs b/contracts/src/access/access_control.rs
index 3961614f5..b587e517b 100644
--- a/contracts/src/access/access_control.rs
+++ b/contracts/src/access/access_control.rs
@@ -6,23 +6,23 @@
 //!
 //! 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
-//! `public constant` hash digests:
+//! `pub const` hash digests:
 //!
-//! TODO: Update example to Rust.
-//!
-//! ```solidity
-//! bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
+//! ```no_run
+//! // Output of `keccak256("MY_ROLE")`.
+//! pub const MY_ROLE: [u8; 32] = [151, 200, 119, 228, 14, 219, 65, 113,
+//!                                15, 11, 175, 88, 140, 135, 142, 225,
+//!                                90, 4, 73, 155, 6, 174, 140, 152, 207,
+//!                                72, 136, 117, 217, 26, 114, 19];
 //! ```
 //!
 //! Roles can be used to represent a set of permissions. To restrict access to a
-//! function call, use `has_role`:
-//!
-//! TODO: Update example to Rust.
+//! function call, use [`AccessControl::has_role`]:
 //!
-//! ```solidity
-//! function foo() public {
-//!     require(hasRole(MY_ROLE, msg.sender));
-//!     ...
+//! ```ignore
+//! pub fn foo() {
+//!   assert!(self.has_role(MY_ROLE.into(), msg::sender());
+//!   ...
 //! }
 //! ```
 //!
@@ -43,7 +43,7 @@
 //! this role.
 //!
 //! [enumerable ext]: TBD
-use alloy_primitives::Address;
+use alloy_primitives::{Address, B256};
 use alloy_sol_types::sol;
 use stylus_proc::SolidityError;
 use stylus_sdk::{
@@ -80,9 +80,10 @@ sol! {
     /// The `account` is missing a role.
     ///
     /// * `account` - Account that was found to not be authorized.
+    /// * `needed_role` - The missing role.
     #[derive(Debug)]
     #[allow(missing_docs)]
-    error AccessControlUnauthorizedAccount(address account, bytes32 neededRole);
+    error AccessControlUnauthorizedAccount(address account, bytes32 needed_role);
     /// The caller of a function is not the expected one.
     ///
     /// NOTE: Don't confuse with [`AccessControlUnauthorizedAccount`].
@@ -100,3 +101,275 @@ pub enum Error {
     /// The caller of a afunction is not the expected one.
     BadConfirmation(AccessControlBadConfirmation),
 }
+
+sol_storage! {
+    /// Information about a specific role.
+    pub struct RoleData {
+        /// Whether an account is member of a certain role.
+        mapping(address => bool) has_role;
+        /// The admin role for this role.
+        bytes32 admin_role;
+    }
+
+    /// State of an `AccessControl` contract.
+    pub struct AccessControl {
+        /// Role identifier -> Role information.
+        mapping(bytes32 => RoleData) _roles;
+    }
+}
+
+#[external]
+impl AccessControl {
+    /// The default admin role. `[0; 32]` by default.
+    pub const DEFAULT_ADMIN_ROLE: [u8; 32] = [0; 32];
+
+    /// Returns `true` if `account` has been granted `role`.
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `account` - The account to check for membership.
+    pub fn has_role(&self, role: B256, account: Address) -> bool {
+        self._roles.getter(role).has_role.get(account)
+    }
+
+    /// Checks if [`msg::sender`] has been granted `role`.
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
+    /// * `role` - The role identifier.
+    ///
+    /// # Errors
+    ///
+    /// * If [`msg::sender`] has not been granted `role`, then
+    /// [`Error::UnauthorizedAccount`] is returned.
+    pub fn only_role(&self, role: B256) -> Result<(), Error> {
+        self._check_role(role, msg::sender())
+    }
+
+    /// Returns the admin role that controls `role`. See [`Self::grant_role`]
+    /// and [`Self::revoke_role`].
+    ///
+    /// To change a role's adming, use [`Self::set_role_admin`].
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
+    /// * `role` - The role identifier.
+    pub fn get_role_admin(&self, role: B256) -> B256 {
+        *self._roles.getter(role).admin_role
+    }
+
+    /// Grants `role` to `account`.
+    ///
+    /// If `account` had not been already granted `role`, emits a
+    /// [`RoleGranted`] event.
+    ///
+    /// # Requirements:
+    ///
+    /// * The caller must have `role`'s admin role.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `account` - The account which will be granted the role.
+    ///
+    /// # Errors
+    ///
+    /// * If [`msg::sender`] has not been granted `role`, then
+    /// [`Error::UnauthorizedAccount`] is returned.
+    ///
+    /// # Events
+    ///
+    /// May emit a [`RoleGranted`] event.
+    pub fn grant_role(
+        &mut self,
+        role: B256,
+        account: Address,
+    ) -> Result<(), Error> {
+        let admin_role = self.get_role_admin(role);
+        self.only_role(admin_role)?;
+        self._grant_role(role, account);
+        Ok(())
+    }
+
+    /// Revokes `role` from `account`.
+    ///
+    /// If `account` had been granted `role`, emits a [`RoleRevoked`] event.
+    ///
+    /// # Requirements:
+    ///
+    /// * The caller must have `role`'s admin role.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `account` - The account which will be revoked the role.
+    ///
+    /// # Errors
+    ///
+    /// * If [`msg::sender`] has not been granted `role`, then
+    /// [`Error::UnauthorizedAccount`] is returned.
+    ///
+    /// # Events
+    ///
+    /// May emit a [`RoleRevoked`] event.
+    pub fn revoke_role(
+        &mut self,
+        role: B256,
+        account: Address,
+    ) -> Result<(), Error> {
+        let admin_role = self.get_role_admin(role);
+        self.only_role(admin_role)?;
+        self._grant_role(role, account);
+        Ok(())
+    }
+
+    /// Revokes `role` from the calling account.
+    ///
+    /// Roles are often managed via [`Self::grant_role`] and
+    /// [`Self::revoke_role`]: this function's purpose is to provide a mechanism
+    /// for accounts to lose their privileges if they are compromised (such as
+    /// when a trusted device is misplaced).
+    ///
+    /// # Requirements:
+    ///
+    /// * The caller must be `callerConfirmation`.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `confirmation` - The account which will be revoked the role.
+    ///
+    /// # Events
+    ///
+    /// If the calling account has its `role` revoked, emits a [`RoleRevoked`]
+    /// event.
+    pub fn renounce_role(
+        &mut self,
+        role: B256,
+        confirmation: Address,
+    ) -> Result<(), Error> {
+        if msg::sender() != confirmation {
+            return Err(Error::BadConfirmation(
+                AccessControlBadConfirmation {},
+            ));
+        }
+
+        self._revoke_role(role, confirmation);
+        Ok(())
+    }
+}
+
+impl AccessControl {
+    /// Sets `admin_role` as `role`'s admin role.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `role` - The identifier of the role we are changing the admin to.
+    /// * `new_admin_role` - The new admin role.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`RoleAdminChanged`] event.
+    pub fn _set_role_admin(&mut self, role: B256, new_admin_role: B256) {
+        let previous_admin_role = self.get_role_admin(role);
+        self._roles.setter(role).admin_role.set(new_admin_role);
+        evm::log(RoleAdminChanged {
+            role: *role,
+            previous_admin_role: *previous_admin_role,
+            new_admin_role: *new_admin_role,
+        });
+    }
+
+    /// Checks if `account` has been granted `role`.
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `account` - The account to check for membership.
+    ///
+    /// # Errors
+    ///
+    /// * If [`msg::sender`] has not been granted `role`, then
+    /// [`Error::UnauthorizedAccount`] is returned.
+    pub fn _check_role(
+        &self,
+        role: B256,
+        account: Address,
+    ) -> Result<(), Error> {
+        if !self.has_role(role, account) {
+            return Err(Error::UnauthorizedAccount(
+                AccessControlUnauthorizedAccount {
+                    account,
+                    needed_role: *role,
+                },
+            ));
+        }
+
+        Ok(())
+    }
+
+    /// Attempts to grant `role` to `account` and returns a boolean indicating
+    /// if `role` was granted.
+    ///
+    /// Internal function without access restriction.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `account` - The account which will be granted the role.
+    ///
+    /// # Events
+    ///
+    /// May emit a [`RoleGranted`] event.
+    pub fn _grant_role(&mut self, role: B256, account: Address) -> bool {
+        if !self.has_role(role, account) {
+            self._roles.setter(role).has_role.insert(account, true);
+            evm::log(RoleGranted {
+                role: *role,
+                account,
+                sender: msg::sender(),
+            });
+            true
+        } else {
+            false
+        }
+    }
+
+    /// Attempts to revoke `role` from `account` and returns a boolean
+    /// indicating if `role` was revoked.
+    ///
+    /// Internal function without access restriction.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `role` - The role identifier.
+    /// * `account` - The account which will be granted the role.
+    ///
+    /// # Events
+    ///
+    /// May emit a [`RoleRevoked`] event.
+    pub fn _revoke_role(&mut self, role: B256, account: Address) -> bool {
+        if !self.has_role(role, account) {
+            self._roles.setter(role).has_role.insert(account, false);
+            evm::log(RoleRevoked {
+                role: *role,
+                account,
+                sender: msg::sender(),
+            });
+            true
+        } else {
+            false
+        }
+    }
+}
diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index 651387272..d908f8538 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -19,7 +19,7 @@ use stylus_sdk::{
 sol! {
     /// Emitted when ownership gets transferred between accounts.
     #[allow(missing_docs)]
-    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
+    event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
 }
 
 sol! {
@@ -171,13 +171,9 @@ impl Ownable {
     /// * `&mut self` - Write access to the contract's state.
     /// * `new_owner` - Account that's gonna be the next owner.
     pub fn _transfer_ownership(&mut self, new_owner: Address) {
-        let old_owner = self._owner.get();
+        let previous_owner = self._owner.get();
         self._owner.set(new_owner);
-
-        evm::log(OwnershipTransferred {
-            previousOwner: old_owner,
-            newOwner: new_owner,
-        });
+        evm::log(OwnershipTransferred { previous_owner, new_owner });
     }
 }
 

From d0d2c4ea5472233191db40dd48a452029987d36c Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Mon, 13 May 2024 20:49:38 +0200
Subject: [PATCH 03/10] fix: typo

---
 contracts/src/access/access_control.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/src/access/access_control.rs b/contracts/src/access/access_control.rs
index b587e517b..af172bb28 100644
--- a/contracts/src/access/access_control.rs
+++ b/contracts/src/access/access_control.rs
@@ -152,7 +152,7 @@ impl AccessControl {
     /// Returns the admin role that controls `role`. See [`Self::grant_role`]
     /// and [`Self::revoke_role`].
     ///
-    /// To change a role's adming, use [`Self::set_role_admin`].
+    /// To change a role's admin, use [`Self::set_role_admin`].
     ///
     /// # Arguments
     ///

From 84bc40112f4553e8e22a6fd4f22435b45d91a367 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Tue, 14 May 2024 15:31:24 +0200
Subject: [PATCH 04/10] feat(access): add AccessControl tests

---
 contracts/src/access/access_control.rs | 251 ++++++++++++++++++++++++-
 1 file changed, 249 insertions(+), 2 deletions(-)

diff --git a/contracts/src/access/access_control.rs b/contracts/src/access/access_control.rs
index af172bb28..b604e7cd4 100644
--- a/contracts/src/access/access_control.rs
+++ b/contracts/src/access/access_control.rs
@@ -225,7 +225,7 @@ impl AccessControl {
     ) -> Result<(), Error> {
         let admin_role = self.get_role_admin(role);
         self.only_role(admin_role)?;
-        self._grant_role(role, account);
+        self._revoke_role(role, account);
         Ok(())
     }
 
@@ -360,7 +360,7 @@ impl AccessControl {
     ///
     /// May emit a [`RoleRevoked`] event.
     pub fn _revoke_role(&mut self, role: B256, account: Address) -> bool {
-        if !self.has_role(role, account) {
+        if self.has_role(role, account) {
             self._roles.setter(role).has_role.insert(account, false);
             evm::log(RoleRevoked {
                 role: *role,
@@ -373,3 +373,250 @@ impl AccessControl {
         }
     }
 }
+
+#[cfg(all(test, feature = "std"))]
+mod tests {
+    use alloy_primitives::{address, Address, U256};
+    use stylus_sdk::{
+        msg,
+        storage::{StorageMap, StorageType},
+    };
+
+    use super::{AccessControl, Error};
+
+    impl Default for AccessControl {
+        fn default() -> Self {
+            AccessControl { _roles: unsafe { StorageMap::new(U256::ZERO, 0) } }
+        }
+    }
+
+    /// Shorthand for declaring variables converted from a hex literla to a
+    /// fixed 32-byte slice;
+    macro_rules! roles {
+        ($($var:ident = $hex:literal);* $(;)?) => {
+            $(
+                const $var: [u8; 32] = alloy_primitives::hex!($hex);
+            )*
+        };
+    }
+
+    roles! {
+        ROLE       = "ed9ea7bc2a13bc59432ab07436e7f7f5450f82d4b48c401bed177bfaf36b1873";
+        OTHER_ROLE = "879ce0d4bfd332649ca3552efe772a38d64a315eb70ab69689fd309c735946b5";
+    }
+
+    #[rustfmt::skip]
+    const BOB  : Address = address!("B0B0cB49ec2e96DF5F5fFB081acaE66A2cBBc2e2");
+    const ALICE: Address = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");
+
+    // Since we don't have constructors, we need to call this  to setup
+    // `msg::sender` as a member of `ROLE`.
+    //
+    // NOTE: Once we have support for setting `msg::sender` and constructor,
+    // this function shouldn't be needed.
+    fn _grant_role_to_msg_sender(contract: &mut AccessControl, role: [u8; 32]) {
+        contract
+            ._roles
+            .setter(role.into())
+            .has_role
+            .insert(msg::sender(), true);
+    }
+
+    #[grip::test]
+    fn default_role_is_default_admin(contract: AccessControl) {
+        let role_admin = contract.get_role_admin(ROLE.into());
+        assert_eq!(role_admin, AccessControl::DEFAULT_ADMIN_ROLE);
+    }
+
+    #[grip::test]
+    fn default_admin_roles_admin_is_itself(contract: AccessControl) {
+        const DEFAULT_ADMIN_ROLE: [u8; 32] = AccessControl::DEFAULT_ADMIN_ROLE;
+        let role_admin = contract.get_role_admin(DEFAULT_ADMIN_ROLE.into());
+        assert_eq!(role_admin, DEFAULT_ADMIN_ROLE);
+    }
+
+    #[grip::test]
+    fn non_admin_cannot_grant_role_to_others(contract: AccessControl) {
+        let err = contract.grant_role(ROLE.into(), ALICE).unwrap_err();
+        assert!(matches!(err, Error::UnauthorizedAccount(_)));
+    }
+
+    #[grip::test]
+    fn accounts_can_be_granted_roles_multiple_times(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, AccessControl::DEFAULT_ADMIN_ROLE);
+
+        contract.grant_role(ROLE.into(), ALICE).unwrap();
+        contract.grant_role(ROLE.into(), ALICE).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, true);
+    }
+
+    #[grip::test]
+    fn not_granted_roles_can_be_revoked(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, AccessControl::DEFAULT_ADMIN_ROLE);
+
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, false);
+        contract.revoke_role(ROLE.into(), ALICE).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, false);
+    }
+
+    #[grip::test]
+    fn admin_can_revoke_role(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, AccessControl::DEFAULT_ADMIN_ROLE);
+        contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true);
+
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, true);
+        contract.revoke_role(ROLE.into(), ALICE).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, false);
+    }
+
+    #[grip::test]
+    fn non_admin_cannot_revoke_role(contract: AccessControl) {
+        contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true);
+
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, true);
+        let err = contract.revoke_role(ROLE.into(), ALICE).unwrap_err();
+        assert!(matches!(err, Error::UnauthorizedAccount(_)));
+    }
+
+    #[grip::test]
+    fn roles_can_be_revoked_multiple_times(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, AccessControl::DEFAULT_ADMIN_ROLE);
+
+        contract.revoke_role(ROLE.into(), ALICE).unwrap();
+        contract.revoke_role(ROLE.into(), ALICE).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, false);
+    }
+
+    #[grip::test]
+    fn not_granted_roles_can_be_renounced(contract: AccessControl) {
+        // FIXME: We can't test this case because we can't check for events. We
+        // need to assert that a RoleRevoked event doesn't get emitted for this
+        // test to make sense.
+        contract.renounce_role(ROLE.into(), msg::sender()).unwrap();
+    }
+
+    #[grip::test]
+    fn bearer_can_renounce_role(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, ROLE);
+
+        let has_role = contract.has_role(ROLE.into(), msg::sender());
+        assert_eq!(has_role, true);
+        contract.renounce_role(ROLE.into(), msg::sender()).unwrap();
+        let has_role = contract.has_role(ROLE.into(), msg::sender());
+        assert_eq!(has_role, false);
+    }
+
+    #[grip::test]
+    fn only_sender_can_renounce(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, ROLE);
+        let err = contract.renounce_role(ROLE.into(), ALICE).unwrap_err();
+        assert!(matches!(err, Error::BadConfirmation(_)));
+    }
+
+    #[grip::test]
+    fn roles_can_be_renounced_multiple_times(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, ROLE);
+
+        let sender = msg::sender();
+        contract.renounce_role(ROLE.into(), sender).unwrap();
+        contract.renounce_role(ROLE.into(), sender).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, false);
+    }
+
+    #[grip::test]
+    fn a_roles_admin_role_can_change(contract: AccessControl) {
+        contract._set_role_admin(ROLE.into(), OTHER_ROLE.into());
+        _grant_role_to_msg_sender(contract, OTHER_ROLE);
+
+        let admin_role = contract.get_role_admin(ROLE.into());
+        assert_eq!(admin_role, OTHER_ROLE);
+    }
+
+    #[grip::test]
+    fn the_new_admin_can_grant_roles(contract: AccessControl) {
+        contract._set_role_admin(ROLE.into(), OTHER_ROLE.into());
+        _grant_role_to_msg_sender(contract, OTHER_ROLE);
+
+        contract.grant_role(ROLE.into(), ALICE).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, true);
+    }
+
+    #[grip::test]
+    fn the_new_admin_can_revoke_roles(contract: AccessControl) {
+        contract._set_role_admin(ROLE.into(), OTHER_ROLE.into());
+        _grant_role_to_msg_sender(contract, OTHER_ROLE);
+
+        contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true);
+        contract.revoke_role(ROLE.into(), ALICE).unwrap();
+        let has_role = contract.has_role(ROLE.into(), ALICE);
+        assert_eq!(has_role, false);
+    }
+
+    #[grip::test]
+    fn previous_admins_no_longer_grant_roles(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, ROLE);
+        contract._set_role_admin(ROLE.into(), OTHER_ROLE.into());
+
+        let err = contract.grant_role(ROLE.into(), ALICE).unwrap_err();
+        assert!(matches!(err, Error::UnauthorizedAccount(_)));
+    }
+
+    #[grip::test]
+    fn previous_admins_no_longer_revoke_roles(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, ROLE);
+        contract._set_role_admin(ROLE.into(), OTHER_ROLE.into());
+
+        let err = contract.revoke_role(ROLE.into(), ALICE).unwrap_err();
+        assert!(matches!(err, Error::UnauthorizedAccount(_)));
+    }
+
+    #[grip::test]
+    fn does_not_revert_if_sender_has_role(contract: AccessControl) {
+        _grant_role_to_msg_sender(contract, ROLE);
+        contract._check_role(ROLE.into(), msg::sender()).unwrap();
+    }
+
+    #[grip::test]
+    fn reverts_if_sender_doesnt_have_role(contract: AccessControl) {
+        let err = contract._check_role(ROLE.into(), msg::sender()).unwrap_err();
+        assert!(matches!(err, Error::UnauthorizedAccount(_)));
+        let err =
+            contract._check_role(OTHER_ROLE.into(), msg::sender()).unwrap_err();
+        assert!(matches!(err, Error::UnauthorizedAccount(_)));
+    }
+
+    #[grip::test]
+    fn internal_grant_role_true_if_no_role(contract: AccessControl) {
+        let role_granted = contract._grant_role(ROLE.into(), ALICE);
+        assert_eq!(role_granted, true);
+    }
+
+    #[grip::test]
+    fn internal_grant_role_false_if_role(contract: AccessControl) {
+        contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true);
+        let role_granted = contract._grant_role(ROLE.into(), ALICE);
+        assert_eq!(role_granted, false);
+    }
+
+    #[grip::test]
+    fn internal_revoke_role_true_if_role(contract: AccessControl) {
+        contract._roles.setter(ROLE.into()).has_role.insert(ALICE, true);
+        let role_revoked = contract._revoke_role(ROLE.into(), ALICE);
+        assert_eq!(role_revoked, true);
+    }
+
+    #[grip::test]
+    fn internal_revoke_role_false_if_no_role(contract: AccessControl) {
+        let role_revoked = contract._revoke_role(ROLE.into(), ALICE);
+        assert_eq!(role_revoked, false);
+    }
+}

From 046d425aa12db9d605abbdeadd8499fb7ee8967d Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Tue, 14 May 2024 15:31:55 +0200
Subject: [PATCH 05/10] ref(access): remove unused BOB address

---
 contracts/src/access/access_control.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/contracts/src/access/access_control.rs b/contracts/src/access/access_control.rs
index b604e7cd4..b50d90e81 100644
--- a/contracts/src/access/access_control.rs
+++ b/contracts/src/access/access_control.rs
@@ -405,8 +405,6 @@ mod tests {
         OTHER_ROLE = "879ce0d4bfd332649ca3552efe772a38d64a315eb70ab69689fd309c735946b5";
     }
 
-    #[rustfmt::skip]
-    const BOB  : Address = address!("B0B0cB49ec2e96DF5F5fFB081acaE66A2cBBc2e2");
     const ALICE: Address = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");
 
     // Since we don't have constructors, we need to call this  to setup

From b785362d2e2f04ef23771ed047167335d22059d2 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Tue, 14 May 2024 15:41:42 +0200
Subject: [PATCH 06/10] feat(access): add AccessControl example

---
 Cargo.lock                         | 11 ++++++
 Cargo.toml                         |  1 +
 examples/access-control/Cargo.toml | 17 +++++++++
 examples/access-control/src/lib.rs | 57 ++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+)
 create mode 100644 examples/access-control/Cargo.toml
 create mode 100644 examples/access-control/src/lib.rs

diff --git a/Cargo.lock b/Cargo.lock
index 7b01c5846..6b1c00fde 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2,6 +2,17 @@
 # It is not intended for manual editing.
 version = 3
 
+[[package]]
+name = "access-control-example"
+version = "0.0.0"
+dependencies = [
+ "alloy-primitives",
+ "contracts",
+ "mini-alloc",
+ "stylus-proc",
+ "stylus-sdk",
+]
+
 [[package]]
 name = "aho-corasick"
 version = "1.1.3"
diff --git a/Cargo.toml b/Cargo.toml
index f78a0b7d0..dfcf8c994 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,6 +8,7 @@ members = [
   "examples/erc721",
   "examples/merkle-proofs",
   "examples/ownable",
+  "examples/access-control",
 ]
 # Explicitly set the resolver to version 2, which is the default for packages
 # with edition >= 2021.
diff --git a/examples/access-control/Cargo.toml b/examples/access-control/Cargo.toml
new file mode 100644
index 000000000..7c7eb8df3
--- /dev/null
+++ b/examples/access-control/Cargo.toml
@@ -0,0 +1,17 @@
+[package]
+name = "access-control-example"
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+publish = false
+version = "0.0.0"
+
+[dependencies]
+contracts = { path = "../../contracts", features = ["ownable", "erc20"] }
+alloy-primitives.workspace = true
+stylus-sdk.workspace = true
+stylus-proc.workspace = true
+mini-alloc.workspace = true
+
+[lib]
+crate-type = ["lib", "cdylib"]
diff --git a/examples/access-control/src/lib.rs b/examples/access-control/src/lib.rs
new file mode 100644
index 000000000..10d089d5d
--- /dev/null
+++ b/examples/access-control/src/lib.rs
@@ -0,0 +1,57 @@
+#![cfg_attr(not(test), no_std, no_main)]
+extern crate alloc;
+
+use alloc::vec::Vec;
+
+use alloy_primitives::{Address, U256};
+use contracts::{access::access_control::AccessControl, erc20::ERC20};
+use stylus_sdk::{
+    msg,
+    prelude::{entrypoint, external, sol_storage},
+};
+
+sol_storage! {
+    #[entrypoint]
+    struct Token {
+        #[borrow]
+        ERC20 erc20;
+        #[borrow]
+        AccessControl access;
+    }
+}
+
+#[external]
+#[inherit(ERC20, AccessControl)]
+impl Token {
+    // `keccak256("TRANSFER_ROLE")`
+    const TRANSFER_ROLE: [u8; 32] = [
+        133, 2, 35, 48, 150, 217, 9, 190, 251, 218, 9, 153, 187, 142, 162, 243,
+        166, 190, 60, 19, 139, 159, 191, 0, 55, 82, 164, 200, 188, 232, 111,
+        108,
+    ];
+
+    pub fn constructor(&mut self) -> Result<(), Vec<u8>> {
+        // Make caller the default admin.
+        self.access.grant_role(
+            AccessControl::DEFAULT_ADMIN_ROLE.into(),
+            msg::sender(),
+        )?;
+        Ok(())
+    }
+
+    pub fn make_admin(&mut self, account: Address) -> Result<(), Vec<u8>> {
+        self.access.only_role(AccessControl::DEFAULT_ADMIN_ROLE.into())?;
+        self.access.grant_role(Token::TRANSFER_ROLE.into(), account)?;
+        Ok(())
+    }
+
+    pub fn transfer(
+        &mut self,
+        to: Address,
+        value: U256,
+    ) -> Result<(), Vec<u8>> {
+        self.access.only_role(Token::TRANSFER_ROLE.into())?;
+        self.erc20.transfer(to, value)?;
+        Ok(())
+    }
+}

From 7c59b6b42e47e0db16d84b748bb836e17b429f28 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Tue, 14 May 2024 15:53:30 +0200
Subject: [PATCH 07/10] fix: set proper feature flag in access-control-example

---
 examples/access-control/Cargo.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/access-control/Cargo.toml b/examples/access-control/Cargo.toml
index 7c7eb8df3..43fd0ee68 100644
--- a/examples/access-control/Cargo.toml
+++ b/examples/access-control/Cargo.toml
@@ -7,7 +7,7 @@ publish = false
 version = "0.0.0"
 
 [dependencies]
-contracts = { path = "../../contracts", features = ["ownable", "erc20"] }
+contracts = { path = "../../contracts", features = ["access_control", "erc20"] }
 alloy-primitives.workspace = true
 stylus-sdk.workspace = true
 stylus-proc.workspace = true

From 4b58ad9e0fdc6e4af9a089583e4ad1cc9c06decb Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Tue, 14 May 2024 16:01:17 +0200
Subject: [PATCH 08/10] lint: appease clippy

---
 contracts/Cargo.toml                                |  2 +-
 .../src/access/{access_control.rs => control.rs}    | 13 ++++++++++---
 contracts/src/access/mod.rs                         |  4 ++--
 examples/access-control/Cargo.toml                  |  2 +-
 examples/access-control/src/lib.rs                  |  2 +-
 5 files changed, 15 insertions(+), 8 deletions(-)
 rename contracts/src/access/{access_control.rs => control.rs} (98%)

diff --git a/contracts/Cargo.toml b/contracts/Cargo.toml
index 58cd8d376..7b2945c85 100644
--- a/contracts/Cargo.toml
+++ b/contracts/Cargo.toml
@@ -37,7 +37,7 @@ erc721_metadata = ["erc721"]
 
 access = []
 ownable = ["access"]
-access_control = ["access"]
+control = ["access"]
 
 # Enables using the standard library. This is not included in the default
 # features, because this crate is meant to be used in a `no_std` environment.
diff --git a/contracts/src/access/access_control.rs b/contracts/src/access/control.rs
similarity index 98%
rename from contracts/src/access/access_control.rs
rename to contracts/src/access/control.rs
index b50d90e81..810b11b1e 100644
--- a/contracts/src/access/access_control.rs
+++ b/contracts/src/access/control.rs
@@ -130,6 +130,7 @@ impl AccessControl {
     /// * `&self` - Read access to the contract's state.
     /// * `role` - The role identifier.
     /// * `account` - The account to check for membership.
+    #[must_use]
     pub fn has_role(&self, role: B256, account: Address) -> bool {
         self._roles.getter(role).has_role.get(account)
     }
@@ -158,6 +159,7 @@ impl AccessControl {
     ///
     /// * `&self` - Read access to the contract's state.
     /// * `role` - The role identifier.
+    #[must_use]
     pub fn get_role_admin(&self, role: B256) -> B256 {
         *self._roles.getter(role).admin_role
     }
@@ -246,6 +248,11 @@ impl AccessControl {
     /// * `role` - The role identifier.
     /// * `confirmation` - The account which will be revoked the role.
     ///
+    /// # Errors
+    ///
+    /// * If [`msg::sender`] is not the `confirmation` address, then
+    /// [`Error::BadConfirmation`] is returned.
+    ///
     /// # Events
     ///
     /// If the calling account has its `role` revoked, emits a [`RoleRevoked`]
@@ -332,7 +339,9 @@ impl AccessControl {
     ///
     /// May emit a [`RoleGranted`] event.
     pub fn _grant_role(&mut self, role: B256, account: Address) -> bool {
-        if !self.has_role(role, account) {
+        if self.has_role(role, account) {
+            false
+        } else {
             self._roles.setter(role).has_role.insert(account, true);
             evm::log(RoleGranted {
                 role: *role,
@@ -340,8 +349,6 @@ impl AccessControl {
                 sender: msg::sender(),
             });
             true
-        } else {
-            false
         }
     }
 
diff --git a/contracts/src/access/mod.rs b/contracts/src/access/mod.rs
index 35420cdf4..744dcab70 100644
--- a/contracts/src/access/mod.rs
+++ b/contracts/src/access/mod.rs
@@ -3,5 +3,5 @@
 #[cfg(feature = "ownable")]
 pub mod ownable;
 
-#[cfg(feature = "access_control")]
-pub mod access_control;
+#[cfg(feature = "control")]
+pub mod control;
diff --git a/examples/access-control/Cargo.toml b/examples/access-control/Cargo.toml
index 43fd0ee68..2eeb8f300 100644
--- a/examples/access-control/Cargo.toml
+++ b/examples/access-control/Cargo.toml
@@ -7,7 +7,7 @@ publish = false
 version = "0.0.0"
 
 [dependencies]
-contracts = { path = "../../contracts", features = ["access_control", "erc20"] }
+contracts = { path = "../../contracts", features = ["control", "erc20"] }
 alloy-primitives.workspace = true
 stylus-sdk.workspace = true
 stylus-proc.workspace = true
diff --git a/examples/access-control/src/lib.rs b/examples/access-control/src/lib.rs
index 10d089d5d..ac17327ba 100644
--- a/examples/access-control/src/lib.rs
+++ b/examples/access-control/src/lib.rs
@@ -4,7 +4,7 @@ extern crate alloc;
 use alloc::vec::Vec;
 
 use alloy_primitives::{Address, U256};
-use contracts::{access::access_control::AccessControl, erc20::ERC20};
+use contracts::{access::control::AccessControl, erc20::ERC20};
 use stylus_sdk::{
     msg,
     prelude::{entrypoint, external, sol_storage},

From af04c1875fcefe9454947c4d5a50d83d5b27e416 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Thu, 16 May 2024 11:06:33 +0200
Subject: [PATCH 09/10] ref(access): improve access-control example

---
 contracts/src/access/control.rs    | 6 +++---
 examples/access-control/src/lib.rs | 9 +++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/contracts/src/access/control.rs b/contracts/src/access/control.rs
index 810b11b1e..9234ad716 100644
--- a/contracts/src/access/control.rs
+++ b/contracts/src/access/control.rs
@@ -28,7 +28,7 @@
 //!
 //! Roles can be granted and revoked dynamically via the `grant_role` and
 //! `revoke_role` functions. Each role has an associated admin role, and only
-//! accounts that have a role's admin role can call `grant_role` and
+//! 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
@@ -64,7 +64,7 @@ sol! {
     /// `sender` is the account that originated the contract call. This account
     /// bears the admin role (for the granted role).
     /// Expected in cases where the role was granted using the internal
-    /// [`AccessControl::grantRole`].
+    /// [`AccessControl::grant_role`].
     #[allow(missing_docs)]
     event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
     /// Emitted when `account` is revoked `role`.
@@ -240,7 +240,7 @@ impl AccessControl {
     ///
     /// # Requirements:
     ///
-    /// * The caller must be `callerConfirmation`.
+    /// * The caller must be `caller_confirmation`.
     ///
     /// # Arguments
     ///
diff --git a/examples/access-control/src/lib.rs b/examples/access-control/src/lib.rs
index ac17327ba..7802096a6 100644
--- a/examples/access-control/src/lib.rs
+++ b/examples/access-control/src/lib.rs
@@ -45,13 +45,14 @@ impl Token {
         Ok(())
     }
 
-    pub fn transfer(
+    pub fn transfer_from(
         &mut self,
+        from: Address,
         to: Address,
         value: U256,
-    ) -> Result<(), Vec<u8>> {
+    ) -> Result<bool, Vec<u8>> {
         self.access.only_role(Token::TRANSFER_ROLE.into())?;
-        self.erc20.transfer(to, value)?;
-        Ok(())
+        let transfer_result = self.erc20.transfer_from(from, to, value)?;
+        Ok(transfer_result)
     }
 }

From 5a760c76c7ffd4e34e13d8bc6ef65d62a1548b88 Mon Sep 17 00:00:00 2001
From: Alexander Gonzalez <alexfertel97@gmail.com>
Date: Thu, 16 May 2024 11:07:22 +0200
Subject: [PATCH 10/10] fix(access): caller_confirmation -> confirmation

---
 contracts/src/access/control.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/src/access/control.rs b/contracts/src/access/control.rs
index 9234ad716..efabc0f08 100644
--- a/contracts/src/access/control.rs
+++ b/contracts/src/access/control.rs
@@ -240,7 +240,7 @@ impl AccessControl {
     ///
     /// # Requirements:
     ///
-    /// * The caller must be `caller_confirmation`.
+    /// * The caller must be `confirmation`.
     ///
     /// # Arguments
     ///