Skip to content

Commit 0eb3000

Browse files
Merge #1186: tests: Tidy context tests
39e8f0e refactor: Separate run_context_tests into static vs proper contexts (Tim Ruffing) a4a0937 tests: Clean up and improve run_context_tests() further (Tim Ruffing) fc90bb5 refactor: Tidy up main() (Tim Ruffing) f32a36f tests: Don't use global context for context tests (Tim Ruffing) ce4f936 tests: Tidy run_context_tests() by extracting functions (Tim Ruffing) 18e0db3 tests: Don't recreate global context in scratch space test (Tim Ruffing) b198061 tests: Use global copy of secp256k1_context_static instead of clone (Tim Ruffing) Pull request description: This is an improved version of some of the tidying/refactoring in #1170. I think it's enough to deserve a separate PR. Once this is merged, I'll get back to the actual goal of #1170 (namely, forbidding cloning and randomizing static contexts.) This PR is a general clean up of the context tests. A notable change is that this avoids a code smell where `run_context_tests()` would use the global `ctx` variable like a local one (i.e., create a context in it and destroy it afterwards). After this PR, the global `ctx` is properly initialized for all the other tests, and they can decide whether they want to use it or not. Same for a global `sttc`, which is a memcpy of the static context (we need a writable copy in order to be able to set callbacks). Note that this touches code which is also affected by #1167 but I refrained from trying to solve this issue. The goal of this PR is simply not to worsen the situation w.r.t. #1167. We should really introduce a macro to solve #1167 but that's another PR. ACKs for top commit: sipa: utACK 39e8f0e apoelstra: ACK 39e8f0e Tree-SHA512: a22471758111061a062b126a52a0de24a1a311d1a0332a4ef006882379a4f3f2b00e53089e3c374bf47c4051bb10bbc6a9fdbcf6d0cd4eca15b5703590395fba
2 parents 2a39ac1 + 39e8f0e commit 0eb3000

File tree

4 files changed

+147
-115
lines changed

4 files changed

+147
-115
lines changed

src/modules/extrakeys/tests_impl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,6 @@ void test_keypair(void) {
336336
secp256k1_xonly_pubkey xonly_pk, xonly_pk_tmp;
337337
int pk_parity, pk_parity_tmp;
338338
int ecount;
339-
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
340339

341340
set_counting_callbacks(ctx, &ecount);
342341
set_counting_callbacks(sttc, &ecount);
@@ -440,7 +439,9 @@ void test_keypair(void) {
440439
memset(&keypair, 0, sizeof(keypair));
441440
CHECK(secp256k1_keypair_sec(ctx, sk_tmp, &keypair) == 1);
442441
CHECK(secp256k1_memcmp_var(zeros96, sk_tmp, sizeof(sk_tmp)) == 0);
443-
secp256k1_context_destroy(sttc);
442+
443+
secp256k1_context_set_error_callback(sttc, NULL, NULL);
444+
secp256k1_context_set_illegal_callback(sttc, NULL, NULL);
444445
}
445446

446447
void test_keypair_add(void) {

src/modules/recovery/tests_impl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ static int recovery_test_nonce_function(unsigned char *nonce32, const unsigned c
3030

3131
void test_ecdsa_recovery_api(void) {
3232
/* Setup contexts that just count errors */
33-
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
3433
secp256k1_pubkey pubkey;
3534
secp256k1_pubkey recpubkey;
3635
secp256k1_ecdsa_signature normal_sig;
@@ -124,7 +123,8 @@ void test_ecdsa_recovery_api(void) {
124123
CHECK(ecount == 7);
125124

126125
/* cleanup */
127-
secp256k1_context_destroy(sttc);
126+
secp256k1_context_set_error_callback(sttc, NULL, NULL);
127+
secp256k1_context_set_illegal_callback(sttc, NULL, NULL);
128128
}
129129

130130
void test_ecdsa_recovery_end_to_end(void) {

src/modules/schnorrsig/tests_impl.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,7 @@ void test_schnorrsig_api(void) {
128128
secp256k1_schnorrsig_extraparams invalid_extraparams = {{ 0 }, NULL, NULL};
129129

130130
/** setup **/
131-
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
132-
int ecount;
131+
int ecount = 0;
133132

134133
secp256k1_context_set_error_callback(ctx, counting_illegal_callback_fn, &ecount);
135134
secp256k1_context_set_illegal_callback(ctx, counting_illegal_callback_fn, &ecount);
@@ -198,7 +197,8 @@ void test_schnorrsig_api(void) {
198197
CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &zero_pk) == 0);
199198
CHECK(ecount == 4);
200199

201-
secp256k1_context_destroy(sttc);
200+
secp256k1_context_set_error_callback(sttc, NULL, NULL);
201+
secp256k1_context_set_illegal_callback(sttc, NULL, NULL);
202202
}
203203

204204
/* Checks that hash initialized by secp256k1_schnorrsig_sha256_tagged has the

0 commit comments

Comments
 (0)