Move coins can be burned twice
Description
The BurnCoins method has special handling for Move coins not generated by 0x1 as they do not have the burn capability for them. Instead, they are sent to the community pool.
// BurnCoins burn coins or send to community pool.
func (k MoveBankKeeper) BurnCoins(
ctx context.Context,
accAddr sdk.AccAddress,
coins sdk.Coins,
) error {
for _, coin := range coins {
// if a coin is not generated from 0x1, then send the coin to community pool
// because we don't have burn capability.
if types.IsMoveCoin(coin) {
if err := k.communityPoolKeeper.FundCommunityPool(ctx, coins, accAddr); err != nil {
return err
}
continue
}
// send tokens to 0x1
err := k.SendCoin(ctx, accAddr, types.StdAddr, coin.Denom, coin.Amount)
if err != nil {
return err
}
// execute burn
metadata, err := types.MetadataAddressFromDenom(coin.Denom)
if err != nil {
return err
}The issue is that the coins parameter is passed to FundCommunityPool, which contains every coin instead of only the coin that matched the condition.
Impact
If one of the coins returns true for types.IsMoveCoin(coin), then all the coins are sent to FundCommunityPool, not just the matching one. Then, the next iteration will send the coins again to 0x1 to be burned. If two Move coins are provided, then double of each will be sent to the fund. If one is Move and the other is not, then the other coin will be sent to the fund as well as burned.
Recommendations
Only the matching coin should be sent to the community pool.