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

Hermes ignores IBC events emitted by BeginBlock/EndBlock #1793

Closed
5 tasks
michaelfig opened this issue Jan 19, 2022 · 8 comments · Fixed by #1801
Closed
5 tasks

Hermes ignores IBC events emitted by BeginBlock/EndBlock #1793

michaelfig opened this issue Jan 19, 2022 · 8 comments · Fixed by #1801
Assignees
Labels
I: logic Internal: related to the relaying logic O: new-feature Objective: cause to add a new feature or support
Milestone

Comments

@michaelfig
Copy link

Crate

relayer::event

Summary of Bug

get_all_events doesn't return IBC events that are raised in BeginBlock or EndBlock, only transactions. This prevents ChannelOpenInit from within EndBlock, which is what Agoric needs for dynamic (smart-contract driven) IBC.

Version

v0.10.0

Steps to Reproduce

  1. Modify your chain to do the following in EndBlock:
	ctx.EventManager().EmitEvents(sdk.Events{
		sdk.NewEvent(
			sdk.EventTypeMessage,
			sdk.NewAttribute(sdk.AttributeKeyModule, channeltypes.AttributeValueCategory),
		),
	})
  1. Run hermes listen mychain
  2. Note that the message.module = 'ibc_channel' event is not detected by Hermes

Acceptance Criteria

hermes listen mychain reports events emitted in EndBlock.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@michaelfig michaelfig changed the title Hermes ignores IBC events in BeginBlock/EndBlock Hermes ignores IBC events emitted by BeginBlock/EndBlock Jan 19, 2022
@adizere
Copy link
Member

adizere commented Jan 20, 2022

This seems either a regression of #1409 (or we need to extend that work to cover additional types of packets beyond SendPacket).

@michaelfig
Copy link
Author

https://github.com/agoric-labs/cosmos-sdk/tree/mfig-endblock-ibc-event has a hack to raise message.module = 'ibc_channel' in EndBlock.

To reproduce this problem, you can do:

git clone https://github.com/agoric-labs/cosmos-sdk -b mfig-endblock-ibc-event
cd cosmos-sdk
make build
./build/simd --help

Simd is then a Cosmos SDK chain that will produce those events on every block.

@ancazamfir
Copy link
Collaborator

Just to double check, does hermes config file include:

[mode.channels]
enabled = true

@michaelfig
Copy link
Author

Just to double check, does hermes config file include:

Yes, I was able to reproduce both with and without the config you mentioned.

@adizere adizere added this to the v0.11.1 milestone Jan 31, 2022
@adizere adizere added O: new-feature Objective: cause to add a new feature or support P-high I: logic Internal: related to the relaying logic labels Jan 31, 2022
@hu55a1n1
Copy link
Member

hu55a1n1 commented Feb 1, 2022

Hi @michaelfig, can you please confirm if this issue is fixed by #1801?

@romac romac modified the milestones: v0.11.1, v0.12.0 Feb 2, 2022
@michaelfig
Copy link
Author

can you please confirm if this issue is fixed by #1801?

Hi @hu55a1n1 ,

After further testing, I find that in the code as of #1801, Hermes doesn't detect newly-created EndBlock events, only ones that happened before Hermes started. So, I have to restart Hermes to get it to process the newly-sent events.

Is there any chance it's related to this issue, or should I file a separate one?

@michaelfig
Copy link
Author

Oh, probably my mistake! I had been testing #1840, and didn't look too closely at it. Turns out #1801 was not included in it.

I'm now testing them both together.

@michaelfig
Copy link
Author

I'm now testing them both together.

And I've verified that this current issue is fixed and should remain closed, but I see a potentially-related problem in #1844.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: logic Internal: related to the relaying logic O: new-feature Objective: cause to add a new feature or support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants