Fix or disable recent nightly clippy lints (#2817)

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>

zebra-chain/src/history_tree.rs

@@ -198,12 +198,14 @@ impl NonEmptyHistoryTree {
         let height = block
             .coinbase_height()
             .expect("block must have coinbase height during contextual verification");
-        if height - self.current_height != 1 {
+
-            panic!(
+        assert!(
-                "added block with height {:?} but it must be {:?}+1",
+            height - self.current_height == 1,
-                height, self.current_height
+            "added block with height {:?} but it must be {:?}+1",
-            );
+            height,
-        }
+            self.current_height
+        );
+
         let network_upgrade = NetworkUpgrade::current(self.network, height);
         if network_upgrade != self.network_upgrade {
             // This is the activation block of a network upgrade.

zebra-chain/src/orchard/commitment.rs

@@ -280,7 +280,7 @@ impl ZcashSerialize for ValueCommitment {
 
 impl ZcashDeserialize for ValueCommitment {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        Self::try_from(reader.read_32_bytes()?).map_err(|e| SerializationError::Parse(e))
+        Self::try_from(reader.read_32_bytes()?).map_err(SerializationError::Parse)
     }
 }
 

zebra-chain/src/orchard/keys.rs

@@ -1099,6 +1099,6 @@ impl ZcashSerialize for EphemeralPublicKey {
 
 impl ZcashDeserialize for EphemeralPublicKey {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        Self::try_from(reader.read_32_bytes()?).map_err(|e| SerializationError::Parse(e))
+        Self::try_from(reader.read_32_bytes()?).map_err(SerializationError::Parse)
     }
 }

zebra-chain/src/primitives/zcash_history.rs

@@ -182,12 +182,13 @@ impl<V: Version> Tree<V> {
             .coinbase_height()
             .expect("block must have coinbase height during contextual verification");
         let network_upgrade = NetworkUpgrade::current(self.network, height);
-        if self.network_upgrade != network_upgrade {
+
-            panic!(
+        assert!(
-                "added block from network upgrade {:?} but history tree is restricted to {:?}",
+            self.network_upgrade == network_upgrade,
-                network_upgrade, self.network_upgrade
+            "added block from network upgrade {:?} but history tree is restricted to {:?}",
-            );
+            network_upgrade,
-        }
+            self.network_upgrade
+        );
 
         let node_data = V::block_to_history_node(block, self.network, sapling_root, orchard_root);
         let appended = self.inner.append_leaf(node_data)?;

zebra-chain/src/sapling/commitment.rs

@@ -264,7 +264,7 @@ impl ZcashSerialize for ValueCommitment {
 
 impl ZcashDeserialize for ValueCommitment {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        Self::try_from(reader.read_32_bytes()?).map_err(|e| SerializationError::Parse(e))
+        Self::try_from(reader.read_32_bytes()?).map_err(SerializationError::Parse)
     }
 }
 

zebra-chain/src/sapling/keys.rs

@@ -1018,6 +1018,6 @@ impl ZcashSerialize for EphemeralPublicKey {
 
 impl ZcashDeserialize for EphemeralPublicKey {
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        Self::try_from(reader.read_32_bytes()?).map_err(|e| SerializationError::Parse(e))
+        Self::try_from(reader.read_32_bytes()?).map_err(SerializationError::Parse)
     }
 }

zebra-consensus/src/primitives/groth16/params.rs

@@ -60,13 +60,14 @@ impl SaplingParams {
         io::copy(&mut spend_fs, &mut sink)?;
         io::copy(&mut output_fs, &mut sink)?;
 
-        if spend_fs.into_hash() != SAPLING_SPEND_HASH {
+        assert!(
-            panic!("Sapling spend parameter is not correct.");
+            spend_fs.into_hash() == SAPLING_SPEND_HASH,
-        }
+            "Sapling spend parameter is not correct."
-
+        );
-        if output_fs.into_hash() != SAPLING_OUTPUT_HASH {
+        assert!(
-            panic!("Sapling output parameter is not correct.");
+            output_fs.into_hash() == SAPLING_OUTPUT_HASH,
-        }
+            "Sapling output parameter is not correct."
+        );
 
         // Prepare verifying keys
         let spend_prepared_verifying_key = groth16::prepare_verifying_key(&spend.vk);

zebra-network/src/peer/connection.rs

@@ -550,20 +550,21 @@ where
         // Error slots use a threaded `std::sync::Mutex`, so accessing the slot
         // can block the async task's current thread. We only perform a single
         // slot update per `Client`, and panic to enforce this constraint.
-        if self.error_slot.try_update_error(e).is_err() {
+        //
-            // This panic typically happens due to these bugs:
+        // This assertion typically fails due to these bugs:
-            // * we mark a connection as failed without using fail_with
+        // * we mark a connection as failed without using fail_with
-            // * we call fail_with without checking for a failed connection
+        // * we call fail_with without checking for a failed connection
-            //   state
+        //   state
-            // * we continue processing messages after calling fail_with
+        // * we continue processing messages after calling fail_with
-            //
+        //
-            // See the original bug #1510 and PR #1531, and the later bug #1599
+        // See the original bug #1510 and PR #1531, and the later bug #1599
-            // and PR #1600.
+        // and PR #1600.
-            panic!("calling fail_with on already-failed connection state: failed connections must stop processing pending requests and responses, then close the connection. state: {:?} client receiver: {:?}",
+        assert!(
-                self.state,
+            self.error_slot.try_update_error(e).is_ok(),
-                self.client_rx
+            "calling fail_with on already-failed connection state: failed connections must stop processing pending requests and responses, then close the connection. state: {:?} client receiver: {:?}",
-            );
+            self.state,
-        }
+            self.client_rx
+        );
 
         // We want to close the client channel and set State::Failed so
         // that we can flush any pending client requests. However, we may have

zebra-state/src/service/non_finalized_state.rs

@@ -83,6 +83,10 @@ impl NonFinalizedState {
     /// Finalize the lowest height block in the non-finalized portion of the best
     /// chain and update all side-chains to match.
     pub fn finalize(&mut self) -> FinalizedBlock {
+        // Chain::cmp uses the partial cumulative work, and the hash of the tip block.
+        // Neither of these fields has interior mutability.
+        // (And when the tip block is dropped for a chain, the chain is also dropped.)
+        #[allow(clippy::mutable_key_type)]
         let chains = mem::take(&mut self.chain_set);
         let mut chains = chains.into_iter();
 
@@ -240,6 +244,9 @@ impl NonFinalizedState {
     where
         F: Fn(&Chain) -> bool,
     {
+        // Chain::cmp uses the partial cumulative work, and the hash of the tip block.
+        // Neither of these fields has interior mutability.
+        #[allow(clippy::mutable_key_type)]
         let chains = mem::take(&mut self.chain_set);
         let mut best_chain_iter = chains.into_iter().rev();
 

zebra-state/src/service/non_finalized_state/chain.rs

@@ -874,6 +874,11 @@ impl Ord for Chain {
     ///
     /// https://zips.z.cash/protocol/protocol.pdf#blockchain
     ///
+    /// # Correctness
+    ///
+    /// `Chain::cmp` is used in a `BTreeSet`, so the fields accessed by `cmp` must not have
+    /// interior mutability.
+    ///
     /// # Panics
     ///
     /// If two chains compare equal.