Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return uint_8 instead of bool from all ckzg functions #353

Closed
wants to merge 2 commits into from

Conversation

pawanjay176
Copy link
Contributor

@pawanjay176 pawanjay176 commented Aug 31, 2023

I think we might have finally found the root cause of all our windows rust issues.
Thanks to @michaelsproul for helping find this 馃檹

blst.h has this redefinition of bool
https://github.com/supranational/blst/blob/78fee18b25e16975e27b2d0314f6a323a23e6e83/bindings/blst.h#L27-L31

Because we import blst.h In c-kzg-4844.h, bool in the c source code is redefined as an int on windows. So in the c interface for all verify_* functions, bool *ok is in fact int *ok.

When generating the rust bindings, bindgen made the stupid mistake of assuming that a bool in C would be a bool in rust. So on calling the underlying C function, rust allocates 1 byte for the bool type where C required 4. This is probably what was causing the undefined behaviour all this time.

Making this PR just to run CI on the issue. There's probably a better way of fixing this in C.

Running this with lighthouse on devnet 8. No crashes so far 馃帀

@jtraglia
Copy link
Member

This is no longer needed. The root issue was identified. Fixed here:

@jtraglia jtraglia closed this Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants