Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding rewards per block with premint #50

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 103 additions & 11 deletions src/RewardsStreamer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
pragma solidity ^0.8.26;

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";

contract RewardsStreamer is ReentrancyGuard {
contract RewardsStreamer is ReentrancyGuard, Ownable {
error StakingManager__AmountCannotBeZero();
error StakingManager__TransferFailed();
error StakingManager__InsufficientBalance();
Expand All @@ -25,9 +26,49 @@

mapping(address account => UserInfo data) public users;

constructor(address _stakingToken, address _rewardToken) {
/**
* @dev The number of reward tokens distributed per block.
*/
uint256 public rewardsPerBlock;

/**
* @dev The block number at which the last reward calculation was performed.
*/
uint256 public lastRewardBlock;

/**
* @dev The block number at which the current reward rate ends.
*/
uint256 public rewardEndBlock;

constructor(address _stakingToken, address _rewardToken, uint256 _rewardsPerBlock) Ownable() {
STAKING_TOKEN = IERC20(_stakingToken);
REWARD_TOKEN = IERC20(_rewardToken);
rewardsPerBlock = _rewardsPerBlock;
lastRewardBlock = block.number;
}

/**
* @dev Calculates the current reward index based on the number of blocks
* since the last update and the rewards per block. This function does not
* modify the state and is used to determine the most up-to-date reward index
* for calculating user rewards.
* @return The current reward index.
*/
function currentRewardIndex() public view returns (uint256) {
if (totalStaked == 0) {
return rewardIndex;
}

uint256 blocksSinceLastUpdate = block.number - lastRewardBlock;
uint256 applicableBlocks = blocksSinceLastUpdate;

if (block.number > rewardEndBlock) {
applicableBlocks = rewardEndBlock - lastRewardBlock;
}

uint256 newRewards = applicableBlocks * rewardsPerBlock;
return rewardIndex + (newRewards * SCALE_FACTOR) / totalStaked;
}

function stake(uint256 amount) external nonReentrant {
Expand Down Expand Up @@ -77,18 +118,35 @@
user.userRewardIndex = rewardIndex;
}

function updateRewardIndex() public {
if (totalStaked == 0) {
/**
* @dev Updates the reward index and accounted rewards based on the current
* block number. This function is called before any state-modifying operations
* to ensure that the reward calculations are up-to-date. It updates the
* `rewardIndex` to reflect the latest calculated value and increments
* `accountedRewards` with the new rewards accrued since the last update.
*/
function updateRewardIndex() internal {
if (totalStaked == 0 || block.number >= rewardEndBlock) {
lastRewardBlock = block.number;
return;
}

uint256 rewardBalance = REWARD_TOKEN.balanceOf(address(this));
uint256 newRewards = rewardBalance > accountedRewards ? rewardBalance - accountedRewards : 0;
uint256 blocksSinceLastUpdate = block.number - lastRewardBlock;
uint256 applicableBlocks = blocksSinceLastUpdate;

if (newRewards > 0) {
rewardIndex += (newRewards * SCALE_FACTOR) / totalStaked;
accountedRewards += newRewards;
if (block.number > rewardEndBlock) {
applicableBlocks = rewardEndBlock - lastRewardBlock;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is problematic because rewardEndBlock is not set during construction. Meaning, even when setRewardsPerBlock is called the first time, rewardEndBlock will be 0 if I'm not mistaken.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, gotta admit I kind of neglected the initiation and edge cases 😅

}

uint256 newRewards = applicableBlocks * rewardsPerBlock;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If one wants to eliminate the REWARD_TOKEN premint, this is where a mint call for newRewards can be instead. Then, rewards are minted as state is updated and not in advance.


// Update the rewardIndex to the current calculated value
rewardIndex = rewardIndex + (newRewards * SCALE_FACTOR) / totalStaked;

// Update accountedRewards with the new rewards
accountedRewards += newRewards;

lastRewardBlock = block.number;
}

function getStakedBalance(address userAddress) public view returns (uint256) {
Expand All @@ -101,7 +159,8 @@

function calculateUserRewards(address userAddress) public view returns (uint256) {
UserInfo storage user = users[userAddress];
return (user.stakedBalance * (rewardIndex - user.userRewardIndex)) / SCALE_FACTOR;
uint256 currentIndex = currentRewardIndex();
return (user.stakedBalance * (currentIndex - user.userRewardIndex)) / SCALE_FACTOR;
}

// send the rewards and updates accountedRewards
Expand All @@ -123,4 +182,37 @@
function getUserInfo(address userAddress) public view returns (UserInfo memory) {
return users[userAddress];
}

/**
* @dev Sets the rewards per block for a specified duration. This function can only be called by the owner.
* It mints the necessary reward tokens for the specified duration, considering any unassigned rewards.
* @param _rewardsPerBlock The new reward rate per block.
* @param _durationInBlocks The duration for which the new reward rate should be applied.
*/
function setRewardsPerBlock(uint256 _rewardsPerBlock, uint256 _durationInBlocks) external onlyOwner {
require(_durationInBlocks > 0, "Duration must be greater than zero");

Check warning on line 193 in src/RewardsStreamer.sol

View workflow job for this annotation

GitHub Actions / lint

Error message for require is too long
updateRewardIndex(); // Ensure rewards are up-to-date before changing the rate

// Calculate the total rewards needed for the specified duration
uint256 totalRewardsNeeded = _rewardsPerBlock * _durationInBlocks;

// Calculate the unassigned rewards currently held by the contract
uint256 currentBalance = REWARD_TOKEN.balanceOf(address(this));
uint256 unassignedRewards = currentBalance > accountedRewards ? currentBalance - accountedRewards : 0;

// Calculate the additional rewards needed
uint256 additionalRewardsNeeded = 0;
if (totalRewardsNeeded > unassignedRewards) {
additionalRewardsNeeded = totalRewardsNeeded - unassignedRewards;
}

// Mint the necessary additional reward tokens
if (additionalRewardsNeeded > 0) {
REWARD_TOKEN.mint(address(this), additionalRewardsNeeded);
}

// Update the rewards per block and reward end block
rewardsPerBlock = _rewardsPerBlock;
rewardEndBlock = block.number + _durationInBlocks;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so here we're saying, we have an admin that is able to set some reward per block for some duration in blocks.
This allows for issuance of rewards "in the future" because there's a fixed amount of rewards being available for claim per block.

This is opposed to prior this change, where, at any point in time one would update the reward index with whatever rewards are in regardless of how many blocks have passed.

To achieve the same result, one would have to send REWARD_TOKEN to the staking contract on a per block basis, so that rewardIndex would be updatable on a per block basis with the newest rewards as well.

I think your proposed change is not bad
What I don't like that much is that the duration unit is blocks. Blocks time may very, so for the same segment of time, there could be different rewards because a different number of blocks has been processed.

I think it'd be more predictable to have a solution that uses actual time as duration instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so here we're saying, we have an admin that is able to set some reward per block for some duration in blocks.

Or you can have epochs and rewards that arrived in the past one determine the current rate of rewards - and you do not need an admin.

What I don't like that much is that the duration unit is blocks.

Just wanted to show a minimal example and this was the easiest way to do it. In terms of UX, it does make sense to use time.

}
Loading