Provide descriptive results in msg response values (channel upgrades)

[colin-axner]

Several msg responses uses a typed result value. This was originally intended for handling no-op processing for packet processing, but it got reused for channel upgradability. I would recommend using a string type rather than a typed result value to increase flexibility in changing the descriptive behaviour in the future. This would allow for upgrade specific results to be included, rather than simply "SUCCESS" and "FAILURE".

Due to timing constraints, the result value will be left as is. If the response value becomes useful, we can deprecate the old field and add a new string field as a replacement. Opening this issue to document this idea and provide a historical record for creating result values in future features

See proposed change:

type MsgChannelUpgradeTryResponse struct {
	ChannelId       string             `protobuf:"bytes,1,opt,name=channel_id,json=channelId,proto3" json:"channel_id,omitempty"`
	Upgrade         Upgrade            `protobuf:"bytes,2,opt,name=upgrade,proto3" json:"upgrade"`
	UpgradeSequence uint64             `protobuf:"varint,3,opt,name=upgrade_sequence,json=upgradeSequence,proto3" json:"upgrade_sequence,omitempty"`
	Status          string `protobuf:"varint,4,opt,name=status,proto3,json:"result,omitempty"`
}

var (
    SUCCESS_UPGRADE_INIT = "SUCCESS_UPGRADE_INIT"
    SUCCESS_UPGRADE_FLUSHING = "SUCCESS_UPGRADE_FLUSHING"
    SUCCESS_UPGRADE_FLUSHCOMPLETE = "SUCCESS_UPGRADE_FLUSHCOMPLETE"
    SUCCESS_UPGRADE_COMPLETE = "SUCCESS_UPGRADE_COMPLETE"
    FAILURE_UPGRADE_CANCELLED = " FAILURE_UPGRADE_CANCELLED"
    FAILURE_COUNTERPARTY_OUT_OF_SYNC = "FAILURE_COUNTERPARTY_OUT_OF_SYNC"
)

I propose a string so that we aren't tied to a specific struct. That way we can just leave the success/failure as is but update to more descriptive responses if we so choose. Just a proposal though

This is similar to what I was ethereum do with one of their api's. They have a response status which is a string and then various string response values (valid, invalid, syncing)

Originally posted by @colin-axner in #3910 (comment)

[chandiniv1]

@colin-axner can I work on this issue?

[crodriguezvega]

Thank you for asking, @chandiniv1. However, this issue is labeled as icebox so there is no immediate need to implement this. We should also still validate if adding these response values would bring any extra benefit. Therefore it would be better to not pick up this issue for the time being.

If you are interested in picking up an issue from the repository, may I suggest this one? This is a bug that we had for a while now, and fixing it is relevant for the release of channel upgradability. If you would like to work on that, please comment on it and I will assign it to you. And if you have any questions about it, please let us know!

[colin-axner]

Closing this issue, with the direction of IBC Eureka, channel upgradability won't be necessary so further improvement to it don't make much sense