cducrest-brainbot - Claiming cvgSDT rewards may run out of gas

[sherlock-admin]

cducrest-brainbot

medium

Claiming cvgSDT rewards may run out of gas

Summary

Claiming rewards via SdtStakingPositionService.claimCvgSdtRewards() may cost more gas than the block limit, resulting in an impossibility for users to claim any rewards.

Vulnerability Detail

The function SdtStakingPositionService.claimCvgSdtRewards() will loop through each unclaimed cycles for the user, for each loop iteration it will call _stakedAmountEligibleAtCycle() which will loop through all the staking history actions to find the last first action performed:

    function _claimCvgSdtRewards(
        uint256 tokenId
    ) internal returns (uint256, ICommonStruct.TokenAmount[] memory tokenAmounts) {
        ...
        for (; lastClaimedSdt < actualCycle; ) {
            /// @dev Retrieve the amount staked at the iterated cycle for this Staking position.
            uint256 tokenStaked = _stakedAmountEligibleAtCycle(lastClaimedSdt, tokenId, lengthHistory);
            /// @dev Retrieve the total amount staked on the iterated cycle.
            uint256 totalStaked = _cycleInfo[lastClaimedSdt].totalStaked;
            /// @dev Nothing to claim on this cycle.
            if (tokenStaked != 0) {
                /// @dev CVG PART
                ///      If the CVG rewards haven't been claimed on the iterated cycle
                if (lastClaimedCvg <= lastClaimedSdt) {
                    /// @dev Computes the staking share of the Staking Position compared to the total Staked.
                    ///      By multiplying this share by the total CVG distributed for the cycle, we get the claimable amount.
                    /// @dev Increments the total amount in CVG to mint to the user
                    _cvgClaimable += ((tokenStaked * _cycleInfo[lastClaimedSdt].cvgRewardsAmount) / totalStaked);
                }

                /// @dev StakeDao PART
                /// @dev We only do the SDT computation when SDT has been processed for the iterated cycle.
                if (_cycleInfo[lastClaimedSdt].isSdtProcessed) {
                    for (uint256 erc20Id; erc20Id < maxLengthRewards; ) {
                        /// @dev Get the ERC20 and the amount distributed on the iterated cycle.
                        ICommonStruct.TokenAmount memory rewardAsset = _sdtRewardsByCycle[lastClaimedSdt][erc20Id + 1];

                        /// @dev If there is no amount of this rewardAsset distributed on this cycle
                        if (rewardAsset.amount != 0) {
                            isSdtRewards = true;

                            /// @dev if the token is set for the first time
                            if (address(_totalRewardsClaimable[erc20Id].token) == address(0)) {
                                /// @dev Get the ERC20 and the amount distributed on the iterated cycle
                                _totalRewardsClaimable[erc20Id].token = rewardAsset.token;
                            }
                            /// @dev Computes the staking share of the Staking Position compared to the total Staked.
                            ///      By multiplying this share by the total of the StakeDao reward distributed for the cycle, we get the claimable amount.
                            /// @dev increment the total rewarded amount for the iterated ERC20
                            _totalRewardsClaimable[erc20Id].amount += ((tokenStaked * rewardAsset.amount) /
                                totalStaked);
                        }
                        unchecked {
                            ++erc20Id;
                        }
                    }
                    newLastClaimSdt = lastClaimedSdt;
                }
            }

            unchecked {
                ++lastClaimedSdt;
            }
        }
        ...
    }

    function _stakedAmountEligibleAtCycle(
        uint256 cycleId,
        uint256 tokenId,
        uint256 lengthHistory
    ) internal view returns (uint256) {
        uint256 i = lengthHistory - 1;
        uint256 historyCycle = _stakingHistoryByToken[tokenId][i];
        /// @dev Finds the cycle of the last first action performed before the {_cycleId}
        while (historyCycle > cycleId) {
            historyCycle = _stakingHistoryByToken[tokenId][i];
            unchecked {
                --i;
            }
        }

        return _tokenInfoByCycle[historyCycle][tokenId].amountStaked;
    }

There are 3 cold SLOADs for _cycleInfo plus the cost of _stakedAmountEligibleAtCycle() per loop, for each reward token there is 1 additional cold SLOAD.

The cost of _stakedAmountEligibleAtCycle() will be one SLOAD per cycle in history above the currently processed cycle. On average that will be history_length/2 warm SLOAD plus one cold SLOAD for return value. The first iteration will be at most history_length cold SLOAD.

A rough estimation of the gas cost considering only SLOAD is:

gas = number_unclaimed_cycles * (2100 + history_length * 100 + 2100 * (3 + number_rewards)) + history_length * 2100

If rewards are not claimed for 10 years for a user and there are 10 rewards token the estimated gas cost for SLOAD only is gas = 29.9M

This estimation does not take into account the gas costs associated with transferring the rewards and possibly swapping them via SdtRewardReceiver. The estimation serves to prove the possibility of the issue happening and actual time needed to wait before problem arise will be lower than 10 years.

Impact

Users that do not claim rewards over a long period of time and keep on depositing in SdtStakingPositionService will never be able to withdraw their rewards. There is no alternative function that allow withdrawing rewards for part of the total available cycles to circumvent this gas issue.

Code Snippet

https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Staking/StakeDAO/SdtStakingPositionService.sol#L429-L515

Tool used

Manual Review

Recommendation

Add a function to allow users to withdraw part of their rewards claimCvgSdtRewards(uint256 _tokenId, uint256 _toCycle, bool _isConvert, bool _isMint) with a _toCycle parameter specifying until which cycles rewards should be processed. This will allow users to withdraw all their rewards in multiple transactions and prevent running out of gas issue.

Duplicate of #198