Automate EventTypeMessage inclusion in every message execution

[ValarDragon]

Summary

Many modules define ad-hoc events in the message server, that (in a boilerplate fashion) encode the message type, source module, and sender. These are not standardized by code.

Problem Definition

~Every message has the following event defined in the message server
Staking Redelegate:

		sdk.NewEvent(
			sdk.EventTypeMessage,
			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
			sdk.NewAttribute(sdk.AttributeKeySender, msg.DelegatorAddress),
		),

Gov vote

	ctx.EventManager().EmitEvent(
		sdk.NewEvent(
			sdk.EventTypeMessage,
			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
			sdk.NewAttribute(sdk.AttributeKeySender, msg.Voter),
		),
	)

Osmosis swap

	ctx.EventManager().EmitEvents(sdk.Events{
		sdk.NewEvent(
			sdk.EventTypeMessage,
			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
			sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender),
		),
	})

The only exception I can see for where this isn't done is authz exec txs.

Proposal

Notice that in ~all cases checked types.AttributeValueCategory = types.ModuleName. The abstraction over msg.Sender, msg.Voter, and msg.DelegatorAddress is basically msg.GetSigners()[0].

We should adapt the event defined here: https://github.com/cosmos/cosmos-sdk/blob/main/x/auth/middleware/run_msgs.go#L91-L93 in the case that this is a non-legacy message. I believe that types.ModuleName is metadata that should be communicatable in the message service registration, and retrievable there.

Then we can delete this event from every msg server.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[aaronc]

Yes, these should probably get removed and just handled in the tx middleware

[julienrbrt]

I am trying to implement this issue, but I wonder what is the use of these events?

[alexanderbez]

We kind of already do this here in BaseApp#runMsgs:

		msgEvents := sdk.Events{
			sdk.NewEvent(sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyAction, eventMsgName)),
		}

We can just expand upon this.

I am trying to implement this issue, but I wonder what is the use of these events?

Clients might want to query for all txs that had messages of type X, e.g. delegations.

[aaronc]

Clients might want to query for all txs that had messages of type X, e.g. delegations.

Why can't they do that by just looking at the messages in the transaction? Isn't this sort of redundant?

[alexanderbez]

I don't follow. The point of these events is to query for blocks/txs that have specific messages/actions. No, it's not redundant.

[aaronc]

I don't follow. The point of these events is to query for blocks/txs that have specific messages/actions. No, it's not redundant.

I'm just saying that the type of message is already present in the transaction itself. The information is there it just needs to be indexed. Emitting the event seems like it's just a work around for poor indexing. I think it makes more sense to improve indexing.

[alexanderbez]

I'm not following. The indexer uses events themselves to index. Specifically, I'm referring to Tendermint's KV indexer and solutions like Numia which use events to index.

Inspecting the transaction itself requires parsing it which not all indexers and post-processing tools can or want to do.

[aaronc]

Yeah, I understand that those indexers don't know how to inspect the transaction and I see the motivation to deal with it this way. I'm just pointing out that this is an indexer deficiency rather than some new useful data that the state machine is producing. So if we need this for now then whatever it's fine, but IMHO we should work towards better indexers.

[alexanderbez]

But better indexers aren't really the issue per se. Imagine you have an indexer that is completely chain agnostic -- i.e. you don't have a codec so you can't decode and inspect the tx. You have to rely on on events and state streaming. In fact, this is how Numia works.

[aaronc]

But better indexers aren't really the issue per se. Imagine you have an indexer that is completely chain agnostic -- i.e. you don't have a codec so you can't decode and inspect the tx. You have to rely on on events and state streaming. In fact, this is how Numia works.

In my mind, a lot of the value in migrating to protobuf was to make it easier for clients to inspect things like tx contents. So maybe an indexer wants to take the position of not knowing whether a chain uses the Cosmos SDK protobuf or not... I don't think that ultimately means we should duplicate data just to accommodate that use case. Short term maybe it makes sense, but really, it's not that hard at all to parse the tx - there are endpoints which return JSON and protobuf isn't really even needed at that point. We have a similar situation with some secondary indexes - sometimes we add indexes just to support queries when that index will never be needed by the state machine itself. Short term yes this is the easiest thing to do, but IMHO long term we want better off-chain indexers and a leaner state machine. Maybe some of this comes down to philosophy of how we use events. As we've discussed there needs to be some bigger discussion around events as I think we all agree there are lots of improvements to be made.

[aaronc]

So if this is a standardized way to separate which events came from which Msg's in a multi-Msg tx which is what it seems like it actually does looking at the code, then it has value and isn't redundant

[alexanderbez]

Ok, let's get this in a PR soon. Thanks @julienrbrt 🙏