Description

In VeTranche, totalLockPoints counts the total share lockpoints of all the locked positions and is the denominator that divides all reward distributions to make sure all locks get the same quantity of reward per share per lockpoint. However, compared to the actual lockpoints, it has much less precision due to being divided by _PRECISION:

function lock(uint256 shares, uint endTime)
public nonReentrant returns (uint256) {
    //...
    lockMultiplierByTokenId[nextTokenId] =
        getLockPoints(endTime - block.timestamp);
    totalLockPoints +=
        (shares * lockMultiplierByTokenId[nextTokenId]) / _PRECISION;

This matters because the lock can be set for any number of shares and days, so the granularity with which the user has control over lockMultiplierByTokenId for a given token is much finer than _PRECISION:

function getLockPoints(uint256 timeLocked)
public view override returns (uint256) {
    uint256 lockedDays = timeLocked > getMinLockTime() ?
        (timeLocked - getMinLockTime()) / 86400 : 0;
    uint256 points = _PRECISION +
        (((lockedDays ** 2) * multiplierCoeff * _PRECISION)
            / multiplierDenom);
    return points;
}

For example, assuming the default parameters of multiplierCoeff = 1815e5 and multiplierDenom = 1960230 * _PRECISION, if a user locks one share for exactly 103 days more than the minimum, then points will be about 1.9823 * _PRECISION. After that is divided by _PRECISION, it rounds down to one.

Impact

Users wishing to lock shares can get more rewards than they are due if they tailor shares and timeLocked to take maximum benefit from this rounding.

In the most extreme case, if the user locks one share at a time for 103 days past the minimum amount of time, they get approximately twice the share than they are entitled to.

Extra USDC given out due to this precision loss causes reward insolvency, where the balance of VeTranche is less than the total outstanding rewards, which causes the last few unlockers to not be able to unlock because the VeTranche runs out of USDC.

See this POC output:

Start
 - totalLockPoints = 0
 - LP 1 reward = 0
 - LP 2 reward = 0
LP 1 locks 100 shares
 - totalLockPoints = 198
 - LP 1 reward = 0
 - LP 2 reward = 0
LP 2 locks 100x 1 share
 - totalLockPoints = 298
 - LP 1 reward = 0
 - LP 2 reward = 0
Distribute 1e10 rewards to VeTranche
LP 2 unlocks 100x
 - totalLockPoints = 198
 - LP 1 reward = 0
 - LP 2 reward = 6652010000
LP 1 tries to unlock
[FAIL. Reason: ERC20: transfer amount exceeds balance]

LP 1 and LP 2 both lock 100 shares in total, for the same amount of time, so they should both get half of the 1e10 WEI = $10,000 total reward distributed while they were locked, $5,000 each. However, when LP 1 locked their shares, totalLockPoints increased by 198, and when LP 2 locked their shares, since they did one share at a time, totalLockPoints only increased by 100.

So, when the rewards are distributed, the denominator is much lower than it is supposed to be — 298 instead of 396. Then, when LP 2 unlocks all their shares, they get $6,652.01 instead of $5,000 - 2/3 of the reward instead of half.

Later, when LP 1 tries to unlock their shares, the logic also tries to send them 2/3 of the reward but fails due to insufficient funds, since only 1/3 of the reward is actually left.

Recommendations

Track the total lockpoints with the same precision as each individual NFT's lockpoints to prevent precision loss. Verify that, after remediation, nothing can violate the invariant that the sum of all lockpoints across all lock NFTs is equal to totalLockPoints.

Remediation

This issue has been acknowledged by Avantis Labs, Inc., and a fix was implemented in commit 9c5865d3↗.