-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: support hello message with protocols #5371
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, makes sense!
|
||
let shared_capability = SharedCapability::new(&name, *version as u8, offset)?; | ||
|
||
match shared_capability { | ||
SharedCapability::UnknownCapability { .. } => { | ||
// Capabilities which are not shared are ignored | ||
debug!("unknown capability: name={:?}, version={}", name, version,); | ||
let proto_version = shared_capabilities.get(&name).expect("shared; qed"); | ||
|
||
// TODO(mattsse): track shared caps | ||
} | ||
SharedCapability::Eth { .. } => { | ||
// increment the offset if the capability is known | ||
offset += shared_capability.num_messages()?; | ||
let shared_capability = SharedCapability::new(&name, proto_version.version as u8, offset)?; | ||
|
||
shared_with_offsets.push(shared_capability); | ||
} | ||
} | ||
offset += proto_version.messages; | ||
shared_with_offsets.push(shared_capability); |
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.
ah, this sense compared to the match, SharedCapability
would have had to be extended for people adding new subprotocols. Whereas this is more flexible
This introduces the
Protocols
andHelloMessageWithProtocols
type, which is now required when handshaking p2p stream.this way we can support any protocol, because message numbers are not exchanged via the handshake we need to rely on input data when computing the offsets for sharedcapabilities.
The
Protocols
type provides this additional information.This will require a few followups to make it a bit nicer overall