Move Routing logic from Msg Server to Channel Keeper

[AdityaSripal]

This will allow other modules to call into the handshake and packet functions and still have the module-specific callbacks be executed.

Mostly necessary for ChanOpenInit since that will most often be called by third-party module, but worth doing for all cases.

[colin-axner]

related to #19. Would like a sketch of some overall design before a pr is opened since I don't think sub modules have the port keeper? Is this suggesting passing the port keeper to each submodule to do the callback from there or to pass the callback to each sub module?

[seantking]

Hey, so this was uncovered due to some debugging we ended up doing with Anka from the hermes team related to the channel capabilities apparently not being set correctly when calling channelkeeper.ChanOpenInit directly.

Defo in agreement about sketching a design that is focused on long-term maintainability. Also, not sure if ADR33 will effect this: https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-033-protobuf-inter-module-comm.md

In the short term for interchain accounts (as I'm not using latest ibc-go yet) I just called the callback function directly -> https://github.com/cosmos/interchain-accounts/pull/52/files#diff-e2884106d9411e0dd6608e8380e173738c5e6ca3d6004554a2c9e3a2922d9594R34

[AdityaSripal]

related to #19. Would like a sketch of some overall design before a pr is opened since I don't think sub modules have the port keeper? Is this suggesting passing the port keeper to each submodule to do the callback from there or to pass the callback to each sub module?

Neither. Just moving the callback call to the channel keeper, will allow third party modules to call the keeper function directly and have the entire handshake/packet process run. This is a pretty simple change that just extends usage for third party modules like ICA. we should have had it this way in the first place, but didn't think things through. I think it's simple enough to just make the quick fix

[colin-axner]

Ah I see because all the app callbacks are all 04 related. This still requires adding the Router to the channel keeper? That would be api breaking since it'd break the NewChannelKeeper api. This sounds fine to me, but the routing should still be separate from the handshake functions.

I'm not sure I'd want to squeeze this into 1.0. Interchain accounts will be released in >= 2.0 anyways and this is the first application to request such functionality? In general I'd prefer not to get in the habit of making last minute exceptions as that always delays releases, even if they are quick fixes

[seantking]

Totally agree let's not put this into 1.0. 👍

I think CCV will also need this functionality but neither module will be on the next hub upgrade.

[colin-axner]

I'm kinda wondering if the proposed solution is heading in the right direction? The SDK has agreed upon ADR 33, intermodule communication, which works by calling the msg server of another module and using a module key as the sender. I don't have strong opinions at the moment, but I would like to take a second to think about what we are doing

What we want to achieve: an API external modules can use to do things like invoke handshake callbacks

The current proposal would create a second entrypoint or effectively move the entrypoint down to 04-channel instead of the core message server. This sounds fine to me, but lets take a look to see possible benefits of going with the SDK method of only exposing the msg server

If we require all entry points to be via the msg server, we could internalize most of the core IBC handler API and only expose the message proto definition as our API. We would only have a single entrypoint. Given that I don't see our internals having much reason to change in the future, I don't see this as necessary. It is hard to say this with confidence though, I could see the implementation of partially ordered channels change this substantially

There's current work ongoing to refactor x/gov to do Msg execution instead of call predefined handlers. I think I remained unconvinced that we should go against the grain instead of taking advantage of the proto msg router and the SDK's work

ICA will already have the capability to call the message server since it has the msg router