fix(ics04): packet receive event type identifier #1179
Conversation
rnbguy
changed the title
rnbguy
changed the title
| @@ -32,13 +32,14 @@ const CHANNEL_OPEN_ACK_EVENT: &str = "channel_open_ack"; | |||
| const CHANNEL_OPEN_CONFIRM_EVENT: &str = "channel_open_confirm"; | |||
| const CHANNEL_CLOSE_INIT_EVENT: &str = "channel_close_init"; | |||
| const CHANNEL_CLOSE_CONFIRM_EVENT: &str = "channel_close_confirm"; | |||
| const CHANNEL_CLOSED_EVENT: &str = "channel_close"; | |||
There was a problem hiding this comment.
Moving the CHANNEL_CLOSED_EVENT constant under the channel event types section introduces some ambiguity regarding the semantics and the purpose of the constant. The string itself being just "channel_close" when all the other "channel_*" constants carry some additional meaning also contributes to the ambiguity.
Can we at least add a docstring comment on the CHANNEL_CLOSED_EVENT constant?
There was a problem hiding this comment.
you mean "channel_close_*", right?
It should definitely be
- "channel_close"
+ "channel_closed"There was a problem hiding this comment.
I would suggest just adding a reference link to ibc-go. Because we copy these values from there.
There was a problem hiding this comment.
ibc-go actually uses channel_close, not channel_closed, which is a bit confusing. But if parity is what we're aiming for, then I guess we go with channel_close.
There was a problem hiding this comment.
Yea, that's what I meant that we don't have a choice here 😅
|
Related to #374 |
* fix: ics04 packet event name * Add Changelog * Fix Changelog * nit on const decl group * nits on changelog entry * Add link to ibc-go channel events --------- Co-authored-by: Ranadeep Biswas <[email protected]> Co-authored-by: Sean Chen <[email protected]>
Closes: #1180
Description
Followup speaking with @rnbguy about a wrong event name I noticed while looking at optionally using ibc-rs for my Cosmos indexer, so it matches the go implementation
PR author checklist:
unclog.docs/).Reviewer checklist:
Files changedin the GitHub PR explorer.