chore: Remove direct dependency on ipa-multipoint #144
Conversation
|
Depends on #143 |
There was a problem hiding this comment.
Hi thanks for all the improvements. This is looking good now.
I believe this PR breaks the Java-side because it assumes that it passes as input uncompressed serialised commitments. If so, this is ok. However, while we are at it, I would also make the assumption that inputs and outputs from/to Java are little endian.
| _class: JClass<'_>, | ||
| input: jbyteArray, | ||
| ) -> jbyteArray { | ||
| let mut input = env |
There was a problem hiding this comment.
Should we use the same way to read inputs as line 46 ? It is safer I believe
let mut input: [u8; 64] = match input.try_into() {
Ok(input) => input,
Err(_) => {
env.throw_new(
"java/lang/IllegalArgumentException",
"Invalid input length. Should be 64-bytes.",
)
.expect("Failed to throw exception");
return std::ptr::null_mut(); // Return null pointer to indicate an error
}
};
There was a problem hiding this comment.
Perhaps I'm misunderstanding; with let mut input: [u8; 64] = match input.try_into() the size of the input is known at compile time, whereas with commit and commitRoot, I'm not sure we know the size of the input(how many scalars we should commit to) at compile time
There was a problem hiding this comment.
Ah, I missed that!
Nonetheless, it is strange that in one case we throw a Java Error and in the other we don't.
However, I am not sure how error propagation works on the JNI side.
I think that we can do as is and if the need emerge to handle errors in another way, we will do it then.
There was a problem hiding this comment.
I will prefer to use
env.throw_new(
"java/lang/IllegalArgumentException",
"message",
)
.expect("Failed to throw exception");
everytime in order to be sure java will catch this cleanly imo
There was a problem hiding this comment.
as it's not related to this PR I think we can merge it
| _class: JClass<'_>, | ||
| input: jbyteArray, | ||
| ) -> jbyteArray { | ||
| let mut input = env |
There was a problem hiding this comment.
Same comment, we should have a uniform way of converting inputs and outputs from/to Java.
| return env.byte_array_from_slice(&commit_bytes).expect("Couldn't convert to byte array"); | ||
| } | ||
| let commitment = ffi_interface::commit_to_scalars(committer, &input).unwrap(); | ||
| let hash = ffi_interface::deprecated_serialize_commitment(commitment); |
There was a problem hiding this comment.
Is deprecated_serialize_commitment deprecated ?
I suppose this is the compressed serialisation.
There was a problem hiding this comment.
There are currently two ways to serialize a group element -- I'm looking to deprecate this one on the API level -- it is used in other places, but ideally not directly by the caller as it is being used now
|
tried this version and I have a valid state root for genesis block 637 ms with this current implementation to generate the root @kevaundray @thomas-quadratic @dragan2234 |
|
Ok, so once the DCO check passes, I think we can merge. |
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
…yperledger#142) Modify test vectors to use valid scalars --------- Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
kevaundray
force-pushed
the
kw/use-context-object
branch
from
2c3afcf
to
8d0c85a
Compare
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Signed-off-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
This removes the direct dependency on ipa-multipoint, so that only one import is needed; ffi-interface.