Reward loss for the staker Due to Manual stakeEscrow Function in stakingRewardV2.sol.
Summary
If two users stake the same amount of tokens and then claim rewards, their rewards are vested in to rewardEscrowV2 contract. Users can compound rewards by staking rewards by calling stakeEscrow. This function should allow users to earn more by staking rewards, but in fact, it also dilutes rewards for those who do not want to restake reward tokens. It happens because stakeEscrow increases total supply and thus decreases reward per one token.
It is fair for the usual staking flow if users stake more, rewards per token decrease. But in case of stakeEscrow users are not transferring actual KWENTA tokens, they just call escrow balance inside the contract
The contract distribute the reward based on totalSuppply and individual balance inside the system to calculate there part of reward share they own, but due to contract current reward distribution logic which is inspired from synthetix due to changes made in that can result in legitimate staker facing reward losses, suppose there are 2 stakers, staker A and B, where both have deposited 50% into the pool lets assume 100 tokens by A and 100 tokens by B,totalSupply = 200, now after sometime they harvest some rewards for simplicity lets assume 10 tokens of USDC and 10 tokens KWENTA, the USDC tokens for both the user are sent to there respective addressess and KWENTA tokens are sent to RewardEscrowV2 which will be Vested there for 1 year.
The issue arises when staker A decide to call stakeEscrow and result in changing state variable which track user A deposit and totalSupply = 210 and B is didn't call stakeEscrow as he is not interested in more rewards which changes the percentage they stake in vault compare to A which was same intially. Assume B is staking from starting of protocol so he should be getting more rewards but as user A called stakeEscrow result in changing totalSupply which reduces the RewardPerToken as the supply increases rewards will get more dilute.
As stakeEscrow is not automated process the staker who don't call this function will face huge amount of loss as number of user call stakeEscrow, the intention of stakeEscrow is to give more profit to Staker but not reducing rewards for then who didn't call it as a result user will also receive less reward for USDC
Impact
Staker who did not call the stakeEscrow function will end up facing huge reward losses as number of staker grows in the system.
Code Snippet
function _stakeEscrow(address _account, uint256 _amount)
internal
whenNotPaused
updateReward(_account)
{
if (_amount == 0) revert AmountZero();
uint256 unstakedEscrow = unstakedEscrowedBalanceOf(_account);
if (_amount > unstakedEscrow) revert InsufficientUnstakedEscrow(unstakedEscrow);
// update state
userLastStakeTime[_account] = block.timestamp;
_addBalancesCheckpoint(_account, balanceOf(_account) + _amount);
_addEscrowedBalancesCheckpoint(_account, escrowedBalanceOf(_account) + _amount);
// updates total supply despite no new staking token being transfered.
// escrowed tokens are locked in RewardEscrow
_addTotalSupplyCheckpoint(totalSupply() + _amount);
// emit escrow staking event and index account
emit EscrowStaked(_account, _amount);
}
Tool used
Manual Review
Recommendation
After _getreward function is called by user, call stakeEscrow for everyone as they all vesting KWENTA for a year. Make escrow automatic for everyone
LonWof-Demon
High
Reward loss for the
stakerDue to ManualstakeEscrowFunction instakingRewardV2.sol.Summary
If two users stake the same amount of tokens and then claim rewards, their rewards are vested in to
rewardEscrowV2contract. Users can compound rewards by staking rewards by callingstakeEscrow. This function should allow users to earn more by staking rewards, but in fact, it also dilutes rewards for those who do not want to restake reward tokens. It happens becausestakeEscrowincreases total supply and thus decreases reward per one token. It is fair for the usual staking flow if users stake more, rewards per token decrease. But in case ofstakeEscrowusers are not transferring actualKWENTAtokens, they just call escrow balance inside the contractVulnerability Detail
https://www.rareskills.io/post/staking-algorithm https://github.com/sherlock-audit/2024-07-kwenta-staking-contracts/blob/main/token/contracts/StakingRewardsV2.sol#L278 https://github.com/sherlock-audit/2024-07-kwenta-staking-contracts/blob/main/token/contracts/StakingRewardsV2.sol#L435 https://github.com/sherlock-audit/2024-07-kwenta-staking-contracts/blob/main/token/contracts/StakingRewardsV2.sol#L447 https://github.com/sherlock-audit/2024-07-kwenta-staking-contracts/blob/main/token/contracts/StakingRewardsV2.sol#L464 https://github.com/sherlock-audit/2024-07-kwenta-staking-contracts/blob/main/token/contracts/StakingRewardsV2.sol#L472
The contract distribute the reward based on
totalSuppplyand individual balance inside the system to calculate there part of reward share they own, but due to contract current reward distribution logic which is inspired from synthetix due to changes made in that can result in legitimate staker facing reward losses, suppose there are 2 stakers, staker A and B, where both have deposited 50% into the pool lets assume 100 tokens by A and 100 tokens by B,totalSupply = 200, now after sometime they harvest some rewards for simplicity lets assume 10 tokens of USDC and 10 tokens KWENTA, the USDC tokens for both the user are sent to there respective addressess and KWENTA tokens are sent to RewardEscrowV2 which will be Vested there for 1 year.The issue arises when staker A decide to call stakeEscrow and result in changing state variable which track user A deposit and totalSupply = 210 and B is didn't call stakeEscrow as he is not interested in more rewards which changes the percentage they stake in vault compare to A which was same intially. Assume B is staking from starting of protocol so he should be getting more rewards but as user A called
stakeEscrowresult in changingtotalSupplywhich reduces theRewardPerTokenas the supply increases rewards will get more dilute.As
stakeEscrowis not automated process the staker who don't call this function will face huge amount of loss as number of user callstakeEscrow, the intention ofstakeEscrowis to give more profit to Staker but not reducing rewards for then who didn't call it as a result user will also receive less reward forUSDCImpact
Staker who did not call the
stakeEscrowfunction will end up facing huge reward losses as number of staker grows in the system.Code Snippet
Tool used
Manual Review
Recommendation
After
_getrewardfunction is called by user, callstakeEscrowfor everyone as they all vesting KWENTA for a year. Make escrow automatic for everyone