From 4e87e6fe94855d19a68b5c7640476e0d6bc0625f Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 13:35:17 +1100 Subject: [PATCH 1/7] Implement is_empty method Clippy warns of missing `is_empty`, trivially implement it by calling through to `self.data.is_empty()`. --- src/ecdh.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ecdh.rs b/src/ecdh.rs index 2174399ff..8242330fb 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -61,6 +61,11 @@ impl SharedSecret { self.len } + /// True if the underlying data buffer is empty. + pub fn is_empty(&self) -> bool { + self.data.is_empty() + } + /// Set the length of the object. pub(crate) fn set_len(&mut self, len: usize) { debug_assert!(len <= self.data.len()); From ed29f122161594acbde95aab2cd54d78ee7f07b0 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 13:38:02 +1100 Subject: [PATCH 2/7] Remove unnecessary return statements Found by clippy. We don't need a `return` for the final statement. --- src/schnorrsig.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/schnorrsig.rs b/src/schnorrsig.rs index 858a1d550..3a9ca764c 100644 --- a/src/schnorrsig.rs +++ b/src/schnorrsig.rs @@ -185,11 +185,11 @@ impl KeyPair { tweak.as_c_ptr(), ); - return if err == 1 { + if err == 1 { Ok(()) } else { Err(Error::InvalidTweak) - }; + } } } } @@ -465,11 +465,11 @@ impl Secp256k1 { pubkey.as_c_ptr(), ); - return if ret == 1 { + if ret == 1 { Ok(()) } else { Err(Error::InvalidSignature) - }; + } } } From 34ad4110f1c460e3d90382f3139dc788102c05ef Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 13:41:03 +1100 Subject: [PATCH 3/7] Remove unused error return value This helper never returns an error, remove the `Result` return type. Found by clippy. --- src/schnorrsig.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/schnorrsig.rs b/src/schnorrsig.rs index 3a9ca764c..b75bae456 100644 --- a/src/schnorrsig.rs +++ b/src/schnorrsig.rs @@ -384,7 +384,7 @@ impl Secp256k1 { msg: &Message, keypair: &KeyPair, nonce_data: *const ffi::types::c_void, - ) -> Result { + ) -> Signature { unsafe { let mut sig = [0u8; constants::SCHNORRSIG_SIGNATURE_SIZE]; assert_eq!( @@ -399,7 +399,7 @@ impl Secp256k1 { ) ); - Ok(Signature(sig)) + Signature(sig) } } @@ -407,7 +407,7 @@ impl Secp256k1 { /// generator to generate the auxiliary random data. /// Requires compilation with "rand-std" feature. #[cfg(any(test, feature = "rand-std"))] - pub fn schnorrsig_sign(&self, msg: &Message, keypair: &KeyPair) -> Result { + pub fn schnorrsig_sign(&self, msg: &Message, keypair: &KeyPair) -> Signature { let mut rng = thread_rng(); self.schnorrsig_sign_with_rng(msg, keypair, &mut rng) } @@ -417,7 +417,7 @@ impl Secp256k1 { &self, msg: &Message, keypair: &KeyPair, - ) -> Result { + ) -> Signature { self.schnorrsig_sign_helper(msg, keypair, ptr::null()) } @@ -427,7 +427,7 @@ impl Secp256k1 { msg: &Message, keypair: &KeyPair, aux_rand: &[u8; 32], - ) -> Result { + ) -> Signature { self.schnorrsig_sign_helper( msg, keypair, @@ -444,7 +444,7 @@ impl Secp256k1 { msg: &Message, keypair: &KeyPair, rng: &mut R, - ) -> Result { + ) -> Signature { let mut aux = [0u8; 32]; rng.fill_bytes(&mut aux); self.schnorrsig_sign_helper(msg, keypair, aux.as_c_ptr() as *const ffi::types::c_void) @@ -533,7 +533,6 @@ mod tests { let mut aux_rand = [0; 32]; rng.fill_bytes(&mut aux_rand); secp.schnorrsig_sign_with_aux_rand(msg, seckey, &aux_rand) - .unwrap() }) } @@ -541,21 +540,20 @@ mod tests { fn test_schnorrsig_sign_with_rng_verify() { test_schnorrsig_sign_helper(|secp, msg, seckey, mut rng| { secp.schnorrsig_sign_with_rng(msg, seckey, &mut rng) - .unwrap() }) } #[test] fn test_schnorrsig_sign_verify() { test_schnorrsig_sign_helper(|secp, msg, seckey, _| { - secp.schnorrsig_sign(msg, seckey).unwrap() + secp.schnorrsig_sign(msg, seckey) }) } #[test] fn test_schnorrsig_sign_no_aux_rand_verify() { test_schnorrsig_sign_helper(|secp, msg, seckey, _| { - secp.schnorrsig_sign_no_aux_rand(msg, seckey).unwrap() + secp.schnorrsig_sign_no_aux_rand(msg, seckey) }) } @@ -575,8 +573,7 @@ mod tests { let expected_sig = Signature::from_str("6470FD1303DDA4FDA717B9837153C24A6EAB377183FC438F939E0ED2B620E9EE5077C4A8B8DCA28963D772A94F5F0DDF598E1C47C137F91933274C7C3EDADCE8").unwrap(); let sig = secp - .schnorrsig_sign_with_aux_rand(&msg, &sk, &aux_rand) - .unwrap(); + .schnorrsig_sign_with_aux_rand(&msg, &sk, &aux_rand); assert_eq!(expected_sig, sig); } @@ -730,8 +727,7 @@ mod tests { let keypair = KeyPair::from_seckey_slice(&s, &[2; 32]).unwrap(); let aux = [3; 32]; let sig = s - .schnorrsig_sign_with_aux_rand(&msg, &keypair, &aux) - .unwrap(); + .schnorrsig_sign_with_aux_rand(&msg, &keypair, &aux); static SIG_BYTES: [u8; constants::SCHNORRSIG_SIGNATURE_SIZE] = [ 0x14, 0xd0, 0xbf, 0x1a, 0x89, 0x53, 0x50, 0x6f, 0xb4, 0x60, 0xf5, 0x8b, 0xe1, 0x41, 0xaf, 0x76, 0x7f, 0xd1, 0x12, 0x53, 0x5f, 0xb3, 0x92, 0x2e, 0xf2, 0x17, 0x30, 0x8e, From ef23cb816754d1e147218c2d9dfe3a7c0e67663d Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 13:45:19 +1100 Subject: [PATCH 4/7] Return Ok directly Clippy emits warning: warning: passing a unit value to a function Just return `Ok(())` after calling `fill_bytes`. --- src/key.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/key.rs b/src/key.rs index bbed29949..460f27c3c 100644 --- a/src/key.rs +++ b/src/key.rs @@ -512,7 +512,8 @@ mod test { self.0 -= 1; } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { - Ok(self.fill_bytes(dest)) + self.fill_bytes(dest); + Ok(()) } } From c92b9464937c552b357bf7790bf094c3324d0ca4 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 14:10:46 +1100 Subject: [PATCH 5/7] Remove unnecessary clone Type is `Copy`, no need for clone. --- src/key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/key.rs b/src/key.rs index 460f27c3c..abcd9a7fc 100644 --- a/src/key.rs +++ b/src/key.rs @@ -796,7 +796,7 @@ mod test { let pk1 = PublicKey::from_slice( &hex!("0241cc121c419921942add6db6482fb36243faf83317c866d2a28d8c6d7089f7ba"), ).unwrap(); - let pk2 = pk1.clone(); + let pk2 = pk1; let pk3 = PublicKey::from_slice( &hex!("02e6642fd69bd211f93f7f1f36ca51a26a5290eb2dd1b0d8279a87bb0d480c8443"), ).unwrap(); From c38136b6bcd20f9ee15f6305a6a10b87fb4dafad Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 14:02:31 +1100 Subject: [PATCH 6/7] Use for loop instead of map Currently we are misusing `map` on an iterator to loop `n` times, additionally the assertion is pointless. Use a for loop and assert against the length of the set. --- src/key.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/key.rs b/src/key.rs index abcd9a7fc..696cadcd7 100644 --- a/src/key.rs +++ b/src/key.rs @@ -762,13 +762,13 @@ mod test { let s = Secp256k1::new(); let mut set = HashSet::new(); const COUNT : usize = 1024; - let count = (0..COUNT).map(|_| { + for _ in 0..COUNT { let (_, pk) = s.generate_keypair(&mut thread_rng()); let hash = hash(&pk); assert!(!set.contains(&hash)); set.insert(hash); - }).count(); - assert_eq!(count, COUNT); + }; + assert_eq!(set.len(), COUNT); } #[test] From a584643486d2ee7a465fec61123d26167e71322f Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 22 Dec 2020 14:11:09 +1100 Subject: [PATCH 7/7] Use ManuallyDrop Suggested by clippy, we need to use ManuallyDrop for these types in order to correctly free up the memory. --- src/lib.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index cadd93205..a0d87b0c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -809,13 +809,15 @@ mod tests { #[test] fn test_raw_ctx() { + use std::mem::ManuallyDrop; + let ctx_full = Secp256k1::new(); let ctx_sign = Secp256k1::signing_only(); let ctx_vrfy = Secp256k1::verification_only(); - let full = unsafe {Secp256k1::from_raw_all(ctx_full.ctx)}; - let sign = unsafe {Secp256k1::from_raw_signining_only(ctx_sign.ctx)}; - let vrfy = unsafe {Secp256k1::from_raw_verification_only(ctx_vrfy.ctx)}; + let mut full = unsafe {Secp256k1::from_raw_all(ctx_full.ctx)}; + let mut sign = unsafe {Secp256k1::from_raw_signining_only(ctx_sign.ctx)}; + let mut vrfy = unsafe {Secp256k1::from_raw_verification_only(ctx_vrfy.ctx)}; let (sk, pk) = full.generate_keypair(&mut thread_rng()); let msg = Message::from_slice(&[2u8; 32]).unwrap(); @@ -827,8 +829,15 @@ mod tests { assert!(vrfy.verify(&msg, &sig, &pk).is_ok()); assert!(full.verify(&msg, &sig, &pk).is_ok()); - drop(full);drop(sign);drop(vrfy); - drop(ctx_full);drop(ctx_sign);drop(ctx_vrfy); + unsafe { + ManuallyDrop::drop(&mut full); + ManuallyDrop::drop(&mut sign); + ManuallyDrop::drop(&mut vrfy); + + } + drop(ctx_full); + drop(ctx_sign); + drop(ctx_vrfy); } #[test]