refactor: simplify envoy system

docs: say "provided" instead of "given"
docs: document "TransferOwnership" event in NatSpec
refactor: order events alphabetically

closes #73

src/PRBProxy.sol

@@ -83,12 +83,8 @@ contract PRBProxy is IPRBProxy, PRBProxyStorage {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc IPRBProxy
-    function getPermission(
-        address envoy,
-        address target,
-        bytes4 selector
-    ) external view override returns (bool permission) {
-        permission = permissions[envoy][target][selector];
+    function getPermission(address envoy, address target) external view override returns (bool permission) {
+        permission = permissions[envoy][target];
     }
 
     /// @inheritdoc IPRBProxy
@@ -104,14 +100,8 @@ contract PRBProxy is IPRBProxy, PRBProxyStorage {
     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) {
-            bytes4 selector = bytes4(data[:4]);
-            if (!permissions[msg.sender][target][selector]) {
-                revert PRBProxy_ExecutionUnauthorized({
-                    owner: owner,
-                    caller: msg.sender,
-                    target: target,
-                    selector: selector
-                });
+            if (!permissions[msg.sender][target]) {
+                revert PRBProxy_ExecutionUnauthorized({ owner: owner, caller: msg.sender, target: target });
             }
         }
 
@@ -171,14 +161,9 @@ contract PRBProxy is IPRBProxy, PRBProxyStorage {
     }
 
     /// @inheritdoc IPRBProxy
-    function setPermission(
-        address envoy,
-        address target,
-        bytes4 selector,
-        bool permission
-    ) external override onlyOwner {
-        permissions[envoy][target][selector] = permission;
-        emit SetPermission(envoy, target, selector, permission);
+    function setPermission(address envoy, address target, bool permission) external override onlyOwner {
+        permissions[envoy][target] = permission;
+        emit SetPermission(envoy, target, permission);
     }
 
     /// @inheritdoc IPRBProxy
@@ -225,7 +210,7 @@ contract PRBProxy is IPRBProxy, PRBProxyStorage {
                           INTERNAL NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Performs a DELEGATECALL to the given address with the given data.
+    /// @notice Performs a DELEGATECALL to the provided address with the provided data.
     /// @dev Shared logic between the {execute} and the {fallback} functions.
     function _safeDelegateCall(address to, bytes memory data) internal returns (bool success, bytes memory response) {
         // Save the owner address in memory. This variable cannot be modified during the DELEGATECALL.
@@ -234,7 +219,7 @@ contract PRBProxy is IPRBProxy, PRBProxyStorage {
         // Reserve some gas to ensure that the function has enough to finish the execution.
         uint256 stipend = gasleft() - minGasReserve;
 
-        // Delegate call to the given contract.
+        // Delegate call to the provided contract.
         (success, response) = to.delegatecall{ gas: stipend }(data);
 
         // Check that the owner has not been changed.

src/PRBProxyStorage.sol

@@ -25,6 +25,5 @@ abstract contract PRBProxyStorage is IPRBProxy {
     mapping(bytes4 method => IPRBProxyPlugin plugin) internal plugins;
 
     /// @dev Maps envoys to target contracts to function selectors to boolean flags.
-    mapping(address envoy => mapping(address target => mapping(bytes4 selector => bool permission)))
-        internal permissions;
+    mapping(address envoy => mapping(address target => bool permission)) internal permissions;
 }

src/interfaces/IPRBProxy.sol

@@ -15,7 +15,7 @@ interface IPRBProxy {
     error PRBProxy_ExecutionReverted();
 
     /// @notice Emitted when the caller is not the owner.
-    error PRBProxy_ExecutionUnauthorized(address owner, address caller, address target, bytes4 selector);
+    error PRBProxy_ExecutionUnauthorized(address owner, address caller, address target);
 
     /// @notice Emitted when the caller is not the owner.
     error PRBProxy_NotOwner(address owner, address caller);
@@ -45,42 +45,43 @@ interface IPRBProxy {
     /// @notice Emitted when a plugin is installed.
     event InstallPlugin(IPRBProxyPlugin indexed plugin);
 
-    /// @notice Emitted when a plugin is run for a given method.
+    /// @notice Emitted when a plugin is run for a provided method.
     event RunPlugin(IPRBProxyPlugin indexed plugin, bytes data, bytes response);
 
     /// @notice Emitted when the owner sets the permission for an envoy.
-    event SetPermission(address indexed envoy, address indexed target, bytes4 indexed selector, bool permission);
+    event SetPermission(address indexed envoy, address indexed target, bool permission);
 
     /// @notice Emitted when the owner changes the proxy's owner.
     event TransferOwnership(address indexed oldOwner, address indexed newOwner);
 
+    /// @notice Emitted when a plugin is uninstalled.
     event UninstallPlugin(IPRBProxyPlugin indexed plugin);
 
     /*//////////////////////////////////////////////////////////////////////////
                               PUBLIC CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Returns a boolean flag that indicates whether the envoy has permission to call the given target
-    /// contract and function selector.
-    function getPermission(address envoy, address target, bytes4 selector) external view returns (bool permission);
+    /// @notice Returns a boolean flag that indicates whether the envoy has permission to call the provided target
+    /// contract.
+    function getPermission(address envoy, address target) external view returns (bool permission);
 
-    /// @notice Returns the address of the plugin installed for the the given method.
+    /// @notice Returns the address of the plugin installed for the the provided method.
     /// @dev Returns the zero address if no plugin is installed.
     /// @param method The signature of the method to make the query for.
     function getPluginForMethod(bytes4 method) external view returns (IPRBProxyPlugin plugin);
 
-    /// @notice The address of the owner account or contract.
-    function owner() external view returns (address);
-
     /// @notice How much gas to reserve for running the remainder of the "execute" function after the DELEGATECALL.
     /// @dev This prevents the proxy from becoming unusable if EVM opcode gas costs change in the future.
     function minGasReserve() external view returns (uint256);
 
+    /// @notice The address of the owner account or contract.
+    function owner() external view returns (address);
+
     /*//////////////////////////////////////////////////////////////////////////
                             PUBLIC NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Delegate calls to the given target contract by forwarding the data. It then returns the data it
+    /// @notice Delegate calls to the provided target contract by forwarding the data. It then returns the data it
     /// gets back, bubbling up any potential revert.
     ///
     /// @dev Emits an {Execute} event.
@@ -112,23 +113,24 @@ interface IPRBProxy {
     /// @param plugin The address of the plugin to install.
     function installPlugin(IPRBProxyPlugin plugin) external;
 
-    /// @notice Gives or takes a permission from an envoy to call the given target contract and function selector
+    /// @notice Gives or takes a permission from an envoy to call the provided target contract and function selector
     /// on behalf of the owner.
     ///
-    /// @dev It is not an error to reset a permission on the same (envoy,target,selector) tuple multiple types.
+    /// @dev It is not an error to reset a permission on the same (envoy,target) tuple multiple types.
     ///
     /// Requirements:
     /// - The caller must be the owner.
     ///
     /// @param envoy The address of the envoy account.
     /// @param target The address of the target contract.
-    /// @param selector The 4 byte function selector on the target contract.
     /// @param permission The boolean permission to set.
-    function setPermission(address envoy, address target, bytes4 selector, bool permission) external;
+    function setPermission(address envoy, address target, bool permission) external;
 
     /// @notice Transfers the owner of the contract to a new account.
     ///
-    /// @dev Requirements:
+    /// @dev Emits a {TransferOwnership} event.
+    ///
+    /// Requirements:
     /// - The caller must be the owner.
     ///
     /// @param newOwner The address of the new owner account.

src/interfaces/IPRBProxyFactory.sol

@@ -33,7 +33,7 @@ interface IPRBProxyFactory {
     /// @param eoa The externally owned account that will own the proxy.
     function getNextSeed(address eoa) external view returns (bytes32 result);
 
-    /// @notice Checks if the given address is a deployed proxy.
+    /// @notice Checks if the provided address is a deployed proxy.
     /// @param proxy The address of the proxy to make the query for.
     function isProxy(IPRBProxy proxy) external view returns (bool result);
 
@@ -46,14 +46,14 @@ interface IPRBProxyFactory {
     /// @return proxy The address of the newly deployed proxy contract.
     function deploy() external returns (IPRBProxy proxy);
 
-    /// @notice Deploys a new proxy with CREATE2 for the given owner.
+    /// @notice Deploys a new proxy with CREATE2 for the provided owner.
     /// @dev Emits a {DeployProxy} event.
     /// @param owner The owner of the proxy.
     /// @return proxy The address of the newly deployed proxy contract.
     function deployFor(address owner) external returns (IPRBProxy proxy);
 
     /// @notice Deploys a new proxy with CREATE2 by setting the caller as the owner, and delegate calls to the
-    /// given target contract by forwarding the data. It returns the data it gets back, bubbling up any potential
+    /// provided target contract by forwarding the data. It returns the data it gets back, bubbling up any potential
     /// revert.
     ///
     /// @dev Emits a {DeployProxy} and an {Execute} event.
@@ -70,7 +70,7 @@ interface IPRBProxyFactory {
         bytes calldata data
     ) external returns (IPRBProxy proxy, bytes memory response);
 
-    /// @notice Deploys a new proxy with CREATE2 for the given owner, and delegate calls to the given target
+    /// @notice Deploys a new proxy with CREATE2 for the provided owner, and delegate calls to the provided target
     /// contract by forwarding the data. It returns the data it gets back, bubbling up any potential revert.
     ///
     /// @dev Emits a {DeployProxy} and an {Execute} event.

src/interfaces/IPRBProxyRegistry.sol

@@ -13,7 +13,7 @@ interface IPRBProxyRegistry {
                                     CUSTOM ERRORS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Emitted when a proxy already exists for the given owner.
+    /// @notice Emitted when a proxy already exists for the provided owner.
     error PRBProxyRegistry_ProxyAlreadyExists(address owner);
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -23,7 +23,7 @@ interface IPRBProxyRegistry {
     /// @notice Address of the proxy factory contract.
     function factory() external view returns (IPRBProxyFactory proxyFactory);
 
-    /// @notice Gets the current proxy of the given owner.
+    /// @notice Gets the current proxy of the provided owner.
     /// @param owner The address of the owner of the current proxy.
     function getCurrentProxy(address owner) external view returns (IPRBProxy proxy);
 
@@ -41,7 +41,7 @@ interface IPRBProxyRegistry {
     /// @return proxy The address of the newly deployed proxy contract.
     function deploy() external returns (IPRBProxy proxy);
 
-    /// @notice Deploys a new proxy via the proxy factory for the given owner.
+    /// @notice Deploys a new proxy via the proxy factory for the provided owner.
     ///
     /// @dev Emits a {DeployProxy} event.
     ///
@@ -53,7 +53,7 @@ interface IPRBProxyRegistry {
     function deployFor(address owner) external returns (IPRBProxy proxy);
 
     /// @notice Deploys a new proxy via the proxy factory by setting the caller as the owner, and delegate calls to the
-    /// given target contract by forwarding the data. It returns the data it gets back, bubbling up any potential
+    /// provided target contract by forwarding the data. It returns the data it gets back, bubbling up any potential
     /// revert.
     ///
     /// @dev Emits a {DeployProxy} and an {Execute} event.
@@ -70,8 +70,8 @@ interface IPRBProxyRegistry {
         bytes calldata data
     ) external returns (IPRBProxy proxy, bytes memory response);
 
-    /// @notice Deploys a new proxy via the proxy factor for the given owner, and delegate calls to the given target
-    /// contract by forwarding the data. It returns the data it gets back, bubbling up any potential revert.
+    /// @notice Deploys a new proxy via the proxy factor for the provided owner, and delegate calls to the provided
+    /// target contract by forwarding the data. It returns the data it gets back, bubbling up any potential revert.
     ///
     /// @dev Emits a {DeployProxy} and an {Execute} event.
     ///

test/proxy/PRBProxy.t.sol

@@ -15,7 +15,7 @@ contract PRBProxy_Test is Base_Test {
 
     event RunPlugin(IPRBProxyPlugin indexed plugin, bytes data, bytes response);
 
-    event SetPermission(address indexed envoy, address indexed target, bytes4 indexed selector, bool permission);
+    event SetPermission(address indexed envoy, address indexed target, bool permission);
 
     event TransferOwnership(address indexed oldOwner, address indexed newOwner);
 

test/proxy/execute/execute.t.sol

@@ -23,25 +23,15 @@ contract Execute_Test is PRBProxy_Test {
                 IPRBProxy.PRBProxy_ExecutionUnauthorized.selector,
                 owner,
                 users.eve,
-                address(targets.dummy),
-                targets.dummy.foo.selector
+                address(targets.dummy)
             )
         );
         proxy.execute(address(targets.dummy), data);
     }
 
-    modifier callerHasPermission() {
-        _;
-    }
-
     /// @dev it should revert.
-    function test_RevertWhen_PermissionDifferentTarget() external callerUnauthorized callerHasPermission {
-        proxy.setPermission({
-            envoy: users.envoy,
-            target: address(targets.echo),
-            selector: targets.dummy.foo.selector,
-            permission: true
-        });
+    function test_RevertWhen_PermissionDifferentTarget() external callerUnauthorized {
+        proxy.setPermission({ envoy: users.envoy, target: address(targets.echo), permission: true });
         changePrank(users.envoy);
 
         bytes memory data = bytes.concat(targets.dummy.foo.selector);
@@ -50,77 +40,12 @@ contract Execute_Test is PRBProxy_Test {
                 IPRBProxy.PRBProxy_ExecutionUnauthorized.selector,
                 owner,
                 users.envoy,
-                address(targets.dummy),
-                targets.dummy.foo.selector
+                address(targets.dummy)
             )
         );
         proxy.execute(address(targets.dummy), data);
     }
 
-    modifier callerPermissionSameTarget() {
-        _;
-    }
-
-    /// @dev it should revert.
-    function test_RevertWhen_TargetDoesNotHaveFallbackFunction()
-        external
-        callerUnauthorized
-        callerHasPermission
-        callerPermissionSameTarget
-    {
-        proxy.setPermission({
-            envoy: users.envoy,
-            target: address(targets.dummy),
-            selector: targets.dummy.foo.selector,
-            permission: true
-        });
-        changePrank(users.envoy);
-
-        bytes memory data = bytes.concat(targets.dummy.bar.selector);
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IPRBProxy.PRBProxy_ExecutionUnauthorized.selector,
-                owner,
-                users.envoy,
-                address(targets.dummy),
-                targets.dummy.bar.selector
-            )
-        );
-        proxy.execute(address(targets.dummy), data);
-    }
-
-    /// @dev it should revert.
-    function test_RevertWhen_TargetHasFallbackFunction()
-        external
-        callerUnauthorized
-        callerHasPermission
-        callerPermissionSameTarget
-    {
-        proxy.setPermission({
-            envoy: users.envoy,
-            target: address(targets.dummyWithFallback),
-            selector: targets.dummyWithFallback.foo.selector,
-            permission: true
-        });
-        changePrank(users.envoy);
-
-        // Fudge the calldata such that `data` is empty, but there is additional calldata after it. This will
-        // attempt to bypass the usual selector checks, and call the fallback function on the target.
-        bytes memory usualCalldata = abi.encodeWithSelector(
-            proxy.execute.selector,
-            address(targets.dummyWithFallback),
-            new bytes(0)
-        );
-        bytes memory data = abi.encodePacked(usualCalldata, targets.dummyWithFallback.foo.selector);
-        (bool success, bytes memory response) = address(proxy).call(data);
-
-        // Assert that the call failed.
-        assertFalse(success);
-
-        // Assert that the call reverted with no response.
-        assertEq(response.length, 0);
-    }
-
     modifier callerAuthorized() {
         _;
     }
@@ -357,15 +282,7 @@ contract Execute_Test is PRBProxy_Test {
     }
 
     modifier targetDoesNotSelfDestruct() {
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoAddress.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoBytesArray.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoBytes32.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoMsgValue.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoString.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoStruct.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoUint8.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoUint256.selector, true);
-        proxy.setPermission(users.envoy, address(targets.echo), targets.echo.echoUint256Array.selector, true);
+        proxy.setPermission({ envoy: users.envoy, target: address(targets.echo), permission: true });
         _;
     }