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

Handle begin & end block channel events #1801

Merged
merged 12 commits into from
Feb 3, 2022

Conversation

hu55a1n1
Copy link
Member

@hu55a1n1 hu55a1n1 commented Jan 24, 2022

Closes: #1793

Description

This PR is mostly based on the code that was removed in PR #1172.


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@hu55a1n1
Copy link
Member Author

Tested hermes listen with the following patch (https://github.com/agoric-labs/cosmos-sdk/tree/mfig-endblock-ibc-event) and it seems to work.

diff --git a/x/staking/module.go b/x/staking/module.go
index b5e778755..624b33a49 100644
--- a/x/staking/module.go
+++ b/x/staking/module.go
@@ -23,6 +23,7 @@ import (
        "github.com/cosmos/cosmos-sdk/x/staking/keeper"
        "github.com/cosmos/cosmos-sdk/x/staking/simulation"
        "github.com/cosmos/cosmos-sdk/x/staking/types"
+       channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
 )
 
 var (
@@ -171,12 +172,21 @@ func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) {
 // EndBlock returns the end blocker for the staking module. It returns no validator
 // updates.
 func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate {
-       // FIGME: Raise an event in end block.
-       AttributeValueCategory := "ibc_channel"
+       ctx.EventManager().EmitEvents(sdk.Events{
+               sdk.NewEvent(
+                       channeltypes.EventTypeChannelOpenInit,
+                       sdk.NewAttribute(channeltypes.AttributeKeyPortID, "transfer"),
+                       sdk.NewAttribute(channeltypes.AttributeKeyChannelID, "channel-0"),
+                       sdk.NewAttribute(channeltypes.AttributeCounterpartyPortID, "transfer"),
+                       sdk.NewAttribute(channeltypes.AttributeCounterpartyChannelID, "channel-0"),
+                       sdk.NewAttribute(channeltypes.AttributeKeyConnectionID, "connection-0"),
+               ),
+       })
+
        ctx.EventManager().EmitEvents(sdk.Events{
                sdk.NewEvent(
                        sdk.EventTypeMessage,
-                       sdk.NewAttribute(sdk.AttributeKeyModule /* channeltypes. */, AttributeValueCategory),
+                       sdk.NewAttribute(sdk.AttributeKeyModule, channeltypes.AttributeValueCategory),
                ),
        })
        return EndBlocker(ctx, am.keeper)

@hu55a1n1 hu55a1n1 marked this pull request as ready for review January 31, 2022 16:26
Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff! LGTM!

Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clear explanation, thank you Shoaib!

I think the PR is ready, but let's let @michaelfig test it out against Agoric devnet and give the final approval.

relayer/src/event/rpc.rs Show resolved Hide resolved
@michaelfig
Copy link

I think the PR is ready, but let's let @michaelfig test it out against Agoric devnet and give the final approval.

Thanks, everybody, for your work on this!

This is high up on my list of priorities. @dckc and I are doing IBC testing, so we'll be able to try a programmatic outbound channel init from an Agoric (dynamic IBC) smart contract in the next day or three.

Will keep you posted!

@dckc
Copy link
Contributor

dckc commented Feb 2, 2022

good news / bad news:

We were able to initiate a channel using JavaScript:

hermes_1  | 2022-02-02T15:55:16.352132Z DEBUG ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: starts processing Some(
hermes_1  |     OpenInitChannel(
hermes_1  |         OpenInit(
hermes_1  |             Attributes {
hermes_1  |                 height: Height {
hermes_1  |                     revision: 0,
hermes_1  |                     height: 11213,
hermes_1  |                 },
hermes_1  |                 port_id: PortId(
hermes_1  |                     "port-4",
hermes_1  |                 ),
hermes_1  |                 channel_id: Some(
hermes_1  |                     ChannelId(
hermes_1  |                         "channel-2",
hermes_1  |                     ),
hermes_1  |                 ),
hermes_1  |                 connection_id: ConnectionId(
hermes_1  |                     "connection-1",
hermes_1  |                 ),
hermes_1  |                 counterparty_port_id: PortId(
hermes_1  |                     "transfer",
hermes_1  |                 ),
hermes_1  |                 counterparty_channel_id: None,
hermes_1  |             },
hermes_1  |         ),
hermes_1  |     ),
hermes_1  | )

but then

2022-02-02T15:55:21.496369Z DEBUG ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: resolving channel version by retrieving destination chain app version chain_id=cosmoshub-testnet port_id=transfer ctx=ChanOpenTry


2022-02-02T15:55:57.279753Z  WARN ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: cannot resolve channel version for unknown port id 'port-4'

hermes_1  | 2022-02-02T15:55:55.768779Z TRACE ThreadId(18) fetching header height=0-204
hermes_1  | 2022-02-02T15:55:57.279380Z DEBUG ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: [agoric -> cosmoshub-testnet:07-tendermint-141] MsgUpdateAnyClient from trusted height 0-203 to target height 0-11221
hermes_1  | 2022-02-02T15:55:57.279575Z DEBUG ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: resolving channel version by retrieving destination chain app version chain_id=cosmoshub-testnet port_id=transfer ctx=ChanOpenTry
hermes_1  | 2022-02-02T15:55:57.279753Z  WARN ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: cannot resolve channel version for unknown port id 'port-4'
hermes_1  | 2022-02-02T15:55:57.279872Z ERROR ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: Failed ChanInit with error: failed during a query for the app version to chain id cosmoshub-testnet: failed to fetch application version: could not resolve channel version because the port is invalid: port-4
hermes_1  | 2022-02-02T15:55:57.279946Z ERROR ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: aborting after encountering fatal error: Worker failed after 7 retries
hermes_1  | 2022-02-02T15:55:57.279978Z  INFO ThreadId(35) channel{channel=channel::channel-2/port-4:agoric -> cosmoshub-testnet}: terminated

full log: https://gist.github.com/39d6fb688ae566dc83fefb85b7c3fa3c

@michaelfig
Copy link

In short, this particular PR looks good and does what it says it should.

Go ahead and land!

@romac romac merged commit 02f2594 into master Feb 3, 2022
@romac romac deleted the hu55a1n1/1793-begin-end-block-chan-events branch February 3, 2022 08:42
hu55a1n1 added a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Implement conversions for channel events

* Implement conversions for packets

* Resurrect code (from PR informalsystems#1172) to extract begin/end-block events from a tendermint NewBlock event

* Add channel events in the right order

* Remove redundant clones

* Minor refactoring

* Fix failing CI tests

* Extract block events without depending on message.action

* Cleanup

* Add .changelog entry

* Document event extraction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hermes ignores IBC events emitted by BeginBlock/EndBlock
6 participants