-
Notifications
You must be signed in to change notification settings - Fork 46
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
feat: stream block updates from pool, use in builder #272
Conversation
4e36656
to
20374d5
Compare
4d08016
to
962879c
Compare
9877ee0
to
1cde0bc
Compare
20374d5
to
b9b3d9e
Compare
2500ca3
to
26b5a2f
Compare
0b86bf4
to
4a479ec
Compare
08ab644
to
69a064a
Compare
4a479ec
to
02011b1
Compare
69a064a
to
ac87557
Compare
02011b1
to
8755bbf
Compare
b24d191
to
0c71771
Compare
ac87557
to
4ae7662
Compare
0c71771
to
7210e9c
Compare
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.
Looks, solid.
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.
Great work! Some sophisticated wiring in here. A few comments inline.
f214510
to
8b58032
Compare
8b58032
to
10f572e
Compare
10f572e
to
73faf6e
Compare
73faf6e
to
4ec2ff7
Compare
@@ -41,20 +41,36 @@ pub async fn run(bundler_args: NodeCliArgs, common_args: CommonArgs) -> anyhow:: | |||
let builder_url = builder_args.url(false); | |||
|
|||
let (tx, rx) = mpsc::channel(1024); |
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.
if all the channels are the same size we should probably make this configurable and use a default channel size
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.
Yeah we have this problem all over the codebase. Definitely something we should clean up. I think we should just have a concept of an unbounded_channel
that we use in most places.
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.
yeh I think we could clean up the thread messaging a bit by using some type of event passing structure that can handle easy iterations of the codebase.
I am going to play around with it over the weekend and see what I can come up with.
pub struct Engine<E, A> {
/// The set of collectors that the engine will use to collect events.
collectors: Vec<Box<dyn Collector<E>>>,
/// The set of executors that the engine will use to execute actions.
executors: Vec<Box<dyn Executor<A>>>,
/// The capacity of the event channel.
event_channel_capacity: usize,
/// The capacity of the action channel.
action_channel_capacity: usize,
}
Something like this then we can have a collector trait that listens for new block events and exectutors that can process the events and bundle the user ops.
will most likely take a while to get right but will let you know how I go
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.
This looks good to me. I don't have any specific ideas on how to improve it at the moment- the stream abstraction sounds pretty reasonable. Thanks for putting so much thought into this 🔥 !
4ec2ff7
to
3e21b0e
Compare
Closes #146