Standardize OMG.Bus event signature

[unnawut]

Opening this issue to track a suggestion stemmed from https://github.com/omisego/elixir-omg/pull/1123#discussion_r352582737

Since this is a broadcast to a more generic event bus (i.e. not restricted to contract events), maybe naming the topic directly after the ethereum event Name might lead to confusion/conflicts. Instead of DepositCreated (which might clash with other events related to a deposit being created), something like root_chain:DepositCreated? (not sure what conventions/style this would have to follow though)

Also for a cleaner and more testable approach, putting code that retrieves the OMG.Bus topic from event_signature fits under Preprocessor responsibilities.

[pgebal]

Idea is that all events are produced by a module that processes the event. Just as we do now with db updates. That requires a change in process events callback API. Now return type would be something like: {:ok, list(Core.db_update()), list(Event.t())} where Event contains a topic and data to be broadcasted.

edit: I decided not to follow that path because of a major refactor needed in OMG.State.Core. I focused just on standardizing topic names.

[boolafish]

Instead of DepositCreated (which might clash with other events related to a deposit being created), something like root_chain:DepositCreated

Another concerns I have is on the exit related events. We have only 1 (payment) exit game so it is fine with current schema. As we are extending, the payment v2 event would crash with payment v1 event.

Shall we start to put the txType as part of the event identifier? so sth like: rootchain:payment:ExitStarted and rootchain: paymentV2:ExitStarted.

Vault could potentially have same issue though the risk there is a lot smaller.

[pgebal]

@boolafish good remark Andy. I think we could aim for enriching topic name with a contract address, instead of payment part. As we do need this now, let's leave it until we'll have to listen to events from multiple payment games. I think it would be good to do it in one piece, as we'll know exactly what we need then.

[boolafish]

sure just make sure this issue or there is some issue left tracking this

[pnowosie]

@boolafish

As we are extending, the payment v2 event would crash with payment v1 event.

IMHO it's totally fine as the metric will store total number of deposits from both V1 & V2. I'm unaware of DD features here, but the metrics I published to other collector allows to specify a tag - so you could count total or filter it further by tags: "V1"... What DD offers here ❓

[boolafish]

@pnowosie I think I did not get what you mean 😅

Are you saying that DD is the only consumer of the OMG.bus for the exit events, and it is fine to not distinguish the events? And DD might have some special magic feature (tag?) to differentiate them later?

Personally I am lost on how...that works? If we put simply eg. ExitStarted for both payment v1 and v2, and some process put that to DD, how does the DD possibly know which tag to use?

Another thing is that long term wise there will be more transaction types to come in with the exchange feature. Personally I want to get prepared for that or understand what are the necessary changes to add the feature.