-
Notifications
You must be signed in to change notification settings - Fork 208
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
Changes necessary for usage on Trezor #53
Changes necessary for usage on Trezor #53
Conversation
* Determine ALIGNMENT more cleverly and move it to util.h * Implement manual_malloc() helper function
9759f6f
to
2905b62
Compare
Forced pushed because I forgot to cherry-pick the (cosmetic) 76c7964 from bitcoin-core/secp256k1#596, and I also added a zkp-specific follow-up commit on this commit. |
Also missing |
Would be good to add I also have a bunch of spacing changes in |
This check is actually present in the current PR in I'm addressing the other issues. |
I added 8558e3d and a fixup from bitcoin-core/secp256k1#596, both just on top. I didn't squash or reorder to make sure you don't have to recreate it again from scratch. Let me know when I should squash. |
ACK. Go ahead and squash. |
This makes WINDOW_G a configurable value in the range of [3..24]. The upper limit of 24 is a defensive choice. The code is probably correct for values up to 33 but those larger values yield in huge tables (>= 256MiB), which are i) unlikely to be really beneficial in practice and ii) increasingly difficult to test.
Before this commit secp256k1_context_randomize called illegal_callback when called on a context not initialized for signing. This is not documented. Moreover, it is not desirable because non-signing contexts may use randomization in the future. This commit makes secp256k1_context_randomize a noop in this case. This is safe because the context cannot be used for signing anyway. This fixes #573 and it fixes rust-bitcoin/rust-secp256k1#82.
da90713
to
cd364a0
Compare
done |
ACK |
This is the result from cherry-picking the following upstream PRs (in this order) in their current state, see the PRs for details:
cc @practicalswift: The reason I asked you to update bitcoin-core/secp256k1#587 is that I needed the change here. Sorry, I didn't mention this, I hope you don't mind. :)