Skip to content

N
nebula
Commit c1422d9b authored by Mark Tyneway's avatar Mark Tyneway
Browse files
Options

contracts-bedrock: remove crossdomain messenger pausability

parent 1b0c6032
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 184 additions and 1407 deletions
packages/contracts-bedrock/.gas-snapshot
View file @ c1422d9b
This diff is collapsed.
packages/contracts-bedrock/.storage-layout
View file @ c1422d9b
......@@ -11,11 +11,11 @@
| spacer_0_0_20 | address | 0 | 0 | 20 | 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 |
| __gap | uint256[50] | 1 | 0 | 1600 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| _owner | address | 51 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[49] | 52 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| _paused | bool | 101 | 0 | 1 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[49] | 102 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| spacer_1_0_1600 | uint256[50] | 1 | 0 | 1600 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| spacer_51_0_20 | address | 51 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| spacer_52_0_1568 | uint256[49] | 52 | 0 | 1568 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| spacer_101_0_1 | bool | 101 | 0 | 1 | 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 |
| __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 |
......@@ -121,11 +121,11 @@
| spacer_0_0_20 | address | 0 | 0 | 20 | 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 |
| __gap | uint256[50] | 1 | 0 | 1600 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| _owner | address | 51 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[49] | 52 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| _paused | bool | 101 | 0 | 1 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[49] | 102 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| spacer_1_0_1600 | uint256[50] | 1 | 0 | 1600 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| spacer_51_0_20 | address | 51 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| spacer_52_0_1568 | uint256[49] | 52 | 0 | 1568 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| spacer_101_0_1 | bool | 101 | 0 | 1 | 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 |
| __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 |
......
packages/contracts-bedrock/contracts/L1/L1CrossDomainMessenger.sol
View file @ c1422d9b
......@@ -29,17 +29,14 @@ contract L1CrossDomainMessenger is CrossDomainMessenger, Semver {
CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER)
{
PORTAL = _portal;
initialize(address(0));
initialize();
}
/**
* @notice Initializer.
*
* @param _owner Address of the initial owner of this contract.
*/
function initialize(address _owner) public initializer {
function initialize() public initializer {
__CrossDomainMessenger_init();
_transferOwnership(_owner);
}
/**
......
packages/contracts-bedrock/contracts/deployment/SystemDictator.sol
View file @ c1422d9b
......@@ -335,7 +335,7 @@ contract SystemDictator is OwnableUpgradeable {
// Try to initialize the L1CrossDomainMessenger, only fail if it's already been initialized.
try
L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy)
.initialize(address(this))
.initialize()
{
// L1CrossDomainMessenger is the one annoying edge case difference between existing
// networks and fresh networks because in existing networks it'll already be
......@@ -375,27 +375,12 @@ contract SystemDictator is OwnableUpgradeable {
payable(config.proxyAddressConfig.l1ERC721BridgeProxy),
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.
*/
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.
config.globalConfig.proxyAdmin.transferOwnership(config.globalConfig.finalOwner);
......
packages/contracts-bedrock/contracts/test/CommonTest.t.sol
View file @ c1422d9b
......@@ -254,7 +254,7 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
"OVM_L1CrossDomainMessenger"
);
L1Messenger = L1CrossDomainMessenger(address(proxy));
L1Messenger.initialize(alice);
L1Messenger.initialize();
vm.etch(
Predeploys.L2_CROSS_DOMAIN_MESSENGER,
......
packages/contracts-bedrock/contracts/test/L1CrossDomainMessenger.t.sol
View file @ c1422d9b
......@@ -25,42 +25,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
// Storage slot of the l2Sender
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
function test_messageVersion_succeeds() external {
(, uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce());
......@@ -274,15 +238,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
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
// fails and ETH gets stuck, but the second message succeeds
function test_relayMessage_retryAfterFailure_succeeds() external {
......
packages/contracts-bedrock/contracts/test/L2CrossDomainMessenger.t.sol
View file @ c1422d9b
......@@ -16,21 +16,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
// Receiver address for testing
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 {
(, uint16 version) = Encoding.decodeVersionedNonce(L2Messenger.messageNonce());
assertEq(version, L2Messenger.MESSAGE_VERSION());
......@@ -191,15 +176,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
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
// fails and ETH gets stuck, but the second message succeeds
function test_relayMessage_retry_succeeds() external {
......
packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol
View file @ c1422d9b
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import {
OwnableUpgradeable
} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import {
PausableUpgradeable
} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import { SafeCall } from "../libraries/SafeCall.sol";
import { Hashing } from "../libraries/Hashing.sol";
import { Encoding } from "../libraries/Encoding.sol";
......@@ -14,12 +9,12 @@ import { Constants } from "../libraries/Constants.sol";
/**
* @custom:legacy
* @title CrossDomainMessengerLegacySpacer
* @title CrossDomainMessengerLegacySpacer0
* @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
* tree of the CrossDomainMessenger
* tree of the CrossDomainMessenger.
*/
contract CrossDomainMessengerLegacySpacer {
contract CrossDomainMessengerLegacySpacer0 {
/**
* @custom:legacy
* @custom:spacer libAddressManager
......@@ -28,6 +23,50 @@ contract CrossDomainMessengerLegacySpacer {
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.
*/
uint256[50] private spacer_1_0_1600;
/**
* @custom:legacy
* @custom:spacer _owner
* @notice Spacer for backwards compatibility.
*/
address private spacer_51_0_20;
/**
* @custom:legacy
* @custom:spacer __gap
* @notice Spacer for backwards compatibility.
*/
uint256[49] private spacer_52_0_1568;
/**
* @custom:legacy
* @custom:spacer _paused
* @notice Spacer for backwards compatibility.
*/
bool private spacer_101_0_1;
/**
* @custom:legacy
* @custom:spacer __gap
* @notice
*/
uint256[49] private spacer_102_0_1568;
}
/**
* @custom:upgradeable
* @title CrossDomainMessenger
......@@ -36,11 +75,13 @@ contract CrossDomainMessengerLegacySpacer {
* 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
* 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
CrossDomainMessengerLegacySpacer,
OwnableUpgradeable,
PausableUpgradeable
CrossDomainMessengerLegacySpacer0,
Initializable,
CrossDomainMessengerLegacySpacer1
{
/**
* @notice Current message version identifier.
......@@ -196,22 +237,6 @@ abstract contract CrossDomainMessenger is
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
* always reverts, then the message will be unrelayable, and any ETH sent will be
......@@ -273,7 +298,7 @@ abstract contract CrossDomainMessenger is
uint256 _value,
uint256 _minGasLimit,
bytes calldata _message
) external payable whenNotPaused {
) external payable {
(, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
require(
version < 2,
......@@ -423,9 +448,6 @@ abstract contract CrossDomainMessenger is
// solhint-disable-next-line func-name-mixedcase
function __CrossDomainMessenger_init() internal onlyInitializing {
xDomainMsgSender = Constants.DEFAULT_L2_SENDER;
__Context_init_unchained();
__Ownable_init_unchained();
__Pausable_init_unchained();
}
/**
......
packages/contracts-bedrock/deploy/021-SystemDictatorSteps-2.ts
View file @ c1422d9b
......@@ -116,7 +116,7 @@ const deployFn: DeployFunction = async (hre) => {
for (const dead of deads) {
assert(
(await AddressManager.getAddress(dead)) ===
ethers.constants.AddressZero
ethers.constants.AddressZero
)
}
},
......@@ -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({
isLiveDeployer,
SystemDictator,
step: 5,
message: `
Step 5 will initialize all Bedrock contracts but will leave the new L1CrossDomainMessenger
paused. After this step is executed, users will be able to deposit and withdraw assets via
the OptimismPortal but not via the L1CrossDomainMessenger. The Proposer will also be able to
submit L2 outputs to the L2OutputOracle.
Step 5 will initialize all Bedrock contracts After this step is executed, the OptimismPortal
will be open for deposits but withdrawals will be paused if deploying a production network.
The Proposer will also be able to submit L2 outputs to the L2OutputOracle.
`,
checks: async () => {
// Check L2OutputOracle was initialized properly.
......@@ -253,7 +252,6 @@ const deployFn: DeployFunction = async (hre) => {
}
// Check L1CrossDomainMessenger was initialized properly.
await assertContractVariable(L1CrossDomainMessenger, 'paused', true)
try {
await L1CrossDomainMessenger.xDomainMessageSender()
assert(false, `L1CrossDomainMessenger was not initialized properly`)
......@@ -292,6 +290,7 @@ const deployFn: DeployFunction = async (hre) => {
},
})
// Step 6 unpauses the OptimismPortal.
if (await isStep(SystemDictator, 6)) {
console.log(`
Unpause the OptimismPortal. The GUARDIAN account should be used. In practice
......@@ -321,21 +320,6 @@ const deployFn: DeployFunction = async (hre) => {
)
}
// Step 6 unpauses the new L1CrossDomainMessenger.
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(`
......
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