search

status-im / nim-blscurve

Nim implementation of BLS signature scheme (Boneh-Lynn-Shacham) over Barreto-Lynn-Scott (BLS) curve BLS12-381
Apache License 2.0
26 stars 11 forks source link

Upgrade BLST and remove 2 now unnecessary workarounds #84

Closed mratsim closed 3 years ago

mratsim commented 3 years ago

Our BLST target commit is slightly old. We should update.

  1. The main benefit is the release of a portable SHA256 which doesn't require SSE3:

And thus we wouldn't require this workaround: https://github.com/status-im/nim-blscurve/blob/86d151d7776d5874a9ea5911bfe12747b25925b2/blscurve/bls_backend.nim#L23-L32

  1. The second benefit is that the aliasing bug https://github.com/supranational/blst/issues/22 as been closed upstream

And thus we wouldn't require this workaround https://github.com/status-im/nim-blscurve/blob/86d151d7776d5874a9ea5911bfe12747b25925b2/blscurve/blst/blst_lowlevel.nim#L12-L18

  1. This is not just a submodule bump, we need to take into account the API change at
stefantalpalaru commented 3 years ago

Do they have any kind of autodetection, or are we supposed to do it for them and set the proper defines?

mratsim commented 3 years ago

no autodetection, they use what the compiler sets:

https://github.com/supranational/blst/blob/a8398ed284b0d78858302ad1ceb25a80e7bbe535/build/assembly.S#L35-L40

stefantalpalaru commented 3 years ago

They do for __ADX__, but it looks like you need to set __BLST_PORTABLE__ yourself, to get the generic ASM on CPUs without SSSE3.

mratsim commented 3 years ago

CLosed in #86