Merge #1126: API cleanup with respect to contexts

jonasnick · jonasnick · commit e3f84777eba5 · 2022-12-06T08:15:03.000Z
4386a23 examples: Switch to NONE contexts (Tim Ruffing) 7289b51 docs: Use doxygen style if and only if comment is user-facing (Tim Ruffing) e7d0185 docs: Get rid of "initialized for signing" terminology (Tim Ruffing) 0612636 docs: Tidy and improve docs about contexts and randomization (Tim Ruffing) e02d686 selftest: Expose in public API (Tim Ruffing) e383fbf selftest: Rename internal function to make name available for API (Tim Ruffing) d2c6d48 tests: Use new name of static context (Tim Ruffing) 53796d2 contexts: Rename static context (Tim Ruffing) 72fedf8 docs: Improve docs for static context (Tim Ruffing) 316ac76 contexts: Deprecate all context flags except SECP256K1_CONTEXT_NONE (Tim Ruffing) 1a553ee docs: Change signature "validation" to "verification" (Tim Ruffing) ee7341f docs: Never require a verification context (Tim Ruffing) Pull request description: ACKs for top commit: sipa: utACK 4386a23 jonasnick: ACK 4386a23 Tree-SHA512: 7bf07dfae0ecbf7de1418de64ef743a23dc5f244aeba2c1cf3ecbdc117d6ac12bb6c8f17f739605566074a9b901765ee4a32288b6edc6f9a0040a70cb472f6ee
diff --git a/contrib/lax_der_privatekey_parsing.h b/contrib/lax_der_privatekey_parsing.h
@@ -43,8 +43,7 @@ extern "C" {
 /** Export a private key in DER format.
  *
  *  Returns: 1 if the private key was valid.
- *  Args: ctx:        pointer to a context object, initialized for signing (cannot
- *                    be NULL)
+ *  Args: ctx:        pointer to a context object (not secp256k1_context_static).
  *  Out: privkey:     pointer to an array for storing the private key in BER.
  *                    Should have space for 279 bytes, and cannot be NULL.
  *       privkeylen:  Pointer to an int where the length of the private key in
diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md
@@ -9,6 +9,13 @@ Each change falls into one of the following categories: Added, Changed, Deprecat
 ### Changed
  - Enable modules schnorrsig, extrakeys and ECDH by default in ./configure
 
+### Deprecated
+ - Deprecated context flags `SECP256K1_CONTEXT_VERIFY` and `SECP256K1_CONTEXT_SIGN`. Use `SECP256K1_CONTEXT_NONE` instead.
+ - Renamed `secp256k1_context_no_precomp` to `secp256k1_context_static`.
+
+### Added
+ - Added `secp256k1_selftest`, to be used in conjunction with `secp256k1_context_static`.
+
 ## [MAJOR.MINOR.PATCH] - YYYY-MM-DD
 
 ### Added/Changed/Deprecated/Removed/Fixed/Security
diff --git a/examples/ecdh.c b/examples/ecdh.c
@@ -30,12 +30,8 @@ int main(void) {
     secp256k1_pubkey pubkey1;
     secp256k1_pubkey pubkey2;
 
-    /* The specification in secp256k1.h states that `secp256k1_ec_pubkey_create`
-     * needs a context object initialized for signing, which is why we create
-     * a context with the SECP256K1_CONTEXT_SIGN flag.
-     * (The docs for `secp256k1_ecdh` don't require any special context, just
-     * some initialized context) */
-    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
+    /* Before we can call actual API functions, we need to create a "context". */
+    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
     if (!fill_random(randomize, sizeof(randomize))) {
         printf("Failed to generate randomness\n");
         return 1;
diff --git a/examples/ecdsa.c b/examples/ecdsa.c
@@ -38,12 +38,8 @@ int main(void) {
     int return_val;
     secp256k1_pubkey pubkey;
     secp256k1_ecdsa_signature sig;
-    /* The specification in secp256k1.h states that `secp256k1_ec_pubkey_create` needs
-     * a context object initialized for signing and `secp256k1_ecdsa_verify` needs
-     * a context initialized for verification, which is why we create a context
-     * for both signing and verification with the SECP256K1_CONTEXT_SIGN and
-     * SECP256K1_CONTEXT_VERIFY flags. */
-    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
+    /* Before we can call actual API functions, we need to create a "context". */
+    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
     if (!fill_random(randomize, sizeof(randomize))) {
         printf("Failed to generate randomness\n");
         return 1;
diff --git a/examples/schnorr.c b/examples/schnorr.c
@@ -30,12 +30,8 @@ int main(void) {
     int return_val;
     secp256k1_xonly_pubkey pubkey;
     secp256k1_keypair keypair;
-    /* The specification in secp256k1_extrakeys.h states that `secp256k1_keypair_create`
-     * needs a context object initialized for signing. And in secp256k1_schnorrsig.h
-     * they state that `secp256k1_schnorrsig_verify` needs a context initialized for
-     * verification, which is why we create a context for both signing and verification
-     * with the SECP256K1_CONTEXT_SIGN and SECP256K1_CONTEXT_VERIFY flags. */
-    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
+    /* Before we can call actual API functions, we need to create a "context". */
+    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
     if (!fill_random(randomize, sizeof(randomize))) {
         printf("Failed to generate randomness\n");
         return 1;
diff --git a/include/secp256k1.h b/include/secp256k1.h
diff --git a/include/secp256k1_extrakeys.h b/include/secp256k1_extrakeys.h
@@ -108,7 +108,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_from_pubke
  *           invalid (only when the tweak is the negation of the corresponding
  *           secret key). 1 otherwise.
  *
- *  Args:           ctx: pointer to a context object initialized for verification.
+ *  Args:           ctx: pointer to a context object.
  *  Out:  output_pubkey: pointer to a public key to store the result. Will be set
  *                       to an invalid value if this function returns 0.
  *  In: internal_pubkey: pointer to an x-only pubkey to apply the tweak to.
@@ -137,7 +137,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add(
  *
  *  Returns: 0 if the arguments are invalid or the tweaked pubkey is not the
  *           result of tweaking the internal_pubkey with tweak32. 1 otherwise.
- *  Args:            ctx: pointer to a context object initialized for verification.
+ *  Args:            ctx: pointer to a context object.
  *  In: tweaked_pubkey32: pointer to a serialized xonly_pubkey.
  *     tweaked_pk_parity: the parity of the tweaked pubkey (whose serialization
  *                        is passed in as tweaked_pubkey32). This must match the
@@ -159,7 +159,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add_
  *
  *  Returns: 1: secret was valid, keypair is ready to use
  *           0: secret was invalid, try again with a different secret
- *  Args:    ctx: pointer to a context object, initialized for signing.
+ *  Args:    ctx: pointer to a context object (not secp256k1_context_static).
  *  Out: keypair: pointer to the created keypair.
  *  In:   seckey: pointer to a 32-byte secret key.
  */
@@ -228,7 +228,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_pub(
  *           invalid (only when the tweak is the negation of the keypair's
  *           secret key). 1 otherwise.
  *
- *  Args:       ctx: pointer to a context object initialized for verification.
+ *  Args:       ctx: pointer to a context object.
  *  In/Out: keypair: pointer to a keypair to apply the tweak to. Will be set to
  *                   an invalid value if this function returns 0.
  *  In:     tweak32: pointer to a 32-byte tweak. If the tweak is invalid according
diff --git a/include/secp256k1_preallocated.h b/include/secp256k1_preallocated.h
@@ -58,6 +58,8 @@ SECP256K1_API size_t secp256k1_context_preallocated_size(
  *                     bytes, as detailed above.
  *           flags:    which parts of the context to initialize.
  *
+ *  See secp256k1_context_create (in secp256k1.h) for further details.
+ *
  *  See also secp256k1_context_randomize (in secp256k1.h)
  *  and secp256k1_context_preallocated_destroy.
  */
diff --git a/include/secp256k1_recovery.h b/include/secp256k1_recovery.h
@@ -72,7 +72,7 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact(
  *
  *  Returns: 1: signature created
  *           0: the nonce generation function failed, or the secret key was invalid.
- *  Args:    ctx:       pointer to a context object, initialized for signing.
+ *  Args:    ctx:       pointer to a context object (not secp256k1_context_static).
  *  Out:     sig:       pointer to an array where the signature will be placed.
  *  In:      msghash32: the 32-byte message hash being signed.
  *           seckey:    pointer to a 32-byte secret key.
@@ -94,7 +94,7 @@ SECP256K1_API int secp256k1_ecdsa_sign_recoverable(
  *
  *  Returns: 1: public key successfully recovered (which guarantees a correct signature).
  *           0: otherwise.
- *  Args:    ctx:       pointer to a context object, initialized for verification.
+ *  Args:    ctx:       pointer to a context object.
  *  Out:     pubkey:    pointer to the recovered public key.
  *  In:      sig:       pointer to initialized signature that supports pubkey recovery.
  *           msghash32: the 32-byte message hash assumed to be signed.
diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h
@@ -106,7 +106,7 @@ typedef struct {
  *  signatures from being valid in multiple contexts by accident.
  *
  *  Returns 1 on success, 0 on failure.
- *  Args:    ctx: pointer to a context object, initialized for signing.
+ *  Args:    ctx: pointer to a context object (not secp256k1_context_static).
  *  Out:   sig64: pointer to a 64-byte array to store the serialized signature.
  *  In:    msg32: the 32-byte message being signed.
  *       keypair: pointer to an initialized keypair.
@@ -161,7 +161,7 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom(
  *
  *  Returns: 1: correct signature
  *           0: incorrect signature
- *  Args:    ctx: a secp256k1 context object, initialized for verification.
+ *  Args:    ctx: a secp256k1 context object.
  *  In:    sig64: pointer to the 64-byte signature to verify.
  *           msg: the message being verified. Can only be NULL if msglen is 0.
  *        msglen: length of the message
diff --git a/src/modules/extrakeys/tests_impl.h b/src/modules/extrakeys/tests_impl.h
@@ -359,7 +359,7 @@ void test_keypair(void) {
     secp256k1_context *none = api_test_context(SECP256K1_CONTEXT_NONE, &ecount);
     secp256k1_context *sign = api_test_context(SECP256K1_CONTEXT_SIGN, &ecount);
     secp256k1_context *verify = api_test_context(SECP256K1_CONTEXT_VERIFY, &ecount);
-    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
+    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
     secp256k1_context_set_error_callback(sttc, counting_illegal_callback_fn, &ecount);
     secp256k1_context_set_illegal_callback(sttc, counting_illegal_callback_fn, &ecount);
 
diff --git a/src/modules/recovery/tests_impl.h b/src/modules/recovery/tests_impl.h
@@ -34,7 +34,7 @@ void test_ecdsa_recovery_api(void) {
     secp256k1_context *sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
     secp256k1_context *vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
     secp256k1_context *both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
-    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
+    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
     secp256k1_pubkey pubkey;
     secp256k1_pubkey recpubkey;
     secp256k1_ecdsa_signature normal_sig;
diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h
@@ -132,7 +132,7 @@ void test_schnorrsig_api(void) {
     secp256k1_context *sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
     secp256k1_context *vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
     secp256k1_context *both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
-    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
+    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
     int ecount;
 
     secp256k1_context_set_error_callback(none, counting_illegal_callback_fn, &ecount);
diff --git a/src/secp256k1.c b/src/secp256k1.c
@@ -64,13 +64,20 @@ struct secp256k1_context_struct {
     int declassify;
 };
 
-static const secp256k1_context secp256k1_context_no_precomp_ = {
+static const secp256k1_context secp256k1_context_static_ = {
     { 0 },
     { secp256k1_default_illegal_callback_fn, 0 },
     { secp256k1_default_error_callback_fn, 0 },
     0
 };
-const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_no_precomp_;
+const secp256k1_context *secp256k1_context_static = &secp256k1_context_static_;
+const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_static_;
+
+void secp256k1_selftest(void) {
+    if (!secp256k1_selftest_passes()) {
+        secp256k1_callback_call(&default_error_callback, "self test failed");
+    }
+}
 
 size_t secp256k1_context_preallocated_size(unsigned int flags) {
     size_t ret = sizeof(secp256k1_context);
@@ -96,9 +103,7 @@ secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigne
     size_t prealloc_size;
     secp256k1_context* ret;
 
-    if (!secp256k1_selftest()) {
-        secp256k1_callback_call(&default_error_callback, "self test failed");
-    }
+    secp256k1_selftest();
 
     prealloc_size = secp256k1_context_preallocated_size(flags);
     if (prealloc_size == 0) {
@@ -150,7 +155,7 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) {
 }
 
 void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) {
-    ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
+    ARG_CHECK_NO_RETURN(ctx != secp256k1_context_static);
     if (ctx != NULL) {
         secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx);
     }
@@ -164,7 +169,7 @@ void secp256k1_context_destroy(secp256k1_context* ctx) {
 }
 
 void secp256k1_context_set_illegal_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) {
-    ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
+    ARG_CHECK_NO_RETURN(ctx != secp256k1_context_static);
     if (fun == NULL) {
         fun = secp256k1_default_illegal_callback_fn;
     }
@@ -173,7 +178,7 @@ void secp256k1_context_set_illegal_callback(secp256k1_context* ctx, void (*fun)(
 }
 
 void secp256k1_context_set_error_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) {
-    ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
+    ARG_CHECK_NO_RETURN(ctx != secp256k1_context_static);
     if (fun == NULL) {
         fun = secp256k1_default_error_callback_fn;
     }
diff --git a/src/selftest.h b/src/selftest.h
@@ -25,7 +25,7 @@ static int secp256k1_selftest_sha256(void) {
     return secp256k1_memcmp_var(out, output32, 32) == 0;
 }
 
-static int secp256k1_selftest(void) {
+static int secp256k1_selftest_passes(void) {
     return secp256k1_selftest_sha256();
 }
 
diff --git a/src/tests.c b/src/tests.c
@@ -141,6 +141,11 @@ void random_scalar_order_b32(unsigned char *b32) {
     secp256k1_scalar_get_b32(b32, &num);
 }
 
+void run_selftest_tests(void) {
+    /* Test public API */
+    secp256k1_selftest();
+}
+
 void run_context_tests(int use_prealloc) {
     secp256k1_pubkey pubkey;
     secp256k1_pubkey zero_pubkey;
@@ -164,12 +169,15 @@ void run_context_tests(int use_prealloc) {
     secp256k1_scalar msg, key, nonce;
     secp256k1_scalar sigr, sigs;
 
+    /* Check that deprecated secp256k1_context_no_precomp is an alias to secp256k1_context_static. */
+    CHECK(secp256k1_context_no_precomp == secp256k1_context_static);
+
     if (use_prealloc) {
         none_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE));
         sign_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN));
         vrfy_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_VERIFY));
         both_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY));
-        sttc_prealloc = malloc(secp256k1_context_preallocated_clone_size(secp256k1_context_no_precomp));
+        sttc_prealloc = malloc(secp256k1_context_preallocated_clone_size(secp256k1_context_static));
         CHECK(none_prealloc != NULL);
         CHECK(sign_prealloc != NULL);
         CHECK(vrfy_prealloc != NULL);
@@ -179,13 +187,13 @@ void run_context_tests(int use_prealloc) {
         sign = secp256k1_context_preallocated_create(sign_prealloc, SECP256K1_CONTEXT_SIGN);
         vrfy = secp256k1_context_preallocated_create(vrfy_prealloc, SECP256K1_CONTEXT_VERIFY);
         both = secp256k1_context_preallocated_create(both_prealloc, SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
-        sttc = secp256k1_context_preallocated_clone(secp256k1_context_no_precomp, sttc_prealloc);
+        sttc = secp256k1_context_preallocated_clone(secp256k1_context_static, sttc_prealloc);
     } else {
         none = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
         sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
         vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
         both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
-        sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
+        sttc = secp256k1_context_clone(secp256k1_context_static);
     }
 
     memset(&zero_pubkey, 0, sizeof(zero_pubkey));
@@ -5799,7 +5807,7 @@ void run_ec_pubkey_parse_test(void) {
     ecount = 0;
     VG_UNDEF(&pubkey, sizeof(pubkey));
     CHECK(secp256k1_ec_pubkey_parse(ctx, &pubkey, pubkeyc, 65) == 1);
-    CHECK(secp256k1_ec_pubkey_parse(secp256k1_context_no_precomp, &pubkey, pubkeyc, 65) == 1);
+    CHECK(secp256k1_ec_pubkey_parse(secp256k1_context_static, &pubkey, pubkeyc, 65) == 1);
     VG_CHECK(&pubkey, sizeof(pubkey));
     CHECK(ecount == 0);
     VG_UNDEF(&ge, sizeof(ge));
@@ -7385,6 +7393,7 @@ int main(int argc, char **argv) {
     secp256k1_testrand_init(argc > 2 ? argv[2] : NULL);
 
     /* initialize */
+    run_selftest_tests();
     run_context_tests(0);
     run_context_tests(1);
     run_scratch_tests();

Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,8 @@ SECP256K1_API size_t secp256k1_context_preallocated_size(`
`58`	`58`	`* bytes, as detailed above.`
`59`	`59`	`* flags: which parts of the context to initialize.`
`60`	`60`	`*`
	`61`	`+ * See secp256k1_context_create (in secp256k1.h) for further details.`
	`62`	`+ *`
`61`	`63`	`* See also secp256k1_context_randomize (in secp256k1.h)`
`62`	`64`	`* and secp256k1_context_preallocated_destroy.`
`63`	`65`	`*/`