VotingEscrow Code Style Findings

VEW-01C: Generic Typographic Mistake

Type	Severity	Location
Code Style		VotingEscrow.sol:L109

Description:

The referenced line contains a typographical mistake (i.e. private variable without an underscore prefix) or generic documentational error (i.e. copy-paste) that should be corrected.

Example:

Copy
109if (lockup.end == 0) revert VotingEscrowStakeAlreadRedeemed();

Recommendation:

We advise this to be done so to enhance the legibility of the codebase.

Alleviation (7ad1f96f091b2bac58f9e800b8361a1c85753854):

The typographic mistake has been corrected per our clarification, addressing this exhibit.

VEW-02C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		VotingEscrow.sol:L80, L251

Description:

The linked mathematical operation is guaranteed to be performed safely by logical inference, such as surrounding conditionals evaluated in require checks or if-else constructs.

Example:

Copy
79_lockups[to].push(lockup);
80lockupId = _lockups[to].length - 1;

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statement to be wrapped in an unchecked code block thereby optimizing its execution cost.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The referenced operations have been properly wrapped in unchecked code blocks, optimizing their gas cost whilst retaining their security guarantees via each operation's preceding statements.

VEW-03C: Inefficient Consecutive Operations

Type	Severity	Location
Gas Optimization		VotingEscrow.sol:L136, L139, L183, L189

Description:

Apart from the atomic instances identified in the VotingEscrow::_stake and VotingEscrow::_unstake functions, the VotingEscrow::merge and VotingEscrow::_extend function implementations are significantly inefficient. Specifically, the _lockups[account] storage location offset will be calculated multiple times redundantly on each VotingEscrow::_unstake and VotingEscrow::_stake invocation.

Example:

Copy
176/// @inheritdoc IMaverickV2VotingEscrowBase
177function merge(uint256[] memory lockupIds) public returns (Lockup memory newLockup) {
178    uint128 cumulativeAmount;
179    uint256 maxEnd;
180
181    Lockup memory oldLockup;
182    for (uint256 k; k < lockupIds.length; k++) {
183        oldLockup = _unstake(msg.sender, lockupIds[k]);
184        cumulativeAmount += oldLockup.amount;
185        maxEnd = Math.max(maxEnd, oldLockup.end);
186    }
187
188    // stake new lockup; checks to ensure new duration is at least min duration.
189    newLockup = _stake(cumulativeAmount, maxEnd - block.timestamp, msg.sender, lockupIds[0]);
190}

Recommendation:

We advise the VotingEscrow::_stake and VotingEscrow::_unstake functions to be potentially refactored, accepting a Lockup[] storage argument instead permitting observable optimizations to be applied.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

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

VEW-04C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		VotingEscrow.sol:L79, L80, L82, L107, L111

Description:

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

Example:

Copy
77// stake to existing or new lockup
78if (lockupId >= lockupCount(to)) {
79    _lockups[to].push(lockup);
80    lockupId = _lockups[to].length - 1;
81} else {
82    _lockups[to][lockupId] = lockup;
83}

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