Fix rewarder hooks and add RollingRewarder#31
Fix rewarder hooks and add RollingRewarder#31Zokunei wants to merge 1 commit intoByte-Masons:masterfrom
Conversation
Zokunei
left a comment
Zokunei
left a comment
There was a problem hiding this comment.
pushed a branch incorporating most of the changes recommended here, since I wanted to make changes to the base Reliquary contract to avoid all the poolBalance calculations in RollingRewarder. please still look at the comments
| import "openzeppelin-contracts/contracts/token/ERC20/extensions/IERC20Metadata.sol"; | ||
| import "openzeppelin-contracts/contracts/access/Ownable.sol"; | ||
|
|
||
| /// @title Simple rewarder that distributes its own token based on a ratio to rewards emitted by the Reliquary |
There was a problem hiding this comment.
think this was just copied from MultiplierRewarder but could use updating
| ) SingleAssetRewarder(_rewardToken, _reliquary) { | ||
| poolId = _poolId; | ||
|
|
||
| uint256[] memory _multipliers = IReliquary(_reliquary) |
There was a problem hiding this comment.
I believe these lines are equivalent to multipliers = IReliquary(_reliquary).getLevelInfo(_poolId).multipliers;
| if (_lastIssuanceTimestamp < _lastDistributionTime) { | ||
| uint256 timeLeft = _lastDistributionTime - _lastIssuanceTimestamp; //time left until final distribution | ||
| uint256 notIssued = getRewardAmount(timeLeft); //how many tokens are left to issue | ||
| amount = amount + (notIssued); // add to the funding amount that hasnt been issued |
There was a problem hiding this comment.
nit: no parentheses needed
| uint256 newAmountMultiplied = amount * multipliers[newLevel]; | ||
|
|
||
| _issueTokens( | ||
| _poolBalance() - newAmountMultiplied + oldAmountMultiplied |
There was a problem hiding this comment.
If _poolBalance() has to be offset here, best to do subtraction last to avoid underflow.
| multipliers[newLevel]; | ||
|
|
||
| _issueTokens( | ||
| _poolBalance() - newAmountMultiplied + oldAmountMultiplied |
There was a problem hiding this comment.
_poolBalance() + oldAmountMultiplied - newAmountMultiplied
| contract RewardsPool { | ||
| address public immutable RewardToken; | ||
| address public Rewarder; | ||
| uint256 public lastRetrievedTime; |
| using SafeERC20 for IERC20; | ||
|
|
||
| uint256 public immutable ACC_REWARD_PRECISION = 1e18; | ||
| uint256 public immutable REWARD_PER_SECOND_PRECISION = 1e2; |
There was a problem hiding this comment.
1e2 seems quite low, recommend at least using BPS (10_000)
| */ | ||
| function onReward( | ||
| uint relicId, | ||
| uint rewardAmount, |
There was a problem hiding this comment.
you can comment out this variable to silence the compiler warning
uint, // rewardAmount
| constructor(address _rewardToken, address rewarder) { | ||
| RewardToken = _rewardToken; | ||
| Rewarder = rewarder; | ||
| IERC20(RewardToken).approve(Rewarder, type(uint256).max); |
There was a problem hiding this comment.
probably more gas efficient to use the stack variables rather than reading from storage here
|
|
||
| contract RewardsPool { | ||
| address public immutable RewardToken; | ||
| address public Rewarder; |
There was a problem hiding this comment.
Rewarder can't be changed so may as well make it immutable
|
|
||
| pragma solidity ^0.8.15; | ||
|
|
||
| import "../Reliquary.sol"; |
| uint oldLevel, | ||
| uint newLevel | ||
| ) external override onlyReliquary { | ||
| super._onReward(relicId, rewardAmount, to); |
There was a problem hiding this comment.
maybe specify the contract we inherit from instead of using super
| override | ||
| returns (address[] memory rewardTokens, uint[] memory rewardAmounts) | ||
| { | ||
| uint length = childrenRewarders.length() + 1; |
There was a problem hiding this comment.
maybe it could make more sense to have the parent rewarder not have a reward token of its own and only manage, but is probably fine as is if it is too breaking of a change
| uint oldLevel, | ||
| uint newLevel | ||
| ) external override onlyReliquary { | ||
| super._onReward(relicId, rewardAmount, to); |
| uint256[] memory _multipliers = IReliquary(_reliquary) | ||
| .getLevelInfo(_poolId) | ||
| .multipliers; | ||
| for (uint i; i < _multipliers.length; ) { |
3 participants
No description provided.