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

fatxpool: event streams moved to view domain #7545

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented Feb 11, 2025

Overview

This pull request refactors the transaction pool graph module by renaming components for better clarity. The EventHandler trait was introduced to enhance flexibility in handling transaction lifecycle events. Changes include renaming graph::Listener to graph::EventDispatcher and moving certain functionalities from graph to view module in order to decouple graph from view-related specifics.

This PR does not introduce changes in the logic.

Notes for Reviewers

All the changes looks dense at first, but in fact following was done:

  • The graph::Listener was renamed to graph::EventDispatcher, to better reflect its role in dispatching transaction-related events from ValidatedPool. The EventDispatcher now utilizes the L: EventHandler generic type to handle transaction status events.
  • The new EventHandler trait was introduced to handle transaction lifecycle events, improving implementation flexibility and providing clearer role descriptions within the system. Introduction of this trait allowed the removal of View related entities (e.g. streams) from the ValidatedPool's event dispatcher (previously listener).
  • The dropped monitoring and aggregated events stream functionalities and related types were moved from graph::listener to the view module. The ViewPoolObserver, which implements EventHandler, now provides the implementation of streams feeding.
  • Fields, arguments, and variables previously named listener were renamed to event_dispatcher to align with their purpose and type naming.
  • Various structs such as Pool and ValidatedPool were updated to include a generic L: EventHandler across the codebase.

@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13272316509
Failed job name: cargo-clippy

@michalkucharczyk
Copy link
Contributor Author

/cmd prdoc --bump minor --audience node_dev

@michalkucharczyk michalkucharczyk added R0-silent Changes should not be mentioned in any release notes T0-node This PR/Issue is related to the topic “node”. labels Feb 11, 2025
@michalkucharczyk michalkucharczyk marked this pull request as ready for review February 11, 2025 21:20
Copy link
Contributor

@iulianbarbu iulianbarbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

prdoc/pr_7545.prdoc Outdated Show resolved Hide resolved
Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just the one comment needs to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants