x/feegrant Updates

[clevinson]

Any issues that come from the x/feegrant audit #8983 .

• Completeness audit @blushi x/feegrant completeness audit updates #9177

  • remove dead code in https://github.com/cosmos/cosmos-sdk/blob/master/x/feegrant/genesis.go#L9-L21 (we're declaring some GenesisState struct while there's already the proto-generated GenesisState type in x/feegrant/types).
  • improve test coverage in InitGenesis in case of error
  • QueryServer in x/feegrant/keeper/grpc_query.go is not covered by tests in x/feegrant/keeper/keeper_test.go but indirectly in x/feegrant/client/rest/grpc_query_test.go using grpc-gateway REST endpoints. This x/feegrant/client/rest folder must be some leftovers of legacy REST routes from the initial feegrant module implementation but it doesn't make sense to keep it since the module doesn't support legacy REST routes. Those tests should be moved to x/feegrant/keeper/keeper_test.go.
  • For the tx commands, in the cobra command Use field, we use granter and grantee as arguments names in one command and in the other one, we use granter_address and grantee_address. It would be good to have consistent naming across all feegrant commands.
  • we are using seconds for time-based expiration from the user, it should be timestamp

• Improve types package test coverage @technicallyty x/feegrant audit: clean up / add test coverage to types package #9193

• Simulation: see x/feegrant State machine audit #9029 (comment) @atheeshp x/feegrant simulation audit changes #9145

• State machine audit @aleem1314 x/feegrant state machine audit updates #9176

  • Msg server:

    • Fix UseGrantedFees logic and improve comments
    • Avoid panic inside keeper here. Lets use UnmarshalBinaryBare instead of MustUnmarshalBinaryBare .
    • Same here.
    • IterateAllGranteeFeeAllowances is defined, but not used anywhere.

  • Tests:

    • Keeper code (x/feegrant/keeper/keeper.go) covered by unit tests but not Msg server (x/feegrant/keeper/msg_server.go) Get rid of most of the keeper public methods (except the one used in ante handler) and test msg_server methods instead
    • (k Keeper) RevokeFeeAllowance: add test case if fee grant not found
    • (k Keeper) GrantFeeAllowance: add test case with grantee account not yet in account state

  • In the file periodic_fee_test.go in test TestPeriodicFeeValidAllow a field blockTime exists in the test case struct, but is never used. Add a test case with a different blockTime. Same for basic_fee_test.go in test TestBasicFeeValidAllow

These sections involve larger code changes/refactor so they should be worked on independently (not at the same time as others to avoid too many merge conflicts):

• Address API audit findings @technicallyty x/feegrant API Audit changes #9194
  • remove unused import cosmos/feegrant/v1beta1/query.proto:4:1: warning: Import gogoproto/gogo.proto is unused.
  • Few proto messages fields missing description FeeAllowanceGrant, QueryFeeAllowanceRequest, MsgGrantFeeAllowance, MsgRevokeFeeAllowance.
  • DeductGrantedFeeDecorator section is outdated, after refactoring ante handlers.
  • Same here.
  • In 03_events.md we can change section title: Handlers -> MsgServer.
  • Same as in x/authz, we should simplify msg and types naming. Also (a FeeAllowanceGrant) GetFeeGrant() method naming is a bit confusing. Why not just use GetAllowance()?
  • Add missing godoc to methods in Msg & Query server.
• Get rid of block height in ExportGenesis and remove height support in general and get rid of PrepareForExport method from FeeAllowanceI iface @aleem1314 x/feegrant remove height base expiration #9206
• Move x/feegrant/types to x/feegrant @blushi Move x/feegrant/types to x/feegrant #9273