[Staking] Fix contract events and add test cases (#6)

### Description - Mainly adds test cases for emitted events in staking contract - Renames `candidateOwner` to `candidateAdmin` for Staking contract struct - Fixes events while claiming the reward - Exposes method `getValidatorCandidateLength`
axieinfinity · Sep 13, 2022 · 2ad8aac · 2ad8aac
1 parent d2fcd65
commit 2ad8aac
Show file tree

Hide file tree

Showing 13 changed files with 447 additions and 198 deletions.
diff --git a/contracts/interfaces/IStaking.sol b/contracts/interfaces/IStaking.sol
@@ -35,12 +35,7 @@ interface IStaking is IRewardPool {
   }
 
   /// @dev Emitted when the validator candidate is proposed.
-  event ValidatorProposed(
-    address indexed consensusAddr,
-    address indexed candidateIdx,
-    uint256 amount,
-    ValidatorCandidate _info
-  );
+  event ValidatorProposed(address indexed consensusAddr, address indexed candidateAdmin, uint256 indexed candidateIdx);
   /// @dev Emitted when the candidate admin staked for themself.
   event Staked(address indexed validator, uint256 amount);
   /// @dev Emitted when the candidate admin unstaked the amount of RON from themself.
@@ -112,6 +107,11 @@ interface IStaking is IRewardPool {
    */
   function getValidatorCandidates() external view returns (ValidatorCandidate[] memory candidates);
 
+  /**
+   * @dev Returns the current candidate length.
+   */
+  function getValidatorCandidateLength() external view returns (uint256);
+
   /**
    * @dev Returns the validator candidate weights.
    */

diff --git a/contracts/staking/RewardCalculation.sol b/contracts/staking/RewardCalculation.sol
@@ -49,14 +49,11 @@ abstract contract RewardCalculation is IRewardPool {
 
     if (_reward.lastSyncedBlock <= _sPool.lastSyncedBlock) {
       uint256 _currentBalance = balanceOf(_poolAddr, _user);
-      _sReward.debited = (_currentBalance * _sPool.accumulatedRps) / 1e18 + _reward.debited - _reward.credited;
-      _sReward.balance = _currentBalance;
-      _sReward.accumulatedRps = _sPool.accumulatedRps;
+      return (_currentBalance * _sPool.accumulatedRps) / 1e18 + _reward.debited - _reward.credited;
     }
 
-    uint256 _balance = _sReward.balance;
-    uint256 _credited = (_balance * _sReward.accumulatedRps) / 1e18;
-    return (_balance * _sPool.accumulatedRps) / 1e18 + _sReward.debited - _credited;
+    uint256 _diffRps = _sPool.accumulatedRps - _sReward.accumulatedRps;
+    return (_sReward.balance * _diffRps) / 1e18 + _sReward.debited;
   }
 
   /**
@@ -126,17 +123,21 @@ abstract contract RewardCalculation is IRewardPool {
   function _claimReward(address _poolAddr, address _user) internal returns (uint256 _amount) {
     _amount = getClaimableReward(_poolAddr, _user);
     emit RewardClaimed(_poolAddr, _user, _amount);
-    SettledPool memory _sPool = _settledPool[_poolAddr];
 
+    SettledPool memory _sPool = _settledPool[_poolAddr];
     PendingRewardFields storage _reward = _pUserReward[_poolAddr][_user];
-    _reward.credited += _amount;
-    _reward.lastSyncedBlock = block.number;
-    emit PendingRewardUpdated(_poolAddr, _user, _reward.debited, _reward.credited);
-
     SettledRewardFields storage _sReward = _sUserReward[_poolAddr][_user];
+
     _sReward.debited = 0;
-    _sReward.accumulatedRps = _sPool.accumulatedRps;
+    if (_reward.lastSyncedBlock <= _sPool.lastSyncedBlock) {
+      _sReward.balance = balanceOf(_poolAddr, _user);
+      _sReward.accumulatedRps = _sPool.accumulatedRps;
+    }
     emit SettledRewardUpdated(_poolAddr, _user, _sReward.balance, 0, _sPool.accumulatedRps);
+
+    _reward.credited += _amount;
+    _reward.lastSyncedBlock = block.number;
+    emit PendingRewardUpdated(_poolAddr, _user, _reward.debited, _reward.credited);
   }
 
   /**

diff --git a/contracts/staking/Staking.sol b/contracts/staking/Staking.sol
@@ -98,6 +98,13 @@ contract Staking is IStaking, StakingManager, Initializable {
     _setMaxValidatorCandidate(_threshold);
   }
 
+  /**
+   * @inheritdoc IStaking
+   */
+  function getValidatorCandidateLength() public view override(IStaking, StakingManager) returns (uint256) {
+    return validatorCandidates.length;
+  }
+
   ///////////////////////////////////////////////////////////////////////////////////////
   //                              FUNCTIONS FOR VALIDATOR                              //
   ///////////////////////////////////////////////////////////////////////////////////////
@@ -206,13 +213,6 @@ contract Staking is IStaking, StakingManager, Initializable {
     _candidateIndex[_consensusAddr] = _candidateIdx;
   }
 
-  /**
-   * @inheritdoc StakingManager
-   */
-  function _getValidatorCandidateLength() internal view override returns (uint256) {
-    return validatorCandidates.length;
-  }
-
   /**
    * @dev Sets the governance admin address.
    *
@@ -279,13 +279,13 @@ contract Staking is IStaking, StakingManager, Initializable {
    */
   function _createValidatorCandidate(
     address _consensusAddr,
-    address _candidateOwner,
+    address _candidateAdmin,
     address payable _treasuryAddr,
     uint256 _commissionRate
   ) internal virtual override returns (ValidatorCandidate memory) {
     ValidatorCandidate storage _candidate = validatorCandidates.push();
     _candidate.consensusAddr = _consensusAddr;
-    _candidate.candidateAdmin = _candidateOwner;
+    _candidate.candidateAdmin = _candidateAdmin;
     _candidate.treasuryAddr = _treasuryAddr;
     _candidate.commissionRate = _commissionRate;
     return _candidate;

diff --git a/contracts/staking/StakingManager.sol b/contracts/staking/StakingManager.sol
@@ -14,7 +14,7 @@ abstract contract StakingManager is IStaking, RewardCalculation {
     _;
   }
 
-  modifier notCandidateOwner(address _consensusAddr) {
+  modifier notCandidateAdmin(address _consensusAddr) {
     ValidatorCandidate memory _candidate = _getCandidate(_consensusAddr);
     require(msg.sender != _candidate.candidateAdmin, "StakingManager: method caller must not be the candidate admin");
     _;
@@ -50,6 +50,11 @@ abstract contract StakingManager is IStaking, RewardCalculation {
    */
   function maxValidatorCandidate() public view virtual returns (uint256);
 
+  /**
+   * @dev IStaking
+   */
+  function getValidatorCandidateLength() public view virtual returns (uint256);
+
   ///////////////////////////////////////////////////////////////////////////////////////
   //                          FUNCTIONS FOR VALIDATOR CANDIDATE                        //
   ///////////////////////////////////////////////////////////////////////////////////////
@@ -112,9 +117,9 @@ abstract contract StakingManager is IStaking, RewardCalculation {
     address payable _treasuryAddr,
     uint256 _commissionRate,
     uint256 _amount,
-    address _candidateOwner
+    address _candidateAdmin
   ) internal returns (uint256 _candidateIdx) {
-    uint256 _length = _getValidatorCandidateLength();
+    uint256 _length = getValidatorCandidateLength();
     require(_length < maxValidatorCandidate(), "StakingManager: query for exceeded validator array length");
     require(_getCandidateIndex(_consensusAddr) == 0, "StakingManager: query for existed candidate");
     require(_amount >= minValidatorBalance(), "StakingManager: insufficient amount");
@@ -123,14 +128,9 @@ abstract contract StakingManager is IStaking, RewardCalculation {
 
     _candidateIdx = ~_length;
     _setCandidateIndex(_consensusAddr, _candidateIdx);
-    ValidatorCandidate memory _candidate = _createValidatorCandidate(
-      _consensusAddr,
-      _candidateOwner,
-      _treasuryAddr,
-      _commissionRate
-    );
-
-    emit ValidatorProposed(_consensusAddr, _candidateOwner, _amount, _candidate);
+    _createValidatorCandidate(_consensusAddr, _candidateAdmin, _treasuryAddr, _commissionRate);
+
+    emit ValidatorProposed(_consensusAddr, _candidateAdmin, _length);
   }
 
   /**
@@ -190,14 +190,14 @@ abstract contract StakingManager is IStaking, RewardCalculation {
   /**
    * @inheritdoc IStaking
    */
-  function delegate(address _consensusAddr) external payable noEmptyValue notCandidateOwner(_consensusAddr) {
+  function delegate(address _consensusAddr) external payable noEmptyValue notCandidateAdmin(_consensusAddr) {
     _delegate(_consensusAddr, msg.sender, msg.value);
   }
 
   /**
    * @inheritdoc IStaking
    */
-  function undelegate(address _consensusAddr, uint256 _amount) external notCandidateOwner(_consensusAddr) {
+  function undelegate(address _consensusAddr, uint256 _amount) external notCandidateAdmin(_consensusAddr) {
     address payable _delegator = payable(msg.sender);
     _undelegate(_consensusAddr, _delegator, _amount);
     // TODO(Thor): replace by `call` and use reentrancy gruard
@@ -211,7 +211,7 @@ abstract contract StakingManager is IStaking, RewardCalculation {
     address _consensusAddrSrc,
     address _consensusAddrDst,
     uint256 _amount
-  ) external notCandidateOwner(_consensusAddrDst) {
+  ) external notCandidateAdmin(_consensusAddrDst) {
     address _delegator = msg.sender;
     _undelegate(_consensusAddrSrc, _delegator, _amount);
     _delegate(_consensusAddrDst, _delegator, _amount);
@@ -251,7 +251,7 @@ abstract contract StakingManager is IStaking, RewardCalculation {
   function delegateRewards(address[] calldata _consensusAddrList, address _consensusAddrDst)
     external
     override
-    notCandidateOwner(_consensusAddrDst)
+    notCandidateAdmin(_consensusAddrDst)
     returns (uint256 _amount)
   {
     return _delegateRewards(msg.sender, _consensusAddrList, _consensusAddrDst);
@@ -362,17 +362,12 @@ abstract contract StakingManager is IStaking, RewardCalculation {
    */
   function _setCandidateIndex(address _consensusAddr, uint256 _candidateIdx) internal virtual;
 
-  /**
-   * @dev Returns the current validator length.
-   */
-  function _getValidatorCandidateLength() internal view virtual returns (uint256);
-
   /**
    * @dev Creates new validator candidate in the storage and returns its struct.
    */
   function _createValidatorCandidate(
     address _consensusAddr,
-    address _candidateOwner,
+    address _candidateAdmin,
     address payable _treasuryAddr,
     uint256 _commissionRate
   ) internal virtual returns (ValidatorCandidate memory);

diff --git a/package.json b/package.json
@@ -16,8 +16,7 @@
   },
   "lint-staged": {
     "contracts/**/*.sol": "prettier --write",
-    "{test,src}/!(types)/*.{js,ts}": "prettier --write",
-    "src/*.{js,ts}": "prettier --write",
+    "{test,src}/**/*.{js,ts}": "prettier --write",
     "hardhat.config.{js,ts}": "prettier --write"
   },
   "dependencies": {

diff --git a/src/deploy/proxy/ronin-validator-proxy.ts b/src/deploy/proxy/ronin-validator-proxy.ts
@@ -29,6 +29,12 @@ const deploy = async ({ getNamedAccounts, deployments }: HardhatRuntimeEnvironme
 };
 
 deploy.tags = ['RoninValidatorSetProxy'];
-deploy.dependencies = ['ProxyAdmin', 'RoninValidatorSetLogic', 'SlashIndicatorProxy', 'StakingProxy', 'StakingVestingProxy'];
+deploy.dependencies = [
+  'ProxyAdmin',
+  'RoninValidatorSetLogic',
+  'SlashIndicatorProxy',
+  'StakingProxy',
+  'StakingVestingProxy',
+];
 
 export default deploy;
diff --git a/src/utils.ts b/src/utils.ts
@@ -1,6 +1,4 @@
-import { expect } from 'chai';
-import { BigNumber, ContractTransaction } from 'ethers';
-import { Interface, LogDescription } from 'ethers/lib/utils';
+import { BigNumber } from 'ethers';
 
 export const DEFAULT_ADDRESS = '0x0000000000000000000000000000000000000000';
 export const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
@@ -11,26 +9,3 @@ export const randomBigNumber = () => {
     .join('');
   return BigNumber.from(`0x${hexString}`);
 };
-
-export const expectEvent = async (
-  contractInterface: Interface,
-  eventName: string,
-  tx: ContractTransaction,
-  expectFn: (log: LogDescription) => void,
-  eventNumbers?: number
-) => {
-  const receipt = await tx.wait();
-  const topic = contractInterface.getEventTopic(eventName);
-  let counter = 0;
-
-  for (let i = 0; i < receipt.logs.length; i++) {
-    const eventLog = receipt.logs[i];
-    if (eventLog.topics[0] == topic) {
-      counter++;
-      const event = contractInterface.parseLog(eventLog);
-      expectFn(event);
-    }
-  }
-
-  expect(counter, 'invalid number of emitted events').eq(eventNumbers);
-};