wip

Author: benluelo

cosmwasm/cw-manager/src/contract.rs

@@ -36,11 +36,12 @@ fn only_authorized(ctx: &mut ExecCtx) -> Result<(), ContractError> {
     _check_authorized(ctx)
 }
 
-// =============================================== ROLE MANAGEMENT ===============================================
+// ======================================= ROLE MANAGEMENT =======================================
 
 /// See [`ExecuteMsg::LabelRole`].
 pub(crate) fn label_role(ctx: &mut ExecCtx, role_id: RoleId, label: &str) {
-    // TODO: figure out how we want to label roles; events like the original solidity implementation or in storage?
+    // TODO: figure out how we want to label roles; events like the original solidity implementation
+    // or in storage?
     ctx.emit(RoleLabel { role_id, label })
 }
 
@@ -125,7 +126,8 @@ pub(crate) fn set_grant_delay(
     _set_grant_delay(ctx, role_id, new_delay)
 }
 
-/// Internal version of [`grant_role`] without access control. Returns true if the role was newly granted.
+/// Internal version of [`grant_role`] without access control. Returns true if the role was newly
+/// granted.
 ///
 /// Emits a [`RoleGranted`] event.
 ///
@@ -188,7 +190,8 @@ pub(crate) fn _grant_role(
     Ok(new_member)
 }
 
-// Internal version of [`revoke_role`] without access control. This logic is also used by {renounceRole}. Returns true if the role was previously granted.
+// Internal version of [`revoke_role`] without access control. This logic is also used by
+// {renounceRole}. Returns true if the role was previously granted.
 ///
 /// Emits a [`RoleRevoked`] event if the account had the role.
 ///
@@ -218,7 +221,7 @@ fn _revoke_role(ctx: &mut ExecCtx, role_id: RoleId, account: &Addr) -> Result<bo
 ///
 /// Emits a [`RoleAdminChanged`] event.
 ///
-/// NOTE: Setting the admin role as the `PUBLIC_ROLE` is allowed, but it will effectively allow
+/// NOTE: Setting the admin role as the [`RoleId::PUBLIC_ROLE`] is allowed, but it will effectively allow
 /// anyone to set grant or revoke such role.
 ///
 /// ```solidity
@@ -248,7 +251,7 @@ fn _set_role_admin(ctx: &mut ExecCtx, role_id: RoleId, admin: RoleId) -> Result<
 ///
 /// Emits a [`RoleGuardianChanged`] event.
 ///
-/// NOTE: Setting the guardian role as the `PUBLIC_ROLE` is allowed, but it will effectively allow
+/// NOTE: Setting the guardian role as the [`RoleId::PUBLIC_ROLE`] is allowed, but it will effectively allow
 /// anyone to cancel any scheduled operation for such role.
 ///
 /// ```solidity
@@ -278,9 +281,9 @@ fn _set_role_guardian(
     Ok(())
 }
 
-/// Internal version of {setGrantDelay} without access control.
+/// Internal version of [`set_grant_delay`] without access control.
 ///  
-/// Emits a {RoleGrantDelayChanged} event.
+/// Emits a [`RoleGrantDelayChanged`] event.
 ///
 /// ```solidity
 /// function _setGrantDelay(uint64 roleId, uint32 newDelay) internal virtual
@@ -324,7 +327,7 @@ fn _set_grant_delay(
     Ok(())
 }
 
-// ============================================= FUNCTION MANAGEMENT ==============================================
+// ===================================== FUNCTION MANAGEMENT ======================================
 
 /// See [`ExecuteMsg::SetTargetFunctionRole`].
 pub(crate) fn set_target_function_role<'a>(
@@ -416,7 +419,7 @@ fn _set_target_admin_delay(
     Ok(())
 }
 
-// =============================================== MODE MANAGEMENT ================================================
+// ======================================= MODE MANAGEMENT ========================================
 
 /// See [`ExecuteMsg::SetTargetClosed`].
 pub(crate) fn set_target_closed(
@@ -454,7 +457,7 @@ fn _set_target_closed(ctx: &mut ExecCtx, target: &Addr, closed: bool) -> Result<
     Ok(())
 }
 
-// ============================================== DELAYED OPERATIONS ==============================================
+// ====================================== DELAYED OPERATIONS ======================================
 
 /// See [`QueryMsg::GetSchedule`].
 pub(crate) fn get_schedule(ctx: QueryCtx, id: H256) -> Result<u64, ContractError> {
@@ -612,7 +615,8 @@ pub(crate) fn cancel(
     if schedule.timepoint == 0 {
         return Err(ContractError::AccessManagerNotScheduled(operation_id));
     } else if caller != msgsender {
-        // calls can only be canceled by the account that scheduled them, a global admin, or by a guardian of the required role.
+        // calls can only be canceled by the account that scheduled them, a global admin, or by a
+        // guardian of the required role.
         let is_admin = has_role(ctx.query_ctx(), RoleId::ADMIN_ROLE, msgsender)?.is_member;
         let is_guardian = has_role(
             ctx.query_ctx(),
@@ -716,7 +720,7 @@ pub(crate) fn hash_operation(caller: &Addr, target: &Addr, data: &str) -> H256 {
     Sha256::digest(format!("{caller}/{target}/{data}",)).into()
 }
 
-// ==================================================== OTHERS ====================================================
+// ============================================ OTHERS ============================================
 
 /// @inheritdoc IAccessManager
 pub(crate) fn update_authority(
@@ -735,11 +739,12 @@ pub(crate) fn update_authority(
     )?))
 }
 
-// ================================================= ADMIN LOGIC ==================================================
+// ========================================= ADMIN LOGIC ==========================================
 
 /// Check if the current call is authorized according to admin and roles logic.
 ///
-/// WARNING: Carefully review the considerations of {AccessManaged-restricted} since they apply to this modifier.
+/// WARNING: Carefully review the considerations of {AccessManaged-restricted} since they apply to
+/// this modifier.
 ///
 /// ```solidity
 /// function _checkAuthorized() private
@@ -770,12 +775,14 @@ fn _check_authorized(ctx: &mut ExecCtx) -> Result<(), ContractError> {
     Ok(())
 }
 
-/// Get the admin restrictions of a given function call based on the function and arguments involved.
+/// Get the admin restrictions of a given function call based on the function and arguments
+/// involved.
 ///
 /// Returns:
 ///  - bool restricted: does this data match a restricted operation
 ///  - uint64: which role is this operation restricted to
-///  - uint32: minimum delay to enforce for that operation (max between operation's delay and admin's execution delay)
+///  - uint32: minimum delay to enforce for that operation (max between operation's delay and
+///    admin's execution delay)
 ///
 /// ```solidity
 /// function _getAdminRestrictions(
@@ -810,7 +817,8 @@ fn _get_admin_restrictions(
             Ok((true, RoleId::ADMIN_ROLE, delay))
         }
 
-        // Restricted to that role's admin with no delay beside any execution delay the caller may have.
+        // Restricted to that role's admin with no delay beside any execution delay the caller may
+        // have.
         Ok(GrantRole { role_id, .. } | RevokeRole { role_id, .. }) => {
             Ok((true, get_role_admin(ctx.query_ctx(), role_id)?, 0))
         }
@@ -823,7 +831,8 @@ fn _get_admin_restrictions(
     }
 }
 
-/// Extracts the selector from calldata. Returns an error if there is no selector in the provided JSON data.
+/// Extracts the selector from calldata. Returns an error if there is no selector in the provided
+/// JSON data.
 ///
 /// ```solidity
 /// function _checkSelector(bytes calldata data) private pure returns (bytes4)
@@ -836,7 +845,7 @@ fn _check_selector(data: &str) -> Result<Selector, ContractError> {
         .selector())
 }
 
-// =================================================== HELPERS ====================================================
+// =========================================== HELPERS ============================================
 
 /// An extended version of [`can_call`] for internal usage that checks [`_can_call_self`]
 /// when the target is this contract.
@@ -876,8 +885,9 @@ fn _can_call_extended(
 /// <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v5.4.0/contracts/access/manager/AccessManager.sol#L685>
 fn _can_call_self(ctx: &mut ExecCtx, caller: &Addr, data: &str) -> Result<CanCall, ContractError> {
     if caller == ctx.address_this() {
-        // Caller is AccessManager, this means the call was sent through {execute} and it already checked
-        // permissions. We verify that the call "identifier", which is set during {execute}, is correct.
+        // Caller is AccessManager, this means the call was sent through {execute} and it already
+        // checked permissions. We verify that the call "identifier", which is set during
+        // `execute`, is correct.
         return Ok(CanCall {
             allowed: _is_executing(ctx.query_ctx(), ctx.address_this(), &_check_selector(data)?)?,
             delay: 0,
@@ -958,8 +968,9 @@ pub(crate) fn can_call(
             delay: 0,
         })
     } else if caller == ctx.address_this() {
-        // Caller is AccessManager, this means the call was sent through {execute} and it already checked
-        // permissions. We verify that the call "identifier", which is set during {execute}, is correct.
+        // Caller is AccessManager, this means the call was sent through `execute` and it already
+        // checked permissions. We verify that the call "identifier", which is set during
+        // `execute`, is correct.
         Ok(CanCall {
             allowed: _is_executing(ctx, target, selector)?,
             delay: 0,
@@ -1139,6 +1150,7 @@ pub struct FullAccess {
     current_delay: u32,
     /// Pending execution delay for the account.
     pending_delay: u32,
-    /// Timestamp at which the pending execution delay will become active. 0 means no delay update is scheduled.
+    /// Timestamp at which the pending execution delay will become active. 0 means no delay update
+    /// is scheduled.
     effect: u64,
 }

cosmwasm/cw-manager/src/event.rs

@@ -101,7 +101,9 @@ impl Into<Event> for RoleLabel<'_> {
 /// Emitted when `account` is granted `roleId`.
 ///
 /// NOTE: The meaning of the `since` argument depends on the `newMember` argument.
-/// If the role is granted to a new member, the `since` argument indicates when the account becomes a member of the role, otherwise it indicates the execution delay for this account and roleId is updated.
+/// If the role is granted to a new member, the `since` argument indicates when the account becomes
+/// a member of the role, otherwise it indicates the execution delay for this account and roleId is
+/// updated.
 ///
 /// ```solidity
 /// event RoleGranted(uint64 indexed roleId, address indexed account, uint32 delay, uint48 since, bool newMember);
@@ -127,7 +129,8 @@ impl Into<Event> for RoleGranted<'_> {
     }
 }
 
-/// Emitted when `account` membership or `roleId` is revoked. Unlike granting, revoking is instantaneous.
+/// Emitted when `account` membership or `roleId` is revoked. Unlike granting, revoking is
+/// instantaneous.
 ///
 /// ```solidity
 /// event RoleRevoked(uint64 indexed roleId, address indexed account);

cosmwasm/cw-manager/src/lib.rs

@@ -2,35 +2,41 @@
 //!
 //! AccessManager is a central contract to store the permissions of a system.
 //!
-//! A smart contract under the control of an AccessManager instance is known as a target, and will inherit from the
-//! {AccessManaged} contract, be connected to this contract as its manager and implement the {AccessManaged-restricted}
-//! modifier on a set of functions selected to be permissioned. Note that any function without this setup won't be
-//! effectively restricted.
+//! A smart contract under the control of an AccessManager instance is known as a target, and will
+//! inherit from the {AccessManaged} contract, be connected to this contract as its manager and
+//! implement the {AccessManaged-restricted} modifier on a set of functions selected to be
+//! permissioned. Note that any function without this setup won't be effectively restricted.
 //!
-//! The restriction rules for such functions are defined in terms of "roles" identified by a [`RoleId`] and scoped
-//! by target ([`Addr`][cosmwasm_std::Addr]) and function selectors ([`Selector`][crate::types::Selector]). These roles are stored in this contract and can be
-//! configured by admins ([`RoleId::ADMIN_ROLE`] members) after a delay (see [`QueryMsg::GetTargetAdminDelay`]).
+//! The restriction rules for such functions are defined in terms of "roles" identified by a
+//! [`RoleId`] and scoped by target ([`Addr`][cosmwasm_std::Addr]) and function selectors
+//! ([`Selector`][crate::types::Selector]). These roles are stored in this contract and can be
+//! configured by admins ([`RoleId::ADMIN_ROLE`] members) after a delay (see
+//! [`QueryMsg::GetTargetAdminDelay`]).
 //!
 //! For each target contract, admins can configure the following without any delay:
 //!
 //! * The target's {AccessManaged-authority} via [`ExecuteMsg::UpdateAuthority`].
 //! * Close or open a target via [`ExecuteMsg::SetTargetClosed`] keeping the permissions intact.
-//! * The roles that are allowed (or disallowed) to call a given function (identified by its selector) through [`ExecuteMsg::SetTargetAdminDelay`].
+//! * The roles that are allowed (or disallowed) to call a given function (identified by its
+//!   selector) through [`ExecuteMsg::SetTargetAdminDelay`].
 //!
-//! By default every address is member of the [`RoleId::PUBLIC_ROLE`] and every target function is restricted to the [`RoleId::ADMIN_ROLE`] until configured otherwise.
-//! Additionally, each role has the following configuration options restricted to this manager's admins:
+//! By default every address is member of the [`RoleId::PUBLIC_ROLE`] and every target function is
+//! restricted to the [`RoleId::ADMIN_ROLE`] until configured otherwise. Additionally, each role has
+//! the following configuration options restricted to this manager's admins:
 //!
 //! * A role's admin role via [`ExecuteMsg::SetRoleAdmin`] who can grant or revoke roles.
 //! * A role's guardian role via [`ExecuteMsg::SetRoleGuardian`] who's allowed to cancel operations.
-//! * A delay in which a role takes effect after being granted through [`ExecuteMsg::SetGrantDelay`].
+//! * A delay in which a role takes effect after being granted through
+//!   [`ExecuteMsg::SetGrantDelay`].
 //! * A delay of any target's admin action via [`ExecuteMsg::SetTargetAdminDelay`].
 //! * A role label for discoverability purposes with [`ExecuteMsg::LabelRole`].
 //!
-//! Any account can be added and removed into any number of these roles by using the [`ExecuteMsg::GrantRole`] and [`ExecuteMsg::RevokeRole`] functions
-//! restricted to each role's admin (see [`QueryMsg::GetRoleAdmin`]).
+//! Any account can be added and removed into any number of these roles by using the
+//! [`ExecuteMsg::GrantRole`] and [`ExecuteMsg::RevokeRole`] functions restricted to each role's
+//! admin (see [`QueryMsg::GetRoleAdmin`]).
 //!
-//! Since all the permissions of the managed system can be modified by the admins of this instance, it is expected that
-//! they will be highly secured (e.g., a multisig or a well-configured DAO).
+//! Since all the permissions of the managed system can be modified by the admins of this instance,
+//! it is expected that they will be highly secured (e.g., a multisig or a well-configured DAO).
 //!
 //! [am]: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v5.4.0/contracts/access/manager/AccessManager.sol