scoreManager changes

celo-org · Oct 15, 2024 · 0763a5d · 0763a5d
1 parent 07896ad
commit 0763a5d
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 2 deletions.
diff --git a/packages/protocol/contracts-0.8/common/ScoreManager.sol b/packages/protocol/contracts-0.8/common/ScoreManager.sol
@@ -8,6 +8,12 @@ import "@openzeppelin/contracts8/access/Ownable.sol";
 import "../../contracts/common/interfaces/IScoreManagerGovernance.sol";
 import "../../contracts/common/interfaces/IScoreManager.sol";
 
+/**
+ * @title ScoreManager contract
+ * @notice This contract updates the score of validators and validator groups on L2.
+ * This replaces the previous method of calculating scores based on validator uptime
+ * with a governable score.
+ **/
 contract ScoreManager is
   Initializable,
   Ownable,
@@ -31,6 +37,9 @@ contract ScoreManager is
   event ValidatorScoreSet(address indexed validator, uint256 score);
   event ScoreManagerSetterSet(address indexed scoreManagerSetter);
 
+  /**
+   * @notice Reverts if msg.sender is not authorized to update score.
+   **/
   modifier onlyAuthorizedToUpdateScore() {
     require(
       msg.sender == owner() || scoreManagerSetter == msg.sender,
@@ -52,7 +61,14 @@ contract ScoreManager is
     _transferOwnership(msg.sender);
   }
 
+  /**
+   * @notice Sets the group score for a specified group.
+   * @param group The address of the group wich score needs to be updated.
+   * @param score The new score of the group to be updated.
+   * @dev Set value to `ZERO_FIXED1_UINT` to set score to zero.
+   **/
   function setGroupScore(address group, uint256 score) external onlyAuthorizedToUpdateScore {
+    require(score > 0, "Score must be greater than ZERO.");
     require(
       score <= ZERO_FIXED1_UINT,
       "Score must be less than or equal to 1e24 or ZERO_FIXED1_UINT."
@@ -62,10 +78,17 @@ contract ScoreManager is
     emit GroupScoreSet(group, score);
   }
 
+  /**
+   * @notice Sets the score for a specified validator.
+   * @param validator The address of the validator wich score needs to be updated.
+   * @param score The new score of the validator to be updated.
+   * @dev Set value to `ZERO_FIXED1_UINT` to set score to zero.
+   **/
   function setValidatorScore(
     address validator,
     uint256 score
   ) external onlyAuthorizedToUpdateScore {
+    require(score > 0, "Score must be greater than ZERO.");
     require(
       score <= ZERO_FIXED1_UINT,
       "Score must be less than or equal to 1e24 or ZERO_FIXED1_UINT."
@@ -75,19 +98,34 @@ contract ScoreManager is
     emit ValidatorScoreSet(validator, score);
   }
 
+  /**
+   * @notice Sets the whitelisted address allowed to set validator and group scores.
+   * @param _scoreManagerSetter Address of whitelisted score setter.
+   **/
   function setScoreManagerSetter(address _scoreManagerSetter) external onlyOwner {
     scoreManagerSetter = _scoreManagerSetter;
     emit ScoreManagerSetterSet(_scoreManagerSetter);
   }
 
+  /**
+   * @notice Returns the score of the specified group.
+   * @param group The address of the group of interest.
+   **/
   function getGroupScore(address group) external view returns (uint256) {
     return getScore(groupScores[group]);
   }
 
+  /**
+   * @notice Returns the score of the specified validator.
+   * @param validator The address of the validator of interest.
+   **/
   function getValidatorScore(address validator) external view returns (uint256) {
     return getScore(validatorScores[validator]);
   }
 
+  /**
+   * @notice Returns the address of the whitelisted score setter.
+   **/
   function getScoreManagerSetter() external view returns (address) {
     return scoreManagerSetter;
   }
@@ -103,6 +141,10 @@ contract ScoreManager is
     return (1, 1, 0, 0);
   }
 
+  /**
+   * @notice Returns the actual score based on the input value.
+   * @param score The value from `validatorScores` or `groupScores` mappings .
+   **/
   function getScore(uint256 score) internal pure returns (uint256) {
     if (score == 0) {
       return FIXED1_UINT;

diff --git a/packages/protocol/test-sol/unit/common/ScoreManager.t.sol b/packages/protocol/test-sol/unit/common/ScoreManager.t.sol
@@ -60,7 +60,7 @@ contract ScoreManagerTest_setGroupScore is ScoreManagerTest {
     scoreManager.setGroupScore(owner, 42);
   }
 
-  function test_Reverts_WhenSetToMoreThan1e24() public {
+  function test_Reverts_WhenSetToMoreThan1e24Plus1() public {
     vm.expectRevert("Score must be less than or equal to 1e24 or ZERO_FIXED1_UINT.");
     scoreManager.setGroupScore(owner, 1e24 + 2);
   }
@@ -69,11 +69,16 @@ contract ScoreManagerTest_setGroupScore is ScoreManagerTest {
     assertEq(scoreManager.getGroupScore(owner), 1e24);
   }
 
-  function test_Returns0WhenGroupScoreIsZero() public {
+  function test_Returns0WhenGroupScoreIsZERO_FIXED1_UINT() public {
     scoreManager.setGroupScore(owner, ZERO_FIXED1_UINT);
     assert(scoreManager.getGroupScore(owner) == 0);
   }
 
+  function test_Reverts_WhenSettingScoreToZero() public {
+    vm.expectRevert("Score must be greater than ZERO.");
+    scoreManager.setGroupScore(owner, 0);
+  }
+
   function test_EmitsGroupScoreSet() public {
     vm.expectEmit(false, false, false, true);
     emit GroupScoreSet(owner, 42);
@@ -111,6 +116,11 @@ contract ScoreManagerTest_setValidatorScore is ScoreManagerTest {
     assert(scoreManager.getValidatorScore(owner) == 0);
   }
 
+  function test_Reverts_WhenSettingScoreToZero() public {
+    vm.expectRevert("Score must be greater than ZERO.");
+    scoreManager.setGroupScore(owner, 0);
+  }
+
   function test_EmitsValidatorScoreSet() public {
     vm.expectEmit(false, false, false, true);
     emit ValidatorScoreSet(owner, 42);