228 Adding StakingRewardsPerEpoch contract #237
Conversation
* adding hot fix * fix typo * Add test for hotfix * fixing naming of the governance staking contract * adding test * adding test * removing openzeppelin * moving test * removing unnecessary tests * Apply suggestions from code review * Revert "removing openzeppelin" This reverts commit 39e120c. * revert: openzeppelin files * fix: hardhat config Co-authored-by: Shloyem <[email protected]> Co-authored-by: sirpy <[email protected]>
* Add GDX recalculation for TokenPurchased * adding new airdrop calculation and staging * Add new holders info to previous airdrop * Add correct merkle tree generation * Fix for exchange purchases not recorded * Comments fixes Co-authored-by: Oleg Bedrin <[email protected]>
* Add stakers gd rewards summing script * add: use gdx mint/burn Co-authored-by: sirpy <[email protected]>
| constructor(address _rewardsToken, address _stakingToken) { | ||
| _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); | ||
| _setupRole(GUARDIAN_ROLE, msg.sender); | ||
| rewardsToken = IERC20(_rewardsToken); | ||
| stakingToken = IERC20(_stakingToken); | ||
| } |
There was a problem hiding this comment.
no need for that, this contract should not handle token transfers
| function recoverERC20(address tokenAddress, uint256 tokenAmount) | ||
| external | ||
| onlyRole(GUARDIAN_ROLE) | ||
| { | ||
| require( | ||
| tokenAddress != address(stakingToken), | ||
| "Cannot withdraw the staking token" | ||
| ); | ||
| IERC20(tokenAddress).safeTransfer(msg.sender, tokenAmount); | ||
| emit Recovered(tokenAddress, tokenAmount); | ||
| } |
There was a problem hiding this comment.
should not be in this contract
| stakersInfo[_from].balance -= _amount; | ||
| } | ||
|
|
||
| stakingToken.safeTransfer(_from, _amount); |
| } | ||
|
|
||
| stakingToken.safeTransfer(_from, _amount); | ||
| emit Withdrawn(_from, _amount); |
There was a problem hiding this comment.
add epoch index to ALL events
| pendingStakes += _amount; | ||
| stakersInfo[_from].pendingStake += _amount; | ||
| stakersInfo[_from].indexOfLastEpochStaked = lastEpochIndex; | ||
| stakingToken.safeTransferFrom(_from, address(this), _amount); |
| uint256 reward = stakersInfo[_to].reward; | ||
| if (reward > 0) { | ||
| stakersInfo[_to].reward = 0; | ||
| rewardsToken.safeTransfer(_to, reward); |
| } | ||
|
|
||
| function _notifyRewardAmount(uint256 reward) internal { | ||
| totalSupply += pendingStakes; |
There was a problem hiding this comment.
shouldnt total supply be updated AFTER we calculate the rewardsPerToken?
| } | ||
|
|
||
| function _addPendingStakesToBalanceOnTimeUpdate(address _account) internal { | ||
| if (stakersInfo[_account].indexOfLastEpochStaked != lastEpochIndex) { |
There was a problem hiding this comment.
also check if staker pendingStake > 0
| import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; | ||
| import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||
|
|
||
| contract StakingRewardsPerEpoch is AccessControl, ReentrancyGuard, Pausable { |
| event RewardPaid(address indexed user, uint256 reward); | ||
| event Recovered(address token, uint256 amount); | ||
|
|
||
| bytes32 public constant GUARDIAN_ROLE = keccak256("GUARDIAN_ROLE"); |
| _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); | ||
| _setupRole(GUARDIAN_ROLE, msg.sender); |
| IERC20 public rewardsToken; | ||
| IERC20 public stakingToken; |
| stakersInfo[_account].reward = earned(_account); | ||
| } | ||
|
|
||
| function _notifyRewardAmount(uint256 reward) internal { |
There was a problem hiding this comment.
what is reward? the newly earned rewards for the stakers?
if so rewardsPerToken is not calculated correctly. rewardsPerToken should be rewardsPerTokenSoFar+newReward/totalProductivity
see original synthetix
https://github.com/Synthetixio/synthetix/blob/89aa38664906b4f08fd393937f7f4e4ebdf267ef/contracts/StakingRewards.sol#L66
| import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; | ||
| import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||
|
|
||
| contract StakingRewardsPerEpoch is AccessControl, ReentrancyGuard, Pausable { |
There was a problem hiding this comment.
move file to staking/utils instead of staking/fuse/utils
| @@ -0,0 +1,146 @@ | |||
| // SPDX-License-Identifier: MIT | |||
There was a problem hiding this comment.
Add comments to all methods (see how we comment in our smart contracts)
Add inline comments explaining all calculations
| emit Recovered(tokenAddress, tokenAmount); | ||
| } | ||
|
|
||
| function _addPendingStakesToBalanceOnTimeUpdate(address _account) internal { |
There was a problem hiding this comment.
method name could be more precise, what is timeupdate? we have epochs
| && stakersInfo[_account].pendingStake > 0) { | ||
| stakersInfo[_account].balance += stakersInfo[_account].pendingStake; | ||
| stakersInfo[_account].pendingStake = 0; | ||
| pendingStakes -= stakersInfo[_account].pendingStake; |
There was a problem hiding this comment.
Needs to come before the line above
stakersInfo[_account].pendingStake = 0;
or we will subtract 0
| function _withdraw(address _from, uint256 _amount) internal virtual { | ||
| // if there are any pending stake for _from | ||
| if (stakersInfo[_from].pendingStake > 0) { | ||
| // if requested sum to withdraw is higher than actual pending stake balance |
There was a problem hiding this comment.
Opposite. If pending stakes are higher or equal to requested withdrawal amount than subtract the whole amount, otherwise subtract just whatever pending stake there is
| emit Staked(_from, _amount, lastEpochIndex); | ||
| } | ||
|
|
||
| function _getReward(address _to) internal virtual returns(uint256 reward) { |
There was a problem hiding this comment.
This is internal, but I don't see we call it anywhere. Reward is subtracted from user but not sent... we better discuss it
| } | ||
|
|
||
| /** | ||
| * @dev A classic ERC20 method that allows staker balance acquiring. |
There was a problem hiding this comment.
| * @dev A classic ERC20 method that allows staker balance acquiring. | |
| * @dev A classic ERC20 method that allows staker balance querying. |
| view | ||
| returns (uint256) | ||
| { | ||
| // The userEpochIndex is used to calculate a reward per toker per epoch |
There was a problem hiding this comment.
| // The userEpochIndex is used to calculate a reward per toker per epoch | |
| // The userEpochIndex is used to calculate a reward per token per epoch |
| } | ||
|
|
||
| function _addPendingStakesToBalanceOnEpoch(address _account) internal { | ||
| // If stakers balance weren't updated when he staked and the staked |
There was a problem hiding this comment.
| // If stakers balance weren't updated when he staked and the staked | |
| // If stakers balance wasn't updated when he staked and the staked |
Changes