From 7a95ffa7b6e116c534b3822ea0d6dc66e609b1c8 Mon Sep 17 00:00:00 2001 From: QUAQ Date: Tue, 3 Dec 2024 19:54:23 -0600 Subject: [PATCH 1/3] update: rollup creator behavior --- src/rollup/RollupCreator.sol | 48 +++++++++++++++++++++----------- test/foundry/RollupCreator.t.sol | 7 +++-- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/rollup/RollupCreator.sol b/src/rollup/RollupCreator.sol index 840cf2df..7e5de91e 100644 --- a/src/rollup/RollupCreator.sol +++ b/src/rollup/RollupCreator.sol @@ -31,7 +31,6 @@ contract RollupCreator is Ownable { address validatorUtils, address validatorWalletCreator ); - event TemplatesUpdated(); struct RollupDeploymentParams { Config config; @@ -45,24 +44,27 @@ contract RollupCreator is Ownable { address eigenDARollupManager; } - BridgeCreator public bridgeCreator; - IOneStepProofEntry public osp; - IChallengeManager public challengeManagerTemplate; - IRollupAdmin public rollupAdminLogic; - IRollupUser public rollupUserLogic; - IUpgradeExecutor public upgradeExecutorLogic; + modifier onlyUnfrozen() { + require(!deploymentFrozen, "Deployment no longer permitted from this creator"); + _; + } - address public validatorUtils; - address public validatorWalletCreator; + BridgeCreator public immutable bridgeCreator; + IOneStepProofEntry public immutable osp; + IChallengeManager public immutable challengeManagerTemplate; + IRollupAdmin public immutable rollupAdminLogic; + IRollupUser public immutable rollupUserLogic; + IUpgradeExecutor public immutable upgradeExecutorLogic; - DeployHelper public l2FactoriesDeployer; + address public immutable validatorUtils; + address public immutable validatorWalletCreator; - constructor() Ownable() {} + DeployHelper public immutable l2FactoriesDeployer; - // creator receives back excess fees (for deploying L2 factories) so it can refund the caller - receive() external payable {} + bool public deploymentFrozen; + string public version; - function setTemplates( + constructor( BridgeCreator _bridgeCreator, IOneStepProofEntry _osp, IChallengeManager _challengeManagerLogic, @@ -71,8 +73,10 @@ contract RollupCreator is Ownable { IUpgradeExecutor _upgradeExecutorLogic, address _validatorUtils, address _validatorWalletCreator, - DeployHelper _l2FactoriesDeployer - ) external onlyOwner { + DeployHelper _l2FactoriesDeployer, + address _creatorOwner, + string memory _version + ) Ownable() { bridgeCreator = _bridgeCreator; osp = _osp; challengeManagerTemplate = _challengeManagerLogic; @@ -82,7 +86,16 @@ contract RollupCreator is Ownable { validatorUtils = _validatorUtils; validatorWalletCreator = _validatorWalletCreator; l2FactoriesDeployer = _l2FactoriesDeployer; - emit TemplatesUpdated(); + + _transferOwnership(_creatorOwner); + version = _version; + } + + // creator receives back excess fees (for deploying L2 factories) so it can refund the caller + receive() external payable {} + + function freezeDeployment() external onlyOwner { + deploymentFrozen = true; } /** @@ -111,6 +124,7 @@ contract RollupCreator is Ownable { function createRollup(RollupDeploymentParams memory deployParams) public payable + onlyUnfrozen returns (address) { { diff --git a/test/foundry/RollupCreator.t.sol b/test/foundry/RollupCreator.t.sol index 4f45bea7..9d239c85 100644 --- a/test/foundry/RollupCreator.t.sol +++ b/test/foundry/RollupCreator.t.sol @@ -56,7 +56,6 @@ contract RollupCreatorTest is Test { function setUp() public { //// deploy rollup creator and set templates vm.startPrank(deployer); - rollupCreator = new RollupCreator(); deployHelper = new DeployHelper(); // deploy BridgeCreators @@ -75,7 +74,7 @@ contract RollupCreatorTest is Test { rollupUser = _rollupUser; //// deploy creator and set logic - rollupCreator.setTemplates( + rollupCreator = new RollupCreator( bridgeCreator, ospEntry, challengeManager, @@ -84,7 +83,9 @@ contract RollupCreatorTest is Test { upgradeExecutorLogic, address(new ValidatorUtils()), address(new ValidatorWalletCreator()), - deployHelper + deployHelper, + deployer, + "4.2.0" ); vm.stopPrank(); From 978e5df8163566b34298112f97db5362d46de69f Mon Sep 17 00:00:00 2001 From: QUAQ Date: Wed, 4 Dec 2024 11:12:33 -0600 Subject: [PATCH 2/3] unit test --- src/rollup/RollupCreator.sol | 2 +- test/foundry/RollupCreator.t.sol | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/rollup/RollupCreator.sol b/src/rollup/RollupCreator.sol index 7e5de91e..98bdc83e 100644 --- a/src/rollup/RollupCreator.sol +++ b/src/rollup/RollupCreator.sol @@ -45,7 +45,7 @@ contract RollupCreator is Ownable { } modifier onlyUnfrozen() { - require(!deploymentFrozen, "Deployment no longer permitted from this creator"); + require(!deploymentFrozen, "Deployment no longer permitted from this RollupCreator"); _; } diff --git a/test/foundry/RollupCreator.t.sol b/test/foundry/RollupCreator.t.sol index 9d239c85..aa36a5d9 100644 --- a/test/foundry/RollupCreator.t.sol +++ b/test/foundry/RollupCreator.t.sol @@ -487,6 +487,67 @@ contract RollupCreatorTest is Test { assertEq(_getImpl(inbox), address(newLogicImpl)); } + function test_freeze() public { + vm.startPrank(deployer); + + rollupCreator.freezeDeployment(); + assertTrue(rollupCreator.deploymentFrozen(), "rollupCreator not frozen"); + + ISequencerInbox.MaxTimeVariation memory timeVars = ISequencerInbox.MaxTimeVariation( + ((60 * 60 * 24) / 15), + 12, + 60 * 60 * 24, + 60 * 60 + ); + Config memory config = Config({ + confirmPeriodBlocks: 20, + extraChallengeTimeBlocks: 200, + stakeToken: address(0), + baseStake: 1000, + wasmModuleRoot: keccak256("wasm"), + owner: rollupOwner, + loserStakeEscrow: address(200), + chainId: 1337, + chainConfig: "abc", + genesisBlockNum: 15_000_000, + sequencerInboxMaxTimeVariation: timeVars + }); + + // prepare funds + uint256 factoryDeploymentFunds = 1 ether; + vm.deal(deployer, factoryDeploymentFunds); + + /// deploy rollup + address[] memory batchPosters = new address[](1); + batchPosters[0] = makeAddr("batch poster 1"); + address batchPosterManager = makeAddr("batch poster manager"); + address[] memory validators = new address[](2); + validators[0] = makeAddr("validator1"); + validators[1] = makeAddr("validator2"); + + address eigenDARollupManager = makeAddr("rollupManager"); + + RollupCreator.RollupDeploymentParams memory deployParams = RollupCreator + .RollupDeploymentParams({ + config: config, + batchPosters: batchPosters, + validators: validators, + maxDataSize: MAX_DATA_SIZE, + nativeToken: address(0), + deployFactoriesToL2: true, + maxFeePerGasForRetryables: MAX_FEE_PER_GAS, + batchPosterManager: batchPosterManager, + eigenDARollupManager: eigenDARollupManager + }); + + vm.expectRevert("Deployment no longer permitted from this RollupCreator"); + rollupCreator.createRollup{value: factoryDeploymentFunds}( + deployParams + ); + + vm.stopPrank(); + } + function _prepareRollupDeployment() internal returns ( From 118ecc66d5f22c12e105ba9b02e3c18d5049c9da Mon Sep 17 00:00:00 2001 From: QUAQ Date: Thu, 5 Dec 2024 10:34:48 -0600 Subject: [PATCH 3/3] revert to old template setting --- src/rollup/RollupCreator.sol | 56 ++++++++------- test/foundry/RollupCreator.t.sol | 113 ++++++++++++++++++++----------- 2 files changed, 105 insertions(+), 64 deletions(-) diff --git a/src/rollup/RollupCreator.sol b/src/rollup/RollupCreator.sol index 98bdc83e..c1130425 100644 --- a/src/rollup/RollupCreator.sol +++ b/src/rollup/RollupCreator.sol @@ -31,6 +31,7 @@ contract RollupCreator is Ownable { address validatorUtils, address validatorWalletCreator ); + event TemplatesUpdated(); struct RollupDeploymentParams { Config config; @@ -49,22 +50,32 @@ contract RollupCreator is Ownable { _; } - BridgeCreator public immutable bridgeCreator; - IOneStepProofEntry public immutable osp; - IChallengeManager public immutable challengeManagerTemplate; - IRollupAdmin public immutable rollupAdminLogic; - IRollupUser public immutable rollupUserLogic; - IUpgradeExecutor public immutable upgradeExecutorLogic; + modifier onlyOnce() { + require(!templatesSet, "Templates already set"); + _; + } + + BridgeCreator public bridgeCreator; + IOneStepProofEntry public osp; + IChallengeManager public challengeManagerTemplate; + IRollupAdmin public rollupAdminLogic; + IRollupUser public rollupUserLogic; + IUpgradeExecutor public upgradeExecutorLogic; - address public immutable validatorUtils; - address public immutable validatorWalletCreator; + address public validatorUtils; + address public validatorWalletCreator; - DeployHelper public immutable l2FactoriesDeployer; + DeployHelper public l2FactoriesDeployer; + bool public templatesSet; bool public deploymentFrozen; - string public version; - constructor( + constructor() Ownable() {} + + // creator receives back excess fees (for deploying L2 factories) so it can refund the caller + receive() external payable {} + + function setTemplates( BridgeCreator _bridgeCreator, IOneStepProofEntry _osp, IChallengeManager _challengeManagerLogic, @@ -73,10 +84,8 @@ contract RollupCreator is Ownable { IUpgradeExecutor _upgradeExecutorLogic, address _validatorUtils, address _validatorWalletCreator, - DeployHelper _l2FactoriesDeployer, - address _creatorOwner, - string memory _version - ) Ownable() { + DeployHelper _l2FactoriesDeployer + ) external onlyOwner onlyOnce { bridgeCreator = _bridgeCreator; osp = _osp; challengeManagerTemplate = _challengeManagerLogic; @@ -87,15 +96,8 @@ contract RollupCreator is Ownable { validatorWalletCreator = _validatorWalletCreator; l2FactoriesDeployer = _l2FactoriesDeployer; - _transferOwnership(_creatorOwner); - version = _version; - } - - // creator receives back excess fees (for deploying L2 factories) so it can refund the caller - receive() external payable {} - - function freezeDeployment() external onlyOwner { - deploymentFrozen = true; + templatesSet = true; + emit TemplatesUpdated(); } /** @@ -250,6 +252,10 @@ contract RollupCreator is Ownable { return address(rollup); } + function freezeDeployment() external onlyOwner { + deploymentFrozen = true; + } + function _deployUpgradeExecutor(address rollupOwner, ProxyAdmin proxyAdmin) internal returns (address) @@ -349,4 +355,4 @@ contract RollupCreator is Ownable { } return scaledAmount; } -} +} \ No newline at end of file diff --git a/test/foundry/RollupCreator.t.sol b/test/foundry/RollupCreator.t.sol index aa36a5d9..da805057 100644 --- a/test/foundry/RollupCreator.t.sol +++ b/test/foundry/RollupCreator.t.sol @@ -56,6 +56,7 @@ contract RollupCreatorTest is Test { function setUp() public { //// deploy rollup creator and set templates vm.startPrank(deployer); + rollupCreator = new RollupCreator(); deployHelper = new DeployHelper(); // deploy BridgeCreators @@ -74,7 +75,7 @@ contract RollupCreatorTest is Test { rollupUser = _rollupUser; //// deploy creator and set logic - rollupCreator = new RollupCreator( + rollupCreator.setTemplates( bridgeCreator, ospEntry, challengeManager, @@ -83,9 +84,7 @@ contract RollupCreatorTest is Test { upgradeExecutorLogic, address(new ValidatorUtils()), address(new ValidatorWalletCreator()), - deployHelper, - deployer, - "4.2.0" + deployHelper ); vm.stopPrank(); @@ -408,10 +407,12 @@ contract RollupCreatorTest is Test { ); } - function test_upgrade() public { + function test_freezeDeployment() public { vm.startPrank(deployer); - // deployment params + rollupCreator.freezeDeployment(); + assertTrue(rollupCreator.deploymentFrozen(), "rollupCreator not frozen"); + ISequencerInbox.MaxTimeVariation memory timeVars = ISequencerInbox.MaxTimeVariation( ((60 * 60 * 24) / 15), 12, @@ -433,7 +434,7 @@ contract RollupCreatorTest is Test { }); // prepare funds - uint256 factoryDeploymentFunds = 0.2 ether; + uint256 factoryDeploymentFunds = 1 ether; vm.deal(deployer, factoryDeploymentFunds); /// deploy rollup @@ -443,6 +444,7 @@ contract RollupCreatorTest is Test { address[] memory validators = new address[](2); validators[0] = makeAddr("validator1"); validators[1] = makeAddr("validator2"); + address eigenDARollupManager = makeAddr("rollupManager"); RollupCreator.RollupDeploymentParams memory deployParams = RollupCreator @@ -457,42 +459,55 @@ contract RollupCreatorTest is Test { batchPosterManager: batchPosterManager, eigenDARollupManager: eigenDARollupManager }); - address rollupAddress = rollupCreator.createRollup{value: factoryDeploymentFunds}( + + vm.expectRevert("Deployment no longer permitted from this RollupCreator"); + rollupCreator.createRollup{value: factoryDeploymentFunds}( deployParams ); - + vm.stopPrank(); + } - //// upgrade inbox - RollupCore rollup = RollupCore(rollupAddress); - address inbox = address(rollup.inbox()); - address proxyAdmin = computeCreateAddress(address(rollupCreator), 1); - IUpgradeExecutor upgradeExecutor = IUpgradeExecutor( - computeCreateAddress(address(rollupCreator), 4) - ); + function test_setTemplates_onlyOnce() public { + vm.startPrank(deployer); - Dummy newLogicImpl = new Dummy(); - bytes memory data = abi.encodeWithSelector( - ProxyUpgradeAction.perform.selector, - address(proxyAdmin), - inbox, - address(newLogicImpl) - ); + BridgeCreator bridgeCreator = new BridgeCreator(ethBasedTemplates, erc20BasedTemplates); - address upgradeAction = address(new ProxyUpgradeAction()); - vm.prank(rollupOwner); - upgradeExecutor.execute(upgradeAction, data); + IUpgradeExecutor upgradeExecutorLogic = new UpgradeExecutorMock(); - // check upgrade was successful - assertEq(_getImpl(inbox), address(newLogicImpl)); + ( + IOneStepProofEntry ospEntry, + IChallengeManager challengeManager, + IRollupAdmin _rollupAdmin, + IRollupUser _rollupUser + ) = _prepareRollupDeployment(); + + rollupAdmin = _rollupAdmin; + rollupUser = _rollupUser; + + ValidatorUtils validatorUtils = new ValidatorUtils(); + ValidatorWalletCreator validatorWalletCreator = new ValidatorWalletCreator(); + + vm.expectRevert("Templates already set"); + rollupCreator.setTemplates( + bridgeCreator, + ospEntry, + challengeManager, + _rollupAdmin, + _rollupUser, + upgradeExecutorLogic, + address(validatorUtils), + address(validatorWalletCreator), + deployHelper + ); + + vm.stopPrank(); } - function test_freeze() public { + function test_upgrade() public { vm.startPrank(deployer); - rollupCreator.freezeDeployment(); - assertTrue(rollupCreator.deploymentFrozen(), "rollupCreator not frozen"); - + // deployment params ISequencerInbox.MaxTimeVariation memory timeVars = ISequencerInbox.MaxTimeVariation( ((60 * 60 * 24) / 15), 12, @@ -514,7 +529,7 @@ contract RollupCreatorTest is Test { }); // prepare funds - uint256 factoryDeploymentFunds = 1 ether; + uint256 factoryDeploymentFunds = 0.2 ether; vm.deal(deployer, factoryDeploymentFunds); /// deploy rollup @@ -524,7 +539,6 @@ contract RollupCreatorTest is Test { address[] memory validators = new address[](2); validators[0] = makeAddr("validator1"); validators[1] = makeAddr("validator2"); - address eigenDARollupManager = makeAddr("rollupManager"); RollupCreator.RollupDeploymentParams memory deployParams = RollupCreator @@ -539,13 +553,34 @@ contract RollupCreatorTest is Test { batchPosterManager: batchPosterManager, eigenDARollupManager: eigenDARollupManager }); - - vm.expectRevert("Deployment no longer permitted from this RollupCreator"); - rollupCreator.createRollup{value: factoryDeploymentFunds}( + address rollupAddress = rollupCreator.createRollup{value: factoryDeploymentFunds}( deployParams ); - + vm.stopPrank(); + + //// upgrade inbox + RollupCore rollup = RollupCore(rollupAddress); + address inbox = address(rollup.inbox()); + address proxyAdmin = computeCreateAddress(address(rollupCreator), 1); + IUpgradeExecutor upgradeExecutor = IUpgradeExecutor( + computeCreateAddress(address(rollupCreator), 4) + ); + + Dummy newLogicImpl = new Dummy(); + bytes memory data = abi.encodeWithSelector( + ProxyUpgradeAction.perform.selector, + address(proxyAdmin), + inbox, + address(newLogicImpl) + ); + + address upgradeAction = address(new ProxyUpgradeAction()); + vm.prank(rollupOwner); + upgradeExecutor.execute(upgradeAction, data); + + // check upgrade was successful + assertEq(_getImpl(inbox), address(newLogicImpl)); } function _prepareRollupDeployment() @@ -608,4 +643,4 @@ contract ProxyUpgradeAction { contract Dummy { function dummy() public {} -} +} \ No newline at end of file