-
Notifications
You must be signed in to change notification settings - Fork 725
Refactor event dispatcher #6765
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
Refactor event dispatcher #6765
Conversation
…3.0.0.1 Merge release/3.3.0.0.1 to master
…3.0.0.2 Merge Release/3.3.0.0.2 to master
The observer is now a pure data struct without any methods. All functionality lives in the dispatcher; the observer just contains the info where to send the data. This is a largely mechanical change: cut & paste; delete now duplicate `delete_payload` method; fix references.
No functional changes, only moving things around
There should be no functional change here.
Since the code in new files is just cut & paste from `event_dispatcher.rs`, I kept the same header from there, but bumped the year to 2025.
This is the final step to making `EventObserver` a "dumb" data object that simply reflects the configuration of the registered observer. Most of this change is straightforward. There is one thing that's worth a mention, and that's the `ProposalCallbackHandler`, a concept that was added in stacks-network#4228. Because block proposal validation happens on a dedicated thread, those validation events are handled a little specially. Since `send_payload` now needs a dispatcher instance as the source of the `db_path`, the list of observers alone is no longer enough. For now I've elected to simply add the DB path to the ProposalCallbackHandler and keep a non-method version of `send_payload` around that gets the DB path passed in. Eventually I may have to clone the dispatcher, but for now this approach had the smallest blast radius. There wasn't any test coverage for this special behavior (at least not on the event dispatcher side -- there are [some tests](https://github.com/stacks-network/stacks-core/blob/45381558f5e79554fc6e7930b0c4091e739e4992/stackslib/src/net/api/tests/postblock_proposal.rs#L234) on the API side of things), so I added that as well.
brice-stacks
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.
LGTM!
jacinta-stacks
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.
Nice clean up 👍
ef4a697
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (67.99%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6765 +/- ##
===========================================
- Coverage 68.95% 67.99% -0.96%
===========================================
Files 582 586 +4
Lines 361570 361650 +80
===========================================
- Hits 249312 245911 -3401
- Misses 112258 115739 +3481
... and 392 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I started doing this as part of #6762, but since this refactoring is a self-contained change, I figured I'd split it into its own PR.
Two primary changes:
event_dispatcher.rsinto multiple files to make it easier to work with, andEventObserverinto a "dumb" struct that contains nothing more than the actual subscriber information and has no functionality of its own, with theEventDispatcherbeing the one doing the real work.I would strongly suggest reviewing commit-by-commit.
The individual commits are structured so that the largely mechanical changes (like moving code from one file to another) can be more quickly verified by diffing the code that was removed from one file with the corresponding code that was added to another. Also take note of the individual commit messages.