Minor fixes in docs, naming, etc. Clean up. (#77)

* Fix [ValidatorSet] Refactor period length of `1 days` to a constant #61 * Place structs on top of the interfaces (Fix #53) * Fix #60 * Fix #52 * Remove redundant castings (Fix #49) * Update docs * Change `memory` to `calldata` in external query array (partially fix #50) * Optimizing equation
axieinfinity · Nov 13, 2022 · bc4ebf7 · bc4ebf7
1 parent 0e41ce2
commit bc4ebf7
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 61 deletions.
diff --git a/contracts/interfaces/IRoninTrustedOrganization.sol b/contracts/interfaces/IRoninTrustedOrganization.sol
@@ -58,8 +58,9 @@ interface IRoninTrustedOrganization is IQuorum {
    *
    * Emits the event `TrustedOrganizationRemoved` once an organization is removed.
    *
+   * @param _consensusAddrs The list of consensus addresses linked to corresponding trusted organization that to be removed.
    */
-  function removeTrustedOrganizations(address[] calldata) external;
+  function removeTrustedOrganizations(address[] calldata _consensusAddrs) external;
 
   /**
    * @dev Returns total weights.

diff --git a/contracts/interfaces/IRoninValidatorSet.sol b/contracts/interfaces/IRoninValidatorSet.sol
@@ -184,17 +184,17 @@ interface IRoninValidatorSet is ICandidateManager {
   /**
    * @dev Returns whether the validators are put in jail (cannot join the set of validators) during the current period.
    */
-  function bulkJailed(address[] memory) external view returns (bool[] memory);
+  function bulkJailed(address[] calldata) external view returns (bool[] memory);
 
   /**
    * @dev Returns whether the incoming reward of the block producers are deprecated during the current period.
    */
-  function miningRewardDeprecated(address[] memory _blockProducers) external view returns (bool[] memory);
+  function miningRewardDeprecated(address[] calldata _blockProducers) external view returns (bool[] memory);
 
   /**
    * @dev Returns whether the incoming reward of the block producers are deprecated during a specific period.
    */
-  function miningRewardDeprecatedAtPeriod(address[] memory _blockProducers, uint256 _period)
+  function miningRewardDeprecatedAtPeriod(address[] calldata _blockProducers, uint256 _period)
     external
     view
     returns (bool[] memory);

diff --git a/contracts/interfaces/staking/IBaseStaking.sol b/contracts/interfaces/staking/IBaseStaking.sol
@@ -3,11 +3,6 @@
 pragma solidity ^0.8.9;
 
 interface IBaseStaking {
-  /// @dev Emitted when the minium number of seconds to undelegate is updated.
-  event CooldownSecsToUndelegateUpdated(uint256 minSecs);
-  /// @dev Emitted when the number of seconds that a candidate must wait to be revoked.
-  event WaitingSecsToRevokeUpdated(uint256 secs);
-
   struct PoolDetail {
     // Address of the pool i.e. consensus address of the validator
     address addr;
@@ -23,6 +18,11 @@ interface IBaseStaking {
     mapping(address => uint256) lastDelegatingTimestamp;
   }
 
+  /// @dev Emitted when the minium number of seconds to undelegate is updated.
+  event CooldownSecsToUndelegateUpdated(uint256 minSecs);
+  /// @dev Emitted when the number of seconds that a candidate must wait to be revoked.
+  event WaitingSecsToRevokeUpdated(uint256 secs);
+
   /**
    * @dev Returns The cooldown time in seconds to undelegate from the last timestamp (s)he delegated.
    */

diff --git a/contracts/interfaces/staking/ICandidateStaking.sol b/contracts/interfaces/staking/ICandidateStaking.sol
@@ -52,7 +52,7 @@ interface ICandidateStaking is IRewardPool {
    * Emits the event `PoolApproved`.
    *
    * @param _candidateAdmin the candidate admin will be stored in the validator contract, used for calling function that affects
-   * to its candidate. IE: scheduling maintenance.
+   * to its candidate, e.g. scheduling maintenance.
    *
    */
   function applyValidatorCandidate(

diff --git a/contracts/interfaces/staking/IRewardPool.sol b/contracts/interfaces/staking/IRewardPool.sol
@@ -5,20 +5,6 @@ pragma solidity ^0.8.9;
 import "../../interfaces/consumers/PeriodWrapperConsumer.sol";
 
 interface IRewardPool is PeriodWrapperConsumer {
-  /// @dev Emitted when the fields to calculate pending reward for the user is updated.
-  event UserRewardUpdated(address indexed poolAddr, address indexed user, uint256 debited);
-  /// @dev Emitted when the user claimed their reward
-  event RewardClaimed(address indexed poolAddr, address indexed user, uint256 amount);
-
-  /// @dev Emitted when the pool shares are updated
-  event PoolSharesUpdated(uint256 indexed period, address indexed poolAddr, uint256 shares);
-  /// @dev Emitted when the pools are updated
-  event PoolsUpdated(uint256 indexed period, address[] poolAddrs, uint256[] aRps, uint256[] shares);
-  /// @dev Emitted when the contract fails when updating the pools
-  event PoolsUpdateFailed(uint256 indexed period, address[] poolAddrs, uint256[] rewards);
-  /// @dev Emitted when the contract fails when updating the pools that already set
-  event PoolsUpdateConflicted(uint256 indexed period, address[] poolAddrs);
-
   struct UserRewardFields {
     // Recorded reward amount.
     uint256 debited;
@@ -37,6 +23,20 @@ interface IRewardPool is PeriodWrapperConsumer {
     PeriodWrapper shares;
   }
 
+  /// @dev Emitted when the fields to calculate pending reward for the user is updated.
+  event UserRewardUpdated(address indexed poolAddr, address indexed user, uint256 debited);
+  /// @dev Emitted when the user claimed their reward
+  event RewardClaimed(address indexed poolAddr, address indexed user, uint256 amount);
+
+  /// @dev Emitted when the pool shares are updated
+  event PoolSharesUpdated(uint256 indexed period, address indexed poolAddr, uint256 shares);
+  /// @dev Emitted when the pools are updated
+  event PoolsUpdated(uint256 indexed period, address[] poolAddrs, uint256[] aRps, uint256[] shares);
+  /// @dev Emitted when the contract fails when updating the pools
+  event PoolsUpdateFailed(uint256 indexed period, address[] poolAddrs, uint256[] rewards);
+  /// @dev Emitted when the contract fails when updating the pools that already set
+  event PoolsUpdateConflicted(uint256 indexed period, address[] poolAddrs);
+
   /**
    * @dev Returns the reward amount that user claimable.
    */

diff --git a/contracts/mocks/MockValidatorSet.sol b/contracts/mocks/MockValidatorSet.sol
@@ -39,16 +39,16 @@ contract MockValidatorSet is IRoninValidatorSet, CandidateManager {
 
   function getLastUpdatedBlock() external view override returns (uint256) {}
 
-  function bulkJailed(address[] memory) external view override returns (bool[] memory) {}
+  function bulkJailed(address[] calldata) external view override returns (bool[] memory) {}
 
-  function miningRewardDeprecatedAtPeriod(address[] memory, uint256 _period)
+  function miningRewardDeprecatedAtPeriod(address[] calldata, uint256 _period)
     external
     view
     override
     returns (bool[] memory)
   {}
 
-  function miningRewardDeprecated(address[] memory) external view override returns (bool[] memory) {}
+  function miningRewardDeprecated(address[] calldata) external view override returns (bool[] memory) {}
 
   function epochOf(uint256 _block) external view override returns (uint256) {}
 

diff --git a/contracts/ronin/staking/CandidateStaking.sol b/contracts/ronin/staking/CandidateStaking.sol
@@ -114,16 +114,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
   }
 
   /**
-   * @dev Proposes a candidate to become a validator.
-   *
-   * Requirements:
-   * - The pool admin is able to receive RON.
-   * - The treasury is able to receive RON.
-   * - The amount is larger than or equal to the minimum validator staking amount `minValidatorStakingAmount()`.
-   *
-   * @param _candidateAdmin the candidate admin will be stored in the validator contract, used for calling function that affects
-   * to its candidate. IE: scheduling maintenance.
-   *
+   * @dev See {ICandidateStaking-applyValidatorCandidate}
    */
   function _applyValidatorCandidate(
     address payable _poolAdmin,
@@ -148,13 +139,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
   }
 
   /**
-   * @dev Stakes for the validator candidate.
-   *
-   * Requirements:
-   * - The address `_requester` must be the pool admin.
-   *
-   * Emits the `Staked` event.
-   *
+   * @dev See {ICandidateStaking-stake}
    */
   function _stake(
     PoolDetail storage _pool,
@@ -168,13 +153,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
   }
 
   /**
-   * @dev Withdraws the staking amount `_amount` for the validator candidate.
-   *
-   * Requirements:
-   * - The address `_requester` must be the pool admin.
-   *
-   * Emits the `Unstaked` event.
-   *
+   * @dev See {ICandidateStaking-unstake}
    */
   function _unstake(
     PoolDetail storage _pool,

diff --git a/contracts/ronin/staking/Staking.sol b/contracts/ronin/staking/Staking.sol
@@ -81,7 +81,7 @@ contract Staking is IStaking, CandidateStaking, DelegatorStaking, Initializable
    * @inheritdoc RewardCalculation
    */
   function _currentPeriod() internal view virtual override returns (uint256) {
-    return IRoninValidatorSet(_validatorContract).currentPeriod();
+    return _validatorContract.currentPeriod();
   }
 
   /**

diff --git a/contracts/ronin/validator/RoninValidatorSet.sol b/contracts/ronin/validator/RoninValidatorSet.sol
@@ -33,6 +33,9 @@ contract RoninValidatorSet is
 {
   using EnumFlags for EnumFlags.ValidatorFlag;
 
+  /// @dev Length of period in seconds
+  uint256 internal constant _periodLength = 1 days;
+
   /// @dev The maximum number of validator.
   uint256 internal _maxValidatorNumber;
   /// @dev The number of blocks in a epoch
@@ -227,7 +230,7 @@ contract RoninValidatorSet is
     }
 
     if (_slashAmount > 0) {
-      IStaking(_stakingContract).deductStakingAmount(_validatorAddr, _slashAmount);
+      _stakingContract.deductStakingAmount(_validatorAddr, _slashAmount);
     }
 
     emit ValidatorPunished(_validatorAddr, _period, _jailedUntil[_validatorAddr], _slashAmount, true, false);
@@ -303,7 +306,7 @@ contract RoninValidatorSet is
   /**
    * @inheritdoc IRoninValidatorSet
    */
-  function bulkJailed(address[] memory _addrList) external view override returns (bool[] memory _result) {
+  function bulkJailed(address[] calldata _addrList) external view override returns (bool[] memory _result) {
     _result = new bool[](_addrList.length);
     for (uint256 _i; _i < _addrList.length; _i++) {
       _result[_i] = _jailed(_addrList[_i]);
@@ -313,7 +316,7 @@ contract RoninValidatorSet is
   /**
    * @inheritdoc IRoninValidatorSet
    */
-  function miningRewardDeprecated(address[] memory _blockProducers)
+  function miningRewardDeprecated(address[] calldata _blockProducers)
     external
     view
     override
@@ -329,7 +332,7 @@ contract RoninValidatorSet is
   /**
    * @inheritdoc IRoninValidatorSet
    */
-  function miningRewardDeprecatedAtPeriod(address[] memory _blockProducers, uint256 _period)
+  function miningRewardDeprecatedAtPeriod(address[] calldata _blockProducers, uint256 _period)
     external
     view
     override
@@ -590,8 +593,7 @@ contract RoninValidatorSet is
   ) internal {
     // Shares equally in case the bridge has nothing to votes
     if (_totalBallots == 0 && _totalVotes == 0) {
-      uint256 _shareRatio = _MAX_PERCENTAGE / totalBridgeOperators();
-      _bridgeOperatingReward[_validator] = (_shareRatio * _totalBridgeReward) / _MAX_PERCENTAGE;
+      _bridgeOperatingReward[_validator] = _totalBridgeReward / totalBridgeOperators();
       return;
     }
 
@@ -606,8 +608,7 @@ contract RoninValidatorSet is
       _bridgeRewardDeprecatedAtPeriod[_validator][_period] = true;
       emit ValidatorPunished(_validator, _period, _jailedUntil[_validator], 0, false, true);
     } else if (_totalBallots > 0) {
-      uint256 _shareRatio = (_validatorBallots * _MAX_PERCENTAGE) / _totalBallots;
-      _bridgeOperatingReward[_validator] = (_shareRatio * _totalBridgeReward) / _MAX_PERCENTAGE;
+      _bridgeOperatingReward[_validator] = _totalBridgeReward / _totalBallots;
     }
   }
 
@@ -863,7 +864,7 @@ contract RoninValidatorSet is
    * @dev Returns the calculated period.
    */
   function _computePeriod(uint256 _timestamp) internal pure returns (uint256) {
-    return _timestamp / 1 days;
+    return _timestamp / _periodLength;
   }
 
   /**