Users do not receive inflation if the delegatee is updated

Summary

Users do not receive inflation if the delegatee is updated, leading to a loss of assets for the affected users.

Vulnerability Detail

Assume that:

Alice set her delegatee to Bob in Epoch 100, and Bob participated in the voting epoch at Epoch 101.
Alice switched her delegatee to Charles in Epoch 102, and Charles participated in the voting epoch in Epoch 103
Alice is entitled to two (2) rounds of inflation to her power token's balance.

Alice performs a sync on Epoch 104 via the _sync function. Note that the lastEpoch_ parameter of the _getUnrealizedInflation function at Line 137 is set to the current epoch (Epoch 104)

https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/abstract/EpochBasedInflationaryVoteToken.sol#L135

File: EpochBasedInflationaryVoteToken.sol
135:     function _sync(address account_) internal virtual {
136:         // Realized the account's unrealized inflation since its last sync.
137:         _addBalance(account_, _getUnrealizedInflation(account_, _clock()));
138:     }

The _getUnrealizedInflation function to compute her unrealized inflation. Alice's _getLastSync is 99. Thus, the function will start looking for inflation from Epoch 99 to Epoch 103 per the for-loop's condition at Line 289.

Note that in Line 282 below, the delegatee_ is set to _getDelegatee(Alice, 104), which will be set to Charles. As a result, the delegatee will be "locked" to Charles for the entire for-loop.

Let's run through the for-loop to determine the number of inflation that Alice will obtain:

Epoch 99 has no participation, so continue is executed. No inflation.
Epoch 100 is not a voting epoch, so continue is executed. No inflation.
Epoch 101. The _hasParticipatedAt(delegatee_, epoch_) will be executed, which will be evaluated to _hasParticipatedAt(Charles, 101). Since Bob participated instead of Charles in Epoch 101, the function will return false. No inflation.
Epoch 102 is not a voting epoch, so continue is executed. No inflation.
Epoch 103. The _hasParticipatedAt(delegatee_, epoch_) will be executed, which will be evaluated to _hasParticipatedAt(Charles, 103). Charles participated in Epoch 101, so Alice's balance will be inflated.

In summary, the above shows that only one inflation occurred instead of the two (2) that Alice is entitled to. In fact, a user will lose all their inflation gained during voting epochs where their previous delegatee took part.

https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/abstract/EpochBasedInflationaryVoteToken.sol#L275

File: EpochBasedInflationaryVoteToken.sol
275:     function _getUnrealizedInflation(address account_, uint16 lastEpoch_) internal view returns (uint240 inflation_) {
276:         // The balance and delegatee the account had at the epoch are the same since the last sync (by definition).
277:         uint240 balance_ = _getBalanceWithoutUnrealizedInflation(account_, lastEpoch_);
278: 
279:         if (balance_ == 0) return 0; // No inflation if the account had no balance.
280: 
281:         uint256 inflatedBalance_ = balance_;
282:         address delegatee_ = _getDelegatee(account_, lastEpoch_); // Internal avoids `_revertIfNotPastTimepoint`.
283: 
284:         // NOTE: Starting from the epoch after the latest sync, before `lastEpoch_`.
285:         // NOTE: If account never synced (i.e. it never interacted with the contract nor received tokens or voting
286:         //       power), then `epoch_` will start at 0, which can result in a longer loop than needed. Inheriting
287:         //       contracts should override `_getLastSync` to return the most recent appropriate epoch for such an
288:         //       account, such as the epoch when the contract was deployed, some bootstrap epoch, etc.
289:         for (uint16 epoch_ = _getLastSync(account_, lastEpoch_); epoch_ < lastEpoch_; ++epoch_) {
290:             // Skip non-voting epochs and epochs when the delegatee did not participate.
291:             if (!_isVotingEpoch(epoch_) || !_hasParticipatedAt(delegatee_, epoch_)) continue;
292: 
293:             unchecked {
294:                 inflatedBalance_ += _getInflation(uint240(inflatedBalance_));
295: 
296:                 // Cap inflation to `type(uint240).max`.
297:                 if (inflatedBalance_ >= type(uint240).max) return type(uint240).max - balance_;
298:             }
299:         }
300: 
301:         return uint240(inflatedBalance_ - balance_);
302:     }

Impact

Loss of power tokens for the affected users.

Code Snippet

https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/abstract/EpochBasedInflationaryVoteToken.sol#L135

https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/abstract/EpochBasedInflationaryVoteToken.sol#L275

Tool used

Manual Review

Recommendation

The delegatee of a user is not constant and can change between the last sync epoch and the current epoch. Thus, consider the following change to ensure that the correct delegatee is fetched for an epoch within the for-loop.

function _getUnrealizedInflation(address account_, uint16 lastEpoch_) internal view returns (uint240 inflation_) {
    // The balance and delegatee the account had at the epoch are the same since the last sync (by definition).
    uint240 balance_ = _getBalanceWithoutUnrealizedInflation(account_, lastEpoch_);

    if (balance_ == 0) return 0; // No inflation if the account had no balance.

    uint256 inflatedBalance_ = balance_;
-   address delegatee_ = _getDelegatee(account_, lastEpoch_); // Internal avoids `_revertIfNotPastTimepoint`.
    ..SNIP..
    for (uint16 epoch_ = _getLastSync(account_, lastEpoch_); epoch_ < lastEpoch_; ++epoch_) {
+       address delegatee_ = _getDelegatee(account_, epoch_);

        // Skip non-voting epochs and epochs when the delegatee did not participate.
        if (!_isVotingEpoch(epoch_) || !_hasParticipatedAt(delegatee_, epoch_)) continue;

        unchecked {
            inflatedBalance_ += _getInflation(uint240(inflatedBalance_));

            // Cap inflation to `type(uint240).max`.
            if (inflatedBalance_ >= type(uint240).max) return type(uint240).max - balance_;
        }
    }

    return uint240(inflatedBalance_ - balance_);
}

sherlock-audit / 2023-10-mzero-judging

xiaoming90 - Users do not receive inflation if the delegatee is updated #64