Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event subscription ADR and refactor event_listener module #313

Closed
liamsi opened this issue Jun 9, 2020 · 3 comments · Fixed by #516
Closed

Event subscription ADR and refactor event_listener module #313

liamsi opened this issue Jun 9, 2020 · 3 comments · Fixed by #516
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request go Compatibility with Go code rpc
Milestone

Comments

@liamsi
Copy link
Member

liamsi commented Jun 9, 2020

// TODO(ismail): this works if subscriptions are fired sequentially and no event or
// ping message gets in the way:

// TODO(ismail): this should live somewhere else; these events are also
// published by the event bus independent from RPC.
// We leave it here for now because unsupported types are still
// decodeable via fallthrough variants (GenericJSONEvent).

// TODO(ismail): these should be the same as abci::responses::BeginBlock
// and abci::responses::EndBlock

Subscribe should return a stream of events tied to the particular query. Pretty much what the go-code returns on subscribe but "rust style". E.g. see: https://docs.tendermint.com/master/rpc/#/Websocket/subscribe

Furthermore, the current code only supports two events:

EventSubscription::TransactionSubscription => "tm.event='Tx'",
EventSubscription::BlockSubscription => "tm.event='NewBlock'",

While tendermint actually has more event types:
https://github.com/tendermint/tendermint/blob/6961c7e5d1f2a187bc5f77ac0056ce5ec5bf7ec6/types/events.go#L21-L24

also

// TODO(ismail): document fields or re-use the abci types

ref: #311

@liamsi liamsi added rpc documentation Improvements or additions to documentation enhancement New feature or request labels Jun 9, 2020
@liamsi liamsi changed the title Write event subscription ADR and refactor module Event subscription ADR and refactor event_listener module Jun 9, 2020
@liamsi liamsi added the go Compatibility with Go code label Jun 9, 2020
@thanethomson thanethomson self-assigned this Jul 10, 2020
@thanethomson
Copy link
Contributor

What level of granularity of event routing to subscribers is needed here? Do we need fine-grained routing to subscribers the way the Go code handles it, or will matching purely based on event type (tm.event) suffice?

@liamsi
Copy link
Member Author

liamsi commented Jul 23, 2020

Good question. I think ideally we would replicate the (fine-grained) go behaviour here. It just makes the most sense to me: a dev coming from tendermint go will immediately be familiar with the rust version and it's likely that you want to build (complex) queries and only receive the events for those for instance.

On the other hand the current simpler approach was successfully used in GoZ and as far as I understand the main use case will be specific events in ibc. So if mimicking the go behaviour overcomplicates this we could go with a simpler variant as long as all use cases in ibc can be handled with it.

@xla
Copy link
Contributor

xla commented Aug 4, 2020

As mentioned in this https://github.com/informalsystems/tendermint-rs/pull/476/files#r465102351 - I think we should take the opportunity and design a query system that fits the language and runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request go Compatibility with Go code rpc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants