search

keis / base58

Base58 and Base58Check implementation compatible with what is used by the bitcoin network.
MIT License
180 stars 59 forks source link

made it handle any base with passing alphabet. #63

Closed tanupoo closed 3 years ago

tanupoo commented 3 years ago

Hi David,

Thanks for providing wonderful code. Many other base58 implementation do not support binary encoding and they ignore the preceding zeros in the plain data. Your code can perfectly handle this case.

I modified and use for my use case i.e. base45. However, I just noticed that my modification was very few and it might be useful for other people. So, I make a pull request.

Please merge my small diff if it would be acceptable in your mind.

Thanks, Shoichi

keis commented 3 years ago

Hey @tanupoo, thanks for the PR

Supporting other bases is not a goal of this project but considering how small of a change this I think it's worth it.

However because I'm only using this for base58 myself could you please add a test with base45 in test_base58.py to catch any regressions on this.

tanupoo commented 3 years ago

Hi David,

Added test_base45.py with one fix in init.py, which is added to check if a space is included in the alphabet. The reason why I added this check is that my alphabet has a space as an encoded charactor.

Just submitted my home work.

I fully understand it's not your goal and this check is not required for base58. So, I don't push my request anymore. I totally leave it your decision.

Thanks, Shoichi

keis commented 3 years ago

Thanks @tanupoo