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

Fix SS58 encoding/decoding and add verification from pubkeys #9

Merged
merged 4 commits into from
Jul 8, 2022

Conversation

elliot-u410
Copy link
Contributor

@elliot-u410 elliot-u410 commented Feb 17, 2022

This PR is designed to be reviewed commit-by-commit.

  • SS58 prefixes are actually uint16 with a very bizarre encoding. This only becomes apparent once you see a prefix > 63. This PR reimplements the SS58 handling based on the Rust implementation in sp-core.
  • Prior to this PR it was not possible to Verify using a public key only. That is, of course, the main reason to use Verify in the first place. This PR adds this functionality.

Note very little attempt was made at maintaining backwards compatibility.

Fixes #10 and #11

@mikiquantum
Copy link

@vedhavyas Do you have a bit of time to review this PR and see if we can get it in? I was going to open one myself to support sr25519 signature verification with only Public Keys and this one seems to address that.

@vedhavyas
Copy link
Owner

ofcourse @mikiquantum! I have some nit picks and get them in once pushed

Apologies @elliot-u410. Didn't see any notification regarding this PR earlier until @mikiquantum tagged me. I hope its okay if i make some minor changes to this PR.

@elliot-u410
Copy link
Contributor Author

Feel free!

@vedhavyas vedhavyas merged commit 99c5528 into vedhavyas:master Jul 8, 2022
@vedhavyas
Copy link
Owner

@elliot-u410 Thank you for the contribution 🙏🏼

@elliot-u410
Copy link
Contributor Author

@vedhavyas Noting that this change was not backwards compatible but the v1.0.4 released version number makes it seem that it was backwards compatible.

@vedhavyas
Copy link
Owner

updated the release now 👍🏼

@mikiquantum
Copy link

Awesome @vedhavyas & thanks @elliot-u410 !

@elliot-u410 elliot-u410 deleted the fixes branch July 10, 2022 19:38
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.

SS58 encoding/decoding is broken for prefix > 63
3 participants