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

support inline public keys #101

Merged
merged 1 commit into from
Sep 25, 2019
Merged

Conversation

tobowers
Copy link
Contributor

We are having trouble because we are using Secp256k1 from Go in our pubsub messages. Go does not set the "key" field of a message when publishing with an inline key. So javascript rejects any signed Go message (using Secp256k1).

There is a TODO here: https://github.com/libp2p/js-libp2p-pubsub/blob/master/src/message/sign.js#L76-L77

Well, hopefully this PR allows for that TODO to happen.

All this does is check if the peerID has identity encoding and if it does, attempt to recover the public key from the encoding.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…ncoding
@tobowers
Copy link
Contributor Author

I fixed the lint failure here.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks for the PR @tobowers

I will add this for peer-id@0.13.3 release!

However, I believe that you will need this to be added on peer-id@0.12.x. Pubsub routers are still using peer-id@0.12.x and we are currently refactoring all our codebase to async, which did not arrive to pubsub routers yet. If so, we need to get this to the peer-id@0.12.x version

@tobowers
Copy link
Contributor Author

What's the best way for me to help make the @0.12.x happen?

@vasco-santos
Copy link
Member

Thanks for helping on it!

Go to this branch: https://github.com/libp2p/js-peer-id/tree/0.12.x
Make these changes and within a new branch create a PR to add the new code to 0.12.x. This way, I can release this from the 0.12.x codebase

@vasco-santos
Copy link
Member

Also, please take into account that in this version you are using the older libp2p-crypto.

https://github.com/libp2p/js-libp2p-crypto/tree/v0.16.2

@tobowers
Copy link
Contributor Author

cool - that was pretty straight forward #102

@vasco-santos
Copy link
Member

Thanks for getting it!

@vasco-santos vasco-santos merged commit f39fb24 into libp2p:master Sep 25, 2019
@vasco-santos
Copy link
Member

Shipped 0.13.3 with this

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.

None yet

2 participants