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

Reduce rewards every X blocks #3

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Reduce rewards every X blocks #3

wants to merge 14 commits into from

Conversation

fbslo
Copy link
Contributor

@fbslo fbslo commented Mar 18, 2022

No description provided.

Copy link
Collaborator

@apbendi apbendi left a comment

Choose a reason for hiding this comment

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

Left an initial set of comments @fbslo. Please address or respond to all comments and notify me when complete. Please do not mark anything as resolved. I will do that when I re-review.

contracts/Minter.sol Outdated Show resolved Hide resolved
contracts/Minter.sol Outdated Show resolved Hide resolved
contracts/Minter.sol Outdated Show resolved Hide resolved
contracts/Minter.sol Outdated Show resolved Hide resolved
contracts/Minter.sol Outdated Show resolved Hide resolved
Copy link
Collaborator

@apbendi apbendi left a comment

Choose a reason for hiding this comment

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

Changes look good @fbslo, but I realized there should be a much easier way to do this. See comments below.

contracts/Minter.sol Outdated Show resolved Hide resolved
contracts/Minter.sol Outdated Show resolved Hide resolved
* @notice Update emissions for one pool
* @param index Index in the array of the pool
*/
function updateEmissions(uint256 index) public {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't know why this didn't occur to me earlier, but I think we can remove a lot of complexity and gas cost by calculating the current emission rather than actually updating the state like this. If you replace lastUpdate in the Pool struct with something like baseBlock:

  struct Pool {
    address receiver;
    uint256 amountPerBlock;
    uint256 reductionBlocks;
    uint256 reductionBps;
    uint256 baseBlock;
  }

Then you should be able to do something like this (untested):

function getPoolEmission(uint256 index) public view returns (uint256) {
    uint256 _blocksElapsed = block.number - pools[index].baseBlock;
    uint256 _reductions = _blocksElapsed / pools[index].reductionBlocks;
    uint256 _reductionBps = _reductions * pools[index].reductionBps;
    return (pools[index].amountPerBlock * (BPS - _reductionBps)) / BPS;
 }

Inside the mint function you can then use the getter above to calculate the amount

uint256 amount = getPoolEmission(i) * mintDifference;

Copy link
Contributor Author

@fbslo fbslo Apr 7, 2022

Choose a reason for hiding this comment

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

I was thinking about the same approach, but the requirements I was given were to use exponential decay ("...after the first day (28800 blocks) it would drop to 0.99 SPS / block, and then the next day it would drop to 0.9801 (1% reduction from 0.99), then 0.9702, etc..."), I tried implementing it, but the standard x*(1-y)**z) formula doesn't work in solidity.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh yes, great point! My version is not exponential. Missed that.

I tried implementing it, but the standard x*(1-y)**z) formula doesn't work in solidity.

Just out of curiosity, what kind of issue did you hit implementing this formula?

If it's not possible, we should still be able to do something iteratively. Here's a quick attempt (not tested):

  function getPoolEmission(uint256 index) public view returns (uint256) {
    uint256 _blocksElapsed = block.number - pools[index].baseBlock;
    uint256 _reductions = _blocksElapsed / pools[index].reductionBlocks;

    uint256 _reductionBps = _reductions * pools[index].reductionBps;
    uint256 _emission = pools[index].amountPerBlock;

    for (uint256 i = 0; i < _reductions; i++) {
      _emission = _emission * (BPS - _reductionBps) / BPS;

      if (minimumPayout > _emission) {
        return 0;
      }
    }

    return _emission;
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue with using x*(1-y)**z) formula was that 1-z should be less than 1, which doesn't work with solidity.

I was thinking about an iterative approach but wasn't sure if gas savings are worth it, did some testing now, and while it's cheaper (~1k gas), I don't think it's worth changing it considering it will be used on BSC anyway, so gas cost is not an issue.

While using a non-iterative approach requires calling updateEmissions every reductionBlocks, minter will be integrated with a yield farming contract where users will call it, so I don't think that will be an issue either.

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

Successfully merging this pull request may close these issues.

2 participants