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

Review usage of exceptions and try/except blocks #57

Closed mratsim closed 4 years ago

mratsim commented 4 years ago

I don't think we should have any exceptions or try/except block at all in a crypto backend.

This was triggered on an empty aggregate test:

image

https://github.com/status-im/nim-blscurve/blob/b435f1a7296fceda8a68aaca391e6d76b2c632d3/blscurve/common.nim#L668-L677

And there is another one here:

https://github.com/status-im/nim-blscurve/blob/b435f1a7296fceda8a68aaca391e6d76b2c632d3/blscurve/common.nim#L594-L601

i.e. {.push raises: [].} and introduce an exception-free hexToSeqBytes

arnetheduck commented 4 years ago

oops - this looks like a bug in hexSeqToByte which should raise a ValueError - that said, I'd agree in general but for that we need to do some groundwork (error-free parser, RNG)

arnetheduck commented 4 years ago

clear your package cache, this was fixed long ago