diff --git a/src/bridge/AbsBridge.sol b/src/bridge/AbsBridge.sol index 88577d2d..bafd56f9 100644 --- a/src/bridge/AbsBridge.sol +++ b/src/bridge/AbsBridge.sol @@ -67,6 +67,12 @@ abstract contract AbsBridge is Initializable, DelegateCallAware, IBridge { _; } + /// @notice Allows the rollup owner to set another rollup address + function updateRollupAddress(IOwnable _rollup) external onlyRollupOrOwner { + rollup = _rollup; + emit RollupUpdated(address(_rollup)); + } + /// @dev returns the address of current active Outbox, or zero if no outbox is active function activeOutbox() public view returns (address) { address outbox = _activeOutbox; diff --git a/src/bridge/AbsOutbox.sol b/src/bridge/AbsOutbox.sol index 2678f1fb..4dabf9ff 100644 --- a/src/bridge/AbsOutbox.sol +++ b/src/bridge/AbsOutbox.sol @@ -13,7 +13,8 @@ import { AlreadySpent, BridgeCallFailed, HadZeroInit, - BadPostUpgradeInit + BadPostUpgradeInit, + RollupNotChanged } from "../libraries/Error.sol"; import "./IBridge.sol"; import "./IOutbox.sol"; @@ -90,6 +91,15 @@ abstract contract AbsOutbox is DelegateCallAware, IOutbox { }); } + /// @notice Allows the rollup owner to sync the rollup address + function updateRollupAddress() external { + if (msg.sender != IOwnable(rollup).owner()) + revert NotOwner(msg.sender, IOwnable(rollup).owner()); + address newRollup = address(bridge.rollup()); + if (rollup == newRollup) revert RollupNotChanged(); + rollup = newRollup; + } + function updateSendRoot(bytes32 root, bytes32 l2BlockHash) external { if (msg.sender != rollup) revert NotRollup(msg.sender, rollup); roots[root] = l2BlockHash; diff --git a/src/bridge/IBridge.sol b/src/bridge/IBridge.sol index 49c3c13f..22388b4b 100644 --- a/src/bridge/IBridge.sol +++ b/src/bridge/IBridge.sol @@ -32,6 +32,8 @@ interface IBridge { event SequencerInboxUpdated(address newSequencerInbox); + event RollupUpdated(address rollup); + function allowedDelayedInboxList(uint256) external returns (address); function allowedOutboxList(uint256) external returns (address); @@ -97,4 +99,6 @@ interface IBridge { function setDelayedInbox(address inbox, bool enabled) external; function setOutbox(address inbox, bool enabled) external; + + function updateRollupAddress(IOwnable _rollup) external; } diff --git a/src/bridge/IOutbox.sol b/src/bridge/IOutbox.sol index be888634..41d0b594 100644 --- a/src/bridge/IOutbox.sol +++ b/src/bridge/IOutbox.sol @@ -31,6 +31,8 @@ interface IOutbox { function updateSendRoot(bytes32 sendRoot, bytes32 l2BlockHash) external; + function updateRollupAddress() external; + /// @notice When l2ToL1Sender returns a nonzero address, the message was originated by an L2 account /// When the return value is zero, that means this is a system message /// @dev the l2ToL1Sender behaves as the tx.origin, the msg.sender should be validated to protect against reentrancies diff --git a/src/bridge/ISequencerInbox.sol b/src/bridge/ISequencerInbox.sol index b4fadddb..7d66befc 100644 --- a/src/bridge/ISequencerInbox.sol +++ b/src/bridge/ISequencerInbox.sol @@ -177,4 +177,6 @@ interface ISequencerInbox is IDelayedMessageProvider { // ---------- initializer ---------- function initialize(IBridge bridge_, MaxTimeVariation calldata maxTimeVariation_) external; + + function updateRollupAddress() external; } diff --git a/src/bridge/SequencerInbox.sol b/src/bridge/SequencerInbox.sol index c1cb745f..51d511e2 100644 --- a/src/bridge/SequencerInbox.sol +++ b/src/bridge/SequencerInbox.sol @@ -20,7 +20,8 @@ import { DataNotAuthenticated, AlreadyValidDASKeyset, NoSuchKeyset, - NotForked + NotForked, + RollupNotChanged } from "../libraries/Error.sol"; import "./IBridge.sol"; import "./IInboxBase.sol"; @@ -91,6 +92,15 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox maxTimeVariation = maxTimeVariation_; } + /// @notice Allows the rollup owner to sync the rollup address + function updateRollupAddress() external { + if (msg.sender != IOwnable(rollup).owner()) + revert NotOwner(msg.sender, IOwnable(rollup).owner()); + IOwnable newRollup = bridge.rollup(); + if (rollup == newRollup) revert RollupNotChanged(); + rollup = newRollup; + } + function getTimeBounds() internal view virtual returns (TimeBounds memory) { TimeBounds memory bounds; if (block.timestamp > maxTimeVariation.delaySeconds) { diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol index e4776d80..fd6f3255 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -175,3 +175,6 @@ error AlreadyValidDASKeyset(bytes32); /// @dev Tried to use or invalidate an already invalid Data Availability Service keyset error NoSuchKeyset(bytes32); + +/// @dev Thrown when rollup is not updated with updateRollupAddress +error RollupNotChanged(); diff --git a/src/mocks/BridgeStub.sol b/src/mocks/BridgeStub.sol index 37c994e4..2e2dee05 100644 --- a/src/mocks/BridgeStub.sol +++ b/src/mocks/BridgeStub.sol @@ -45,6 +45,10 @@ contract BridgeStub is IBridge, IEthBridge { revert("NOT_IMPLEMENTED"); } + function updateRollupAddress(IOwnable) external pure { + revert("NOT_IMPLEMENTED"); + } + function enqueueDelayedMessage( uint8 kind, address sender, diff --git a/src/rollup/AbsRollupEventInbox.sol b/src/rollup/AbsRollupEventInbox.sol index b2f89fcd..c0866d9e 100644 --- a/src/rollup/AbsRollupEventInbox.sol +++ b/src/rollup/AbsRollupEventInbox.sol @@ -12,7 +12,7 @@ import "../libraries/ArbitrumChecker.sol"; import "../bridge/IDelayedMessageProvider.sol"; import "../libraries/DelegateCallAware.sol"; import {INITIALIZATION_MSG_TYPE} from "../libraries/MessageTypes.sol"; -import {AlreadyInit, HadZeroInit} from "../libraries/Error.sol"; +import {AlreadyInit, HadZeroInit, RollupNotChanged} from "../libraries/Error.sol"; /** * @title The inbox for rollup protocol events @@ -37,6 +37,15 @@ abstract contract AbsRollupEventInbox is rollup = address(_bridge.rollup()); } + /// @notice Allows the rollup owner to sync the rollup address + function updateRollupAddress() external { + if (msg.sender != IOwnable(rollup).owner()) + revert NotOwner(msg.sender, IOwnable(rollup).owner()); + address newRollup = address(bridge.rollup()); + if (rollup == newRollup) revert RollupNotChanged(); + rollup = newRollup; + } + function rollupInitialized(uint256 chainId, string calldata chainConfig) external override diff --git a/src/rollup/IRollupEventInbox.sol b/src/rollup/IRollupEventInbox.sol index beb1b4ed..2e79f7e6 100644 --- a/src/rollup/IRollupEventInbox.sol +++ b/src/rollup/IRollupEventInbox.sol @@ -13,5 +13,7 @@ interface IRollupEventInbox { function rollup() external view returns (address); + function updateRollupAddress() external; + function rollupInitialized(uint256 chainId, string calldata chainConfig) external; } diff --git a/src/rollup/RollupUserLogic.sol b/src/rollup/RollupUserLogic.sol index bd16ad5e..9807f201 100644 --- a/src/rollup/RollupUserLogic.sol +++ b/src/rollup/RollupUserLogic.sol @@ -27,6 +27,11 @@ abstract contract AbsRollupUserLogic is _; } + modifier whenNotPausedOrDeprecated() { + require(!paused() || address(bridge.rollup()) != address(this), "PAUSED_AND_ACTIVE"); + _; + } + uint256 internal immutable deployTimeChainId = block.chainid; function _chainIdChanged() internal view returns (bool) { @@ -234,7 +239,12 @@ abstract contract AbsRollupUserLogic is * and move it to the desired node. * @param stakerAddress Address of the staker whose stake is refunded */ - function returnOldDeposit(address stakerAddress) external override onlyValidator whenNotPaused { + function returnOldDeposit(address stakerAddress) + external + override + onlyValidator + whenNotPausedOrDeprecated + { require(latestStakedNode(stakerAddress) <= latestConfirmed(), "TOO_RECENT"); requireUnchallengedStaker(stakerAddress); withdrawStaker(stakerAddress); @@ -258,7 +268,7 @@ abstract contract AbsRollupUserLogic is * @notice Reduce the amount staked for the sender (difference between initial amount staked and target is creditted back to the sender). * @param target Target amount of stake for the staker. If this is below the current minimum, it will be set to minimum instead */ - function reduceDeposit(uint256 target) external onlyValidator whenNotPaused { + function reduceDeposit(uint256 target) external onlyValidator whenNotPausedOrDeprecated { requireUnchallengedStaker(msg.sender); uint256 currentRequired = currentRequiredStake(); if (target < currentRequired) { @@ -659,7 +669,13 @@ contract RollupUserLogic is AbsRollupUserLogic, IRollupUser { /** * @notice Withdraw uncommitted funds owned by sender from the rollup chain */ - function withdrawStakerFunds() external override onlyValidator whenNotPaused returns (uint256) { + function withdrawStakerFunds() + external + override + onlyValidator + whenNotPausedOrDeprecated + returns (uint256) + { uint256 amount = withdrawFunds(msg.sender); // This is safe because it occurs after all checks and effects // solhint-disable-next-line avoid-low-level-calls @@ -731,7 +747,13 @@ contract ERC20RollupUserLogic is AbsRollupUserLogic, IRollupUserERC20 { /** * @notice Withdraw uncommitted funds owned by sender from the rollup chain */ - function withdrawStakerFunds() external override onlyValidator whenNotPaused returns (uint256) { + function withdrawStakerFunds() + external + override + onlyValidator + whenNotPausedOrDeprecated + returns (uint256) + { uint256 amount = withdrawFunds(msg.sender); // This is safe because it occurs after all checks and effects require(IERC20Upgradeable(stakeToken).transfer(msg.sender, amount), "TRANSFER_FAILED"); diff --git a/src/test-helpers/BridgeTester.sol b/src/test-helpers/BridgeTester.sol index bd1fc7da..0bcbe00f 100644 --- a/src/test-helpers/BridgeTester.sol +++ b/src/test-helpers/BridgeTester.sol @@ -74,6 +74,10 @@ contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge { rollup = rollup_; } + function updateRollupAddress(IOwnable _rollup) external { + rollup = _rollup; + } + function activeOutbox() public view returns (address) { if (_activeOutbox == EMPTY_ACTIVEOUTBOX) return address(uint160(0)); return _activeOutbox; diff --git a/src/test-helpers/OutboxWithoutOptTester.sol b/src/test-helpers/OutboxWithoutOptTester.sol index 0036e3c1..89dcc1e1 100644 --- a/src/test-helpers/OutboxWithoutOptTester.sol +++ b/src/test-helpers/OutboxWithoutOptTester.sol @@ -56,6 +56,10 @@ contract OutboxWithoutOptTester is DelegateCallAware, IOutbox { emit SendRootUpdated(root, l2BlockHash); } + function updateRollupAddress() external onlyDelegated onlyProxyOwner { + rollup = address(bridge.rollup()); + } + /// @notice When l2ToL1Sender returns a nonzero address, the message was originated by an L2 account /// When the return value is zero, that means this is a system message /// @dev the l2ToL1Sender behaves as the tx.origin, the msg.sender should be validated to protect against reentrancies diff --git a/test/contract/arbRollup.spec.ts b/test/contract/arbRollup.spec.ts index fca5eb97..ac836d1d 100644 --- a/test/contract/arbRollup.spec.ts +++ b/test/contract/arbRollup.spec.ts @@ -46,6 +46,7 @@ import { RollupUserLogic__factory, SequencerInbox, SequencerInbox__factory, + Bridge, } from '../../build/types' import { abi as UpgradeExecutorABI, @@ -86,6 +87,7 @@ const wasmModuleRoot = let rollup: RollupContract let rollupUser: RollupUserLogic let rollupAdmin: RollupAdminLogic +let bridge: Bridge let accounts: Signer[] let validators: Signer[] let sequencerInbox: SequencerInbox @@ -93,6 +95,7 @@ let admin: Signer let sequencer: Signer let challengeManager: ChallengeManager let upgradeExecutor: string +let adminproxy: string async function getDefaultConfig( _confirmPeriodBlocks = confirmationPeriodBlocks @@ -275,6 +278,7 @@ const setup = async () => { await val1.getAddress(), await val2.getAddress(), await val3.getAddress(), + await val4.getAddress(), ], maxDataSize: 117964, nativeToken: ethers.constants.AddressZero, @@ -298,6 +302,7 @@ const setup = async () => { const rollupUser = rollupUserLogicFac .attach(rollupCreatedEvent.rollupAddress) .connect(user) + const bridge = ethBridgeFac.attach(rollupCreatedEvent.bridge).connect(user) sequencerInbox = ( (await ethers.getContractFactory( @@ -328,8 +333,9 @@ const setup = async () => { sequencerInbox: rollupCreatedEvent.sequencerInbox, delayedBridge: rollupCreatedEvent.bridge, delayedInbox: rollupCreatedEvent.inboxAddress, - bridge: rollupCreatedEvent.bridge, + bridge, upgradeExecutorAddress: rollupCreatedEvent.upgradeExecutor, + adminproxy: rollupCreatedEvent.adminProxy, } } @@ -509,15 +515,19 @@ describe('ArbRollup', () => { const { rollupAdmin: rollupAdminContract, rollupUser: rollupUserContract, + bridge: bridgeContract, admin: adminI, validators: validatorsI, upgradeExecutorAddress, + adminproxy: adminproxyAddress, } = await setup() rollupAdmin = rollupAdminContract rollupUser = rollupUserContract + bridge = bridgeContract admin = adminI validators = validatorsI upgradeExecutor = upgradeExecutorAddress + adminproxy = adminproxyAddress rollup = new RollupContract(rollupUser.connect(validators[0])) }) @@ -580,6 +590,9 @@ describe('ArbRollup', () => { await rollupUser .connect(validators[2]) .newStakeOnExistingNode(1, prevNode.nodeHash, { value: stake }) + await rollupUser + .connect(validators[3]) + .newStakeOnExistingNode(1, prevNode.nodeHash, { value: stake }) }) it('should move stake to a new node', async function () { @@ -789,6 +802,44 @@ describe('ArbRollup', () => { await rollup.confirmNextNode(challengerNode) }) + it('allow force refund staker with pending node', async function () { + await rollupAdmin.connect(await impersonateAccount(upgradeExecutor)).pause() + await ( + await rollupAdmin + .connect(await impersonateAccount(upgradeExecutor)) + .forceRefundStaker([await validators[3].getAddress()]) + ).wait() + + await expect( + rollup.rollup.connect(validators[3]).withdrawStakerFunds() + ).to.be.revertedWith('PAUSED_AND_ACTIVE') + // staker can only withdraw if rollup address changed when paused + await bridge + .connect(await impersonateAccount(rollup.rollup.address)) + .updateRollupAddress(ethers.constants.AddressZero) + + await ( + await rollup.rollup.connect(validators[3]).withdrawStakerFunds() + ).wait() + await rollupAdmin + .connect(await impersonateAccount(upgradeExecutor)) + .resume() + + // restore rollup address + await bridge + .connect(await impersonateAccount(ethers.constants.AddressZero)) + .updateRollupAddress(rollupUser.address) + + const postWithdrawablefunds = await rollup.rollup.withdrawableFunds( + await validators[3].getAddress() + ) + expect(postWithdrawablefunds, 'withdrawable funds').to.equal(0) + const stake = await rollup.rollup.amountStaked( + await validators[3].getAddress() + ) + expect(stake, 'amount staked').to.equal(0) + }) + it('should add and remove stakes correctly', async function () { /* RollupUser functions that alter stake and their respective Core logic