x/globalfee audit

Conclusion

No issues found. Some stuff can be simplified, lacks some tests and godocs.

module.go

RegisterGRPCGatewayRoutes

• Has an ignored error, in the original implementation it panics.

fee_utils.go

• Move tests from antetest/fee_utils_test.go to the same directory; and suite is unnecessary (they are unit tests).
• getMinGasPrice and bypassMinFeeMsgs don't have a unit test
• GetTxPriority is not used

fee.go

• !ctx.IsCheckTx() || simulate { can be moved some lines up in AnteHandle to return earlier.
• In getGlobalFee DefaultZeroGlobalFee's error is not returned until the end of the function. Was this intentional?
• Add a godoc to getGlobalFee

params.go

DecCoins Validate() can be simplified (I have this in local if you prefer a PR):

func (coins DecCoins) Validate() error {
	if len(coins) == 0 {
		return nil
	}

	lowDenom := ""
	seenDenoms := make(map[string]bool)

	for _, coin := range coins {
		if err := coin.Validate(); err != nil {
			return err
		}

		if seenDenoms[coin.Denom] {
			return fmt.Errorf("duplicate denomination %s", coin.Denom)
		}

		if coin.Denom <= lowDenom {
			return fmt.Errorf("denomination %s is not sorted", coin.Denom)
		}

		// we compare each coin against the last denom
		lowDenom = coin.Denom
		seenDenoms[coin.Denom] = true
	}

	return nil
}

This is possible because the original sdk.DecCoins and sdk.DecCoin validation works differently. In this implementation the DecCoins validation works like sdk.DecCoin (accepting zero values).

params_test.go

Would like to add a couple of tests cases:

		// these 3 last cases should be impossible to reach if constructors are properly used
		"negative amount, fail": {
			sdk.DecCoins{sdk.DecCoin{Denom: "photon", Amount: sdk.OneDec().Neg()}},
			true,
		},
		"invalid denom, fail": {
			sdk.DecCoins{sdk.DecCoin{Denom: "photon!", Amount: sdk.OneDec()}},
			true,
		},
		"invalid second denom, fail": {
			sdk.DecCoins{
				sdk.DecCoin{Denom: "photon", Amount: sdk.OneDec()},
				sdk.DecCoin{Denom: "photon!", Amount: sdk.OneDec()},
			},
			true,
		},