Skip to content

Commit

Permalink
refactor ABSTAIN to UNSET
Browse files Browse the repository at this point in the history
  • Loading branch information
Ramarti committed Apr 14, 2024
1 parent c26eab5 commit d826eca
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 23 deletions.
18 changes: 9 additions & 9 deletions contracts/access/AccessController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { Errors } from "../lib/Errors.sol";
///
/// Each policy is represented as a mapping from an IP account address to a signer address to a recipient
/// address to a function selector to a permission level.
/// The permission level can be 0 (ABSTAIN), 1 (ALLOW), or 2 (DENY).
/// The permission level can be 0 (UNSET), 1 (ALLOW), or 2 (DENY).
///
/// The contract includes the following functions:
/// - initialize: Sets the addresses of the IP account registry and the module registry.
Expand Down Expand Up @@ -95,9 +95,9 @@ contract AccessController is IAccessController, ProtocolPausableUpgradeable, UUP

/// @notice Sets the permission for a specific function call
/// @dev Each policy is represented as a mapping from an IP account address to a signer address to a recipient
/// address to a function selector to a permission level. The permission level can be 0 (ABSTAIN), 1 (ALLOW), or
/// address to a function selector to a permission level. The permission level can be 0 (UNSET), 1 (ALLOW), or
/// 2 (DENY).
/// @dev By default, all policies are set to 0 (ABSTAIN), which means that the permission is not set.
/// @dev By default, all policies are set to 0 (UNSET), which means that the permission is not set.
/// The owner of ipAccount by default has all permission.
/// address(0) => wildcard
/// bytes4(0) => wildcard
Expand Down Expand Up @@ -125,7 +125,7 @@ contract AccessController is IAccessController, ProtocolPausableUpgradeable, UUP
if (!IIPAccountRegistry($.ipAccountRegistry).isIpAccount(ipAccount)) {
revert Errors.AccessController__IPAccountIsNotValid(ipAccount);
}
// permission must be one of ABSTAIN, ALLOW, DENY
// permission must be one of UNSET, ALLOW, DENY
if (permission > 2) {
revert Errors.AccessController__PermissionIsNotValid();
}
Expand All @@ -141,7 +141,7 @@ contract AccessController is IAccessController, ProtocolPausableUpgradeable, UUP
/// Otherwise, the function is a noop.
/// @dev This function checks the permission level for a specific function call.
/// If a specific permission is set, it overrides the general (wildcard) permission.
/// If the current level permission is ABSTAIN, the final permission is determined by the upper level.
/// If the current level permission is UNSET, the final permission is determined by the upper level.
/// @param ipAccount The address of the IP account that grants the permission for `signer`
/// @param signer The address that can call `to` on behalf of the `ipAccount`
/// @param to The address that can be called by the `signer` (currently only modules can be `to`)
Expand Down Expand Up @@ -175,15 +175,15 @@ contract AccessController is IAccessController, ProtocolPausableUpgradeable, UUP
return;
}

// If specific function permission is ABSTAIN, check module level permission
if (functionPermission == AccessPermission.ABSTAIN) {
// If specific function permission is UNSET, check module level permission
if (functionPermission == AccessPermission.UNSET) {
uint8 modulePermission = getPermission(ipAccount, signer, to, bytes4(0));
// Return true if allow to call all functions of the module
if (modulePermission == AccessPermission.ALLOW) {
return;
}
// If module level permission is ABSTAIN, check transaction signer level permission
if (modulePermission == AccessPermission.ABSTAIN) {
// If module level permission is UNSET, check transaction signer level permission
if (modulePermission == AccessPermission.UNSET) {
// Pass if the ipAccount allow the signer can call all functions of all modules
// Otherwise, revert
if (getPermission(ipAccount, signer, address(0), bytes4(0)) == AccessPermission.ALLOW) {
Expand Down
6 changes: 3 additions & 3 deletions contracts/interfaces/access/IAccessController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ interface IAccessController {

/// @notice Sets the permission for a specific function call
/// @dev Each policy is represented as a mapping from an IP account address to a signer address to a recipient
/// address to a function selector to a permission level. The permission level can be 0 (ABSTAIN), 1 (ALLOW), or
/// address to a function selector to a permission level. The permission level can be 0 (UNSET), 1 (ALLOW), or
/// 2 (DENY).
/// @dev By default, all policies are set to 0 (ABSTAIN), which means that the permission is not set.
/// @dev By default, all policies are set to 0 (UNSET), which means that the permission is not set.
/// The owner of ipAccount by default has all permission.
/// address(0) => wildcard
/// bytes4(0) => wildcard
Expand All @@ -44,7 +44,7 @@ interface IAccessController {
/// Otherwise, the function is a noop.
/// @dev This function checks the permission level for a specific function call.
/// If a specific permission is set, it overrides the general (wildcard) permission.
/// If the current level permission is ABSTAIN, the final permission is determined by the upper level.
/// If the current level permission is UNSET, the final permission is determined by the upper level.
/// @param ipAccount The address of the IP account that grants the permission for `signer`
/// @param signer The address that can call `to` on behalf of the `ipAccount`
/// @param to The address that can be called by the `signer` (currently only modules can be `to`)
Expand Down
6 changes: 3 additions & 3 deletions contracts/lib/AccessPermission.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ pragma solidity 0.8.23;
/// @notice Library for IPAccount access control permissions.
/// These permissions are used by the AccessController.
library AccessPermission {
/// @notice ABSTAIN means having not enough information to make decision at current level, deferred decision to up
/// @notice UNSET means having not enough information to make decision at current level, deferred decision to up
/// level permission.
uint8 public constant ABSTAIN = 0;
uint8 public constant UNSET = 0;

/// @notice ALLOW means the permission is granted to transaction signer to call the function.
uint8 public constant ALLOW = 1;
Expand All @@ -20,7 +20,7 @@ library AccessPermission {
/// @param signer The address that can call `to` on behalf of the `ipAccount`
/// @param to The address that can be called by the `signer` (currently only modules can be `to`)
/// @param func The function selector of `to` that can be called by the `signer` on behalf of the `ipAccount`
/// @param permission The permission level for the transaction (0 = ABSTAIN, 1 = ALLOW, 2 = DENY).
/// @param permission The permission level for the transaction (0 = UNSET, 1 = ALLOW, 2 = DENY).
struct Permission {
address ipAccount;
address signer;
Expand Down
16 changes: 8 additions & 8 deletions test/foundry/access/AccessController.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);

accessController.checkPermission(
Expand Down Expand Up @@ -261,7 +261,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);
vm.expectRevert(
abi.encodeWithSelector(
Expand Down Expand Up @@ -306,7 +306,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);
accessController.checkPermission(
address(ipAccount),
Expand Down Expand Up @@ -342,7 +342,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);
vm.expectRevert(
abi.encodeWithSelector(
Expand Down Expand Up @@ -626,7 +626,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);

accessController.checkPermission(
Expand Down Expand Up @@ -682,7 +682,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);
vm.expectRevert(
abi.encodeWithSelector(
Expand Down Expand Up @@ -1211,7 +1211,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);
}

Expand Down Expand Up @@ -1292,7 +1292,7 @@ contract AccessControllerTest is BaseTest {
address(mockModule),
mockModule.executeSuccessfully.selector
),
AccessPermission.ABSTAIN
AccessPermission.UNSET
);

vm.prank(address(0xbeefbeef));
Expand Down

0 comments on commit d826eca

Please sign in to comment.