ZenGo-X / class

Rust library for building IQC: cryptography based on class groups of imaginary quadratic orders
GNU General Public License v3.0
63 stars 31 forks source link

Update to curv@0.9. #52

Closed tmpfs closed 2 years ago

tmpfs commented 2 years ago

We also have to update this library for multi-party-ecdsa otherwise there is a cargo error with mismatched dependencies.

tmpfs commented 2 years ago

@survived, what is the replacement for bytes_compressed_to_big_int() please?

Edit: So it looks like I can call to_bytes(true) on an Point::<Secp256k1> to get an EncodedPoint but I am not sure how to get the BigInt from the EncodedPoint correctly.

Edit: So I ended up using BigInt::from_bytes() on the reference of the EncodedPoint, please check this is correct: https://github.com/ZenGo-X/class/pull/52/commits/3573e471ee68f355b3aaaffb256c139b89989479

tmpfs commented 2 years ago

@survived, please check I got the update to the Hmac handling correct: https://github.com/ZenGo-X/class/pull/52/commits/09a8bdf3d51d91e0e0d6d0c71dfafe2883bf0224 :pray:

tmpfs commented 2 years ago

@survived, down to one compiler error but I am a little bit stumped by this:

error[E0277]: the trait bound `BigInt: RingNormalize` is not satisfied
   --> src/lib.rs:350:29
    |
350 |         let mut b: BigInt = ring_algorithm::chinese_remainder_theorem(&[b1, b0], &[a1, g])?;
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `RingNormalize` is not implemented for `BigInt`
    |
   ::: /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/ring-algorithm-0.2.3/src/lib.rs:228:74
    |
228 |     T: sealed::Sized + Clone + Eq + num_traits::Zero + num_traits::One + RingNormalize,
    |                                                                          ------------- required by this bound in `chinese_remainder_theorem`

Clearly BigInt would need to implement RingNormalize but I am not clear whether implementing this trait is a new requirement and how this ever worked.

Also I noticed that ring_algorithm@0.2.3 which is in the cargo dependency list has been yanked: https://docs.rs/ring-algorithm/0.2.3/ring_algorithm/trait.RingNormalize.html

tmpfs commented 2 years ago

@survived, the tests are not compiling but I am getting a segault, here is the rust-lldb output:

Process 7004 launched: '/home/muji/git/consensys/class/target/debug/deps/class_group-51b5d77c8abbf2c6' (x86_64)

running 30 tests
Process 7004 stopped
* thread #4: tid = 7009, 0x00007ffff6efa438 libc.so.6`__GI_raise(sig=6) + 56 at raise.c:54, name = 'primitives::cl_', stop reason = signal SIGABRT
    frame #0: 0x00007ffff6efa438 libc.so.6`__GI_raise(sig=6) + 56 at raise.c:54
(lldb) bt
* thread #4: tid = 7009, 0x00007ffff6efa438 libc.so.6`__GI_raise(sig=6) + 56 at raise.c:54, name = 'primitives::cl_', stop reason = signal SIGABRT
  * frame #0: 0x00007ffff6efa438 libc.so.6`__GI_raise(sig=6) + 56 at raise.c:54
    frame #1: 0x00007ffff6efc03a libc.so.6`__GI_abort + 362 at abort.c:89
    frame #2: 0x00007ffff6f3c7fa libc.so.6`__libc_message(do_abort=<unavailable>, fmt=<unavailable>) + 730 at libc_fatal.c:175
    frame #3: 0x00007ffff6f4538a libc.so.6`_int_free + 1578 at malloc.c:5020
    frame #4: 0x00007ffff6f45310 libc.so.6`_int_free(av=<unavailable>, p=<unavailable>, have_lock=0) + 1456 at malloc.c:3874
    frame #5: 0x00007ffff6f4958c libc.so.6`__GI___libc_free(mem=<unavailable>) + 76 at malloc.c:2975
    frame #6: 0x0000555555cd7c03 class_group-51b5d77c8abbf2c6`expand_path + 99
    frame #7: 0x0000555555cd8074 class_group-51b5d77c8abbf2c6`pari_init_paths + 20
    frame #8: 0x0000555555d15239 class_group-51b5d77c8abbf2c6`pari_init_opts + 953
    frame #9: 0x0000555555683ad7 class_group-51b5d77c8abbf2c6`class_group::primitives::cl_dl_lcm::HSMCL::keygen::he01fe27cfcb0761a + 103 at cl_dl_lcm.rs:84
    frame #10: 0x0000555555657212 class_group-51b5d77c8abbf2c6`class_group::primitives::cl_dl_lcm::tests::test_encryption_mul_by_scalar_secp256k1_lcm::hd47373b233b94a88 + 274 at cl_dl_lcm.rs:622
    frame #11: 0x000055555567e4ba class_group-51b5d77c8abbf2c6`class_group::primitives::cl_dl_lcm::tests::test_encryption_mul_by_scalar_secp256k1_lcm::_$u7b$$u7b$closure$u7d$$u7d$::h3d9d547fdac783a7 + 10 at cl_dl_lcm.rs:609
    frame #12: 0x00005555556315ae class_group-51b5d77c8abbf2c6`core::ops::function::FnOnce::call_once::h3e04af64967d79ff + 14 at function.rs:227
    frame #13: 0x0000555555e21fd3 class_group-51b5d77c8abbf2c6`test::__rust_begin_short_backtrace::h0908f3b1fdbb72ab + 3 at function.rs:227
    frame #14: 0x0000555555e20a90 class_group-51b5d77c8abbf2c6`test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::h6fdc58c6d5f5b6e3 + 832 at boxed.rs:1575
    frame #15: 0x0000555555dfa142 class_group-51b5d77c8abbf2c6`std::sys_common::backtrace::__rust_begin_short_backtrace::hb560b77c4f701523 + 178 at lib.rs:519
    frame #16: 0x0000555555dfdcf8 class_group-51b5d77c8abbf2c6`core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h745ea30807a29e55 + 104 at mod.rs:481
    frame #17: 0x0000555555fb21f7 class_group-51b5d77c8abbf2c6`std::sys::unix::thread::Thread::new::thread_start::h8c7c4450dba62914 + 39 at boxed.rs:1575
    frame #18: 0x00007ffff77a36ba libpthread.so.0`start_thread + 202
    frame #19: 0x00007ffff6fcc4dd libc.so.6`__clone + 109 at clone.S:109
tmpfs commented 2 years ago

I am also seeing this error sometimes when I run the tests:

running 30 tests
  ***   the PARI stack overflows !
  current stack size: 10000000 (9.537 Mbytes)
  [hint] set 'parisizemax' to a non-zero value in your GPRC
  ***   Error in the PARI system. End of program.
error: test failed, to rerun pass '--lib'

Edit: I checked out master and ran the tests and still get this error so it appears it is not related to the changes in this PR.

tmpfs commented 2 years ago

Ok my bad, I should have read the README, running cargo test -- --test-threads=1 works on master and this branch.

Apologies for the noise :pray:

survived commented 2 years ago

what is the replacement for bytes_compressed_to_big_int() please?

So I ended up using BigInt::from_bytes() on the reference of the EncodedPoint

You're completely right, it should nicely match previous behaviour

please check I got the update to the Hmac handling correct

Yep, you got it correctly. I would even get rid of HmacSha512 type alias, Hmac<Sha512> should work just fine.

Also I noticed that ring_algorithm@0.2.3 which is in the cargo dependency list has been yanked

We used to have curv's bigints implementing RingNormalize trait, but the crate was yanked as you correctly observed. That's basically why we removed dependency on ring_algorithm in curv (by copypasting algorithms that we used to use). I would suggest to do the same in this crate, let me try to do this - I'll copypast chinese_remainder_theorem and we won't need to introduce this dependency again.

tmpfs commented 2 years ago

Thanks @survived, I now have multi-party-ecdsa spitting out all the usual compiler errors so it looks like we are on the home stretch for the upgrade :+1:

survived commented 2 years ago

Great, @tmpfs! While you're fighting with multi-party-ecdsa, I'll resolve these clippy warnings we have here

tmpfs commented 2 years ago

@survived, I think this is ready for review now :+1: