refactor: move hardcoded value to config (#270)

* refactor: move hardcoded value to config * refactor: minimumAssertionPeriod and validatorAfkBlocks in bold upgrade setting * perf: remove onchain verification * chore: update sigs * refactor: remove hardcode in rollup initialize * chore: update example * test: new config param * fix: format test * test: fix config * Apply suggestions from code review Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com> --------- Co-authored-by: Henry <11198460+godzillaba@users.noreply.github.com>
OffchainLabs · Dec 10, 2024 · 2b13047 · 2b13047
1 parent 2c72296
commit 2b13047
Show file tree

Hide file tree

Showing 16 changed files with 60 additions and 29 deletions.
diff --git a/scripts/boldUpgradeCommon.ts b/scripts/boldUpgradeCommon.ts
@@ -76,6 +76,8 @@ export interface Config {
     stakeAmt: BigNumber
     miniStakeAmounts: BigNumber[]
     chainId: number
+    minimumAssertionPeriod: number
+    validatorAfkBlocks: number
     disableValidatorWhitelist: boolean
     maxDataSize: number
     blockLeafSize: number

diff --git a/scripts/config.ts.example b/scripts/config.ts.example
@@ -17,6 +17,8 @@ export const config = {
     owner: '0x1234123412341234123412341234123412341234',
     loserStakeEscrow: ethers.constants.AddressZero,
     chainId: ethers.BigNumber.from('13331370'),
+    minimumAssertionPeriod: 75,
+    validatorAfkBlocks: 201600,
     chainConfig:
       '{"chainId":13331370,"homesteadBlock":0,"daoForkBlock":null,"daoForkSupport":true,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"berlinBlock":0,"londonBlock":0,"clique":{"period":0,"epoch":0},"arbitrum":{"EnableArbOS":true,"AllowDebugPrecompiles":false,"DataAvailabilityCommittee":false,"InitialArbOSVersion":10,"InitialChainOwner":"0x1234123412341234123412341234123412341234","GenesisBlockNum":0}}',
     genesisBlockNum: ethers.BigNumber.from('0'),

diff --git a/scripts/files/configs/arb1.ts b/scripts/files/configs/arb1.ts
@@ -29,6 +29,8 @@ export const arb1: Config = {
     stakeAmt: parseEther('3600'),
     miniStakeAmounts: [parseEther('0'), parseEther('555'), parseEther('79')],
     chainId: 42161,
+    minimumAssertionPeriod: 75,
+    validatorAfkBlocks: 201600,
     disableValidatorWhitelist: true,
     blockLeafSize: 2 ** 26,
     bigStepLeafSize: 2 ** 19,

diff --git a/scripts/files/configs/local.ts b/scripts/files/configs/local.ts
@@ -35,6 +35,8 @@ export const local: Config = {
       parseEther('1'),
     ],
     chainId: 412346,
+    minimumAssertionPeriod: 0,
+    validatorAfkBlocks: 201600,
     disableValidatorWhitelist: true,
     blockLeafSize: 1048576,
     bigStepLeafSize: 512,

diff --git a/scripts/files/configs/nova.ts b/scripts/files/configs/nova.ts
@@ -30,6 +30,8 @@ export const nova: Config = {
     stakeAmt: parseEther('1'),
     miniStakeAmounts: [parseEther('0'), parseEther('1'), parseEther('1')],
     chainId: 42170,
+    minimumAssertionPeriod: 75,
+    validatorAfkBlocks: 201600,
     disableValidatorWhitelist: false,
     blockLeafSize: 2 ** 26, // leaf sizes same as arb1
     bigStepLeafSize: 2 ** 19,

diff --git a/scripts/files/configs/sepolia.ts b/scripts/files/configs/sepolia.ts
@@ -28,6 +28,8 @@ export const sepolia: Config = {
     stakeAmt: parseEther('36'), // 1/100th of arb1, same for mini stakes
     miniStakeAmounts: [parseEther('0'), parseEther('5.5'), parseEther('0.79')],
     chainId: 421614,
+    minimumAssertionPeriod: 75,
+    validatorAfkBlocks: 201600,
     disableValidatorWhitelist: false,
     blockLeafSize: 2 ** 26, // leaf sizes same as arb1
     bigStepLeafSize: 2 ** 19,

diff --git a/scripts/rollupCreation.ts b/scripts/rollupCreation.ts
@@ -303,6 +303,8 @@ async function _getDevRollupConfig(
       loserStakeEscrow: ethers.constants.AddressZero,
       chainId: JSON.parse(chainConfig)['chainId'],
       chainConfig: chainConfig,
+      minimumAssertionPeriod: 75,
+      validatorAfkBlocks: 201600,
       genesisAssertionState: {}, // AssertionState
       genesisInboxCount: 0,
       miniStakeValues: [

diff --git a/src/rollup/BOLDUpgradeAction.sol b/src/rollup/BOLDUpgradeAction.sol
@@ -185,6 +185,9 @@ contract BOLDUpgradeAction {
     address public immutable STAKE_TOKEN;
     uint256 public immutable STAKE_AMOUNT;
     uint256 public immutable CHAIN_ID;
+    uint256 public immutable MINIMUM_ASSERTION_PERIOD;
+    uint64 public immutable VALIDATOR_AFK_BLOCKS;
+
     bool public immutable DISABLE_VALIDATOR_WHITELIST;
     uint64 public immutable CHALLENGE_GRACE_PERIOD_BLOCKS;
     address public immutable MINI_STAKE_AMOUNTS_STORAGE;
@@ -223,6 +226,8 @@ contract BOLDUpgradeAction {
         uint256 stakeAmt;
         uint256[] miniStakeAmounts;
         uint256 chainId;
+        uint256 minimumAssertionPeriod;
+        uint64 validatorAfkBlocks;
         bool disableValidatorWhitelist;
         uint256 blockLeafSize;
         uint256 bigStepLeafSize;
@@ -296,6 +301,8 @@ contract BOLDUpgradeAction {
         IMPL_CHALLENGE_MANAGER = implementations.challengeManager;
 
         CHAIN_ID = settings.chainId;
+        MINIMUM_ASSERTION_PERIOD = settings.minimumAssertionPeriod;
+        VALIDATOR_AFK_BLOCKS = settings.validatorAfkBlocks;
         CONFIRM_PERIOD_BLOCKS = settings.confirmPeriodBlocks;
         CHALLENGE_PERIOD_BLOCKS = settings.challengePeriodBlocks;
         STAKE_TOKEN = settings.stakeToken;
@@ -373,6 +380,8 @@ contract BOLDUpgradeAction {
             loserStakeEscrow: EXCESS_STAKE_RECEIVER, // additional funds get sent to the l1 timelock
             chainId: CHAIN_ID,
             chainConfig: "", // we can use an empty chain config it wont be used in the rollup initialization because we check if the rei is already connected there
+            minimumAssertionPeriod: MINIMUM_ASSERTION_PERIOD,
+            validatorAfkBlocks: VALIDATOR_AFK_BLOCKS,
             miniStakeValues: ConstantArrayStorage(MINI_STAKE_AMOUNTS_STORAGE).array(),
             sequencerInboxMaxTimeVariation: maxTimeVariation,
             layerZeroBlockEdgeHeight: BLOCK_LEAF_SIZE,
@@ -433,17 +442,6 @@ contract BOLDUpgradeAction {
             PROXY_ADMIN_SEQUENCER_INBOX.upgrade(sequencerInbox, IMPL_SEQUENCER_INBOX);
         }
 
-        // verify
-        require(
-            PROXY_ADMIN_SEQUENCER_INBOX.getProxyImplementation(sequencerInbox)
-                == IMPL_SEQUENCER_INBOX,
-            "DelayBuffer: new seq inbox implementation not set"
-        );
-        require(
-            ISequencerInbox(SEQ_INBOX).isDelayBufferable() == IS_DELAY_BUFFERABLE,
-            "DelayBuffer: isDelayBufferable not set"
-        );
-
         (uint256 delayBlocks, uint256 futureBlocks, uint256 delaySeconds, uint256 futureSeconds) =
             ISequencerInbox(SEQ_INBOX).maxTimeVariation();
 
@@ -465,14 +463,6 @@ contract BOLDUpgradeAction {
             })
         );
 
-        // verify
-        (uint256 _delayBlocks, uint256 _futureBlocks, uint256 _delaySeconds, uint256 _futureSeconds)
-        = ISequencerInbox(SEQ_INBOX).maxTimeVariation();
-        require(_delayBlocks == delayBlocks, "DelayBuffer: delayBlocks");
-        require(_delaySeconds == delaySeconds, "DelayBuffer: delaySeconds");
-        require(_futureBlocks == futureBlocks, "DelayBuffer: futureBlocks");
-        require(_futureSeconds == futureSeconds, "DelayBuffer: futureSeconds");
-
         ISequencerInbox(SEQ_INBOX).updateRollupAddress();
     }
 

diff --git a/src/rollup/Config.sol b/src/rollup/Config.sol
@@ -23,6 +23,8 @@ struct Config {
     address loserStakeEscrow;
     uint256 chainId;
     string chainConfig;
+    uint256 minimumAssertionPeriod;
+    uint64 validatorAfkBlocks;
     uint256[] miniStakeValues;
     ISequencerInbox.MaxTimeVariation sequencerInboxMaxTimeVariation;
     uint256 layerZeroBlockEdgeHeight;

diff --git a/src/rollup/RollupAdminLogic.sol b/src/rollup/RollupAdminLogic.sol
@@ -53,12 +53,12 @@ contract RollupAdminLogic is RollupCore, IRollupAdmin, DoubleLogicUUPSUpgradeabl
         chainId = config.chainId;
         baseStake = config.baseStake;
         wasmModuleRoot = config.wasmModuleRoot;
-        // A little over 15 minutes
-        minimumAssertionPeriod = 75;
-        // ValidatorAfkBlocks is defaulted to 28 days assuming a 12 seconds block time.
+        // minimumAssertionPeriod was defaulted to 75 which is a little over 15 minutes
+        minimumAssertionPeriod = config.minimumAssertionPeriod;
+        // ValidatorAfkBlocks was defaulted to 201600 which is 28 days assuming a 12 seconds block time.
         // Since it can take 14 days under normal circumstances to confirm an assertion, this means
         // the validators will have been inactive for a further 14 days before the whitelist is removed.
-        validatorAfkBlocks = 201600;
+        validatorAfkBlocks = config.validatorAfkBlocks;
         challengeGracePeriodBlocks = config.challengeGracePeriodBlocks;
 
         // loser stake is now sent directly to loserStakeEscrow, it must not

diff --git a/test/Rollup.t.sol b/test/Rollup.t.sol
@@ -154,6 +154,8 @@ contract RollupTest is Test {
             baseStake: BASE_STAKE,
             chainId: 0,
             chainConfig: "{}",
+            minimumAssertionPeriod: 75,
+            validatorAfkBlocks: 201600,
             confirmPeriodBlocks: uint64(CONFIRM_PERIOD_BLOCKS),
             owner: owner,
             sequencerInboxMaxTimeVariation: ISequencerInbox.MaxTimeVariation({

diff --git a/test/e2e/orbitChain.ts b/test/e2e/orbitChain.ts
@@ -834,6 +834,8 @@ describe('Orbit Chain', () => {
       chainId: ethers.BigNumber.from('433333'),
       chainConfig:
         '{"chainId":433333,"homesteadBlock":0,"daoForkBlock":null,"daoForkSupport":true,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"berlinBlock":0,"londonBlock":0,"clique":{"period":0,"epoch":0},"arbitrum":{"EnableArbOS":true,"AllowDebugPrecompiles":false,"DataAvailabilityCommittee":false,"InitialArbOSVersion":10,"InitialChainOwner":"0x72f7EEedF02C522242a4D3Bdc8aE6A8583aD7c5e","GenesisBlockNum":0}}',
+      minimumAssertionPeriod: 75,
+      validatorAfkBlocks: 201600,
       genesisAssertionState: genesisAssertionState, // AssertionState
       genesisInboxCount: 0,
       miniStakeValues: [

diff --git a/test/foundry/RollupCreator.t.sol b/test/foundry/RollupCreator.t.sol
@@ -112,7 +112,9 @@ contract RollupCreatorTest is Test {
             baseStake: 1000,
             chainId: 1337,
             chainConfig: "abc",
-            confirmPeriodBlocks: 20,
+            minimumAssertionPeriod: 75,
+            validatorAfkBlocks: 1234,
+            confirmPeriodBlocks: 567,
             owner: rollupOwner,
             sequencerInboxMaxTimeVariation: timeVars,
             stakeToken: address(token),
@@ -184,6 +186,12 @@ contract RollupCreatorTest is Test {
             batchPosterManager,
             "Invalid batch poster manager"
         );
+        assertEq(
+            rollup.validatorAfkBlocks(), config.validatorAfkBlocks, "Invalid validatorAfkBlocks"
+        );
+        assertEq(
+            rollup.confirmPeriodBlocks(), config.confirmPeriodBlocks, "Invalid confirmPeriodBlocks"
+        );
 
         // check proxy admin for non-rollup contracts
         address proxyAdminExpectedAddress = computeCreateAddress(address(rollupCreator), 1);
@@ -285,7 +293,9 @@ contract RollupCreatorTest is Test {
             baseStake: 1000,
             chainId: 1337,
             chainConfig: "abc",
-            confirmPeriodBlocks: 20,
+            minimumAssertionPeriod: 75,
+            validatorAfkBlocks: 1234,
+            confirmPeriodBlocks: 567,
             owner: rollupOwner,
             sequencerInboxMaxTimeVariation: timeVars,
             stakeToken: address(token),
@@ -332,11 +342,12 @@ contract RollupCreatorTest is Test {
         vm.stopPrank();
 
         _postCreateERC20RollupChecks(
-            rollupAddress, batchPosterManager, nativeToken, validators, batchPosters
+            config, rollupAddress, batchPosterManager, nativeToken, validators, batchPosters
         );
     }
 
     function _postCreateERC20RollupChecks(
+        Config memory config,
         address rollupAddress,
         address batchPosterManager,
         address nativeToken,
@@ -371,6 +382,12 @@ contract RollupCreatorTest is Test {
             batchPosterManager,
             "Invalid batch poster manager"
         );
+        assertEq(
+            rollup.validatorAfkBlocks(), config.validatorAfkBlocks, "Invalid validatorAfkBlocks"
+        );
+        assertEq(
+            rollup.confirmPeriodBlocks(), config.confirmPeriodBlocks, "Invalid confirmPeriodBlocks"
+        );
 
         // native token check
         IBridge bridge = RollupCore(address(rollupAddress)).bridge();
@@ -456,7 +473,9 @@ contract RollupCreatorTest is Test {
             baseStake: 1000,
             chainId: 1337,
             chainConfig: "abc",
-            confirmPeriodBlocks: 20,
+            minimumAssertionPeriod: 75,
+            validatorAfkBlocks: 1234,
+            confirmPeriodBlocks: 567,
             owner: rollupOwner,
             sequencerInboxMaxTimeVariation: timeVars,
             stakeToken: address(token),

diff --git a/test/signatures/RollupAdminLogic b/test/signatures/RollupAdminLogic
@@ -20,7 +20,7 @@
   "getStakerAddress(uint64)": "6ddd3744",
   "getValidators()": "b7ab4db5",
   "inbox()": "fb0e722b",
-  "initialize((uint64,address,uint256,bytes32,address,address,uint256,string,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),(address,address,address,address,address,address,address,address,address))": "9e7e6aa7",
+  "initialize((uint64,address,uint256,bytes32,address,address,uint256,string,uint256,uint64,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),(address,address,address,address,address,address,address,address,address))": "0ee5ef0c",
   "isFirstChild(bytes32)": "30836228",
   "isPending(bytes32)": "e531d8c7",
   "isStaked(address)": "6177fd18",

diff --git a/test/signatures/RollupCreator b/test/signatures/RollupCreator
@@ -1,7 +1,7 @@
 {
   "bridgeCreator()": "f860cefa",
   "challengeManagerTemplate()": "9c683d10",
-  "createRollup(((uint64,address,uint256,bytes32,address,address,uint256,string,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),address[],uint256,address,bool,uint256,address[],address))": "5491abea",
+  "createRollup(((uint64,address,uint256,bytes32,address,address,uint256,string,uint256,uint64,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),address[],uint256,address,bool,uint256,address[],address))": "a2f454fc",
   "l2FactoriesDeployer()": "ac0425bc",
   "osp()": "f26a62c6",
   "owner()": "8da5cb5b",

diff --git a/test/stakingPool/AssertionStakingPool.t.sol b/test/stakingPool/AssertionStakingPool.t.sol
@@ -154,6 +154,8 @@ contract AssertionPoolTest is Test {
             baseStake: BASE_STAKE,
             chainId: 0,
             chainConfig: "{}",
+            minimumAssertionPeriod: 75,
+            validatorAfkBlocks: 201600,
             confirmPeriodBlocks: uint64(CONFIRM_PERIOD_BLOCKS),
             owner: owner,
             sequencerInboxMaxTimeVariation: ISequencerInbox.MaxTimeVariation({