hyperledger / besu-native

Apache License 2.0
12 stars 35 forks source link

add update commitment to ipa_multipoint JNI #127

Closed dragan2234 closed 7 months ago

dragan2234 commented 9 months ago

Changing out ipa_multipoint API to these functions:

Everything expects BE bytes, and returns BE bytes.

I still think Java is working on BE bytes by default. Please re-run TestPedersenCommitment and put a breakpoint. Example and screenshot:

995CEEBE-40F4-4DAD-B511-8706FA202285

so [28, -127, 74 ....

is 1c,81,4a in hex

Only thing that is working on LE is "serialize" call from arkworks

dragan2234 commented 9 months ago

This is ready for review @matkt @thomas-quadratic .

Note that it will break some things in besu-verkle-trie repo since we changed the API a bit per the description above.

So in order to get "commitment" what is described as commitment in besu-verkle-trie you just call LibIpaMultipoint.commit() and you get the C serialized to bytes(Fp).

Now if you want to reuse in parent or save it as "a hash" you just call LibIpaMultipoint.groupToField().

Same thing applies for LibIpaMultipoint.updateCommitment().

thomas-quadratic commented 9 months ago

This PR addresses issues #129 #128

thomas-quadratic commented 9 months ago

Thanks @dragan2234 for the good work.

Concerning LE vs BE bytes, this is another issue, and we can address it in another PR if needed.

Concerning the breaking change to the API, I would prefer to make the necessary changes on the Java side to test it and make sure we are ready before merging this PR. This way, we limit chances of interruption with current testing against testnet.

It is a good idea to separate groupToField from commitment. It will allow also further help in batching inversions for more optimisation. However, note that currently to retrieve commitment_hash, we need 2 calls from Java, one to retrieve the commitment, another one to map it to field. Unfortunately, currently deserialisation from Java to rust is costly. Good news is that it should be addressed with uncompressed serialisation.

In summary : nothing to change in the PR, but would like to wait before merging for integration in besu-verkle-trie.