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.

Sign up for GitHub

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

Jump to bottom

Migrate all protocols to use quick-protobuf-codec #2500

Closed
3 tasks done
mxinden opened this issue Feb 9, 2022 · 6 comments · Fixed by #4787
Closed
3 tasks done

Migrate all protocols to use quick-protobuf-codec #2500

mxinden opened this issue Feb 9, 2022 · 6 comments · Fixed by #4787

Comments

@mxinden
Copy link
Member

mxinden commented Feb 9, 2022 •
edited by thomaseizinger
Loading

Instead of implementing Protobuf en-/decoding in each protocol and for each message exchange, one can instead implement a custom encoder abstracting the en-/decoding of Protobuf messages.

Tasks
Preview Give feedback
  1. noise: migrate to quick-protobuf-codec #4488
    difficulty:moderate help wanted
    0xcrust
  2. kad: migrate to quick-protobuf-codec crate #4487
    difficulty:moderate help wanted
    0xcrust
  3. autonat: migrate to quick-protobuf-codec #4489
    difficulty:moderate help wanted
@mxinden mxinden added difficulty:easy help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Feb 9, 2022
@mxinden mxinden mentioned this issue May 3, 2022
Merged
4 tasks
@nloadholtes
Copy link
Contributor

Hi! I have read through this and would like to take it on. I'm still pretty new to Rust, so apologies in advance if I ask some basic questions. 😃

I see that the Encoder and Decoder are now in prost-codec, and it looks like protocols/dcutr, protocols/identity, and protocols/relay are using it. I'll work on refactoring the other protocols packages to use prost-codec.

For this ticket, would you prefer 1 PR per package, or everything in 1 big PR? (My plan is to do 1 package in a draft PR and then have you/someone look at it to make sure I'm on the right path, and then go from there)

@thomaseizinger
Copy link
Contributor

Hi! I have read through this and would like to take it on. I'm still pretty new to Rust, so apologies in advance if I ask some basic questions. 😃

I see that the Encoder and Decoder are now in prost-codec, and it looks like protocols/dcutr, protocols/identity, and protocols/relay are using it. I'll work on refactoring the other protocols packages to use prost-codec.

For this ticket, would you prefer 1 PR per package, or everything in 1 big PR? (My plan is to do 1 package in a draft PR and then have you/someone look at it to make sure I'm on the right path, and then go from there)

1 PR per crate would be preferable!

@mxinden
Copy link
Member Author

mxinden commented Oct 20, 2022

Thanks @nloadholtes for volunteering.

@nloadholtes nloadholtes mentioned this issue Oct 30, 2022
Merged
4 tasks
@nloadholtes
Copy link
Contributor

Hi @mxinden and @thomaseizinger , I've made a WIP PR that I'd like some feedback on: #3070

Basically I'm looking at protocols/dcutr and trying to make sure that I can make protocols/gossipsub "look" like that. I ran the changes locally and the tests passed so I'm hopeful that is a good sign. 😄

Thank you in advance for any feedback, I'm still learning the ways of rust, but I'm really liking what I'm seeing so far.

@nloadholtes

This comment was marked as outdated.

Sign in to view
@nloadholtes nloadholtes mentioned this issue Dec 10, 2022
Merged
4 tasks
mergify bot pushed a commit that referenced this issue Dec 19, 2022
@nloadholtes
feat(floodsub): make use of prost-codec (#3224)
2dd188e 
This patch addresses #2500 for the `libp2p-floodsub` crate.

For this PR the existing code was upgraded to use `Framed` with the `prost_codec::Codec` as the standard codec for handling the RPC message format serialization/deserialization.
This was referenced Sep 12, 2023
Closed
Closed
Closed
@thomaseizinger thomaseizinger removed getting-started Issues that can be tackled if you don't know the internals of libp2p very well difficulty:easy help wanted labels Sep 12, 2023
@thomaseizinger thomaseizinger changed the title protocols/*: Use Protobuf Codec pattern Migrate all protocols to use quick-protobuf-codec Sep 12, 2023
@thomaseizinger
Copy link
Contributor

I've converted the remaining tasks into dedicated issues so they can be picked up more easily!

@thomaseizinger thomaseizinger mentioned this issue Nov 2, 2023
Merged
4 tasks
@mergify mergify bot closed this as completed in #4787 Nov 2, 2023
mergify bot pushed a commit that referenced this issue Nov 2, 2023
@thomaseizinger
refactor(autonat): use quick-protobuf-codec
d05d836 
Resolves: #4489.
Resolves: #2500.

Pull-Request: #4787.
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this issue Mar 8, 2024
@thomaseizinger
refactor(autonat): use quick-protobuf-codec
426606d 
Resolves: libp2p#4489.
Resolves: libp2p#2500.

Pull-Request: libp2p#4787.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(autonat): use quick-protobuf-codec libp2p/rust-libp2p
3 participants
@nloadholtes @thomaseizinger @mxinden