Remove temporary peer ID compatibility mode. #1608
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the temporary compatibility mode between peer IDs using identity hashing and sha256, thus being the last step in the context of libp2p#555.
Member
|
I think we can go ahead with this now. |
tomaka
reviewed
Aug 25, 2020
core/src/peer_id.rs
Outdated
| Err(data) | ||
| pub fn from_multihash(data: Multihash) -> Result<PeerId, Multihash> { | ||
| match data.algorithm() { | ||
| Code::Sha2_256 | Code::Identity => Ok(PeerId { multihash: data }), |
Member
There was a problem hiding this comment.
This isn't the case right now, but maybe we should check that data.len() <= MAX_INLINE_KEY_LENGTH when the algorithm is identity?
twittner
reviewed
Aug 25, 2020
Co-authored-by: Toralf Wittner <[email protected]>
twittner
approved these changes
Aug 25, 2020
tomaka
approved these changes
Aug 25, 2020
tomaka
reviewed
Aug 25, 2020
Comment on lines
-80
to
-87
| // Note: before 0.12, this was incorrectly implemented and `SHA2256` was always used. | ||
| // Starting from version 0.13, rust-libp2p accepts both hashed and non-hashed keys as | ||
| // input (see `from_bytes`). Starting from version 0.16 rust-libp2p will compare | ||
| // `PeerId`s of different hashes equal, which makes it possible to connect through | ||
| // secio or noise to nodes with an identity hash. Starting from version 0.17, rust-libp2p | ||
| // will switch to not hashing the key (i.e. the correct behaviour). | ||
| // In other words, rust-libp2p 0.16 is compatible with all versions of rust-libp2p. | ||
| // Rust-libp2p 0.12 and below is **NOT** compatible with rust-libp2p 0.17 and above. |
Member
There was a problem hiding this comment.
Maybe we could leave that comment, but I don't have a strong opinion.
Contributor
Author
There was a problem hiding this comment.
I incorporated it into the changelog (see 7504764).
added 2 commits
August 26, 2020 11:55
This was referenced Sep 11, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the temporary compatibility mode between peer IDs using identity hashing and sha256, thus being
the last step in the context of #555. This has low priority, I just wanted to get this out as a PR as a reminder to do this.