tests: Use global copy of secp256k1_context_static instead of clone

bitcoin-core · Jan 4, 2023 · b198061 · b198061
1 parent 2a39ac1
commit b198061
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 16 deletions.
diff --git a/src/modules/extrakeys/tests_impl.h b/src/modules/extrakeys/tests_impl.h
@@ -336,7 +336,6 @@ void test_keypair(void) {
     secp256k1_xonly_pubkey xonly_pk, xonly_pk_tmp;
     int pk_parity, pk_parity_tmp;
     int ecount;
-    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
 
     set_counting_callbacks(ctx, &ecount);
     set_counting_callbacks(sttc, &ecount);
@@ -440,7 +439,9 @@ void test_keypair(void) {
     memset(&keypair, 0, sizeof(keypair));
     CHECK(secp256k1_keypair_sec(ctx, sk_tmp, &keypair) == 1);
     CHECK(secp256k1_memcmp_var(zeros96, sk_tmp, sizeof(sk_tmp)) == 0);
-    secp256k1_context_destroy(sttc);
+
+    secp256k1_context_set_error_callback(sttc, NULL, NULL);
+    secp256k1_context_set_illegal_callback(sttc, NULL, NULL);
 }
 
 void test_keypair_add(void) {

diff --git a/src/modules/recovery/tests_impl.h b/src/modules/recovery/tests_impl.h
@@ -30,7 +30,6 @@ static int recovery_test_nonce_function(unsigned char *nonce32, const unsigned c
 
 void test_ecdsa_recovery_api(void) {
     /* Setup contexts that just count errors */
-    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
     secp256k1_pubkey pubkey;
     secp256k1_pubkey recpubkey;
     secp256k1_ecdsa_signature normal_sig;
@@ -124,7 +123,8 @@ void test_ecdsa_recovery_api(void) {
     CHECK(ecount == 7);
 
     /* cleanup */
-    secp256k1_context_destroy(sttc);
+    secp256k1_context_set_error_callback(sttc, NULL, NULL);
+    secp256k1_context_set_illegal_callback(sttc, NULL, NULL);
 }
 
 void test_ecdsa_recovery_end_to_end(void) {

diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h
@@ -128,8 +128,7 @@ void test_schnorrsig_api(void) {
     secp256k1_schnorrsig_extraparams invalid_extraparams = {{ 0 }, NULL, NULL};
 
     /** setup **/
-    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
-    int ecount;
+    int ecount = 0;
 
     secp256k1_context_set_error_callback(ctx, counting_illegal_callback_fn, &ecount);
     secp256k1_context_set_illegal_callback(ctx, counting_illegal_callback_fn, &ecount);
@@ -198,7 +197,8 @@ void test_schnorrsig_api(void) {
     CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &zero_pk) == 0);
     CHECK(ecount == 4);
 
-    secp256k1_context_destroy(sttc);
+    secp256k1_context_set_error_callback(sttc, NULL, NULL);
+    secp256k1_context_set_illegal_callback(sttc, NULL, NULL);
 }
 
 /* Checks that hash initialized by secp256k1_schnorrsig_sha256_tagged has the

diff --git a/src/tests.c b/src/tests.c
@@ -29,6 +29,7 @@
 
 static int count = 64;
 static secp256k1_context *ctx = NULL;
+static secp256k1_context *sttc = NULL;
 
 static void counting_illegal_callback_fn(const char* str, void* data) {
     /* Dummy callback function that just counts. */
@@ -180,9 +181,7 @@ void run_context_tests(int use_prealloc) {
     unsigned char ctmp[32];
     int32_t ecount;
     int32_t ecount2;
-    secp256k1_context *sttc;
     void *ctx_prealloc = NULL;
-    void *sttc_prealloc = NULL;
 
     secp256k1_gej pubj;
     secp256k1_ge pub;
@@ -196,11 +195,7 @@ void run_context_tests(int use_prealloc) {
         ctx_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE));
         CHECK(ctx_prealloc != NULL);
         ctx = secp256k1_context_preallocated_create(ctx_prealloc, SECP256K1_CONTEXT_NONE);
-        sttc_prealloc = malloc(secp256k1_context_preallocated_clone_size(secp256k1_context_static));
-        CHECK(sttc_prealloc != NULL);
-        sttc = secp256k1_context_preallocated_clone(secp256k1_context_static, sttc_prealloc);
     } else {
-        sttc = secp256k1_context_clone(secp256k1_context_static);
         ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
     }
 
@@ -312,12 +307,9 @@ void run_context_tests(int use_prealloc) {
     /* cleanup */
     if (use_prealloc) {
         secp256k1_context_preallocated_destroy(ctx);
-        secp256k1_context_preallocated_destroy(sttc);
         free(ctx_prealloc);
-        free(sttc_prealloc);
     } else {
         secp256k1_context_destroy(ctx);
-        secp256k1_context_destroy(sttc);
     }
     /* Defined as no-op. */
     secp256k1_context_destroy(NULL);
@@ -7357,6 +7349,15 @@ int main(int argc, char **argv) {
     secp256k1_testrand_init(argc > 2 ? argv[2] : NULL);
 
     /* initialize */
+    /* Make a writable copy of secp256k1_context_static in order to test the effect of API functions
+       that write to the context. The API does not support cloning the static context, so we use
+       memcpy instead. The user is not supposed to copy a context but we should still ensure that
+       the API functions handle copies of the static context gracefully. */
+    sttc = malloc(sizeof(*secp256k1_context_static));
+    CHECK(sttc != NULL);
+    memcpy(sttc, secp256k1_context_static, sizeof(secp256k1_context));
+    CHECK(!secp256k1_context_is_proper(sttc));
+
     run_selftest_tests();
     run_context_tests(0);
     run_context_tests(1);
@@ -7463,6 +7464,7 @@ int main(int argc, char **argv) {
     secp256k1_testrand_finish();
 
     /* shutdown */
+    free(sttc);
     secp256k1_context_destroy(ctx);
 
     printf("no problems found\n");