cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.2k stars 3.58k forks source link

types: unnecessary code inside removeZeroCoins adding extra runtime and less readability #13958

Closed odeke-em closed 1 year ago

odeke-em commented 1 year ago

Summary of Bug

Noticed while auditing cosmos/gaia that if we examine the code inside https://github.com/cosmos/cosmos-sdk/blob/8cce7480917c90326c0c8a3bc2d26d1d3c29a33f/types/coin.go#L782-L803 we can see that the first for loop of code tries to check if at least one coin is zero then exit else if at the end return. However in the next loop, it performs the iteration and checks for coins that are non-zero which thereby renders the prior loop useless. image

Suggestion

Simply remove the first loop and the other unnecessary checks then only add coins that are non-zero

func removeZeroCoins(coins Coins) Coins {
    nonZeros := make([]Coin, 0, len(coins))

    for _, coin := range coins {
        if !coin.IsZero() {
            nonZeros = append(nonZeros, coin)
        }
    }

    return nonZeros
}

/cc @elias-orijtech

tac0turtle commented 1 year ago

want to submit a pr?

odeke-em commented 1 year ago

Sure @tac0turtle I've mailed PR https://github.com/cosmos/cosmos-sdk/pull/13967 though ideally on a code binge we open a bunch of issues firstly then we shall mail fixes :-)