Commit 75089d0a authored by smartcontracts's avatar smartcontracts Committed by GitHub

feat(ctb): clean up initialization logic (#2942)

* feat(ctb): clean up initialization logic

Cleans and standardizes the initialization logic across all contracts.
Importantly, removes reinitialization.

* contracts-bedrock: clean up hh genesis task
Co-authored-by: default avatarMark Tyneway <mark.tyneway@gmail.com>
parent 7d9820b6
---
'@eth-optimism/contracts-bedrock': patch
---
Cleans up initialization logic everywhere
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
......@@ -38,8 +38,6 @@
| receivedMessages | mapping(bytes32 => bool) | 205 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
|------------------------+--------------------------+------+--------+-------+----------------------------------------------------------------|
| blockedSystemAddresses | mapping(address => bool) | 206 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
|------------------------+--------------------------+------+--------+-------+----------------------------------------------------------------|
| portal | contract OptimismPortal | 207 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
+------------------------+--------------------------+------+--------+-------+----------------------------------------------------------------+
=======================
......@@ -49,15 +47,15 @@
+---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------+
| Name | Type | Slot | Offset | Bytes | Contract |
+==============================================================================================================================================+
| messenger | contract CrossDomainMessenger | 0 | 0 | 20 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
| _initialized | uint8 | 0 | 0 | 1 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| otherBridge | contract StandardBridge | 1 | 0 | 20 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
| _initializing | bool | 0 | 1 | 1 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| deposits | mapping(address => mapping(address => uint256)) | 2 | 0 | 32 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
| messenger | contract CrossDomainMessenger | 0 | 2 | 20 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| _initialized | uint8 | 3 | 0 | 1 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
| otherBridge | contract StandardBridge | 1 | 0 | 20 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| _initializing | bool | 3 | 1 | 1 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
| deposits | mapping(address => mapping(address => uint256)) | 2 | 0 | 32 | contracts/L1/L1StandardBridge.sol:L1StandardBridge |
+---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------+
=======================
......@@ -207,15 +205,19 @@
➡ L2StandardBridge
=======================
+-------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------+
| Name | Type | Slot | Offset | Bytes | Contract |
+============================================================================================================================================+
| messenger | contract CrossDomainMessenger | 0 | 0 | 20 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
|-------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| otherBridge | contract StandardBridge | 1 | 0 | 20 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
|-------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| deposits | mapping(address => mapping(address => uint256)) | 2 | 0 | 32 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
+-------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------+
+---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------+
| Name | Type | Slot | Offset | Bytes | Contract |
+==============================================================================================================================================+
| _initialized | uint8 | 0 | 0 | 1 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| _initializing | bool | 0 | 1 | 1 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| messenger | contract CrossDomainMessenger | 0 | 2 | 20 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| otherBridge | contract StandardBridge | 1 | 0 | 20 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
|---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------|
| deposits | mapping(address => mapping(address => uint256)) | 2 | 0 | 32 | contracts/L2/L2StandardBridge.sol:L2StandardBridge |
+---------------+-------------------------------------------------+------+--------+-------+----------------------------------------------------+
=======================
➡ L2ToL1MessagePasser
......@@ -341,8 +343,7 @@
➡ OptimismMintableERC20Factory
=======================
+--------+---------+------+--------+-------+-----------------------------------------------------------------------------------+
| Name | Type | Slot | Offset | Bytes | Contract |
+==============================================================================================================================+
| bridge | address | 0 | 0 | 20 | contracts/universal/OptimismMintableERC20Factory.sol:OptimismMintableERC20Factory |
+--------+---------+------+--------+-------+-----------------------------------------------------------------------------------+
+------+------+------+--------+-------+----------+
| Name | Type | Slot | Offset | Bytes | Contract |
+================================================+
+------+------+------+--------+-------+----------+
......@@ -13,34 +13,29 @@ import { CrossDomainMessenger } from "../universal/CrossDomainMessenger.sol";
* interface instead of interacting with lower-level contracts directly.
*/
contract L1CrossDomainMessenger is CrossDomainMessenger {
/**
* @notice Contract version number.
*/
uint8 public constant VERSION = 1;
/**
* @notice Address of the OptimismPortal.
*/
OptimismPortal public portal;
OptimismPortal public immutable portal;
/**
* @param _portal Address of the OptimismPortal to send and receive messages through.
* @param _portal Address of the OptimismPortal contract on this network.
*/
constructor(OptimismPortal _portal) public {
// Mutables
initialize(_portal);
portal = _portal;
initialize();
}
/**
* @notice Intializes mutable variables.
*
* @param _portal Address of the OptimismPortal to send and receive messages through.
* @notice Initializer.
*/
function initialize(OptimismPortal _portal) public reinitializer(VERSION) {
portal = _portal;
function initialize() public initializer {
address[] memory blockedSystemAddresses = new address[](1);
blockedSystemAddresses[0] = address(this);
_initialize(PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER, blockedSystemAddresses);
__CrossDomainMessenger_init(
PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER,
blockedSystemAddresses
);
}
/**
......
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.9;
import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
import { PredeployAddresses } from "../libraries/PredeployAddresses.sol";
import { StandardBridge } from "../universal/StandardBridge.sol";
......@@ -12,12 +11,7 @@ import { StandardBridge } from "../universal/StandardBridge.sol";
* L2. ERC20 tokens deposited into L2 are escrowed within this contract until withdrawal.
* ETH is transferred to and escrowed within the OptimismPortal contract.
*/
contract L1StandardBridge is StandardBridge, Initializable {
/**
* @notice Contract version number.
*/
uint8 public constant VERSION = 1;
contract L1StandardBridge is StandardBridge {
/**
* @custom:legacy
* @notice Emitted whenever a deposit of ETH from L1 into L2 is initiated.
......@@ -94,17 +88,16 @@ contract L1StandardBridge is StandardBridge, Initializable {
* @param _messenger Address of the L1CrossDomainMessenger.
*/
constructor(address payable _messenger) public {
// Mutables
initialize(_messenger);
}
/**
* @notice Intializes mutable variables.
* @notice Initializer.
*
* @param _messenger Address of the L1CrossDomainMessenger.
*/
function initialize(address payable _messenger) public reinitializer(VERSION) {
_initialize(_messenger, payable(PredeployAddresses.L2_STANDARD_BRIDGE));
function initialize(address payable _messenger) public initializer {
__StandardBridge_init(_messenger, payable(PredeployAddresses.L2_STANDARD_BRIDGE));
}
/**
......
......@@ -14,11 +14,6 @@ import {
*/
// slither-disable-next-line locked-ether
contract L2OutputOracle is OwnableUpgradeable {
/**
* @notice Contract version number.
*/
uint8 public constant VERSION = 1;
/**
* @notice OutputProposal represents a commitment to the L2 state.
* The timestamp is the L1 timestamp that the output root is posted.
......@@ -139,19 +134,17 @@ contract L2OutputOracle is OwnableUpgradeable {
"Output Oracle: Initial L2 block time must be less than current time"
);
// Immutables
SUBMISSION_INTERVAL = _submissionInterval;
HISTORICAL_TOTAL_BLOCKS = _historicalTotalBlocks;
STARTING_BLOCK_NUMBER = _startingBlockNumber;
STARTING_TIMESTAMP = _startingTimestamp;
L2_BLOCK_TIME = _l2BlockTime;
// Mutables
initialize(_genesisL2Output, _startingBlockNumber, _sequencer, _owner);
}
/**
* @notice Intializes mutable variables.
* @notice Initializer.
*
* @param _genesisL2Output The initial L2 output of the L2 chain.
* @param _startingBlockNumber The timestamp to start L2 block at.
......@@ -163,7 +156,7 @@ contract L2OutputOracle is OwnableUpgradeable {
uint256 _startingBlockNumber,
address _sequencer,
address _owner
) public reinitializer(VERSION) {
) public initializer {
l2Outputs[_startingBlockNumber] = OutputProposal(_genesisL2Output, block.timestamp);
latestBlockNumber = _startingBlockNumber;
__Ownable_init();
......
......@@ -16,11 +16,6 @@ import { ResourceMetering } from "./ResourceMetering.sol";
* Users are encouraged to use the L1CrossDomainMessenger for a higher-level interface.
*/
contract OptimismPortal is Initializable, ResourceMetering {
/**
* @notice Contract version number.
*/
uint8 public constant VERSION = 1;
/**
* @notice Emitted when a transaction is deposited from L1 to L2. The parameters of this event
* are read by the rollup node and used to derive deposit transactions on L2.
......@@ -98,18 +93,16 @@ contract OptimismPortal is Initializable, ResourceMetering {
* @param _finalizationPeriodSeconds Output finalization time in seconds.
*/
constructor(L2OutputOracle _l2Oracle, uint256 _finalizationPeriodSeconds) {
// Immutables
L2_ORACLE = _l2Oracle;
FINALIZATION_PERIOD_SECONDS = _finalizationPeriodSeconds;
// Mutables
initialize();
}
/**
* @notice Intializes mutable variables.
*/
function initialize() public reinitializer(VERSION) {
function initialize() public initializer {
l2Sender = DEFAULT_L2_SENDER;
__ResourceMetering_init();
}
......
......@@ -16,16 +16,22 @@ import { L2ToL1MessagePasser } from "./L2ToL1MessagePasser.sol";
*/
contract L2CrossDomainMessenger is CrossDomainMessenger {
/**
* @notice Initializes the L2CrossDomainMessenger.
* @param _l1CrossDomainMessenger Address of the L1CrossDomainMessenger contract.
*/
constructor(address _l1CrossDomainMessenger) {
initialize(_l1CrossDomainMessenger);
}
/**
* @notice Initializer.
*
* @param _l1CrossDomainMessenger Address of the L1CrossDomainMessenger contract.
*/
function initialize(address _l1CrossDomainMessenger) external initializer {
function initialize(address _l1CrossDomainMessenger) public initializer {
address[] memory blockedSystemAddresses = new address[](2);
blockedSystemAddresses[0] = address(this);
blockedSystemAddresses[1] = PredeployAddresses.L2_TO_L1_MESSAGE_PASSER;
_initialize(_l1CrossDomainMessenger, blockedSystemAddresses);
__CrossDomainMessenger_init(_l1CrossDomainMessenger, blockedSystemAddresses);
}
/**
......
......@@ -79,12 +79,19 @@ contract L2StandardBridge is StandardBridge {
);
/**
* @notice Initializes the L2StandardBridge.
* @param _otherBridge Address of the L1StandardBridge.
*/
constructor(address payable _otherBridge) {
initialize(_otherBridge);
}
/**
* @notice Initializer.
*
* @param _otherBridge Address of the L1StandardBridge.
*/
function initialize(address payable _otherBridge) public {
_initialize(payable(PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER), _otherBridge);
function initialize(address payable _otherBridge) public initializer {
__StandardBridge_init(payable(PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER), _otherBridge);
}
/**
......
......@@ -190,11 +190,11 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
"OVM_L1CrossDomainMessenger"
);
L1Messenger = L1CrossDomainMessenger(address(proxy));
L1Messenger.initialize(op);
L1Messenger.initialize();
vm.etch(
PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER,
address(new L2CrossDomainMessenger()).code
address(new L2CrossDomainMessenger(address(L1Messenger))).code
);
L2Messenger.initialize(address(L1Messenger));
......@@ -358,18 +358,19 @@ contract Bridge_Initializer is Messenger_Initializer {
// Deploy the L2StandardBridge, move it to the correct predeploy
// address and then initialize it
L2StandardBridge l2B = new L2StandardBridge();
L2StandardBridge l2B = new L2StandardBridge(payable(PredeployAddresses.L2_STANDARD_BRIDGE));
vm.etch(PredeployAddresses.L2_STANDARD_BRIDGE, address(l2B).code);
L2Bridge = L2StandardBridge(payable(PredeployAddresses.L2_STANDARD_BRIDGE));
L2Bridge.initialize(payable(address(L1Bridge)));
// Set up the L2 mintable token factory
OptimismMintableERC20Factory factory = new OptimismMintableERC20Factory();
OptimismMintableERC20Factory factory = new OptimismMintableERC20Factory(
PredeployAddresses.L2_STANDARD_BRIDGE
);
vm.etch(PredeployAddresses.L2_STANDARD_TOKEN_FACTORY, address(factory).code);
L2TokenFactory = OptimismMintableERC20Factory(
PredeployAddresses.L2_STANDARD_TOKEN_FACTORY
);
L2TokenFactory.initialize(PredeployAddresses.L2_STANDARD_BRIDGE);
vm.etch(PredeployAddresses.LEGACY_ERC20_ETH, address(new LegacyERC20ETH()).code);
......@@ -385,8 +386,7 @@ contract Bridge_Initializer is Messenger_Initializer {
);
NativeL2Token = new ERC20("Native L2 Token", "L2T");
L1TokenFactory = new OptimismMintableERC20Factory();
L1TokenFactory.initialize(address(L1Bridge));
L1TokenFactory = new OptimismMintableERC20Factory(address(L1Bridge));
RemoteL1Token = OptimismMintableERC20(
L1TokenFactory.createStandardL2Token(
......
......@@ -16,11 +16,6 @@ contract OptimismMintableTokenFactory_Test is Bridge_Initializer {
super.setUp();
}
function test_initializeShouldRevert() external {
vm.expectRevert("OptimismMintableERC20Factory: already initialized");
L2TokenFactory.initialize(address(L1Bridge));
}
function test_bridge() external {
assertEq(address(L2TokenFactory.bridge()), address(L2Bridge));
}
......
......@@ -312,18 +312,16 @@ abstract contract CrossDomainMessenger is
* detailed information about what this block list can and
* cannot be used for.
*/
function _initialize(address _otherMessenger, address[] memory _blockedSystemAddresses)
internal
{
function __CrossDomainMessenger_init(
address _otherMessenger,
address[] memory _blockedSystemAddresses
) internal onlyInitializing {
xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;
otherMessenger = _otherMessenger;
for (uint256 i = 0; i < _blockedSystemAddresses.length; i++) {
blockedSystemAddresses[_blockedSystemAddresses[i]] = true;
}
// TODO: ensure we know what these are doing and why they are here
// Initialize upgradable OZ contracts
__Context_init_unchained();
__Ownable_init_unchained();
__Pausable_init_unchained();
......
......@@ -41,15 +41,12 @@ contract OptimismMintableERC20Factory {
/**
* @notice Address of the StandardBridge on this chain.
*/
address public bridge;
address public immutable bridge;
/**
* @notice Initializer.
*
* @param _bridge Address of the StandardBridge on this chain.
*/
function initialize(address _bridge) public {
require(bridge == address(0), "OptimismMintableERC20Factory: already initialized");
constructor(address _bridge) {
bridge = _bridge;
}
......
......@@ -5,6 +5,7 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import { Address } from "@openzeppelin/contracts/utils/Address.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import { IRemoteToken, IL1Token } from "./SupportedInterfaces.sol";
import { CrossDomainMessenger } from "./CrossDomainMessenger.sol";
import { OptimismMintableERC20 } from "./OptimismMintableERC20.sol";
......@@ -13,7 +14,7 @@ import { OptimismMintableERC20 } from "./OptimismMintableERC20.sol";
* @title StandardBridge
* @notice StandardBridge is a base contract for the L1 and L2 standard ERC20 bridges.
*/
abstract contract StandardBridge {
abstract contract StandardBridge is Initializable {
using SafeERC20 for IERC20;
/**
......@@ -367,9 +368,10 @@ abstract contract StandardBridge {
* @param _messenger Address of CrossDomainMessenger on this network.
* @param _otherBridge Address of the other StandardBridge contract.
*/
function _initialize(address payable _messenger, address payable _otherBridge) internal {
require(address(messenger) == address(0), "Contract has already been initialized.");
function __StandardBridge_init(address payable _messenger, address payable _otherBridge)
internal
onlyInitializing
{
messenger = CrossDomainMessenger(_messenger);
otherBridge = StandardBridge(_otherBridge);
}
......
......@@ -37,9 +37,7 @@ const deployFn: DeployFunction = async (hre) => {
const upgradeTx = await Proxy.upgradeToAndCall(
messenger.address,
L1CrossDomainMessenger.interface.encodeFunctionData('initialize(address)', [
portal.address,
])
L1CrossDomainMessenger.interface.encodeFunctionData('initialize')
)
await upgradeTx.wait()
......
......@@ -17,15 +17,16 @@ const deployFn: DeployFunction = async (hre) => {
waitConfirmations: deployConfig.deploymentWaitConfirmations,
})
const bridge = await hre.deployments.get('L1StandardBridgeProxy')
await deploy('OptimismMintableERC20Factory', {
from: deployer,
args: [],
args: [bridge.address],
log: true,
waitConfirmations: deployConfig.deploymentWaitConfirmations,
})
const factory = await hre.deployments.get('OptimismMintableERC20Factory')
const bridge = await hre.deployments.get('L1StandardBridgeProxy')
const proxy = await hre.deployments.get('OptimismMintableERC20FactoryProxy')
const Proxy = await hre.ethers.getContractAt('Proxy', proxy.address)
......@@ -34,13 +35,7 @@ const deployFn: DeployFunction = async (hre) => {
proxy.address
)
const upgradeTx = await Proxy.upgradeToAndCall(
factory.address,
OptimismMintableERC20Factory.interface.encodeFunctionData(
'initialize(address)',
[bridge.address]
)
)
const upgradeTx = await Proxy.upgradeTo(factory.address)
await upgradeTx.wait()
if (bridge.address !== (await OptimismMintableERC20Factory.bridge())) {
......
......@@ -3,6 +3,7 @@ import path from 'path'
import assert from 'assert'
import { OptimismGenesis, State } from '@eth-optimism/core-utils'
import 'hardhat-deploy'
import '@eth-optimism/hardhat-deploy-config'
import { ethers } from 'ethers'
import { task } from 'hardhat/config'
......@@ -38,7 +39,8 @@ const getStorageLayout = async (
for (const key of keys) {
if (name === path.basename(key, '.sol')) {
const contract = buildInfo.output.contracts[key]
return (contract[name] as any).storageLayout
const storageLayout = (contract[name] as any).storageLayout
return storageLayout || { storage: [], types: {} }
}
}
throw new Error(`Cannot locate storageLayout for ${name}`)
......@@ -102,9 +104,6 @@ task('genesis-l2', 'create a genesis config')
SequencerFeeVault: {
l1FeeWallet: ethers.constants.AddressZero,
},
OptimismMintableERC20Factory: {
bridge: ethers.constants.AddressZero,
},
L1Block: {
number: deployConfig.l1BlockInitialNumber,
timestamp: deployConfig.l1BlockInitialTimestamp,
......@@ -247,6 +246,9 @@ task('genesis-l2', 'create a genesis config')
}
const storageLayout = await getStorageLayout(hre, name)
if (storageLayout === undefined) {
throw new Error(`cannot find storage layout for ${name}`)
}
const slots = computeStorageSlots(storageLayout, variables[name])
for (const slot of slots) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment