MaverickV2IncentiveMatcher Code Style Findings

MVI-01C: Generic Typographic Mistakes

Type	Severity	Location
Code Style		MaverickV2IncentiveMatcher.sol:L265, L306

Description:

The referenced lines contain typographical mistakes (i.e. private variable without an underscore prefix) or generic documentational errors (i.e. copy-paste) that should be corrected.

Example:

Copy
265// check is vote period is over

Recommendation:

We advise them to be corrected enhancing the legibility of the codebase.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The referenced typographic mistakes in the comment statements have been corrected, addressing this exhibit.

MVI-02C: Incorrect Vetoing Start Period (Error Argument)

Type	Severity	Location
Logical Fault		MaverickV2IncentiveMatcher.sol:L267

Description:

The IncentiveMatcherVetoPeriodNotActive error will be yielded with an incorrect vetoStart argument as it is set to epoch.

Impact:

As the flaw is present in a revert scenario and affects the data point yielded by the emitted error, its severity is set to informational.

Example:

Copy
265// check is vote period is over
266if (!vetoingIsActive(epoch)) {
267    revert IncentiveMatcherVetoPeriodNotActive(block.timestamp, epoch, vetoingEnd(epoch));
268}

Recommendation:

We advise the epoch argument to be updated to epochEnd(epoch), properly reflecting the check performed by the MaverickV2IncentiveMatcher::vetoingIsActive function.

Alleviation (07ad29f773):

The Maverick Protocol team stated that they have remediated this exhibit in their response document, however, it remains open in the commit hash we evaluated.

Alleviation (7ad1f96f09):

The error declaration's emitted arguments have been corrected, properly signalling the veto period's start in the overall IncentiveMatcherVetoPeriodNotActive error emitted.

MVI-03C: Inefficient Voting Budget Rollover Calculation

Type	Severity	Location
Gas Optimization		MaverickV2IncentiveMatcher.sol:L545, L553

Description:

The referenced calculation is inefficient as, whenever it is evaluated, it will either result in 0 or the matchAmounts.voteBudget.

Example:

Copy
542if (rewardTotals.proRataProduct == 0) {
543    // if there was zero pro rata product, then none of the vote budget
544    // was allocated and all of it can be rolled over.
545    voteRolloverAmount = checkpoint.voteBudget;
546} else {
547    // all vote budget was allocated
548    voteRolloverAmount = 0;
549}
550
551// pro rate by the amount this matcher contributed to the budget
552if (checkpoint.voteBudget != 0)
553    voteRolloverAmount = OzMath.mulDiv(voteRolloverAmount, matchAmounts.voteBudget, checkpoint.voteBudget);

Recommendation:

We advise the conditional to be removed entirely, and the voteRolloverAmount assignment that precedes it to be assigned to the matchAmounts.voteBudget directly thus optimizing the code's gas cost.

Alleviation (7ad1f96f091b2bac58f9e800b8361a1c85753854):

The vote rollover calculations have been simplified as advised, assigning the matchAmounts.voteBudget value directly to the voteRolloverAmount when the proRataProduct is 0.

MVI-04C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		MaverickV2IncentiveMatcher.sol:L228, L229

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
228checkpoint.matcherAmounts[msg.sender].matchBudget += matchBudget;
229checkpoint.matcherAmounts[msg.sender].voteBudget += voteBudget;

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The Maverick Protocol team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase

MVI-05C: Inexistent Re-Use of Local Variable

Type	Severity	Location
Gas Optimization		MaverickV2IncentiveMatcher.sol:L536, L554, L555

Description:

The matchBudget local variable is solely used for the matchRolloverAmount calculation even though the checkpoint.matchBudget entry is re-read at least once and at most twice after it.

Example:

Copy
532// Two steps:
533// - compute total rollover amount
534// - pro rate that total by the amount this sender contributed to the
535// budget
536uint256 matchBudget = checkpoint.matchBudget;
537// roll over any unmatched. if matched pro rata, there is no match
538// budget to roll over, if not, then there will be some excess unmatched
539// allocation to roll over
540matchRolloverAmount = Math.clip(matchBudget, _getAdjustedExternalIncentives(rewardTotals));
541
542if (rewardTotals.proRataProduct == 0) {
543    // if there was zero pro rata product, then none of the vote budget
544    // was allocated and all of it can be rolled over.
545    voteRolloverAmount = checkpoint.voteBudget;
546} else {
547    // all vote budget was allocated
548    voteRolloverAmount = 0;
549}
550
551// pro rate by the amount this matcher contributed to the budget
552if (checkpoint.voteBudget != 0)
553    voteRolloverAmount = OzMath.mulDiv(voteRolloverAmount, matchAmounts.voteBudget, checkpoint.voteBudget);
554if (checkpoint.matchBudget != 0)
555    matchRolloverAmount = OzMath.mulDiv(matchRolloverAmount, matchAmounts.matchBudget, checkpoint.matchBudget);

Recommendation:

We advise the ensuing checkpoint.matchBudget read operations to be replaced by the matchBudget variable directly, optimizing the code's gas cost.

Alleviation (7ad1f96f091b2bac58f9e800b8361a1c85753854):

The two instances of checkpoint.matchBudget have been properly replaced by the existing matchBudget variable, optimizing the code's gas cost.

MVI-06C: Redundant Duplicate Safe Casts

Type	Severity	Location
Gas Optimization		MaverickV2IncentiveMatcher.sol:L259, L321

Description:

The referenced safe SafeCast::toUint128 operations are redundant as the variable has already been safely cast in the preceding statement and is thus safe to cast without any further checks.

Example:

Copy
258currentCheckpoint.totalExternalIncentivesAdded += amount.toUint128();
259currentCheckpoint.externalIncentivesByReward[rewardContract] += amount.toUint128();

Recommendation:

We advise the referenced casting operations to be performed directly without the usage of the SafeCast library, optimizing the code's gas cost whilst retaining the same level of security.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The redundant duplicate safe casts have been optimized in both pairs highlighted by this exhibit, rendering it addressed.

MVI-07C: Redundant Local Variables

Type	Severity	Location
Gas Optimization		MaverickV2IncentiveMatcher.sol:L252, L416

Description:

The referenced local variables store immutable variables to memory redundantly, increasing the gas cost of the involved statements.

Example:

Copy
252IERC20 _baseToken = baseToken;
253_baseToken.safeTransferFrom(msg.sender, address(rewardContract), amount);
254duration = rewardContract.notifyRewardAmount(_baseToken, _duration);

Recommendation:

We advise the local variables to be omitted, and all usages of the local variables to be replaced by direct usage of the respective immutable variables optimizing their gas cost.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The referenced local variables of immutable declarations have been properly removed, optimizing each statement that makes use of the immutable declaration directly.

MVI-08C: Redundant Unconditional Calculation of Maximum Vote Match

Type	Severity	Location
Gas Optimization		MaverickV2IncentiveMatcher.sol:L398-L402

Description:

The maxVoteMatch variable is calculated regardless of whether the ensuing if block which it is used in is triggered.

Example:

Copy
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 maxVoteMatch declaration to be relocated within the ensuing if block, ensuring that it is calculated and its memory is reserved solely when it is expected to be utilized.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The vote matching mechanism has been significantly refactored as a result of exhibits MVI-03M and MVI-04M, rendering this optimization no longer applicable.