search

ethereum / c-kzg-4844

A minimal implementation of the Polynomial Commitments API for EIP-4844 and EIP-7594, written in C.
Apache License 2.0
116 stars 107 forks source link

Crashes with SIGILL on Linux/amd64 i7 CPU #291

Closed karalabe closed 1 year ago

karalabe commented 1 year ago

Caught SIGILL in blst_cgo_init [...]

karalabe commented 1 year ago

Seems my older laptop CPU is too old for for some instructions. This is bad really, the upstream solution is "if it crashes, rebuild with flag xyz". Can we please fix the upstream BLS library to detect and not use unavailable CPU. instructions, or alternatively have this library add the "portable" flags via CGO build constraints? I cannot add the flag downstream into Geth.

jtraglia commented 1 year ago

We cannot add __BLST_PORTABLE__ to the CGO_CFLAGS either. I suggest you make a PR to blst.

karalabe commented 1 year ago

I'm reluctant to accept that suggestion as a viable one.

The issuer was opened on their repo 3 years ago and they don't seem to care: https://github.com/supranational/blst/issues/10

In the issue, it's also written that about 25% of cloud VMs will crash, interestingly, at random since you don't know exactly which CPU you will get (unless you pay enough to have a specifically new gen one).

As for fixing it upstream, with all due respect, it is your dependency - and hence your library - that is broken. It is your task to fix it or work around a broken dependency. My choices are either to use CKZG or to not (and be very vocal about why it is broken so others won't use it either).