LZ_security - By transferring uallo tokens to another chain via IBC, the reward amount is affected.

[sherlock-admin3]

LZ_security

Medium

By transferring uallo tokens to another chain via IBC, the reward amount is affected.

Summary

By transferring uallo tokens to another chain via IBC, the reward amount is affected.

Vulnerability Detail

func GetEmissionPerMonth(
    ctx sdk.Context,
    k keeper.Keeper,
    blocksPerMonth uint64,
    params types.Params,
    ecosystemMintSupplyRemaining math.Int,
    validatorsPercent math.LegacyDec,
) (
    emissionPerMonth math.Int,
    emissionPerUnitStakedToken math.LegacyDec,
    err error,
) {
    // Get the expected amount of emissions this block
    networkStaked, err := keeper.GetNumStakedTokens(ctx, k) 
    if err != nil {
        return math.Int{}, math.LegacyDec{}, err
    }
@>> totalSupply := k.GetTotalCurrTokenSupply(ctx).Amount //@audit 
    lockedSupply := keeper.GetLockedTokenSupply(
        blocksPerMonth,
        math.NewIntFromUint64(uint64(ctx.BlockHeight())),
        params,
    )
@>> circulatingSupply := totalSupply.Sub(lockedSupply) //
    if circulatingSupply.IsNegative() {
        circulatingSupply = math.ZeroInt()
    }
    // T_{total,i} = ecosystemMintableRemaining
    // N_{staked,i} = networkStaked
    // N_{circ,i} = circulatingSupply
    // N_{total,i} = totalSupply
    ctx.Logger().Info("Emission Per Unit Staked Token Calculation",
        "FEmission", params.FEmission.String(),
        "ecosystemMintSupplyRemaining", ecosystemMintSupplyRemaining.String(),
        "networkStaked", networkStaked.String(),
        "circulatingSupply", circulatingSupply.String(),
        "totalSupply", totalSupply.String(),
        "lockedSupply", lockedSupply.String(),
    )
@>> targetRewardEmissionPerUnitStakedToken, 
        err := keeper.GetTargetRewardEmissionPerUnitStakedToken(
        params.FEmission,
        ecosystemMintSupplyRemaining,
        networkStaked,
        circulatingSupply,
        params.MaxSupply, 
    )
//----------skip------------
}

We can see that the monthly release of tokens is influenced by the circulating supply. By transferring uallo tokens to another chain via IBC, the total supply is reduced, thereby decreasing the circulating supply. Specifically, in GetTargetRewardEmissionPerUnitStakedToken, the reduction in circulating supply leads to a decrease in the overall release amount.”

func GetTargetRewardEmissionPerUnitStakedToken(
    fEmission math.LegacyDec,
    ecosystemMintableRemaining math.Int,
    networkStaked math.Int,
    circulatingSupply math.Int,
    maxSupply math.Int,
) (math.LegacyDec, error) {
    if networkStaked.IsZero() ||
        maxSupply.IsZero() {
        return math.LegacyDec{}, errors.Wrapf(
            types.ErrZeroDenominator,
            "denominator is zero: %s | %s",
            networkStaked.String(),
            maxSupply.String(),
        )
    }
    // T_{total,i} = ecosystemMintableRemaining
    // N_{staked,i} = networkStaked
    // N_{circ,i} = circulatingSupply
    // N_{total,i} = totalSupply
@>  ratioCirculating := circulatingSupply.ToLegacyDec().Quo(maxSupply.ToLegacyDec())
    ratioEcosystemToStaked := ecosystemMintableRemaining.ToLegacyDec().Quo(networkStaked.ToLegacyDec())
    ret := fEmission.
        Mul(ratioEcosystemToStaked).
@>      Mul(ratioCirculating)
    if ret.IsNegative() {
        return math.LegacyDec{}, errors.Wrapf(
            types.ErrNegativeTargetEmissionPerToken,
            "target emission per token is negative: %s | %s | %s",
            ratioCirculating.String(),
            ratioEcosystemToStaked.String(),
            ret.String(),
        )
    }

    return ret, nil
}

An attacker can reduce the monthly release amount by transferring uallo tokens to another chain via IBC, thereby reducing the monthly rewards for topics, workers, and reputers.

Impact

An attacker can reduce the monthly release amount by transferring uallo tokens to another chain via IBC, thereby reducing the monthly rewards for topics, workers, and reputers.

Code Snippet

https://github.com/sherlock-audit/2024-06-allora/blob/main/allora-chain/x/mint/module/abci.go#L12C1-L86C2

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

Tool used

Manual Review

Recommendation

Consider the impact of cross-chain transfers on reward distribution.

[relyt29]

note we no longer use the token supply from the bank module but rather use the max supply of 1e9 allo / 1e27 uallo

this change was made in https://github.com/allora-network/allora-chain/pull/479

[0502lian]

note we no longer use the token supply from the bank module but rather use the max supply of 1e9 allo / 1e27 uallo

this change was made in allora-network/allora-chain#479

Does it mean that the issue was valid before this change？ So this should be considered a valid issue. If I am wrong, please correct me. Thank you. @relyt29 @mystery0x @WangSecurity

[relyt29]

GetTotalCurrTokenSupply in the repo at that time called bankKeeper.GetSupply(ctx, params.BaseCoinUnit)

off the top of my head I don't know if an IBC transfer would change the bankKeeper's total supply count for that token. I would assume it wouldn't, since the supply of the token is the same no matter which chain it's on but if it indeed does change the bank keepers accounting of the total supply of the token then yes this bug would be valid bug, in my opinion