Should ResponseDeliverTx.Log Include Events?

[alexanderbez]

Currently, BaseApp returns events, as determined and defined by the application, in ResponseDeliverTx in two locations -- Log and Events. Note, NEITHER Log or Events are merkle-ized and thus are NOT part of consensus (atm).

Now since we include events in both fields, there is obvious redundancy and duplication. However, the Log field contains two distinctions:

1. It contains the message index for which the events correspond to (ref)
2. It's string-ified slice of ABCIMessageLog objects (ref)

So should ABCIMessageLog contain the Events field? If we remove this events from ABCIMessageLog, then I do not believe clients will be able to understand what events pertain to which messages.

cc @tac0turtle @aaronc @ValarDragon

[alexanderbez]

If we were to remove Events from ABCIMessageLog, an alternative idea/proposal is to prefix event attributes with {message_url}_{msg_index}, e.g. sdk.bank.MsgSend.0.sender=cosmos1....

It should be trivial for clients to parse the attribute by splitting on . and extract:

• message url
• index
• event attribute
• event value

[robert-zaremba]

Why don't we extend abci.Event (by adding msg index) and remove Log?

[alexanderbez]

Why don't we extend abci.Event (by adding msg index) and remove Log?

I suppose we could. We could raise this with the CometBFT team and see if they'll accept it.

[webmaster128]

then I do not believe clients will be able to understand what events pertain to which messages.

Good point. I have never used this for now, but might be useful.

Why don't we extend abci.Event (by adding msg index) and remove Log?

Does CometBFT have a concept of messages?

[alexanderbez]

Good point. I have never used this for now, but might be useful.

This is exactly why we added events to Log :p

Does CometBFT have a concept of messages?

Nope. MsgIndex would solely be for the SDK's use. Which is kinda mehhh I agree

[ValarDragon]

Theres two problems I feel with the duplication:

• Excessive amounts of data for storing tx results
  • Historically this didn't compress well due to the encoding differences. This may now be fixed in next SDK release.
• When being printed out in logs, or parsed elsewhere, too much space is consumed. Adds sizable human-time overhead for working with it
  • CLI, grpc tx query, mintscan "raw logs", etc.

Perhaps this problem of associating an event with a particular message can be solved via just adding an attribute for message_index to every event, in the case where the message is a multi_message?

[alexanderbez]

Yes @ValarDragon I think that's another option -- you could add an EventAttribute, where Key: "message_index" and Value: "string(<msg_idx>)"

[tac0turtle]

i like the approach where we own the change, we talked before about owning the event system in the sdk instead of tendermint, we will end having to do some sort of message_index there as well

[tac0turtle]

mentioned this in slack, it seems no one from the relayers use this. If we add message index to the normal events in the form of an attribute or something then i think we can remove it and save some space