-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perturbing/add msm bls #514
base: master
Are you sure you want to change the base?
Perturbing/add msm bls #514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't looked into ffi calls, just at the haskell portion of it.
I'll look into this in more depth once PR is out of draft
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
Thank you for taking a first look @lehins, much appreciated! The FFI is not working yet, I am getting some segmentation faults that I am trying to debug with valgrind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have quickly looked through the PR, but haven't spotted exactly what is wrong with this PR, but there is definitely something seriously wrong with memory management in this PR. All the non-deterministic test failures, which should not be the case with functionality that pretends to be "pure" serve as a good indicator that the functionality has a bug. Moreover, the fact that unrelated tests are affected is another strong indicator that something is seriously wrong!
I'll try to dig deeper into this functionality some time this week. Maybe I can help you get to the bottom of this.
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
…1/Internal.hs Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
…1/Internal.hs Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
…1/Internal.hs Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
…1/Internal.hs Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
…1/Internal.hs Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
Co-authored-by: Alexey Kuleshevich <lehins@yandex.ru>
cardano-crypto-class/src/Cardano/Crypto/EllipticCurve/BLS12_381/Internal.hs
Outdated
Show resolved
Hide resolved
e51ee19
to
53daf8e
Compare
Description
Checklist
CHANGELOG.md
for the affected packages.New section is never added with the code changes. (See RELEASING.md)
.cabal
andCHANGELOG.md
files according to theversioning process.
.cabal
files for all affected packages are updated.If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
.git-blame-ignore-revs