-
Notifications
You must be signed in to change notification settings - Fork 4
Initial Impl For Event Channeling #14
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
Conversation
0cdf8a5 to
4efba82
Compare
0xNeshi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks good, but will approve once PR #15 is merged in after review the full PR.
| to_block: u64, | ||
| event_channels: &HashMap<String, mpsc::Sender<Log>>, | ||
| ) -> anyhow::Result<()> { | ||
| for event_filter in &self.tracked_events { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can each of event process be done in parallel?
If you're not sure, feel free to just create an issue to track parallel processing feature, we can handle that later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is what this is achieving. The basic flow is:
- Create a mapping between event --> channel
- Create background process per event.
- Blocks are processed as a single thread, the events are sent to their resepcitve channels / threads
- Each thread independently calls their respective callbacks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocks are processed as a single thread, the events are sent to their resepcitve channels / threads
Isn't this function fetching events separately for each event filter? Meaning, if you have 3 event filters, it would invoke 3 different fetch requests for the same block range?
0xNeshi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving in advance, the remaining open discussion can continue despite merging.
If a change becomes necessary, we should create an issue and address it in a separate PR.
Solves #9