Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit "message" event for IBC relayer #563

Closed
Tracked by #431
yito88 opened this issue Mar 22, 2023 · 2 comments · Fixed by #564
Closed
Tracked by #431

Emit "message" event for IBC relayer #563

yito88 opened this issue Mar 22, 2023 · 2 comments · Fixed by #564
Milestone

Comments

@yito88
Copy link
Contributor

yito88 commented Mar 22, 2023

Bug Summary

Each handler should emit also "message" event for IBC relayer according to https://github.com/cosmos/ibc-go/blob/main/docs/ibc/events.md

Details

In Namada testing, after executing sending a token, Hermes wasn't notified because the message event wasn't emitted.

ctx_a.emit_ibc_event(IbcEvent::SendPacket(SendPacket::new(

Hermes' monitor subscribes to the IBC-specific events with message.module.
https://github.com/informalsystems/hermes/blob/a285c015c604819b213c1582d901e5ef140d3f47/crates/relayer/src/event/monitor.rs#L147-L165

Version

0.32.0

@yito88 yito88 mentioned this issue Mar 22, 2023
7 tasks
@plafer
Copy link
Contributor

plafer commented Mar 23, 2023

Interesting, when implementing #137, I had initially added this event, but then removed in after I realized that it was an "internal event" for SDK chains (used internally to route the event or something). I'm surprised that hermes needs it; let me look into that.

@plafer
Copy link
Contributor

plafer commented Mar 23, 2023

Agreed, let's emit these events too, as hermes does indeed rely on them for relaying.

Opened informalsystems/hermes#3190 so that hopefully we can remove them at some point is the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants