Omniscia Maverick Protocol Audit

MaverickV2IncentiveMatcher Manual Review Findings

MaverickV2IncentiveMatcher Manual Review Findings

MVI-01M: Unhandled Revert Errors

Description:

The MaverickV2IncentiveMatcher::rewardHasVe function does not properly handle a revert of the IMaverickV2Reward::tokenIndex or the IMaverickV2Reward::veTokenByIndex functions, which are expected to revert per their implementations.

Impact:

The MaverickV2IncentiveMatcher::rewardHasVe function is solely utilized in the MaverickV2IncentiveMatcher::addIncentives function and as such, its failure does not affect any sensitive operation of the contract and is expected in such a scenario rendering this exhibit to be informational.

Example:

v2-rewards/contracts/MaverickV2IncentiveMatcher.sol
209/// @inheritdoc IMaverickV2IncentiveMatcher
210function rewardHasVe(IMaverickV2Reward rewardContract) public view returns (bool) {
211    uint8 index = rewardContract.tokenIndex(baseToken);
212    // only need to check address zero as reward contract is a factor
213    // contract and the factory ensures that any non-zero ve contract is
214    // the ve contract for the base token
215    if (address(rewardContract.veTokenByIndex(index)) == address(0)) return false;
216    return true;
217}

Recommendation:

We advise the code to opportunistically acquire the index of the baseToken as well as the veToken as either function may revert and the MaverickV2IncentiveMatcher::rewardHasVe function will never reach the false return statement.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The Maverick Protocol team stated that they do not envision the MaverickV2Reward::veTokenByIndex function to revert in an unhandled manner which we agree with, however, the exhibit also details the MaverickV2Reward::tokenIndex function which may not identify the baseToken involved.

As such, we still advise the recommended alleviation to be incorporated albeit solely for the MaverickV2Reward::tokenIndex function.

MVI-02M: Misnamed Validation Check

Description:

The MaverickV2IncentiveMatcher::isEpoch function is meant to validate whether **an epoch is not correct (i.e. is not wholly divisible by EPOCH_PERIOD) which contradicts its name as well as usage by the MaverickV2IncentiveMatcher::checkEpoch function.

Impact:

Integrators of the MaverickV2IncentiveMatcher::isEpoch function will expect the function to yield whether an epoch value is a valid epoch which is not its present implementation.

Example:

v2-rewards/contracts/MaverickV2IncentiveMatcher.sol
119modifier checkEpoch(uint256 epoch) {
120    if (isEpoch(epoch)) revert IncentiveMatcherInvalidEpoch(epoch);
121    _;
122}

Recommendation:

We advise the function to be renamed or adjusted in content and usage by the MaverickV2IncentiveMatcher::checkEpoch, the latter of which we consider the correct approach.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The latter of our two recommendations was incorporated, ensuring that the MaverickV2IncentiveMatcher::isEpoch function yields whether the input epoch is valid.

Its usage was also adjusted in the MaverickV2IncentiveMatcher::checkEpoch modifier by negating it, addressing this exhibit in full.

MVI-03M: Improper Vote Budget Accounting

Description:

The voting budget is distributed incorrectly as it will never be utilized in full even if one reward contract has been voted by all parties. Specifically, the voteBudget is first proportionately reduced to the allocation of the reward contract and then further reduced by the votes accumulated by the reward contract.

The double reduction resulting from these operations results in a deficient system whereby the voting budget is improperly distributed when more than one reward contracts are involved. As an example test suite, we advise the following test to be included to the IncentiveMatcher.t.sol file:

function testVoteFullRewardsMultipleRecipients() public {
    skip(351 days);


    incentiveMatcherParameters = IMaverickV2IncentiveMatcherFactory.IncentiveMatcherParameters(
        token1,
        ve,
        rewardFactory
    );
    IMaverickV2IncentiveMatcher incentiveMatcher = new MaverickV2IncentiveMatcher();
    uint256 epoch = incentiveMatcher.currentEpoch();


    // add budget
    token1.approve(address(incentiveMatcher), MAX);
    incentiveMatcher.addMatchingBudget(10e18, 10e18, epoch);
    assertEq(token1.balanceOf(address(incentiveMatcher)), 20e18);


    IMaverickV2Reward[] memory voteTargets = new IMaverickV2Reward[](2);
    if (reward1 < reward2) {
        voteTargets[0] = reward1;
        voteTargets[1] = reward2;
    } else {
        voteTargets[1] = reward1;
        voteTargets[0] = reward2;
    }


    // Equal voting budget for both, representing 50% of the total
    uint256[] memory weights = new uint256[](2);
    weights[0] = 0.5e18;
    weights[1] = 0.5e18;
    MockVe(address(ve)).setVotes(1e18);


    // Equal incentives for both, representing 50% of the total
    MockReward(address(voteTargets[0])).setVe(ve);
    incentiveMatcher.addIncentives(voteTargets[0], 1e18, 20 days);
    MockReward(address(voteTargets[1])).setVe(ve);
    incentiveMatcher.addIncentives(voteTargets[1], 1e18, 20 days);


    vm.warp(incentiveMatcher.votingStart(epoch));


    incentiveMatcher.vote(voteTargets, weights);


    skip(8 days);


    // The matching amount is 2e18 as base, 10e18 for votes
    // Based on voting above, the system should disburse 1e18 + 5e18 for each target
    uint256 matchAmount = incentiveMatcher.distribute(voteTargets[0], epoch);
    // The actual match amount is 3.5e18, because of the following calculations in the contract:
    // - Maximum Vote Budget: 50% of total vote budget = 5e18
    // - Vote Budget Allocated Based on Votes: 50% of total votes allocated = 2.5e18
    assertEq(matchAmount, 6e18);
}


function testVoteFullRewards() public {
    skip(351 days);


    incentiveMatcherParameters = IMaverickV2IncentiveMatcherFactory.IncentiveMatcherParameters(
        token1,
        ve,
        rewardFactory
    );
    IMaverickV2IncentiveMatcher incentiveMatcher = new MaverickV2IncentiveMatcher();
    uint256 epoch = incentiveMatcher.currentEpoch();


    // add budget
    token1.approve(address(incentiveMatcher), MAX);
    incentiveMatcher.addMatchingBudget(10e18, 10e18, epoch);
    assertEq(token1.balanceOf(address(incentiveMatcher)), 20e18);


    IMaverickV2Reward[] memory voteTargets = new IMaverickV2Reward[](1);
    voteTargets[0] = reward1;


    // Single voting budget representing full votes
    uint256[] memory weights = new uint256[](1);
    weights[0] = 1e18;
    MockVe(address(ve)).setVotes(1e18);


    // Equal incentives for both, representing 50% of the total
    MockReward(address(voteTargets[0])).setVe(ve);
    incentiveMatcher.addIncentives(voteTargets[0], 1e18, 20 days);
    MockReward(address(reward2)).setVe(ve);
    incentiveMatcher.addIncentives(reward2, 1e18, 20 days);


    vm.warp(incentiveMatcher.votingStart(epoch));


    incentiveMatcher.vote(voteTargets, weights);


    skip(8 days);


    // The matching amount is 2e18 as base, 10e18 for votes
    // Based on voting above, the system should disburse 1e18 + 10e18 for the voted target, 1e18 + 0e18 for the other party
    uint256 matchAmount = incentiveMatcher.distribute(voteTargets[0], epoch);
    // The actual match amount is 6e18, because of the following calculations in the contract:
    // - Maximum Vote Budget: 50% of total vote budget = 5e18
    // - Vote Budget Allocated Based on Votes: 100% of total votes allocated = 5e18
    assertEq(matchAmount, 11e18);
}

As illustrated in the test case above, the voting budget will never be allocated in full which we consider invalid and a capital inefficiency.

Impact:

The system will never fully utilize its totalVote budget when multiple reward contracts have been matched which, combined with the fact that match budgets cannot be extracted from the system, will result in capital being permanently locked within the contract and not actively utilized.

Example:

v2-rewards/contracts/MaverickV2IncentiveMatcher.sol
398uint256 maxVoteMatch = Math.mulDivFloor(
399    checkpoint.voteBudget,
400    checkpoint.externalIncentivesByReward[rewardContract],
401    checkpoint.totalExternalIncentivesAdded
402);
403

404if (checkpoint.totalVote != 0) {
405    uint256 totalVote = checkpoint.totalVote;
406    uint256 voteMatch = Math.mulDivFloor(checkpoint.votesByReward[rewardContract], maxVoteMatch, totalVote);
407    // decrement by 1 wei to "round down" since voteMatch is the result
408    // of rounding down, straight subtraction could lead to be
409    // voteRollover to be an implicit round up operation.  But rounding
410    // up must be avoided as round up operations will accumulate and
411    // lead to a cumulative rollover value that is too large.
412    checkpoint.voteRollover += Math.clip(maxVoteMatch - voteMatch, 1).toUint128();
413

414    matchAmount += voteMatch;
415}

Recommendation:

We advise the code to properly match the voting budget per voted entry by either refactoring votes to be cast multiple times across the reward contracts and tracked once in the totalVote value of the checkpoint, or by not diminishing the initial potential budget to maxVoteMatch, either of which we consider an adequate resolution to this exhibit.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The system was revised creating a secondary pro-rata system that will scale the power of a "vote" based on the actual allocation of the reward contract the vote is cast for, permitting the ultimate voting budget allocated for a reward contract to scale with both the total incentives that are allocated for it and the votes that have been cast to it.

We thoroughly evaluated the new implementation, and confirmed that it behaves as expected and will result in either a complete distribution of the voting budget or no usage at all, permitting it to be rolled over to another epoch.

To note, some optimizations were identified and have been listed as new findings, however, we consider this exhibit fully alleviated as the voting budget inefficiency is no longer present.

MVI-04M: Insecure Rollover of Vote Budget

Description:

The rollover of excess budget can be performed only once, however, the voteRollover of a particular checkpoint may be incremented at any time.

Impact:

Any rollover of excess voting budget when not all epochs have been distributed will result in permanent loss of funds as the rollover operation is permanent and cannot be performed again.

Example:

v2-rewards/contracts/MaverickV2IncentiveMatcher.sol
368/// @inheritdoc IMaverickV2IncentiveMatcher
369function distribute(
370    IMaverickV2Reward rewardContract,
371    uint256 epoch
372) public checkEpoch(epoch) nonReentrant returns (uint256 matchAmount) {
373    _checkVetoPeriodEnded(epoch);
374

375    CheckpointData storage checkpoint = checkpoints[epoch];
376

377    if (checkpoint.totalExternalIncentivesAdded == 0)
378        revert IncentiveMatcherNoExternalIncentivesToDistributed(rewardContract, epoch);
379    if (hasDistributed[rewardContract][epoch])
380        revert IncentiveMatcherEpochAlreadyDistributed(epoch, rewardContract);
381

382    // compute how much this reward gets matched;
383    // half match budget goes to straight matching;
384    // need to check if we have enough for full match or if we have to pro rate
385    uint256 matchBudget = checkpoint.matchBudget;
386    if (matchBudget >= checkpoint.totalExternalIncentivesAdded) {
387        // straight match
388        matchAmount = checkpoint.externalIncentivesByReward[rewardContract];
389    } else {
390        // pro rate the match
391        matchAmount = Math.mulDivFloor(
392            matchBudget,
393            checkpoint.externalIncentivesByReward[rewardContract],
394            checkpoint.totalExternalIncentivesAdded
395        );
396    }
397

398    uint256 maxVoteMatch = Math.mulDivFloor(
399        checkpoint.voteBudget,
400        checkpoint.externalIncentivesByReward[rewardContract],
401        checkpoint.totalExternalIncentivesAdded
402    );
403

404    if (checkpoint.totalVote != 0) {
405        uint256 totalVote = checkpoint.totalVote;
406        uint256 voteMatch = Math.mulDivFloor(checkpoint.votesByReward[rewardContract], maxVoteMatch, totalVote);
407        // decrement by 1 wei to "round down" since voteMatch is the result
408        // of rounding down, straight subtraction could lead to be
409        // voteRollover to be an implicit round up operation.  But rounding
410        // up must be avoided as round up operations will accumulate and
411        // lead to a cumulative rollover value that is too large.
412        checkpoint.voteRollover += Math.clip(maxVoteMatch - voteMatch, 1).toUint128();
413

414        matchAmount += voteMatch;
415    }
416    IERC20 _baseToken = baseToken;
417    if (matchAmount != 0) {
418        // send match to reward and notify
419        _baseToken.safeTransfer(address(rewardContract), matchAmount);
420        rewardContract.notifyRewardAmount(_baseToken, NOTIFY_PERIOD);
421    }
422    hasDistributed[rewardContract][epoch] = true;
423

424    emit Distribute(epoch, rewardContract, _baseToken, matchAmount);
425}
426

427/// @inheritdoc IMaverickV2IncentiveMatcher
428function rolloverExcessBudget(
429    uint256 matchedEpoch,
430    uint256 newEpoch
431)
432    public
433    checkEpoch(matchedEpoch)
434    checkEpoch(newEpoch)
435    returns (uint256 matchRolloverAmount, uint256 voteRolloverAmount)
436{
437    // can only rollover after vetoing ended
438    _checkVetoPeriodEnded(matchedEpoch);
439

440    CheckpointData storage checkpoint = checkpoints[matchedEpoch];
441    MatchPair memory matchAmounts = checkpoint.matcherAmounts[msg.sender];
442    // check if any budget to rollover for this sender
443    if (matchAmounts.voteBudget == 0 && matchAmounts.matchBudget == 0)
444        revert IncentiveMatcherNothingToRollover(msg.sender, matchedEpoch);
445

446    // Two steps:
447    // - compute total rollover amount
448    // - pro rate that total by the amount this sender contributed to the
449    // budget
450    uint256 matchBudget = checkpoint.matchBudget;
451    // roll over any unmatched. if we matched pro rata, there is no match
452    // budget to roll over, if not, then there will be some excess unmatch
453    // allocation to roll over
454    matchRolloverAmount = Math.clip(matchBudget, checkpoint.totalExternalIncentivesAdded);
455

456    if (checkpoint.totalVote == 0) {
457        // if there was zero voting, then none of the vote budget was
458        // allocated and all of it can be rolledover.
459        voteRolloverAmount = checkpoint.voteBudget;
460    } else {
461        voteRolloverAmount = checkpoint.voteRollover;
462    }
463

464    // pro rate by the amount this matcher contributed to the budget
465    voteRolloverAmount = Math.mulDivDown(voteRolloverAmount, matchAmounts.voteBudget, checkpoint.voteBudget);
466    matchRolloverAmount = Math.mulDivDown(matchRolloverAmount, matchAmounts.matchBudget, checkpoint.matchBudget);
467

468    // delete budget account so user can not rollover twice.
469    delete checkpoint.matcherAmounts[msg.sender];
470    emit BudgetRolledOver(msg.sender, matchRolloverAmount, voteRolloverAmount, matchedEpoch, newEpoch);
471

472    // add budgets to new epoch; checks that new epoch is not over yet
473    _addBudget(matchRolloverAmount.toUint128(), voteRolloverAmount.toUint128(), newEpoch);
474}

Recommendation:

We advise the MaverickV2IncentiveMatcher::rolloverExcessBudget function to ensure that all reward contracts of a particular epoch have been correctly distributed before the voting budget is rolled over.

This trait needs to be separately tracked in the MaverickV2IncentiveMatcher::distribute function, potentially by keeping track of a new totalExternalIncentivesMatched variable.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The overall vote budget accounting system has been refactored to disburse the voteBudget in full if at least a single vote was cast, effectively permitting roll-overs to either happen in full or not happen at all.

As such, the code was updated to prevent roll-overs of voting budget in epochs where votes were cast effectively ensuring the revamped system will correctly roll-over the voting budget under all cases, alleviating this exhibit in full.

Prev

MaverickV2VotingEscrowFactory.sol (MVY-S)

Next

MaverickV2Reward.sol (MVR-M)

On this page