Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor ABSTAIN to UNSET #96

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading