Add update sparse commitment #149
Conversation
| public void testUpdateCommitmentSparse() { | ||
| // Numbers and result is taken from: https://github.com/crate-crypto/rust-verkle/blob/bb5af2f2fe9788d49d2896b9614a3125f8227818/ffi_interface/src/lib.rs#L576 | ||
| // Identity element | ||
| byte[] old_commitment = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, |
There was a problem hiding this comment.
we are more using camelCase convention in this project. I think it will be better to keep the same convenstion everywhere in this code
| } | ||
|
|
||
| @Test | ||
| public void testUpdateCommitmentSparse() { |
There was a problem hiding this comment.
https://github.com/crate-crypto/rust-verkle/blob/bb5af2f2fe9788d49d2896b9614a3125f8227818/ffi_interface/src/lib.rs#L529. maybe we need to add this test. it seems to add only test with commitment which is identity(empty) is just testing a part of the code
There was a problem hiding this comment.
@matkt thanks, added it now
| byte[] old_commitment = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, | ||
| 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; | ||
|
|
||
| byte[] old_scalar_new_scalar_index = new byte[]{2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, |
There was a problem hiding this comment.
why not having that in hex ? maybe simpler ?
There was a problem hiding this comment.
@matkt fixed that now too
|
build is failing because of rust version you need to update Line 12 in b3f626d |
Signed-off-by: Dragan Pilipovic <draganpilipovic1997bp@gmail.com>
Signed-off-by: Dragan Pilipovic <draganpilipovic1997bp@gmail.com>
dragan2234
force-pushed
the
add-update-sparse-commitment
branch
from
c375ee3
to
fd241cf
Compare
Signed-off-by: Dragan Pilipovic <draganpilipovic1997bp@gmail.com>
Signed-off-by: Dragan Pilipovic <draganpilipovic1997bp@gmail.com>
|
Can we merge this? Or we're waiting on something before merging it? |
|
@gfukushima From my side it's fine to merge, this is needed for the work on Java trie (it's not used currently). |
Adding updateSparseCommitment with test. This can be merged after #146
Closes #129