-
Notifications
You must be signed in to change notification settings - Fork 108
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
Simplify nim bindings #482
Conversation
03be579
to
c88a853
Compare
3e9c78f
to
50c3c29
Compare
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.
LGTM. Besides loadTrustedSetupFromString
, Nim actually can optimize it further via compile time mechanism to load baked in array from executable binary. But that better put into another PR.
@jangko Oh cool. Yes let's do that in another PR. Any thoughts on the optimization issue on Windows? I've disabled compiler optimizations as a bandaid fix until we figure it out. I asked about it on Discord (in Nimbus's PeerDAS channel):
To be clear, this happens when the high level wrapper function ( |
Thank you @jangko for identifying the issue:
After adding this to the type, the Windows CI tests pass again. |
This branch was tested by Agnish (nimbus dev) and confirmed it compiles & passes the reference tests. Going to merge. |
I would like to simplify the Nim bindings before the big v2.0 release. This PR does the following:
KzgCtx
like the other bindings do.free_trusted_setup
is called on exit.Some thoughts on this from our discussion on discord:
Opening as a draft until @agnxsh tests it.
I would also appreciate a review from @jangko, the author of these bindings.