tests: Use global copy of secp256k1_context_static instead of clone

sipa · 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");