volodya - topic's funds is being used twice when activating a topic

[sherlock-admin2]

volodya

Medium

topic's funds is being used twice when activating a topic

Summary

Vulnerability Detail

AddTopicFeeRevenue adds funds to topic fee revenue and activateTopicIfWeightAtLeastGlobalMin adds more fee revenue doubling the amoun

func (ms msgServer) FundTopic(ctx context.Context, msg *types.MsgFundTopic) (*types.MsgFundTopicResponse, error) {
...
    err = ms.k.AddTopicFeeRevenue(ctx, msg.TopicId, msg.Amount)

    err = activateTopicIfWeightAtLeastGlobalMin(ctx, ms, msg.TopicId, msg.Amount)
    return &types.MsgFundTopicResponse{}, err
}

msgserver/msg_server_demand.go#L51

func (k *Keeper) GetCurrentTopicWeight(
    ctx context.Context,
    topicId TopicId,
    topicEpochLength BlockHeight,
    topicRewardAlpha alloraMath.Dec,
    stakeImportance alloraMath.Dec,
    feeImportance alloraMath.Dec,
    additionalRevenue cosmosMath.Int,
) (weight alloraMath.Dec, topicRevenue cosmosMath.Int, err error) {
...
    // Get and total topic fee revenue
    topicFeeRevenue, err := k.GetTopicFeeRevenue(ctx, topicId)
    if err != nil {
        return alloraMath.Dec{}, cosmosMath.Int{}, errors.Wrapf(err, "failed to get topic fee revenue")
    }

    // Calc target weight using fees, epoch length, stake, and params
    newFeeRevenue := additionalRevenue.Add(topicFeeRevenue)
    feeRevenue, err := alloraMath.NewDecFromSdkInt(newFeeRevenue)
...

    return alloraMath.ZeroDec(), topicFeeRevenue, nil
}

emissions/keeper/topic_weight.go#L68

Impact

Double accounting of topic funds

Code Snippet

Tool used

Manual Review

Recommendation

func (ms msgServer) FundTopic(ctx context.Context, msg *types.MsgFundTopic) (*types.MsgFundTopicResponse, error) {
    // Check the topic is valid
    topicExists, err := ms.k.TopicExists(ctx, msg.TopicId)
    if err != nil {
        return nil, err
    }
    if !topicExists {
        return nil, types.ErrInvalidTopicId
    }

    // Check that the request isn't spam by checking that the amount of funds it bids is greater than a global minimum demand per request
    params, err := ms.k.GetParams(ctx)
    if err != nil {
        return nil, err
    }
    amountDec, err := alloraMath.NewDecFromSdkInt(msg.Amount)
    if err != nil {
        return nil, err
    }
    if amountDec.Lte(params.Epsilon) {
        return nil, types.ErrFundAmountTooLow
    }
    // Check sender has funds to pay for the inference request
    // bank module does this for us in module SendCoins / subUnlockedCoins so we don't need to check
    // Send funds
    coins := sdk.NewCoins(sdk.NewCoin(appParams.DefaultBondDenom, msg.Amount))
    err = ms.k.SendCoinsFromAccountToModule(ctx, msg.Sender, minttypes.EcosystemModuleName, coins)
    if err != nil {
        return nil, err
    }

    // Account for the revenue the topic has generated
    err = ms.k.AddTopicFeeRevenue(ctx, msg.TopicId, msg.Amount)
    if err != nil {
        return nil, err
    }

    // Activate topic if it exhibits minimum weight
-   err = activateTopicIfWeightAtLeastGlobalMin(ctx, ms, msg.TopicId, msg.Amount)
+   err = activateTopicIfWeightAtLeastGlobalMin(ctx, ms, msg.TopicId, 0)
    return &types.MsgFundTopicResponse{}, err
}

Duplicate of #46

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Topic funding amount is incorrectly accounted for twice

[sherlock-admin2]

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