diff --git a/packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol b/packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol index 8c37ecb378475..d7865b1b380ea 100644 --- a/packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol +++ b/packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2; /* Library Imports */ import { Lib_ExecutionManagerWrapper } from "../optimistic-ethereum/libraries/wrappers/Lib_ExecutionManagerWrapper.sol"; import { Lib_MerkleTree } from "../optimistic-ethereum/libraries/utils/Lib_MerkleTree.sol"; +import { Lib_EIP155Tx } from "../optimistic-ethereum/libraries/codec/Lib_EIP155Tx.sol"; /* External Imports */ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; @@ -22,6 +23,13 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; */ contract L2ChugSplashDeployer is Ownable { + /************* + * Libraries * + *************/ + + using Lib_EIP155Tx for Lib_EIP155Tx.EIP155Tx; + + /********* * Enums * *********/ @@ -71,6 +79,14 @@ contract L2ChugSplashDeployer is Ownable { } + /************* + * Constants * + *************/ + + // bytes32(uint256(keccak256("upgrading")) - 1); + bytes32 public constant SLOT_KEY_IS_UPGRADING = 0xac04bb17f7be83a1536e4b894c20a9b8acafb7c35cd304dfa3dabeee91e3c4c2; + + /************* * Variables * *************/ @@ -107,6 +123,54 @@ contract L2ChugSplashDeployer is Ownable { } + /********************* + * Fallback Function * + *********************/ + + fallback() + external + { + // Fallback function is used as a way to gracefully handle upgrades. When + // `isUpgrading() == true`, all transactions are automatically routed to this contract. + // msg.data may (or may not) be a properly encoded EIP-155 transaction. However, it’s ok to + // assume that the data *is* properly encoded because any improperly encoded transactions + // will simply trigger a revert when we try to decode them below. Since L1 => L2 messages + // are *not* EIP-155 transactions, they’ll revert here (meaning those messages will fail). + // As a result, any L1 => L2 messages executed during an upgrade will have to be replayed. + + // We use this twice, so it’s more gas efficient to store a copy of it (barely). + bytes memory encodedTx = msg.data; + + // Decode the tx with the correct chain ID. + Lib_EIP155Tx.EIP155Tx memory decodedTx = Lib_EIP155Tx.decode( + encodedTx, + Lib_ExecutionManagerWrapper.ovmCHAINID() + ); + + // Make sure that the transaction target is the L2ChugSplashDeployer. Any transactions that + // were not intended to be sent to this contract will revert at this point. + require( + decodedTx.to == address(this), + "L2ChugSplashDeployer: the system is currently undergoing an upgrade" + ); + + // Call into this contract with the decoded transaction data. Of course this means that + // any functions with onlyOwner cannot be triggered via this fallback, but that’s ok + // because we only need to be able to trigger executeAction. + (bool success, bytes memory returndata) = address(this).call(decodedTx.data); + + if (success) { + assembly { + return(add(returndata, 0x20), mload(returndata)) + } + } else { + assembly { + revert(add(returndata, 0x20), mload(returndata)) + } + } + } + + /******************** * Public Functions * ********************/ @@ -114,17 +178,18 @@ contract L2ChugSplashDeployer is Ownable { /** * @return boolean, whether or not an upgrade is currently being executed. */ - function hasActiveBundle() + function isUpgrading() public view returns ( bool ) { - return ( - currentBundleHash != bytes32(0) - && currentBundleTxsExecuted < currentBundleSize - ); + bool status; + assembly { + status := sload(SLOT_KEY_IS_UPGRADING) + } + return status; } /** @@ -150,7 +215,7 @@ contract L2ChugSplashDeployer is Ownable { ); require( - hasActiveBundle() == false, + isUpgrading() == false, "ChugSplashDeployer: previous bundle is still active" ); @@ -180,7 +245,7 @@ contract L2ChugSplashDeployer is Ownable { public { require( - hasActiveBundle() == true, + isUpgrading() == true, "ChugSplashDeployer: there is no active bundle" ); @@ -258,7 +323,7 @@ contract L2ChugSplashDeployer is Ownable { /********************** * Internal Functions * **********************/ - + /** * Sets the system status to "upgrading" or "done upgrading" depending on the boolean input. * @param _upgrading `true` sets status to "upgrading", `false` to "done upgrading." @@ -268,6 +333,8 @@ contract L2ChugSplashDeployer is Ownable { ) internal { - // TODO: Requires system status work planned for Ben. + assembly { + sstore(SLOT_KEY_IS_UPGRADING, _upgrading) + } } } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol index d72de07edd6f9..8b9953047d5de 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol @@ -204,12 +204,31 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { // // Check gas right before the call to get total gas consumed by OVM transaction. // uint256 gasProvided = gasleft(); - // Run the transaction, make sure to meter the gas usage. - (, bytes memory returndata) = ovmCALL( - _transaction.gasLimit - gasMeterConfig.minTransactionGasLimit, - _transaction.entrypoint, - _transaction.data - ); + bytes memory returndata; + if (_isUpgrading() == true) { + // When we’re in the middle of an upgrade we completely ignore + // `transaction._entrypoint` and direct *all* transactions to the L2ChugSplashDeployer + // located at 0x42...0D. L1 => L2 messages executed during the middle of an upgrade + // will fail. Any transactions *not* intended to be sent to the L2ChugSplashDeployer + // will also fail and must be submitted again. + (bool success, bytes memory ret) = ovmCALL( + _transaction.gasLimit - gasMeterConfig.minTransactionGasLimit, + 0x420000000000000000000000000000000000000D, + _transaction.data + ); + + returndata = abi.encode( + success, + ret + ); + } else { + // Run the transaction, make sure to meter the gas usage. + (, returndata) = ovmCALL( + _transaction.gasLimit - gasMeterConfig.minTransactionGasLimit, + _transaction.entrypoint, + _transaction.data + ); + } // TEMPORARY: Gas metering is disabled for minnet. // // Update the cumulative gas based on the amount of gas used. @@ -1790,6 +1809,23 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { } + /******************************** + * Internal Functions: Upgrades * + ********************************/ + + function _isUpgrading() + internal + returns ( + bool + ) + { + return uint256(_getContractStorage( + 0x420000000000000000000000000000000000000D, + 0xac04bb17f7be83a1536e4b894c20a9b8acafb7c35cd304dfa3dabeee91e3c4c2 + )) != 0; + } + + /***************************************** * Internal Functions: Execution Context * *****************************************/ diff --git a/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager.gas-spec.ts b/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager.gas-spec.ts index c6fdaa0577b22..c24e16af914a3 100644 --- a/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager.gas-spec.ts +++ b/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager.gas-spec.ts @@ -81,6 +81,8 @@ describe('OVM_ExecutionManager gas consumption', () => { MOCK__STATE_MANAGER.smocked.hasAccount.will.return.with(true) MOCK__STATE_MANAGER.smocked.testAndSetAccountLoaded.will.return.with(true) + MOCK__STATE_MANAGER.smocked.hasContractStorage.will.return.with(true) + await AddressManager.setAddress( 'OVM_StateManagerFactory', MOCK__STATE_MANAGER.address @@ -110,7 +112,7 @@ describe('OVM_ExecutionManager gas consumption', () => { ) console.log(`calculated gas cost of ${gasCost}`) - const benchmark: number = 106_000 + const benchmark: number = 117_000 expect(gasCost).to.be.lte(benchmark) expect(gasCost).to.be.gte( benchmark - 1_000, diff --git a/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts b/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts index 2e234daf2ca57..5bc6b5f2bd6c3 100644 --- a/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts +++ b/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts @@ -2,7 +2,7 @@ import { expect } from '../../setup' /* Imports: External */ import hre from 'hardhat' -import { ethers, Contract, Signer, ContractFactory } from 'ethers' +import { ethers, Contract, Signer, ContractFactory, Wallet } from 'ethers' import { MockContract, smockit } from '@eth-optimism/smock' /* Imports: Internal */ @@ -15,6 +15,11 @@ import { NON_NULL_BYTES32, NON_ZERO_ADDRESS } from '../../helpers' import { toPlainObject } from 'lodash' describe('L2ChugSplashDeployer', () => { + let wallet: Wallet + before(async () => { + ;[wallet] = hre.waffle.provider.getWallets() + }) + let signer1: Signer let signer2: Signer before(async () => { @@ -26,6 +31,8 @@ describe('L2ChugSplashDeployer', () => { Mock__OVM_ExecutionManager = await smockit('OVM_ExecutionManager', { address: predeploys.OVM_ExecutionManagerWrapper, }) + + Mock__OVM_ExecutionManager.smocked.ovmCHAINID.will.return.with(420) }) let Factory__L2ChugSplashDeployer: ContractFactory @@ -236,13 +243,13 @@ describe('L2ChugSplashDeployer', () => { }) it('should change the upgrade status when the bundle is complete', async () => { - expect(await L2ChugSplashDeployer.hasActiveBundle()).to.equal(true) + expect(await L2ChugSplashDeployer.isUpgrading()).to.equal(true) for (const action of bundle.actions) { await L2ChugSplashDeployer.executeAction(action.action, action.proof) } - expect(await L2ChugSplashDeployer.hasActiveBundle()).to.equal(false) + expect(await L2ChugSplashDeployer.isUpgrading()).to.equal(false) }) it('should allow the upgrader to submit a new bundle when the previous bundle is complete', async () => { @@ -259,4 +266,72 @@ describe('L2ChugSplashDeployer', () => { }) }) }) + + describe('fallback', () => { + it('should revert if not provided a valid EIP155 tx', async () => { + await expect( + signer1.sendTransaction({ + to: L2ChugSplashDeployer.address, + data: '0x', + }) + ).to.be.reverted + }) + + it('should revert if the target is not the L2ChugSplashDeployer', async () => { + await expect( + signer1.sendTransaction({ + to: L2ChugSplashDeployer.address, + data: await wallet.signTransaction({ + chainId: 420, + to: await signer1.getAddress(), + data: '0x', + }), + }) + ).to.be.reverted + }) + + it('should revert if trying to call approveTransactionBundle', async () => { + await expect( + signer1.sendTransaction({ + to: L2ChugSplashDeployer.address, + data: await wallet.signTransaction({ + chainId: 420, + to: L2ChugSplashDeployer.address, + data: L2ChugSplashDeployer.interface.encodeFunctionData( + 'approveTransactionBundle', + [ethers.constants.HashZero, 1234] + ), + }), + }) + ).to.be.reverted + }) + + it('should be able to trigger executeAction', async () => { + const bundle: ChugSplashActionBundle = getChugSplashActionBundle([ + { + target: NON_ZERO_ADDRESS, + code: '0x1234', + }, + ]) + + await L2ChugSplashDeployer.connect(signer1).approveTransactionBundle( + bundle.root, + bundle.actions.length + ) + + await expect( + signer1.sendTransaction({ + to: L2ChugSplashDeployer.address, + data: await wallet.signTransaction({ + chainId: 420, + to: L2ChugSplashDeployer.address, + data: L2ChugSplashDeployer.interface.encodeFunctionData( + 'executeAction', + [bundle.actions[0].action, bundle.actions[0].proof] + ), + }), + }) + ).to.not.be.reverted + }) + }) })