feat(contracts): apply PR reviews

packages/contracts/contracts/L1/AssetManager.sol

@@ -339,6 +339,11 @@ contract AssetManager is ISemver, IERC721Receiver {
      */
     error NotAllowedCaller();
 
+    /**
+     * @notice Reverts when constructor parameters are invalid.
+     */
+    error InvalidConstructorParams();
+
     /**
      * @notice Reverts when the status of validator is improper.
      */
@@ -426,6 +431,8 @@ contract AssetManager is ISemver, IERC721Receiver {
         uint128 _slashingRate,
         uint128 _minSlashingAmount
     ) {
+        if (_slashingRate > SLASHING_RATE_DENOM) revert InvalidConstructorParams();
+
         ASSET_TOKEN = _assetToken;
         KGH = _kgh;
         KGH_MANAGER = _kghManager;
@@ -774,7 +781,7 @@ contract AssetManager is ISemver, IERC721Receiver {
     }
 
     /**
-     * @notice Initiate KRO undelegation of given shares for given validator.
+     * @notice Initiate the KRO undelegation of given shares for the given validator.
      *
      * @param validator Address of the validator.
      * @param shares    The amount of shares to undelegate.

packages/contracts/contracts/L1/ValidatorManager.sol

@@ -239,7 +239,7 @@ contract ValidatorManager is ISemver, IValidatorManager {
      * @inheritdoc IValidatorManager
      */
     function changeCommissionRate(uint8 newCommissionRate) external {
-        if (getStatus(msg.sender) <= ValidatorStatus.INACTIVE) revert ImproperValidatorStatus();
+        if (getStatus(msg.sender) < ValidatorStatus.ACTIVE) revert ImproperValidatorStatus();
 
         Validator storage validatorInfo = _validatorInfo[msg.sender];
 
@@ -453,12 +453,12 @@ contract ValidatorManager is ISemver, IValidatorManager {
     }
 
     /**
-     * @notice Internal function to add output submission rewards to the vaults of finalized output
+     * @notice Private function to add output submission rewards to the vaults of finalized output
      *         submitters.
      *
      * @return Whether the reward distribution is done at least once or not.
      */
-    function _distributeReward() internal returns (bool) {
+    function _distributeReward() private returns (bool) {
         uint256 outputIndex = L2_ORACLE.latestFinalizedOutputIndex() + 1;
         uint256 latestOutputIndex = L2_ORACLE.latestOutputIndex();
 
@@ -578,9 +578,9 @@ contract ValidatorManager is ISemver, IValidatorManager {
      */
     function _updatePriorityValidator() private {
         uint120 weightSum = startedValidatorTotalWeight();
-        uint256 latestFinalizedOutputIndex = L2_ORACLE.latestFinalizedOutputIndex();
 
         if (weightSum > 0) {
+            uint256 latestFinalizedOutputIndex = L2_ORACLE.latestFinalizedOutputIndex();
             Types.CheckpointOutput memory output = L2_ORACLE.getL2Output(
                 latestFinalizedOutputIndex
             );

packages/contracts/contracts/L1/ValidatorPool.sol

@@ -179,9 +179,9 @@ contract ValidatorPool is ReentrancyGuardUpgradeable, ISemver {
     }
     /**
      * @notice Semantic version.
-     * @custom:semver 1.1.1
+     * @custom:semver 1.1.0
      */
-    string public constant version = "1.1.1";
+    string public constant version = "1.1.0";
 
     /**
      * @notice Constructs the ValidatorPool contract.

packages/contracts/contracts/test/AssetManager.t.sol

@@ -231,6 +231,20 @@ contract AssetManagerTest is L2OutputOracle_ValidatorSystemUpgrade_Initializer {
         assertEq(assetManager.MIN_SLASHING_AMOUNT(), minSlashingAmount);
     }
 
+    function test_constructor_largeSlashingRate_reverts() external {
+        vm.expectRevert(AssetManager.InvalidConstructorParams.selector);
+        new MockAssetManager(
+            kro,
+            kgh,
+            kghManager,
+            address(guardian),
+            valMan,
+            uint128(undelegationPeriod),
+            1001,
+            minSlashingAmount
+        );
+    }
+
     function test_delegate_succeeds() external {
         _setUpKroDelegation(100e18);
 

packages/contracts/deploy/L1/007-ValidatorPool.ts

@@ -65,6 +65,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['ValidatorPool', 'setup', 'l1']
+deployFn.tags = ['ValidatorPool', 'setup', 'l1', 'validatorSystemUpgrade']
 
 export default deployFn

packages/contracts/deploy/L1/010-L2OutputOracle.ts → packages/contracts/deploy/L1/008-L2OutputOracle.ts

@@ -84,6 +84,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['L2OutputOracle', 'setup', 'l1']
+deployFn.tags = ['L2OutputOracle', 'setup', 'l1', 'validatorSystemUpgrade']
 
 export default deployFn

packages/contracts/deploy/L1/012-Colosseum.ts → packages/contracts/deploy/L1/010-Colosseum.ts

@@ -8,7 +8,10 @@ import {
 } from '../../src/deploy-utils'
 
 const deployFn: DeployFunction = async (hre) => {
-  const zkVerifierProxyAddress = await getDeploymentAddress(hre, 'ZKVerifierProxy')
+  const zkVerifierProxyAddress = await getDeploymentAddress(
+    hre,
+    'ZKVerifierProxy'
+  )
   const l2OutputOracleProxyAddress = await getDeploymentAddress(
     hre,
     'L2OutputOracleProxy'
@@ -90,6 +93,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['Colosseum', 'setup', 'l1']
+deployFn.tags = ['Colosseum', 'setup', 'l1', 'validatorSystemUpgrade']
 
 export default deployFn

packages/contracts/deploy/L1/008-AssetManager.ts → packages/contracts/deploy/L1/018-AssetManager.ts

@@ -72,6 +72,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['AssetManager', 'setup', 'l1']
+deployFn.tags = ['AssetManager', 'setup', 'l1', 'validatorSystemUpgrade']
 
 export default deployFn

packages/contracts/deploy/L1/009-ValidatorManager.ts → packages/contracts/deploy/L1/019-ValidatorManager.ts

@@ -107,6 +107,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['ValidatorManager', 'setup', 'l1']
+deployFn.tags = ['ValidatorManager', 'setup', 'l1', 'validatorSystemUpgrade']
 
 export default deployFn