secp256k1 icon indicating copy to clipboard operation
secp256k1 copied to clipboard

Further changes after making tables static

Open real-or-random opened this issue 2 years ago • 2 comments

More things to improve after #988:

  • [x] Compile precomputation as a separate object file and link it (solved by #1042)
  • [x] Speed up secp256k1_ecmult_gen_context_build at context creation. It currently computes fixed values which could be made static (open PR: https://github.com/bitcoin-core/secp256k1/pull/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")
  • [x] Document the backwards-compatible API changes made in #988 and in #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.
  • [x] 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 avatar Jan 17 '22 18:01 real-or-random

* [ ]  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 avatar Feb 02 '22 12:02 real-or-random

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.

jonasnick avatar Feb 22 '22 15:02 jonasnick