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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes necessary for usage on Trezor #53

Merged
merged 19 commits into from
Apr 12, 2019

Conversation

real-or-random
Copy link
Collaborator

@real-or-random
Copy link
Collaborator Author

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.

@apoelstra
Copy link
Contributor

Also missing 8558e3d from bitcoin-core/secp256k1#595

@apoelstra
Copy link
Contributor

Would be good to add ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp); to secp256k1_cotnext_destroy as well as secp256k1_prealloc_destroy when cherry-picking 2ca27e2 from bitcoin-core/secp256k1#595

I also have a bunch of spacing changes in a6add6c, also from bitcoin-core/secp256k1#595, that you don't have. I think in this case I screwed up the cherry-pick and created a bigger diff than I needed to.

@real-or-random
Copy link
Collaborator Author

Would be good to add ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp); to secp256k1_cotnext_destroy as well as secp256k1_prealloc_destroy when cherry-picking 2ca27e2 from bitcoin-core/secp256k1#595

This check is actually present in the current PR in secp256k1_preallocated_destroy (and secp256k1_context_destroy is also covered because it calls secp256k1_preallocated_destroy internally).

I'm addressing the other issues.

@real-or-random
Copy link
Collaborator Author

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.

@apoelstra
Copy link
Contributor

ACK. Go ahead and squash.

real-or-random and others added 6 commits April 12, 2019 17:36
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.
@real-or-random
Copy link
Collaborator Author

done

@apoelstra
Copy link
Contributor

ACK

@apoelstra apoelstra merged commit 1c830b4 into BlockstreamResearch:secp256k1-zkp Apr 12, 2019
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.

3 participants