MaverickV2VotingEscrowWSync Manual Review Findings

MVS-01M: Potentially Insecure Voting Power & Sync Balances

Type	Severity	Location
Logical Fault		MaverickV2VotingEscrowWSync.sol:L44

Description:

The present MaverickV2VotingEscrowWSync system is insecure as its trust assumption relies on a responsible party synchronizing the balances of all users continuously, ensuring that they are synchronized when their lockups expire to erase their voting power.

This assumption is not secure, as the latency involved between invocation of the MaverickV2VotingEscrowWSync::sync function and the expiry of the lockup being synchronized will result in a historical balance remaining recorded forever.

Example:

Copy
30/// @inheritdoc IMaverickV2VotingEscrowWSync
31function sync(address staker, uint256 legacyLockupIndex) public nonReentrant returns (uint256 newBalance) {
32    uint256 oldBalance = syncBalances[staker][legacyLockupIndex];
33    Lockup memory lockup = ILegacyVeMav(address(legacyVeMav)).lockups(staker, legacyLockupIndex);
34    if (lockup.end != 0 && lockup.end < block.timestamp + MIN_SYNC_DURATION)
35        revert VotingEscrowLockupEndTooShortToSync(lockup.end, block.timestamp + MIN_SYNC_DURATION);
36
37    newBalance = lockup.votes;
38    if (newBalance != oldBalance) {
39        if (newBalance > oldBalance) {
40            _mint(staker, newBalance - oldBalance);
41        } else if (newBalance < oldBalance) {
42            _burn(staker, oldBalance - newBalance);
43        }
44        syncBalances[staker][legacyLockupIndex] = newBalance;
45        emit Sync(staker, legacyLockupIndex, newBalance);
46    }
47}

Recommendation:

The problem outlined is not trivial, and any solution would significantly increase the complexity of the MaverickV2VotingEscrowWSync contract.

As a potential remediation, we advise all historical balance / power measurements to be overridden with a custom implementation that subtracts the value of any synchronized balances that have expired. For this mechanism to properly function, the MaverickV2VotingEscrowWSync would need to only mint a token balance. This means that, while the user's balance may remain non-zero forever, the voting power measurement functions would subtract the expired portion when needed leading to correct historical balance measurements by effectively introducing a secondary system on top of the existing checkpoint-based Votes system.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The Maverick Protocol team evaluated this exhibit and after engaging in discussions with our team opted to acknowledge the risks associated with the present synchronization system.

In detail, the Maverick Protocol team envision three mitigations to the proposed desynchronization that are present in the existing system\:

• A user needs to have a lock beyond 1 year to synchronize, preventing short-term attacks
• As time elapses, new vote-escrowed entries are worth exponentially more thereby gradually deprecating and devaluing synchronized locks
• The synchronization mechanism is permissionless, permitting any party with a vested interest in the Maverick Protocol to synchronize potentially out-of-date accounts

As a result of the above mitigations that are already present in the codebase, we consider the severity of the exhibit to be informational and it to be safely acknowledged.

MVS-02M: Inexistent Permittance of Downward Synchronization

Type	Severity	Location
Logical Fault		MaverickV2VotingEscrowWSync.sol:L34-L35

Description:

The MaverickV2VotingEscrowWSync::sync function does not permit a synchronization operation to be performed for a lockup that has expired which is incorrect, as a synchronization should be performed to erase the votes attached to it.

Impact:

Presently, lockups that have expired will not be possible to synchronize to zero effectively providing free voting power to users.

Example:

Copy
30/// @inheritdoc IMaverickV2VotingEscrowWSync
31function sync(address staker, uint256 legacyLockupIndex) public nonReentrant returns (uint256 newBalance) {
32    uint256 oldBalance = syncBalances[staker][legacyLockupIndex];
33    Lockup memory lockup = ILegacyVeMav(address(legacyVeMav)).lockups(staker, legacyLockupIndex);
34    if (lockup.end != 0 && lockup.end < block.timestamp + MIN_SYNC_DURATION)
35        revert VotingEscrowLockupEndTooShortToSync(lockup.end, block.timestamp + MIN_SYNC_DURATION);
36
37    newBalance = lockup.votes;
38    if (newBalance != oldBalance) {
39        if (newBalance > oldBalance) {
40            _mint(staker, newBalance - oldBalance);
41        } else if (newBalance < oldBalance) {
42            _burn(staker, oldBalance - newBalance);
43        }
44        syncBalances[staker][legacyLockupIndex] = newBalance;
45        emit Sync(staker, legacyLockupIndex, newBalance);
46    }
47}

Recommendation:

We advise the MaverickV2VotingEscrowWSync::sync function to assign 0 to the newBalance if the lockup has expired, properly erasing it if needed.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The Maverick Protocol team evaluated this exhibit and ascertained that our interpretation of the voting power system is misconstrued.

Specifically, a vote-escrowed entry should retain its voting power until it is unstaked and not until its expiry which, while non-standard, represents a desirable business requirement by the Maverick Protocol team.

As such, the exhibit is invalid given that a synchronization should only be possible when an entry is either newly introduced to the system or is meant to be removed after having been unstaked in the original VotingEscrow implementation.