feat: Use encodeCall for type safety (#13561)

* feat: Use encodeCall for type safety

A very common issue (both from experience and observation) is
failing to correctly encode the initializer arguments in
`encodeSystemConfigInitializer()`. This fixes that.

I believe that at one point we needed to be able to pass the 4 bytes
selector in as a string, but that no longer seems to be the case.

* semver

* feat: expand use of encodeCall to rest of OPCM

* semver lock

packages/contracts-bedrock/snapshots/semver-lock.json

@@ -16,8 +16,8 @@
     "sourceCodeHash": "0xa91b445bdc666a02ba18e3b91ba94b6d54bbe65da714002fc734814201319d57"
   },
   "src/L1/OPContractsManager.sol": {
-    "initCodeHash": "0x9b704574a7005dc2aa8d6a3e0d85572493cc4bbd60033a23e437632a5fef7720",
-    "sourceCodeHash": "0x05ed7ad68e4e9bca7334314e794a1f66e5899532bb01cfa3a7716cb2688df9d5"
+    "initCodeHash": "0x4b413cbe79bd10d41d8f3e9f0408e773dd49ced823d457b9f9aa92f446828105",
+    "sourceCodeHash": "0xe5179a20ae40d4e4773c52df98bac67e73e04044bec9e8750073b4e2f14fe81b"
   },
   "src/L1/OptimismPortal2.sol": {
     "initCodeHash": "0xfd14fd690752519064d6de6c3e15d69ec9146bc8714e56ac286305773dbb1533",

packages/contracts-bedrock/src/L1/OPContractsManager.sol

@@ -114,8 +114,8 @@ contract OPContractsManager is ISemver {
 
     // -------- Constants and Variables --------
 
-    /// @custom:semver 1.0.0-beta.26
-    string public constant version = "1.0.0-beta.26";
+    /// @custom:semver 1.0.0-beta.27
+    string public constant version = "1.0.0-beta.27";
 
     /// @notice Represents the interface version so consumers know how to decode the DeployOutput struct
     /// that's emitted in the `Deployed` event. Whenever that struct changes, a new version should be used.
@@ -286,12 +286,12 @@ contract OPContractsManager is ISemver {
         // -------- Set and Initialize Proxy Implementations --------
         bytes memory data;
 
-        data = encodeL1ERC721BridgeInitializer(IL1ERC721Bridge.initialize.selector, output);
+        data = encodeL1ERC721BridgeInitializer(output);
         upgradeAndCall(
             output.opChainProxyAdmin, address(output.l1ERC721BridgeProxy), implementation.l1ERC721BridgeImpl, data
         );
 
-        data = encodeOptimismPortalInitializer(IOptimismPortal2.initialize.selector, output);
+        data = encodeOptimismPortalInitializer(output);
         upgradeAndCall(
             output.opChainProxyAdmin, address(output.optimismPortalProxy), implementation.optimismPortalImpl, data
         );
@@ -304,7 +304,7 @@ contract OPContractsManager is ISemver {
             output.opChainProxyAdmin, address(output.systemConfigProxy), implementation.systemConfigImpl, data
         );
 
-        data = encodeOptimismMintableERC20FactoryInitializer(IOptimismMintableERC20Factory.initialize.selector, output);
+        data = encodeOptimismMintableERC20FactoryInitializer(output);
         upgradeAndCall(
             output.opChainProxyAdmin,
             address(output.optimismMintableERC20FactoryProxy),
@@ -312,7 +312,7 @@ contract OPContractsManager is ISemver {
             data
         );
 
-        data = encodeL1CrossDomainMessengerInitializer(IL1CrossDomainMessenger.initialize.selector, output);
+        data = encodeL1CrossDomainMessengerInitializer(output);
         upgradeAndCall(
             output.opChainProxyAdmin,
             address(output.l1CrossDomainMessengerProxy),
@@ -320,12 +320,12 @@ contract OPContractsManager is ISemver {
             data
         );
 
-        data = encodeL1StandardBridgeInitializer(IL1StandardBridge.initialize.selector, output);
+        data = encodeL1StandardBridgeInitializer(output);
         upgradeAndCall(
             output.opChainProxyAdmin, address(output.l1StandardBridgeProxy), implementation.l1StandardBridgeImpl, data
         );
 
-        data = encodeDelayedWETHInitializer(IDelayedWETH.initialize.selector, _input);
+        data = encodeDelayedWETHInitializer(_input);
         // Eventually we will switch from DelayedWETHPermissionedGameProxy to DelayedWETHPermissionlessGameProxy.
         upgradeAndCall(
             output.opChainProxyAdmin,
@@ -335,7 +335,7 @@ contract OPContractsManager is ISemver {
         );
 
         // We set the initial owner to this contract, set game implementations, then transfer ownership.
-        data = encodeDisputeGameFactoryInitializer(IDisputeGameFactory.initialize.selector, _input);
+        data = encodeDisputeGameFactoryInitializer();
         upgradeAndCall(
             output.opChainProxyAdmin,
             address(output.disputeGameFactoryProxy),
@@ -347,7 +347,7 @@ contract OPContractsManager is ISemver {
         );
         output.disputeGameFactoryProxy.transferOwnership(address(_input.roles.opChainProxyAdminOwner));
 
-        data = encodeAnchorStateRegistryInitializer(IAnchorStateRegistry.initialize.selector, _input);
+        data = encodeAnchorStateRegistryInitializer(_input);
         upgradeAndCall(
             output.opChainProxyAdmin,
             address(output.anchorStateRegistryProxy),
@@ -394,7 +394,7 @@ contract OPContractsManager is ISemver {
     /// @notice Helper method for computing a salt that's used in CREATE2 deployments.
     /// Including the contract name ensures that the resultant address from CREATE2 is unique
     /// across our smart contract system. For example, we deploy multiple proxy contracts
-    /// with the same bytecode from this contract, so they need different salts to avoid an address collision
+    /// with the same bytecode from this contract, so they each require a unique salt for determinism.
     function computeSalt(
         uint256 _l2ChainId,
         string memory _saltMixer,
@@ -426,34 +426,30 @@ contract OPContractsManager is ISemver {
     // -------- Initializer Encoding --------
 
     /// @notice Helper method for encoding the L1ERC721Bridge initializer data.
-    function encodeL1ERC721BridgeInitializer(
-        bytes4 _selector,
-        DeployOutput memory _output
-    )
+    function encodeL1ERC721BridgeInitializer(DeployOutput memory _output)
         internal
         view
         virtual
         returns (bytes memory)
     {
-        return abi.encodeWithSelector(_selector, _output.l1CrossDomainMessengerProxy, superchainConfig);
+        return abi.encodeCall(IL1ERC721Bridge.initialize, (_output.l1CrossDomainMessengerProxy, superchainConfig));
     }
 
     /// @notice Helper method for encoding the OptimismPortal initializer data.
-    function encodeOptimismPortalInitializer(
-        bytes4 _selector,
-        DeployOutput memory _output
-    )
+    function encodeOptimismPortalInitializer(DeployOutput memory _output)
         internal
         view
         virtual
         returns (bytes memory)
     {
-        return abi.encodeWithSelector(
-            _selector,
+        return abi.encodeCall(
+            IOptimismPortal2.initialize,
+            (
                 _output.disputeGameFactoryProxy,
                 _output.systemConfigProxy,
                 superchainConfig,
                 GameTypes.PERMISSIONED_CANNON
+            )
         );
     }
 
@@ -467,12 +463,12 @@ contract OPContractsManager is ISemver {
         virtual
         returns (bytes memory)
     {
-        bytes4 selector = ISystemConfig.initialize.selector;
         (IResourceMetering.ResourceConfig memory referenceResourceConfig, ISystemConfig.Addresses memory opChainAddrs) =
-            defaultSystemConfigParams(selector, _input, _output);
+            defaultSystemConfigParams(_input, _output);
 
-        return abi.encodeWithSelector(
-            selector,
+        return abi.encodeCall(
+            ISystemConfig.initialize,
+            (
                 _input.roles.systemConfigOwner,
                 _input.basefeeScalar,
                 _input.blobBasefeeScalar,
@@ -482,69 +478,53 @@ contract OPContractsManager is ISemver {
                 referenceResourceConfig,
                 chainIdToBatchInboxAddress(_input.l2ChainId),
                 opChainAddrs
+            )
         );
     }
 
     /// @notice Helper method for encoding the OptimismMintableERC20Factory initializer data.
-    function encodeOptimismMintableERC20FactoryInitializer(
-        bytes4 _selector,
-        DeployOutput memory _output
-    )
+    function encodeOptimismMintableERC20FactoryInitializer(DeployOutput memory _output)
         internal
         pure
         virtual
         returns (bytes memory)
     {
-        return abi.encodeWithSelector(_selector, _output.l1StandardBridgeProxy);
+        return abi.encodeCall(IOptimismMintableERC20Factory.initialize, (address(_output.l1StandardBridgeProxy)));
     }
 
     /// @notice Helper method for encoding the L1CrossDomainMessenger initializer data.
-    function encodeL1CrossDomainMessengerInitializer(
-        bytes4 _selector,
-        DeployOutput memory _output
-    )
+    function encodeL1CrossDomainMessengerInitializer(DeployOutput memory _output)
         internal
         view
         virtual
         returns (bytes memory)
     {
-        return
-            abi.encodeWithSelector(_selector, superchainConfig, _output.optimismPortalProxy, _output.systemConfigProxy);
+        return abi.encodeCall(
+            IL1CrossDomainMessenger.initialize,
+            (superchainConfig, _output.optimismPortalProxy, _output.systemConfigProxy)
+        );
     }
 
     /// @notice Helper method for encoding the L1StandardBridge initializer data.
-    function encodeL1StandardBridgeInitializer(
-        bytes4 _selector,
-        DeployOutput memory _output
-    )
+    function encodeL1StandardBridgeInitializer(DeployOutput memory _output)
         internal
         view
         virtual
         returns (bytes memory)
     {
-        return abi.encodeWithSelector(
-            _selector, _output.l1CrossDomainMessengerProxy, superchainConfig, _output.systemConfigProxy
+        return abi.encodeCall(
+            IL1StandardBridge.initialize,
+            (_output.l1CrossDomainMessengerProxy, superchainConfig, _output.systemConfigProxy)
         );
     }
 
-    function encodeDisputeGameFactoryInitializer(
-        bytes4 _selector,
-        DeployInput memory
-    )
-        internal
-        view
-        virtual
-        returns (bytes memory)
-    {
+    function encodeDisputeGameFactoryInitializer() internal view virtual returns (bytes memory) {
         // This contract must be the initial owner so we can set game implementations, then
         // ownership is transferred after.
-        return abi.encodeWithSelector(_selector, address(this));
+        return abi.encodeCall(IDisputeGameFactory.initialize, (address(this)));
     }
 
-    function encodeAnchorStateRegistryInitializer(
-        bytes4 _selector,
-        DeployInput memory _input
-    )
+    function encodeAnchorStateRegistryInitializer(DeployInput memory _input)
         internal
         view
         virtual
@@ -552,19 +532,11 @@ contract OPContractsManager is ISemver {
     {
         IAnchorStateRegistry.StartingAnchorRoot[] memory startingAnchorRoots =
             abi.decode(_input.startingAnchorRoots, (IAnchorStateRegistry.StartingAnchorRoot[]));
-        return abi.encodeWithSelector(_selector, startingAnchorRoots, superchainConfig);
+        return abi.encodeCall(IAnchorStateRegistry.initialize, (startingAnchorRoots, superchainConfig));
     }
 
-    function encodeDelayedWETHInitializer(
-        bytes4 _selector,
-        DeployInput memory _input
-    )
-        internal
-        view
-        virtual
-        returns (bytes memory)
-    {
-        return abi.encodeWithSelector(_selector, _input.roles.opChainProxyAdminOwner, superchainConfig);
+    function encodeDelayedWETHInitializer(DeployInput memory _input) internal view virtual returns (bytes memory) {
+        return abi.encodeCall(IDelayedWETH.initialize, (_input.roles.opChainProxyAdminOwner, superchainConfig));
     }
 
     function encodePermissionedDisputeGameConstructor(
@@ -595,7 +567,6 @@ contract OPContractsManager is ISemver {
     /// @notice Returns default, standard config arguments for the SystemConfig initializer.
     /// This is used by subclasses to reduce code duplication.
     function defaultSystemConfigParams(
-        bytes4, /* selector */
         DeployInput memory, /* _input */
         DeployOutput memory _output
     )

packages/contracts-bedrock/src/L1/OPContractsManagerInterop.sol

@@ -34,9 +34,8 @@ contract OPContractsManagerInterop is OPContractsManager {
         override
         returns (bytes memory)
     {
-        bytes4 selector = ISystemConfigInterop.initialize.selector;
         (IResourceMetering.ResourceConfig memory referenceResourceConfig, ISystemConfig.Addresses memory opChainAddrs) =
-            defaultSystemConfigParams(selector, _input, _output);
+            defaultSystemConfigParams(_input, _output);
 
         // TODO For now we assume that the dependency manager is the same as system config owner.
         // This is currently undefined since it's not part of the standard config, so we may need
@@ -45,8 +44,9 @@ contract OPContractsManagerInterop is OPContractsManager {
         // we will make the change described in https://github.com/ethereum-optimism/optimism/issues/11783.
         address dependencyManager = address(_input.roles.systemConfigOwner);
 
-        return abi.encodeWithSelector(
-            selector,
+        return abi.encodeCall(
+            ISystemConfigInterop.initialize,
+            (
                 _input.roles.systemConfigOwner,
                 _input.basefeeScalar,
                 _input.blobBasefeeScalar,
@@ -57,6 +57,7 @@ contract OPContractsManagerInterop is OPContractsManager {
                 chainIdToBatchInboxAddress(_input.l2ChainId),
                 opChainAddrs,
                 dependencyManager
+            )
         );
     }
 }