Merge pull request #81 from PaulRBerg/refactor/registry

Merge registry with factory + move `transferOwnership` to registry

.gas-snapshot

@@ -1,76 +1,55 @@
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Deploy() (gas: 952734)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Deploy() (gas: 979763)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Event_Deploy() (gas: 525544)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Event_Deploy() (gas: 554663)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Event_Execute() (gas: 519880)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Event_Execute() (gas: 546887)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Execute() (gas: 508706)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Execute() (gas: 535731)
-DeployAndExecuteFor_Test:test_DeployAndExecuteFor_UpdateCurrentProxies() (gas: 552259)
-DeployAndExecute_Test:test_DeployAndExecute_Deploy() (gas: 950461)
-DeployAndExecute_Test:test_DeployAndExecute_Deploy() (gas: 977486)
-DeployAndExecute_Test:test_DeployAndExecute_Event_Deploy() (gas: 522975)
-DeployAndExecute_Test:test_DeployAndExecute_Event_Deploy() (gas: 552202)
-DeployAndExecute_Test:test_DeployAndExecute_Event_Execute() (gas: 517579)
-DeployAndExecute_Test:test_DeployAndExecute_Event_Execute() (gas: 544582)
-DeployAndExecute_Test:test_DeployAndExecute_Execute() (gas: 508684)
-DeployAndExecute_Test:test_DeployAndExecute_Execute() (gas: 535709)
-DeployAndExecute_Test:test_DeployAndExecute_UpdateCurrentProxies() (gas: 549954)
-DeployFor_Test:test_DeployFor() (gas: 1884092)
-DeployFor_Test:test_DeployFor_CurrentProxies() (gas: 1452611)
-DeployFor_Test:test_DeployFor_DeployerDidNotDeployAnotherProxyForHimselfViaFactory() (gas: 1421528)
-DeployFor_Test:test_DeployFor_DeployerDidNotDeployAnotherProxyForTheOwnerViaFactory() (gas: 959008)
-DeployFor_Test:test_DeployFor_DeployerSameAsOwner() (gas: 958954)
-DeployFor_Test:test_DeployFor_Event() (gas: 533810)
-DeployFor_Test:test_DeployFor_Event() (gas: 988554)
-DeployFor_Test:test_DeployFor_FirstProxy() (gas: 955454)
-DeployFor_Test:test_DeployFor_OwnerTransferredOwnership() (gas: 1424159)
-DeployFor_Test:test_DeployFor_TxOriginNotSameAsOwner_NotFirstProxy() (gas: 1415989)
-DeployFor_Test:test_DeployFor_TxOriginNotSameAsOwner_NotFirstProxy_UpdateNextSeeds() (gas: 975062)
-DeployFor_Test:test_DeployFor_TxOriginSameAsOwner() (gas: 933280)
-DeployFor_Test:test_DeployFor_UpdateProxies() (gas: 972847)
-Deploy_Test:test_Deploy() (gas: 931076)
-Deploy_Test:test_Deploy() (gas: 956786)
-Deploy_Test:test_Deploy_Event() (gas: 503819)
-Deploy_Test:test_Deploy_Event() (gas: 531475)
-Deploy_Test:test_Deploy_UpdateCurrentProxies() (gas: 529349)
-Deploy_Test:test_Deploy_UpdateNextSeeds() (gas: 490206)
-Deploy_Test:test_Deploy_UpdateProxies() (gas: 487985)
-Execute_Test:testFuzz_Execute_Event(uint256) (runs: 5000, μ: 78939, ~: 78939)
-Execute_Test:testFuzz_Execute_ReturnAddress(address) (runs: 5000, μ: 71244, ~: 71244)
-Execute_Test:testFuzz_Execute_ReturnBytes32(bytes32) (runs: 5000, μ: 71164, ~: 71164)
-Execute_Test:testFuzz_Execute_ReturnBytesArray(bytes) (runs: 5000, μ: 78826, ~: 77491)
-Execute_Test:testFuzz_Execute_ReturnString(string) (runs: 5000, μ: 79291, ~: 79654)
-Execute_Test:testFuzz_Execute_ReturnStruct((uint256,uint256,uint256)) (runs: 5000, μ: 74909, ~: 74909)
-Execute_Test:testFuzz_Execute_ReturnUint256(uint256) (runs: 5000, μ: 71099, ~: 71099)
-Execute_Test:testFuzz_Execute_ReturnUint256Array(uint256[]) (runs: 5000, μ: 344106, ~: 340937)
-Execute_Test:testFuzz_Execute_ReturnUint8(uint8) (runs: 5000, μ: 71143, ~: 71143)
+DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Deploy(address,address,address) (runs: 5000, μ: 545020, ~: 545024)
+DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Event_Deploy(address,address,address) (runs: 5000, μ: 548307, ~: 548311)
+DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Event_Execute(address,address,address) (runs: 5000, μ: 544068, ~: 544072)
+DeployAndExecuteFor_Test:test_DeployAndExecuteFor_Execute(address,address,address) (runs: 5000, μ: 537704, ~: 537708)
+DeployAndExecuteFor_Test:test_DeployAndExecuteFor_UpdateCurrentProxies(address,address,address) (runs: 5000, μ: 546144, ~: 546148)
+DeployAndExecute_Test:testFuzz_DeployAndExecute_Deploy(address,address) (runs: 5000, μ: 544741, ~: 544749)
+DeployAndExecute_Test:testFuzz_DeployAndExecute_Event_Deploy(address,address) (runs: 5000, μ: 547923, ~: 547931)
+DeployAndExecute_Test:testFuzz_DeployAndExecute_Event_Execute(address,address) (runs: 5000, μ: 543758, ~: 543766)
+DeployAndExecute_Test:testFuzz_DeployAndExecute_Execute(address,address) (runs: 5000, μ: 537381, ~: 537389)
+DeployAndExecute_Test:testFuzz_DeployAndExecute_UpdateProxies(address,address) (runs: 5000, μ: 545833, ~: 545841)
+DeployFor_Test:testFuzz_DeployFor(address,address,address) (runs: 5000, μ: 522870, ~: 522874)
+DeployFor_Test:testFuzz_DeployFor_Event(address,address,address) (runs: 5000, μ: 526097, ~: 526101)
+DeployFor_Test:testFuzz_DeployFor_UpdateProxies(address,address,address) (runs: 5000, μ: 523951, ~: 523955)
+Deploy_Test:testFuzz_Deploy(address,address) (runs: 5000, μ: 522715, ~: 522723)
+Deploy_Test:testFuzz_Deploy_Event(address,address) (runs: 5000, μ: 525971, ~: 525979)
+Deploy_Test:testFuzz_Deploy_UpdateProxies(address,address) (runs: 5000, μ: 523791, ~: 523799)
+Execute_Test:testFuzz_Execute_Event(uint256) (runs: 5000, μ: 78955, ~: 78955)
+Execute_Test:testFuzz_Execute_ReturnAddress(address) (runs: 5000, μ: 71267, ~: 71267)
+Execute_Test:testFuzz_Execute_ReturnBytes32(bytes32) (runs: 5000, μ: 71158, ~: 71158)
+Execute_Test:testFuzz_Execute_ReturnBytesArray(bytes) (runs: 5000, μ: 78820, ~: 77485)
+Execute_Test:testFuzz_Execute_ReturnString(string) (runs: 5000, μ: 79298, ~: 79648)
+Execute_Test:testFuzz_Execute_ReturnStruct((uint256,uint256,uint256)) (runs: 5000, μ: 74903, ~: 74903)
+Execute_Test:testFuzz_Execute_ReturnUint256(uint256) (runs: 5000, μ: 71093, ~: 71093)
+Execute_Test:testFuzz_Execute_ReturnUint256Array(uint256[]) (runs: 5000, μ: 344100, ~: 340931)
+Execute_Test:testFuzz_Execute_ReturnUint8(uint8) (runs: 5000, μ: 71137, ~: 71137)
 Execute_Test:test_Execute_EtherSent() (gas: 27293)
 Execute_Test:test_Execute_TargetSelfDestructs() (gas: 31001)
-InstallPlugin_Test:test_InstallPlugin() (gas: 77335)
-InstallPlugin_Test:test_InstallPlugin_Event() (gas: 76596)
-InstallPlugin_Test:test_InstallPlugin_PluginInstalledBefore() (gas: 88158)
+InstallPlugin_Test:test_InstallPlugin() (gas: 77387)
+InstallPlugin_Test:test_InstallPlugin_Event() (gas: 76579)
+InstallPlugin_Test:test_InstallPlugin_PluginInstalledBefore() (gas: 88204)
 Receive_Test:test_Receive() (gas: 12159)
-RunPlugin_Test:test_RunPlugin_EtherSent() (gas: 243895)
-RunPlugin_Test:test_RunPlugin_Event() (gas: 86531)
-RunPlugin_Test:test_RunPlugin_PluginSelfDestructs() (gas: 66008)
-RunPlugin_Test:test_RunPlugin_Zzz() (gas: 79745)
-SetMinGasReserve_Test:testFuzz_SetMinGasReserve_Event(uint256) (runs: 5000, μ: 25338, ~: 25555)
-SetMinGasReserve_Test:testFuzz_SetMinGasReserve_Update(uint256) (runs: 5000, μ: 24939, ~: 25156)
-SetPermission_Test:test_SetPermission_PermissionNotSet() (gas: 50520)
-SetPermission_Test:test_SetPermission_PermissionSet_ResetPermission() (gas: 60454)
-SetPermission_Test:test_SetPermission_PermissionSet_ResetPermission_Event() (gas: 64042)
-SetPermission_Test:test_SetPermission_PermissionSet_UnsetPermission() (gas: 43056)
-SetPermission_Test:test_SetPermission_PermissionSet_UnsetPermission_Event() (gas: 47038)
+RunPlugin_Test:test_RunPlugin_EtherSent() (gas: 243911)
+RunPlugin_Test:test_RunPlugin_Event() (gas: 86558)
+RunPlugin_Test:test_RunPlugin_PluginSelfDestructs() (gas: 66024)
+RunPlugin_Test:test_RunPlugin_Zzz() (gas: 79761)
+SetMinGasReserve_Test:testFuzz_SetMinGasReserve_Event(uint256) (runs: 5000, μ: 25335, ~: 25552)
+SetMinGasReserve_Test:testFuzz_SetMinGasReserve_Update(uint256) (runs: 5000, μ: 24958, ~: 25175)
+SetPermission_Test:test_SetPermission_PermissionNotSet() (gas: 50536)
+SetPermission_Test:test_SetPermission_PermissionSet_ResetPermission() (gas: 60531)
+SetPermission_Test:test_SetPermission_PermissionSet_ResetPermission_Event() (gas: 64041)
+SetPermission_Test:test_SetPermission_PermissionSet_UnsetPermission() (gas: 43028)
+SetPermission_Test:test_SetPermission_PermissionSet_UnsetPermission_Event() (gas: 47020)
 Storage_Test:test_Slot0() (gas: 7965)
 Storage_Test:test_Slot1() (gas: 7966)
 Storage_Test:test_Slot2() (gas: 7943)
 Storage_Test:test_Slot3() (gas: 7988)
-TransferOwnership_Test:test_TransferOwnership() (gas: 15257)
-TransferOwnership_Test:test_TransferOwnership_Event() (gas: 20875)
-TransferOwnership_Test:test_TransferOwnership_ToZeroAddress() (gas: 8285)
-UninstallPlugin_Test:test_UninstallPlugin() (gas: 66299)
-UninstallPlugin_Test:test_UninstallPlugin_Event() (gas: 65896)
-UninstallPlugin_Test:test_UninstallPlugin_PluginNotInstalledBefore() (gas: 37319)
-Version_Test:test_Version() (gas: 5559)
-Version_Test:test_Versions() (gas: 10856)
+TransferOwnership_Test:test_TransferOwnership(address) (runs: 5000, μ: 21392, ~: 21393)
+TransferOwnership_Test:test_TransferOwnership(address) (runs: 5000, μ: 563917, ~: 563921)
+TransferOwnership_Test:test_TransferOwnership_Event(address) (runs: 5000, μ: 22329, ~: 22330)
+TransferOwnership_Test:test_TransferOwnership_Event(address) (runs: 5000, μ: 564921, ~: 564925)
+UninstallPlugin_Test:test_UninstallPlugin() (gas: 66336)
+UninstallPlugin_Test:test_UninstallPlugin_Event() (gas: 65877)
+UninstallPlugin_Test:test_UninstallPlugin_PluginNotInstalledBefore() (gas: 37382)
+Version_Test:test_Version() (gas: 7458)
+Version_Test:test_Versions() (gas: 13510)

.gitmodules

@@ -1,7 +1,7 @@
 [submodule "lib/forge-std"]
-  branch = "v1"
+  branch = "master"
   path = "lib/forge-std"
-  url = "https://github.com/foundry-rs/forge-std"
+  url = "https://github.com/PaulRBerg/forge-std"
 [submodule "lib/prb-contracts"]
   branch = "v5"
   path = "lib/prb-contracts"

foundry.toml

@@ -3,12 +3,7 @@
   bytecode_hash = "none"
   cbor_metadata = false
   fs_permissions = [{ access = "read", path = "./optimized-out" }]
-  gas_reports = [
-    "PRBProxy",
-    "PRBProxyHelpers",
-    "PRBProxyFactory",
-    "PRBProxyRegistry",
-  ]
+  gas_reports = ["PRBProxy", "PRBProxyHelpers", "PRBProxyRegistry"]
   ignored_error_codes = [
     5159, # ignore selfdestruct warning
   ]

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@prb/proxy",
   "description": "Proxy contract to compose Ethereum transactions on behalf of the owner",
-  "version": "4.0.0-beta.1",
+  "version": "4.0.0-beta.2",
   "author": {
     "name": "Paul Razvan Berg",
     "url": "https://github.com/PaulRBerg"
@@ -43,9 +43,9 @@
     "clean": "rimraf broadcast cache out",
     "lint": "yarn lint:sol && yarn prettier:check",
     "lint:sol": "forge fmt --check && yarn solhint \"{script,src,test}/**/*.sol\"",
-    "gas:report": "forge test --gas-report --no-match-test \"test_RevertWhen_\"",
-    "gas:snapshot": "forge snapshot --no-match-test \"test_RevertWhen_\"",
-    "gas:snapshot:optimized": "yarn build:optimized && FOUNDRY_PROFILE=test-optimized forge snapshot --no-match-test \"test_RevertWhen_\"",
+    "gas:report": "forge test --gas-report --no-match-test \"test(Fuzz)?_RevertWhen_\"",
+    "gas:snapshot": "forge snapshot --no-match-test \"test(Fuzz)?_RevertWhen_\"",
+    "gas:snapshot:optimized": "yarn build:optimized && FOUNDRY_PROFILE=test-optimized forge snapshot --no-match-test \"test(Fuzz)?_RevertWhen_\"",
     "prettier:check": "prettier --check \"**/*.{json,md,yml}\"",
     "prettier:write": "prettier --write \"**/*.{json,md,yml}\""
   }

script/deploy/Deploy.s.sol

@@ -3,25 +3,18 @@ pragma solidity >=0.8.18 <0.9.0;
 
 import { Script } from "forge-std/Script.sol";
 
-import { PRBProxyFactory } from "../../src/PRBProxyFactory.sol";
 import { PRBProxyHelpers } from "../../src/PRBProxyHelpers.sol";
 import { PRBProxyRegistry } from "../../src/PRBProxyRegistry.sol";
 
 import { BaseScript } from "../shared/Base.s.sol";
 
-/// @dev Deploys the {PRBProxyFactory} and the {PRBProxyRegistry} contracts at deterministic addresses across all
+/// @dev Deploys the {PRBProxyRegistry} and the {PRBProxyHelpers} contracts at deterministic addresses across all
 /// chains. Reverts if any of the two contracts has already been deployed.
 contract Deploy is Script, BaseScript {
     /// @dev The presence of the salt instructs Forge to deploy the contract via a deterministic CREATE2 factory.
     /// https://github.com/Arachnid/deterministic-deployment-proxy
-    function run()
-        public
-        virtual
-        broadcaster
-        returns (PRBProxyFactory factory, PRBProxyHelpers helpers, PRBProxyRegistry registry)
-    {
-        factory = new PRBProxyFactory{ salt: ZERO_SALT }();
+    function run() public virtual broadcaster returns (PRBProxyHelpers helpers, PRBProxyRegistry registry) {
+        registry = new PRBProxyRegistry{ salt: ZERO_SALT }();
         helpers = new PRBProxyHelpers{ salt: ZERO_SALT }();
-        registry = new PRBProxyRegistry{ salt: ZERO_SALT }(factory);
     }
 }

script/deploy/DeployHelpers.s.sol

@@ -7,7 +7,7 @@ import { PRBProxyHelpers } from "../../src/PRBProxyHelpers.sol";
 
 import { BaseScript } from "../shared/Base.s.sol";
 
-/// @notice Deploys the {DeployHelpers} contract at a deterministic address across all chains. Reverts if the contract
+/// @notice Deploys the {PRBProxyHelpers} contract at a deterministic address across all chains. Reverts if the contract
 /// has already been deployed.
 contract DeployHelpers is Script, BaseScript {
     /// @dev The presence of the salt instructs Forge to deploy the contract via a deterministic CREATE2 factory.

script/deploy/DeployProxy.s.sol

@@ -3,16 +3,15 @@ pragma solidity >=0.8.18 <0.9.0;
 
 import { Script } from "forge-std/Script.sol";
 
-import { PRBProxy } from "../../src/PRBProxy.sol";
+import { IPRBProxy } from "../../src/interfaces/IPRBProxy.sol";
+import { IPRBProxyRegistry } from "../../src/interfaces/IPRBProxyRegistry.sol";
+import { PRBProxyHelpers } from "../../src/PRBProxyHelpers.sol";
 
 import { BaseScript } from "../shared/Base.s.sol";
 
-/// @notice Deploys the {PRBProxy} contract at a deterministic address across all chains. Reverts if the contract
-/// has already been deployed.
+/// @notice Deploys the {PRBProxy} contract by calling the `deploy` function on the registry.
 contract DeployProxy is Script, BaseScript {
-    /// @dev The presence of the salt instructs Forge to deploy the contract via a deterministic CREATE2 factory.
-    /// https://github.com/Arachnid/deterministic-deployment-proxy
-    function run() public virtual broadcaster returns (PRBProxy proxy) {
-        proxy = new PRBProxy{ salt: ZERO_SALT }();
+    function run(IPRBProxyRegistry registry) public virtual broadcaster returns (IPRBProxy proxy) {
+        proxy = registry.deploy();
     }
 }

script/deploy/DeployRegistry.s.sol

@@ -3,17 +3,16 @@ pragma solidity >=0.8.18 <0.9.0;
 
 import { Script } from "forge-std/Script.sol";
 
-import { IPRBProxyFactory } from "../../src/interfaces/IPRBProxyFactory.sol";
 import { PRBProxyRegistry } from "../../src/PRBProxyRegistry.sol";
 
 import { BaseScript } from "../shared/Base.s.sol";
 
-/// @notice Deploys the {PRBProxyFactory} contract at a deterministic address across all chains. Reverts if the contract
-/// has already been deployed.
+/// @notice Deploys the {PRBProxyRegistry} contract at a deterministic address across all chains. Reverts if the
+/// contract has already been deployed.
 contract DeployRegistry is Script, BaseScript {
     /// @dev The presence of the salt instructs Forge to deploy the contract via a deterministic CREATE2 factory.
     /// https://github.com/Arachnid/deterministic-deployment-proxy
-    function run(IPRBProxyFactory factory) public virtual broadcaster returns (PRBProxyRegistry registry) {
-        registry = new PRBProxyRegistry{ salt: ZERO_SALT }(factory);
+    function run() public virtual broadcaster returns (PRBProxyRegistry registry) {
+        registry = new PRBProxyRegistry{ salt: ZERO_SALT }();
     }
 }

slither.config.json

@@ -1,4 +1,4 @@
 {
-  "detectors_to_exclude": "low-level-calls,reentrancy-events,solc-version",
+  "detectors_to_exclude": "low-level-calls,naming-convention,reentrancy-events,solc-version",
   "filter_paths": "(lib|test)"
 }

src/PRBProxy.sol

@@ -3,10 +3,22 @@ pragma solidity >=0.8.18;
 
 import { IPRBProxy } from "./interfaces/IPRBProxy.sol";
 import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
+import { IPRBProxyRegistry } from "./interfaces/IPRBProxyRegistry.sol";
 
 /// @title PRBProxy
 /// @dev This contract implements the {IPRBProxy} interface.
 contract PRBProxy is IPRBProxy {
+    /*//////////////////////////////////////////////////////////////////////////
+                                     CONSTANTS
+    //////////////////////////////////////////////////////////////////////////*/
+
+    /// @inheritdoc IPRBProxy
+    IPRBProxyRegistry public immutable override registry;
+
+    /*//////////////////////////////////////////////////////////////////////////
+                                   PUBLIC STORAGE
+    //////////////////////////////////////////////////////////////////////////*/
+
     /// @inheritdoc IPRBProxy
     address public override owner;
 
@@ -27,10 +39,12 @@ contract PRBProxy is IPRBProxy {
                                      CONSTRUCTOR
     //////////////////////////////////////////////////////////////////////////*/
 
+    /// @notice Constructs the proxy by fetching the constructor parameters from the registry.
+    /// @dev This is implemented like this to make it easy to precompute the CREATE2 address of the proxy.
     constructor() {
         minGasReserve = 5000;
-        owner = msg.sender;
-        emit TransferOwnership({ oldOwner: address(0), newOwner: msg.sender });
+        registry = IPRBProxyRegistry(msg.sender);
+        owner = IPRBProxyRegistry(msg.sender).transientProxyOwner();
     }
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -124,16 +138,13 @@ contract PRBProxy is IPRBProxy {
 
     /// @inheritdoc IPRBProxy
     function transferOwnership(address newOwner) external override {
-        // Check that the caller is the owner.
-        if (owner != msg.sender) {
-            revert PRBProxy_NotOwner({ owner: owner, caller: msg.sender });
+        // 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;
-
-        // Log the transfer of the owner.
-        emit TransferOwnership({ oldOwner: msg.sender, newOwner: newOwner });
     }
 
     /*//////////////////////////////////////////////////////////////////////////