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(net): ress subprotocol #14687

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(net): ress subprotocol #14687

wants to merge 2 commits into from

Conversation

rkrasiuk
Copy link
Member

Description

This PR adds reth-network-ress crate which defines ress RLPx subprotocol for stateless nodes.

@rkrasiuk rkrasiuk added C-enhancement New feature or request A-networking Related to networking in general labels Feb 24, 2025
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

some suggestions/questions

still need to take another closer look

Comment on lines +22 to +35
/// Get block headers.
GetHeaders {
/// The request for block headers.
request: GetHeaders,
/// The sender for the response.
tx: oneshot::Sender<Vec<Header>>,
},
/// Get block bodies.
GetBlockBodies {
/// The request for block bodies.
request: Vec<BlockHash>,
/// The sender for the response.
tx: oneshot::Sender<Vec<BlockBody>>,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need those, because those are already part of the eth proto and we could reuse those instead?

}

loop {
if let Poll::Ready(Some(cmd)) = this.commands.poll_next_unpin(cx) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can poll a finished stream, if the exist logic is independent from that you can fuse this receiver

}
};

match msg.message {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we move this to a function?

_direction: Direction,
_peer_id: PeerId,
) -> OnNotSupported {
OnNotSupported::Disconnect
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should probably check the nodetype, because we dont want to disconnect if the node is stateful but the other peer doesnt support it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general C-enhancement New feature or request
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants