From 39dbe5ce745bb623572a331f00f8b354135a41b5 Mon Sep 17 00:00:00 2001 From: Vitomir Pavlov Date: Mon, 22 Apr 2024 19:09:13 +0300 Subject: [PATCH] optimize rsi create new custom errors for the rsi; ensure that the new RSI is between the range - 0% to 50%; set initial rsi bonus on stake position; adapt tests and create new ones for the APR; --- contracts/RewardPool/modules/APR.sol | 11 +- .../RewardPool/modules/DelegationRewards.sol | 6 +- contracts/RewardPool/modules/Vesting.sol | 2 +- contracts/common/Errors.sol | 2 + docs/RewardPool/RewardPool.md | 34 ++++ docs/RewardPool/modules/APR.md | 37 +++++ docs/RewardPool/modules/DelegationRewards.md | 34 ++++ docs/RewardPool/modules/StakingRewards.md | 34 ++++ docs/RewardPool/modules/Vesting.md | 37 +++++ test/RewardPool/APR.test.ts | 155 ++++++++++++++++++ test/ValidatorSet/Staking.test.ts | 4 +- test/ValidatorSet/ValidatorSet.test.ts | 5 + test/constants.ts | 14 +- test/helper.ts | 16 +- 14 files changed, 367 insertions(+), 24 deletions(-) create mode 100644 test/RewardPool/APR.test.ts diff --git a/contracts/RewardPool/modules/APR.sol b/contracts/RewardPool/modules/APR.sol index 150a3236..98a63f97 100644 --- a/contracts/RewardPool/modules/APR.sol +++ b/contracts/RewardPool/modules/APR.sol @@ -4,6 +4,8 @@ pragma solidity 0.8.17; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts/access/AccessControl.sol"; +import "./../../common/Errors.sol"; + contract APR is Initializable, AccessControl { uint256 public constant INITIAL_BASE_APR = 500; uint256 public constant INITIAL_MACRO_FACTOR = 7500; @@ -41,7 +43,10 @@ contract APR is Initializable, AccessControl { } function setRSI(uint256 newRSI) public onlyRole(MANAGER_ROLE) { - require(newRSI <= getMaxRSI(), "TOO_HIGH_RSI"); + if (newRSI < INITIAL_RSI_BONUS) revert TooLowRSI(newRSI, INITIAL_RSI_BONUS); + + uint256 maxRSI = getMaxRSI(); + if (newRSI > getMaxRSI()) revert TooHighRSI(newRSI, maxRSI); rsi = newRSI; } @@ -57,10 +62,10 @@ contract APR is Initializable, AccessControl { function getMaxAPR() public view returns (uint256 nominator, uint256 denominator) { // TODO: Base + vesting and RSI must return the max possible value here (implement max base) - uint256 vesting = getVestingBonus(52); + uint256 vestBonus = getVestingBonus(52); uint256 rsiBonusFactor = getMaxRSI(); - nominator = (base + vesting) * macroFactor * rsiBonusFactor; + nominator = (base + vestBonus) * macroFactor * rsiBonusFactor; denominator = 10000 * 10000 * 10000; } diff --git a/contracts/RewardPool/modules/DelegationRewards.sol b/contracts/RewardPool/modules/DelegationRewards.sol index 628b999b..c2bc3e6e 100644 --- a/contracts/RewardPool/modules/DelegationRewards.sol +++ b/contracts/RewardPool/modules/DelegationRewards.sol @@ -192,12 +192,12 @@ abstract contract DelegationRewards is RewardPoolBase, Vesting, RewardsWithdrawa uint256 newBalance = balance + amount; if (newBalance < minDelegation) revert DelegateRequirement({src: "vesting", msg: "DELEGATION_TOO_LOW"}); - // @note Potentially use memory variable to avoid get from storage twice - if (delegationPositions[validator][delegator].isMaturing()) { + VestingPosition memory position = delegationPositions[validator][delegator]; + if (position.isMaturing()) { revert DelegateRequirement({src: "vesting", msg: "POSITION_MATURING"}); } - if (delegationPositions[validator][delegator].isActive()) { + if (position.isActive()) { revert DelegateRequirement({src: "vesting", msg: "POSITION_ACTIVE"}); } diff --git a/contracts/RewardPool/modules/Vesting.sol b/contracts/RewardPool/modules/Vesting.sol index 5afa44a4..bd3f9cb0 100644 --- a/contracts/RewardPool/modules/Vesting.sol +++ b/contracts/RewardPool/modules/Vesting.sol @@ -125,7 +125,7 @@ abstract contract Vesting is APR { uint256 durationIncrease = _calculateDurationIncrease(amount, oldBalance, duration); positions[staker].duration = duration + durationIncrease; positions[staker].end = positions[staker].end + durationIncrease; - positions[staker].rsiBonus = 0; + positions[staker].rsiBonus = rsi; } /** diff --git a/contracts/common/Errors.sol b/contracts/common/Errors.sol index 0d7b75d8..7d8dc92d 100644 --- a/contracts/common/Errors.sol +++ b/contracts/common/Errors.sol @@ -10,3 +10,5 @@ error SendFailed(); error AlreadyRegistered(address validator); error InvalidCommission(uint256 commission); error InvalidMinStake(uint256 minStake); +error TooLowRSI(uint256 newRSI, uint256 minRSI); +error TooHighRSI(uint256 newRSI, uint256 maxRSI); diff --git a/docs/RewardPool/RewardPool.md b/docs/RewardPool/RewardPool.md index 4d6a2326..312dea95 100644 --- a/docs/RewardPool/RewardPool.md +++ b/docs/RewardPool/RewardPool.md @@ -1785,6 +1785,40 @@ error StakeRequirement(string src, string msg) | src | string | undefined | | msg | string | undefined | +### TooHighRSI + +```solidity +error TooHighRSI(uint256 newRSI, uint256 maxRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| maxRSI | uint256 | undefined | + +### TooLowRSI + +```solidity +error TooLowRSI(uint256 newRSI, uint256 minRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| minRSI | uint256 | undefined | + ### Unauthorized ```solidity diff --git a/docs/RewardPool/modules/APR.md b/docs/RewardPool/modules/APR.md index 30c47281..5464374b 100644 --- a/docs/RewardPool/modules/APR.md +++ b/docs/RewardPool/modules/APR.md @@ -562,3 +562,40 @@ event RoleRevoked(bytes32 indexed role, address indexed account, address indexed +## Errors + +### TooHighRSI + +```solidity +error TooHighRSI(uint256 newRSI, uint256 maxRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| maxRSI | uint256 | undefined | + +### TooLowRSI + +```solidity +error TooLowRSI(uint256 newRSI, uint256 minRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| minRSI | uint256 | undefined | + + diff --git a/docs/RewardPool/modules/DelegationRewards.md b/docs/RewardPool/modules/DelegationRewards.md index 5da330f1..1305a759 100644 --- a/docs/RewardPool/modules/DelegationRewards.md +++ b/docs/RewardPool/modules/DelegationRewards.md @@ -1559,6 +1559,40 @@ error StakeRequirement(string src, string msg) | src | string | undefined | | msg | string | undefined | +### TooHighRSI + +```solidity +error TooHighRSI(uint256 newRSI, uint256 maxRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| maxRSI | uint256 | undefined | + +### TooLowRSI + +```solidity +error TooLowRSI(uint256 newRSI, uint256 minRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| minRSI | uint256 | undefined | + ### Unauthorized ```solidity diff --git a/docs/RewardPool/modules/StakingRewards.md b/docs/RewardPool/modules/StakingRewards.md index 71745a94..d3d3e302 100644 --- a/docs/RewardPool/modules/StakingRewards.md +++ b/docs/RewardPool/modules/StakingRewards.md @@ -1509,4 +1509,38 @@ error StakeRequirement(string src, string msg) | src | string | undefined | | msg | string | undefined | +### TooHighRSI + +```solidity +error TooHighRSI(uint256 newRSI, uint256 maxRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| maxRSI | uint256 | undefined | + +### TooLowRSI + +```solidity +error TooLowRSI(uint256 newRSI, uint256 minRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| minRSI | uint256 | undefined | + diff --git a/docs/RewardPool/modules/Vesting.md b/docs/RewardPool/modules/Vesting.md index 80b97d4d..5da50514 100644 --- a/docs/RewardPool/modules/Vesting.md +++ b/docs/RewardPool/modules/Vesting.md @@ -875,3 +875,40 @@ event RoleRevoked(bytes32 indexed role, address indexed account, address indexed +## Errors + +### TooHighRSI + +```solidity +error TooHighRSI(uint256 newRSI, uint256 maxRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| maxRSI | uint256 | undefined | + +### TooLowRSI + +```solidity +error TooLowRSI(uint256 newRSI, uint256 minRSI) +``` + + + + + +#### Parameters + +| Name | Type | Description | +|---|---|---| +| newRSI | uint256 | undefined | +| minRSI | uint256 | undefined | + + diff --git a/test/RewardPool/APR.test.ts b/test/RewardPool/APR.test.ts new file mode 100644 index 00000000..03640d04 --- /dev/null +++ b/test/RewardPool/APR.test.ts @@ -0,0 +1,155 @@ +/* eslint-disable node/no-extraneous-import */ +import { loadFixture } from "@nomicfoundation/hardhat-network-helpers"; +import { expect } from "chai"; +import { + DENOMINATOR, + EPOCHS_YEAR, + ERRORS, + INITIAL_BASE_APR, + INITIAL_MACRO_FACTOR, + INITIAL_RSI_BONUS, + MAX_RSI, +} from "../constants"; +import { ethers } from "hardhat"; +import { applyMaxReward } from "../helper"; + +export function RunAPRTests(): void { + describe("Initialization", function () { + it("should initialize correctly", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + const managerRole = await rewardPool.MANAGER_ROLE(); + + expect(await rewardPool.hasRole(managerRole, this.signers.system.address)).to.be.true; + expect(await rewardPool.base()).to.be.equal(INITIAL_BASE_APR); + expect(await rewardPool.macroFactor()).to.be.equal(INITIAL_MACRO_FACTOR); + expect(await rewardPool.rsi()).to.be.equal(INITIAL_RSI_BONUS); + expect(await rewardPool.EPOCHS_YEAR()).to.be.equal(EPOCHS_YEAR); + expect(await rewardPool.DENOMINATOR()).to.be.equal(DENOMINATOR); + }); + + it("should initialize vesting bonus", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + expect(await rewardPool.getVestingBonus(1)).to.be.equal(6); + expect(await rewardPool.getVestingBonus(52)).to.be.equal(2178); + }); + + it("should get initial RSI", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + expect(await rewardPool.getDefaultRSI()).to.be.equal(INITIAL_RSI_BONUS); + }); + + it("should get max RSI", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + expect(await rewardPool.getMaxRSI()).to.be.equal(MAX_RSI); + }); + + it("should get max APR", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + const base = await rewardPool.base(); + const macroFactor = await rewardPool.macroFactor(); + const vestingBonus = await rewardPool.getVestingBonus(52); + const rsiBonus = await rewardPool.getMaxRSI(); + const nominator = base.add(vestingBonus).mul(macroFactor).mul(rsiBonus); + const denominator = DENOMINATOR.mul(DENOMINATOR).mul(DENOMINATOR); + + const maxAPR = await rewardPool.getMaxAPR(); + expect(maxAPR.nominator).to.be.equal(nominator); + expect(maxAPR.denominator).to.be.equal(denominator); + }); + + it("should apply max rewards", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + const reward = ethers.BigNumber.from(10); + const maxReward = await applyMaxReward(rewardPool, reward); + + expect(await rewardPool.applyMaxReward(reward)).to.be.equal(maxReward); + }); + + it("should get epoch max reward", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + const totalStaked = ethers.BigNumber.from(1000); + const maxAPR = await rewardPool.getMaxAPR(); + const maxEpochReward = totalStaked.mul(maxAPR.nominator).div(maxAPR.denominator).div(EPOCHS_YEAR); + + expect(await rewardPool.getEpochMaxReward(totalStaked)).to.be.equal(maxEpochReward); + }); + }); + + describe("Setters", function () { + it("should revert when trying to set base without manager role", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + const managerRole = await rewardPool.MANAGER_ROLE(); + + await expect(rewardPool.setBase(1500)).to.be.revertedWith( + ERRORS.accessControl(this.signers.accounts[0].address.toLocaleLowerCase(), managerRole) + ); + }); + + it("should revert when trying to set macro without manager role", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + const managerRole = await rewardPool.MANAGER_ROLE(); + + await expect(rewardPool.setMacro(10000)).to.be.revertedWith( + ERRORS.accessControl(this.signers.accounts[0].address.toLocaleLowerCase(), managerRole) + ); + }); + + it("should revert when trying to set rsi without manager role", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + const managerRole = await rewardPool.MANAGER_ROLE(); + + await expect(rewardPool.setRSI(12000)).to.be.revertedWith( + ERRORS.accessControl(this.signers.accounts[0].address.toLocaleLowerCase(), managerRole) + ); + }); + + it("should revert when trying to set lower rsi", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + const newRSI = 5000; + + await expect(rewardPool.connect(this.signers.system).setRSI(newRSI)) + .to.be.revertedWithCustomError(rewardPool, "TooLowRSI") + .withArgs(newRSI, INITIAL_RSI_BONUS); + }); + + it("should revert when trying to set higher rsi", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + const newRSI = 20000; + const maxRSI = await rewardPool.getMaxRSI(); + + await expect(rewardPool.connect(this.signers.system).setRSI(newRSI)) + .to.be.revertedWithCustomError(rewardPool, "TooHighRSI") + .withArgs(newRSI, maxRSI); + }); + + it("should set base", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + await rewardPool.connect(this.signers.system).setBase(1500); + + expect(await rewardPool.base()).to.be.equal(1500); + }); + + it("should set macro", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + await rewardPool.connect(this.signers.system).setMacro(10000); + + expect(await rewardPool.macroFactor()).to.be.equal(10000); + }); + + it("should set rsi", async function () { + const { rewardPool } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); + + await rewardPool.connect(this.signers.system).setRSI(12000); + + expect(await rewardPool.rsi()).to.be.equal(12000); + }); + }); +} diff --git a/test/ValidatorSet/Staking.test.ts b/test/ValidatorSet/Staking.test.ts index 50ccfa24..65de6d60 100644 --- a/test/ValidatorSet/Staking.test.ts +++ b/test/ValidatorSet/Staking.test.ts @@ -3,7 +3,7 @@ import { loadFixture, time } from "@nomicfoundation/hardhat-network-helpers"; import { expect } from "chai"; import * as hre from "hardhat"; -import { WEEK, VESTING_DURATION_WEEKS } from "../constants"; +import { WEEK, VESTING_DURATION_WEEKS, INITIAL_RSI_BONUS } from "../constants"; import { calculatePenalty, commitEpochs, getValidatorReward, registerValidator } from "../helper"; import { RunStakingClaimTests } from "../RewardPool/RewardPool.test"; @@ -185,7 +185,7 @@ export function RunStakingTests(): void { expect(vestingData.duration, "duration").to.be.equal(vestingDuration * 2); expect(vestingData.end, "end").to.be.equal(vestingData.start.add(vestingDuration * 2)); - expect(vestingData.rsiBonus, "rsiBonus").to.be.equal(0); + expect(vestingData.rsiBonus, "rsiBonus").to.be.equal(INITIAL_RSI_BONUS); await commitEpochs( systemValidatorSet, diff --git a/test/ValidatorSet/ValidatorSet.test.ts b/test/ValidatorSet/ValidatorSet.test.ts index 9854e936..a709cb8d 100644 --- a/test/ValidatorSet/ValidatorSet.test.ts +++ b/test/ValidatorSet/ValidatorSet.test.ts @@ -10,6 +10,7 @@ import { commitEpoch, generateValidatorBls, initializeContext } from "../helper" import { RunSystemTests } from "./System.test"; import { RunStakingTests } from "./Staking.test"; import { RunDelegationTests } from "./Delegation.test"; +import { RunAPRTests } from "../RewardPool/APR.test"; describe("ValidatorSet", function () { /** Variables */ @@ -332,6 +333,10 @@ describe("ValidatorSet", function () { expect(storedEpoch.epochRoot).to.equal(hre.ethers.constants.HashZero); }); + describe("APR", function () { + RunAPRTests(); + }); + describe("Whitelist", function () { it("should be modified only by the owner", async function () { const { validatorSet } = await loadFixture(this.fixtures.initializedValidatorSetStateFixture); diff --git a/test/constants.ts b/test/constants.ts index 92719364..b91e195c 100644 --- a/test/constants.ts +++ b/test/constants.ts @@ -12,8 +12,12 @@ export const INITIAL_COMMISSION = ethers.BigNumber.from(10); export const MAX_COMMISSION = ethers.BigNumber.from(100); export const WEEK = 60 * 60 * 24 * 7; export const VESTING_DURATION_WEEKS = 10; // in weeks -export const EPOCHS_YEAR = 31500; -export const DENOMINATOR = 10000; +export const EPOCHS_YEAR = ethers.BigNumber.from(31500); +export const INITIAL_BASE_APR = ethers.BigNumber.from(500); +export const INITIAL_MACRO_FACTOR = ethers.BigNumber.from(7500); +export const INITIAL_RSI_BONUS = ethers.BigNumber.from(10000); +export const DENOMINATOR = ethers.BigNumber.from(10000); +export const MAX_RSI = ethers.BigNumber.from(15000); /// @notice This bytecode is used to mock and return true with any input export const alwaysTrueBytecode = "0x600160005260206000F3"; @@ -21,3 +25,9 @@ export const alwaysTrueBytecode = "0x600160005260206000F3"; export const alwaysFalseBytecode = "0x60206000F3"; /// @notice This bytecode is used to mock and revert with any input export const alwaysRevertBytecode = "0x60006000FD"; + +export const ERRORS = { + accessControl: (account: string, role: string) => { + return `AccessControl: account ${account} is missing role ${role}`; + }, +}; diff --git a/test/helper.ts b/test/helper.ts index d4fbfdbf..75943ab6 100644 --- a/test/helper.ts +++ b/test/helper.ts @@ -249,12 +249,7 @@ export async function calculateExpectedReward( reward: BigNumber ) { // calculate expected reward based on the given apr factors - return base - .add(vestBonus) - .mul(rsi) - .mul(reward) - .div(DENOMINATOR * DENOMINATOR) - .div(EPOCHS_YEAR); + return base.add(vestBonus).mul(rsi).mul(reward).div(DENOMINATOR.mul(DENOMINATOR)).div(EPOCHS_YEAR); } export async function applyMaxReward(rewardPool: RewardPool, reward: BigNumber) { @@ -263,12 +258,7 @@ export async function applyMaxReward(rewardPool: RewardPool, reward: BigNumber) const vestBonus = await rewardPool.getVestingBonus(52); // calculate expected reward - return base - .add(vestBonus) - .mul(rsi) - .mul(reward) - .div(DENOMINATOR * DENOMINATOR) - .div(EPOCHS_YEAR); + return base.add(vestBonus).mul(rsi).mul(reward).div(DENOMINATOR.mul(DENOMINATOR)).div(EPOCHS_YEAR); } export async function applyCustomReward( @@ -284,7 +274,7 @@ export async function applyCustomReward( let divider = DENOMINATOR; if (rsi) { bonus = bonus.mul(position.rsiBonus); - divider *= DENOMINATOR; + divider = divider.mul(DENOMINATOR); } return reward.mul(bonus).div(divider).div(EPOCHS_YEAR);