-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Wrap txpool sender #438
Wrap txpool sender #438
Conversation
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
@@ -26,6 +27,65 @@ pub trait TxPoolDb: | |||
} | |||
} | |||
|
|||
#[derive(Clone, Deref, DerefMut)] | |||
pub struct Sender(mpsc::Sender<TxPoolMpsc>); |
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.
[nit] I think we could increase the simplicity even further by making this abstraction avoid channel terminology as much as possible. Focusing on emphasizing the capabilities and intention of this facade, vs the mechanics of how it interacts with other components at a lower level.
pub struct Sender(mpsc::Sender<TxPoolMpsc>); | |
pub struct TxPoolClient(mpsc::Sender<TxPoolMpsc>); |
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 like this naming, will make the change.
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.
Why is moving away from channel terminology a good thing when this is just a wrapper and channels are async primitives? As with Client
, we are adding new abstraction naming when in essence it is mpsc::Sender
inside.
I made the change here, but while coding the naming felt weird. 288ccef
thanks for cleaning up the |
Wrap txpool mpsc::Sender around Sender structure that gives us better ergonomic in calling functionalities from txpool,
related to: #429