0x3b - `DripTopicFeeRevenue` drips the internal `topicFeeRevenue` and not the one provided by `GetCurrentTopicWeight`

[sherlock-admin3]

0x3b

Medium

DripTopicFeeRevenue drips the internal topicFeeRevenue and not the one provided by GetCurrentTopicWeight

Summary

DripTopicFeeRevenue drips the topicFeeRevenue storage value instead of the value provided by the calculations inside GetCurrentTopicWeight. This can lead to different drip amounts than the ones calculated.

Vulnerability Detail

When GetAndUpdateActiveTopicWeights calls DripTopicFeeRevenue, it doesn't take the previously calculated topicFeeRevenue from GetCurrentTopicWeight.

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

        weight, topicFeeRevenue, err := k.GetCurrentTopicWeight(
            ctx,
            topic.Id,
            topic.EpochLength,
            moduleParams.TopicRewardAlpha,
            moduleParams.TopicRewardStakeImportance,
            moduleParams.TopicRewardFeeRevenueImportance,
            cosmosMath.ZeroInt(),
        )

Instead, DripTopicFeeRevenue extracts the revenue from storage and drips that.

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/keeper.go#L1692-L1696

func (k *Keeper) DripTopicFeeRevenue(ctx context.Context, topicId TopicId, block BlockHeight) error {
    topicFeeRevenue, err := k.GetTopicFeeRevenue(ctx, topicId)
    if err != nil {
        return err
    }

However, if GetCurrentTopicWeight calculates a larger revenue because it includes a bonus, that bonus will never be dripped. The bonus is calculated here inside GetCurrentTopicWeight:

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/emissions/keeper/topic_weight.go#L67-L71

    newFeeRevenue := additionalRevenue.Add(topicFeeRevenue)
    feeRevenue, err := alloraMath.NewDecFromSdkInt(newFeeRevenue)
    if err != nil {
        return alloraMath.Dec{}, cosmosMath.Int{}, errors.Wrapf(err, "failed to convert topic fee revenue to dec")
    }

Impact

DripTopicFeeRevenue may drip less than what is reported in totalRevenue.

Code Snippet

        weight, topicFeeRevenue, err := k.GetCurrentTopicWeight(
            ctx,
            topic.Id,
            topic.EpochLength,
            moduleParams.TopicRewardAlpha,
            moduleParams.TopicRewardStakeImportance,
            moduleParams.TopicRewardFeeRevenueImportance,
            cosmosMath.ZeroInt(),
        )

        if err != nil {
            return errors.Wrapf(err, "failed to get current topic weight")
        }

        err = k.SetPreviousTopicWeight(ctx, topic.Id, weight)
        if err != nil {
            return errors.Wrapf(err, "failed to set previous topic weight")
        }

                //@audit doesn't take `topicFeeRevenue` as an input, but calculates its own revenue
        err = k.DripTopicFeeRevenue(ctx, topic.Id, block)
        if err != nil {
            return errors.Wrapf(err, "failed to reset topic fee revenue")
        }

Tool Used

Manual Review

Recommendation

Make DripTopicFeeRevenue take a parameter topicFeeRevenue and drip that amount.

[sherlock-admin2]

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