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: Support sending raw capability messages #13028

Merged
merged 4 commits into from
Dec 5, 2024

Conversation

stevencartavia
Copy link
Contributor

should close #13005

@stevencartavia
Copy link
Contributor Author

should I update self.bytecode_hash.is_none_or(|hash| hash == KECCAK_EMPTY)? I don't see the relationship with the changes

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.

great start

would be nice to get a test for this as well

see

async fn ethstream_over_p2p() {


/// Sends a raw capability message directly over the stream
pub fn start_send_raw(&mut self, msg: RawCapabilityMessage) -> Result<(), EthStreamError> {
self.inner.start_send_unpin(msg.payload)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what we need to do here instead is encode the entire msg together with the id.

see:

/// Encodes the protocol message into bytes. The message type is encoded as a single byte and
/// prepended to the message.
fn encode(&self, out: &mut dyn BufMut) {
self.message_type.encode(out);
self.message.encode(out);

@mattsse mattsse marked this pull request as ready for review November 30, 2024 06:42
@mattsse mattsse added the A-networking Related to networking in general label Nov 30, 2024
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.

very nice!

@mattsse mattsse enabled auto-merge December 5, 2024 15:08
@mattsse mattsse added this pull request to the merge queue Dec 5, 2024
Merged via the queue into paradigmxyz:main with commit 6f0cfef Dec 5, 2024
41 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support sending raw capability messages
2 participants