SystemOwnerSafe as an owner

celo-org · Oct 10, 2024 · dda276b · dda276b
1 parent fb72612
commit dda276b
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 18 deletions.
diff --git a/bedrock-devnet/devnet/__init__.py b/bedrock-devnet/devnet/__init__.py
@@ -30,6 +30,7 @@
 DEVNET_ALTDA = os.getenv('DEVNET_ALTDA') == "true"
 GENERIC_ALTDA = os.getenv('GENERIC_ALTDA') == "true"
 DEVNET_CELO = os.getenv('DEVNET_CELO') == "true"
+SAFE_AS_OWNER = os.getenv('SAFE_AS_OWNER') == "true"
 
 class Bunch:
     def __init__(self, **kwds):
@@ -135,6 +136,8 @@ def init_devnet_l1_deploy_config(paths, update_timestamp=False):
         # Usage of the zero address in combination of the useCustomGasToken == True
         # will deploy a new contract
         deploy_config['customGasTokenAddress'] = "0x0000000000000000000000000000000000000000"
+    if SAFE_AS_OWNER:
+        deploy_config['safeAsOwner'] = True
     write_json(paths.devnet_config_path, deploy_config)
 
 def devnet_l1_allocs(paths):

diff --git a/op-chain-ops/genesis/config.go b/op-chain-ops/genesis/config.go
@@ -830,6 +830,9 @@ type DeployConfig struct {
 
 	// DeployCeloContracts indicates whether to deploy Celo contracts.
 	DeployCeloContracts bool `json:"deployCeloContracts"`
+
+	// SafeAsOwner indicates whether specific proxy contracts should be owned by the Safe contract.
+	SafeAsOwner bool `json:"safeAsOwner"`
 }
 
 // Copy will deeply copy the DeployConfig. This does a JSON roundtrip to copy

diff --git a/op-chain-ops/genesis/testdata/test-deploy-config-full.json b/op-chain-ops/genesis/testdata/test-deploy-config-full.json
@@ -94,5 +94,6 @@
   "daResolveWindow": 0,
   "daResolverRefundPercentage": 0,
   "deployCeloContracts": false,
-  "eip1559BaseFeeFloor": 5000000000
+  "eip1559BaseFeeFloor": 5000000000,
+  "safeAsOwner": false
 }
diff --git a/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol b/packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol
@@ -32,6 +32,7 @@ library ChainAssertions {
 
     /// @notice Asserts the correctness of an L1 deployment. This function expects that all contracts
     ///         within the `prox` ContractSet are proxies that have been setup and initialized.
+    /// this whole method is not being used - changes only for compilation sake
     function postDeployAssertions(
         Types.ContractSet memory _prox,
         DeployConfig _cfg,
@@ -46,7 +47,7 @@ library ChainAssertions {
         ResourceMetering.ResourceConfig memory dflt = Constants.DEFAULT_RESOURCE_CONFIG();
         require(keccak256(abi.encode(rcfg)) == keccak256(abi.encode(dflt)));
 
-        checkSystemConfig({ _contracts: _prox, _cfg: _cfg, _isProxy: true });
+        checkSystemConfig({ _contracts: _prox, _cfg: _cfg, _isProxy: true, expectedOwner: address(0) });
         checkL1CrossDomainMessenger({ _contracts: _prox, _vm: _vm, _isProxy: true });
         checkL1StandardBridge({ _contracts: _prox, _isProxy: true });
         checkL2OutputOracle({
@@ -59,11 +60,11 @@ library ChainAssertions {
         checkL1ERC721Bridge({ _contracts: _prox, _isProxy: true });
         checkOptimismPortal({ _contracts: _prox, _cfg: _cfg, _isProxy: true });
         checkOptimismPortal2({ _contracts: _prox, _cfg: _cfg, _isProxy: true });
-        checkProtocolVersions({ _contracts: _prox, _cfg: _cfg, _isProxy: true });
+        checkProtocolVersions({ _contracts: _prox, _cfg: _cfg, _isProxy: true, expectedOwner: address(0) });
     }
 
     /// @notice Asserts that the SystemConfig is setup correctly
-    function checkSystemConfig(Types.ContractSet memory _contracts, DeployConfig _cfg, bool _isProxy) internal view {
+    function checkSystemConfig(Types.ContractSet memory _contracts, DeployConfig _cfg, bool _isProxy, address expectedOwner) internal view {
         console.log("Running chain assertions on the SystemConfig");
         SystemConfig config = SystemConfig(_contracts.SystemConfig);
 
@@ -73,7 +74,7 @@ library ChainAssertions {
         ResourceMetering.ResourceConfig memory resourceConfig = config.resourceConfig();
 
         if (_isProxy) {
-            require(config.owner() == _cfg.finalSystemOwner());
+            require(config.owner() == expectedOwner);
             require(config.basefeeScalar() == _cfg.basefeeScalar());
             require(config.blobbasefeeScalar() == _cfg.blobbasefeeScalar());
             require(config.batcherHash() == bytes32(uint256(uint160(_cfg.batchSenderAddress()))));
@@ -419,7 +420,8 @@ library ChainAssertions {
     function checkProtocolVersions(
         Types.ContractSet memory _contracts,
         DeployConfig _cfg,
-        bool _isProxy
+        bool _isProxy,
+        address expectedOwner
     )
         internal
         view
@@ -431,7 +433,7 @@ library ChainAssertions {
         assertSlotValueIsOne({ _contractAddress: address(versions), _slot: 0, _offset: 0 });
 
         if (_isProxy) {
-            require(versions.owner() == _cfg.finalSystemOwner());
+            require(versions.owner() == expectedOwner);
             require(ProtocolVersion.unwrap(versions.required()) == _cfg.requiredProtocolVersion());
             require(ProtocolVersion.unwrap(versions.recommended()) == _cfg.recommendedProtocolVersion());
         } else {

diff --git a/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol b/packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
@@ -260,15 +260,15 @@ contract Deploy is Deployer {
         }
     }
 
-    /// @notice Transfer ownership of a Proxy to the ProxyAdmin contract
+    /// @notice Transfer admin role of a Proxy to the ProxyAdmin contract
     ///         This is expected to be used in conjusting with deployERC1967ProxyWithOwner after setup actions
     ///         have been performed on the proxy.
     /// @param _name The name of the proxy to transfer ownership of.
     function transferProxyToProxyAdmin(string memory _name) public broadcast {
         Proxy proxy = Proxy(mustGetAddress(_name));
         address proxyAdmin = mustGetAddress("ProxyAdmin");
         proxy.changeAdmin(proxyAdmin);
-        console.log("Proxy %s ownership transferred to ProxyAdmin at: %s", _name, proxyAdmin);
+        console.log("Proxy %s admin role transferred to ProxyAdmin at: %s", _name, proxyAdmin);
     }
 
     ////////////////////////////////////////////////////////////////
@@ -803,7 +803,7 @@ contract Deploy is Deployer {
         // are always proxies.
         Types.ContractSet memory contracts = _proxiesUnstrict();
         contracts.ProtocolVersions = address(versions);
-        ChainAssertions.checkProtocolVersions({ _contracts: contracts, _cfg: cfg, _isProxy: false });
+        ChainAssertions.checkProtocolVersions({ _contracts: contracts, _cfg: cfg, _isProxy: false, expectedOwner: address(0) });
 
         addr_ = address(versions);
     }
@@ -858,7 +858,7 @@ contract Deploy is Deployer {
         // are always proxies.
         Types.ContractSet memory contracts = _proxiesUnstrict();
         contracts.SystemConfig = addr_;
-        ChainAssertions.checkSystemConfig({ _contracts: contracts, _cfg: cfg, _isProxy: false });
+        ChainAssertions.checkSystemConfig({ _contracts: contracts, _cfg: cfg, _isProxy: false, expectedOwner: address(0) });
     }
 
     /// @notice Deploy the L1StandardBridge
@@ -1070,13 +1070,19 @@ contract Deploy is Deployer {
             customGasTokenAddress = cfg.customGasTokenAddress();
         }
 
+        address systemConfigOwner = cfg.finalSystemOwner();
+        if (cfg.safeAsOwner()) {
+            console.log("Setting SystemConfig owner to SystemOwnerSafe");
+            systemConfigOwner = mustGetAddress("SystemOwnerSafe");
+        }
+
         _upgradeAndCallViaSafe({
             _proxy: payable(systemConfigProxy),
             _implementation: systemConfig,
             _innerCallData: abi.encodeCall(
                 SystemConfig.initialize,
                 (
-                    cfg.finalSystemOwner(),
+                    systemConfigOwner,
                     cfg.basefeeScalar(),
                     cfg.blobbasefeeScalar(),
                     batcherHash,
@@ -1101,7 +1107,7 @@ contract Deploy is Deployer {
         string memory version = config.version();
         console.log("SystemConfig version: %s", version);
 
-        ChainAssertions.checkSystemConfig({ _contracts: _proxies(), _cfg: cfg, _isProxy: true });
+        ChainAssertions.checkSystemConfig({ _contracts: _proxies(), _cfg: cfg, _isProxy: true, expectedOwner: systemConfigOwner });
     }
 
     /// @notice Initialize the L1StandardBridge
@@ -1361,13 +1367,19 @@ contract Deploy is Deployer {
         uint256 requiredProtocolVersion = cfg.requiredProtocolVersion();
         uint256 recommendedProtocolVersion = cfg.recommendedProtocolVersion();
 
+        address protocolVersionsOwner = cfg.finalSystemOwner();
+        if (cfg.safeAsOwner()) {
+            console.log("Setting ProtocolVersions owner to SystemOwnerSafe");
+            protocolVersionsOwner = mustGetAddress("SystemOwnerSafe");
+        }
+
         _upgradeAndCallViaSafe({
             _proxy: payable(protocolVersionsProxy),
             _implementation: protocolVersions,
             _innerCallData: abi.encodeCall(
                 ProtocolVersions.initialize,
                 (
-                    finalSystemOwner,
+                    protocolVersionsOwner,
                     ProtocolVersion.wrap(requiredProtocolVersion),
                     ProtocolVersion.wrap(recommendedProtocolVersion)
                 )
@@ -1378,7 +1390,7 @@ contract Deploy is Deployer {
         string memory version = versions.version();
         console.log("ProtocolVersions version: %s", version);
 
-        ChainAssertions.checkProtocolVersions({ _contracts: _proxiesUnstrict(), _cfg: cfg, _isProxy: true });
+        ChainAssertions.checkProtocolVersions({ _contracts: _proxiesUnstrict(), _cfg: cfg, _isProxy: true, expectedOwner: protocolVersionsOwner });
     }
 
     /// @notice Transfer ownership of the DisputeGameFactory contract to the final system owner
@@ -1629,20 +1641,26 @@ contract Deploy is Deployer {
         uint256 daBondSize = cfg.daBondSize();
         uint256 daResolverRefundPercentage = cfg.daResolverRefundPercentage();
 
+        address dataAvailabilityChallengeOwner = cfg.finalSystemOwner();
+        if (cfg.safeAsOwner()) {
+            console.log("Setting DataAvailabilityChallenge owner to SystemOwnerSafe");
+            dataAvailabilityChallengeOwner = mustGetAddress("SystemOwnerSafe");
+        }
+
         _upgradeAndCallViaSafe({
             _proxy: payable(dataAvailabilityChallengeProxy),
             _implementation: dataAvailabilityChallenge,
             _innerCallData: abi.encodeCall(
                 DataAvailabilityChallenge.initialize,
-                (finalSystemOwner, daChallengeWindow, daResolveWindow, daBondSize, daResolverRefundPercentage)
+                (dataAvailabilityChallengeOwner, daChallengeWindow, daResolveWindow, daBondSize, daResolverRefundPercentage)
             )
         });
 
         DataAvailabilityChallenge dac = DataAvailabilityChallenge(payable(dataAvailabilityChallengeProxy));
         string memory version = dac.version();
         console.log("DataAvailabilityChallenge version: %s", version);
 
-        require(dac.owner() == finalSystemOwner);
+        require(dac.owner() == dataAvailabilityChallengeOwner);
         require(dac.challengeWindow() == daChallengeWindow);
         require(dac.resolveWindow() == daResolveWindow);
         require(dac.bondSize() == daBondSize);

diff --git a/packages/contracts-bedrock/scripts/deploy/DeployConfig.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployConfig.s.sol
@@ -93,6 +93,7 @@ contract DeployConfig is Script {
     bool public useInterop;
 
     bool public deployCeloContracts;
+    bool public safeAsOwner;
 
     function read(string memory _path) public {
         console.log("DeployConfig: reading file %s", _path);
@@ -181,6 +182,7 @@ contract DeployConfig is Script {
 
         // Celo specific config
         deployCeloContracts = _readOr(_json, "$.deployCeloContracts", false);
+        safeAsOwner = _readOr(_json, "$.safeAsOwner", false);
     }
 
     function fork() public view returns (Fork fork_) {

diff --git a/packages/contracts-bedrock/scripts/getting-started/config-vars-celo.sh b/packages/contracts-bedrock/scripts/getting-started/config-vars-celo.sh
@@ -43,6 +43,7 @@ reqenv "USE_ALTDA"
 reqenv "DEPLOY_CELO_CONTRACTS"
 reqenv "USE_CUSTOM_GAS_TOKEN"
 reqenv "CUSTOM_GAS_TOKEN_ADDRESS"
+reqenv "SAFE_AS_OWNER"
 
 # Get the finalized block timestamp and hash
 block=$(cast block finalized --rpc-url "$L1_RPC_URL")
@@ -97,6 +98,7 @@ cat << EOL > tmp_config.json
   "gasPriceOracleScalar": 1000000,
 
   "deployCeloContracts": $DEPLOY_CELO_CONTRACTS,
+  "safeAsOwner": $SAFE_AS_OWNER,
 
   "enableGovernance": $ENABLE_GOVERNANCE,
   "governanceTokenSymbol": "OP",
@@ -166,7 +168,8 @@ cat << EOL >> tmp_config.json
   "daResolveWindow": 1,
 
   "useCustomGasToken": $USE_CUSTOM_GAS_TOKEN,
-  "customGasTokenAddress": "$CUSTOM_GAS_TOKEN_ADDRESS"
+  "customGasTokenAddress": "$CUSTOM_GAS_TOKEN_ADDRESS",
+  "safeAsOwner": $SAFE_AS_OWNER
 }
 EOL