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.

Sign up for GitHub

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

Jump to bottom

Further changes after making tables static #1065

Open
4 of 5 tasks
real-or-random opened this issue Jan 17, 2022 · 2 comments
Open
4 of 5 tasks

Further changes after making tables static #1065

real-or-random opened this issue Jan 17, 2022 · 2 comments
Labels
build user-documentation user-facing documentation
Milestone
stable release (1...

Comments

@real-or-random
Copy link
Contributor

real-or-random commented Jan 17, 2022 •
edited
Loading

More things to improve after #988:

  • Compile precomputation as a separate object file and link it (solved by Follow-ups to making all tables fully static #1042)
  • Speed up secp256k1_ecmult_gen_context_build at context creation. It currently computes fixed values which could be made static (open PR: ecmult_gen: Skip RNG when creating blinding if no seed is available #1120)
  • Document (or set by default) build options to remove unused static tables (and code) when no signing/verification function is called (something like --disable-shared CFLAGS="-fdata-sections -ffunction-sections -O2 -g" LDFLAGS="-Wl,--gc-sections")
  • Document the backwards-compatible API changes made in Make signing table fully static #988 and in Replace ecmult_context with a generated static array. #956: All contexts except the no_precomp context are now effectively signing contexts. The no_precomp context is effectively a verification context., and name is misleading as no context uses dynamic precompuation now. The reason why no_precomp is different is that it's impossible to re-randomize it.
  • Decide what to do with the no_precomp context: Possibilities include: renaming it, deprecating it (its main user rust-secp256k1 won't like this), and/or promote it a full signing context, maybe with a verbose name such as "global-context-less-secure" in the spirit of what rust-secp256k1 is doing.
@real-or-random real-or-random mentioned this issue Jan 19, 2022
Merged
@real-or-random real-or-random mentioned this issue Feb 1, 2022
Open
@real-or-random
Copy link
Contributor Author

real-or-random commented Feb 2, 2022 •
edited
Loading 

* [ ]  Decide what to do with the `no_precomp` context: Possibilities include: renaming it, deprecating it (its main user rust-secp256k1 won't like this), and/or promote it a full signing context, maybe with a verbose name such as "global-context-less-secure" in the spirit of what rust-secp256k1 is doing.

I suggest we

  • rename _no_precomp to _builtin (or similar)
  • keep a deprecated alias _no_precomp
  • expose the self-tests in the public API

We should get rid of the "signing"/"verification" terminology. But there's potential for bikeshedding. We could also just call it secp256k1_context_builtin, or secp256k1_context_static, or secp256k1_context_basic which is generic enough that we'll never have to rename it again, even if we add cpuid/whatever in the future (see #780). A more verbose version will be _no_secret_ops. But ECDH is a secret op but we don't have blinding... And I don't mind that people are forced to at least have a glimpse at the docs.

Does this sound good?

As a next step, if desired, we could introduce a variant builtin_secret_ops_less_secure (or similar) which is a "signing" context that cannot be randomized... Not sure if we want this.

@real-or-random real-or-random changed the title Futher changes after making tables static Further changes after making tables static Feb 8, 2022
@jonasnick
Copy link
Contributor

I suggest we

  • rename _no_precomp to _builtin (or similar)
  • keep a deprecated alias _no_precomp
  • expose the self-tests in the public API

Concept ACK

I'm not aware of the full scope of the context redesign discussions, but it seems like people find the no_precomp context useful. You mentioned the name secp256k1_context_static in PM, which I prefer over the mentioned alternatives.

@real-or-random real-or-random mentioned this issue Jul 5, 2022
Merged
real-or-random added a commit that referenced this issue Jul 7, 2022
@real-or-random
Merge #1120: ecmult_gen: Skip RNG when creating blinding if no seed i…
63a3565 
…s available

55f8bc9 ecmult_gen: Improve comments about projective blinding (Tim Ruffing)
7a86955 ecmult_gen: Simplify code (no observable change) (Tim Ruffing)
4cc0b1b ecmult_gen: Skip RNG when creating blinding if no seed is available (Tim Ruffing)

Pull request description:

  Running the RNG is pointless if no seed is available because the key
  will be fixed. The computation just wastes time.

  Previously, users could avoid this computation at least by asking for
  a context without signing capabilities. But since 3b0c218 we always
  build an ecmult_gen context, ignoring the context flags. Moreover,
  users could never avoid this pointless computation when asking for
  the creation of a signing context.

  This fixes one item in #1065.

ACKs for top commit:
  sipa:
    ACK 55f8bc9
  apoelstra:
    ACK 55f8bc9

Tree-SHA512: 5ccba56041f94fa8f40a8a56ce505369ff2e0ed20cd7f0bfc3fdfffa5fa7bf826a93602b9b2455a352865a9548ab4928e858c19bb5af7ec221594a3bf25c4f3d
@real-or-random real-or-random added user-documentation user-facing documentation build labels Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build user-documentation user-facing documentation
Projects
None yet
Milestone
stable release (1.0.0-rc.1)
Development

No branches or pull requests
2 participants
@real-or-random @jonasnick