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

Simplify and Streamify EventSubscription #413

Closed
jsdw opened this issue Jan 27, 2022 · 3 comments · Fixed by #442
Closed

Simplify and Streamify EventSubscription #413

jsdw opened this issue Jan 27, 2022 · 3 comments · Fixed by #442
Assignees

Comments

@jsdw
Copy link
Collaborator

jsdw commented Jan 27, 2022

I reckon we could improve/simplify the EventSubscription struct. Some thoughts:

  • Impl Stream (and so the StreamExt methods), so that the filtering logic can be (mostly?) moved out of this struct.
  • Remove filtering on events from a specific block (if needed, add a separate method to obtain events from a single block which potentially returns the same event stream as we'd get here; this method could be reused in the transaction.rs stuff too if it existed).
  • Remove filtering of events from a specific extrinsic (we get this now via the API in transaction.rs, and ideally it would be easy to manually filter the Stream for such events).
  • Remove filtering of events by pallet/event index; make it easy to do so via the Stream.
  • Update transfer_subscribe example to subscribe to events separately from making any transaction, and show off filtering and such. If needed, make transactions etc in a separate thread just to produce the desired events. (why? we shouldn't use this to get events for the current transaction as we have a proper API for that; but that's what the example seems to do).
  • Possibly add other examples.
@paulormart
Copy link
Contributor

Just read this issue and PR #423 and i'am fully supportive for the implementation of this feature. This is definitely a must have for this library :)

I'm currently using a custom events subscription so that block number and authority index could be easily accessible here

@jsdw
Copy link
Collaborator Author

jsdw commented Feb 2, 2022

Given your feedback and the PR, I think I'll bump this up my todo list and start working on it fairly soon!

@jsdw jsdw self-assigned this Feb 2, 2022
@jsdw
Copy link
Collaborator Author

jsdw commented Feb 10, 2022

@paulormart , @lamafab and anybody else interested, feel free to check out #442 and leave feedback :)

@jsdw jsdw closed this as completed in #442 Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants