Standardize sub-keeper usage within the ibc keeper

[chatton]

Summary

Currently, there is a mix of both reference and value types used for keepers in the app.go wiring. This has been problematic when the same keeper instance is expected to be passed to different keepers.

For example, the clientKeeper is passed to the connectionKeeper as well as the ibcKeeper. If a mutable field is added to the clientKeeper, this field will be discarded if the keeper is de-referenced and passed by value to one, and passed by reference to another.

This has caused multiple subtle bugs, recently we ran into an issue with the consensus host validation where the overridden validation implementation was correctly assigned to ibcKeeper's clientKeeper but was not overridden in the connectionKeeper's clientKeeper.

There was also a previous issue where the router was being discarded due to the same de-referencing issue.

Because the intent is that all sub-keepers of the ibc keeper refer to the same instances, I propose we simply make all keeper references pointers, instead of values.

I think this will effectively remove this class of error from the codebase.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[crodriguezvega]

Thanks, @chatton. So we should change this into something like this, right?

type Keeper struct {
  // implements gRPC QueryServer interface
  types.QueryServer

  cdc codec.BinaryCodec

  ClientKeeper     *clientkeeper.Keeper
  ConnectionKeeper *connectionkeeper.Keeper
  ChannelKeeper    *channelkeeper.Keeper
  PortKeeper       *portkeeper.Keeper
  Router           *porttypes.Router

  authority string
}

[chatton]

Yep @crodriguezvega ! Pretty much exactly that! For all submodule keepers as well.

[damiannolan]

Yeah! I think submodules should be fine, once the NewKeeper funcs return a pointer to core and are all plumbed through correctly it shouldn't need many other changes as submodules use expected interfaces.