Remove KeypairInsecureClone trait and add insecure_clone() instead (#27396)

See discussion in #26248

core/src/replay_stage.rs

@@ -3527,7 +3527,7 @@ pub(crate) mod tests {
             hash::{hash, Hash},
             instruction::InstructionError,
             poh_config::PohConfig,
-            signature::{Keypair, KeypairInsecureClone, Signer},
+            signature::{Keypair, Signer},
             system_transaction,
             transaction::TransactionError,
         },
@@ -3607,7 +3607,7 @@ pub(crate) mod tests {
         let my_pubkey = my_keypairs.node_keypair.pubkey();
         let cluster_info = ClusterInfo::new(
             Node::new_localhost_with_pubkey(&my_pubkey).info,
-            Arc::new(my_keypairs.node_keypair.clone()),
+            Arc::new(my_keypairs.node_keypair.insecure_clone()),
             SocketAddrSpace::Unspecified,
         );
         assert_eq!(my_pubkey, cluster_info.id());

local-cluster/src/local_cluster.rs

@@ -31,7 +31,7 @@ use {
         message::Message,
         poh_config::PohConfig,
         pubkey::Pubkey,
-        signature::{Keypair, KeypairInsecureClone, Signer},
+        signature::{Keypair, Signer},
         stake::{
             config as stake_config, instruction as stake_instruction,
             state::{Authorized, Lockup},
@@ -54,7 +54,6 @@ use {
         collections::HashMap,
         io::{Error, ErrorKind, Result},
         iter,
-        ops::Deref,
         path::{Path, PathBuf},
         sync::{Arc, RwLock},
     },
@@ -203,8 +202,8 @@ impl LocalCluster {
                     if *in_genesis {
                         Some((
                             ValidatorVoteKeypairs {
-                                node_keypair: node_keypair.deref().clone(),
-                                vote_keypair: vote_keypair.deref().clone(),
+                                node_keypair: node_keypair.insecure_clone(),
+                                vote_keypair: vote_keypair.insecure_clone(),
                                 stake_keypair: Keypair::new(),
                             },
                             stake,
@@ -263,8 +262,8 @@ impl LocalCluster {
         let mut leader_config = safe_clone_config(&config.validator_configs[0]);
         leader_config.rpc_addrs = Some((leader_node.info.rpc, leader_node.info.rpc_pubsub));
         Self::sync_ledger_path_across_nested_config_fields(&mut leader_config, &leader_ledger_path);
-        let leader_keypair = Arc::new(leader_keypair.clone());
-        let leader_vote_keypair = Arc::new(leader_vote_keypair.clone());
+        let leader_keypair = Arc::new(leader_keypair.insecure_clone());
+        let leader_vote_keypair = Arc::new(leader_vote_keypair.insecure_clone());
 
         let leader_server = Validator::new(
             leader_node,
@@ -314,7 +313,7 @@ impl LocalCluster {
             .map(|keypairs| {
                 (
                     keypairs.node_keypair.pubkey(),
-                    Arc::new(keypairs.vote_keypair.clone()),
+                    Arc::new(keypairs.vote_keypair.insecure_clone()),
                 )
             })
             .collect();

local-cluster/tests/local_cluster_slow_1.rs

@@ -29,13 +29,12 @@ use {
         clock::{Slot, MAX_PROCESSING_AGE},
         hash::Hash,
         pubkey::Pubkey,
-        signature::{KeypairInsecureClone, Signer},
+        signature::Signer,
     },
     solana_streamer::socket::SocketAddrSpace,
     solana_vote_program::{vote_state::MAX_LOCKOUT_HISTORY, vote_transaction},
     std::{
         collections::{BTreeSet, HashSet},
-        ops::Deref,
         path::Path,
         sync::{
             atomic::{AtomicBool, Ordering},
@@ -464,7 +463,7 @@ fn test_duplicate_shreds_broadcast_leader() {
     let (gossip_service, _tcp_listener, cluster_info) = gossip_service::make_gossip_node(
         // Need to use our validator's keypair to gossip EpochSlots and votes for our
         // node later.
-        node_keypair.deref().clone(),
+        node_keypair.insecure_clone(),
         Some(&cluster.entry_point_info.gossip),
         &exit,
         None,

runtime/src/bank.rs

@@ -19582,7 +19582,6 @@ pub(crate) mod tests {
     /// 2. The bank's accounts_data_size is unmodified
     #[test]
     fn test_cap_accounts_data_allocations_per_transaction() {
-        use solana_sdk::signature::KeypairInsecureClone;
         const NUM_MAX_SIZE_ALLOCATIONS_PER_TRANSACTION: usize =
             MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION as usize
                 / MAX_PERMITTED_DATA_LENGTH as usize;
@@ -19595,7 +19594,7 @@ pub(crate) mod tests {
         bank.activate_feature(&feature_set::cap_accounts_data_allocations_per_transaction::id());
 
         let mut instructions = Vec::new();
-        let mut keypairs = vec![mint_keypair.clone()];
+        let mut keypairs = vec![mint_keypair.insecure_clone()];
         for _ in 0..=NUM_MAX_SIZE_ALLOCATIONS_PER_TRANSACTION {
             let keypair = Keypair::new();
             let instruction = system_instruction::create_account(

runtime/src/genesis_utils.rs

@@ -7,7 +7,7 @@ use {
         genesis_config::{ClusterType, GenesisConfig},
         pubkey::Pubkey,
         rent::Rent,
-        signature::{Keypair, KeypairInsecureClone, Signer},
+        signature::{Keypair, Signer},
         stake::state::StakeState,
         system_program,
     },
@@ -111,7 +111,7 @@ pub fn create_genesis_config_with_vote_accounts_and_cluster_type(
     assert_eq!(voting_keypairs.len(), stakes.len());
 
     let mint_keypair = Keypair::new();
-    let voting_keypair = voting_keypairs[0].borrow().vote_keypair.clone();
+    let voting_keypair = voting_keypairs[0].borrow().vote_keypair.insecure_clone();
 
     let validator_pubkey = voting_keypairs[0].borrow().node_keypair.pubkey();
     let genesis_config = create_genesis_config_with_leader_ex(

sdk/src/signer/keypair.rs

@@ -64,6 +64,21 @@ impl Keypair {
     pub fn secret(&self) -> &ed25519_dalek::SecretKey {
         &self.0.secret
     }
+
+    /// Allows Keypair cloning
+    ///
+    /// Note that the `Clone` trait is intentionally unimplemented because making a
+    /// second copy of sensitive secret keys in memory is usually a bad idea.
+    ///
+    /// Only use this in tests or when strictly required. Consider using Arc<Keypair>
+    /// instead.
+    pub fn insecure_clone(&self) -> Self {
+        Self(ed25519_dalek::Keypair {
+            // This will never error since self is a valid keypair
+            secret: ed25519_dalek::SecretKey::from_bytes(self.0.secret.as_bytes()).unwrap(),
+            public: self.0.public,
+        })
+    }
 }
 
 impl Signer for Keypair {
@@ -97,26 +112,6 @@ where
     }
 }
 
-/// Allows Keypair cloning
-///
-/// Note that the `Clone` trait is intentionally unimplemented because making a
-/// second copy of sensitive secret keys in memory is usually a bad idea.
-///
-/// Only use this in tests or when strictly required.
-pub trait KeypairInsecureClone {
-    fn clone(&self) -> Self;
-}
-
-impl KeypairInsecureClone for Keypair {
-    fn clone(&self) -> Self {
-        Self(ed25519_dalek::Keypair {
-            // This will never error since self is a valid keypair
-            secret: ed25519_dalek::SecretKey::from_bytes(self.0.secret.as_bytes()).unwrap(),
-            public: self.0.public,
-        })
-    }
-}
-
 /// Reads a JSON-encoded `Keypair` from a `Reader` implementor
 pub fn read_keypair<R: Read>(reader: &mut R) -> Result<Keypair, Box<dyn error::Error>> {
     let bytes: Vec<u8> = serde_json::from_reader(reader)?;