clippy: enable a bunch of casting and safety lints

Also enable a grab bag of other lints; I basically copied the giant list
from rust-elements and removed the ones that seemed too annoying to fix
and not very important.

Author: apoelstra

Cargo.toml

@@ -68,3 +68,47 @@ members = ["bitcoind-tests", "fuzz"]
 
 [lints.rust]
 unexpected_cfgs = { level = "deny", check-cfg = ['cfg(miniscript_bench)'] }
+
+[lints.clippy]
+# Exclude lints we don't think are valuable.
+needless_question_mark = "allow" # https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
+manual_range_contains = "allow" # More readable than clippy's format.
+uninlined_format_args = "allow" # This is a subjective style choice.
+float_cmp = "allow" # Bitcoin floats are typically limited to 8 decimal places and we want them exact.
+match_bool = "allow" # Adds extra indentation and LOC.
+match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other.
+must_use_candidate = "allow" # Useful for audit but many false positives.
+similar_names = "allow" # Too many (subjectively) false positives.
+# Exhaustive list of pedantic clippy lints
+assigning_clones = "warn"
+borrow_as_ptr = "warn"
+cast_lossless = "warn"
+cast_possible_truncation = "allow" # All casts should include a code comment (except test code).
+cast_possible_wrap = "allow" # Same as above re code comment.
+cast_precision_loss = "warn"
+cast_ptr_alignment = "warn"
+cast_sign_loss = "allow" # All casts should include a code comment (except in test code).
+invalid_upcast_comparisons = "warn"
+option_as_ref_cloned = "warn"
+ptr_as_ptr = "warn"
+ptr_cast_constness = "warn"
+ref_as_ptr = "warn"
+ref_binding_to_reference = "warn"
+ref_option_ref = "warn"
+same_functions_in_if_condition = "warn"
+should_panic_without_expect = "warn"
+single_char_pattern = "warn"
+stable_sort_primitive = "warn"
+str_split_at_newline = "warn"
+string_add_assign = "warn"
+transmute_ptr_to_ptr = "warn"
+trivially_copy_pass_by_ref = "warn"
+unchecked_duration_subtraction = "warn"
+unicode_not_nfc = "warn"
+unnecessary_box_returns = "warn"
+unnecessary_join = "warn"
+unnecessary_literal_bound = "warn"
+unnecessary_wraps = "warn"
+unsafe_derive_deserialize = "warn"
+unused_async = "warn"
+unused_self = "warn"

src/descriptor/mod.rs

@@ -969,7 +969,7 @@ impl<Ext: Extension + ParseableExt> Descriptor<DescriptorPublicKey, Ext> {
 
         impl<'a> Translator<DescriptorPublicKey, String, ()> for KeyMapLookUp<'a> {
             fn pk(&mut self, pk: &DescriptorPublicKey) -> Result<String, ()> {
-                key_to_string(pk, self.0)
+                Ok(key_to_string(pk, self.0))
             }
 
             fn sha256(&mut self, sha256: &sha256::Hash) -> Result<String, ()> {
@@ -989,11 +989,11 @@ impl<Ext: Extension + ParseableExt> Descriptor<DescriptorPublicKey, Ext> {
             }
         }
 
-        fn key_to_string(pk: &DescriptorPublicKey, key_map: &KeyMap) -> Result<String, ()> {
-            Ok(match key_map.get(pk) {
+        fn key_to_string(pk: &DescriptorPublicKey, key_map: &KeyMap) -> String {
+            match key_map.get(pk) {
                 Some(secret) => secret.to_string(),
                 None => pk.to_string(),
-            })
+            }
         }
 
         let descriptor = self

src/extensions/introspect_ops.rs

@@ -623,7 +623,7 @@ fn spk(pref: i8, prog: &[u8]) -> Option<elements::Script> {
     } else if pref <= 16 && pref >= 0 {
         Some(
             script::Builder::new()
-                .push_int(pref as i64)
+                .push_int(i64::from(pref))
                 .push_slice(prog)
                 .into_script(),
         )
@@ -664,7 +664,7 @@ impl AssetExpr<CovExtArgs> {
                     Asset::Explicit(a) => builder.push_slice(a.into_inner().as_ref()).push_int(1), // explicit prefix
                     Asset::Confidential(c) => {
                         let ser = c.serialize();
-                        builder.push_slice(&ser[1..]).push_int(ser[0] as i64)
+                        builder.push_slice(&ser[1..]).push_int(i64::from(ser[0]))
                     }
                 }
             }
@@ -752,7 +752,7 @@ impl ValueExpr<CovExtArgs> {
                     } // explicit prefix
                     confidential::Value::Confidential(c) => {
                         let ser = c.serialize();
-                        builder.push_slice(&ser[1..]).push_int(ser[0] as i64)
+                        builder.push_slice(&ser[1..]).push_int(i64::from(ser[0]))
                     }
                 }
             }
@@ -838,7 +838,7 @@ impl SpkExpr<CovExtArgs> {
                     SpkInner::Script(s) => spk_to_components(s),
                     SpkInner::Hashed(h) => (-1, h.to_byte_array().to_vec()),
                 };
-                builder.push_slice(&prog).push_int(ver as i64)
+                builder.push_slice(&prog).push_int(i64::from(ver))
             }
             SpkExpr::Const(_) => unreachable!(
                 "Both constructors from_str and from_token_iter

src/interpreter/mod.rs

@@ -701,20 +701,12 @@ where
                 Terminal::After(ref n) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);
                     debug_assert_eq!(node_state.n_satisfied, 0);
-                    let res = self
-                        .stack
-                        .evaluate_after(&LockTime::from(*n), self.lock_time);
-                    if res.is_some() {
-                        return res;
-                    }
+                    return Some(self.stack.evaluate_after(LockTime::from(*n), self.lock_time));
                 }
                 Terminal::Older(ref n) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);
                     debug_assert_eq!(node_state.n_satisfied, 0);
-                    let res = self.stack.evaluate_older(n, self.age);
-                    if res.is_some() {
-                        return res;
-                    }
+                    return Some(self.stack.evaluate_older(*n, self.age));
                 }
                 Terminal::Sha256(ref hash) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);

src/interpreter/stack.rs

@@ -245,27 +245,27 @@ impl<'txin> Stack<'txin> {
     /// booleans
     pub(super) fn evaluate_after<Ext: Extension>(
         &mut self,
-        n: &LockTime,
+        n: LockTime,
         lock_time: LockTime,
-    ) -> Option<Result<SatisfiedConstraint<Ext>, Error>> {
+    ) -> Result<SatisfiedConstraint<Ext>, Error> {
         use LockTime::*;
 
-        let is_satisfied = match (*n, lock_time) {
+        let is_satisfied = match (n, lock_time) {
             (Blocks(n), Blocks(lock_time)) => n <= lock_time,
             (Seconds(n), Seconds(lock_time)) => n <= lock_time,
             _ => {
-                return Some(Err(Error::AbsoluteLocktimeComparisonInvalid(
+                return Err(Error::AbsoluteLocktimeComparisonInvalid(
                     n.to_consensus_u32(),
                     lock_time.to_consensus_u32(),
-                )))
+                ))
             }
         };
 
         if is_satisfied {
             self.push(Element::Satisfied);
-            Some(Ok(SatisfiedConstraint::AbsoluteTimelock { n: *n }))
+            Ok(SatisfiedConstraint::AbsoluteTimelock { n })
         } else {
-            Some(Err(Error::AbsoluteLocktimeNotMet(n.to_consensus_u32())))
+            Err(Error::AbsoluteLocktimeNotMet(n.to_consensus_u32()))
         }
     }
 
@@ -277,14 +277,14 @@ impl<'txin> Stack<'txin> {
     /// booleans
     pub(super) fn evaluate_older<Ext: Extension>(
         &mut self,
-        n: &Sequence,
+        n: Sequence,
         age: Sequence,
-    ) -> Option<Result<SatisfiedConstraint<Ext>, Error>> {
-        if age >= *n {
+    ) -> Result<SatisfiedConstraint<Ext>, Error> {
+        if age >= n {
             self.push(Element::Satisfied);
-            Some(Ok(SatisfiedConstraint::RelativeTimelock { n: *n }))
+            Ok(SatisfiedConstraint::RelativeTimelock { n })
         } else {
-            Some(Err(Error::RelativeLocktimeNotMet(n.to_consensus_u32())))
+            Err(Error::RelativeLocktimeNotMet(n.to_consensus_u32()))
         }
     }
 

src/lib.rs

@@ -534,7 +534,7 @@ impl fmt::Display for Error {
             Error::BareDescriptorAddr => write!(f, "Bare descriptors don't have address"),
             Error::BtcError(ref e) => write!(f, " Bitcoin Miniscript Error {}", e),
             Error::CovError(ref e) => write!(f, "Covenant Error: {}", e),
-            Error::PubKeyCtxError(ref pk, ref ctx) => {
+            Error::PubKeyCtxError(ref pk, ctx) => {
                 write!(f, "Pubkey error: {} under {} scriptcontext", pk, ctx)
             }
             Error::MultiATooManyKeys(k) => write!(f, "MultiA too many keys {}", k),

src/miniscript/astelem.rs

@@ -740,7 +740,7 @@ pub trait StackCtxOperations: Sized {
 impl StackCtxOperations for script::Builder {
     fn check_item_eq(self, idx: u32, target: &[u8]) -> Self {
         self.push_opcode(opcodes::all::OP_DEPTH)
-            .push_int(idx as i64)
+            .push_int(i64::from(idx))
             .push_opcode(opcodes::all::OP_SUB)
             .push_opcode(opcodes::all::OP_PICK)
             .push_slice(target)
@@ -764,7 +764,7 @@ impl StackCtxOperations for script::Builder {
 
         builder
             .push_opcode(opcodes::all::OP_DEPTH)
-            .push_int(idx as i64)
+            .push_int(i64::from(idx))
             .push_opcode(opcodes::all::OP_SUB)
             .push_opcode(opcodes::all::OP_PICK)
             .push_opcode(opcodes::all::OP_EQUAL)

src/miniscript/context.rs

@@ -111,7 +111,7 @@ impl fmt::Display for ScriptContextError {
                     pk
                 )
             }
-            ScriptContextError::XOnlyKeysNotAllowed(ref pk, ref ctx) => {
+            ScriptContextError::XOnlyKeysNotAllowed(ref pk, ctx) => {
                 write!(f, "x-only key {} not allowed in {}", pk, ctx)
             }
             ScriptContextError::UncompressedKeysNotAllowed => {

src/miniscript/lex.rs

@@ -180,20 +180,19 @@ impl<'s> Iterator for TokenIter<'s> {
 pub fn lex(script: &script::Script) -> Result<Vec<Token<'_>>, Error> {
     let mut ret = Vec::with_capacity(script.len());
 
-    fn process_candidate_push(ret: &mut [Token<'_>]) -> Result<(), Error> {
+    fn process_candidate_push(ret: &mut [Token<'_>]) {
         let ret_len = ret.len();
 
         if ret_len < 2 || ret[ret_len - 1] != Token::Swap {
-            return Ok(());
+            return;
         }
         let token = match &ret[ret_len - 2] {
             Token::Hash20(x) => Token::Push(x.to_vec()),
             Token::Bytes32(x) | Token::Bytes33(x) | Token::Bytes65(x) => Token::Push(x.to_vec()),
-            Token::Num(k) => Token::Push(build_scriptint(*k as i64)),
-            _x => return Ok(()), // no change required
+            Token::Num(k) => Token::Push(build_scriptint(i64::from(*k))),
+            _x => return, // no change required
         };
         ret[ret_len - 2] = token;
-        Ok(())
     }
 
     for ins in script.instructions_minimal() {
@@ -329,7 +328,7 @@ pub fn lex(script: &script::Script) -> Result<Vec<Token<'_>>, Error> {
                 ret.push(Token::Dup2);
             }
             script::Instruction::Op(opcodes::all::OP_CAT) => {
-                process_candidate_push(&mut ret)?;
+                process_candidate_push(&mut ret);
                 ret.push(Token::Cat);
             }
             script::Instruction::Op(opcodes::all::OP_CODESEPARATOR) => {

src/miniscript/satisfy.rs

@@ -41,7 +41,7 @@ pub fn elementssig_to_rawsig(sig: &ElementsSig) -> Vec<u8> {
 /// Returns underlying secp if the Signature is not of correct format
 pub fn elementssig_from_rawsig(rawsig: &[u8]) -> Result<ElementsSig, crate::interpreter::Error> {
     let (flag, sig) = rawsig.split_last().unwrap();
-    let flag = elements::EcdsaSighashType::from_u32(*flag as u32);
+    let flag = elements::EcdsaSighashType::from_u32(u32::from(*flag));
     let sig = secp256k1_zkp::ecdsa::Signature::from_der(sig)?;
     Ok((sig, flag))
 }