imp: add passthrough handlers for callbacks mw module #5376
Conversation
…he-channel-upgradability-application-callbacks-for-mod/callbacks
charleenfei
requested review from
colin-axner,
AdityaSripal,
damiannolan and
srdtrk
as code owners
…he-channel-upgradability-application-callbacks-for-mod/callbacks
…he-channel-upgradability-application-callbacks-for-mod/callbacks
…he-channel-upgradability-application-callbacks-for-mod/callbacks
This reverts commit 0665746.
| func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { | ||
| cbs, ok := im.app.(porttypes.UpgradableModule) | ||
| if !ok { | ||
| return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack") |
There was a problem hiding this comment.
nit: these err messages seem slightly off structurally wise?
There was a problem hiding this comment.
how so? what would you expect them to look like?
I think this is how I structured them in the initial PR for core and other mods like fee etc, I'm open to suggestions if you want to open a PR and address all of them together after this then go ahead!
| @@ -20,6 +20,7 @@ import ( | |||
| var ( | |||
| _ porttypes.Middleware = (*IBCMiddleware)(nil) | |||
| _ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil) | |||
| _ porttypes.UpgradableModule = (*IBCMiddleware)(nil) | |||
There was a problem hiding this comment.
I assume its intensional that we never added an app upgrade callback for chanUpgradeConfirm, its been so long that I don't really remember, but I think it was
There was a problem hiding this comment.
yes it is intentional. I think there's nothing to do in that step since it is just acknowledging that the counterparty set your timeout timer
| func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { | ||
| cbs, ok := im.app.(porttypes.UpgradableModule) | ||
| if !ok { | ||
| return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack") |
There was a problem hiding this comment.
how so? what would you expect them to look like?
I think this is how I structured them in the initial PR for core and other mods like fee etc, I'm open to suggestions if you want to open a PR and address all of them together after this then go ahead!
…he-channel-upgradability-application-callbacks-for-mod/callbacks
charleenfei
deleted the
charly/issue#4575-implement-the-channel-upgradability-application-callbacks-for-mod/callbacks
branch
Description
closes: #4575
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.