From d826ecae61be9f762d61386f8a48b46f51cd7230 Mon Sep 17 00:00:00 2001 From: Raul Date: Sun, 14 Apr 2024 19:14:40 -0300 Subject: [PATCH] refactor ABSTAIN to UNSET --- contracts/access/AccessController.sol | 18 +++++++++--------- .../interfaces/access/IAccessController.sol | 6 +++--- contracts/lib/AccessPermission.sol | 6 +++--- test/foundry/access/AccessController.t.sol | 16 ++++++++-------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/contracts/access/AccessController.sol b/contracts/access/AccessController.sol index 2aaf66d6..f91b1aec 100644 --- a/contracts/access/AccessController.sol +++ b/contracts/access/AccessController.sol @@ -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. @@ -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 @@ -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(); } @@ -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`) @@ -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) { diff --git a/contracts/interfaces/access/IAccessController.sol b/contracts/interfaces/access/IAccessController.sol index d0a64216..6792d4a9 100644 --- a/contracts/interfaces/access/IAccessController.sol +++ b/contracts/interfaces/access/IAccessController.sol @@ -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 @@ -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`) diff --git a/contracts/lib/AccessPermission.sol b/contracts/lib/AccessPermission.sol index 45f74ca8..139c64df 100644 --- a/contracts/lib/AccessPermission.sol +++ b/contracts/lib/AccessPermission.sol @@ -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; @@ -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; diff --git a/test/foundry/access/AccessController.t.sol b/test/foundry/access/AccessController.t.sol index 7e2b76b1..7c4766e8 100644 --- a/test/foundry/access/AccessController.t.sol +++ b/test/foundry/access/AccessController.t.sol @@ -224,7 +224,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); accessController.checkPermission( @@ -261,7 +261,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); vm.expectRevert( abi.encodeWithSelector( @@ -306,7 +306,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); accessController.checkPermission( address(ipAccount), @@ -342,7 +342,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); vm.expectRevert( abi.encodeWithSelector( @@ -626,7 +626,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); accessController.checkPermission( @@ -682,7 +682,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); vm.expectRevert( abi.encodeWithSelector( @@ -1211,7 +1211,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); } @@ -1292,7 +1292,7 @@ contract AccessControllerTest is BaseTest { address(mockModule), mockModule.executeSuccessfully.selector ), - AccessPermission.ABSTAIN + AccessPermission.UNSET ); vm.prank(address(0xbeefbeef));