feature/ implement substrate event parser

[TxCorpi0x]

Description

The event parser is responsible for the translation of the IBC events received from the composable RPC endpoint to the IBC message types needed for processing client, channel, connection, and packet information of un-relayed packets.

Implementations

• ibcMessage definition
• Implement handleIBCMessagesFromEvents which is combination of functionalities of ibcMessagesFromTransaction and handleMessage of the cosmos event parser
• Define clientInfo
• Define clientUpdateInfo
• Define packetInfo
• Define channelInfo
• Define connectionInfo
• Match MarshalLogObject between cosmos implementation and substrate

Considerations

The IBC events end-point of the Composable chain returns JSON of defined types of ibc-rs, So there is no need for parseAttr implementation, then it is removed from the ibcMessageInfo.

[jackzampolin]

should be able to move this up into the top level require block afaik

[TxCorpi0x]

This is a fork of centrifuge repo, I made it separate from other packages because we need to replace it with the upstream version after merging to their repo. however, i can move it next to the rest.

[TxCorpi0x]

Done

[agouin]

This version of go-schnorrkel conflicts with the one used by tendermint, which is the cause of most of the errors in the CI. I'd suggest using this at the bottom of go.mod instead:

replace github.com/ChainSafe/go-schnorrkel/1 => github.com/ChainSafe/go-schnorrkel v1.0.0

Then you can use import "github.com/ChainSafe/go-schnorrkel/1" anywhere go-schnorrkel is used. Since this is in go-substrate-rpc-client, you would need to make the go mod change in that repo (or a fork that is used here) also.

[TxCorpi0x]

The go-supstrat-rpc-client is using the go-subkey as a direct dependency so the github.com/ChainSafe/go-schnorrkel v1.0.0 is comming as indirect dependency to this repo. If we decide to modify the version, we need to modify the go-subkey repo which is using the go-schnorrkel.

[TxCorpi0x]

However, I will fork the sub-key and change it in the Composable organization

[TxCorpi0x]

It has been fixed and using
github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d

[agouin]

It looks like a fallthrough was intended here. In go, cases will not fall through by default.

These can be simplified to, for example:
case "CreateClient", "UpgradeClient", "ClientMisbehaviour":

[TxCorpi0x]

Done

[agouin]

The Ack bytes come from the WriteAcknowledgement event, while the rest of the params come from the ReceivePacket event. We use packetAccumulator for combining the info from both events in the cosmos event parser.

[blasrodri]

Does it mean

info = &packetInfo{Height: height}

[agouin]

In cosmos, both the WriteAcknowledgement and ReceivePacket event will be included in a single IBC message. The packetInfo should be constructed with the Height property, as you have shown, if info is nil. If info is not nil, it should be casted to packetInfo. The attributes should be parsed into the resulting packetInfo. This will "accumulate" the properties from multiple events into a single packetInfo. The case of the WriteAcknowledgement/ReceivePacket events in the same message is the only need for the accumulator, the rest of the message types do not need an accumulator.

relayer/relayer/chains/cosmos/event_parser.go

Line 98 in 91182c9

if msg.info == nil {

[blasrodri]

In this context, where is msg?
Given that the function signature is

func (scp *SubstrateChainProcessor) handleIBCMessagesFromEvents(ibcEvents rpcclienttypes.IBCEventsQueryResult, height uint64, c processor.IBCMessagesCache) error {

[TxCorpi0x]

Done, Please check the implementation.

[blasrodri]

@agouin can you confirm, please?

[agouin]

The PathProcessor depends on the eventType being in the format of ibc-go. For example, instead of SendPacket, it should be chantypes.EventTypeSendPacket (which is send_packet). A helper might be nice to translate from the substrate ibc event type to the ibc-go event type.

[blasrodri]

@agouin can you point me to a place to lookup for the mappings? I've started here already 39d09b5

[agouin]

Here's a consolidated place to find them in the cosmos event parser

relayer/relayer/chains/cosmos/event_parser.go

Line 86 in 91182c9

switch event.Type {

[blasrodri]

is there a reason for which this function does not return an error?

[TxCorpi0x]

Errors are being logged in parseAttr, The IBC message is populated from the events, so there is no way to return the error to the source, So in these methods, the errors is being logged. @agouin can clarify it better.

[agouin]

In the cosmos event parser, it makes a best effort attempt to parse all of the attributes that are present, but does not quit if there is an issue parsing a single attribute. This is consistent here also.

[agouin]

Can we expect that all of the ibcEvents are relative to a single channel? If not, I think the key here will need to change from uint64 to a type that has the sequence and the channel information, e.g.:

type ibcPacketKey struct {
  sequence uint64
  channelKey provider.ChannelKey
}

[TxCorpi0x]

Added

[agouin]

chantypes.EventTypeAcknowledgePacket

[TxCorpi0x]

Done

[agouin]

chantypes.EventTypeWriteAck

[TxCorpi0x]

Done

[agouin]

chantypes.EventTypeRecvPacket

[TxCorpi0x]

Done

[agouin]

clienttypes.EventTypeSubmitMisbehaviour

[TxCorpi0x]

Done

[agouin]

conntypes.EventTypeConnectionOpenInit

[TxCorpi0x]

Done

[agouin]

conntypes.EventTypeConnectionOpenTry

[TxCorpi0x]

Done

[agouin]

conntypes.EventTypeConnectionOpenAck

[TxCorpi0x]

Done

[agouin]

conntypes.EventTypeConnectionOpenConfirm

[agouin]

rather than setting info here, since the accumulation might not be done yet for the packetInfo, we can remove this line, then iterate over the packetAccumulator map outside of the for loop (down on line 103) and append the final accumulated packetInfos to messages there.

[TxCorpi0x]

In the substrate chain query here, we are able to get events of a certain height, this means that it is possible to rewrite the info by other irrelevant event info in client, connection, and channel events as well. so I think we should store all of them in separate lists and loop through them after for loop finish. I think in the current approach we will lost most of events and create last event message in append to messages list.

[agouin]

I think we should store all of them in separate lists

The packetAccumulator map here is already used for this to store packetInfo for unique channel/sequence.

... and loop through them after for loop finish.

this can be iterating over the packetAccumulator on line 103 to append to messages, and removing line 81.

[TxCorpi0x]

I am talking about other types of events than packet -related events, what about them? There is no guarantee that exist one and only one of these types. For example events may be consist of multiple CreateClient, but this approach assumes that there is only one CreateClient which is set into ClientInfo.

[agouin]

the only reason for the packetAccumulator in the cosmos implementation is to accumulate the ack bytes from the write_acknowledgement event with recv_packet event. Other than that, all other IBC messages can be considered as 1-1 with the event. This brings up a good point though, this switch case only needs to handle the ReceivePacket and WriteAcknowledgement. There can be another switch case added for SendPacket, AcknowledgePacket, TimeoutPacket, TimeoutOnClosePacket which can do the single parseAttrs and set info like the other switch cases.

[TxCorpi0x]

Done

[agouin]

For example events may be consist of multiple CreateClient, but this approach assumes that there is only one CreateClient which is set into ClientInfo.

Yes, var info ibcMessageInfo should move to line 36 and the bracket on line 90 should move to line 101 so that the info nil check and messages append are on each event.

[agouin]

I see that info is now scoped inside the events loop in the latest, that looks good.

[agouin]

We don't want to set info for this case.

[TxCorpi0x]

Done

[agouin]

here we should iterate over packetAccumulator and append to messages for each with an ibcMessage like:

ibcMessage {
  eventType: chantypes.EventTypeRecvPacket,
  info: pi,
}

[TxCorpi0x]

Done

[agouin]

eventType = intoIBCEventType(eType) here

[TxCorpi0x]

Done

[agouin]

🥳 lets go!

[agouin]

Looks like we just need a go mod tidy inside the ibctest directory