Single AccessManagedProxy implementation: mutable and skippable

.github/workflows/tests.yaml

@@ -15,16 +15,10 @@ jobs:
           node-version: "22"
           cache: "npm"
       - run: npm ci
-      - run: node scripts/generateAMPSkips.js 1 5 10 24 40
       - run: npx hardhat compile
       - run: npx hardhat size-contracts
       - run: npm run solhint
       - run: npx hardhat test
         env:
           REPORT_GAS: "1"
-      - name: Clean to avoid sol-coverage error with AccessManagedProxyS40
-        run: |
-          npx hardhat clean
-          rm contracts/amps/*.sol
-          node scripts/generateAMPSkips.js 1 5 10 24
       - run: npx hardhat coverage

.gitignore

@@ -17,6 +17,3 @@ node_modules
 ignition/deployments/chain-31337
 
 soljson-latest.js
-
-# AMP generated files
-contracts/amps/*.sol

README.md

@@ -34,18 +34,25 @@ configuration for that method.
 
 More details on the motivation of this idea here: https://forum.openzeppelin.com/t/accessmanagedproxy-is-a-good-idea/41917
 
+Also check https://www.youtube.com/watch?v=DKdwJ9Ap9vM for a presentation on this approach.
+
 ## 📝 Details
 
-The package includes the AccessManagedProxy contract, and AccessManagedProxyS1 to AccessManagedProxyS24 that are
-versions that accept from 1 to 24 methods (selectors) that will be skipped of the access manager check, to reduce gas
-usage or to access immutability on some methods.
+The AccessManagedProxy contract stores the configuration in the storage (uses namespaced storage layout, see EIP-7201),
+but it doesn't include in the proxy functions to modify it. The implementation contracts should add the functions to
+change the access manager (`setAuthority(...)`, following IAccessManaged interface of OZ 5.x) or the passThruMethods.
+
+The AMPUtils library includes several functions to modify the custom storage and other operations like making custom
+access control checks.
+
+Also, an AccessManagedProxyBase abstract contract is provided in case you prefer to use immutable storage or other
+variants.
 
 ## Development
 
 Try running some of the following tasks:
 
 ```shell
-npx hardhat help
-npx hardhat test
 REPORT_GAS=true npx hardhat test
+npx hardhat coverage
 ```

contracts/AMPUtils.sol

@@ -1,7 +1,8 @@
-// SPDX-License-Identifier: MIT
+// SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.0;
 
-import {AccessManagedProxy} from "./AccessManagedProxy.sol";
+import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol";
+import {IAccessManagedProxy} from "./interfaces/IAccessManagedProxy.sol";
 
 /**
  * @title AMPUtils
@@ -13,14 +14,64 @@ library AMPUtils {
   // Error copied from IAccessManaged
   error AccessManagedUnauthorized(address caller);
 
+  struct AccessManagedProxyStorage {
+    IAccessManager accessManager;
+    bytes4[] passThruMethods; // This is used for observability
+    mapping(bytes4 => bool) skipAc; // This is the used for actual lookup
+  }
+
+  /**
+   * @notice Storage slot with the address of the current access mananger.
+   * @dev Computed as: `keccak256(
+   *    abi.encode(uint256(keccak256("ensuro.storage.AccessManagedProxy")) - 1)
+   * ) & ~bytes32(uint256(0xff))
+   */
+  // solhint-disable-next-line const-name-snakecase
+  bytes32 internal constant AccessManagedProxyStorageLocation =
+    0x787c9d7ac910d64252bcea05acd5b7af6d59644e0451a8bb5674587555049c00;
+
+  function getAccessManagedProxyStorage() internal pure returns (AccessManagedProxyStorage storage $) {
+    // solhint-disable-next-line no-inline-assembly
+    assembly {
+      $.slot := AccessManagedProxyStorageLocation
+    }
+  }
+
+  function setAccessManager(IAccessManager accessManager) internal {
+    if (address(accessManager).code.length == 0) {
+      revert IAccessManagedProxy.AccessManagedInvalidAuthority(address(accessManager));
+    }
+    getAccessManagedProxyStorage().accessManager = accessManager;
+    emit IAccessManagedProxy.AuthorityUpdated(address(accessManager));
+  }
+
+  function setPassThruMethods(bytes4[] memory passThruMethods) internal {
+    AccessManagedProxyStorage storage $ = AMPUtils.getAccessManagedProxyStorage();
+    $.passThruMethods = new bytes4[](passThruMethods.length);
+    for (uint256 i; i < passThruMethods.length; ++i) {
+      $.passThruMethods[i] = passThruMethods[i];
+      $.skipAc[passThruMethods[i]] = true;
+    }
+    emit IAccessManagedProxy.PassThruMethodsChanged(passThruMethods);
+  }
+
+  function replacePassThruMethods(bytes4[] memory newPassThruMethods) internal {
+    AccessManagedProxyStorage storage $ = AMPUtils.getAccessManagedProxyStorage();
+    bytes4[] memory oldPassThruMethods = $.passThruMethods;
+    for (uint256 i; i < oldPassThruMethods.length; ++i) {
+      $.skipAc[oldPassThruMethods[i]] = false;
+    }
+    setPassThruMethods(newPassThruMethods);
+  }
+
   /**
    * @dev Checks if the user can call a particular selector, assuming the calling contract was deployed as an AMP.
    *
    * @param user The user for which you want to check the access, typically msg.sender
    * @param selector The selector of the method called (or a fake selector generated with makeSelector or another way)
    */
   function checkCanCall(address user, bytes4 selector) internal view {
-    (bool immediate, ) = AccessManagedProxy(payable(address(this))).ACCESS_MANAGER().canCall(
+    (bool immediate, ) = IAccessManagedProxy(payable(address(this))).ACCESS_MANAGER().canCall(
       user,
       address(this),
       selector

contracts/AccessManagedProxy.sol

@@ -1,55 +1,73 @@
-// SPDX-License-Identifier: MIT
+// SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.0;
 
 import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol";
+import {IAccessManagedProxy} from "./interfaces/IAccessManagedProxy.sol";
 import {AccessManagedProxyBase} from "./AccessManagedProxyBase.sol";
+import {AMPUtils} from "./AMPUtils.sol";
 
 /**
  * @title AccessManagedProxy
- * @notice Proxy contract using IAccessManager to manage access control before delegating calls (immutable version)
+ * @notice Proxy contract using IAccessManager to manage access control before delegating calls (mutable version)
  * @dev It's a variant of ERC1967Proxy.
  *
- *      Currently the check is executed on any call received by the proxy contract even calls to view methods
- *      (staticcall). In the setup of the ACCESS_MANAGER permissions you would want to make all the views and pure
- *      functions enabled for the PUBLIC_ROLE.
+ *      Currently the check is executed on any call received by the proxy contract (even calls to view methods, i.e.
+ *      staticcall). For gas efficiency, you can also have `passThruMethods`, and for those methods it will skip
+ *      the call to the AccessManager, calling directly to the implementation contract.
  *
- *      For gas efficiency, the ACCESS_MANAGER is immutable, so take care you don't lose control of it, otherwise
- *      it will make your contract inaccesible or other bad things will happen.
+ *      The accessManager and the passThruMethods are in the storage, but this contract doesn't include any method
+ *      to modify them. They should be modified by the implementation contract (check AMPUtils for functions that
+ *      encapsulate these operations).
  *
  *      Check https://forum.openzeppelin.com/t/accessmanagedproxy-is-a-good-idea/41917 for a discussion on the
- *      advantages and disadvantages of using it.
+ *      advantages and disadvantages of using it. Also https://www.youtube.com/watch?v=DKdwJ9Ap9vM for a presentation
+ *      on this approach.
  *
  * @custom:security-contact [email protected]
  * @author Ensuro
  */
 contract AccessManagedProxy is AccessManagedProxyBase {
-  /**
-   * @notice AccessManager contract that handles the permissions to access the implementation methods
-   */
-  IAccessManager internal immutable _accessManager;
+  /// @custom:storage-location erc7201:ensuro.storage.AccessManagedProxy
+  /// For struct AMPUtils.AccessManagedProxyStorage
 
   /**
    * @notice Constructor of the proxy, defining the implementation and the access manager
    * @dev Initializes the upgradeable proxy with an initial implementation specified by `implementation` and
-   *      with `manager` as the ACCESS_MANAGER that will handle access control.
+   *      with `accessManager` as the ACCESS_MANAGER that will handle access control.
    *
    * @param implementation The initial implementation contract.
    * @param _data If nonempty, it's used as data in a delegate call to `implementation`. This will typically be an
    *              encoded function call, and allows initializing the storage of the proxy like a Solidity constructor.
-   * @param manager The access manager that will handle access control
+   * @param accessManager The access manager that will handle access control
+   * @param passThruMethods The selector of methods that will skip the access control validation, typically used for
+   *                        views and other methods for gas optimization.
    *
-   * @custom:pre If `data` is empty, `msg.value` must be zero.
+   * @custom:pre If `_data` is empty, `msg.value` must be zero.
    */
   constructor(
     address implementation,
     bytes memory _data,
-    IAccessManager manager
+    IAccessManager accessManager,
+    bytes4[] memory passThruMethods
   ) payable AccessManagedProxyBase(implementation, _data) {
-    _accessManager = manager;
+    AMPUtils.setAccessManager(accessManager);
+    AMPUtils.setPassThruMethods(passThruMethods);
+  }
+
+  /// @inheritdoc AccessManagedProxyBase
+  function _skipAC(bytes4 selector) internal view override returns (bool) {
+    return AMPUtils.getAccessManagedProxyStorage().skipAc[selector];
+  }
+
+  /// @inheritdoc IAccessManagedProxy
+  // solhint-disable-next-line func-name-mixedcase
+  function PASS_THRU_METHODS() external view override returns (bytes4[] memory methods) {
+    return AMPUtils.getAccessManagedProxyStorage().passThruMethods;
   }
 
+  /// @inheritdoc IAccessManagedProxy
   // solhint-disable-next-line func-name-mixedcase
   function ACCESS_MANAGER() public view override returns (IAccessManager) {
-    return _accessManager;
+    return AMPUtils.getAccessManagedProxyStorage().accessManager;
   }
 }

contracts/AccessManagedProxyBase.sol

@@ -1,8 +1,9 @@
-// SPDX-License-Identifier: MIT
+// SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.0;
 
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol";
+import {IAccessManagedProxy} from "./interfaces/IAccessManagedProxy.sol";
 
 /**
  * @title AccessManagedProxyBase
@@ -21,10 +22,7 @@ import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessMana
  * @custom:security-contact [email protected]
  * @author Ensuro
  */
-abstract contract AccessManagedProxyBase is ERC1967Proxy {
-  // Error copied from IAccessManaged
-  error AccessManagedUnauthorized(address caller);
-
+abstract contract AccessManagedProxyBase is ERC1967Proxy, IAccessManagedProxy {
   /**
    * @notice Constructor of the proxy, defining the implementation and the access manager
    * @dev Initializes the upgradeable proxy with an initial implementation specified by `implementation` and
@@ -34,18 +32,19 @@ abstract contract AccessManagedProxyBase is ERC1967Proxy {
    * @param _data If nonempty, it's used as data in a delegate call to `implementation`. This will typically be an
    *              encoded function call, and allows initializing the storage of the proxy like a Solidity constructor.
    *
-   * Requirements:
-   *
-   * - If `data` is empty, `msg.value` must be zero.
+   * @custom:pre If `_data` is empty, `msg.value` must be zero.
    */
   constructor(address implementation, bytes memory _data) payable ERC1967Proxy(implementation, _data) {}
 
-  /**
-   * @notice AccessManager contract that handles the permissions to access the implementation methods
-   */
+  /// @inheritdoc IAccessManagedProxy
   // solhint-disable-next-line func-name-mixedcase
   function ACCESS_MANAGER() public view virtual returns (IAccessManager);
 
+  /// @inheritdoc IAccessManagedProxy
+  function authority() external view virtual returns (address) {
+    return address(ACCESS_MANAGER());
+  }
+
   /**
    * @notice Intercepts the super._delegate call to implement access control
    * @dev Checks with the ACCESS_MANAGER if msg.sender is authorized to call the current call's function,
@@ -71,8 +70,5 @@ abstract contract AccessManagedProxyBase is ERC1967Proxy {
    * @param selector The selector of the method called
    * @return Whether the access control using ACCESS_MANAGER should be skipped or not
    */
-  // solhint-disable-next-line no-unused-vars
-  function _skipAC(bytes4 selector) internal view virtual returns (bool) {
-    return false;
-  }
+  function _skipAC(bytes4 selector) internal view virtual returns (bool);
 }