Skip to content

Commit

Permalink
feat[contracts]: add ability to pause EM during upgrades (#892)
Browse files Browse the repository at this point in the history 
* wip: start work on pausing em during upgrade

* fix: execution manager gas test

* test: add tests for deployer fallback

* add some additional comments
  • Loading branch information
@smartcontracts @gakonst
smartcontracts authored and gakonst committed May 21, 2021
1 parent b3fba19 commit 466827d
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 19 deletions.
85 changes: 76 additions & 9 deletions packages/contracts/contracts/chugsplash/L2ChugSplashDeployer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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 *
*********/
Expand Down Expand Up @@ -71,6 +79,14 @@ contract L2ChugSplashDeployer is Ownable {
}


/*************
* Constants *
*************/

// bytes32(uint256(keccak256("upgrading")) - 1);
bytes32 public constant SLOT_KEY_IS_UPGRADING = 0xac04bb17f7be83a1536e4b894c20a9b8acafb7c35cd304dfa3dabeee91e3c4c2;


/*************
* Variables *
*************/
Expand Down Expand Up @@ -107,24 +123,73 @@ 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 *
********************/

/**
* @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;
}

/**
Expand All @@ -150,7 +215,7 @@ contract L2ChugSplashDeployer is Ownable {
);

require(
hasActiveBundle() == false,
isUpgrading() == false,
"ChugSplashDeployer: previous bundle is still active"
);

Expand Down Expand Up @@ -180,7 +245,7 @@ contract L2ChugSplashDeployer is Ownable {
public
{
require(
hasActiveBundle() == true,
isUpgrading() == true,
"ChugSplashDeployer: there is no active bundle"
);

Expand Down Expand Up @@ -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."
Expand All @@ -268,6 +333,8 @@ contract L2ChugSplashDeployer is Ownable {
)
internal
{
// TODO: Requires system status work planned for Ben.
assembly {
sstore(SLOT_KEY_IS_UPGRADING, _upgrading)
}
}
}
48 changes: 42 additions & 6 deletions packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 *
*****************************************/
Expand Down
4 changes: 3 additions & 1 deletion packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager.gas-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
81 changes: 78 additions & 3 deletions packages/contracts/test/contracts/chugsplash/L2ChugSplashDeployer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 () => {
Expand All @@ -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
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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
})
})
})

0 comments on commit 466827d

Please sign in to comment.