refactor: move plugins and permissions to registry

feat: add "getPluginByProxy" getter feat: add "getPermissionByProxy" getter refactor: delete all logic from annex refactor: get rid of proxy storage refactor: rename getters refactor: rename "selector" param to "method" test: update tests in light of new contract API
PaulRBerg · Jun 18, 2023 · e366797 · e366797
1 parent c026f78
commit e366797
Show file tree

Hide file tree

Showing 48 changed files with 641 additions and 476 deletions.
diff --git a/.solhint.json b/.solhint.json
@@ -10,7 +10,7 @@
     "contract-name-camelcase": "off",
     "func-name-mixedcase": "off",
     "func-visibility": ["error", { "ignoreConstructors": true }],
-    "max-line-length": ["error", 120],
+    "max-line-length": ["error", 123],
     "no-complex-fallback": "off",
     "no-empty-blocks": "off",
     "no-inline-assembly": "off",

diff --git a/script/SetPermission.s.sol b/script/SetPermission.s.sol
@@ -1,28 +1,14 @@
 // SPDX-License-Identifier: MIT
 pragma solidity >=0.8.19 <0.9.0;
 
-import { IPRBProxy } from "../src/interfaces/IPRBProxy.sol";
-import { IPRBProxyAnnex } from "../src/interfaces/IPRBProxyAnnex.sol";
+import { IPRBProxyRegistry } from "../src/interfaces/IPRBProxyRegistry.sol";
 
 import { BaseScript } from "./Base.s.sol";
 
-/// @notice Bootstraps the proxy system by giving permission to an envoy and installing a plugin.
+/// @notice Permits an envoy to delegate call to a target contract.
 contract SetPermission is BaseScript {
-    function run(
-        IPRBProxy proxy,
-        IPRBProxyAnnex annex,
-        address target
-    )
-        public
-        broadcaster
-        returns (bytes memory response)
-    {
-        // ABI encode the call to `setPermission`.
+    function run(IPRBProxyRegistry registry, address target, bool permission) public broadcaster {
         address envoy = vm.addr(vm.deriveKey(mnemonic, 1));
-        bool permission = true;
-        bytes memory data = abi.encodeCall(annex.setPermission, (envoy, target, permission));
-
-        // Execute the call to the annex.
-        response = proxy.execute(address(annex), data);
+        registry.setPermission({ envoy: envoy, target: target, permission: permission });
     }
 }
diff --git a/src/PRBProxy.sol b/src/PRBProxy.sol
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity >=0.8.18;
 
-import { PRBProxyStorage } from "./abstracts/PRBProxyStorage.sol";
 import { IPRBProxy } from "./interfaces/IPRBProxy.sol";
 import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
 import { IPRBProxyRegistry } from "./interfaces/IPRBProxyRegistry.sol";
@@ -19,10 +18,7 @@ import { IPRBProxyRegistry } from "./interfaces/IPRBProxyRegistry.sol";
 
 /// @title PRBProxy
 /// @dev See the documentation in {IPRBProxy}.
-contract PRBProxy is
-    PRBProxyStorage, // 1 inherited component
-    IPRBProxy // 1 inherited component
-{
+contract PRBProxy is IPRBProxy {
     /*//////////////////////////////////////////////////////////////////////////
                                      CONSTANTS
     //////////////////////////////////////////////////////////////////////////*/
@@ -56,10 +52,10 @@ contract PRBProxy is
     /// @notice Fallback function used to run plugins.
     /// @dev WARNING: anyone can call this function and thus run any installed plugin.
     fallback(bytes calldata data) external payable returns (bytes memory response) {
-        // Check if the function signature exists in the installed plugins mapping.
-        IPRBProxyPlugin plugin = plugins[msg.sig];
+        // Check if the function signature points to a known installed plugin.
+        IPRBProxyPlugin plugin = registry.getPluginByOwner({ owner: owner, method: msg.sig });
         if (address(plugin) == address(0)) {
-            revert PRBProxy_PluginNotInstalledForMethod({ caller: msg.sender, selector: msg.sig });
+            revert PRBProxy_PluginNotInstalledForMethod({ caller: msg.sender, owner: owner, method: msg.sig });
         }
 
         // Delegate call to the plugin.
@@ -93,7 +89,8 @@ contract PRBProxy is
     /// @inheritdoc IPRBProxy
     function execute(address target, bytes calldata data) external payable override returns (bytes memory response) {
         // Check that the caller is either the owner or an envoy with permission.
-        if (owner != msg.sender && !permissions[msg.sender][target]) {
+        bool permission = registry.getPermissionByOwner({ owner: owner, envoy: msg.sender, target: target });
+        if (owner != msg.sender && !permission) {
             revert PRBProxy_ExecutionUnauthorized({ owner: owner, caller: msg.sender, target: target });
         }
 
@@ -105,7 +102,7 @@ contract PRBProxy is
                           INTERNAL NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Executes a DELEGATECALL to the provided `target` with the provided `data`.
+    /// @notice Executes a DELEGATECALL to the provided target with the provided data.
     /// @dev Shared logic between the constructor and the {execute} function.
     function _execute(address target, bytes memory data) internal returns (bytes memory response) {
         // Check that the target is a contract, and it is not the registry.

diff --git a/src/PRBProxyAnnex.sol b/src/PRBProxyAnnex.sol
@@ -1,9 +1,7 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity >=0.8.18;
 
-import { PRBProxyStorage } from "./abstracts/PRBProxyStorage.sol";
 import { IPRBProxyAnnex } from "./interfaces/IPRBProxyAnnex.sol";
-import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
 
 /*
 
@@ -25,70 +23,11 @@ import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
 
 /// @title PRBProxyAnnex
 /// @dev See the documentation in {IPRBProxyAnnex}.
-contract PRBProxyAnnex is
-    IPRBProxyAnnex, // 0 inherited components
-    PRBProxyStorage // 1 inherited component
-{
+contract PRBProxyAnnex is IPRBProxyAnnex {
     /*//////////////////////////////////////////////////////////////////////////
                                 USER-FACING STORAGE
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc IPRBProxyAnnex
     string public constant override VERSION = "4.0.0-beta.5";
-
-    /*//////////////////////////////////////////////////////////////////////////
-                               NON-CONSTANT FUNCTIONS
-    //////////////////////////////////////////////////////////////////////////*/
-
-    /// @inheritdoc IPRBProxyAnnex
-    function installPlugin(IPRBProxyPlugin plugin) external override {
-        // Get the method list to install.
-        bytes4[] memory methodList = plugin.methodList();
-
-        // The plugin must have at least one listed method.
-        uint256 length = methodList.length;
-        if (length == 0) {
-            revert PRBProxy_NoPluginMethods(plugin);
-        }
-
-        // Enable every method in the list.
-        for (uint256 i = 0; i < length;) {
-            plugins[methodList[i]] = plugin;
-            unchecked {
-                i += 1;
-            }
-        }
-
-        // Log the plugin installation.
-        emit InstallPlugin(plugin);
-    }
-
-    /// @inheritdoc IPRBProxyAnnex
-    function setPermission(address envoy, address target, bool permission) external override {
-        permissions[envoy][target] = permission;
-        emit SetPermission(envoy, target, permission);
-    }
-
-    /// @inheritdoc IPRBProxyAnnex
-    function uninstallPlugin(IPRBProxyPlugin plugin) external {
-        // Get the method list to uninstall.
-        bytes4[] memory methodList = plugin.methodList();
-
-        // The plugin must have at least one listed method.
-        uint256 length = methodList.length;
-        if (length == 0) {
-            revert PRBProxy_NoPluginMethods(plugin);
-        }
-
-        // Disable every method in the list.
-        for (uint256 i = 0; i < length;) {
-            delete plugins[methodList[i]];
-            unchecked {
-                i += 1;
-            }
-        }
-
-        // Log the plugin uninstallation.
-        emit UninstallPlugin(plugin);
-    }
 }
diff --git a/src/PRBProxyRegistry.sol b/src/PRBProxyRegistry.sol
@@ -2,6 +2,7 @@
 pragma solidity >=0.8.18;
 
 import { IPRBProxy } from "./interfaces/IPRBProxy.sol";
+import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
 import { IPRBProxyRegistry } from "./interfaces/IPRBProxyRegistry.sol";
 import { PRBProxy } from "./PRBProxy.sol";
 
@@ -47,26 +48,73 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
                                   INTERNAL STORAGE
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @dev Maps owner addresses to proxy contracts.
+    mapping(address owner => mapping(address envoy => mapping(address target => bool permission))) internal _permissions;
+
+    mapping(address owner => mapping(bytes4 method => IPRBProxyPlugin plugin)) internal _plugins;
+
     mapping(address owner => IPRBProxy proxy) internal _proxies;
 
     /*//////////////////////////////////////////////////////////////////////////
                                      MODIFIERS
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @notice Check that the owner does not have a proxy.
-    modifier noProxy(address owner) {
+    modifier onlyCallerWithoutProxy(address owner) {
         IPRBProxy proxy = _proxies[owner];
         if (address(proxy) != address(0)) {
             revert PRBProxyRegistry_OwnerHasProxy(owner, proxy);
         }
         _;
     }
 
+    /// @notice Checks that the caller has a proxy.
+    modifier onlyCallerWithProxy() {
+        if (address(_proxies[msg.sender]) == address(0)) {
+            revert PRBProxyRegistry_CallerDoesNotHaveProxy(msg.sender);
+        }
+        _;
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
                            USER-FACING CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
+    /// @inheritdoc IPRBProxyRegistry
+    function getPermissionByOwner(
+        address owner,
+        address envoy,
+        address target
+    )
+        external
+        view
+        returns (bool permission)
+    {
+        permission = _permissions[owner][envoy][target];
+    }
+
+    /// @inheritdoc IPRBProxyRegistry
+    function getPermissionByProxy(
+        IPRBProxy proxy,
+        address envoy,
+        address target
+    )
+        external
+        view
+        returns (bool permission)
+    {
+        permission = _permissions[proxy.owner()][envoy][target];
+    }
+
+    /// @inheritdoc IPRBProxyRegistry
+    function getPluginByOwner(address owner, bytes4 method) external view returns (IPRBProxyPlugin plugin) {
+        plugin = _plugins[owner][method];
+    }
+
+    /// @inheritdoc IPRBProxyRegistry
+    function getPluginByProxy(IPRBProxy proxy, bytes4 method) external view returns (IPRBProxyPlugin plugin) {
+        plugin = _plugins[proxy.owner()][method];
+    }
+
     /// @inheritdoc IPRBProxyRegistry
     function getProxy(address owner) external view returns (IPRBProxy proxy) {
         proxy = _proxies[owner];
@@ -77,7 +125,7 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc IPRBProxyRegistry
-    function deploy() external override noProxy(msg.sender) returns (IPRBProxy proxy) {
+    function deploy() external override onlyCallerWithoutProxy(msg.sender) returns (IPRBProxy proxy) {
         proxy = _deploy({ owner: msg.sender });
     }
 
@@ -88,7 +136,7 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
     )
         external
         override
-        noProxy(msg.sender)
+        onlyCallerWithoutProxy(msg.sender)
         returns (IPRBProxy proxy)
     {
         // Load the next seed.
@@ -125,10 +173,65 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
     }
 
     /// @inheritdoc IPRBProxyRegistry
-    function deployFor(address owner) public override noProxy(owner) returns (IPRBProxy proxy) {
+    function deployFor(address owner) public override onlyCallerWithoutProxy(owner) returns (IPRBProxy proxy) {
         proxy = _deploy(owner);
     }
 
+    /// @inheritdoc IPRBProxyRegistry
+    function installPlugin(IPRBProxyPlugin plugin) external override onlyCallerWithProxy {
+        // Get the method list to install.
+        bytes4[] memory methodList = plugin.methodList();
+
+        // The plugin must have at least one listed method.
+        uint256 length = methodList.length;
+        if (length == 0) {
+            revert PRBProxyRegistry_PluginEmptyMethodList(plugin);
+        }
+
+        // Install every method in the list.
+        address owner = msg.sender;
+        for (uint256 i = 0; i < length;) {
+            _plugins[owner][methodList[i]] = plugin;
+            unchecked {
+                i += 1;
+            }
+        }
+
+        // Log the plugin installation.
+        emit InstallPlugin(owner, _proxies[owner], plugin);
+    }
+
+    /// @inheritdoc IPRBProxyRegistry
+    function setPermission(address envoy, address target, bool permission) external override onlyCallerWithProxy {
+        address owner = msg.sender;
+        _permissions[owner][envoy][target] = permission;
+        emit SetPermission(owner, _proxies[owner], envoy, target, permission);
+    }
+
+    /// @inheritdoc IPRBProxyRegistry
+    function uninstallPlugin(IPRBProxyPlugin plugin) external override onlyCallerWithProxy {
+        // Get the method list to uninstall.
+        bytes4[] memory methodList = plugin.methodList();
+
+        // The plugin must have at least one listed method.
+        uint256 length = methodList.length;
+        if (length == 0) {
+            revert PRBProxyRegistry_PluginEmptyMethodList(plugin);
+        }
+
+        // Uninstall every method in the list.
+        address owner = msg.sender;
+        for (uint256 i = 0; i < length;) {
+            delete _plugins[owner][methodList[i]];
+            unchecked {
+                i += 1;
+            }
+        }
+
+        // Log the plugin uninstallation.
+        emit UninstallPlugin(owner, _proxies[owner], plugin);
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
                           INTERNAL NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/

diff --git a/src/abstracts/PRBProxyPlugin.sol b/src/abstracts/PRBProxyPlugin.sol
diff --git a/src/abstracts/PRBProxyStorage.sol b/src/abstracts/PRBProxyStorage.sol