ADR 031: Typed Events

[jackzampolin]

Link to rendered ADR

Context

Currently in the SDK, events are defined in the handlers for each message, meaning each module doesn't have a cannonical set of types for each event. Above all else this makes these events difficult to consume as it requires a great deal of raw string matching and parsing. This proposal focuses on updating the events to use typed events defined in each module such that emiting and subscribing to events will be much easier. This workflow comes from the experience of the Akash Network team.

Our platform requires a number of programatic on chain interactions both on the provider (datacenter - to bid on new orders and listen for leases created) and user (application developer - to send the app manifest to the provider) side. In addition the Akash team is now maintaining the IBC relayer, another very event driven process. In working on these core pieces of infrastructure, and integrating lessons learned from Kubernetes developement, our team has developed a standard method for defining and consuming typed events in SDK modules. We have found that it is extremely useful in building this type of event driven application.

As the SDK gets used more extensively for apps like peggy, other peg zones, IBC, DeFi, etc... there will be an exploding demand for event driven applications to support new features desired by users. We propose upstreaming our findings into the SDK to enable all SDK applications to quickly and easily build event driven apps to aid their core application. Wallets, exchanges, explorers, and defi protocols all stand to benefit from this work.

If this proposal is accepted, users will be able to build event driven SDK apps in go by just writing EventHandlers for their specific event types and passing them to EventEmitters that are defined in the SDK.

The end of this proposal contains a detailed example of how to consume events after this refactor.

[alexanderbez]

Why Sev? Also, you don't define what StringEvent is. Does Sev not encapsulate Base (seems this way based off of the godoc)?

[jackzampolin]

Maybe this is a typo, I'm trying to reference the StringEvents type already defined in the events.go. StringEvents does contain the Module and Action but it is inconvenient to pull out. We could also add some functions here to StringEvents (func (sev StringEvents) Module() string and func (sev StringEvents) Action() string) and dispense with the BaseModuleEvent and SDKEvent types.

[aaronc]

I am strongly in favor of typed events.

If we're going to go ahead and refactor events, I think we could simplify this a bit by leveraging what we get from protobuf, especially the built in message name. For the case described here we'd have the following proto definition:

package cosmos.gov;

message EventSubmitProposal {
  string from_address = 1;
  uint64 proposal_id = 2;
  google.protobuf.Any proposal = 3;  
 }

This gives us a message name cosmos.gov.EventSubmitProposal. We could say that what we're now calling the module name is the package cosmos.gov and that the action is message name EventSubmitProposal.

Using this protobuf approach, clients would know the module and action just from the .proto files, instead of the current approach where that information lives out of band in golang source code. This could also simplify things for module developers by eliminating the need to implement this BaseModuleEvent stuff.

Obviously, there are some legacy considerations we'd need to address around the current module and action names. But going forward I'd like to see the .proto files as the source of truth for as much as possible. That eventually could look like events being communicated as Anys rather than key/value pairs. That might be a larger discussion but if we're refactoring events anyway and we just did this big proto migrations, we should align those two.

[jackzampolin]

The problem with that @aaronc is that the tendermint API returns them in map[string]string format. That change would be far deeper and require a lot more work and breaking changes on the tendermint side. This approach is ready to implement now and usable with the current software.

Also BaseModuleEvent could easily be eliminated by adding func Module() string and func Action() string to the ModuleEvent interface. And it is not a requirement that app developers use this system. They could avoid this and have their events not typed optionally.

In addition, this isn't a refactor. Its adding functionality using the existing code. I does not refactor anything.

[aaronc]

The problem with that @aaronc is that the tendermint API returns them in map[string]string format. That change would be far deeper and require a lot more work and breaking changes on the tendermint side. This approach is ready to implement now and usable with the current software.

We could serialize events using protobuf pretty easily using the current ABCI events by setting Event.Type to the proto message's type URL and each of the key value attributes would become an EventAttribute with the values JSON serialized. I think it'd be pretty straightforward.

Also BaseModuleEvent could easily be eliminated by adding func Module() string and func Action() string to the ModuleEvent interface. And it is not a requirement that app developers use this system. They could avoid this and have their events not typed optionally.

That would be preferable I think.

It would even be nice to see a method on EventManager like EmitModuleEvent that avoided the need to have an ABCIEvent method. That could be done with reflection or JSON serialization and then mapping the JSON k/v pairs to event attributes.

In addition, this isn't a refactor. Its adding functionality using the existing code. I does not refactor anything.

Gotcha. But without stronger typed events like I'm proposing with .proto defintions, this ADR only accomplishes its goals for other golang developers. I see how that works for Akash's use case. But just noting that this benefit doesn't translate to clients in other languages.

Also I would note that the protobuf approach I'm proposing would obviate the need to implement all those ParseEvent methods. All of that could be dealt with via reflection. So basically it wouldn't have the negative of Requires a substantial amount of additional code plus several other positives for clients in other languages.

[jackzampolin]

@aaronc that sounds ideal tbh. would love to see an adr for that because I don't 100% understand what you are proposing. Maybe @anilcse could help me with on for that approach?

[aaronc]

Here's the gist of what I'm thinking.

We add a function EmitTypedEvent to EventManager:

func (em *EventManager) EmitTypedEvent(event proto.Message) error {
	evtType := proto.MessageName(event)
	evtJson, err := codec.ProtoMarshalJSON(event)
	if err != nil {
		return err
	}

	var attrMap map[string]json.RawMessage
	err = json.Unmarshal(evtJson, &attrMap)
	if err != nil {
		return err
	}

	var attrs []abci.EventAttribute
	for k, v := range attrMap {
		attrs = append(attrs, abci.EventAttribute{
			Key:   []byte(k),
			Value: v,
		})
	}

	em.EmitEvent(Event{
		Type:       evtType,
		Attributes: attrs,
	})

	return nil
}

Then we have a corresponding function ParseTypedEvent which is basically the reverse of the above:

func ParseTypedEvent(event abci.Event) (proto.Message, error) {
  // look up proto.Message type by event.Type
  // populate attributes into map[string]json.RawMessage and marshal that to a json string
  // unmarshal the json string to the proto.Message
}

[colin-axner]

The problem with that @aaronc is that the tendermint API returns them in map[string]string format.

relevant context: issue on tendermint event subscription

[anilcse]

Thanks @aaronc, your approach looks great.

We could serialize events using protobuf pretty easily using the current ABCI events by setting Event.Type to the proto message's type URL and each of the key value attributes would become an EventAttribute with the values JSON serialized. I think it'd be pretty straightforward.

Is there a way to support nested objects with this? Even if it doesn't we can have a ParseEvent just for that particular event and avoid else where.

[aaronc]

Is there a way to support nested objects with this? Even if it doesn't we can have a ParseEvent just for that particular event and avoid else where.

I'm not sure I quite follow. What do you mean by nested objects?

[anilcse]

Is there a way to support nested objects with this? Even if it doesn't we can have a ParseEvent just for that particular event and avoid else where.

I'm not sure I quite follow. What do you mean by nested objects?

@aaronc I am not sure if there's a requirement for this but let's say we want to emit an event with another proto.Message wrapped inside. For example sake, lets consider that we want to emit TxResponse, it has Tx object wrapped as below.

message TxResponse {
  int64 height = 1;
  string txhash = 2 [(gogoproto.customname) = "TxHash"];
  string codespace = 3;
  uint32 code = 4;
  ...
  google.protobuf.Any tx = 11;
  ...
}

Or even any simple proto message with repeated keys.

[aaronc]

@anilcse can you think of a different example besides TxResponse? That seems like it really would never be an event.

But if it were, with the approach I'm destining nested Anys or repeated fields would get their normal proto JSON representation as the value part of the event attribute.

[anilcse]

@anilcse can you think of a different example besides TxResponse? That seems like it really would never be an event.

But if it were, with the approach I'm destining nested Anys or repeated fields would get their normal proto JSON representation as the value part of the event attribute.

Thanks @aaronc. Even if there's a case as such, we just get to add a custom ParseEvent which we don't need to worry about now.

[jackzampolin]

Ok, @anilcse and I are going to rework this proposal. Thanks for the feedback @aaronc