feat(contracts): various deployment improvements (#321)

* feat(contracts): support reusing existing deployed implementation (savings $80) * feat(contracts): support proxy managed via ProxyAdmin (default op configuration) * feat(contracts): optionally use different key for deployment * chore: improve * add * wip * add * add * fix * add --------- Co-authored-by: Ratan Kaliani <[email protected]>
succinctlabs · Jan 17, 2025 · dc6bbb5 · dc6bbb5
1 parent 2817295
commit dc6bbb5
Show file tree

Hide file tree

Showing 10 changed files with 159 additions and 48 deletions.
diff --git a/book/contracts/configuration.md b/book/contracts/configuration.md
@@ -27,4 +27,9 @@ You can configure additional parameters when deploying or upgrading the `OPSucci
 | `FINALIZATION_PERIOD_SECS` | Default: `3600` (1 hour). The time period (in seconds) after which a proposed output becomes finalized and withdrawals can be processed. |
 | `PROPOSER` | Default: The address of the account associated with `PRIVATE_KEY`. If `PRIVATE_KEY` is not set, `address(0)`. An Ethereum address authorized to submit proofs. Set to `address(0)` to allow permissionless submissions. **Note: Use `addProposer` and `removeProposer` functions to update the list of approved proposers.** |
 | `CHALLENGER` | Default: The address of the account associated with `PRIVATE_KEY`. If `PRIVATE_KEY` is not set, `address(0)`. Ethereum address authorized to dispute proofs. Set to `address(0)` for no challenging. |
-| `OWNER` | Default: The address of the account associated with `PRIVATE_KEY`. If `PRIVATE_KEY` is not set, `address(0)`. Ethereum address authorized to update the `aggregationVkey`, `rangeVkeyCommitment`, `verifier`, and `rollupConfigHash` parameters. Can also transfer ownership of the contract and update the approved proposers. In a production setting, set to the governance smart contract or multi-sig of the chain. |
+| `OWNER` | Default: The address of the account associated with `PRIVATE_KEY`. If `PRIVATE_KEY` is not set, `address(0)`. Ethereum address authorized to update the `aggregationVkey`, `rangeVkeyCommitment`, `verifier`, and `rollupConfigHash` parameters. Can also transfer ownership of the contract and |update the approved proposers. In a production setting, set to the governance smart contract or multi-sig of the chain. |
+| `DEPLOY_PK` | Default: The address of the account associated with `PRIVATE_KEY`. The private key of the account that will be deploying the contract. |
+| `ADMIN_PK` | Default: The address of the account associated with `PRIVATE_KEY`. The private key of the account that will be deploying the contract. |
+| `PROXY_ADMIN` | Default: `address(0)`. The address of the L1 `ProxyAdmin` contract used to manage the `OPSuccinctL2OutputOracle` proxy. More information can be found [here](https://docs.optimism.io/chain/security/privileged-roles#l1-proxy-admin). |
+| `OP_SUCCINCT_L2_OUTPUT_ORACLE_IMPL` | Default: `address(0)`. The address of an already-deployed `OPSuccinctL2OutputOracle` implementation contract. If this is not set, a new `OPSuccinctL2OutputOracle` implementation contract will be deployed. |
+
diff --git a/contracts/opsuccinctl2ooconfig-test.json b/contracts/opsuccinctl2ooconfig-test.json
@@ -2,6 +2,7 @@
   "challenger": "0x0000000000000000000000000000000000000000",
   "finalizationPeriod": 3600,
   "l2BlockTime": 2,
+  "opSuccinctL2OutputOracleImpl": "0x0000000000000000000000000000000000000000",
   "owner": "0xDEd0000E32f8F40414d3ab3a830f735a3553E18e",
   "proposer": "0x0000000000000000000000000000000000000000",
   "rollupConfigHash": "0x0d7101e2acc7eae1fb42cfce5c604d14da561726e4e01b509315e5a9f97a9816",
@@ -11,5 +12,6 @@
   "submissionInterval": 1200,
   "verifier": "0x397A5f7f3dBd538f23DE225B51f532c34448dA9B",  
   "aggregationVkey": "0x00d4e72bc998d0528b0722a53bedd9c6f0143c9157af194ad4bb2502e37a496f",
-  "rangeVkeyCommitment": "0x33e3678015df481724af3aac49d000923caeec277027610b1490f857769f9459"
+  "rangeVkeyCommitment": "0x33e3678015df481724af3aac49d000923caeec277027610b1490f857769f9459",
+  "proxyAdmin": "0x0000000000000000000000000000000000000000"
 }
diff --git a/contracts/opsuccinctl2ooconfig.json b/contracts/opsuccinctl2ooconfig.json
@@ -2,14 +2,16 @@
   "challenger": "0x788c45CafaB3ea427b9079889BE43D7d3cd7500C",
   "finalizationPeriod": 3600,
   "l2BlockTime": 2,
+  "opSuccinctL2OutputOracleImpl": "0xe2314c6d41f0b73d8998e3bd3174c3b149c394d5",
   "owner": "0x788c45CafaB3ea427b9079889BE43D7d3cd7500C",
   "proposer": "0x788c45CafaB3ea427b9079889BE43D7d3cd7500C",
   "rollupConfigHash": "0x78e77d3560e6cfb84e49b62ce66ccce1a8c347b1f3e7c6353a590dcb373a6a4e",
-  "startingBlockNumber": 8740000,
-  "startingOutputRoot": "0x7046a9509eb163d33a906daf1564c33854432ea4fdd37371623df75878c71dbe",
-  "startingTimestamp": 1736815639,
-  "submissionInterval": 2500,
+  "startingBlockNumber": 8909351,
+  "startingOutputRoot": "0x6c8fc616e3d706041663887f26d1f7cf1ae4ee2930f32dce37b14920cb1ebfda",
+  "startingTimestamp": 1737154341,
+  "submissionInterval": 3600,
   "verifier": "0x397A5f7f3dBd538f23DE225B51f532c34448dA9B",
-  "aggregationVkey": "0x002a9b63d01f9e55e791671ff8c42008da75b0804983f2aeb501d5652ea4b03b",
-  "rangeVkeyCommitment": "0x17f2876a475131400204ac2d0a4b046d4a116d2f1b38acfd74b39acf1b27b97b"
+  "aggregationVkey": "0x00613438cec0bce6dba4091a5e2d831dd9531605f0b85fe2762c664b35288c1a",
+  "rangeVkeyCommitment": "0x5f1667fd22a4ac5c016edf3f583e20c6250d9eb776dfaac902a25a14426b7dc9",
+  "proxyAdmin": "0x0000000000000000000000000000000000000000"
 }
diff --git a/contracts/script/OPSuccinctDeployer.s.sol b/contracts/script/OPSuccinctDeployer.s.sol
@@ -9,15 +9,37 @@ import {console} from "forge-std/console.sol";
 
 contract OPSuccinctDeployer is Script, Utils {
     function run() public returns (address) {
-        vm.startBroadcast();
+        uint256 deployPk = vm.envOr("DEPLOY_PK", uint256(0));
+        uint256 adminPk = vm.envOr("ADMIN_PK", uint256(0));
+        // If deployPk is not set, use the default key.
+        if (deployPk != uint256(0)) {
+            vm.startBroadcast(deployPk);
+        } else {
+            vm.startBroadcast();
+        }
 
         Config memory config = readJson("opsuccinctl2ooconfig.json");
 
-        // This initializes the proxy
-        OPSuccinctL2OutputOracle oracleImpl = new OPSuccinctL2OutputOracle();
-        Proxy proxy = new Proxy(msg.sender);
+        // Set the implementation address if it is not already set.
+        if (config.opSuccinctL2OutputOracleImpl == address(0)) {
+            console.log("Deploying new OPSuccinctL2OutputOracle impl");
+            config.opSuccinctL2OutputOracleImpl = address(new OPSuccinctL2OutputOracle());
+        }
 
-        upgradeAndInitialize(address(oracleImpl), config, address(proxy), true);
+        // If the Admin PK is set, use it as the owner of the proxy.
+        address proxyOwner = adminPk != uint256(0) ? vm.addr(adminPk) : msg.sender;
+
+        Proxy proxy = new Proxy(proxyOwner);
+
+        vm.stopBroadcast();
+
+        if (adminPk != uint256(0)) {
+            vm.startBroadcast(adminPk);
+        } else {
+            vm.startBroadcast();
+        }
+
+        upgradeAndInitialize(config, address(proxy), true);
 
         vm.stopBroadcast();
 

diff --git a/contracts/script/OPSuccinctUpgrader.s.sol b/contracts/script/OPSuccinctUpgrader.s.sol
@@ -9,16 +9,40 @@ import {console} from "forge-std/console.sol";
 
 contract OPSuccinctUpgrader is Script, Utils {
     function run() public {
-        vm.startBroadcast();
-
         Config memory cfg = readJson("opsuccinctl2ooconfig.json");
 
         address l2OutputOracleProxy = vm.envAddress("L2OO_ADDRESS");
         bool executeUpgradeCall = vm.envOr("EXECUTE_UPGRADE_CALL", true);
 
-        address OPSuccinctL2OutputOracleImpl = address(new OPSuccinctL2OutputOracle());
+        // Use implementation address from config
+        address OPSuccinctL2OutputOracleImpl = cfg.opSuccinctL2OutputOracleImpl;
+        address proxyAdmin = cfg.proxyAdmin;
+
+        // optionally use a different key for deployment
+        uint256 deployPk = vm.envOr("DEPLOY_PK", uint256(0));
+        uint256 adminPk = vm.envOr("ADMIN_PK", uint256(0));
+
+        // If deployPk is not set, use the default key.
+        if (deployPk != uint256(0)) {
+            vm.startBroadcast(deployPk);
+        } else {
+            vm.startBroadcast();
+        }
+
+        if (OPSuccinctL2OutputOracleImpl == address(0)) {
+            console.log("Deploying new OPSuccinctL2OutputOracle impl");
+            OPSuccinctL2OutputOracleImpl = address(new OPSuccinctL2OutputOracle());
+        }
+
+        vm.stopBroadcast();
+
+        if (adminPk != uint256(0)) {
+            vm.startBroadcast(adminPk);
+        } else {
+            vm.startBroadcast();
+        }
 
-        upgradeAndInitialize(OPSuccinctL2OutputOracleImpl, cfg, l2OutputOracleProxy, executeUpgradeCall);
+        upgradeAndInitialize(cfg, l2OutputOracleProxy, executeUpgradeCall);
 
         vm.stopBroadcast();
     }

diff --git a/contracts/test/Upgrade.t.sol b/contracts/test/Upgrade.t.sol
@@ -28,7 +28,9 @@ contract UpgradeTest is Test, Utils {
             submissionInterval: 2,
             verifier: address(0x397A5f7f3dBd538f23DE225B51f532c34448dA9B),
             aggregationVkey: bytes32(0x00ea4171dbd0027768055bee7f6d64e17e9cec99b29aad5d18e5d804b967775b),
-            rangeVkeyCommitment: bytes32(0x1a4ebe5c47d55436319c425951eb1a7e04f560945e29eb454215d30b30987bbb)
+            rangeVkeyCommitment: bytes32(0x1a4ebe5c47d55436319c425951eb1a7e04f560945e29eb454215d30b30987bbb),
+            proxyAdmin: address(0x0000000000000000000000000000000000000000),
+            opSuccinctL2OutputOracleImpl: address(0x0000000000000000000000000000000000000000)
         });
 
         // This is never called, so we just need to add some code to the address so the check passes.

diff --git a/contracts/test/helpers/JSONDecoder.sol b/contracts/test/helpers/JSONDecoder.sol
@@ -7,8 +7,10 @@ contract JSONDecoder {
         address challenger;
         uint256 finalizationPeriod;
         uint256 l2BlockTime;
+        address opSuccinctL2OutputOracleImpl;
         address owner;
         address proposer;
+        address proxyAdmin;
         bytes32 rangeVkeyCommitment;
         bytes32 rollupConfigHash;
         uint256 startingBlockNumber;

diff --git a/contracts/test/helpers/Utils.sol b/contracts/test/helpers/Utils.sol
@@ -5,21 +5,23 @@ import {Test, console} from "forge-std/Test.sol";
 import {JSONDecoder} from "./JSONDecoder.sol";
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
 import {Proxy} from "@optimism/src/universal/Proxy.sol";
+import {ProxyAdmin} from "@optimism/src/universal/ProxyAdmin.sol";
 import {OPSuccinctL2OutputOracle} from "src/OPSuccinctL2OutputOracle.sol";
 
 contract Utils is Test, JSONDecoder {
     function deployWithConfig(Config memory cfg) public returns (address) {
-        address OPSuccinctL2OutputOracleImpl = address(new OPSuccinctL2OutputOracle());
+        if (cfg.opSuccinctL2OutputOracleImpl == address(0)) {
+            cfg.opSuccinctL2OutputOracleImpl = address(new OPSuccinctL2OutputOracle());
+        }
+
         Proxy l2OutputOracleProxy = new Proxy(msg.sender);
-        upgradeAndInitialize(OPSuccinctL2OutputOracleImpl, cfg, address(l2OutputOracleProxy), true);
+        upgradeAndInitialize(cfg, address(l2OutputOracleProxy), true);
 
         return address(l2OutputOracleProxy);
     }
 
     // If `executeUpgradeCall` is false, the upgrade call will not be executed.
-    function upgradeAndInitialize(address impl, Config memory cfg, address l2OutputOracleProxy, bool executeUpgradeCall)
-        public
-    {
+    function upgradeAndInitialize(Config memory cfg, address l2OutputOracleProxy, bool executeUpgradeCall) public {
         // Require that the verifier gateway is deployed
         require(
             address(cfg.verifier).code.length > 0, "OPSuccinctL2OutputOracleUpgrader: verifier gateway not deployed"
@@ -45,12 +47,29 @@ contract Utils is Test, JSONDecoder {
             abi.encodeWithSelector(OPSuccinctL2OutputOracle.initialize.selector, initParams);
 
         if (executeUpgradeCall) {
-            Proxy existingProxy = Proxy(payable(l2OutputOracleProxy));
-            existingProxy.upgradeToAndCall(impl, initializationParams);
+            if (cfg.proxyAdmin == address(0)) {
+                Proxy existingProxy = Proxy(payable(l2OutputOracleProxy));
+                existingProxy.upgradeToAndCall(cfg.opSuccinctL2OutputOracleImpl, initializationParams);
+            } else {
+                // This is used if the ProxyAdmin contract is deployed.
+                ProxyAdmin(payable(cfg.proxyAdmin)).upgradeAndCall(
+                    payable(l2OutputOracleProxy), cfg.opSuccinctL2OutputOracleImpl, initializationParams
+                );
+            }
         } else {
             // Raw calldata for an upgrade call by a multisig.
-            bytes memory multisigCalldata =
-                abi.encodeWithSelector(Proxy.upgradeToAndCall.selector, impl, initializationParams);
+            bytes memory multisigCalldata = "";
+
+            if (cfg.proxyAdmin == address(0)) {
+                multisigCalldata = abi.encodeWithSelector(
+                    Proxy.upgradeToAndCall.selector, cfg.opSuccinctL2OutputOracleImpl, initializationParams
+                );
+            } else {
+                multisigCalldata = abi.encodeWithSelector(
+                    ProxyAdmin.upgradeAndCall.selector, cfg.opSuccinctL2OutputOracleImpl, initializationParams
+                );
+            }
+
             console.log("The calldata for upgrading the contract with the new initialization parameters is:");
             console.logBytes(multisigCalldata);
         }

diff --git a/justfile b/justfile
@@ -113,8 +113,12 @@ deploy-oracle env_file=".env":
       VERIFY="--verify --verifier etherscan --etherscan-api-key $ETHERSCAN_API_KEY"
     fi
 
+    ENV_VARS=""
+    if [ -n "${ADMIN_PK:-}" ]; then ENV_VARS="$ENV_VARS ADMIN_PK=$ADMIN_PK"; fi
+    if [ -n "${DEPLOY_PK:-}" ]; then ENV_VARS="$ENV_VARS DEPLOY_PK=$DEPLOY_PK"; fi
+
     # Run the forge deployment script
-    forge script script/OPSuccinctDeployer.s.sol:OPSuccinctDeployer \
+    $ENV_VARS forge script script/OPSuccinctDeployer.s.sol:OPSuccinctDeployer \
         --rpc-url $L1_RPC \
         --private-key $PRIVATE_KEY \
         --broadcast \
@@ -138,13 +142,19 @@ upgrade-oracle env_file=".env":
     forge install
 
     # Run the forge upgrade script
+
+    ENV_VARS="L2OO_ADDRESS=$L2OO_ADDRESS"
+    if [ -n "${EXECUTE_UPGRADE_CALL:-}" ]; then ENV_VARS="$ENV_VARS EXECUTE_UPGRADE_CALL=$EXECUTE_UPGRADE_CALL"; fi
+    if [ -n "${ADMIN_PK:-}" ]; then ENV_VARS="$ENV_VARS ADMIN_PK=$ADMIN_PK"; fi
+    if [ -n "${DEPLOY_PK:-}" ]; then ENV_VARS="$ENV_VARS DEPLOY_PK=$DEPLOY_PK"; fi
+
     if [ "${EXECUTE_UPGRADE_CALL:-true}" = "false" ]; then
-        L2OO_ADDRESS=$L2OO_ADDRESS forge script script/OPSuccinctUpgrader.s.sol:OPSuccinctUpgrader \
+        $ENV_VARS forge script script/OPSuccinctUpgrader.s.sol:OPSuccinctUpgrader \
             --rpc-url $L1_RPC \
             --private-key $PRIVATE_KEY \
             --etherscan-api-key $ETHERSCAN_API_KEY
     else
-        L2OO_ADDRESS=$L2OO_ADDRESS forge script script/OPSuccinctUpgrader.s.sol:OPSuccinctUpgrader \
+        $ENV_VARS forge script script/OPSuccinctUpgrader.s.sol:OPSuccinctUpgrader \
             --rpc-url $L1_RPC \
             --private-key $PRIVATE_KEY \
             --verify \
@@ -171,12 +181,19 @@ update-parameters env_file=".env":
     forge install
 
     # Run the forge upgrade script
+    ENV_VARS="L2OO_ADDRESS=$L2OO_ADDRESS"
+    if [ -n "${EXECUTE_UPGRADE_CALL:-}" ]; then ENV_VARS="$ENV_VARS EXECUTE_UPGRADE_CALL=$EXECUTE_UPGRADE_CALL"; fi
+    if [ -n "${ADMIN_PK:-}" ]; then ENV_VARS="$ENV_VARS ADMIN_PK=$ADMIN_PK"; fi
+    if [ -n "${DEPLOY_PK:-}" ]; then ENV_VARS="$ENV_VARS DEPLOY_PK=$DEPLOY_PK"; fi
+
+
     if [ "${EXECUTE_UPGRADE_CALL:-true}" = "false" ]; then
-        L2OO_ADDRESS=$L2OO_ADDRESS forge script script/OPSuccinctParameterUpdater.s.sol:OPSuccinctParameterUpdater \
+        $ENV_VARS forge script script/OPSuccinctParameterUpdater.s.sol:OPSuccinctParameterUpdater \
             --rpc-url $L1_RPC \
-            --private-key $PRIVATE_KEY
+            --private-key $PRIVATE_KEY \
+            --broadcast
     else
-        L2OO_ADDRESS=$L2OO_ADDRESS forge script script/OPSuccinctParameterUpdater.s.sol:OPSuccinctParameterUpdater \
+        $ENV_VARS forge script script/OPSuccinctParameterUpdater.s.sol:OPSuccinctParameterUpdater \
             --rpc-url $L1_RPC \
             --private-key $PRIVATE_KEY \
             --broadcast
@@ -205,4 +222,5 @@ deploy-dispute-game-factory env_file=".env":
         --rpc-url $L1_RPC \
         --private-key $PRIVATE_KEY \
         --broadcast \
-        $VERIFY
+        $VERIFY
+    fi
diff --git a/scripts/utils/bin/fetch_rollup_config.rs b/scripts/utils/bin/fetch_rollup_config.rs
@@ -1,5 +1,5 @@
 use alloy::{eips::BlockId, hex, signers::local::PrivateKeySigner};
-use alloy_primitives::{Address, B256};
+use alloy_primitives::Address;
 use anyhow::Result;
 use op_succinct_client_utils::{boot::hash_rollup_config, types::u32_to_u8};
 use op_succinct_host_utils::fetcher::{OPSuccinctDataFetcher, RPCMode, RunContext};
@@ -22,6 +22,7 @@ struct L2OOConfig {
     challenger: String,
     finalization_period: u64,
     l2_block_time: u64,
+    op_succinct_l2_output_oracle_impl: String,
     owner: String,
     proposer: String,
     rollup_config_hash: String,
@@ -32,20 +33,29 @@ struct L2OOConfig {
     verifier: String,
     aggregation_vkey: String,
     range_vkey_commitment: String,
+    proxy_admin: String,
 }
 
-/// If the environment variable is set for the address, return it. Otherwise, return the address associated with the private key. If the private key is not set, return the zero address.
-fn get_address(env_var: &str) -> String {
-    let private_key = env::var("PRIVATE_KEY").unwrap_or_else(|_| B256::ZERO.to_string());
+/// Returns an address based on environment variables and private key settings:
+/// - If env_var exists, returns that address
+/// - Otherwise if private_key_by_default=true and PRIVATE_KEY exists, returns address derived from private key
+/// - Otherwise returns zero address
+fn get_address(env_var: &str, private_key_by_default: bool) -> String {
+    // First try to get address directly from env var
+    if let Ok(addr) = env::var(env_var) {
+        return addr;
+    }
 
-    env::var(env_var).unwrap_or_else(|_| {
-        if private_key == B256::ZERO.to_string() {
-            Address::ZERO.to_string()
-        } else {
-            let signer: PrivateKeySigner = private_key.parse().unwrap();
-            signer.address().to_string()
+    // Next try to derive address from private key if enabled
+    if private_key_by_default {
+        if let Ok(pk) = env::var("PRIVATE_KEY") {
+            let signer: PrivateKeySigner = pk.parse().unwrap();
+            return signer.address().to_string();
         }
-    })
+    }
+
+    // Fallback to zero address
+    Address::ZERO.to_string()
 }
 
 /// Update the L2OO config with the rollup config hash and other relevant data before the contract is deployed.
@@ -117,9 +127,12 @@ async fn update_l2oo_config() -> Result<()> {
         .unwrap_or(DEFAULT_FINALIZATION_PERIOD_SECS);
 
     // Default to the address associated with the private key if the environment variable is not set. If private key is not set, default to zero address.
-    let proposer = get_address("PROPOSER");
-    let owner = get_address("OWNER");
-    let challenger = get_address("CHALLENGER");
+    let proposer = get_address("PROPOSER", true);
+    let owner = get_address("OWNER", true);
+    let challenger = get_address("CHALLENGER", true);
+
+    let proxy_admin = get_address("PROXY_ADMIN", false);
+    let op_succinct_l2_output_oracle_impl = get_address("OP_SUCCINCT_L2_OUTPUT_ORACLE_IMPL", false);
 
     let prover = ProverClient::builder().cpu().build();
     let (_, agg_vkey) = prover.setup(AGG_ELF);
@@ -142,6 +155,8 @@ async fn update_l2oo_config() -> Result<()> {
         verifier,
         aggregation_vkey,
         range_vkey_commitment,
+        proxy_admin,
+        op_succinct_l2_output_oracle_impl,
     };
 
     write_l2oo_config(l2oo_config, workspace_root.as_std_path())?;