about event type message inside x/bank

[0Tech]

Looking around #13532, which I think is a nice refactoring, I found some events left behind.

cosmos-sdk/x/bank/keeper/send.go

Lines 140 to 143 in e3a0486

	sdk.NewEvent(
	sdk.EventTypeMessage,
	sdk.NewAttribute(types.AttributeKeySender, input.Address),
	),

cosmos-sdk/x/bank/keeper/send.go

Lines 211 to 214 in e3a0486

	sdk.NewEvent(
	sdk.EventTypeMessage,
	sdk.NewAttribute(types.AttributeKeySender, fromAddr.String()),
	),

I'm not sure whether it is a bug or not. Is it intentional, like for the backward-compatibility? Or it would be better to remove them.

[julienrbrt]

The PR you mentioned only removed events from the message server. These two snippets are from bank keeper. As those functions can be and are used by modules, we could be losing information.

[0Tech]

One can get the sender information in the both cases.

In the first case, InputOutputCoins() is only called from the x/bank's message server, so the sender would be input[0] of MsgMultiSend. In the second case, the corresponding transfer event contains it, so it would be obvious.

As those functions can be and are used by modules

I agree. However, other modules are already using its message server, e.g. x/authz and x/group. So we are losing the information by the PR, technically. So if there is no strict requirement on it, maybe we can consider removing it from the keeper too.

[julienrbrt]

This makes sense to me! Do you want to open a PR?

[0Tech]

Thank you! I will submit the corresponding PR.