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

Updating base reward should include only the lower bound #4

Open
cleanunicorn opened this issue Dec 20, 2019 · 0 comments
Open

Updating base reward should include only the lower bound #4

cleanunicorn opened this issue Dec 20, 2019 · 0 comments
Assignees

Comments

@cleanunicorn
Copy link
Contributor

Description

The private method _updateBaseRewardHistory updates the list containing the base rewards based on the active rewardConfig. If currentTotalStake is within the active rewardConfig lower and upper bounds, a new baseRewardHistory is not added to the list.

This functionality is implemented in _updateBaseRewardHistory:

https://github.com/ElrondNetwork/pre-staking/blob/a305562874ee0537c7277d78ed16be5178b9e8a1/contracts/StakingContract.sol#L453-L469

The interval check is done in this comparison and it verifies if the currentTotalStake is within the bounds or equal to the lower or the higher bound:

https://github.com/ElrondNetwork/pre-staking/blob/a305562874ee0537c7277d78ed16be5178b9e8a1/contracts/StakingContract.sol#L463

The user's expectation is that if they stake enough tokens to hit the higher bound, it should push the reward into the next rewardConfig.

Recommendation

To make sure that the higher bound is not included in the comparison the condition should be changed to:

// Do nothing if currentTotalStake is in the current base reward bounds
if (currentBaseReward.lowerBound <= currentTotalStake && currentTotalStake < currentBaseReward.upperBound) {
    return;
}

The higher bound check was changed from a lower than or equal to a strictly lower comparison.

Because this change affects how the upper bound is handled, the findUpperBound method has to be updated to reflect the current logic (the upper bound should be strictly lower).

The Arrays library is no longer needed, but a new method needs to be implemented.

This means that _computeCurrentBaseReward has to be changed to use the new findUpperBound method:

function _computeCurrentBaseReward()
private
view
returns (uint256)
{
    uint256 index = findIntervalIndex(rewardConfig.upperBounds, currentTotalStake);

    require(index < rewardConfig.upperBounds.length, "[NotFound] The current total staked is out of bounds");

    return index;
}

And a new method findIntervalIndex has to be implemented:

function findIntervalIndex(uint[] storage array, uint element) internal view returns(uint) {
    if (array.length == 0 || element < array[0]) {
        return 0;
    }

    for (uint i = array.length - 1; i >= 0; i--) {
        if (element >= array[i]) {
            return i + 1;
        }
    }
    
    return 0;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants