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

feat: rlpx multiplexing #5413

Merged
merged 9 commits into from
Nov 22, 2023
Merged

feat: rlpx multiplexing #5413

merged 9 commits into from
Nov 22, 2023

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Nov 13, 2023

adds support for multiplexing rlpx protocols

this is tailored for custom protocols in reth-network

this needs several followups but should be included to unblock

@mattsse mattsse marked this pull request as ready for review November 22, 2023 15:25
@mattsse mattsse changed the title wip:feat: rlpx multiplexing feat: rlpx multiplexing Nov 22, 2023
@mattsse mattsse added the C-enhancement New feature or request label Nov 22, 2023
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

lgtm, the multiplexer makes sense, looking forward to easy multi-cap support!

Comment on lines +291 to +317
// TODO optimize this by keeping a better index
let mut lowest_satellite = None;
// find the protocol with the lowest offset that is greater than the message
// offset
for (i, proto) in this.satellites.iter().enumerate() {
let proto_offset = proto.cap.relative_message_id_offset();
if proto_offset >= offset {
if let Some((_, lowest_offset)) = lowest_satellite {
if proto_offset < lowest_offset {
lowest_satellite = Some((i, proto_offset));
}
} else {
lowest_satellite = Some((i, proto_offset));
}
}
}

if let Some((idx, lowest_offset)) = lowest_satellite {
if lowest_offset < this.primary_capability.relative_message_id_offset()
{
// delegate to satellite
this.satellites[idx].send_raw(msg);
continue
}
}
// delegate to primary
let _ = this.to_primary.send(msg);
Copy link
Member

Choose a reason for hiding this comment

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

yeah, an index would be nice for this

Poll::Ready(Some(msg)) => {
this.out_buffer.push_back(msg);
}
Poll::Ready(None) => return Poll::Ready(None),
Copy link
Member

Choose a reason for hiding this comment

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

This was confusing to me at first, but if any of the streams are closed, we
need to return Poll::Ready(None)

@mattsse
Copy link
Collaborator Author

mattsse commented Nov 22, 2023

will merge and push smaller followups

@mattsse mattsse added this pull request to the merge queue Nov 22, 2023
Merged via the queue into main with commit 3598a23 Nov 22, 2023
27 checks passed
@mattsse mattsse deleted the matt/rlpx-multiplexing branch November 22, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants