StakingMath Manual Review Findings

SMH-01M: Non-Standard Overflow-Prone Reward Accounting

Type	Severity	Location
Mathematical Operations		StakingMath.sol: • I-1: L18 • I-2: L22 • I-3: L34 • I-4: L71

Description:

The overall StakingMath system is restricted to uint128 variables even though they incur extra gas cost to operate on and are more prone to overflow errors.

Additionally, their usage does not presently result in gas savings as all uint128 variables except for the StakeAccount::stakes data entry would occupy their full 256-bit slot regardless.

These data types result in potential casting truncations such as the one evidenced in the StakingMath::_calculateRewardsOwed function.

Impact:

The rewards of a user are presently insecurely cast to the uint128 data type and may ultimately result in fund loss in case significant rewards of high-decimal tokens are issued.

Example:

Copy
16struct StakeAccount {
17    bool disabled;
18    uint128 stakes;
19}
20
21struct Ledger {
22    uint128 totalStakes;
23    mapping(address => StakeAccount) accounts;
24    mapping(address => Reward) rewards;
25}
26
27struct Reward {
28    uint256 rewardGrowthX128;
29    mapping(address => Account) accounts;
30}
31
32struct Account {
33    uint256 rewardGrowthLastX128;
34    uint128 rewardsOwed;
35}

Recommendation:

We advise the data type to be increased to uint256, benefiting from increased accuracy in all mathematical calculations.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The totalStakes data type of the Ledger was updated to a uint256 variable, however, the rest of the entries have remained the same.

The Kyo Finance team considers the accuracy used in the rest of the data entries to be sufficient for their use-case, rendering this exhibit to be acknowledged.

SMH-02M: Incorrect Claimable Calculation

Type	Severity	Location
Logical Fault		StakingMath.sol:L145

Description:

The StakingMath::claimable function will incorrectly calculate the claimable balance by adding the result of StakingMath::_calculateRewardsOwed to the acc.rewardsOwed value even though the function already takes that value into account, effectively causing it to be counted twice.

Impact:

Any function that integrates the relevant view implementation in a state-mutating way, with the primary example being the UniswapV3Pool::_updateRewardGrowth function, will fail as it will overestimate the rewards that an account is due.

Example:

Copy
140function claimable(Ledger storage self, address rewardToken, uint128 newRewards, address account) internal view returns (uint128) {
141    Reward storage reward = self.rewards[rewardToken];
142    Account storage acc = reward.accounts[account];
143    uint256 rewardGrowthX128 = reward.rewardGrowthX128 + _calculateRewardGrowth(self.totalStakes, newRewards);
144
145    return acc.rewardsOwed + _calculateRewardsOwed(self.effectiveStakes(account), acc.rewardsOwed, rewardGrowthX128, acc.rewardGrowthLastX128);
146}

Recommendation:

We advise either the addition to be removed or the function to be revised per our gas-related optimization exhibit, either of which we consider an adequate alleviation for this exhibit.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The code was updated to ensure that past rewardsOwed values are accounted for once in the overall calculation, alleviating this exhibit in full.