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

Java implementation #19

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Java implementation #19

wants to merge 8 commits into from

Conversation

SamouraiDev
Copy link

@SamouraiDev SamouraiDev commented Jun 19, 2017

Bech32 encoding of SegWit addresses in Java. Build instructions are in README.md.


private static byte[] createChecksum(byte[] hrp, byte[] data) {

byte[] zeroes = new byte[] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
Copy link
Owner

Choose a reason for hiding this comment

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

final?

Copy link
Author

Choose a reason for hiding this comment

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

Updated.


public static Pair<byte[], byte[]> bech32Decode(String bech) throws Exception {

if(!bech.equals(bech.toLowerCase()) && !bech.equals(bech.toUpperCase())) {
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a risk that toLowerCase/toUpperCase are locale dependent?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think so given the character set being used. In any case, placing the test after the byte value test should remove any doubt.

Copy link
Owner

Choose a reason for hiding this comment

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

I'd still be a bit more confortable with bech.toLowerCase(Locale.ROOT), which should be deterministic?

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

if (!hrp.equals(hrpgotStr)) {
throw new Exception("mismatching bech32 human readeable part");
}
if (!hrpgotStr.equalsIgnoreCase("bc") && !hrpgotStr.equalsIgnoreCase("tb")) {
Copy link
Owner

Choose a reason for hiding this comment

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

I would leave that up to the caller, who should know what hrp is expected anyway.

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

ret.add((byte)((acc << (toBits - bits)) & maxv));
}
else if (bits >= fromBits || (byte)(((acc << (toBits - bits)) & maxv)) != 0) {
throw new Exception("panic");
Copy link
Owner

Choose a reason for hiding this comment

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

This case is different from the cases above, as it can occur without any programmer error (malformed input can trigger it), so I'm unsure if an exception is the right way to deal with it.

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@sipa
Copy link
Owner

sipa commented Sep 18, 2017

Thanks a lot for doing this, and my apologies for the slow responses.

Two comments:

  • I prefer to have "reference implementations" - things that are easily copy-pastable (and possibly modifiable) into your own project, rather than fully-fledged packages. That means preferably few dependencies or build systems - obviously depending on the programming culture in that language. Are the two apache things you depend on typical things to use?
  • Could the hrp argument to encode, and the left value of the pair returned by decode by a string rather than a byte vector? That would seem more intuitive and easier to use, as the hrp is intended to be human readable anyway.

@SamouraiDev
Copy link
Author

SamouraiDev commented Sep 18, 2017

  • 'hrp' is now passed and returned as a String.
  • The commons.lang lib, while not part of the base language, is fairly standard. I've replaced the ArrayUtils calls by straightforward conversion code. A 'Pair' class was dropped in that is source code compatible with the commons.lang version so anyone using the classes can import 'org.apache.commons.lang3.tuple.Pair' and discard the provided 'Pair' class if so desired.
  • The only remaining 3rd party lib used is in the test harness. None are used in the bech32 classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants