-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Update Base58 codec and remove Bitcoin support: #3581
Conversation
Use C++17 constant expressions to calculate the inverse alphabet map at compile time instead of at runtime. Remove support for encoding & decoding tokens using the Bitcoin alphabet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good. There is one assert I'd like you to remove, but other than that it looks great. Thanks for the contribution!
ohoh, what happened ?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thanks!
Oops. One more mechanical change needed. The clang-format continuous integration check is failing. We need to reformat a little bit of the code to make clang-format happy. You can do that yourself by running clang-format over the changed files. Or you can cherry-pick the changes that clang-format will make from this commit: scottschurr@1483ffe Sorry that I missed that earlier. |
Oh, and since I didn't mention it before, nice job on the commit messages. Looks great. 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks!
Reopening the closed PR #3575 sorry fo the problem @scottschurr and @HowardHinnant!