replace secp256k1 to libsecp256k1-rs

[namuyan]

#43
test passed by cargo test --features ec_secp256k1

type	before	after	test code link
1 of 1 aggregate	112ms	3ms	aggregate_1_of_1.py
n of n aggregate	458ms	7ms	aggregate_n_of_n.py
t of n threshold	2475ms	68ms	thresholdbig_t_of_n.py

performance improved!
please check my code, thank you.

[namuyan]

Now I find two library libsecp256k1-rs and libsecp256k1, I use libsecp256k1-rs.
libsecp256k1-rs is fork of libsecp256k1, and have Copy trait and overwrite operators but is not main stream and no production code.
libsecp256k1 require wide modification and have influence to others, how do you think?

[omershlo]

Hi @namuyan - looks like great work.
We started experimenting with this idea in another project: https://github.com/KZen-networks/emerald-city
there we took only the secp256k1 relevant parts of curv and replaced them with libsecp256k1 (the second lib, the native one).

[namuyan]

Thank you for reply.
I now use curv for my project, but emerald-city looks more functional and pure-Rust implement (and compatibility with curv?).
I may use the latter, thank you.

[namuyan]

I just change README.md?
UNLUCKY DICE, It looks a low repeatability bug.
https://github.com/KZen-networks/curv/blob/master/src/cryptographic_primitives/commitments/hash_commitment.rs#L78

[omershlo]

you are right, we just need to run it again. The problem is that it is unknown for me how to do unit test for probabilistic code. We got stuck on this a while ago (#10).

Any suggestions will be welcome.

[vhnatyk]

Thank you for reply.
I now use curv for my project, but emerald-city looks more functional and pure-Rust implement (and compatibility with curv?).
I may use the latter, thank you.

rust-bitcoin/rust-secp256k1#125 (comment)

According to latest merged PR in original rust-secp256k1 - it now supports WASM that was only benefit of libsecp256k1. Performance benefit over rust-secp256k1 is supposedly due to later being constant time. Additionally rust-secp256k1 is a wrapper over original Bitcoin's secp256k1 that is most tested and commonly used library out there. So maybe it's a good idea to switch back to it in emerald-city as well.

[omershlo]

@vhnatyk I agree. Let's switch back.
@elichai kudos

[elichai]

Glad to hear :)
Using good vetted crypto libraries is important.

[elichai]

FYI After this: rust-bitcoin/rust-secp256k1#132
There should be a ~15% performance improvement in verification time.

[elichai]

Oh.
I just realized why your performance is so bad.
you launched a new context for every operation: https://github.com/KZen-networks/emerald-city/pull/6/files#diff-cd37acb447aaca91b67e2e383e1b43afL391

Context creation in Secp256k1 is the most intensive part of the library.

You should use lazy_static! or std::syn::Once()(example: https://github.com/elichai/ecc-secp256k1/blob/master/src/secp256k1.rs#L480)
to create the context once and then use it everywhere.
(the context is thread safe as it doesn't mutate after creation)

[elichai]

Ok. let's put the benchmarking stats to bed once and for all:
I used this repo: https://github.com/namuyan/multi-party-schnorr on commit: namuyan/multi-party-schnorr@6c11c40

Before I changed anything I got consistently these stats:
1-of-1 = 142 mSec
n-of-n = 447 mSec
t of n threshold = 3048 mSec

After I changed the context to be created before the tests and then used it globally I got these stats:
1-of-1 = 1mSec
n-of-n = 1.5 mSec
t of n threshold = 42mSec

And this is with a lot of over head.
in secp256k1 sign/verify time are measured in uSec(micro). not mSec(mili).

@omershlo @namuyan.
If you want I can open a PR for lazy_static/sync::Once

And this is before the 15% faster verification using gmp

[omershlo]

@elichai please!

[elichai]

Will do.
Do you prefer unsafe code or a new dependency? (in reality their doing almost the same thing)

[omershlo]

unsafe

[namuyan]

Thank you for quality up!

After I changed the context to be created before the tests and then used it globally I got these stats:
1-of-1 = 1mSec
n-of-n = 1.5 mSec
t of n threshold = 42mSec

Is this performance with emerald-city? or curv?

I replaced to https://github.com/elichai/emerald-city/tree/secp
patch: https://pastebin.com/pMvmPdB0
But I cannot measure performance, 1-of-1 and n-of-n show 0nS, t-of-n shows 32mS.

[elichai]

curv. and create a key before you measure performance so that the context will be available already (you shouldn't count the context creation as part of the performance test) (and change your performance tests to use nanoseconds)