Commit 8969e46b authored by Matthew Slipper's avatar Matthew Slipper Committed by GitHub

Merge pull request #4913 from ethereum-optimism/feat/ctb-remove-xdm-pause

contracts-bedrock: remove crossdomain messenger pausability
parents 10cecba1 608ebc9a
...@@ -414,9 +414,7 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage ...@@ -414,9 +414,7 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage
} }
storage["L2CrossDomainMessenger"] = state.StorageValues{ storage["L2CrossDomainMessenger"] = state.StorageValues{
"_initialized": 1, "_initialized": 1,
"_owner": config.ProxyAdminOwner,
"_initializing": false, "_initializing": false,
"_paused": false,
"xDomainMsgSender": "0x000000000000000000000000000000000000dEaD", "xDomainMsgSender": "0x000000000000000000000000000000000000dEaD",
"msgNonce": 0, "msgNonce": 0,
} }
......
...@@ -141,10 +141,7 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) { ...@@ -141,10 +141,7 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
data, err = l1XDMABI.Pack( data, err = l1XDMABI.Pack("initialize")
"initialize",
config.FinalSystemOwner,
)
if err != nil { if err != nil {
return nil, fmt.Errorf("cannot abi encode initialize for L1CrossDomainMessenger: %w", err) return nil, fmt.Errorf("cannot abi encode initialize for L1CrossDomainMessenger: %w", err)
} }
......
This diff is collapsed.
...@@ -11,11 +11,11 @@ ...@@ -11,11 +11,11 @@
| spacer_0_0_20 | address | 0 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_0_0_20 | address | 0 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| _initialized | uint8 | 0 | 20 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | _initialized | uint8 | 0 | 20 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| _initializing | bool | 0 | 21 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | _initializing | bool | 0 | 21 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[50] | 1 | 0 | 1600 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_1_0_1600 | uint256[50] | 1 | 0 | 1600 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| _owner | address | 51 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_51_0_20 | address | 51 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[49] | 52 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_52_0_1568 | uint256[49] | 52 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| _paused | bool | 101 | 0 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_101_0_1 | bool | 101 | 0 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[49] | 102 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_102_0_1568 | uint256[49] | 102 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| spacer_151_0_32 | uint256 | 151 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_151_0_32 | uint256 | 151 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap_reentrancy_guard | uint256[49] | 152 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | __gap_reentrancy_guard | uint256[49] | 152 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| spacer_201_0_32 | mapping(bytes32 => bool) | 201 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | spacer_201_0_32 | mapping(bytes32 => bool) | 201 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
...@@ -121,11 +121,11 @@ ...@@ -121,11 +121,11 @@
| spacer_0_0_20 | address | 0 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_0_0_20 | address | 0 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| _initialized | uint8 | 0 | 20 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | _initialized | uint8 | 0 | 20 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| _initializing | bool | 0 | 21 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | _initializing | bool | 0 | 21 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[50] | 1 | 0 | 1600 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_1_0_1600 | uint256[50] | 1 | 0 | 1600 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| _owner | address | 51 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_51_0_20 | address | 51 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[49] | 52 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_52_0_1568 | uint256[49] | 52 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| _paused | bool | 101 | 0 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_101_0_1 | bool | 101 | 0 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[49] | 102 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_102_0_1568 | uint256[49] | 102 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| spacer_151_0_32 | uint256 | 151 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_151_0_32 | uint256 | 151 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap_reentrancy_guard | uint256[49] | 152 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | __gap_reentrancy_guard | uint256[49] | 152 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| spacer_201_0_32 | mapping(bytes32 => bool) | 201 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | spacer_201_0_32 | mapping(bytes32 => bool) | 201 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
......
...@@ -29,17 +29,14 @@ contract L1CrossDomainMessenger is CrossDomainMessenger, Semver { ...@@ -29,17 +29,14 @@ contract L1CrossDomainMessenger is CrossDomainMessenger, Semver {
CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER) CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER)
{ {
PORTAL = _portal; PORTAL = _portal;
initialize(address(0)); initialize();
} }
/** /**
* @notice Initializer. * @notice Initializer.
*
* @param _owner Address of the initial owner of this contract.
*/ */
function initialize(address _owner) public initializer { function initialize() public initializer {
__CrossDomainMessenger_init(); __CrossDomainMessenger_init();
_transferOwnership(_owner);
} }
/** /**
......
...@@ -335,7 +335,7 @@ contract SystemDictator is OwnableUpgradeable { ...@@ -335,7 +335,7 @@ contract SystemDictator is OwnableUpgradeable {
// Try to initialize the L1CrossDomainMessenger, only fail if it's already been initialized. // Try to initialize the L1CrossDomainMessenger, only fail if it's already been initialized.
try try
L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy) L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy)
.initialize(address(this)) .initialize()
{ {
// L1CrossDomainMessenger is the one annoying edge case difference between existing // L1CrossDomainMessenger is the one annoying edge case difference between existing
// networks and fresh networks because in existing networks it'll already be // networks and fresh networks because in existing networks it'll already be
...@@ -375,27 +375,12 @@ contract SystemDictator is OwnableUpgradeable { ...@@ -375,27 +375,12 @@ contract SystemDictator is OwnableUpgradeable {
payable(config.proxyAddressConfig.l1ERC721BridgeProxy), payable(config.proxyAddressConfig.l1ERC721BridgeProxy),
address(config.implementationAddressConfig.l1ERC721BridgeImpl) address(config.implementationAddressConfig.l1ERC721BridgeImpl)
); );
// Pause the L1CrossDomainMessenger, chance to check that everything is OK.
L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy).pause();
}
/**
* @notice Unpauses the system at which point the system should be fully operational.
*/
function step6() external onlyOwner step(6) {
// Unpause the L1CrossDomainMessenger.
L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy).unpause();
} }
/** /**
* @notice Tranfers admin ownership to the final owner. * @notice Tranfers admin ownership to the final owner.
*/ */
function finalize() external onlyOwner { function finalize() external onlyOwner {
// Transfer ownership of the L1CrossDomainMessenger to the final owner.
L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy)
.transferOwnership(config.globalConfig.finalOwner);
// Transfer ownership of the ProxyAdmin to the final owner. // Transfer ownership of the ProxyAdmin to the final owner.
config.globalConfig.proxyAdmin.transferOwnership(config.globalConfig.finalOwner); config.globalConfig.proxyAdmin.transferOwnership(config.globalConfig.finalOwner);
......
...@@ -254,7 +254,7 @@ contract Messenger_Initializer is L2OutputOracle_Initializer { ...@@ -254,7 +254,7 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
"OVM_L1CrossDomainMessenger" "OVM_L1CrossDomainMessenger"
); );
L1Messenger = L1CrossDomainMessenger(address(proxy)); L1Messenger = L1CrossDomainMessenger(address(proxy));
L1Messenger.initialize(alice); L1Messenger.initialize();
vm.etch( vm.etch(
Predeploys.L2_CROSS_DOMAIN_MESSENGER, Predeploys.L2_CROSS_DOMAIN_MESSENGER,
......
...@@ -25,42 +25,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer { ...@@ -25,42 +25,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
// Storage slot of the l2Sender // Storage slot of the l2Sender
uint256 constant senderSlotIndex = 50; uint256 constant senderSlotIndex = 50;
function setUp() public override {
super.setUp();
}
// pause: should pause the contract when called by the current owner
function test_pause_succeeds() external {
vm.prank(alice);
L1Messenger.pause();
assert(L1Messenger.paused());
}
// pause: should not pause the contract when called by account other than the owner
function test_pause_callerIsNotOwner_reverts() external {
vm.expectRevert("Ownable: caller is not the owner");
vm.prank(address(0xABBA));
L1Messenger.pause();
}
// unpause: should unpause the contract when called by the current owner
function test_unpause_succeeds() external {
vm.prank(alice);
L1Messenger.pause();
assert(L1Messenger.paused());
vm.prank(alice);
L1Messenger.unpause();
assert(!L1Messenger.paused());
}
// unpause: should not unpause the contract when called by account other than the owner
function test_unpause_callerIsNotOwner_reverts() external {
vm.expectRevert("Ownable: caller is not the owner");
vm.prank(address(0xABBA));
L1Messenger.unpause();
}
// the version is encoded in the nonce // the version is encoded in the nonce
function test_messageVersion_succeeds() external { function test_messageVersion_succeeds() external {
(, uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce()); (, uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce());
...@@ -274,15 +238,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer { ...@@ -274,15 +238,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
L1Messenger.xDomainMessageSender(); L1Messenger.xDomainMessageSender();
} }
// relayMessage: should revert if paused
function test_relayMessage_paused_reverts() external {
vm.prank(L1Messenger.owner());
L1Messenger.pause();
vm.expectRevert("Pausable: paused");
L1Messenger.relayMessage(0, address(0), address(0), 0, 0, hex"");
}
// relayMessage: should send a successful call to the target contract after the first message // relayMessage: should send a successful call to the target contract after the first message
// fails and ETH gets stuck, but the second message succeeds // fails and ETH gets stuck, but the second message succeeds
function test_relayMessage_retryAfterFailure_succeeds() external { function test_relayMessage_retryAfterFailure_succeeds() external {
......
...@@ -16,21 +16,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer { ...@@ -16,21 +16,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
// Receiver address for testing // Receiver address for testing
address recipient = address(0xabbaacdc); address recipient = address(0xabbaacdc);
function setUp() public override {
super.setUp();
}
function test_pause_succeeds() external {
L2Messenger.pause();
assert(L2Messenger.paused());
}
function test_pause_notOwner_reverts() external {
vm.expectRevert("Ownable: caller is not the owner");
vm.prank(address(0xABBA));
L2Messenger.pause();
}
function test_messageVersion_succeeds() external { function test_messageVersion_succeeds() external {
(, uint16 version) = Encoding.decodeVersionedNonce(L2Messenger.messageNonce()); (, uint16 version) = Encoding.decodeVersionedNonce(L2Messenger.messageNonce());
assertEq(version, L2Messenger.MESSAGE_VERSION()); assertEq(version, L2Messenger.MESSAGE_VERSION());
...@@ -191,15 +176,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer { ...@@ -191,15 +176,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
L2Messenger.xDomainMessageSender(); L2Messenger.xDomainMessageSender();
} }
// relayMessage: should revert if paused
function test_relayMessage_paused_reverts() external {
vm.prank(L2Messenger.owner());
L2Messenger.pause();
vm.expectRevert("Pausable: paused");
L2Messenger.relayMessage(0, address(0), address(0), 0, 0, hex"");
}
// relayMessage: should send a successful call to the target contract after the first message // relayMessage: should send a successful call to the target contract after the first message
// fails and ETH gets stuck, but the second message succeeds // fails and ETH gets stuck, but the second message succeeds
function test_relayMessage_retry_succeeds() external { function test_relayMessage_retry_succeeds() external {
......
// SPDX-License-Identifier: MIT // SPDX-License-Identifier: MIT
pragma solidity 0.8.15; pragma solidity 0.8.15;
import { import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
OwnableUpgradeable
} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import {
PausableUpgradeable
} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import { SafeCall } from "../libraries/SafeCall.sol"; import { SafeCall } from "../libraries/SafeCall.sol";
import { Hashing } from "../libraries/Hashing.sol"; import { Hashing } from "../libraries/Hashing.sol";
import { Encoding } from "../libraries/Encoding.sol"; import { Encoding } from "../libraries/Encoding.sol";
...@@ -14,12 +9,12 @@ import { Constants } from "../libraries/Constants.sol"; ...@@ -14,12 +9,12 @@ import { Constants } from "../libraries/Constants.sol";
/** /**
* @custom:legacy * @custom:legacy
* @title CrossDomainMessengerLegacySpacer * @title CrossDomainMessengerLegacySpacer0
* @notice Contract only exists to add a spacer to the CrossDomainMessenger where the * @notice Contract only exists to add a spacer to the CrossDomainMessenger where the
* libAddressManager variable used to exist. Must be the first contract in the inheritance * libAddressManager variable used to exist. Must be the first contract in the inheritance
* tree of the CrossDomainMessenger * tree of the CrossDomainMessenger.
*/ */
contract CrossDomainMessengerLegacySpacer { contract CrossDomainMessengerLegacySpacer0 {
/** /**
* @custom:legacy * @custom:legacy
* @custom:spacer libAddressManager * @custom:spacer libAddressManager
...@@ -28,6 +23,83 @@ contract CrossDomainMessengerLegacySpacer { ...@@ -28,6 +23,83 @@ contract CrossDomainMessengerLegacySpacer {
address private spacer_0_0_20; address private spacer_0_0_20;
} }
/**
* @custom:legacy
* @title CrossDomainMessengerLegacySpacer1
* @notice Contract only exists to add a spacer to the CrossDomainMessenger where the
* PausableUpgradable and OwnableUpgradeable variables used to exist. Must be
* the third contract in the inheritance tree of the CrossDomainMessenger.
*/
contract CrossDomainMessengerLegacySpacer1 {
/**
* @custom:legacy
* @custom:spacer __gap
* @notice Spacer for backwards compatibility. Comes from OpenZeppelin
* ContextUpgradable via OwnableUpgradeable.
*
*/
uint256[50] private spacer_1_0_1600;
/**
* @custom:legacy
* @custom:spacer _owner
* @notice Spacer for backwards compatibility.
* Come from OpenZeppelin OwnableUpgradeable.
*/
address private spacer_51_0_20;
/**
* @custom:legacy
* @custom:spacer __gap
* @notice Spacer for backwards compatibility. Comes from OpenZeppelin
* ContextUpgradable via PausableUpgradable.
*/
uint256[49] private spacer_52_0_1568;
/**
* @custom:legacy
* @custom:spacer _paused
* @notice Spacer for backwards compatibility. Comes from OpenZeppelin
* PausableUpgradable.
*/
bool private spacer_101_0_1;
/**
* @custom:legacy
* @custom:spacer __gap
* @notice Spacer for backwards compatibility. Comes from OpenZeppelin
* PausableUpgradable.
*/
uint256[49] private spacer_102_0_1568;
/**
* @custom:legacy
* @custom:spacer ReentrancyGuardUpgradeable's `_status` field.
* @notice Spacer for backwards compatibility
*/
uint256 private spacer_151_0_32;
/**
* @custom:spacer ReentrancyGuardUpgradeable
* @notice Spacer for backwards compatibility
*/
uint256[49] private __gap_reentrancy_guard;
/**
* @custom:legacy
* @custom:spacer blockedMessages
* @notice Spacer for backwards compatibility.
*/
mapping(bytes32 => bool) private spacer_201_0_32;
/**
* @custom:legacy
* @custom:spacer relayedMessages
* @notice Spacer for backwards compatibility.
*/
mapping(bytes32 => bool) private spacer_202_0_32;
}
/** /**
* @custom:upgradeable * @custom:upgradeable
* @title CrossDomainMessenger * @title CrossDomainMessenger
...@@ -36,11 +108,13 @@ contract CrossDomainMessengerLegacySpacer { ...@@ -36,11 +108,13 @@ contract CrossDomainMessengerLegacySpacer {
* needs to be extended slightly to provide low-level message passing functionality on each * needs to be extended slightly to provide low-level message passing functionality on each
* chain it's deployed on. Currently only designed for message passing between two paired * chain it's deployed on. Currently only designed for message passing between two paired
* chains and does not support one-to-many interactions. * chains and does not support one-to-many interactions.
*
* Any changes to this contract MUST result in a semver bump for contracts that inherit it.
*/ */
abstract contract CrossDomainMessenger is abstract contract CrossDomainMessenger is
CrossDomainMessengerLegacySpacer, CrossDomainMessengerLegacySpacer0,
OwnableUpgradeable, Initializable,
PausableUpgradeable CrossDomainMessengerLegacySpacer1
{ {
/** /**
* @notice Current message version identifier. * @notice Current message version identifier.
...@@ -82,32 +156,6 @@ abstract contract CrossDomainMessenger is ...@@ -82,32 +156,6 @@ abstract contract CrossDomainMessenger is
*/ */
address public immutable OTHER_MESSENGER; address public immutable OTHER_MESSENGER;
/**
* @custom:spacer ReentrancyGuardUpgradeable's `_status` field.
* @notice Spacer for backwards compatibility
*/
uint256 private spacer_151_0_32;
/**
* @custom:spacer ReentrancyGuardUpgradeable
* @notice Spacer for backwards compatibility
*/
uint256[49] private __gap_reentrancy_guard;
/**
* @custom:legacy
* @custom:spacer blockedMessages
* @notice Spacer for backwards compatibility.
*/
mapping(bytes32 => bool) private spacer_201_0_32;
/**
* @custom:legacy
* @custom:spacer relayedMessages
* @notice Spacer for backwards compatibility.
*/
mapping(bytes32 => bool) private spacer_202_0_32;
/** /**
* @notice Mapping of message hashes to boolean receipt values. Note that a message will only * @notice Mapping of message hashes to boolean receipt values. Note that a message will only
* be present in this mapping if it has successfully been relayed on this chain, and * be present in this mapping if it has successfully been relayed on this chain, and
...@@ -196,22 +244,6 @@ abstract contract CrossDomainMessenger is ...@@ -196,22 +244,6 @@ abstract contract CrossDomainMessenger is
OTHER_MESSENGER = _otherMessenger; OTHER_MESSENGER = _otherMessenger;
} }
/**
* @notice Allows the owner of this contract to temporarily pause message relaying. Backup
* security mechanism just in case. Owner should be the same as the upgrade wallet to
* maintain the security model of the system as a whole.
*/
function pause() external onlyOwner {
_pause();
}
/**
* @notice Allows the owner of this contract to resume message relaying once paused.
*/
function unpause() external onlyOwner {
_unpause();
}
/** /**
* @notice Sends a message to some target address on the other chain. Note that if the call * @notice Sends a message to some target address on the other chain. Note that if the call
* always reverts, then the message will be unrelayable, and any ETH sent will be * always reverts, then the message will be unrelayable, and any ETH sent will be
...@@ -273,7 +305,7 @@ abstract contract CrossDomainMessenger is ...@@ -273,7 +305,7 @@ abstract contract CrossDomainMessenger is
uint256 _value, uint256 _value,
uint256 _minGasLimit, uint256 _minGasLimit,
bytes calldata _message bytes calldata _message
) external payable whenNotPaused { ) external payable {
(, uint16 version) = Encoding.decodeVersionedNonce(_nonce); (, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
require( require(
version < 2, version < 2,
...@@ -423,9 +455,6 @@ abstract contract CrossDomainMessenger is ...@@ -423,9 +455,6 @@ abstract contract CrossDomainMessenger is
// solhint-disable-next-line func-name-mixedcase // solhint-disable-next-line func-name-mixedcase
function __CrossDomainMessenger_init() internal onlyInitializing { function __CrossDomainMessenger_init() internal onlyInitializing {
xDomainMsgSender = Constants.DEFAULT_L2_SENDER; xDomainMsgSender = Constants.DEFAULT_L2_SENDER;
__Context_init_unchained();
__Ownable_init_unchained();
__Pausable_init_unchained();
} }
/** /**
......
import { ethers } from 'ethers'
import { DeployFunction } from 'hardhat-deploy/dist/types' import { DeployFunction } from 'hardhat-deploy/dist/types'
import { import {
...@@ -23,11 +22,6 @@ const deployFn: DeployFunction = async (hre) => { ...@@ -23,11 +22,6 @@ const deployFn: DeployFunction = async (hre) => {
'PORTAL', 'PORTAL',
OptimismPortalProxy.address OptimismPortalProxy.address
) )
await assertContractVariable(
contract,
'owner',
ethers.constants.AddressZero
)
}, },
}) })
} }
......
...@@ -204,16 +204,15 @@ const deployFn: DeployFunction = async (hre) => { ...@@ -204,16 +204,15 @@ const deployFn: DeployFunction = async (hre) => {
) )
} }
// Step 5 initializes all contracts and pauses the new L1CrossDomainMessenger. // Step 5 initializes all contracts.
await doStep({ await doStep({
isLiveDeployer, isLiveDeployer,
SystemDictator, SystemDictator,
step: 5, step: 5,
message: ` message: `
Step 5 will initialize all Bedrock contracts but will leave the new L1CrossDomainMessenger Step 5 will initialize all Bedrock contracts. After this step is executed, the OptimismPortal
paused. After this step is executed, users will be able to deposit and withdraw assets via will be open for deposits but withdrawals will be paused if deploying a production network.
the OptimismPortal but not via the L1CrossDomainMessenger. The Proposer will also be able to The Proposer will also be able to submit L2 outputs to the L2OutputOracle.
submit L2 outputs to the L2OutputOracle.
`, `,
checks: async () => { checks: async () => {
// Check L2OutputOracle was initialized properly. // Check L2OutputOracle was initialized properly.
...@@ -253,18 +252,12 @@ const deployFn: DeployFunction = async (hre) => { ...@@ -253,18 +252,12 @@ const deployFn: DeployFunction = async (hre) => {
} }
// Check L1CrossDomainMessenger was initialized properly. // Check L1CrossDomainMessenger was initialized properly.
await assertContractVariable(L1CrossDomainMessenger, 'paused', true)
try { try {
await L1CrossDomainMessenger.xDomainMessageSender() await L1CrossDomainMessenger.xDomainMessageSender()
assert(false, `L1CrossDomainMessenger was not initialized properly`) assert(false, `L1CrossDomainMessenger was not initialized properly`)
} catch (err) { } catch (err) {
// Expected. // Expected.
} }
await assertContractVariable(
L1CrossDomainMessenger,
'owner',
SystemDictator.address
)
// Check L1StandardBridge was initialized properly. // Check L1StandardBridge was initialized properly.
await assertContractVariable( await assertContractVariable(
...@@ -292,6 +285,7 @@ const deployFn: DeployFunction = async (hre) => { ...@@ -292,6 +285,7 @@ const deployFn: DeployFunction = async (hre) => {
}, },
}) })
// Step 6 unpauses the OptimismPortal.
if (await isStep(SystemDictator, 6)) { if (await isStep(SystemDictator, 6)) {
console.log(` console.log(`
Unpause the OptimismPortal. The GUARDIAN account should be used. In practice Unpause the OptimismPortal. The GUARDIAN account should be used. In practice
...@@ -319,29 +313,13 @@ const deployFn: DeployFunction = async (hre) => { ...@@ -319,29 +313,13 @@ const deployFn: DeployFunction = async (hre) => {
30000, 30000,
1000 1000
) )
}
// Step 6 unpauses the new L1CrossDomainMessenger. await assertContractVariable(OptimismPortal, 'paused', false)
await doStep({
isLiveDeployer,
SystemDictator,
step: 6,
message: `
Step 6 will unpause the new L1CrossDomainMessenger. After this step is executed, users will
be able to deposit and withdraw assets via the L1CrossDomainMessenger and the system will be
fully operational.
`,
checks: async () => {
await assertContractVariable(L1CrossDomainMessenger, 'paused', false)
},
})
// At the end we finalize the upgrade.
if (await isStep(SystemDictator, 7)) {
console.log(` console.log(`
You must now finalize the upgrade by calling finalize() on the SystemDictator. This will You must now finalize the upgrade by calling finalize() on the SystemDictator. This will
transfer ownership of the ProxyAdmin and the L1CrossDomainMessenger to the final system owner transfer ownership of the ProxyAdmin to the final system owner as specified in the deployment
as specified in the deployment configuration. configuration.
`) `)
if (isLiveDeployer) { if (isLiveDeployer) {
...@@ -363,11 +341,6 @@ const deployFn: DeployFunction = async (hre) => { ...@@ -363,11 +341,6 @@ const deployFn: DeployFunction = async (hre) => {
1000 1000
) )
await assertContractVariable(
L1CrossDomainMessenger,
'owner',
hre.deployConfig.finalSystemOwner
)
await assertContractVariable( await assertContractVariable(
ProxyAdmin, ProxyAdmin,
'owner', 'owner',
......
...@@ -5,6 +5,31 @@ ...@@ -5,6 +5,31 @@
"offset": 0, "offset": 0,
"length": 20 "length": 20
}, },
"spacer_1_0_1600": {
"slot": 1,
"offset": 0,
"length": 1600
},
"spacer_51_0_20": {
"slot": 51,
"offset": 0,
"length": 20
},
"spacer_52_0_1568": {
"slot": 52,
"offset": 0,
"length": 1568
},
"spacer_101_0_1": {
"slot": 101,
"offset": 0,
"length": 1
},
"spacer_102_0_1568": {
"slot": 102,
"offset": 0,
"length": 1568
},
"spacer_151_0_32": { "spacer_151_0_32": {
"slot": 151, "slot": 151,
"offset": 0, "offset": 0,
...@@ -27,6 +52,31 @@ ...@@ -27,6 +52,31 @@
"offset": 0, "offset": 0,
"length": 20 "length": 20
}, },
"spacer_1_0_1600": {
"slot": 1,
"offset": 0,
"length": 1600
},
"spacer_51_0_20": {
"slot": 51,
"offset": 0,
"length": 20
},
"spacer_52_0_1568": {
"slot": 52,
"offset": 0,
"length": 1568
},
"spacer_101_0_1": {
"slot": 101,
"offset": 0,
"length": 1
},
"spacer_102_0_1568": {
"slot": 102,
"offset": 0,
"length": 1568
},
"spacer_151_0_32": { "spacer_151_0_32": {
"slot": 151, "slot": 151,
"offset": 0, "offset": 0,
......
...@@ -269,10 +269,6 @@ const check = { ...@@ -269,10 +269,6 @@ const check = {
await checkProxy(hre, 'L2CrossDomainMessenger', signer.provider) await checkProxy(hre, 'L2CrossDomainMessenger', signer.provider)
await assertProxy(hre, 'L2CrossDomainMessenger', signer.provider) await assertProxy(hre, 'L2CrossDomainMessenger', signer.provider)
const owner = await L2CrossDomainMessenger.owner()
assert(owner !== hre.ethers.constants.AddressZero)
yell(` - owner: ${owner}`)
const MESSAGE_VERSION = await L2CrossDomainMessenger.MESSAGE_VERSION() const MESSAGE_VERSION = await L2CrossDomainMessenger.MESSAGE_VERSION()
console.log(` - MESSAGE_VERSION: ${MESSAGE_VERSION}`) console.log(` - MESSAGE_VERSION: ${MESSAGE_VERSION}`)
const MIN_GAS_CALLDATA_OVERHEAD = const MIN_GAS_CALLDATA_OVERHEAD =
......
import { task } from 'hardhat/config' import { task } from 'hardhat/config'
import { parseFullyQualifiedName } from 'hardhat/utils/contract-names'
import { HardhatRuntimeEnvironment } from 'hardhat/types'
import layoutLock from '../layout-lock.json' import layoutLock from '../layout-lock.json'
// Artifacts that should be skipped when inspecting their storage layout
const skipped = {
// Both of these are skipped because they are meant to be inherited
// by the CrossDomainMessenger. It is the CrossDomainMessenger that
// should be inspected, not these contracts.
CrossDomainMessengerLegacySpacer0: true,
CrossDomainMessengerLegacySpacer1: true,
}
/** /**
* Parses out variable info from the variable structure in standard compiler json output. * Parses out variable info from the variable structure in standard compiler json output.
* *
...@@ -28,8 +39,25 @@ const parseVariableInfo = ( ...@@ -28,8 +39,25 @@ const parseVariableInfo = (
variableLength = variable.type.match(/uint([0-9]+)/)?.[1] variableLength = variable.type.match(/uint([0-9]+)/)?.[1]
} else if (variable.type.startsWith('t_address')) { } else if (variable.type.startsWith('t_address')) {
variableLength = 20 variableLength = 20
} else if (variable.type.startsWith('t_bool')) {
variableLength = 1
} else if (variable.type.startsWith('t_array')) {
// Figure out the size of the type inside of the array
// and then multiply that by the length of the array.
// This does not support recursion multiple times for simplicity
const type = variable.type.match(/^t_array\((\w+)\)/)?.[1]
const info = parseVariableInfo({
label: variable.label,
offset: variable.offset,
slot: variable.slot,
type,
})
const size = variable.type.match(/^t_array\(\w+\)([0-9]+)/)?.[1]
variableLength = info.length * parseInt(size, 10)
} else { } else {
throw new Error('unsupported type, add it to the script') throw new Error(
`${variable.label}: unsupported type ${variable.type}, add it to the script`
)
} }
return { return {
...@@ -43,31 +71,50 @@ const parseVariableInfo = ( ...@@ -43,31 +71,50 @@ const parseVariableInfo = (
task( task(
'validate-spacers', 'validate-spacers',
'validates that spacer variables are in the correct positions' 'validates that spacer variables are in the correct positions'
).setAction(async (args, hre) => { ).setAction(async ({}, hre: HardhatRuntimeEnvironment) => {
const accounted: string[] = [] const accounted: string[] = []
const names = await hre.artifacts.getAllFullyQualifiedNames() const names = await hre.artifacts.getAllFullyQualifiedNames()
for (const name of names) { for (const fqn of names) {
// Script is remarkably slow because of getBuildInfo, so better to skip test files since they // Script is remarkably slow because of getBuildInfo, so better to skip test files since they
// don't matter for this check. // don't matter for this check.
if (name.includes('.t.sol')) { if (fqn.includes('.t.sol')) {
continue
}
console.log(`Processing ${fqn}`)
const parsed = parseFullyQualifiedName(fqn)
const contractName = parsed.contractName
if (skipped[contractName] === true) {
console.log(`Skipping ${contractName} because it is marked as skippable`)
continue continue
} }
// Some files may not have buildInfo (certain libraries). We can safely skip these because we // Some files may not have buildInfo (certain libraries). We can safely skip these because we
// make sure that everything is accounted for anyway. // make sure that everything is accounted for anyway.
const buildInfo = await hre.artifacts.getBuildInfo(name) const buildInfo = await hre.artifacts.getBuildInfo(fqn)
if (buildInfo === undefined) { if (buildInfo === undefined) {
console.log(`Skipping ${name} because it has no buildInfo`) console.log(`Skipping ${fqn} because it has no buildInfo`)
continue continue
} }
for (const source of Object.values(buildInfo.output.contracts)) { const sources = buildInfo.output.contracts
for (const [contractName, contract] of Object.entries(source)) { for (const [sourceName, source] of Object.entries(sources)) {
// The source file may have our contract
if (sourceName.includes(parsed.sourceName)) {
const contract = source[contractName]
if (!contract) {
console.log(`Skipping ${contractName} as its not found in the source`)
continue
}
const storageLayout = (contract as any).storageLayout const storageLayout = (contract as any).storageLayout
if (!storageLayout) {
continue
}
// Check that the layout lock is respected.
if (layoutLock[contractName]) { if (layoutLock[contractName]) {
console.log(`Processing layout lock for ${contractName}`)
const removed = Object.entries(layoutLock[contractName]).filter( const removed = Object.entries(layoutLock[contractName]).filter(
([key, val]: any) => { ([key, val]: any) => {
const storage = storageLayout?.storage || [] const storage = storageLayout?.storage || []
...@@ -80,6 +127,7 @@ task( ...@@ -80,6 +127,7 @@ task(
// Make sure the variable matches **exactly**. // Make sure the variable matches **exactly**.
const variableInfo = parseVariableInfo(item) const variableInfo = parseVariableInfo(item)
return ( return (
variableInfo.name === key && variableInfo.name === key &&
variableInfo.offset === val.offset && variableInfo.offset === val.offset &&
...@@ -95,11 +143,10 @@ task( ...@@ -95,11 +143,10 @@ task(
`variable(s) removed from ${contractName}: ${removed.join(', ')}` `variable(s) removed from ${contractName}: ${removed.join(', ')}`
) )
} }
console.log(`Valid layout lock for ${contractName}`)
accounted.push(contractName) accounted.push(contractName)
} }
// Check that the positions have not changed.
for (const variable of storageLayout?.storage || []) { for (const variable of storageLayout?.storage || []) {
if (variable.label.startsWith('spacer_')) { if (variable.label.startsWith('spacer_')) {
const [, slot, offset, length] = variable.label.split('_') const [, slot, offset, length] = variable.label.split('_')
...@@ -125,6 +172,8 @@ task( ...@@ -125,6 +172,8 @@ task(
`${contractName} ${variable.label} is ${variableInfo.length} bytes long but should be ${length}` `${contractName} ${variable.label} is ${variableInfo.length} bytes long but should be ${length}`
) )
} }
console.log(`${contractName}.${variable.label} is valid`)
} }
} }
} }
......
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