0x3b - `GenerateForecastScores` acidentally updates inferences scores

[sherlock-admin4]

0x3b

High

GenerateForecastScores acidentally updates inferences scores

Summary

GenerateForecastScores, used to update forecasts, will update a worker's inference scores with the forecast values. This can have a massive impact as it messes up both scores — by not updating the forecast and setting the inference to a new, different value.

Vulnerability Detail

GenerateRewardsDistributionByTopicParticipant is used to update and calculate each party's rewards. Inside it, it makes a plethora of calls, one of which is GenerateForecastScores, used to update forecast score values.

The issue we face is that in GenerateForecastScores, if there is only one forecaster, it will insert its inference score in the place of the forecast using InsertWorkerInferenceScore.

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/module/rewards/scores.go#L195

func GenerateForecastScores(
    ctx sdk.Context,
    keeper keeper.Keeper,
    topicId uint64,
    block int64,
    networkLosses types.ValueBundle,
) ([]types.Score, error) {
    var newScores []types.Score

    if len(networkLosses.ForecasterValues) == 1 {
        newScore := types.Score{
            TopicId:     topicId,
            BlockHeight: block,
            Address:     networkLosses.InfererValues[0].Worker,
            Score:       alloraMath.ZeroDec(),
        }
        //@audit H why InferenceScore when we are doing forecast ?
        err := keeper.InsertWorkerInferenceScore(ctx, topicId, block, newScore)
        ...
    }

This can pose a major issue as GetWorkersRewardFractions calculates worker rewards based on their last few scores. Insering a wrong score messes up their rewards.

Having only one forecaster may be considered rare, however, that is not the case, as these forecasts are per topic per block. This means that each topic (there can be a lot of them) can have different forecasts each new block (block time ~5 seconds). Taking into account that the chain will operate 24/7 this can occurrence can take place quite often.

Impact

Internal accounting of worker scores and rewards (they are calculated based on score) are messed up. Depending on the values, workers will receive more or less rewards than they should.

Code Snippet

func GenerateForecastScores(
    ctx sdk.Context,
    keeper keeper.Keeper,
    topicId uint64,
    block int64,
    networkLosses types.ValueBundle,
) ([]types.Score, error) {
    var newScores []types.Score

    if len(networkLosses.ForecasterValues) == 1 {
        newScore := types.Score{
            TopicId:     topicId,
            BlockHeight: block,
            Address:     networkLosses.InfererValues[0].Worker,
            Score:       alloraMath.ZeroDec(),
        }
        //@audit H why InferenceScore when we are doing forecast ?
        err := keeper.InsertWorkerInferenceScore(ctx, topicId, block, newScore)
        ...
    }

Tool used

Manual Review

Recommendation

Change the insertion to InsertWorkerForecastScore:

-   err := keeper.InsertWorkerInferenceScore(ctx, topicId, block, newScore)
+   err := keeper.InsertWorkerForecastScore(ctx, topicId, block, newScore)

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/allora-network/allora-chain/pull/466