feat: bold reduce base stake (#391)

* feat: added base stake reduction with additional check

* style: formatting

* test: move missing rollup tests into foundry

* test: Added tests for setting base stake

* refactor: simplify confirmed stakers check

* doc: clarified comment about permissioned stakrs

* style: formatting

* feat: separate increase/decrease base stake and added additional checks and tests for them

* feat: remove getAllStakers public function and instead make stakerlist internal

* chore: remove unused comment

* fix: yarn format

* chore: remove redundent import

* chore: update slither db

* chore: update 4bytes

* refactor: use getStakerAddress

* chore: avoid shadowing

* chore: keep getStaker external

---------

Co-authored-by: gzeon <im@gzeon.dev>
Co-authored-by: gzeon <hng@offchainlabs.com>

Author: yahgwai

foundry.toml

@@ -25,6 +25,7 @@ additional_compiler_profiles = [
 ]
 compilation_restrictions = [
     { paths = "src/rollup/RollupUserLogic.sol", optimizer_runs = 20 },
+    { paths = "src/rollup/RollupAdminLogic.sol", optimizer_runs = 20 },
     { paths = "src/challengeV2/EdgeChallengeManager.sol", optimizer_runs = 200 },
 ]
 skip = ['test/*']

hardhat.config.ts

@@ -38,6 +38,13 @@ const solidity: SolidityConfig = {
         optimizer: { ...commonSetting.optimizer, runs: 20 },
       },
     },
+    'src/rollup/RollupAdminLogic.sol': {
+      version: '0.8.17',
+      settings: {
+        ...commonSetting,
+        optimizer: { ...commonSetting.optimizer, runs: 20 },
+      },
+    },
     'src/challengeV2/EdgeChallengeManager.sol': {
       version: '0.8.17',
       settings: {

slither.db.json

@@ -8,7 +8,6 @@ import "@openzeppelin/contracts-upgradeable/utils/Create2Upgradeable.sol";
 import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
 import "./RollupProxy.sol";
 import "./RollupLib.sol";
-import "./RollupAdminLogic.sol";
 
 struct Node {
     // Hash of the state of the chain as of this node

src/rollup/BOLDUpgradeAction.sol

@@ -150,10 +150,22 @@ interface IRollupAdmin {
     ) external;
 
     /**
-     * @notice Set base stake required for an assertion
-     * @param newBaseStake maximum avmgas to be used per block
+     * @notice Decrease the base stake required for creating an assertion
+     * @dev    Can only be called on permissioned chains with whitelist enabled
+     *         Can only be called when there is exactly one stake on a pending assertion - this ensures a linear chain of assertions
+     *         Cannot be called immediately after genesis since there are no pending assertions
+     *         After decreasing the base stake the current staker will still have full stake locked up. They can release it by creating a new staker with the
+     *         new smaller amount, and using it to create a child of the latest pending assertion. This will make the old staker inactive and withdrawable.
+     * @param newBaseStake New base stake to be set. Must be less than current base stake, otherwise use increaseBaseStake
+     * @param latestNextInboxPosition The nextInboxPosition of the only pending latestStakedAssertion
      */
-    function setBaseStake(
+    function decreaseBaseStake(uint256 newBaseStake, uint64 latestNextInboxPosition) external;
+
+    /**
+     * @notice Increase the base stake required for creating an assertion
+     * @param newBaseStake New base stake to be set. Must be greater than current base stake, otherwise use reduceBaseStake
+     */
+    function increaseBaseStake(
         uint256 newBaseStake
     ) external;
 

src/rollup/IRollupAdmin.sol

@@ -10,7 +10,6 @@ import "./RollupCore.sol";
 import "../bridge/IOutbox.sol";
 import "../bridge/ISequencerInbox.sol";
 import "../libraries/DoubleLogicUUPSUpgradeable.sol";
-import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
 
 contract RollupAdminLogic is RollupCore, IRollupAdmin, DoubleLogicUUPSUpgradeable {
     using AssertionStateLib for AssertionState;
@@ -264,20 +263,80 @@ contract RollupAdminLogic is RollupCore, IRollupAdmin, DoubleLogicUUPSUpgradeabl
     }
 
     /**
-     * @notice Set base stake required for an assertion
-     * @param newBaseStake minimum amount of stake required
+     * @inheritdoc IRollupAdmin
      */
-    function setBaseStake(
-        uint256 newBaseStake
+    function decreaseBaseStake(
+        uint256 newBaseStake,
+        uint64 latestNextInboxPosition
     ) external override {
-        // we do not currently allow base stake to be reduced since as doing so might allow a malicious party
+        require(newBaseStake < baseStake, "BASE_STAKE_NOT_DECREASED");
+
+        // if we're decreasing the stake we need to be more careful not to allow a malicious party
         // to withdraw some (up to the difference between baseStake and newBaseStake) honest funds from this contract
         // The sequence of events is as follows:
         // 1. The malicious party creates a sibling assertion, stake size is currently S
-        // 2. The base stake is then reduced to S'
+        // 2. The base stake is then decreased to S'
         // 3. The malicious party uses a different address to create a child of the malicious assertion, using stake size S'
         // 4. This allows the malicious party to withdraw the stake S, since assertions with children set the staker to "inactive"
-        require(newBaseStake > baseStake, "BASE_STAKE_MUST_BE_INCREASED");
+
+        // for this reason the base stake can only be decreased on permissioned chains where the creation of pending assertions can be controlled
+        // In a permissionless setting, it's possible for a staker to DOS base stake decrease by creating themselves as a staker.
+        // we can do a basic safety check to ensure that we're in the permissioned setting by checking that there is only one
+        // staker on a pending assertion. It could be that we're in the permissionless setting and that pending assertion is a malicious one
+        // but we assume that the rollup admin will ensure that's not the case before calling this function by checking that a staker they trust is
+        // on a correct pending assertion.
+        require(validatorWhitelistDisabled == false, "DECREASE_ONLY_FOR_PERMISSIONED_CHAINS");
+
+        bytes32 currentConfigHash = RollupLib.configHash({
+            wasmModuleRoot: wasmModuleRoot,
+            requiredStake: baseStake,
+            challengeManager: address(challengeManager),
+            confirmPeriodBlocks: confirmPeriodBlocks,
+            nextInboxPosition: uint64(latestNextInboxPosition)
+        });
+
+        uint256 pendingCount = 0;
+        uint64 nStakers = stakerCount();
+        for (uint64 i = 0; i < nStakers; i++) {
+            bytes32 latestStaked = latestStakedAssertion(getStakerAddress(i));
+            AssertionNode storage latestAssertion = getAssertionStorage(latestStaked);
+            if (latestAssertion.status == AssertionStatus.Pending) {
+                // check that in overwriting the config hash, we'll only be updating the required stake field
+                require(latestAssertion.configHash == currentConfigHash, "EXPIRED_CONFIG_HASH");
+                // In order to support transitioning from permissioned to permissionless, we need to ensure that after the transition
+                // a malicious party cannot created decreased stake assertions as children of historical (even confirmed) assertions.
+                // To avoid this an additional check has been added in RollupUserLogic.createNewAssertion to ensure that the base stake
+                // is always >= the required stake of the prev assertion. Having added this check we also need to override the config hash of
+                // the latest staked assertion so that children can be created from it in the normal way. We do that below
+                latestAssertion.configHash = RollupLib.configHash({
+                    wasmModuleRoot: wasmModuleRoot,
+                    requiredStake: newBaseStake,
+                    challengeManager: address(challengeManager),
+                    confirmPeriodBlocks: confirmPeriodBlocks,
+                    nextInboxPosition: uint64(latestNextInboxPosition)
+                });
+
+                pendingCount++;
+            }
+
+            require(pendingCount <= 1, "TOO_MANY_PENDING_STAKERS");
+        }
+
+        // we need to have a non zero pending count to ensure that exactly one config hash was updated
+        // if no config hashes are updated then no new assertions will be created, since they will all trigger STAKE_TOO_LOW
+        require(pendingCount == 1, "PENDING_ASSERTION_NOT_UPDATED");
+
+        baseStake = newBaseStake;
+        emit BaseStakeSet(newBaseStake);
+    }
+
+    /**
+     * @inheritdoc IRollupAdmin
+     */
+    function increaseBaseStake(
+        uint256 newBaseStake
+    ) external override {
+        require(newBaseStake > baseStake, "BASE_STAKE_NOT_INCREASED");
         baseStake = newBaseStake;
         emit BaseStakeSet(newBaseStake);
         // previously: emit OwnerFunctionCalled(12);

src/rollup/RollupAdminLogic.sol

@@ -171,7 +171,7 @@ abstract contract RollupCore is IRollupCore, PausableUpgradeable {
      */
     function getStakerAddress(
         uint64 stakerNum
-    ) external view override returns (address) {
+    ) public view override returns (address) {
         return _stakerList[stakerNum];
     }
 

src/rollup/RollupCore.sol

@@ -184,6 +184,12 @@ contract RollupUserLogic is RollupCore, UUPSNotUpgradeable, IRollupUser {
             "INSUFFICIENT_STAKE"
         );
 
+        // the new assertion will be created with requiredStake equal to the baseStake
+        // however, for the reasons explained in RollupAdminLogic.reduceBaseStake we cannot allow an assertion to be created
+        // base stake than it's prev. In order to reduce the base stake the config hash of the latest assertion
+        // must be overridden, this occurs in the RollupAdminLogic.reduceBaseStake function
+        require(baseStake >= assertion.beforeStateData.configData.requiredStake, "STAKE_TOO_LOW");
+
         bytes32 prevAssertion = RollupLib.assertionHash(
             assertion.beforeStateData.prevPrevAssertionHash,
             assertion.beforeState,