Add upgrade callbacks IBCModule interface

[damiannolan]

Summary

Update the IBCModule interface to include application callback handlers for the 04-channel upgrade API.

As adapted from ICS004 Channel Upgrades:

func OnChanUpgradeInit(
        ctx sdk.Context,
        portID,
        channelID string,
        version string,
        order channeltypes.Order,
        connectionHops []string,
        previousVersion string,
) (string, error)

func OnChanUpgradeTry(
        ctx sdk.Context,
        portID,
        channelID string,
        order channeltypes.Order,
        connectionHops []string,
        counterpartyVersion string,
) (string, error)

func OnChanUpgradeAck(
        ctx sdk.Context,
        portID,
        channelID, 
        counterpartyChannelID,
        counterpartyVersion string
) error

func OnChanUpgradeConfirm(
        ctx sdk.Context,
        portID,
        channelID string,
) error

func OnChanUpgradeRestore(
        ctx sdk.Context,
        portID,
        channelID string,
) error

---

For Admin Use

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

[AdityaSripal]

Transfer and ICA will be trivial since upgrades are not supported at the moment.

We will want to provide a path for non-fee enabled channels to become fee enabled by using the upgrade callbacks in ics-29

[damiannolan]

Check the spec for any updates to callbacks. (e.g. previous channel version should be included)

[damiannolan]

Scaffolding with no-op callback handlers has been added in #3323

[AdityaSripal]

I think this should be a separate interface so it isn't absolutely necessary to implement for IBC functionality.

Similar to how AppModule in the SDK actually has multiple interfaces that each extend the previous with more functionality

[damiannolan]

I agree it would be nice to not enforce it.

So essentially remove this https://github.com/cosmos/ibc-go/blob/04-channel-upgrades/modules/core/05-port/types/module.go#L15. When we retrieve cbs using the Router we will need to do a conversion then in order to invoke the upgrade callbacks.

[chatton]

Conceptually I like the idea of having this split off as separate interface entirely, but this might lead to a bad UX for middleware devs.

If a middleware implements the IBCModule interface (but not the UpgradableModule interface ), we could end up in a situation where a component of the stack does not implement this interface and it would fail.

It adds more work for middle ware developers to make sure that they implement these 2 interfaces and would not get compile time errors in their app.go as the only requirement is the IBCModule module.

To me it is preferable to just need to worry about a single interface and let the compiler help with showing which methods need to be defined.

I discussed this a while back with @colin-axner if you have anything else to add!

[DimitrisJim]

on the flip-side this imposes a change on all middleware devs and could be another reason why someone wouldn't update to the most recent version of ibc-go. (Even if the change is merely stub impls of the relevant methods)

Do we expect all middleware to support upgradability? Must they? (What happens if they don't?). If so, I'd say its sensible to enforce it, if not, would be nice to let them choose.

Considering this, do we have a channel we could get feed-back on from third-party consumers of the interface? Would be nice to hear other opinions too.

[chatton]

@DimitrisJim good point, one thing to possibly consider is exporting an UpgradeUnsupported type that implements all the upgrade handlers returning errors.

This makes the code change a 1 line where you would just need to embed this type into the middleware

[chatton]

Adding the needs discussion label to this one because I'm not sure there is a consensus on how we want to proceed with this one!

[DimitrisJim]

Ah, I think I was thinking of this at some point and remember flipping and leaning towards your position. Would be nice to discuss tho.

[damiannolan]

The Upgrade interface is currently embedded within the IBCModule interface: https://github.com/cosmos/ibc-go/blob/04-channel-upgrades/modules/core/05-port/types/module.go#L15

This makes it an API breaking change for app developers. If we wish to release channel upgrades in a minor version release, e.g. v8.1.0 we should remove the embedded interface and instead use a type assertion in the core msg server handler.

Example:

+ app, ok := k.Router.GetRoute(module)
+ if !ok {
+	ctx.Logger().Error("channel upgrade init failed", "port-id", msg.PortId, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module))
+	return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module)
+ }

- cbs, ok := k.Router.GetRoute(module).(porttypes.UpgradableModule)
+ cbs, ok := app.(porttypes.UpgradableModule)
if !ok {
	ctx.Logger().Error("channel upgrade init failed", "port-id", msg.PortId, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module))
	return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module)
}