Commit d3281892 authored by Mark Tyneway's avatar Mark Tyneway Committed by GitHub

Merge pull request #4921 from ethereum-optimism/feat/portal-pausable

feat: portal pausable
parents a6a14b70 820c230c
This source diff could not be displayed because it is too large. You can view the blob instead.
This diff is collapsed.
......@@ -123,7 +123,8 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
if err != nil {
return nil, err
}
data, err = portalABI.Pack("initialize")
// Initialize the OptimismPortal without being paused
data, err = portalABI.Pack("initialize", false)
if err != nil {
return nil, fmt.Errorf("cannot abi encode initialize for OptimismPortal: %w", err)
}
......@@ -290,9 +291,15 @@ func deployL1Contracts(config *DeployConfig, backend *backends.SimulatedBackend)
},
},
{
// The implementation of the OptimismPortal is deployed
// as being paused to prevent invalid usage of the network
// as only the proxy should be used
Name: "OptimismPortal",
Args: []interface{}{
predeploys.DevL2OutputOracleAddr,
config.FinalSystemOwner,
uint642Big(config.FinalizationPeriodSeconds),
true, // _paused
},
},
{
......@@ -354,8 +361,10 @@ func l1Deployer(backend *backends.SimulatedBackend, opts *bind.TransactOpts, dep
_, tx, _, err = bindings.DeployOptimismPortal(
opts,
backend,
predeploys.DevL2OutputOracleAddr,
deployment.Args[0].(*big.Int),
deployment.Args[0].(common.Address),
deployment.Args[2].(*big.Int),
deployment.Args[1].(common.Address),
deployment.Args[3].(bool),
)
case "L1CrossDomainMessenger":
_, tx, _, err = bindings.DeployL1CrossDomainMessenger(
......
This diff is collapsed.
......@@ -63,6 +63,7 @@
| l2Sender | address | 50 | 0 | 20 | contracts/L1/OptimismPortal.sol:OptimismPortal |
| finalizedWithdrawals | mapping(bytes32 => bool) | 51 | 0 | 32 | contracts/L1/OptimismPortal.sol:OptimismPortal |
| provenWithdrawals | mapping(bytes32 => struct OptimismPortal.ProvenWithdrawal) | 52 | 0 | 32 | contracts/L1/OptimismPortal.sol:OptimismPortal |
| paused | bool | 53 | 0 | 1 | contracts/L1/OptimismPortal.sol:OptimismPortal |
=======================
➡ contracts/L1/SystemConfig.sol:SystemConfig
......
......@@ -58,6 +58,11 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
*/
L2OutputOracle public immutable L2_ORACLE;
/**
* @notice Address that has the ability to pause and unpause deposits and withdrawals.
*/
address public immutable GUARDIAN;
/**
* @notice Address of the L2 account which initiated a withdrawal in this transaction. If the
* of this variable is the default L2 sender address, then we are NOT inside of a call
......@@ -75,6 +80,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
*/
mapping(bytes32 => ProvenWithdrawal) public provenWithdrawals;
/**
* @notice Determines if cross domain messaging is paused. When set to true,
* deposits and withdrawals are paused. This may be removed in the
* future.
*/
bool public paused;
/**
* @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.
......@@ -111,25 +123,74 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
event WithdrawalFinalized(bytes32 indexed withdrawalHash, bool success);
/**
* @custom:semver 1.0.0
* @notice Emitted when the pause is triggered.
*
* @param account Address of the account triggering the pause.
*/
event Paused(address account);
/**
* @notice Emitted when the pause is lifted.
*
* @param account Address of the account triggering the unpause.
*/
event Unpaused(address account);
/**
* @notice Reverts when paused.
*/
modifier whenNotPaused() {
require(paused == false, "OptimismPortal: paused");
_;
}
/**
* @custom:semver 1.1.0
*
* @param _l2Oracle Address of the L2OutputOracle contract.
* @param _guardian Address that can pause deposits and withdrawals.
* @param _finalizationPeriodSeconds Output finalization time in seconds.
* @param _paused Sets the contract's pausability state.
*/
constructor(L2OutputOracle _l2Oracle, uint256 _finalizationPeriodSeconds) Semver(1, 0, 0) {
constructor(
L2OutputOracle _l2Oracle,
uint256 _finalizationPeriodSeconds,
address _guardian,
bool _paused
) Semver(1, 1, 0) {
L2_ORACLE = _l2Oracle;
GUARDIAN = _guardian;
FINALIZATION_PERIOD_SECONDS = _finalizationPeriodSeconds;
initialize();
initialize(_paused);
}
/**
* @notice Initializer.
*/
function initialize() public initializer {
function initialize(bool _paused) public initializer {
l2Sender = Constants.DEFAULT_L2_SENDER;
paused = _paused;
__ResourceMetering_init();
}
/**
* @notice Pause deposits and withdrawals.
*/
function pause() external {
require(msg.sender == GUARDIAN, "OptimismPortal: only guardian can pause");
paused = true;
emit Paused(msg.sender);
}
/**
* @notice Unpause deposits and withdrawals.
*/
function unpause() external {
require(msg.sender == GUARDIAN, "OptimismPortal: only guardian can unpause");
paused = false;
emit Unpaused(msg.sender);
}
/**
* @notice Accepts value so that users can send ETH directly to this contract and have the
* funds be deposited to their address on L2. This is intended as a convenience
......@@ -162,7 +223,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
uint256 _l2OutputIndex,
Types.OutputRootProof calldata _outputRootProof,
bytes[] calldata _withdrawalProof
) external {
) external whenNotPaused {
// Prevent users from creating a deposit transaction where this address is the message
// sender on L2. Because this is checked here, we do not need to check again in
// `finalizeWithdrawalTransaction`.
......@@ -240,7 +301,10 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
*
* @param _tx Withdrawal transaction to finalize.
*/
function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external {
function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx)
external
whenNotPaused
{
// Make sure that the l2Sender has not yet been set. The l2Sender is set to a value other
// than the default value when a withdrawal transaction is being finalized. This check is
// a defacto reentrancy guard.
......
......@@ -111,6 +111,12 @@ contract SystemDictator is OwnableUpgradeable {
*/
L2OutputOracleDynamicConfig public l2OutputOracleDynamicConfig;
/**
* @notice Dynamic configuration for the OptimismPortal. Determines
* if the system should be paused when initialized.
*/
bool public optimismPortalDynamicConfig;
/**
* @notice Current step;
*/
......@@ -160,14 +166,17 @@ contract SystemDictator is OwnableUpgradeable {
}
/**
* @notice Allows the owner to update dynamic L2OutputOracle config.
* @notice Allows the owner to update dynamic config.
*
* @param _l2OutputOracleDynamicConfig Dynamic L2OutputOracle config.
* @param _optimismPortalDynamicConfig Dynamic OptimismPortal config.
*/
function updateL2OutputOracleDynamicConfig(
L2OutputOracleDynamicConfig memory _l2OutputOracleDynamicConfig
function updateDynamicConfig(
L2OutputOracleDynamicConfig memory _l2OutputOracleDynamicConfig,
bool _optimismPortalDynamicConfig
) external onlyOwner {
l2OutputOracleDynamicConfig = _l2OutputOracleDynamicConfig;
optimismPortalDynamicConfig = _optimismPortalDynamicConfig;
dynamicConfigSet = true;
}
......@@ -314,7 +323,7 @@ contract SystemDictator is OwnableUpgradeable {
config.globalConfig.proxyAdmin.upgradeAndCall(
payable(config.proxyAddressConfig.optimismPortalProxy),
address(config.implementationAddressConfig.optimismPortalImpl),
abi.encodeCall(OptimismPortal.initialize, ())
abi.encodeCall(OptimismPortal.initialize, (optimismPortalDynamicConfig))
);
// Upgrade the L1CrossDomainMessenger.
......
......@@ -9,7 +9,12 @@ contract EchidnaFuzzOptimismPortal {
bool internal failedToComplete;
constructor() {
portal = new OptimismPortal(L2OutputOracle(address(0)), 10);
portal = new OptimismPortal({
_l2Oracle: L2OutputOracle(address(0)),
_guardian: address(0),
_finalizationPeriodSeconds: 10,
_paused: false
});
}
// A test intended to identify any unexpected halting conditions
......
......@@ -99,6 +99,7 @@ contract L2OutputOracle_Initializer is CommonTest {
uint256 internal l2BlockTime = 2;
uint256 internal startingBlockNumber = 200;
uint256 internal startingTimestamp = 1000;
address guardian;
// Test data
uint256 initL1Time;
......@@ -119,6 +120,8 @@ contract L2OutputOracle_Initializer is CommonTest {
function setUp() public virtual override {
super.setUp();
guardian = makeAddr("guardian");
// By default the first block has timestamp and number zero, which will cause underflows in the
// tests, so we'll move forward to these block values.
initL1Time = startingTimestamp + 1;
......@@ -164,12 +167,17 @@ contract Portal_Initializer is L2OutputOracle_Initializer {
function setUp() public virtual override {
super.setUp();
opImpl = new OptimismPortal(oracle, 7 days);
opImpl = new OptimismPortal({
_l2Oracle: oracle,
_guardian: guardian,
_finalizationPeriodSeconds: 7 days,
_paused: true
});
Proxy proxy = new Proxy(multisig);
vm.prank(multisig);
proxy.upgradeToAndCall(
address(opImpl),
abi.encodeWithSelector(OptimismPortal.initialize.selector)
abi.encodeWithSelector(OptimismPortal.initialize.selector, false)
);
op = OptimismPortal(payable(address(proxy)));
}
......@@ -223,7 +231,12 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
super.setUp();
// Deploy the OptimismPortal
op = new OptimismPortal(oracle, 7 days);
op = new OptimismPortal({
_l2Oracle: oracle,
_guardian: guardian,
_finalizationPeriodSeconds: 7 days,
_paused: false
});
vm.label(address(op), "OptimismPortal");
// Deploy the address manager
......
......@@ -11,10 +11,83 @@ import { Hashing } from "../libraries/Hashing.sol";
import { Proxy } from "../universal/Proxy.sol";
contract OptimismPortal_Test is Portal_Initializer {
event Paused(address);
event Unpaused(address);
function test_constructor_succeeds() external {
assertEq(op.FINALIZATION_PERIOD_SECONDS(), 7 days);
assertEq(address(op.L2_ORACLE()), address(oracle));
assertEq(op.l2Sender(), 0x000000000000000000000000000000000000dEaD);
assertEq(op.paused(), false);
}
/**
* @notice The OptimismPortal can be paused by the GUARDIAN
*/
function test_pause_succeeds() external {
address guardian = op.GUARDIAN();
assertEq(op.paused(), false);
vm.expectEmit(true, true, true, true, address(op));
emit Paused(guardian);
vm.prank(guardian);
op.pause();
assertEq(op.paused(), true);
}
/**
* @notice The OptimismPortal reverts when an account that is not the
* GUARDIAN calls `pause()`
*/
function test_pause_onlyGuardian_reverts() external {
assertEq(op.paused(), false);
assertTrue(op.GUARDIAN() != alice);
vm.expectRevert("OptimismPortal: only guardian can pause");
vm.prank(alice);
op.pause();
assertEq(op.paused(), false);
}
/**
* @notice The OptimismPortal can be unpaused by the GUARDIAN
*/
function test_unpause_succeeds() external {
address guardian = op.GUARDIAN();
vm.prank(guardian);
op.pause();
assertEq(op.paused(), true);
vm.expectEmit(true, true, true, true, address(op));
emit Unpaused(guardian);
vm.prank(guardian);
op.unpause();
assertEq(op.paused(), false);
}
/**
* @notice The OptimismPortal reverts when an account that is not
* the GUARDIAN calls `unpause()`
*/
function test_unpause_onlyGuardian_reverts() external {
address guardian = op.GUARDIAN();
vm.prank(guardian);
op.pause();
assertEq(op.paused(), true);
assertTrue(op.GUARDIAN() != alice);
vm.expectRevert("OptimismPortal: only guardian can unpause");
vm.prank(alice);
op.unpause();
assertEq(op.paused(), true);
}
function test_receive_succeeds() external {
......@@ -343,6 +416,22 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
assertFalse(op.finalizedWithdrawals(Hashing.hashWithdrawal(_defaultTx)));
}
/**
* @notice Proving withdrawal transactions should revert when paused
*/
function test_proveWithdrawalTransaction_paused_reverts() external {
vm.prank(op.GUARDIAN());
op.pause();
vm.expectRevert("OptimismPortal: paused");
op.proveWithdrawalTransaction({
_tx: _defaultTx,
_l2OutputIndex: _proposedOutputIndex,
_outputRootProof: _outputRootProof,
_withdrawalProof: _withdrawalProof
});
}
// Test: proveWithdrawalTransaction cannot prove a withdrawal with itself (the OptimismPortal) as the target.
function test_proveWithdrawalTransaction_onSelfCall_reverts() external {
_defaultTx.target = address(op);
......@@ -539,6 +628,17 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
assert(address(bob).balance == bobBalanceBefore + 100);
}
/**
* @notice Finalizing withdrawal transactions should revert when paused
*/
function test_finalizeWithdrawalTransaction_paused_reverts() external {
vm.prank(op.GUARDIAN());
op.pause();
vm.expectRevert("OptimismPortal: paused");
op.finalizeWithdrawalTransaction(_defaultTx);
}
// Test: finalizeWithdrawalTransaction reverts if the withdrawal has not been proven.
function test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() external {
uint256 bobBalanceBefore = address(bob).balance;
......@@ -956,12 +1056,12 @@ contract OptimismPortalUpgradeable_Test is Portal_Initializer {
function test_initialize_cannotInitProxy_reverts() external {
vm.expectRevert("Initializable: contract is already initialized");
OptimismPortal(payable(proxy)).initialize();
OptimismPortal(payable(proxy)).initialize(false);
}
function test_initialize_cannotInitImpl_reverts() external {
vm.expectRevert("Initializable: contract is already initialized");
OptimismPortal(opImpl).initialize();
OptimismPortal(opImpl).initialize(false);
}
function test_upgradeToAndCall_upgrading_succeeds() external {
......
......@@ -8,17 +8,42 @@ import {
} from '../src/deploy-utils'
const deployFn: DeployFunction = async (hre) => {
const { deployer } = await hre.getNamedAccounts()
const isLiveDeployer =
deployer.toLowerCase() === hre.deployConfig.controller.toLowerCase()
const L2OutputOracleProxy = await getContractFromArtifact(
hre,
'L2OutputOracleProxy'
)
const finalSystemOwner = hre.deployConfig.finalSystemOwner
const finalSystemOwnerCode = await hre.ethers.provider.getCode(
finalSystemOwner
)
if (finalSystemOwnerCode === '0x') {
console.log(
`WARNING: setting OptimismPortal.GUARDIAN to ${finalSystemOwner} and it has no code`
)
if (!isLiveDeployer) {
throw new Error(
'Do not deploy to production networks without the GUARDIAN being a contract'
)
}
}
// Deploy the OptimismPortal implementation as paused to
// ensure that users do not interact with it and instead
// interact with the proxied contract.
// The `finalSystemOwner` is set at the GUARDIAN.
await deploy({
hre,
name: 'OptimismPortal',
args: [
L2OutputOracleProxy.address,
hre.deployConfig.finalizationPeriodSeconds,
finalSystemOwner,
true, // paused
],
postDeployAction: async (contract) => {
await assertContractVariable(
......@@ -31,6 +56,11 @@ const deployFn: DeployFunction = async (hre) => {
'FINALIZATION_PERIOD_SECONDS',
hre.deployConfig.finalizationPeriodSeconds
)
await assertContractVariable(
contract,
'GUARDIAN',
hre.deployConfig.finalSystemOwner
)
},
})
}
......
......@@ -171,21 +171,24 @@ const deployFn: DeployFunction = async (hre) => {
deployL2StartingTimestamp = l1StartingBlock.timestamp
}
await SystemDictator.updateL2OutputOracleDynamicConfig({
l2OutputOracleStartingBlockNumber:
hre.deployConfig.l2OutputOracleStartingBlockNumber,
l2OutputOracleStartingTimestamp: deployL2StartingTimestamp,
})
await SystemDictator.updateDynamicConfig(
{
l2OutputOracleStartingBlockNumber:
hre.deployConfig.l2OutputOracleStartingBlockNumber,
l2OutputOracleStartingTimestamp: deployL2StartingTimestamp,
},
false // do not pause the the OptimismPortal when initializing
)
} else {
const tx =
await SystemDictator.populateTransaction.updateL2OutputOracleDynamicConfig(
{
l2OutputOracleStartingBlockNumber:
hre.deployConfig.l2OutputOracleStartingBlockNumber,
l2OutputOracleStartingTimestamp:
hre.deployConfig.l2OutputOracleStartingTimestamp,
}
)
const tx = await SystemDictator.populateTransaction.updateDynamicConfig(
{
l2OutputOracleStartingBlockNumber:
hre.deployConfig.l2OutputOracleStartingBlockNumber,
l2OutputOracleStartingTimestamp:
hre.deployConfig.l2OutputOracleStartingTimestamp,
},
true
)
console.log(`Please update dynamic oracle config...`)
console.log(`MSD address: ${SystemDictator.address}`)
console.log(`JSON:`)
......@@ -243,6 +246,12 @@ const deployFn: DeployFunction = async (hre) => {
(await hre.ethers.provider.getBalance(L1StandardBridge.address)).eq(0)
)
if (isLiveDeployer) {
await assertContractVariable(OptimismPortal, 'paused', false)
} else {
await assertContractVariable(OptimismPortal, 'paused', true)
}
// Check L1CrossDomainMessenger was initialized properly.
await assertContractVariable(L1CrossDomainMessenger, 'paused', true)
try {
......@@ -283,6 +292,35 @@ const deployFn: DeployFunction = async (hre) => {
},
})
if (await isStep(SystemDictator, 6)) {
console.log(`
Unpause the OptimismPortal. The GUARDIAN account should be used. In practice
this is the multisig. In test networks, the OptimismPortal is initialized
without being paused.
`)
if (isLiveDeployer) {
console.log('WARNING: OptimismPortal configured to not be paused')
console.log('This should only happen for test environments')
await assertContractVariable(OptimismPortal, 'paused', false)
} else {
const tx = await OptimismPortal.populateTransaction.unpause()
console.log(`Please unpause the OptimismPortal...`)
console.log(`OptimismPortal address: ${OptimismPortal.address}`)
console.log(`JSON:`)
console.log(jsonifyTransaction(tx))
}
await awaitCondition(
async () => {
const paused = await OptimismPortal.paused()
return !paused
},
30000,
1000
)
}
// Step 6 unpauses the new L1CrossDomainMessenger.
await doStep({
isLiveDeployer,
......
......@@ -19,6 +19,6 @@ This invariant asserts that there is no chain of calls that can be made that wil
## Deposits of any value should always succeed unless `_to` = `address(0)` or `_isCreation` = `true`.
**Test:** [`FuzzOptimismPortal.sol#L37`](../contracts/echidna/FuzzOptimismPortal.sol#L37)
**Test:** [`FuzzOptimismPortal.sol#L42`](../contracts/echidna/FuzzOptimismPortal.sol#L42)
All deposits, barring creation transactions and transactions sent to `address(0)`, should always succeed.
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