add the jni binding for schnorr sign
Details
Details
- Reviewers
deadalnix Fabien jasonbcox - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project - Commits
- rSTAGING60e30bca748a: [secp256k1] add schnorr sign jni binding
rABC60e30bca748a: [secp256k1] add schnorr sign jni binding
run the single test but ill add more tests. any suggestions where i can take failing and positive tests is welcome
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
The diff title is is about schnorr_verify but the content is about schnorr_sig.
Maybe you can look at the tests from libsecp256k1 to add the missing failing cases.
| src/secp256k1/src/java/org/bitcoin/NativeSecp256k1.java | ||
|---|---|---|
| 427 ↗ | (On Diff #7874) | Is an ECDSA buffer appropriated for Schnorr signature ? |
| src/secp256k1/src/java/org_bitcoin_NativeSecp256k1.c | ||
| 370 ↗ | (On Diff #7874) | Why not intsarray[0] = secp256k1_schnorr_sign(ctx, sig, data, secKey, NULL, NULL); ? |
| src/secp256k1/src/java/org/bitcoin/NativeSecp256k1.java | ||
|---|---|---|
| 435 ↗ | (On Diff #7880) | Just from looking at this diff, it looks like`byteBuff` and seckey are not being explicitly cleaned up before this function exits. Given that Java's GC is "at will" the secret key could be floating around in memory. If this memory is not being cleaned up somewhere, we may have to zero them out and clear them before exiting. The exact behavior may be impacted by my other question below about memory leaking. If a new object is NOT created, then we need to know the consequences of altering that memory. Note that this issue may also impact other ECDSA functions in this file as well. |
| 427 ↗ | (On Diff #7874) | Changes to this will likely depend on D2756 |
| src/secp256k1/src/java/org_bitcoin_NativeSecp256k1.c | ||
| 360 ↗ | (On Diff #7880) | I could only find the Java doc for this function: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/jni-14.html#GetDirectBufferAddress it seems to indicate that a new array is allocated. Does that mean this is leaking? I don't see data being cleaned up. |
Comment Actions
Accepted, but it will need to be rebased on master.
| src/secp256k1/src/java/org/bitcoin/NativeSecp256k1.java | ||
|---|---|---|
| 435 ↗ | (On Diff #7880) | Created a task for this since it's not necessary to block this diff for it: https://reviews.bitcoinabc.org/T587 |
| src/secp256k1/src/java/org_bitcoin_NativeSecp256k1.c | ||
| 360 ↗ | (On Diff #7880) | After more digging, @floreslorca pointed me to https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.vm.80.doc/docs/jni_refs.html which indicates that objects created by the JNI will be cleaned up when the native function returns to the calling JVM. Assuming this is right, there should be no leak here. |
| src/secp256k1/src/java/org/bitcoin/NativeSecp256k1Test.java | ||
|---|---|---|
| 321 ↗ | (On Diff #8061) | Nit: move the comment above |