cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.23k stars 3.6k forks source link

Expose Tendermint EventBus interface. #2770

Closed hleb-albau closed 5 years ago

hleb-albau commented 5 years ago

Summary

Expose Tendermint EventBus.

Problem Definition

Some apps can produce own set of events, instead of raw txes indexed by tags. Also, one tx can produce N events. So introducing this feature will open new possibilities to developers to create more fancy rpc endpoints. Dis: Only will be supported by started in process nodes.

PC: Seems, tendermint can be responsible this. Than TxResult will contain addition field events. May be we should discuss it in tendermint repo first.


For Admin Use

alexanderbez commented 5 years ago

@hleb-albau agreed in that this should be discussed in Tendermint first. Can we move the issue there please?

hleb-albau commented 5 years ago

@alexanderbez Sure.

hleb-albau commented 5 years ago

@alexanderbez @cwgoes Start to think, that cosmos should be responsible for this. Raw tendermint should know only about Raw txes. But events is some kind of application stuff (like internal transactions and etc). Also, it is not complicated to implement events in cosmos-sdk. Just store events from msg processing results indexed by tag in same ways as tendermint does and publishing them on default tendermint event bus.

I can create a prototype.

alexanderbez commented 5 years ago

Ahhh yes, fair point. I'm just saying we have to keep both components in mind.

alexanderbez commented 5 years ago

Also, this might have some overlap with #2448. Care to comment there?

hleb-albau commented 5 years ago

@alexanderbez @cwgoes I am ready to try to implement events indexed on cosmos-sdk side, just need your approval.

alexanderbez commented 5 years ago

@hleb-albau do you have a PR open?

hleb-albau commented 5 years ago

@melekes @ebuchman Currently tmtypes.EventBus does not expose PublishWithTags method. Is it possible(right) to open this method? If so, I will provide PR.

func (b *EventBus) PublishWithTags(eventData TMEventData, tags TagMap) error {
    // no explicit deadline for publishing events
    ctx := context.Background()
    b.pubsub.PublishWithTags(ctx, eventData, tags)
    return nil
}

Edit: It will provide subscription extension point for all apps using Tendermint as lib.

melekes commented 5 years ago

Note: early versions of Tendermint had this feature (ability to send & subscribe to application events via RPC). We removed it because nobody was using it. Maybe it's time to reconsider.

@hleb-albau can you share your use-case with us?

hleb-albau commented 5 years ago

@melekes For example, cosmos-sdk itself can use such functionality for https://github.com/cosmos/cosmos-sdk/issues/2805. Sdk can create own kv index for query routines + publish notifications on default tdm bus (or may be create own bus and subscribe endpoint subscribeEvents).

cwgoes commented 5 years ago

Is there a way to do this which fits your use case and is uniform (e.g. anyone interested in subscribing to events doesn't need to fork the codebase and add their own events to each module)? How about optionally publishing events on the event bus for all tags in a transaction, when we run a transaction?

melekes commented 5 years ago

@hleb-albau Why did you close?

hleb-albau commented 5 years ago

@cwgoes @melekes I agree with @ebuchman, that We should be wary for now of special privileges for Go apps. So, now I think it is bad idea, to use tendermint event bus for cosmos events for 2 reasons:

  1. Such events should be indexed too .
  2. It will work only for in-process tendermint.

Generally, either tdm should provide spec for events(list of list tags), either cosmos should implements it's own event bus similarly to tdm one. For me, cosmos own bus sees more clear way. It provide more flexibility and do not touch raw-tx tendermint protocol.