-
-
Notifications
You must be signed in to change notification settings - Fork 366
Fix base58 implementation #8240
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
base: main
Are you sure you want to change the base?
Conversation
|
Can you take a quick look at this @rtfeldman since you wrote the original code? I can do a thorough review after. |
|
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
The base58 implementation seems to be wrong and very over complicated Signed-off-by: Prokop Randáček <prokop@rdck.dev>
8a9b383 to
85d9729
Compare
|
It may be a while before we get to this @ProkopRandacek because we're focused on the core functionality of the new compiler. We definitely do appreciate your contribution :) |
|
all good |
|
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
|
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
The base58 implementation seems to be wrong (doesn't agree with online tools) and very over complicated.
The new implementation agrees with two online tools I tried but I'm worried about endianness on other architectures.
I dropped the padding with '1's because the code is always working with 32 byte strings to the number of leading zeroes doesn't need to be stored at all.