Remove improper uses of &Arc<Bank> (#32802)

In most cases, either a &Bank or an Arc<Bank> is more proper.
- &Bank is used if the function only needs a momentary reference
- Arc<Bank> is used if the function needs its' own copy

This PR leaves several instances of &Arc<Bank> around; these instances
are situations where a clone may only happen conditionally.

banking-bench/src/main.rs

@@ -416,8 +416,12 @@ fn main() {
             Blockstore::open(&ledger_path).expect("Expected to be able to open database ledger"),
         );
         let leader_schedule_cache = Arc::new(LeaderScheduleCache::new_from_bank(&bank));
-        let (exit, poh_recorder, poh_service, signal_receiver) =
-            create_test_recorder(&bank, blockstore.clone(), None, Some(leader_schedule_cache));
+        let (exit, poh_recorder, poh_service, signal_receiver) = create_test_recorder(
+            bank.clone(),
+            blockstore.clone(),
+            None,
+            Some(leader_schedule_cache),
+        );
         let (banking_tracer, tracer_thread) =
             BankingTracer::new(matches.is_present("trace_banking").then_some((
                 &blockstore.banking_trace_path(),
@@ -528,7 +532,8 @@ fn main() {
                 poh_time.stop();
 
                 let mut new_bank_time = Measure::start("new_bank");
-                let new_bank = Bank::new_from_parent(&bank, &collector, bank.slot() + 1);
+                let new_slot = bank.slot() + 1;
+                let new_bank = Bank::new_from_parent(bank, &collector, new_slot);
                 new_bank_time.stop();
 
                 let mut insert_time = Measure::start("insert_time");

client-test/tests/client.rs

@@ -136,7 +136,7 @@ fn test_account_subscription() {
     let blockhash = bank.last_blockhash();
     let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
     let bank0 = bank_forks.read().unwrap().get(0).unwrap();
-    let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), 1);
+    let bank1 = Bank::new_from_parent(bank0, &Pubkey::default(), 1);
     bank_forks.write().unwrap().insert(bank1);
     let bob = Keypair::new();
     let max_complete_transaction_status_slot = Arc::new(AtomicU64::default());
@@ -342,7 +342,7 @@ fn test_program_subscription() {
     let blockhash = bank.last_blockhash();
     let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
     let bank0 = bank_forks.read().unwrap().get(0).unwrap();
-    let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), 1);
+    let bank1 = Bank::new_from_parent(bank0, &Pubkey::default(), 1);
     bank_forks.write().unwrap().insert(bank1);
     let bob = Keypair::new();
     let max_complete_transaction_status_slot = Arc::new(AtomicU64::default());
@@ -429,7 +429,7 @@ fn test_root_subscription() {
     let bank = Bank::new_for_tests(&genesis_config);
     let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
     let bank0 = bank_forks.read().unwrap().get(0).unwrap();
-    let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), 1);
+    let bank1 = Bank::new_from_parent(bank0, &Pubkey::default(), 1);
     bank_forks.write().unwrap().insert(bank1);
     let max_complete_transaction_status_slot = Arc::new(AtomicU64::default());
     let max_complete_rewards_slot = Arc::new(AtomicU64::default());

core/benches/banking_stage.rs

@@ -86,7 +86,7 @@ fn bench_consume_buffered(bencher: &mut Bencher) {
             Blockstore::open(&ledger_path).expect("Expected to be able to open database ledger"),
         );
         let (exit, poh_recorder, poh_service, _signal_receiver) =
-            create_test_recorder(&bank, blockstore, None, None);
+            create_test_recorder(bank, blockstore, None, None);
 
         let recorder = poh_recorder.read().unwrap().new_recorder();
         let bank_start = poh_recorder.read().unwrap().bank_start().unwrap();
@@ -282,7 +282,7 @@ fn bench_banking(bencher: &mut Bencher, tx_type: TransactionType) {
             Blockstore::open(&ledger_path).expect("Expected to be able to open database ledger"),
         );
         let (exit, poh_recorder, poh_service, signal_receiver) =
-            create_test_recorder(&bank, blockstore, None, None);
+            create_test_recorder(bank.clone(), blockstore, None, None);
         let cluster_info = {
             let keypair = Arc::new(Keypair::new());
             let node = Node::new_localhost_with_pubkey(&keypair.pubkey());

core/benches/consumer.rs

@@ -122,7 +122,7 @@ fn setup(apply_cost_tracker_during_replay: bool) -> BenchFrame {
         Blockstore::open(ledger_path.path()).expect("Expected to be able to open database ledger"),
     );
     let (exit, poh_recorder, poh_service, signal_receiver) =
-        create_test_recorder(&bank, blockstore, None, None);
+        create_test_recorder(bank.clone(), blockstore, None, None);
 
     BenchFrame {
         bank,

core/src/banking_stage.rs

@@ -650,7 +650,7 @@ mod tests {
         let genesis_config = create_genesis_config(2).genesis_config;
         let bank = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);
         let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
-        let bank = Arc::new(bank_forks.read().unwrap().get(0).unwrap());
+        let bank = bank_forks.read().unwrap().get(0).unwrap();
         let banking_tracer = BankingTracer::new_disabled();
         let (non_vote_sender, non_vote_receiver) = banking_tracer.create_channel_non_vote();
         let (tpu_vote_sender, tpu_vote_receiver) = banking_tracer.create_channel_tpu_vote();
@@ -663,7 +663,7 @@ mod tests {
                     .expect("Expected to be able to open database ledger"),
             );
             let (exit, poh_recorder, poh_service, _entry_receiever) =
-                create_test_recorder(&bank, blockstore, None, None);
+                create_test_recorder(bank, blockstore, None, None);
             let (_, cluster_info) = new_test_cluster_info(/*keypair:*/ None);
             let cluster_info = Arc::new(cluster_info);
             let (replay_vote_sender, _replay_vote_receiver) = unbounded();
@@ -701,7 +701,7 @@ mod tests {
         let num_extra_ticks = 2;
         let bank = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);
         let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
-        let bank = Arc::new(bank_forks.read().unwrap().get(0).unwrap());
+        let bank = bank_forks.read().unwrap().get(0).unwrap();
         let start_hash = bank.last_blockhash();
         let banking_tracer = BankingTracer::new_disabled();
         let (non_vote_sender, non_vote_receiver) = banking_tracer.create_channel_non_vote();
@@ -719,7 +719,7 @@ mod tests {
                 ..PohConfig::default()
             };
             let (exit, poh_recorder, poh_service, entry_receiver) =
-                create_test_recorder(&bank, blockstore, Some(poh_config), None);
+                create_test_recorder(bank.clone(), blockstore, Some(poh_config), None);
             let (_, cluster_info) = new_test_cluster_info(/*keypair:*/ None);
             let cluster_info = Arc::new(cluster_info);
             let (replay_vote_sender, _replay_vote_receiver) = unbounded();
@@ -780,7 +780,7 @@ mod tests {
         } = create_slow_genesis_config(10);
         let bank = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);
         let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
-        let bank = Arc::new(bank_forks.read().unwrap().get(0).unwrap());
+        let bank = bank_forks.read().unwrap().get(0).unwrap();
         let start_hash = bank.last_blockhash();
         let banking_tracer = BankingTracer::new_disabled();
         let (non_vote_sender, non_vote_receiver) = banking_tracer.create_channel_non_vote();
@@ -800,7 +800,7 @@ mod tests {
                 ..PohConfig::default()
             };
             let (exit, poh_recorder, poh_service, entry_receiver) =
-                create_test_recorder(&bank, blockstore, Some(poh_config), None);
+                create_test_recorder(bank.clone(), blockstore, Some(poh_config), None);
             let (_, cluster_info) = new_test_cluster_info(/*keypair:*/ None);
             let cluster_info = Arc::new(cluster_info);
             let (replay_vote_sender, _replay_vote_receiver) = unbounded();
@@ -951,7 +951,7 @@ mod tests {
                 // start a banking_stage to eat verified receiver
                 let bank = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);
                 let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
-                let bank = Arc::new(bank_forks.read().unwrap().get(0).unwrap());
+                let bank = bank_forks.read().unwrap().get(0).unwrap();
                 let blockstore = Arc::new(
                     Blockstore::open(ledger_path.path())
                         .expect("Expected to be able to open database ledger"),
@@ -963,7 +963,7 @@ mod tests {
                     ..PohConfig::default()
                 };
                 let (exit, poh_recorder, poh_service, entry_receiver) =
-                    create_test_recorder(&bank, blockstore, Some(poh_config), None);
+                    create_test_recorder(bank.clone(), blockstore, Some(poh_config), None);
                 let (_, cluster_info) = new_test_cluster_info(/*keypair:*/ None);
                 let cluster_info = Arc::new(cluster_info);
                 let _banking_stage = BankingStage::new_num_threads(
@@ -1136,7 +1136,7 @@ mod tests {
         );
         let bank = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);
         let bank_forks = Arc::new(RwLock::new(BankForks::new(bank)));
-        let bank = Arc::new(bank_forks.read().unwrap().get(0).unwrap());
+        let bank = bank_forks.read().unwrap().get(0).unwrap();
         let start_hash = bank.last_blockhash();
         let banking_tracer = BankingTracer::new_disabled();
         let (non_vote_sender, non_vote_receiver) = banking_tracer.create_channel_non_vote();
@@ -1156,7 +1156,7 @@ mod tests {
                 ..PohConfig::default()
             };
             let (exit, poh_recorder, poh_service, _entry_receiver) =
-                create_test_recorder(&bank, blockstore, Some(poh_config), None);
+                create_test_recorder(bank.clone(), blockstore, Some(poh_config), None);
             let (_, cluster_info) = new_test_cluster_info(/*keypair:*/ None);
             let cluster_info = Arc::new(cluster_info);
             let (replay_vote_sender, _replay_vote_receiver) = unbounded();

core/src/banking_stage/consumer.rs

@@ -688,7 +688,7 @@ impl Consumer {
     /// * `transactions` - a batch of transactions deserialized from packets
     /// * `pending_indexes` - identifies which indexes in the `transactions` list are still pending
     fn filter_pending_packets_from_pending_txs(
-        bank: &Arc<Bank>,
+        bank: &Bank,
         transactions: &[SanitizedTransaction],
         pending_indexes: &[usize],
     ) -> Vec<usize> {
@@ -1701,7 +1701,7 @@ mod tests {
         let address_table_state = generate_new_address_lookup_table(None, 2);
         store_address_lookup_table(&bank, address_table_key, address_table_state);
 
-        let bank = Arc::new(Bank::new_from_parent(&bank, &Pubkey::new_unique(), 1));
+        let bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::new_unique(), 1));
         let message = VersionedMessage::V0(v0::Message {
             header: MessageHeader {
                 num_required_signatures: 1,

core/src/banking_stage/decision_maker.rs

@@ -151,12 +151,13 @@ mod tests {
         let ledger_path = temp_dir();
         let blockstore = Arc::new(Blockstore::open(ledger_path.as_path()).unwrap());
         let (exit, poh_recorder, poh_service, _entry_receiver) =
-            create_test_recorder(&bank, blockstore, None, None);
+            create_test_recorder(bank.clone(), blockstore, None, None);
 
         let my_pubkey = Pubkey::new_unique();
         let decision_maker = DecisionMaker::new(my_pubkey, poh_recorder.clone());
         poh_recorder.write().unwrap().reset(bank.clone(), None);
-        let bank = Arc::new(Bank::new_from_parent(&bank, &my_pubkey, bank.slot() + 1));
+        let slot = bank.slot() + 1;
+        let bank = Arc::new(Bank::new_from_parent(bank, &my_pubkey, slot));
 
         // Currently Leader - Consume
         {

core/src/banking_stage/forwarder.rs

@@ -323,7 +323,7 @@ mod tests {
         };
 
         let (exit, poh_recorder, poh_service, _entry_receiver) =
-            create_test_recorder(&bank, blockstore, Some(poh_config), None);
+            create_test_recorder(bank, blockstore, Some(poh_config), None);
 
         let (local_node, cluster_info) = new_test_cluster_info(Some(validator_keypair));
         let cluster_info = Arc::new(cluster_info);

core/src/banking_stage/latest_unprocessed_votes.rs

@@ -116,7 +116,7 @@ impl LatestValidatorVotePacket {
 // This requires updating dependencies of ed25519-dalek as rand_core is not compatible cross
 // version https://github.com/dalek-cryptography/ed25519-dalek/pull/214
 pub(crate) fn weighted_random_order_by_stake<'a>(
-    bank: &Arc<Bank>,
+    bank: &Bank,
     pubkeys: impl Iterator<Item = &'a Pubkey>,
 ) -> impl Iterator<Item = Pubkey> {
     // Efraimidis and Spirakis algo for weighted random sample without replacement

core/src/banking_stage/leader_slot_metrics.rs

@@ -868,7 +868,7 @@ mod tests {
 
         // Create a child descended from the first bank
         let next_bank = Arc::new(Bank::new_from_parent(
-            &first_bank,
+            first_bank.clone(),
             &Pubkey::new_unique(),
             first_bank.slot() + 1,
         ));

core/src/banking_stage/qos_service.rs

@@ -14,10 +14,7 @@ use {
         saturating_add_assign,
         transaction::{self, SanitizedTransaction, TransactionError},
     },
-    std::sync::{
-        atomic::{AtomicU64, Ordering},
-        Arc,
-    },
+    std::sync::atomic::{AtomicU64, Ordering},
 };
 
 // QosService is local to each banking thread, each instance of QosService provides services to
@@ -137,7 +134,7 @@ impl QosService {
     pub fn update_costs<'a>(
         transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
         transaction_committed_status: Option<&Vec<CommitTransactionDetails>>,
-        bank: &Arc<Bank>,
+        bank: &Bank,
     ) {
         if let Some(transaction_committed_status) = transaction_committed_status {
             Self::update_committed_transaction_costs(
@@ -152,7 +149,7 @@ impl QosService {
     pub fn remove_costs<'a>(
         transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
         transaction_committed_status: Option<&Vec<CommitTransactionDetails>>,
-        bank: &Arc<Bank>,
+        bank: &Bank,
     ) {
         match transaction_committed_status {
             Some(transaction_committed_status) => Self::remove_uncommitted_transaction_costs(
@@ -167,7 +164,7 @@ impl QosService {
     fn remove_uncommitted_transaction_costs<'a>(
         transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
         transaction_committed_status: &Vec<CommitTransactionDetails>,
-        bank: &Arc<Bank>,
+        bank: &Bank,
     ) {
         let mut cost_tracker = bank.write_cost_tracker().unwrap();
         transaction_cost_results
@@ -186,7 +183,7 @@ impl QosService {
     fn update_committed_transaction_costs<'a>(
         transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
         transaction_committed_status: &Vec<CommitTransactionDetails>,
-        bank: &Arc<Bank>,
+        bank: &Bank,
     ) {
         let mut cost_tracker = bank.write_cost_tracker().unwrap();
         transaction_cost_results
@@ -206,7 +203,7 @@ impl QosService {
 
     fn remove_transaction_costs<'a>(
         transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
-        bank: &Arc<Bank>,
+        bank: &Bank,
     ) {
         let mut cost_tracker = bank.write_cost_tracker().unwrap();
         transaction_cost_results.for_each(|tx_cost| {
@@ -599,6 +596,7 @@ mod tests {
             system_transaction,
         },
         solana_vote_program::vote_transaction,
+        std::sync::Arc,
     };
 
     #[test]

core/src/banking_stage/read_write_account_set.rs

@@ -182,12 +182,9 @@ mod tests {
         account.set_data(data);
         bank.store_account(&address_table_key, &account);
 
+        let slot = bank.slot() + 1;
         (
-            Arc::new(Bank::new_from_parent(
-                &bank,
-                &Pubkey::new_unique(),
-                bank.slot() + 1,
-            )),
+            Arc::new(Bank::new_from_parent(bank, &Pubkey::new_unique(), slot)),
             address_table_key,
         )
     }

core/src/banking_stage/unprocessed_transaction_storage.rs

@@ -711,7 +711,7 @@ impl ThreadLocalUnprocessedPackets {
     fn sanitize_unforwarded_packets(
         &mut self,
         packets_to_process: &[Arc<ImmutableDeserializedPacket>],
-        bank: &Arc<Bank>,
+        bank: &Bank,
         total_dropped_packets: &mut usize,
     ) -> (Vec<SanitizedTransaction>, Vec<usize>) {
         // Get ref of ImmutableDeserializedPacket
@@ -721,11 +721,7 @@ impl ThreadLocalUnprocessedPackets {
                 .enumerate()
                 .filter_map(|(packet_index, deserialized_packet)| {
                     deserialized_packet
-                        .build_sanitized_transaction(
-                            &bank.feature_set,
-                            bank.vote_only_bank(),
-                            bank.as_ref(),
-                        )
+                        .build_sanitized_transaction(&bank.feature_set, bank.vote_only_bank(), bank)
                         .map(|transaction| (transaction, packet_index))
                 })
                 .unzip();
@@ -740,7 +736,7 @@ impl ThreadLocalUnprocessedPackets {
     /// Checks sanitized transactions against bank, returns valid transaction indexes
     fn filter_invalid_transactions(
         transactions: &[SanitizedTransaction],
-        bank: &Arc<Bank>,
+        bank: &Bank,
         total_dropped_packets: &mut usize,
     ) -> Vec<usize> {
         let filter = vec![Ok(()); transactions.len()];

core/src/cluster_info_vote_listener.rs

@@ -958,7 +958,7 @@ mod tests {
             .read()
             .unwrap()
             .contains_key(&bank.slot()));
-        let bank1 = Bank::new_from_parent(&bank, &Pubkey::default(), bank.slot() + 1);
+        let bank1 = Bank::new_from_parent(bank.clone(), &Pubkey::default(), bank.slot() + 1);
         vote_tracker.progress_with_new_root_bank(&bank1);
         assert!(!vote_tracker
             .slot_vote_trackers
@@ -969,12 +969,10 @@ mod tests {
         // Check `keys` and `epoch_authorized_voters` are purged when new
         // root bank moves to the next epoch
         let current_epoch = bank.epoch();
-        let new_epoch_bank = Bank::new_from_parent(
-            &bank,
-            &Pubkey::default(),
-            bank.epoch_schedule()
-                .get_first_slot_in_epoch(current_epoch + 1),
-        );
+        let new_epoch_slot = bank
+            .epoch_schedule()
+            .get_first_slot_in_epoch(current_epoch + 1);
+        let new_epoch_bank = Bank::new_from_parent(bank, &Pubkey::default(), new_epoch_slot);
         vote_tracker.progress_with_new_root_bank(&new_epoch_bank);
     }
 
@@ -1020,7 +1018,7 @@ mod tests {
         let bank0 = Bank::new_for_tests(&genesis_config);
         // Votes for slots less than the provided root bank's slot should not be processed
         let bank3 = Arc::new(Bank::new_from_parent(
-            &Arc::new(bank0),
+            Arc::new(bank0),
             &Pubkey::default(),
             3,
         ));
@@ -1541,7 +1539,7 @@ mod tests {
             .collect();
 
         let new_root_bank =
-            Bank::new_from_parent(&bank, &Pubkey::default(), first_slot_in_new_epoch - 2);
+            Bank::new_from_parent(bank, &Pubkey::default(), first_slot_in_new_epoch - 2);
         ClusterInfoVoteListener::filter_and_confirm_with_new_votes(
             &vote_tracker,
             vote_txs,
@@ -1757,10 +1755,11 @@ mod tests {
             1
         );
 
+        let slot = current_leader_bank.slot() + 1;
         let current_leader_bank = Arc::new(Bank::new_from_parent(
-            &current_leader_bank,
+            current_leader_bank,
             &Pubkey::default(),
-            current_leader_bank.slot() + 1,
+            slot,
         ));
         ClusterInfoVoteListener::check_for_leader_bank_and_send_votes(
             &mut bank_vote_sender_state_option,