JNI rebased #364
JNI rebased #364
Conversation
|
Thanks for pulling this forward and testing. It looks like Travis is still having a few problems in picking up the jni object, but I'm going to close my original PR in favor of this, as its rebased and tests are passing (running make check-java) locally at least |
|
@faizkhan00 thank you and @theuni (and anyone else involved I missed) for the work done thus far. tests are passing locally but there seems to be a race condition in make where by the shared library is not fully linked by the time the test starts (but strangely only in travis, we can't reproduce locally) - looks like we need to fix the dependency we tried to fix that and failed (we are not too strong on makefiles) so we asked @theuni and he suggested he may have a look at this at some point but anyone with any input is welcome |
|
@greenaddress Sorry for not getting to this at the meetup as promised, I kept getting pulled away. Anyway, that's probably for the best. This turned out to be non-trivial to track down, and not build-system related at all. See the top few commits here for the fix: https://github.com/theuni/secp256k1/commits/jni-test tl;dr: JNIEXPORT on some Java versions (the one Travis uses by default, at least) is busted wrt symbol visibility. |
|
@theuni no worries! Cheers for finding the actual source of the problem! |
|
Concept ACK, but can you rebase and squash the fixups? |
11f1e27 to
0a30103
Compare
|
|
||
| assertEquals(pubArr.length, pubLen, "Got bad signature length."); | ||
|
|
||
| assertEquals(retVal, 1, "Failed return value check."); |
There was a problem hiding this comment.
This function can legitimately return 0, if the passed signature's R coordinate does not correspond to a point on the curve (around 50% chance for arbitrary byte arrays).
Squashed and rebased. Thanks to @theuni and @faizkhan00 for doing the majority of work here! Also thanks to @btchip for help with debugging and review.
0a30103 to
86e2d07
Compare
|
I've now removed the unimplemented and commented parts and cleaned it up in general. I think it's reasonable to keep these two commits for future reference, in case someone wants to look up the incomplete implementations in git history. |
This is another continuation of #64 and #248 implementing JNI library for secp256k1, rebased to master.
Also it includes a fix for a bug we noticed while including it in GreenBits - buffers were overflowing if two different functions were called consecutively with the second one needing more buffer space.