Minor bug: hardcoded base58 alphabet character #6321

issue vinniefalco opened this issue on June 22, 2015

vinniefalco commented at 8:20 PM on June 22, 2015: contributor
Sorry for not creating a pull request but this line: https://github.com/bitcoin/bitcoin/blob/master/src/base58.cpp#L96 should read:
```
str.assign(zeroes, pszBase58[0]);
```
luke-jr commented at 8:29 PM on June 22, 2015: member

Looks fine to me...
penguin1333 commented at 11:33 PM on June 22, 2015: none

Looks fine, but it is more efficient.

Funny though because I spotted this and sent a pull request for fixing it about 30 minutes before finding this :P
paveljanik commented at 5:45 AM on June 23, 2015: contributor

Please close this and continue the discussion in the PR #6323.
sipa commented at 9:19 AM on June 23, 2015: member

I don't understand how a vector lookup (which requires at least dereferencing a pointer and reading from memory) can be faster than using a constant.

I agree with Wladimir though; not for or against, but pointless.
paveljanik commented at 9:25 AM on June 23, 2015: contributor

In this case, the vector is const. It could be interesting to compare the actual output of compilers in both cases... But this is something completely different. This issue is really pointless.
vinniefalco commented at 10:29 AM on June 23, 2015: contributor

If someone were to change the alphabet, the function would no longer produce the correct result. For Bitcoin's purposes, its unlikely that the string would ever change. I did say this was minor.
luke-jr commented at 8:02 PM on June 23, 2015: member

@vinniefalco To be extremely blunt: Bitcoin Core does not exist to make Ripple (or anything non-Bitcoin) produce the correct results.
vinniefalco closed this on Jun 23, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors