Merge pull request #6410 from jferrant/chore/remove-blanket-copy-from-impl-array-newtype

Remove blanket copy/clone from impl_array_newtype

Author: jferrant

.cargo/config.toml

@@ -2,7 +2,7 @@
 stacks-node = "run --package stacks-node --"
 fmt-stacks = "fmt -- --config group_imports=StdExternalCrate,imports_granularity=Module"
 clippy-stacks = "clippy -p stx-genesis -p libstackerdb -p stacks-signer -p pox-locking -p clarity-serialization -p clarity -p libsigner -p stacks-common --no-deps --tests --all-features -- -D warnings"
-clippy-stackslib = "clippy -p stackslib --no-deps -- -Aclippy::all -Wclippy::indexing_slicing -Wclippy::nonminimal_bool"
+clippy-stackslib = "clippy -p stackslib --no-deps -- -Aclippy::all -Wclippy::indexing_slicing -Wclippy::nonminimal_bool -Wclippy::clone_on_copy"
 
 # Uncomment to improve performance slightly, at the cost of portability
 #   * Note that native binaries may not run on CPUs that are different from the build machine

libsigner/src/tests/signer_state.rs

@@ -112,7 +112,7 @@ impl SignerStateTest {
             self.active_signer_protocol_version,
             self.local_supported_signer_protocol_version,
             StateMachineUpdateContent::V1 {
-                burn_block: self.burn_block,
+                burn_block: self.burn_block.clone(),
                 burn_block_height: self.burn_block_height,
                 current_miner: self.current_miner.clone(),
                 replay_transactions: transactions,
@@ -219,7 +219,7 @@ fn determine_latest_supported_signer_protocol_versions() {
             active_signer_protocol_version,
             new_version,
             StateMachineUpdateContent::V0 {
-                burn_block,
+                burn_block: burn_block.clone(),
                 burn_block_height,
                 current_miner: current_miner.clone(),
             },
@@ -286,17 +286,17 @@ fn determine_global_burn_views() {
 
     assert_eq!(
         global_eval.determine_global_burn_view().unwrap(),
-        (burn_block, burn_block_height)
+        (&burn_block, burn_block_height)
     );
 
     // Let's update 3 signers (60 percent) to support a new burn block view
     let new_update = StateMachineUpdateMessage::new(
         active_signer_protocol_version,
         local_supported_signer_protocol_version,
         StateMachineUpdateContent::V0 {
-            burn_block,
+            burn_block: burn_block.clone(),
             burn_block_height: burn_block_height.wrapping_add(1),
-            current_miner: current_miner.clone(),
+            current_miner,
         },
     )
     .unwrap();
@@ -313,7 +313,7 @@ fn determine_global_burn_views() {
     global_eval.insert_update(local_address, new_update);
     assert_eq!(
         global_eval.determine_global_burn_view().unwrap(),
-        (burn_block, burn_block_height.wrapping_add(1))
+        (&burn_block, burn_block_height.wrapping_add(1))
     );
 }
 
@@ -344,9 +344,9 @@ fn determine_global_states() {
     };
 
     let state_machine = SignerStateMachine {
-        burn_block,
+        burn_block: burn_block.clone(),
         burn_block_height,
-        current_miner: (&current_miner).into(),
+        current_miner: current_miner.clone().into(),
         active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
     };
@@ -365,7 +365,7 @@ fn determine_global_states() {
         active_signer_protocol_version,
         local_supported_signer_protocol_version,
         StateMachineUpdateContent::V0 {
-            burn_block,
+            burn_block: burn_block.clone(),
             burn_block_height,
             current_miner: new_miner.clone(),
         },
@@ -385,7 +385,7 @@ fn determine_global_states() {
     let state_machine = SignerStateMachine {
         burn_block,
         burn_block_height,
-        current_miner: (&new_miner).into(),
+        current_miner: new_miner.into(),
         active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
     };
@@ -425,7 +425,7 @@ fn determine_global_states_with_tx_replay_set() {
     let state_machine = SignerStateMachine {
         burn_block,
         burn_block_height,
-        current_miner: (&current_miner).into(),
+        current_miner: current_miner.clone().into(),
         active_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
     };
@@ -460,9 +460,9 @@ fn determine_global_states_with_tx_replay_set() {
     global_eval.insert_update(local_address.clone(), no_tx_replay_set_update.clone());
 
     let new_burn_view_state_machine = SignerStateMachine {
-        burn_block,
+        burn_block: burn_block.clone(),
         burn_block_height,
-        current_miner: (&current_miner).into(),
+        current_miner: current_miner.clone().into(),
         active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
     };
@@ -493,7 +493,7 @@ fn determine_global_states_with_tx_replay_set() {
         active_signer_protocol_version,
         local_supported_signer_protocol_version,
         StateMachineUpdateContent::V1 {
-            burn_block,
+            burn_block: burn_block.clone(),
             burn_block_height,
             current_miner: current_miner.clone(),
             replay_transactions: vec![tx.clone()],
@@ -518,7 +518,7 @@ fn determine_global_states_with_tx_replay_set() {
     let tx_replay_state_machine = SignerStateMachine {
         burn_block,
         burn_block_height,
-        current_miner: (&current_miner).into(),
+        current_miner: current_miner.into(),
         active_signer_protocol_version,
         tx_replay_set: ReplayTransactionSet::new(vec![tx]),
     };

libsigner/src/v0/messages.rs

@@ -1238,10 +1238,10 @@ impl BlockResponse {
     }
 
     /// Get the signer signature hash from the block response
-    pub fn get_signer_signature_hash(&self) -> Sha512Trunc256Sum {
+    pub fn get_signer_signature_hash(&self) -> &Sha512Trunc256Sum {
         match self {
-            BlockResponse::Accepted(accepted) => accepted.signer_signature_hash,
-            BlockResponse::Rejected(rejection) => rejection.signer_signature_hash,
+            BlockResponse::Accepted(accepted) => &accepted.signer_signature_hash,
+            BlockResponse::Rejected(rejection) => &rejection.signer_signature_hash,
         }
     }
 

libsigner/src/v0/signer_state.rs

@@ -53,7 +53,7 @@ impl GlobalStateEvaluator {
     }
 
     /// Determine what the maximum signer protocol version that a majority of signers can support
-    pub fn determine_latest_supported_signer_protocol_version(&mut self) -> Option<u64> {
+    pub fn determine_latest_supported_signer_protocol_version(&self) -> Option<u64> {
         let mut protocol_versions = HashMap::new();
         for (address, update) in &self.address_updates {
             let Some(weight) = self.address_weights.get(address) else {
@@ -78,7 +78,7 @@ impl GlobalStateEvaluator {
     }
 
     /// Determine what the global burn view is if there is one
-    pub fn determine_global_burn_view(&mut self) -> Option<(ConsensusHash, u64)> {
+    pub fn determine_global_burn_view(&self) -> Option<(&ConsensusHash, u64)> {
         let mut burn_blocks = HashMap::new();
         for (address, update) in &self.address_updates {
             let Some(weight) = self.address_weights.get(address) else {
@@ -91,14 +91,14 @@ impl GlobalStateEvaluator {
                 .or_insert_with(|| 0);
             *entry += weight;
             if self.reached_agreement(*entry) {
-                return Some((*burn_block, burn_block_height));
+                return Some((burn_block, burn_block_height));
             }
         }
         None
     }
 
     /// Check if there is an agreed upon global state
-    pub fn determine_global_state(&mut self) -> Option<SignerStateMachine> {
+    pub fn determine_global_state(&self) -> Option<SignerStateMachine> {
         let active_signer_protocol_version =
             self.determine_latest_supported_signer_protocol_version()?;
         let mut state_views = HashMap::new();
@@ -114,9 +114,9 @@ impl GlobalStateEvaluator {
             let tx_replay_set = update.content.tx_replay_set();
 
             let state_machine = SignerStateMachine {
-                burn_block: *burn_block,
+                burn_block: burn_block.clone(),
                 burn_block_height,
-                current_miner: current_miner.into(),
+                current_miner: current_miner.clone().into(),
                 active_signer_protocol_version,
                 // We need to calculate the threshold for the tx_replay_set separately
                 tx_replay_set: ReplayTransactionSet::none(),
@@ -396,9 +396,9 @@ pub enum MinerState {
     NoValidMiner,
 }
 
-impl From<&StateMachineUpdateMinerState> for MinerState {
-    fn from(val: &StateMachineUpdateMinerState) -> Self {
-        match *val {
+impl From<StateMachineUpdateMinerState> for MinerState {
+    fn from(val: StateMachineUpdateMinerState) -> Self {
+        match val {
             StateMachineUpdateMinerState::NoValidMiner => MinerState::NoValidMiner,
             StateMachineUpdateMinerState::ActiveMiner {
                 current_miner_pkh,

libstackerdb/src/libstackerdb.rs

@@ -213,7 +213,7 @@ impl StackerDBChunkData {
             slot_id: self.slot_id,
             slot_version: self.slot_version,
             data_hash: self.data_hash(),
-            signature: self.sig,
+            signature: self.sig.clone(),
         }
     }
 

stacks-common/src/deps_common/bitcoin/blockdata/block.rs

@@ -32,7 +32,7 @@ use crate::util::uint::Uint256;
 
 /// A block header, which contains all the block's information except
 /// the actual transactions
-#[derive(Copy, PartialEq, Eq, Clone, Debug)]
+#[derive(PartialEq, Eq, Clone, Debug)]
 pub struct BlockHeader {
     /// The protocol version. Should always be 1.
     pub version: u32,

stacks-common/src/deps_common/bitcoin/blockdata/transaction.rs

@@ -36,7 +36,7 @@ use crate::deps_common::bitcoin::network::serialize::{
 use crate::deps_common::bitcoin::util::hash::Sha256dHash;
 
 /// A reference to a transaction output
-#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
 pub struct OutPoint {
     /// The referenced transaction's txid
     pub txid: Sha256dHash,
@@ -161,7 +161,7 @@ impl Transaction {
                 .map(|txin| TxIn {
                     script_sig: Script::new(),
                     witness: vec![],
-                    ..*txin
+                    ..txin.clone()
                 })
                 .collect(),
             output: self.output.clone(),
@@ -228,7 +228,7 @@ impl Transaction {
         // Add all inputs necessary..
         if anyone_can_pay {
             tx.input = vec![TxIn {
-                previous_output: self.input[input_index].previous_output,
+                previous_output: self.input[input_index].previous_output.clone(),
                 script_sig: script_pubkey.clone(),
                 sequence: self.input[input_index].sequence,
                 witness: vec![],
@@ -237,7 +237,7 @@ impl Transaction {
             tx.input = Vec::with_capacity(self.input.len());
             for (n, input) in self.input.iter().enumerate() {
                 tx.input.push(TxIn {
-                    previous_output: input.previous_output,
+                    previous_output: input.previous_output.clone(),
                     script_sig: if n == input_index {
                         script_pubkey.clone()
                     } else {

stacks-common/src/deps_common/bitcoin/util/hash.rs

@@ -433,7 +433,7 @@ pub fn bitcoin_merkle_root(data: Vec<Sha256dHash>) -> Sha256dHash {
         return Default::default();
     }
     if data.len() == 1 {
-        return data[0];
+        return data[0].clone();
     }
     // Recursion
     let iterations = data.len().div_ceil(2);

stacks-common/src/types/chainstate.rs

@@ -33,7 +33,7 @@ pub type StacksPublicKey = Secp256k1PublicKey;
 pub type StacksPrivateKey = Secp256k1PrivateKey;
 
 /// Hash of a Trie node.  This is a SHA2-512/256.
-#[derive(Default)]
+#[derive(Default, Copy)]
 pub struct TrieHash(pub [u8; 32]);
 impl_array_newtype!(TrieHash, u8, 32);
 impl_array_hexstring_fmt!(TrieHash);

stacks-common/src/util/macros.rs

@@ -427,14 +427,12 @@ macro_rules! impl_array_newtype {
         }
 
         impl Clone for $thing {
-            #[inline]
-            fn clone(&self) -> $thing {
-                *self
+            #[allow(clippy::non_canonical_clone_impl)]
+            fn clone(&self) -> Self {
+                $thing(self.0.clone())
             }
         }
 
-        impl Copy for $thing {}
-
         impl ::std::hash::Hash for $thing {
             #[inline]
             fn hash<H>(&self, state: &mut H)