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

Update Base58 codec and remove Bitcoin support: #3581

Closed
wants to merge 5 commits into from

Conversation

a-noni-mousse
Copy link
Contributor

Reopening the closed PR #3575 sorry fo the problem @scottschurr and @HowardHinnant!

a-noni-mousse and others added 3 commits August 12, 2020 21:10
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.
Copy link
Collaborator

@scottschurr scottschurr left a 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!

@sublimator
Copy link
Contributor

sublimator commented Aug 15, 2020 via email

Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks!

@scottschurr
Copy link
Collaborator

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.

@scottschurr
Copy link
Collaborator

Oh, and since I didn't mention it before, nice job on the commit messages. Looks great. 👍

@carlhua carlhua added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Aug 24, 2020
Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

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

👍 Thanks!

@nbougalis nbougalis mentioned this pull request Sep 1, 2020
mDuo13 added a commit to mDuo13/xrpl-dev-portal that referenced this pull request Feb 23, 2021
@a-noni-mousse a-noni-mousse deleted the base58 branch March 12, 2021 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants