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

feat(noise): deprecate all handshake patterns apart from XX #3768

Merged
merged 15 commits into from
Apr 28, 2023
Merged

feat(noise): deprecate all handshake patterns apart from XX #3768

merged 15 commits into from
Apr 28, 2023

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented Apr 11, 2023 •
edited
Loading

Description

In the libp2p specs, the only handshake pattern that is specified is the XX handshake. Support for other handshake patterns can be added through external modules. While we are at it, we rename the remaining types to following the laid out naming convention.

The tests for handshakes other than XX are removed. The handshakes still work as we don't touch them in this patch.

Related #2217.

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@thomaseizinger thomaseizinger mentioned this pull request Apr 11, 2023
Open
@mergify

This comment was marked as resolved.

Sign in to view
@thomaseizinger thomaseizinger mentioned this pull request Apr 11, 2023
Closed
@thomaseizinger
Merge branch 'master' into feat/deprecate-noise
363561e
@thomaseizinger
Deprecate enum directly
f8c3b40 
`pub type` aliases don't export the enum contructors unfortunately.
@mergify

This comment was marked as resolved.

Sign in to view
@thomaseizinger thomaseizinger mentioned this pull request Apr 24, 2023
Closed
4 tasks
mxinden
mxinden reviewed Apr 25, 2023
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks for the additional simplifications via Config!

examples/autonat/src/bin/autonat_client.rs Show resolved Hide resolved
interop-tests/src/bin/ping.rs Outdated Show resolved Hide resolved
libp2p/src/lib.rs Show resolved Hide resolved
protocols/perf/src/bin/perf-client.rs Show resolved Hide resolved
@thomaseizinger
Copy link
Contributor Author

Thanks for catching the missing version bumps. I am looking forward to #3715 when this no longer requires a manual check.

I did not bump the versions of the dependent crates. I figured, just bump dependencies is not worth it. Do you agree?

@mergify

This comment was marked as resolved.

Sign in to view
mxinden
mxinden previously approved these changes Apr 26, 2023
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Only outstanding comment would be https://github.com/libp2p/rust-libp2p/pull/3768/#discussion_r1177358842. Otherwise this looks good to me.

libp2p/Cargo.toml Show resolved Hide resolved
protocols/perf/Cargo.toml Show resolved Hide resolved
@mergify

This comment was marked as resolved.

Sign in to view
mxinden
mxinden approved these changes Apr 28, 2023
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

🙏

@mergify mergify bot merged commit 2f5270b into master Apr 28, 2023
@mergify mergify bot deleted the feat/deprecate-noise branch April 28, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mxinden mxinden mxinden approved these changes

Assignees
No one assigned
Labels
send-it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thomaseizinger @mxinden