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.

Sign up for GitHub

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

Jump to bottom

👷 KSX - Remove escrow on inflationary rewards and remove staking cooldown #252

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

👷 KSX - Remove escrow on inflationary rewards and remove staking cooldown #252

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
62462f1
👷 remove staking cooldown
Flocqst Jul 2, 2024
6006911
👷 update interface
Flocqst Jul 2, 2024
1052a6d
✅ Adapt test suite to staking cooldown removal
Flocqst Jul 2, 2024
5142e90
📚 update doc
Flocqst Jul 2, 2024
1735867
👷 Remove escrow on inflationary rewards
Flocqst Jul 5, 2024
43300af
📚 @custom:todo adjust tests to non escrowed rewards
Flocqst Jul 8, 2024
371f847
👷 Adjust compound rewards to newly non escrowed rewards
Flocqst Jul 8, 2024
b4b69a7
👷 adjust for compounding on behalf
Flocqst Jul 16, 2024
d33ee53
✅ Adjust tests to non escrowed rewards
Flocqst Jul 16, 2024
9dee0eb
✅ fix fork test
Flocqst Jul 16, 2024
fb03e81
✅ Add unstakeEscrowAdmin tests
Flocqst Jul 17, 2024
6336698
✅ rename function helper
Flocqst Jul 17, 2024
7ff1a0a
📚 Add author handle
Flocqst Jul 17, 2024
15eea33
👷 fix updateReward modifier for compoundOnBehalf
Flocqst Jul 22, 2024
9cb9509
🔀 Resolved conflicts after merging main into staking-cooldown
Flocqst Aug 27, 2024
427134b
🔀 Resolved conflicts after merging main into remove-inflation-escrow
Flocqst Aug 27, 2024
827e85c
✅ Adjust stake Escrow on behalf tests
Flocqst Aug 27, 2024
e4d6166
Merge branch 'staking-cooldown' into remove-inflation-escrow
Flocqst Aug 27, 2024
48a2b16
✅ Adjust precision delta usdc reward calculation
Flocqst Aug 27, 2024
0d173e7
👷 Remove uses of userLastStakeTime
Flocqst Sep 11, 2024
0f1db85
👷 Refactor processReward
Flocqst Sep 11, 2024
c9e48f6
👷 add updateReward(_account) modifier to _processReward()
cmontecoding Oct 28, 2024
bf210e9
✅ test_compound add assert
cmontecoding Oct 28, 2024
c6d26b6
✅ add asserts to test_compoundOnBehalf
cmontecoding Oct 28, 2024
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
56 changes: 40 additions & 16 deletions contracts/StakingRewardsV2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ contract StakingRewardsV2 is
/// @param _rewardEscrow The address for the RewardEscrowV2 contract
/// @param _rewardsNotifier The address for the StakingRewardsNotifier contract
constructor(address _kwenta, address _rewardEscrow, address _rewardsNotifier) {
if (_kwenta == address(0) || _rewardEscrow == address(0) || _rewardsNotifier == address(0)) {
if (_kwenta == address(0) || _rewardEscrow == address(0) || _rewardsNotifier == address(0))
{
revert ZeroAddress();
}

Expand Down Expand Up @@ -213,19 +214,27 @@ contract StakingRewardsV2 is
///////////////////////////////////////////////////////////////*/

/// @inheritdoc IStakingRewardsV2
function stake(uint256 _amount) external whenNotPaused updateReward(msg.sender) {
function stake(uint256 _amount) external whenNotPaused {
Flocqst marked this conversation as resolved.
Show resolved Hide resolved
_stake(msg.sender, _amount);

// transfer token to this contract from the caller
kwenta.transferFrom(msg.sender, address(this), _amount);
}

function _stake(address _account, uint256 _amount)
internal
whenNotPaused
updateReward(msg.sender)
Flocqst marked this conversation as resolved.
Show resolved Hide resolved
{
if (_amount == 0) revert AmountZero();

// update state
userLastStakeTime[msg.sender] = block.timestamp;
userLastStakeTime[_account] = block.timestamp;
jcmonte marked this conversation as resolved.
Show resolved Hide resolved
_addTotalSupplyCheckpoint(totalSupply() + _amount);
_addBalancesCheckpoint(msg.sender, balanceOf(msg.sender) + _amount);

// emit staking event and index msg.sender
emit Staked(msg.sender, _amount);
_addBalancesCheckpoint(_account, balanceOf(_account) + _amount);

// transfer token to this contract from the caller
kwenta.transferFrom(msg.sender, address(this), _amount);
// emit staking event and index _account
emit Staked(_account, _amount);
}

/// @inheritdoc IStakingRewardsV2
Expand Down Expand Up @@ -343,10 +352,25 @@ contract StakingRewardsV2 is
// emit reward claimed event and index account
emit RewardPaid(_account, reward);

// transfer token from this contract to the rewardEscrow
// and create a vesting entry at the _to address
kwenta.transfer(address(rewardEscrow), reward);
rewardEscrow.appendVestingEntry(_to, reward);
// transfer token from this contract to the account
// as newly issued rewards from inflation are now issued as non-escrowed
kwenta.transfer(_to, reward);
Flocqst marked this conversation as resolved.
Show resolved Hide resolved
}
}

function _getRewardCompounding(address _account)
Copy link
Contributor

Choose a reason for hiding this comment

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

needs natspec. i am confused what this is supposed to do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is called when compounding, basically it is doing the same as _getReward but without transfering because the rewards are staked right after this (can be staked immediatly after calling this (see _compound()).

Added natspec to specify this.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, is there a difference to this vs. the code in _getReward other than transferring? I see duplicate code. I was thinking that this reward calculating logic would be shared between the two and getReward only has the distinction of transferring to the user.

ie.

getReward() {
    (kwenta, usdc) = _calculateReward()
    transfer kwenta
    transfer USDC
}

compound() {
    (kwenta, usdc) = _calculateReward()
    stake kwenta
    transfer USDC
}

_calculateReward() {
...
}

TLDR; we should be practicing DRY here to minimize room for error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored with a _processReward so that there is no duplicate logic

internal
whenNotPaused
updateReward(_account)
returns (uint256 reward)
{
reward = rewards[_account];
if (reward > 0) {
// update state (first)
rewards[_account] = 0;

// emit reward claimed event and index account
emit RewardPaid(_account, reward);
}
}

Expand All @@ -358,8 +382,8 @@ contract StakingRewardsV2 is
/// @dev internal helper to compound for a given account
/// @param _account the account to compound for
function _compound(address _account) internal {
_getReward(_account);
_stakeEscrow(_account, unstakedEscrowedBalanceOf(_account));
uint256 reward = _getRewardCompounding(_account);
_stake(_account, reward);
}

/*///////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -502,7 +526,7 @@ contract StakingRewardsV2 is
/// @param _timestamp: timestamp to check
/// @dev returns 0 if no checkpoints exist, uses iterative binary search
/// @dev if called with a timestamp that equals the current block timestamp, then the function might return inconsistent
/// values as further transactions changing the balances can still occur within the same block.
/// values as further transactions changing the balances can still occur within the same block.
function _checkpointBinarySearch(Checkpoint[] storage _checkpoints, uint256 _timestamp)
internal
view
Expand Down
12 changes: 6 additions & 6 deletions test/foundry/integration/stakingV2.migration.fork.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ contract StakingV2MigrationForkTests is StakingTestHelpers {
// set owners address code to trick the test into allowing onlyOwner functions to be called via script
vm.etch(owner, address(new Migrate()).code);

(rewardEscrowV2, stakingRewardsV2, escrowMigrator, rewardsNotifier) = Migrate(
owner
).runCompleteMigrationProcess({
(rewardEscrowV2, stakingRewardsV2, escrowMigrator, rewardsNotifier) = Migrate(owner)
.runCompleteMigrationProcess({
_owner: owner,
_kwenta: address(kwenta),
_supplySchedule: address(supplySchedule),
Expand Down Expand Up @@ -101,8 +100,11 @@ contract StakingV2MigrationForkTests is StakingTestHelpers {
// get rewards
getStakingRewardsV2(user1);

// assert v2 rewards have been earned
assertGt(kwenta.balanceOf(user1), 0);

// stake the rewards
stakeAllUnstakedEscrowV2(user1);
stakeFundsV2(user1, kwenta.balanceOf(user1));

// check StakingRewardsV1 balance unchanged
assertEq(stakingRewardsV1.nonEscrowedBalanceOf(user1), 0);
Expand All @@ -115,8 +117,6 @@ contract StakingV2MigrationForkTests is StakingTestHelpers {
uint256 user1EscrowStakedV2 = stakingRewardsV2.escrowedBalanceOf(user1);
uint256 user1NonEscrowedStakeV2 = stakingRewardsV2.nonEscrowedBalanceOf(user1);

// assert v2 rewards have been earned
assertGt(rewardEscrowV2.escrowedBalanceOf(user1), 0);
// v2 staked balance is equal to escrowed + non-escrowed balance
assertEq(stakingRewardsV2.balanceOf(user1), user1EscrowStakedV2 + user1NonEscrowedStakeV2);
// v2 reward escrow balance is equal to escrow staked balance
Expand Down
29 changes: 17 additions & 12 deletions test/foundry/integration/stakingV2.migration.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,25 @@ contract StakingV2MigrationTests is StakingTestHelpers {
warpAndMint(2 weeks);
warpAndMint(2 weeks);

// checks everything is staked
assertEq(kwenta.balanceOf(user1), 0);
assertEq(kwenta.balanceOf(user2), 0);
assertEq(kwenta.balanceOf(user3), 0);

// get rewards
getStakingRewardsV2(user1);
getStakingRewardsV2(user2);
getStakingRewardsV2(user3);

// assert v2 rewards have been earned
assertGt(kwenta.balanceOf(user1), 0);
assertGt(kwenta.balanceOf(user2), 0);
assertGt(kwenta.balanceOf(user3), 0);

// stake the rewards
stakeAllUnstakedEscrowV2(user1);
stakeAllUnstakedEscrowV2(user2);
stakeAllUnstakedEscrowV2(user3);
stakeFundsV2(user1, kwenta.balanceOf(user1));
stakeFundsV2(user2, kwenta.balanceOf(user2));
stakeFundsV2(user3, kwenta.balanceOf(user3));

// check StakingRewardsV1 balance unchanged
assertEq(stakingRewardsV1.nonEscrowedBalanceOf(user1), 0);
Expand Down Expand Up @@ -135,11 +145,6 @@ contract StakingV2MigrationTests is StakingTestHelpers {
user2NonEscrowedStakeV2 = stakingRewardsV2.nonEscrowedBalanceOf(user2);
user3NonEscrowedStakeV2 = stakingRewardsV2.nonEscrowedBalanceOf(user3);

// assert v2 rewards have been earned
assertGt(rewardEscrowV2.escrowedBalanceOf(user1), 0);
assertGt(rewardEscrowV2.escrowedBalanceOf(user2), 0);
assertGt(rewardEscrowV2.escrowedBalanceOf(user3), 0);

// v2 staked balance is equal to escrowed + non-escrowed balance
assertEq(stakingRewardsV2.balanceOf(user1), user1EscrowStakedV2 + user1NonEscrowedStakeV2);
assertEq(stakingRewardsV2.balanceOf(user2), user2EscrowStakedV2 + user2NonEscrowedStakeV2);
Expand Down Expand Up @@ -243,8 +248,11 @@ contract StakingV2MigrationTests is StakingTestHelpers {
// get rewards
getStakingRewardsV2(stakers[i]);

// assert v2 rewards have been earned
assertGt(kwenta.balanceOf(stakers[i]), 0);

// stake the rewards
stakeAllUnstakedEscrowV2(stakers[i]);
stakeFundsV2(stakers[i], kwenta.balanceOf(stakers[i]));
}

// check StakingRewardsV1 balance unchanged
Expand All @@ -264,9 +272,6 @@ contract StakingV2MigrationTests is StakingTestHelpers {
uint256 userEscrowStakedV2 = stakingRewardsV2.escrowedBalanceOf(stakers[i]);
uint256 userNonEscrowedStakeV2 = stakingRewardsV2.nonEscrowedBalanceOf(stakers[i]);

// assert v2 rewards have been earned
assertGt(rewardEscrowV2.escrowedBalanceOf(stakers[i]), 0);

// v2 staked balance is equal to escrowed + non-escrowed balance
assertEq(
stakingRewardsV2.balanceOf(stakers[i]), userEscrowStakedV2 + userNonEscrowedStakeV2
Expand Down
14 changes: 7 additions & 7 deletions test/foundry/integration/stakingV2.upgrade.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,7 @@ contract StakingV2UpgradeTests is DefaultStakingV2Setup {
{
stakingRewardsV3Implementation = address(
new MockStakingRewardsV3(
address(kwenta),
address(rewardEscrowV2),
address(rewardsNotifier)
address(kwenta), address(rewardEscrowV2), address(rewardsNotifier)
)
);
}
Expand Down Expand Up @@ -234,14 +232,16 @@ contract StakingV2UpgradeTests is DefaultStakingV2Setup {
// claim the rewards
getStakingRewardsV2(user1);
assertEq(1 ether, stakingRewardsV2.balanceOf(user1));
assertEq(2, rewardEscrowV2.balanceOf(user1));
assertEq(1 ether + 1 weeks, rewardEscrowV2.escrowedBalanceOf(user1));
assertEq(1 ether + 1 weeks, rewardEscrowV2.unstakedEscrowedBalanceOf(user1));
assertEq(1 weeks, kwenta.balanceOf(user1));
assertEq(1, rewardEscrowV2.balanceOf(user1));
assertEq(1 ether, rewardEscrowV2.escrowedBalanceOf(user1));
assertEq(1 ether, rewardEscrowV2.unstakedEscrowedBalanceOf(user1));

// stake the rewards
stakeAllUnstakedEscrowV2(user1);
stakeFundsV2(user1, 1 weeks);
assertEq(2 ether + 1 weeks, stakingRewardsV2.balanceOf(user1));
assertEq(1 ether + 1 weeks, stakingRewardsV2.escrowedBalanceOf(user1));
assertEq(1 ether, stakingRewardsV2.escrowedBalanceOf(user1));
assertEq(0, rewardEscrowV2.unstakedEscrowedBalanceOf(user1));
Copy link
Contributor

@cmontecoding cmontecoding Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this test suite needs to also test that all the accessible storage slots are not bricked

}

Expand Down
18 changes: 6 additions & 12 deletions test/foundry/unit/StakingRewardsV2/StakingRewardsV2.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,7 @@ contract StakingRewardsV2Test is DefaultStakingV2Setup {

function test_Can_Recover_Non_Staking_Token() public {
// create mockToken
IERC20 mockToken = new Kwenta(
"Mock",
"MOCK",
INITIAL_SUPPLY,
address(this),
treasury
);
IERC20 mockToken = new Kwenta("Mock", "MOCK", INITIAL_SUPPLY, address(this), treasury);

// transfer in non staking tokens
vm.prank(treasury);
Expand Down Expand Up @@ -738,14 +732,14 @@ contract StakingRewardsV2Test is DefaultStakingV2Setup {
getReward
//////////////////////////////////////////////////////////////*/

function test_getReward_Increases_Balance_In_Escrow() public {
function test_getReward_Increases_Balance() public {
fundAndApproveAccountV2(address(this), TEST_VALUE);

uint256 initialEscrowBalance = rewardEscrowV2.escrowedBalanceOf(address(this));

// stake
stakingRewardsV2.stake(TEST_VALUE);

uint256 initialBalance = kwenta.balanceOf(address(this));

// configure reward rate
vm.prank(address(rewardsNotifier));
stakingRewardsV2.notifyRewardAmount(TEST_VALUE);
Expand All @@ -756,8 +750,8 @@ contract StakingRewardsV2Test is DefaultStakingV2Setup {
// get reward
stakingRewardsV2.getReward();

// check reward escrow balance increased
assertGt(rewardEscrowV2.escrowedBalanceOf(address(this)), initialEscrowBalance);
// check reward balance increased
assertGt(kwenta.balanceOf(address(this)), initialBalance);
}

/*//////////////////////////////////////////////////////////////
Expand Down
35 changes: 13 additions & 22 deletions test/foundry/unit/StakingRewardsV2/StakingRewardsV2Compound.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ contract StakingRewardsV2CompoundTests is DefaultStakingV2Setup {

function test_compound() public {
fundAndApproveAccountV2(address(this), TEST_VALUE);
uint256 initialEscrowBalance = rewardEscrowV2.escrowedBalanceOf(address(this));

// stake
stakingRewardsV2.stake(TEST_VALUE);

uint256 initialBalance = kwenta.balanceOf(address(this));

// configure reward rate
addNewRewardsToStakingRewardsV2(TEST_VALUE);

Expand All @@ -27,15 +28,10 @@ contract StakingRewardsV2CompoundTests is DefaultStakingV2Setup {
// compound rewards
stakingRewardsV2.compound();

// check reward escrow balance increased
uint256 finalEscrowBalance = rewardEscrowV2.escrowedBalanceOf(address(this));
assertGt(finalEscrowBalance, initialEscrowBalance);

// check all escrowed rewards were staked
uint256 totalRewards = finalEscrowBalance - initialEscrowBalance;
assertEq(totalRewards, stakingRewardsV2.escrowedBalanceOf(address(this)));
assertEq(totalRewards + TEST_VALUE, stakingRewardsV2.balanceOf(address(this)));
assertEq(rewardEscrowV2.unstakedEscrowedBalanceOf(address(this)), 0);
// check all rewards were staked and that staker balance increased
uint256 finalBalance = kwenta.balanceOf(address(this));
assertEq(initialBalance, finalBalance);
assertGt(stakingRewardsV2.balanceOf(address(this)), TEST_VALUE);
}

function test_compound_Fuzz(uint32 initialStake, uint32 newRewards) public {
Expand All @@ -45,11 +41,11 @@ contract StakingRewardsV2CompoundTests is DefaultStakingV2Setup {

fundAndApproveAccountV2(address(this), initialStake);

uint256 initialEscrowBalance = rewardEscrowV2.escrowedBalanceOf(address(this));

// stake
stakingRewardsV2.stake(initialStake);

uint256 initialBalance = kwenta.balanceOf(address(this));

// configure reward rate
addNewRewardsToStakingRewardsV2(newRewards);

Expand All @@ -59,15 +55,10 @@ contract StakingRewardsV2CompoundTests is DefaultStakingV2Setup {
// compound rewards
stakingRewardsV2.compound();

// check reward escrow balance increased
uint256 finalEscrowBalance = rewardEscrowV2.escrowedBalanceOf(address(this));
assertGt(finalEscrowBalance, initialEscrowBalance);

// check all escrowed rewards were staked
uint256 totalRewards = finalEscrowBalance - initialEscrowBalance;
assertEq(totalRewards, stakingRewardsV2.escrowedBalanceOf(address(this)));
assertEq(totalRewards + initialStake, stakingRewardsV2.balanceOf(address(this)));
assertEq(rewardEscrowV2.unstakedEscrowedBalanceOf(address(this)), 0);
// check all rewards were staked and that staker balance increased
uint256 finalBalance = kwenta.balanceOf(address(this));
assertEq(initialBalance, finalBalance);
assertGt(stakingRewardsV2.balanceOf(address(this)), initialStake);
}

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -112,7 +103,7 @@ contract StakingRewardsV2CompoundTests is DefaultStakingV2Setup {
vm.expectEmit(true, true, false, true);
emit RewardPaid(address(this), 1 weeks);
vm.expectEmit(true, true, false, true);
emit EscrowStaked(address(this), 1 weeks);
emit Staked(address(this), 1 weeks);

// compound rewards
stakingRewardsV2.compound();
Expand Down
Loading
Loading