[RSE-04M] Unscalable Revenue System

[chasebrownn]

RSE-04M: Unscalable Revenue System

Type	Severity	Location
Logical Fault		RevenueStreamETH.sol:L272-L305, L313-L347

Description:

The structure of the RevenueStreamETH::_claimable and RevenueStreamETH::_checkForExpiredRevenue functions prevent the system from scaling, potentially resulting in inoperable functions if many cycles are introduced (for expiry) or are left unclaimed (for users).

Impact:

Revenue will become permanently unclaimable if multiple cycles elapse due to a claim transaction breaching the block gas limit.

Similarly, expired revenue will be impossible to clear as the loop always starts at 1.

Example:

function _claimable(
    address account
) internal view returns (
    uint256 amount,
    uint256[] memory cyclesClaimable,
    uint256[] memory amountsClaimable,
    uint256 num
) {
    uint256 numCycles = cycles.length;
    uint256 lastCycle = lastClaim[account];
    uint256 i = 1;
    uint256 cycle = cycles[numCycles - i];

    cyclesClaimable = new uint256[](numCycles);
    amountsClaimable = new uint256[](numCycles);

    while (i < numCycles && cycle > lastCycle) {
        uint256 currentRevenue = revenue[cycle];

        uint256 votingPowerAtTime = Votes(votingEscrow).getPastVotes(account, cycle);
        uint256 totalPowerAtTime = Votes(votingEscrow).getPastTotalSupply(cycle);

        if (votingPowerAtTime != 0 && !expiredRevClaimed[cycle]) {
            uint256 claimableForCycle = (currentRevenue * votingPowerAtTime) / totalPowerAtTime;
            amount += claimableForCycle;

            cyclesClaimable[i-1] = cycle;
            amountsClaimable[i-1] = claimableForCycle;
            ++num;
        }

        cycle = cycles[numCycles - (++i)];
    }
}

Recommendation:

We advise the code to be refactored, permitting users to gradually catch up to the latest cycle.

As an example implementation to achieve this, the system could store the latest cycle index that was claimed in lastClaim (i.e. renamed to lastClaimIndex) and permit the user to specify the number of cycles they wish to iterate and claim.

Such a mechanism would permit the RevenueStreamETH::claimETH function to be invoke-able regardless of how many cycles have elapsed as the user could iterate f.e. 100 at a time.

Identically, the RevenueStreamETH::_checkForExpiredRevenue should store the last cycle that was "expired" and continue from there with the same approach.

[chasebrownn]

Resolved