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

Use SHA256 again for from_public_key #1444

Merged
merged 2 commits into from
Feb 12, 2020
Merged

Conversation

tomaka
Copy link
Member

@tomaka tomaka commented Feb 11, 2020

Reverts parts of #1413 (which hasn't been published yet)

Summary of the story:

Libp2p 0.12 only hashes public keys using SHA2256, which is not conforming to specs. Keys less than 42 bytes have to be encoded using the identity hash.

Libp2p 0.13 and above no longer sees an identity peer ID as an error by itself.
However we forgot a detail: if we try to connect to a node that has an identity peer ID, it would always fail because the public key sent by secio/noise is always locally encoded using SHA256 before comparison.

#1413 fixes that and makes it possible to connect to nodes with an "identity" hash, and also encodes small public key using the "identity" hash by default.

Unfortunately, upgrading from a network before #1413 to a network after #1413 is kind of tricky. Nodes with #1413 will send peer IDs with the identity hash to nodes without #1413, which will then fail to connect. It could cause a netsplit.

What this PR does is revert the identity hashing part of #1413, but keep the part where we compare keys equal.
The intent is that we release version 0.16 with this PR, then later we fully apply #1413 and release 0.17.
Then 0.16 will be fully compatible with both 0.15 and 0.17.

@tomaka
Copy link
Member Author

tomaka commented Feb 11, 2020

Hopefully we're nearing the end of this saga.

Copy link
Contributor

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable way to solve the issue

@tomaka tomaka merged commit e855cd5 into libp2p:master Feb 12, 2020
@tomaka tomaka deleted the inline-peer-id-oops branch February 12, 2020 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants