refactor: remove "transferOwnership"

chore: ignore and clean "coverage" directory
feat: add "ConstructorParams" in registry
refactor: delete "owner" from storage
refactor: make "owner" immutable
refactor: disallow delegate calling to the registry
test: add tests for proxy owner
test: delete unused imports
test: polish error messages in assertions

.gitignore

@@ -2,6 +2,7 @@
 artifacts
 broadcast
 cache
+coverage
 node_modules
 out-optimized
 out

.prettierignore

@@ -1,6 +1,7 @@
 # directories
 broadcast
 cache
+coverage
 lib
 node_modules
 out-optimized

package.json

@@ -39,7 +39,7 @@
   "scripts": {
     "build": "forge build",
     "build:optimized": "FOUNDRY_PROFILE=optimized forge build",
-    "clean": "rm -rf artifacts broadcast cache docs out out-optimized",
+    "clean": "rm -rf artifacts broadcast cache coverage docs out out-optimized",
     "lint": "pnpm lint:sol && pnpm prettier:check",
     "lint:sol": "forge fmt --check && pnpm solhint \"{script,src,test}/**/*.sol\"",
     "gas:report": "forge test --gas-report --no-match-test \"test(Fuzz)?_RevertWhen_\"",

src/PRBProxy.sol

@@ -27,18 +27,26 @@ contract PRBProxy is
                                      CONSTANTS
     //////////////////////////////////////////////////////////////////////////*/
 
+    /// @inheritdoc IPRBProxy
+    address public immutable override owner;
+
     /// @inheritdoc IPRBProxy
     IPRBProxyRegistry public immutable override registry;
 
     /*//////////////////////////////////////////////////////////////////////////
                                      CONSTRUCTOR
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Constructs the proxy by fetching the params from the registry.
-    /// @dev This is implemented like this so that the proxy's CREATE2 address doesn't depend on the constructor params.
+    /// @notice Creates the proxy by fetching the constructor params from the registry, optionally delegate calling
+    /// to a target contract if one is provided.
+    /// @dev The rationale of this approach is to have the proxy's CREATE2 address not depend on any constructor params.
     constructor() {
         registry = IPRBProxyRegistry(msg.sender);
-        owner = registry.transientProxyOwner();
+        (address owner_, address target, bytes memory data) = registry.constructorParams();
+        owner = owner_;
+        if (target != address(0)) {
+            _execute(target, data);
+        }
     }
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -56,7 +64,7 @@ contract PRBProxy is
 
         // Delegate call to the plugin.
         bool success;
-        (success, response) = _safeDelegateCall(address(plugin), data);
+        (success, response) = address(plugin).delegatecall(data);
 
         // Log the plugin run.
         emit RunPlugin(plugin, data, response);
@@ -89,14 +97,27 @@ contract PRBProxy is
             revert PRBProxy_ExecutionUnauthorized({ owner: owner, caller: msg.sender, target: target });
         }
 
-        // Check that the target is a contract.
+        // Delegate call to the target contract, and handle the response.
+        response = _execute(target, data);
+    }
+
+    /*//////////////////////////////////////////////////////////////////////////
+                          INTERNAL NON-CONSTANT FUNCTIONS
+    //////////////////////////////////////////////////////////////////////////*/
+
+    /// @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.
         if (target.code.length == 0) {
             revert PRBProxy_TargetNotContract(target);
+        } else if (target == address(registry)) {
+            revert PRBProxy_TargetRegistry();
         }
 
         // Delegate call to the target contract.
         bool success;
-        (success, response) = _safeDelegateCall(target, data);
+        (success, response) = target.delegatecall(data);
 
         // Log the execution.
         emit Execute(target, data, response);
@@ -115,34 +136,4 @@ contract PRBProxy is
             }
         }
     }
-
-    /// @inheritdoc IPRBProxy
-    function transferOwnership(address newOwner) external override {
-        // Check that the caller is the registry.
-        if (address(registry) != msg.sender) {
-            revert PRBProxy_CallerNotRegistry({ registry: registry, caller: msg.sender });
-        }
-
-        // Effects: update the owner.
-        owner = newOwner;
-    }
-
-    /*//////////////////////////////////////////////////////////////////////////
-                          INTERNAL NON-CONSTANT FUNCTIONS
-    //////////////////////////////////////////////////////////////////////////*/
-
-    /// @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 so that this variable cannot be modified during the DELEGATECALL.
-        address owner_ = owner;
-
-        // Delegate call to the provided contract.
-        (success, response) = to.delegatecall(data);
-
-        // Check that the owner has not been changed.
-        if (owner_ != owner) {
-            revert PRBProxy_OwnerChanged(owner_, owner);
-        }
-    }
 }

src/PRBProxyRegistry.sol

@@ -38,13 +38,13 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc IPRBProxyRegistry
-    mapping(address origin => bytes32 seed) public override nextSeeds;
+    ConstructorParams public override constructorParams;
 
     /// @inheritdoc IPRBProxyRegistry
-    mapping(address owner => IPRBProxy proxy) public override proxies;
+    mapping(address origin => bytes32 seed) public override nextSeeds;
 
     /// @inheritdoc IPRBProxyRegistry
-    address public override transientProxyOwner;
+    mapping(address owner => IPRBProxy proxy) public override proxies;
 
     /*//////////////////////////////////////////////////////////////////////////
                                      MODIFIERS
@@ -81,68 +81,25 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
         external
         override
         noProxy(msg.sender)
-        returns (IPRBProxy proxy, bytes memory response)
+        returns (IPRBProxy proxy)
     {
-        (proxy, response) = _deployAndExecute({ owner: msg.sender, target: target, data: data });
-    }
-
-    /// @inheritdoc IPRBProxyRegistry
-    function deployAndExecuteFor(
-        address owner,
-        address target,
-        bytes calldata data
-    )
-        public
-        override
-        noProxy(owner)
-        returns (IPRBProxy proxy, bytes memory response)
-    {
-        (proxy, response) = _deployAndExecute(owner, target, data);
-    }
-
-    /// @inheritdoc IPRBProxyRegistry
-    function transferOwnership(address newOwner) external override noProxy(newOwner) {
-        // Check that the caller has a proxy.
-        IPRBProxy proxy = proxies[msg.sender];
-        if (address(proxy) == address(0)) {
-            revert PRBProxyRegistry_OwnerDoesNotHaveProxy({ owner: msg.sender });
-        }
-
-        // Delete the proxy for the caller.
-        delete proxies[msg.sender];
-
-        // Set the proxy for the new owner.
-        proxies[newOwner] = proxy;
-
-        // Transfer the proxy.
-        proxy.transferOwnership(newOwner);
-
-        // Log the transfer of the proxy ownership.
-        emit TransferOwnership({ proxy: proxy, oldOwner: msg.sender, newOwner: newOwner });
-    }
-
-    /*//////////////////////////////////////////////////////////////////////////
-                          INTERNAL NON-CONSTANT FUNCTIONS
-    //////////////////////////////////////////////////////////////////////////*/
-
-    /// @dev See the documentation for the public functions that call this internal function.
-    function _deploy(address owner) internal returns (IPRBProxy proxy) {
         // Load the next seed.
         bytes32 seed = nextSeeds[tx.origin];
 
         // Prevent front-running the salt by hashing the concatenation of "tx.origin" and the user-provided seed.
         bytes32 salt = keccak256(abi.encode(tx.origin, seed));
 
-        // Deploy the proxy with CREATE2.
-        transientProxyOwner = owner;
+        // Deploy the proxy with CREATE2, and execute the delegate call in the constructor.
+        address owner = msg.sender;
+        constructorParams = ConstructorParams({ owner: owner, target: target, data: data });
         proxy = new PRBProxy{ salt: salt }();
-        delete transientProxyOwner;
+        delete constructorParams;
 
-        // Set the proxy for the owner.
+        // Associate the the owner with the proxy in the mapping.
         proxies[owner] = proxy;
 
         // Increment the seed.
-        // We're using unchecked arithmetic here because this cannot realistically overflow, ever.
+        // Using unchecked arithmetic here because this cannot realistically overflow, ever.
         unchecked {
             nextSeeds[tx.origin] = bytes32(uint256(seed) + 1);
         }
@@ -159,27 +116,24 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
         });
     }
 
+    /*//////////////////////////////////////////////////////////////////////////
+                          INTERNAL NON-CONSTANT FUNCTIONS
+    //////////////////////////////////////////////////////////////////////////*/
+
     /// @dev See the documentation for the public functions that call this internal function.
-    function _deployAndExecute(
-        address owner,
-        address target,
-        bytes calldata data
-    )
-        internal
-        returns (IPRBProxy proxy, bytes memory response)
-    {
+    function _deploy(address owner) internal returns (IPRBProxy proxy) {
         // Load the next seed.
         bytes32 seed = nextSeeds[tx.origin];
 
         // Prevent front-running the salt by hashing the concatenation of "tx.origin" and the user-provided seed.
         bytes32 salt = keccak256(abi.encode(tx.origin, seed));
 
-        // Deploy the proxy with CREATE2. The registry will temporarily be the owner of the proxy.
-        transientProxyOwner = address(this);
+        // Deploy the proxy with CREATE2.
+        constructorParams.owner = owner;
         proxy = new PRBProxy{ salt: salt }();
-        delete transientProxyOwner;
+        delete constructorParams;
 
-        // Set the proxy for the owner.
+        // Associate the the owner with the proxy in the mapping.
         proxies[owner] = proxy;
 
         // Increment the seed.
@@ -188,12 +142,6 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
             nextSeeds[tx.origin] = bytes32(uint256(seed) + 1);
         }
 
-        // Delegate call to the target contract.
-        response = proxy.execute(target, data);
-
-        // Transfer the ownership to the specified owner.
-        proxy.transferOwnership(owner);
-
         // Log the proxy via en event.
         // forgefmt: disable-next-line
         emit DeployProxy({

src/abstracts/PRBProxyStorage.sol

@@ -7,9 +7,6 @@ import { IPRBProxyStorage } from "../interfaces/IPRBProxyStorage.sol";
 /// @title PRBProxyStorage
 /// @dev This is meant to be inherited by plugins and targets. See the documentation in {IPRBProxyStorage}.
 abstract contract PRBProxyStorage is IPRBProxyStorage {
-    /// @inheritdoc IPRBProxyStorage
-    address public override owner;
-
     /// @inheritdoc IPRBProxyStorage
     mapping(bytes4 method => IPRBProxyPlugin plugin) public plugins;
 

src/interfaces/IPRBProxy.sol

@@ -33,6 +33,9 @@ interface IPRBProxy is IPRBProxyStorage {
     /// @notice Thrown when a non-contract address is passed as the target.
     error PRBProxy_TargetNotContract(address target);
 
+    /// @notice Thrown when the registry is passed as the target.
+    error PRBProxy_TargetRegistry();
+
     /*//////////////////////////////////////////////////////////////////////////
                                        EVENTS
     //////////////////////////////////////////////////////////////////////////*/
@@ -47,6 +50,9 @@ interface IPRBProxy is IPRBProxyStorage {
                                  CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
+    /// @notice The address of the owner account or contract, which controls the proxy.
+    function owner() external view returns (address);
+
     /// @notice The address of the registry that has deployed this proxy.
     function registry() external view returns (IPRBProxyRegistry);
 
@@ -62,18 +68,10 @@ interface IPRBProxy is IPRBProxyStorage {
     /// Requirements:
     /// - The caller must be either an owner or an envoy with permission.
     /// - `target` must be a contract.
-    /// - The owner must not be changed during the DELEGATECALL.
+    /// - `target` must not be the registry.
     ///
     /// @param target The address of the target contract.
     /// @param data Function selector plus ABI encoded data.
     /// @return response The response received from the target contract.
     function execute(address target, bytes calldata data) external payable returns (bytes memory response);
-
-    /// @notice Transfers the owner of the contract to a new account.
-    ///
-    /// @dev Requirements:
-    /// - The caller must be the owner.
-    ///
-    /// @param newOwner The address of the new owner account.
-    function transferOwnership(address newOwner) external;
 }