Skip to content

Commit

Permalink
delete test_manual_create_destroy test
Browse files Browse the repository at this point in the history
This is just a bad test. It constructs a preallocated context object by
starting from a non-preallocated context object, in a way that can't be
done by users (since it directly constructs a `Secp256k1` struct) and a
way that is very difficult to unwind, because you wind up with two
pointers to the same underlying context object, one a "preallocated" one
and one a normal one.

If you then drop the preallocated one, it will call
`secp256k1_context_destroy`, forcing you to manually deallocate the
other one. If you drop the normally-allocated one, you need to
mem::forget the preallocated one to avoid calling
`secp256k1_context_destroy` twice. The whole thing is pretty fragile.

There is another unit test, `test_raw_ctx`, which gets into the same
situation but using the public API, and demonstrates a few ways to get
out of it.
  • Loading branch information
apoelstra authored and Davidson-Souza committed Sep 30, 2023
1 parent 04ce508 commit acf9ac1
Showing 1 changed file with 0 additions and 34 deletions.
34 changes: 0 additions & 34 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,40 +555,6 @@ mod tests {
}};
}

#[test]
#[cfg(feature = "rand-std")]
fn test_manual_create_destroy() {
use std::marker::PhantomData;

let ctx_full = unsafe { ffi::secp256k1_context_create(AllPreallocated::FLAGS) };
let ctx_sign = unsafe { ffi::secp256k1_context_create(SignOnlyPreallocated::FLAGS) };
let ctx_vrfy = unsafe { ffi::secp256k1_context_create(VerifyOnlyPreallocated::FLAGS) };

let full: Secp256k1<AllPreallocated> = Secp256k1 { ctx: ctx_full, phantom: PhantomData };
let sign: Secp256k1<SignOnlyPreallocated> =
Secp256k1 { ctx: ctx_sign, phantom: PhantomData };
let vrfy: Secp256k1<VerifyOnlyPreallocated> =
Secp256k1 { ctx: ctx_vrfy, phantom: PhantomData };

let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());
let msg = Message::from_digest_slice(&[2u8; 32]).unwrap();
// Try signing
assert_eq!(sign.sign_ecdsa(&msg, &sk), full.sign_ecdsa(&msg, &sk));
let sig = full.sign_ecdsa(&msg, &sk);

// Try verifying
assert!(vrfy.verify_ecdsa(&msg, &sig, &pk).is_ok());
assert!(full.verify_ecdsa(&msg, &sig, &pk).is_ok());

drop(full);
drop(sign);
drop(vrfy);

unsafe { ffi::secp256k1_context_destroy(ctx_vrfy) };
unsafe { ffi::secp256k1_context_destroy(ctx_sign) };
unsafe { ffi::secp256k1_context_destroy(ctx_full) };
}

#[test]
#[cfg(feature = "rand-std")]
// In rustc 1.72 this Clippy lint was pulled out of clippy and into rustc, and
Expand Down

0 comments on commit acf9ac1

Please sign in to comment.