[Staking] Rename for clarity between pools and active pools (#134)

* Rename modifier

* Fix error message

* Refactor getters

* Rename cross-contract methods

* Replace by custom errors

* Fix more custom errors

* Fix assertion and test

* Rename to `ErrZeroValue`

contracts/interfaces/staking/IBaseStaking.sol

@@ -23,16 +23,48 @@ interface IBaseStaking {
   /// @dev Emitted when the number of seconds that a candidate must wait to be revoked.
   event WaitingSecsToRevokeUpdated(uint256 secs);
 
+  /// @dev Error of cannot transfer RON.
+  error ErrCannotTransferRON();
+  /// @dev Error of receiving zero message value.
+  error ErrZeroValue();
+  /// @dev Error of pool admin is not allowed to call.
+  error ErrPoolAdminForbidden();
+  /// @dev Error of no one is allowed to call but the pool's admin.
+  error ErrOnlyPoolAdminAllowed();
+  /// @dev Error of admin of any active pool cannot delegate.
+  error ErrAdminOfAnyActivePoolForbidden(address admin);
+  /// @dev Error of querying inactive pool.
+  error ErrInactivePool(address poolAddr);
+  /// @dev Error of length of input arrays are not of the same.
+  error ErrInvalidArrays();
+
   /**
    * @dev Returns whether the `_poolAdminAddr` is currently active.
    */
-  function isActivePoolAdmin(address _poolAdminAddr) external view returns (bool);
+  function isAdminOfActivePool(address _poolAdminAddr) external view returns (bool);
 
   /**
    * @dev Returns the consensus address corresponding to the pool admin.
    */
   function getPoolAddressOf(address _poolAdminAddr) external view returns (address);
 
+  /**
+   * @dev Returns the staking pool detail.
+   */
+  function getPoolDetail(address)
+    external
+    view
+    returns (
+      address _admin,
+      uint256 _stakingAmount,
+      uint256 _stakingTotal
+    );
+
+  /**
+   * @dev Returns the self-staking amounts of the pools.
+   */
+  function getManySelfStakings(address[] calldata) external view returns (uint256[] memory);
+
   /**
    * @dev Returns The cooldown time in seconds to undelegate from the last timestamp (s)he delegated.
    */

contracts/interfaces/staking/ICandidateStaking.sol

@@ -32,6 +32,21 @@ interface ICandidateStaking is IRewardPool {
     uint256 contractBalance
   );
 
+  /// @dev Error of cannot transfer RON to specified target.
+  error ErrCannotInitTransferRON(address addr, string extraInfo);
+  /// @dev Error of three interaction addresses must be of the same in applying for validator candidate.
+  error ErrThreeInteractionAddrsNotEqual();
+  /// @dev Error of three operation addresses must be distinct in applying for validator candidate.
+  error ErrThreeOperationAddrsNotDistinct();
+  /// @dev Error of unstaking zero amount.
+  error ErrUnstakeZeroAmount();
+  /// @dev Error of invalid staking amount left after deducted.
+  error ErrStakingAmountLeft();
+  /// @dev Error of insufficient staking amount for unstaking.
+  error ErrInsufficientStakingAmount();
+  /// @dev Error of unstaking too early.
+  error ErrUnstakeTooEarly();
+
   /**
    * @dev Returns the minimum threshold for being a validator candidate.
    */

contracts/interfaces/staking/IDelegatorStaking.sol

@@ -10,6 +10,13 @@ interface IDelegatorStaking is IRewardPool {
   /// @dev Emitted when the delegator unstaked from a validator candidate.
   event Undelegated(address indexed delegator, address indexed consensuAddr, uint256 amount);
 
+  /// @dev Error of undelegating zero amount.
+  error ErrUndelegateZeroAmount();
+  /// @dev Error of undelegating insufficient amount.
+  error ErrInsufficientDelegatingAmount();
+  /// @dev Error of undelegating too early.
+  error ErrUndelegateTooEarly();
+
   /**
    * @dev Stakes for a validator candidate `_consensusAddr`.
    *

contracts/interfaces/staking/IRewardPool.sol

@@ -37,6 +37,9 @@ interface IRewardPool is PeriodWrapperConsumer {
   /// @dev Emitted when the contract fails when updating the pools that already set
   event PoolsUpdateConflicted(uint256 indexed period, address[] poolAddrs);
 
+  /// @dev Error of invalid pool share.
+  error ErrInvalidPoolShare();
+
   /**
    * @dev Returns the reward amount that user claimable.
    */

contracts/interfaces/staking/IStaking.sol

@@ -11,7 +11,7 @@ interface IStaking is IRewardPool, IBaseStaking, ICandidateStaking, IDelegatorSt
    * @dev Records the amount of rewards `_rewards` for the pools `_consensusAddrs`.
    *
    * Requirements:
-   * - The method caller is validator contract.
+   * - The method caller must be validator contract.
    *
    * Emits the event `PoolsUpdated` once the contract recorded the rewards successfully.
    * Emits the event `PoolsUpdateFailed` once the input array lengths are not equal.
@@ -20,7 +20,7 @@ interface IStaking is IRewardPool, IBaseStaking, ICandidateStaking, IDelegatorSt
    * Note: This method should be called once at the period ending.
    *
    */
-  function recordRewards(
+  function execRecordRewards(
     address[] calldata _consensusAddrs,
     uint256[] calldata _rewards,
     uint256 _period
@@ -30,29 +30,12 @@ interface IStaking is IRewardPool, IBaseStaking, ICandidateStaking, IDelegatorSt
    * @dev Deducts from staking amount of the validator `_consensusAddr` for `_amount`.
    *
    * Requirements:
-   * - The method caller is validator contract.
+   * - The method caller must be validator contract.
    *
    * Emits the event `Unstaked`.
    *
    */
-  function deductStakingAmount(address _consensusAddr, uint256 _amount)
+  function execDeductStakingAmount(address _consensusAddr, uint256 _amount)
     external
     returns (uint256 _actualDeductingAmount);
-
-  /**
-   * @dev Returns the staking pool detail.
-   */
-  function getStakingPool(address)
-    external
-    view
-    returns (
-      address _admin,
-      uint256 _stakingAmount,
-      uint256 _stakingTotal
-    );
-
-  /**
-   * @dev Returns the self-staking amounts of the pools.
-   */
-  function getManySelfStakings(address[] calldata) external view returns (uint256[] memory);
 }

contracts/mocks/MockStaking.sol

@@ -30,7 +30,7 @@ contract MockStaking is RewardCalculation {
     uint256[] memory _rewards = new uint256[](1);
     _addrs[0] = poolAddr;
     _rewards[0] = pendingReward;
-    this.recordRewards(_addrs, _rewards);
+    this.execRecordRewards(_addrs, _rewards);
 
     pendingReward = 0;
     lastUpdatedPeriod++;
@@ -64,7 +64,7 @@ contract MockStaking is RewardCalculation {
     pendingReward -= _amount;
   }
 
-  function recordRewards(address[] calldata _addrList, uint256[] calldata _rewards) external {
+  function execRecordRewards(address[] calldata _addrList, uint256[] calldata _rewards) external {
     _recordRewards(_addrList, _rewards, _currentPeriod());
   }
 

contracts/ronin/staking/BaseStaking.sol

@@ -24,46 +24,72 @@ abstract contract BaseStaking is
   /// @dev The number of seconds that a candidate must wait to be revoked and take the self-staking amount back.
   uint256 internal _waitingSecsToRevoke;
 
-  /// @dev Mapping from active pool admin address => consensus address.
-  mapping(address => address) internal _activePoolAdminMapping;
+  /// @dev Mapping from admin address of an active pool => consensus address.
+  mapping(address => address) internal _adminOfActivePoolMapping;
   /**
    * @dev This empty reserved space is put in place to allow future versions to add new
    * variables without shifting down storage in the inheritance chain.
    */
   uint256[49] private ______gap;
 
   modifier noEmptyValue() {
-    require(msg.value > 0, "BaseStaking: query with empty value");
+    if (msg.value == 0) revert ErrZeroValue();
     _;
   }
 
   modifier notPoolAdmin(PoolDetail storage _pool, address _delegator) {
-    require(_pool.admin != _delegator, "BaseStaking: delegator must not be the pool admin");
+    if (_pool.admin == _delegator) revert ErrPoolAdminForbidden();
     _;
   }
 
   modifier onlyPoolAdmin(PoolDetail storage _pool, address _requester) {
-    require(_pool.admin == _requester, "BaseStaking: requester must be the pool admin");
+    if (_pool.admin != _requester) revert ErrOnlyPoolAdminAllowed();
     _;
   }
 
-  modifier poolExists(address _poolAddr) {
-    require(_validatorContract.isValidatorCandidate(_poolAddr), "BaseStaking: query for non-existent pool");
+  modifier poolIsActive(address _poolAddr) {
+    if (!_validatorContract.isValidatorCandidate(_poolAddr)) revert ErrInactivePool(_poolAddr);
     _;
   }
 
   /**
    * @inheritdoc IBaseStaking
    */
-  function isActivePoolAdmin(address _poolAdminAddr) public view override returns (bool) {
-    return _activePoolAdminMapping[_poolAdminAddr] != address(0);
+  function isAdminOfActivePool(address _poolAdminAddr) public view override returns (bool) {
+    return _adminOfActivePoolMapping[_poolAdminAddr] != address(0);
   }
 
   /**
    * @inheritdoc IBaseStaking
    */
   function getPoolAddressOf(address _poolAdminAddr) external view override returns (address) {
-    return _activePoolAdminMapping[_poolAdminAddr];
+    return _adminOfActivePoolMapping[_poolAdminAddr];
+  }
+
+  /**
+   * @inheritdoc IBaseStaking
+   */
+  function getPoolDetail(address _poolAddr)
+    external
+    view
+    returns (
+      address _admin,
+      uint256 _stakingAmount,
+      uint256 _stakingTotal
+    )
+  {
+    PoolDetail storage _pool = _stakingPool[_poolAddr];
+    return (_pool.admin, _pool.stakingAmount, _pool.stakingTotal);
+  }
+
+  /**
+   * @inheritdoc IBaseStaking
+   */
+  function getManySelfStakings(address[] calldata _pools) external view returns (uint256[] memory _selfStakings) {
+    _selfStakings = new uint256[](_pools.length);
+    for (uint _i = 0; _i < _pools.length; _i++) {
+      _selfStakings[_i] = _stakingPool[_pools[_i]].stakingAmount;
+    }
   }
 
   /**
@@ -104,7 +130,7 @@ abstract contract BaseStaking is
     override
     returns (uint256[] memory _stakingAmounts)
   {
-    require(_poolAddrs.length == _userList.length, "BaseStaking: invalid input array");
+    if (_poolAddrs.length != _userList.length) revert ErrInvalidArrays();
     _stakingAmounts = new uint256[](_poolAddrs.length);
     for (uint _i = 0; _i < _stakingAmounts.length; _i++) {
       _stakingAmounts[_i] = _stakingPool[_poolAddrs[_i]].delegatingAmount[_userList[_i]];

contracts/ronin/staking/CandidateStaking.sol

@@ -40,7 +40,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
     address _bridgeOperatorAddr,
     uint256 _commissionRate
   ) external payable override nonReentrant {
-    require(!isActivePoolAdmin(msg.sender), "CandidateStaking: pool admin is active");
+    if (isAdminOfActivePool(msg.sender)) revert ErrAdminOfAnyActivePoolForbidden(msg.sender);
 
     uint256 _amount = msg.value;
     address payable _poolAdmin = payable(msg.sender);
@@ -57,7 +57,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
     PoolDetail storage _pool = _stakingPool[_consensusAddr];
     _pool.admin = _poolAdmin;
     _pool.addr = _consensusAddr;
-    _activePoolAdminMapping[_poolAdmin] = _consensusAddr;
+    _adminOfActivePoolMapping[_poolAdmin] = _consensusAddr;
 
     _stake(_stakingPool[_consensusAddr], _poolAdmin, _amount);
     emit PoolApproved(_consensusAddr, _poolAdmin);
@@ -70,7 +70,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
     address _consensusAddr,
     uint256 _effectiveDaysOnwards,
     uint256 _commissionRate
-  ) external override poolExists(_consensusAddr) onlyPoolAdmin(_stakingPool[_consensusAddr], msg.sender) {
+  ) external override poolIsActive(_consensusAddr) onlyPoolAdmin(_stakingPool[_consensusAddr], msg.sender) {
     _validatorContract.execRequestUpdateCommissionRate(_consensusAddr, _effectiveDaysOnwards, _commissionRate);
   }
 
@@ -86,7 +86,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
     for (uint _i = 0; _i < _pools.length; _i++) {
       PoolDetail storage _pool = _stakingPool[_pools[_i]];
       // Deactivate the pool admin in the active mapping.
-      delete _activePoolAdminMapping[_pool.admin];
+      delete _adminOfActivePoolMapping[_pool.admin];
 
       // Deduct and transfer the self staking amount to the pool admin.
       _amount = _pool.stakingAmount;
@@ -104,22 +104,27 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
   /**
    * @inheritdoc ICandidateStaking
    */
-  function stake(address _consensusAddr) external payable override noEmptyValue poolExists(_consensusAddr) {
+  function stake(address _consensusAddr) external payable override noEmptyValue poolIsActive(_consensusAddr) {
     _stake(_stakingPool[_consensusAddr], msg.sender, msg.value);
   }
 
   /**
    * @inheritdoc ICandidateStaking
    */
-  function unstake(address _consensusAddr, uint256 _amount) external override nonReentrant poolExists(_consensusAddr) {
-    require(_amount > 0, "CandidateStaking: invalid amount");
-    address _delegator = msg.sender;
+  function unstake(address _consensusAddr, uint256 _amount)
+    external
+    override
+    nonReentrant
+    poolIsActive(_consensusAddr)
+  {
+    if (_amount == 0) revert ErrUnstakeZeroAmount();
+    address _requester = msg.sender;
     PoolDetail storage _pool = _stakingPool[_consensusAddr];
     uint256 _remainAmount = _pool.stakingAmount - _amount;
-    require(_remainAmount >= _minValidatorStakingAmount, "CandidateStaking: invalid staking amount left");
+    if (_remainAmount < _minValidatorStakingAmount) revert ErrStakingAmountLeft();
 
-    _unstake(_pool, _delegator, _amount);
-    require(_sendRON(payable(_delegator), _amount), "CandidateStaking: could not transfer RON");
+    _unstake(_pool, _requester, _amount);
+    if (!_unsafeSendRON(payable(_requester), _amount, 3500)) revert ErrCannotTransferRON();
   }
 
   /**
@@ -128,7 +133,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
   function requestRenounce(address _consensusAddr)
     external
     override
-    poolExists(_consensusAddr)
+    poolIsActive(_consensusAddr)
     onlyPoolAdmin(_stakingPool[_consensusAddr], msg.sender)
   {
     _validatorContract.requestRevokeCandidate(_consensusAddr, _waitingSecsToRevoke);
@@ -140,7 +145,7 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
   function requestEmergencyExit(address _consensusAddr)
     external
     override
-    poolExists(_consensusAddr)
+    poolIsActive(_consensusAddr)
     onlyPoolAdmin(_stakingPool[_consensusAddr], msg.sender)
   {
     _validatorContract.execEmergencyExit(_consensusAddr, _waitingSecsToRevoke);
@@ -158,23 +163,17 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
     uint256 _commissionRate,
     uint256 _amount
   ) internal {
-    require(_sendRON(_poolAdmin, 0), "CandidateStaking: pool admin cannot receive RON");
-    require(_sendRON(_treasuryAddr, 0), "CandidateStaking: treasury cannot receive RON");
-    require(_amount >= _minValidatorStakingAmount, "CandidateStaking: insufficient amount");
+    if (!_unsafeSendRON(_poolAdmin, 0)) revert ErrCannotInitTransferRON(_poolAdmin, "pool admin");
+    if (!_unsafeSendRON(_treasuryAddr, 0)) revert ErrCannotInitTransferRON(_treasuryAddr, "treasury");
+    if (_amount < _minValidatorStakingAmount) revert ErrInsufficientStakingAmount();
 
-    require(
-      _poolAdmin == _candidateAdmin && _candidateAdmin == _treasuryAddr,
-      "CandidateStaking: three interaction addresses must be of the same"
-    );
+    if (_poolAdmin != _candidateAdmin || _candidateAdmin != _treasuryAddr) revert ErrThreeInteractionAddrsNotEqual();
 
     address[] memory _diffAddrs = new address[](3);
     _diffAddrs[0] = _poolAdmin;
     _diffAddrs[1] = _consensusAddr;
     _diffAddrs[2] = _bridgeOperatorAddr;
-    require(
-      !AddressArrayUtils.hasDuplicate(_diffAddrs),
-      "CandidateStaking: three operation addresses must be distinct"
-    );
+    if (AddressArrayUtils.hasDuplicate(_diffAddrs)) revert ErrThreeOperationAddrsNotDistinct();
 
     _validatorContract.grantValidatorCandidate(
       _candidateAdmin,
@@ -207,11 +206,10 @@ abstract contract CandidateStaking is BaseStaking, ICandidateStaking {
     address _requester,
     uint256 _amount
   ) internal onlyPoolAdmin(_pool, _requester) {
-    require(_amount <= _pool.stakingAmount, "CandidateStaking: insufficient staking amount");
-    require(
-      _pool.lastDelegatingTimestamp[_requester] + _cooldownSecsToUndelegate <= block.timestamp,
-      "CandidateStaking: unstake too early"
-    );
+    if (_amount > _pool.stakingAmount) revert ErrInsufficientStakingAmount();
+    if (_pool.lastDelegatingTimestamp[_requester] + _cooldownSecsToUndelegate > block.timestamp) {
+      revert ErrUnstakeTooEarly();
+    }
 
     _pool.stakingAmount -= _amount;
     _changeDelegatingAmount(_pool, _requester, _pool.stakingAmount, _pool.stakingTotal - _amount);