Skip to content

Commit

Permalink
Document AccessManager functions and events in IAccessManager (#4660)
Browse files Browse the repository at this point in the history
Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Ernesto García <ernestognw@gmail.com>
(cherry picked from commit e78628b)
  • Loading branch information
Amxx authored and frangio committed Oct 5, 2023
1 parent cff0434 commit d27635f
Show file tree
Hide file tree
Showing 6 changed files with 451 additions and 281 deletions.
97 changes: 97 additions & 0 deletions certora/diff/access_manager_AccessManager.sol.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
--- access/manager/AccessManager.sol 2023-10-05 12:17:09.694051809 -0300
+++ access/manager/AccessManager.sol 2023-10-05 12:26:18.498688718 -0300
@@ -6,7 +6,6 @@
import {IAccessManaged} from "./IAccessManaged.sol";
import {Address} from "../../utils/Address.sol";
import {Context} from "../../utils/Context.sol";
-import {Multicall} from "../../utils/Multicall.sol";
import {Math} from "../../utils/math/Math.sol";
import {Time} from "../../utils/types/Time.sol";

@@ -57,7 +56,8 @@
* mindful of the danger associated with functions such as {{Ownable-renounceOwnership}} or
* {{AccessControl-renounceRole}}.
*/
-contract AccessManager is Context, Multicall, IAccessManager {
+// NOTE: The FV version of this contract doesn't include Multicall because CVL HAVOCs on any `delegatecall`.
+contract AccessManager is Context, IAccessManager {
using Time for *;

// Structure that stores the details for a target contract.
@@ -105,7 +105,7 @@

// Used to identify operations that are currently being executed via {execute}.
// This should be transient storage when supported by the EVM.
- bytes32 private _executionId;
+ bytes32 internal _executionId; // private → internal for FV

/**
* @dev Check that the caller is authorized to perform the operation, following the restrictions encoded in
@@ -253,6 +253,11 @@
_setGrantDelay(roleId, newDelay);
}

+ // Exposed for FV
+ function _getTargetAdminDelayFull(address target) internal view virtual returns (uint32, uint32, uint48) {
+ return _targets[target].adminDelay.getFull();
+ }
+
/**
* @dev Internal version of {grantRole} without access control. Returns true if the role was newly granted.
*
@@ -287,6 +292,11 @@
return newMember;
}

+ // Exposed for FV
+ function _getRoleGrantDelayFull(uint64 roleId) internal view virtual returns (uint32, uint32, uint48) {
+ return _roles[roleId].grantDelay.getFull();
+ }
+
/**
* @dev Internal version of {revokeRole} without access control. This logic is also used by {renounceRole}.
* Returns true if the role was previously granted.
@@ -586,7 +596,7 @@
/**
* @dev Check if the current call is authorized according to admin logic.
*/
- function _checkAuthorized() private {
+ function _checkAuthorized() internal virtual { // private → internal virtual for FV
address caller = _msgSender();
(bool immediate, uint32 delay) = _canCallSelf(caller, _msgData());
if (!immediate) {
@@ -609,7 +619,7 @@
*/
function _getAdminRestrictions(
bytes calldata data
- ) private view returns (bool restricted, uint64 roleAdminId, uint32 executionDelay) {
+ ) internal view returns (bool restricted, uint64 roleAdminId, uint32 executionDelay) { // private → internal for FV
if (data.length < 4) {
return (false, 0, 0);
}
@@ -662,7 +672,7 @@
address caller,
address target,
bytes calldata data
- ) private view returns (bool immediate, uint32 delay) {
+ ) internal view returns (bool immediate, uint32 delay) { // private → internal for FV
if (target == address(this)) {
return _canCallSelf(caller, data);
} else {
@@ -716,14 +726,14 @@
/**
* @dev Extracts the selector from calldata. Panics if data is not at least 4 bytes
*/
- function _checkSelector(bytes calldata data) private pure returns (bytes4) {
+ function _checkSelector(bytes calldata data) internal pure returns (bytes4) { // private → internal for FV
return bytes4(data[0:4]);
}

/**
* @dev Hashing function for execute protection
*/
- function _hashExecutionId(address target, bytes4 selector) private pure returns (bytes32) {
+ function _hashExecutionId(address target, bytes4 selector) internal pure returns (bytes32) { // private → internal for FV
return keccak256(abi.encode(target, selector));
}
}
6 changes: 5 additions & 1 deletion contracts/access/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ This directory provides ways to restrict who can access the functions of a contr

{{IAuthority}}

{{IAccessManager}}

{{AccessManager}}

{{IAccessManaged}}

{{AccessManaged}}

{{AccessManagerAdapter}}
{{AuthorityUtils}}
14 changes: 3 additions & 11 deletions contracts/access/manager/AccessManaged.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,12 @@ abstract contract AccessManaged is Context, IAccessManaged {
_;
}

/**
* @dev Returns the current authority.
*/
/// @inheritdoc IAccessManaged
function authority() public view virtual returns (address) {
return _authority;
}

/**
* @dev Transfers control to a new authority. The caller must be the current authority.
*/
/// @inheritdoc IAccessManaged
function setAuthority(address newAuthority) public virtual {
address caller = _msgSender();
if (caller != authority()) {
Expand All @@ -79,11 +75,7 @@ abstract contract AccessManaged is Context, IAccessManaged {
_setAuthority(newAuthority);
}

/**
* @dev Returns true only in the context of a delayed restricted call, at the moment that the scheduled operation is
* being consumed. Prevents denial of service for delayed restricted calls in the case that the contract performs
* attacker controlled calls.
*/
/// @inheritdoc IAccessManaged
function isConsumingScheduledOp() public view returns (bytes4) {
return _consumingSchedule ? this.isConsumingScheduledOp.selector : bytes4(0);
}
Expand Down
Loading

0 comments on commit d27635f

Please sign in to comment.