Add clarity on how OPCM deals with salts (#13273)

* fix: clarity around how salts are made.

* fix: removed named return value.

* fix: semver lock updated.

* fix: pr comments.

* fix: adding natpsec to the new salt helper functions.

fix: adding natpsec to the new salt helper functions.

* fix: more details around why proxy salts include contract name.

* Update packages/contracts-bedrock/src/L1/OPContractsManager.sol
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

* Update packages/contracts-bedrock/src/L1/OPContractsManager.sol
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

* fix: only having one compute salt function

* fix: pre-pr ran

* Update packages/contracts-bedrock/src/L1/OPContractsManager.sol
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

* fix: fix pre-pr

---------
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>

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

@@ -20,8 +20,8 @@
     "sourceCodeHash": "0x8aafeffb41332fddf2fb1ef4fc033bd1f323cdc5b199c6951da73e3cb86276e6"
   },
   "src/L1/OPContractsManager.sol": {
-    "initCodeHash": "0x1eb781ca3f3609dbf13ecb9fe34063155871510b148ac63348a4947858c196ba",
-    "sourceCodeHash": "0xf1e9fe3f37414e1f1824ce18cd6164c33ca64527b419d6fa52690b6351534822"
+    "initCodeHash": "0x73dbd0af5c85a20a147a87e795526c2da31e56157d11cfc8db5445f4bd1fa13f",
+    "sourceCodeHash": "0x52b2ea7508d89e51412de333950da8f5a504a214590bcb67566151c5240eaad1"
   },
   "src/L1/OptimismPortal.sol": {
     "initCodeHash": "0xa8b2f8a6d1092c5e64529736462ebb35daa9ea9e67585f7de8e3e5394682ee64",

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

@@ -114,8 +114,8 @@ contract OPContractsManager is ISemver {
 
     // -------- Constants and Variables --------
 
-    /// @custom:semver 1.0.0-beta.24
-    string public constant version = "1.0.0-beta.24";
+    /// @custom:semver 1.0.0-beta.25
+    string public constant version = "1.0.0-beta.25";
 
     /// @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.
@@ -195,9 +195,7 @@ contract OPContractsManager is ISemver {
     function deploy(DeployInput calldata _input) external returns (DeployOutput memory) {
         assertValidInputs(_input);
         uint256 l2ChainId = _input.l2ChainId;
-        // The salt for a non-proxy contract is a function of the chain ID and the salt mixer.
         string memory saltMixer = _input.saltMixer;
-        bytes32 salt = keccak256(abi.encode(l2ChainId, saltMixer));
         DeployOutput memory output;
 
         // -------- Deploy Chain Singletons --------
@@ -206,9 +204,16 @@ contract OPContractsManager is ISemver {
         // this contract, and then transfer ownership to the specified owner at the end of deployment.
         // The AddressManager is used to store the implementation for the L1CrossDomainMessenger
         // due to it's usage of the legacy ResolvedDelegateProxy.
-        output.addressManager = IAddressManager(Blueprint.deployFrom(blueprint.addressManager, salt));
-        output.opChainProxyAdmin =
-            IProxyAdmin(Blueprint.deployFrom(blueprint.proxyAdmin, salt, abi.encode(address(this))));
+        output.addressManager = IAddressManager(
+            Blueprint.deployFrom(
+                blueprint.addressManager, computeSalt(l2ChainId, saltMixer, "AddressManager"), abi.encode()
+            )
+        );
+        output.opChainProxyAdmin = IProxyAdmin(
+            Blueprint.deployFrom(
+                blueprint.proxyAdmin, computeSalt(l2ChainId, saltMixer, "ProxyAdmin"), abi.encode(address(this))
+            )
+        );
         output.opChainProxyAdmin.setAddressManager(output.addressManager);
 
         // -------- Deploy Proxy Contracts --------
@@ -230,12 +235,22 @@ contract OPContractsManager is ISemver {
 
         // Deploy legacy proxied contracts.
         output.l1StandardBridgeProxy = IL1StandardBridge(
-            payable(Blueprint.deployFrom(blueprint.l1ChugSplashProxy, salt, abi.encode(output.opChainProxyAdmin)))
+            payable(
+                Blueprint.deployFrom(
+                    blueprint.l1ChugSplashProxy,
+                    computeSalt(l2ChainId, saltMixer, "L1StandardBridge"),
+                    abi.encode(output.opChainProxyAdmin)
+                )
+            )
         );
         output.opChainProxyAdmin.setProxyType(address(output.l1StandardBridgeProxy), IProxyAdmin.ProxyType.CHUGSPLASH);
         string memory contractName = "OVM_L1CrossDomainMessenger";
         output.l1CrossDomainMessengerProxy = IL1CrossDomainMessenger(
-            Blueprint.deployFrom(blueprint.resolvedDelegateProxy, salt, abi.encode(output.addressManager, contractName))
+            Blueprint.deployFrom(
+                blueprint.resolvedDelegateProxy,
+                computeSalt(l2ChainId, saltMixer, "L1CrossDomainMessenger"),
+                abi.encode(output.addressManager, contractName)
+            )
         );
         output.opChainProxyAdmin.setProxyType(
             address(output.l1CrossDomainMessengerProxy), IProxyAdmin.ProxyType.RESOLVED
@@ -246,7 +261,11 @@ contract OPContractsManager is ISemver {
         // The AnchorStateRegistry Implementation is not MCP Ready, and therefore requires an implementation per chain.
         // It must be deployed after the DisputeGameFactoryProxy so that it can be provided as a constructor argument.
         output.anchorStateRegistryImpl = IAnchorStateRegistry(
-            Blueprint.deployFrom(blueprint.anchorStateRegistry, salt, abi.encode(output.disputeGameFactoryProxy))
+            Blueprint.deployFrom(
+                blueprint.anchorStateRegistry,
+                computeSalt(l2ChainId, saltMixer, "AnchorStateRegistry"),
+                abi.encode(output.disputeGameFactoryProxy)
+            )
         );
 
         // Eventually we will switch from DelayedWETHPermissionedGameProxy to DelayedWETHPermissionlessGameProxy.
@@ -259,7 +278,7 @@ contract OPContractsManager is ISemver {
             Blueprint.deployFrom(
                 blueprint.permissionedDisputeGame1,
                 blueprint.permissionedDisputeGame2,
-                salt,
+                computeSalt(l2ChainId, saltMixer, "PermissionedDisputeGame"),
                 encodePermissionedDisputeGameConstructor(_input, output)
             )
         );
@@ -372,6 +391,22 @@ contract OPContractsManager is ISemver {
         return address(uint160(bytes20(bytes.concat(versionByte, first19Bytes))));
     }
 
+    /// @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
+    function computeSalt(
+        uint256 _l2ChainId,
+        string memory _saltMixer,
+        string memory _contractName
+    )
+        internal
+        pure
+        returns (bytes32)
+    {
+        return keccak256(abi.encode(_l2ChainId, _saltMixer, _contractName));
+    }
+
     /// @notice Deterministically deploys a new proxy contract owned by the provided ProxyAdmin.
     /// The salt is computed as a function of the L2 chain ID, the salt mixer and the contract name.
     /// This is required because we deploy many identical proxies, so they each require a unique salt for determinism.
@@ -384,7 +419,7 @@ contract OPContractsManager is ISemver {
         internal
         returns (address)
     {
-        bytes32 salt = keccak256(abi.encode(_l2ChainId, _saltMixer, _contractName));
+        bytes32 salt = computeSalt(_l2ChainId, _saltMixer, _contractName);
         return Blueprint.deployFrom(blueprint.proxy, salt, abi.encode(_proxyAdmin));
     }