diff --git a/.changeset/fluffy-knives-impress.md b/.changeset/fluffy-knives-impress.md new file mode 100644 index 0000000000000..ff6c6e4ef8149 --- /dev/null +++ b/.changeset/fluffy-knives-impress.md @@ -0,0 +1,5 @@ +--- +'@eth-optimism/contracts': patch +--- + +enables l2 upgrades to be initiated by an l1 to l2 message diff --git a/packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol b/packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol index e709f9d18b65c..59aaad87d2d77 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 { OVM_CrossDomainEnabled } from "../optimistic-ethereum/libraries/bridge/OVM_CrossDomainEnabled.sol"; /* External Imports */ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; @@ -20,7 +21,7 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; * actions (in any order) by demonstrating with a Merkle proof that the action was approved by the * contract owner. Only a single upgrade may be active at any given time. */ -contract L2ChugSplashDeployer is Ownable { +contract L2ChugSplashDeployer is Ownable, OVM_CrossDomainEnabled { /********* * Enums * @@ -100,7 +101,7 @@ contract L2ChugSplashDeployer is Ownable { /*************** * Constructor * ***************/ - + /** * @param _owner Address that will initially own the L2ChugSplashDeployer. */ @@ -108,11 +109,33 @@ contract L2ChugSplashDeployer is Ownable { address _owner ) Ownable() + OVM_CrossDomainEnabled(0x4200000000000000000000000000000000000007) { transferOwnership(_owner); } + /********************* + * Function Modifier * + *********************/ + + /** + * A modifier like "onlyOwner" but also alternatively allows the owner to call this contract + * via an L1 => L2 message. + */ + modifier onlyOwnerOrCrossDomainOwner() { + require( + msg.sender == owner() + || ( + msg.sender == address(getCrossDomainMessenger()) + && getCrossDomainMessenger().xDomainMessageSender() == owner() + ), + "ChugSplashDeployer: caller is not the owner" + ); + _; + } + + /******************** * Public Functions * ********************/ @@ -143,7 +166,7 @@ contract L2ChugSplashDeployer is Ownable { uint256 _bundleSize ) public - onlyOwner + onlyOwnerOrCrossDomainOwner { require( _bundleHash != bytes32(0), @@ -179,7 +202,7 @@ contract L2ChugSplashDeployer is Ownable { */ function cancelTransactionBundle() public - onlyOwner + onlyOwnerOrCrossDomainOwner { require( hasActiveBundle() == true, @@ -208,7 +231,7 @@ contract L2ChugSplashDeployer is Ownable { uint256 _bundleSize ) public - onlyOwner + onlyOwnerOrCrossDomainOwner { cancelTransactionBundle(); approveTransactionBundle(_bundleHash, _bundleSize); diff --git a/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts b/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts index dbc34a845593e..39ac4775fe08c 100644 --- a/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts +++ b/packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts @@ -4,6 +4,7 @@ import { expect } from '../../setup' import hre from 'hardhat' import { ethers, Contract, Signer, ContractFactory } from 'ethers' import { MockContract, smockit } from '@eth-optimism/smock' +import { toPlainObject } from 'lodash' /* Imports: Internal */ import { @@ -12,7 +13,6 @@ import { predeploys, } from '../../../src' import { NON_NULL_BYTES32, NON_ZERO_ADDRESS } from '../../helpers' -import { toPlainObject } from 'lodash' describe('L2ChugSplashDeployer', () => { let signer1: Signer @@ -22,10 +22,17 @@ describe('L2ChugSplashDeployer', () => { }) let Mock__OVM_ExecutionManager: MockContract + let Mock__OVM_L2CrossDomainMessenger: MockContract before(async () => { Mock__OVM_ExecutionManager = await smockit('OVM_ExecutionManager', { address: predeploys.OVM_ExecutionManagerWrapper, }) + Mock__OVM_L2CrossDomainMessenger = await smockit( + 'OVM_L2CrossDomainMessenger', + { + address: predeploys.OVM_L2CrossDomainMessenger, + } + ) }) let Factory__L2ChugSplashDeployer: ContractFactory @@ -59,7 +66,7 @@ describe('L2ChugSplashDeployer', () => { ethers.constants.HashZero, 0 ) - ).to.be.revertedWith('Ownable: caller is not the owner') + ).to.be.revertedWith('ChugSplashDeployer: caller is not the owner') }) it('should allow the owner to approve a new transaction bundle', async () => { @@ -77,6 +84,34 @@ describe('L2ChugSplashDeployer', () => { expect(await L2ChugSplashDeployer.currentBundleSize()).to.equal(1234) }) + it('should allow the owner to approve a new transaction bundle via an L1 => L2 message', async () => { + Mock__OVM_L2CrossDomainMessenger.smocked.xDomainMessageSender.will.return.with( + await signer1.getAddress() + ) + + await expect( + L2ChugSplashDeployer.connect( + hre.ethers.provider + ).callStatic.approveTransactionBundle(NON_NULL_BYTES32, 1234, { + from: Mock__OVM_L2CrossDomainMessenger.address, + }) + ).to.not.be.reverted + }) + + it('should revert if cross domain message sender is not the owner', async () => { + Mock__OVM_L2CrossDomainMessenger.smocked.xDomainMessageSender.will.return.with( + await signer2.getAddress() + ) + + await expect( + L2ChugSplashDeployer.connect( + hre.ethers.provider + ).callStatic.approveTransactionBundle(NON_NULL_BYTES32, 1234, { + from: Mock__OVM_L2CrossDomainMessenger.address, + }) + ).to.be.revertedWith('ChugSplashDeployer: caller is not the owner') + }) + it('should revert if trying to approve a bundle with the empty hash', async () => { await expect( L2ChugSplashDeployer.connect(signer1).approveTransactionBundle( @@ -264,7 +299,21 @@ describe('L2ChugSplashDeployer', () => { it('should revert if caller is not the owner', async () => { await expect( L2ChugSplashDeployer.connect(signer2).cancelTransactionBundle() - ).to.be.revertedWith('Ownable: caller is not the owner') + ).to.be.revertedWith('ChugSplashDeployer: caller is not the owner') + }) + + it('should revert if cross domain message sender is not the owner', async () => { + Mock__OVM_L2CrossDomainMessenger.smocked.xDomainMessageSender.will.return.with( + await signer2.getAddress() + ) + + await expect( + L2ChugSplashDeployer.connect( + hre.ethers.provider + ).callStatic.cancelTransactionBundle({ + from: Mock__OVM_L2CrossDomainMessenger.address, + }) + ).to.be.revertedWith('ChugSplashDeployer: caller is not the owner') }) it('should revert if there is no active bundle', async () => { @@ -308,6 +357,20 @@ describe('L2ChugSplashDeployer', () => { expect(await L2ChugSplashDeployer.currentBundleSize()).to.equal(0) }) + it('should allow the owner to cancel a bundle via an L1 => L2 message', async () => { + Mock__OVM_L2CrossDomainMessenger.smocked.xDomainMessageSender.will.return.with( + await signer1.getAddress() + ) + + await expect( + L2ChugSplashDeployer.connect( + hre.ethers.provider + ).callStatic.cancelTransactionBundle({ + from: Mock__OVM_L2CrossDomainMessenger.address, + }) + ).to.not.be.reverted + }) + it('should allow the owner to cancel an active bundle after some actions have been executed', async () => { await L2ChugSplashDeployer.executeAction( bundle.actions[0].action, @@ -339,7 +402,21 @@ describe('L2ChugSplashDeployer', () => { NON_NULL_BYTES32, 1234 ) - ).to.be.revertedWith('Ownable: caller is not the owner') + ).to.be.revertedWith('ChugSplashDeployer: caller is not the owner') + }) + + it('should revert if cross domain message sender is not the owner', async () => { + Mock__OVM_L2CrossDomainMessenger.smocked.xDomainMessageSender.will.return.with( + await signer2.getAddress() + ) + + await expect( + L2ChugSplashDeployer.connect( + hre.ethers.provider + ).callStatic.overrideTransactionBundle(NON_NULL_BYTES32, 1234, { + from: Mock__OVM_L2CrossDomainMessenger.address, + }) + ).to.be.revertedWith('ChugSplashDeployer: caller is not the owner') }) it('should allow the owner to override the current active bundle', async () => { @@ -360,5 +437,24 @@ describe('L2ChugSplashDeployer', () => { ) expect(await L2ChugSplashDeployer.currentBundleSize()).to.equal(1234) }) + + it('should allow the owner to override a bundle via an L1 => L2 message', async () => { + Mock__OVM_L2CrossDomainMessenger.smocked.xDomainMessageSender.will.return.with( + await signer1.getAddress() + ) + + await L2ChugSplashDeployer.connect(signer1).approveTransactionBundle( + '0x' + 'FF'.repeat(32), + 12345 + ) + + await expect( + L2ChugSplashDeployer.connect( + hre.ethers.provider + ).callStatic.overrideTransactionBundle(NON_NULL_BYTES32, 1234, { + from: Mock__OVM_L2CrossDomainMessenger.address, + }) + ).to.not.be.reverted + }) }) })