remove new_bank_from_parent_with_id() (#3039)

runtime/src/bank.rs

@@ -28,7 +28,6 @@ use solana_sdk::token_program;
 use solana_sdk::transaction::Transaction;
 use solana_sdk::vote_program::{self, VoteState};
 use std::result;
-use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::{Arc, RwLock};
 use std::time::Instant;
 
@@ -139,7 +138,7 @@ impl Bank {
     }
 
     /// Create a new bank that points to an immutable checkpoint of another bank.
-    pub fn new_from_parent_and_id(parent: &Arc<Bank>, collector_id: Pubkey, id: u64) -> Self {
+    pub fn new_from_parent(parent: &Arc<Bank>, collector_id: Pubkey, id: u64) -> Self {
         parent.freeze();
 
         let mut bank = Self::default();
@@ -171,18 +170,6 @@ impl Bank {
         bank
     }
 
-    /// Create a new bank that points to an immutable checkpoint of another bank.
-    /// TODO: remove me in favor of _and_id(), id should not be an assumed value
-    pub fn new_from_parent(parent: &Arc<Bank>) -> Self {
-        static BANK_ID: AtomicUsize = AtomicUsize::new(1);
-        let collector_id = parent.collector_id;
-        Self::new_from_parent_and_id(
-            parent,
-            collector_id,
-            BANK_ID.fetch_add(1, Ordering::Relaxed) as u64,
-        )
-    }
-
     pub fn slot(&self) -> u64 {
         self.slot
     }
@@ -233,7 +220,7 @@ impl Bank {
         assert!(genesis_block.tokens >= genesis_block.bootstrap_leader_tokens);
         assert!(genesis_block.bootstrap_leader_tokens >= 2);
 
-        // Bootstrap leader collects fees until `new_from_parent_and_id` is called.
+        // Bootstrap leader collects fees until `new_from_parent` is called.
         self.collector_id = genesis_block.bootstrap_leader_id;
 
         let mint_tokens = genesis_block.tokens - genesis_block.bootstrap_leader_tokens;
@@ -1312,13 +1299,17 @@ mod tests {
         res[0].clone().unwrap_err();
     }
 
+    fn new_from_parent(parent: &Arc<Bank>) -> Bank {
+        Bank::new_from_parent(parent, Pubkey::default(), parent.slot() + 1)
+    }
+
     /// Verify that the parent's vector is computed correctly
     #[test]
     fn test_bank_parents() {
         let (genesis_block, _) = GenesisBlock::new(1);
         let parent = Arc::new(Bank::new(&genesis_block));
 
-        let bank = Bank::new_from_parent(&parent);
+        let bank = new_from_parent(&parent);
         assert!(Arc::ptr_eq(&bank.parents()[0], &parent));
     }
 
@@ -1332,7 +1323,7 @@ mod tests {
         let tx =
             SystemTransaction::new_move(&mint_keypair, key1.pubkey(), 1, genesis_block.hash(), 0);
         assert_eq!(parent.process_transaction(&tx), Ok(()));
-        let bank = Bank::new_from_parent(&parent);
+        let bank = new_from_parent(&parent);
         assert_eq!(
             bank.process_transaction(&tx),
             Err(BankError::DuplicateSignature)
@@ -1350,7 +1341,7 @@ mod tests {
         let tx =
             SystemTransaction::new_move(&mint_keypair, key1.pubkey(), 1, genesis_block.hash(), 0);
         assert_eq!(parent.process_transaction(&tx), Ok(()));
-        let bank = Bank::new_from_parent(&parent);
+        let bank = new_from_parent(&parent);
         let tx = SystemTransaction::new_move(&key1, key2.pubkey(), 1, genesis_block.hash(), 0);
         assert_eq!(bank.process_transaction(&tx), Ok(()));
         assert_eq!(parent.get_signature_status(&tx.signatures[0]), None);
@@ -1375,7 +1366,7 @@ mod tests {
         assert_eq!(bank0.hash_internal_state(), bank1.hash_internal_state());
 
         // Checkpointing should not change its state
-        let bank2 = Bank::new_from_parent(&Arc::new(bank1));
+        let bank2 = new_from_parent(&Arc::new(bank1));
         assert_eq!(bank0.hash_internal_state(), bank2.hash_internal_state());
     }
 
@@ -1390,7 +1381,7 @@ mod tests {
     fn test_bank_hash_internal_state_squash() {
         let collector_id = Pubkey::default();
         let bank0 = Arc::new(Bank::new(&GenesisBlock::new(10).0));
-        let bank1 = Bank::new_from_parent_and_id(&bank0, collector_id, 1);
+        let bank1 = Bank::new_from_parent(&bank0, collector_id, 1);
 
         // no delta in bank1, hashes match
         assert_eq!(bank0.hash_internal_state(), bank1.hash_internal_state());
@@ -1416,7 +1407,7 @@ mod tests {
         assert_eq!(parent.process_transaction(&tx_move_mint_to_1), Ok(()));
         assert_eq!(parent.transaction_count(), 1);
 
-        let bank = Bank::new_from_parent(&parent);
+        let bank = new_from_parent(&parent);
         assert_eq!(bank.transaction_count(), 0);
         let tx_move_1_to_2 =
             SystemTransaction::new_move(&key1, key2.pubkey(), 1, genesis_block.hash(), 0);
@@ -1460,7 +1451,7 @@ mod tests {
             .transfer(1, &mint_keypair, key1.pubkey(), genesis_block.hash())
             .unwrap();
         assert_eq!(parent.get_balance(&key1.pubkey()), 1);
-        let bank = Bank::new_from_parent(&parent);
+        let bank = new_from_parent(&parent);
         bank.squash();
         assert_eq!(parent.get_balance(&key1.pubkey()), 1);
     }
@@ -1496,24 +1487,31 @@ mod tests {
             Some((*key, None))
         }
 
-        assert!(parent.epoch_vote_states(1, all).is_some());
+        let mut i = 1;
-        assert!(parent.epoch_vote_states(2, all).is_some());
+        loop {
+            if i > STAKERS_SLOT_OFFSET / SLOTS_PER_EPOCH {
+                break;
+            }
+            assert!(parent.epoch_vote_states(i, all).is_some());
+            i += 1;
+        }
 
         // child crosses epoch boundary and is the first slot in the epoch
-        let child = Bank::new_from_parent_and_id(
+        let child = Bank::new_from_parent(
             &parent,
             leader_id,
             SLOTS_PER_EPOCH - (STAKERS_SLOT_OFFSET % SLOTS_PER_EPOCH),
         );
-        assert!(child.epoch_vote_states(3, all).is_some());
+
+        assert!(child.epoch_vote_states(i, all).is_some());
 
         // child crosses epoch boundary but isn't the first slot in the epoch
-        let child = Bank::new_from_parent_and_id(
+        let child = Bank::new_from_parent(
             &parent,
             leader_id,
             SLOTS_PER_EPOCH - (STAKERS_SLOT_OFFSET % SLOTS_PER_EPOCH) + 1,
         );
-        assert!(child.epoch_vote_states(3, all).is_some());
+        assert!(child.epoch_vote_states(i, all).is_some());
     }
 
 }

src/bank_forks.rs

@@ -69,13 +69,14 @@ mod tests {
     use super::*;
     use solana_sdk::genesis_block::GenesisBlock;
     use solana_sdk::hash::Hash;
+    use solana_sdk::pubkey::Pubkey;
 
     #[test]
     fn test_bank_forks() {
         let (genesis_block, _) = GenesisBlock::new(10_000);
         let bank = Bank::new(&genesis_block);
         let mut bank_forks = BankForks::new(0, bank);
-        let child_bank = Bank::new_from_parent(&bank_forks[0u64]);
+        let child_bank = Bank::new_from_parent(&bank_forks[0u64], Pubkey::default(), 1);
         child_bank.register_tick(&Hash::default());
         bank_forks.insert(1, child_bank);
         assert_eq!(bank_forks[1u64].tick_height(), 1);

src/blocktree_processor.rs

@@ -124,7 +124,7 @@ pub fn process_blocktree(
         let (slot, starting_bank, starting_entry_height, mut last_entry_hash) =
             pending_slots.pop().unwrap();
 
-        let bank = Arc::new(Bank::new_from_parent_and_id(
+        let bank = Arc::new(Bank::new_from_parent(
             &starting_bank,
             leader_schedule_utils::slot_leader_at(slot, &starting_bank),
             starting_bank.slot(),
@@ -213,7 +213,7 @@ pub fn process_blocktree(
 
         // This is a fork point, create a new child bank for each fork
         pending_slots.extend(meta.next_slots.iter().map(|next_slot| {
-            let child_bank = Arc::new(Bank::new_from_parent_and_id(
+            let child_bank = Arc::new(Bank::new_from_parent(
                 &bank,
                 leader_schedule_utils::slot_leader_at(*next_slot, &bank),
                 *next_slot,

src/replay_stage.rs

@@ -200,7 +200,7 @@ impl ReplayStage {
             // If the next slot is going to be a new slot and we're the leader for that slot,
             // make a new working bank, set it as the working bank.
             if tick_height + 1 == first_tick_in_slot && leader_id == my_id {
-                bank = Self::create_and_set_working_bank(slot, &bank_forks, &old_bank);
+                bank = Self::create_and_set_working_bank(&old_bank, leader_id, slot, &bank_forks);
             }
 
             // Send a rotation notification back to Fullnode to initialize the TPU to the right
@@ -254,9 +254,10 @@ impl ReplayStage {
                             trace!("{} replay_stage: new_slot found: {:?}", my_id, new_slot);
                             // Reset the state
                             bank = Self::create_and_set_working_bank(
+                                &bank,
+                                current_leader_id,
                                 new_slot.unwrap(),
                                 &bank_forks,
-                                &bank,
                             );
                             current_slot = new_slot;
                             Self::reset_state(
@@ -346,9 +347,10 @@ impl ReplayStage {
                         if my_id == leader_id {
                             // Create new bank for next slot if we are the leader for that slot
                             bank = Self::create_and_set_working_bank(
+                                &old_bank,
+                                leader_id,
                                 next_slot,
                                 &bank_forks,
-                                &old_bank,
                             );
                             current_slot = Some(next_slot);
                             Self::reset_state(
@@ -406,11 +408,12 @@ impl ReplayStage {
     }
 
     fn create_and_set_working_bank(
+        parent: &Arc<Bank>,
+        leader_id: Pubkey,
         slot: u64,
         bank_forks: &Arc<RwLock<BankForks>>,
-        parent: &Arc<Bank>,
     ) -> Arc<Bank> {
-        let new_bank = Bank::new_from_parent(&parent);
+        let new_bank = Bank::new_from_parent(&parent, leader_id, slot);
         new_bank.squash();
         let mut bank_forks = bank_forks.write().unwrap();
         bank_forks.insert(slot, new_bank);

src/rpc_pubsub.rs

@@ -168,6 +168,7 @@ mod tests {
     use solana_sdk::budget_program;
     use solana_sdk::budget_transaction::BudgetTransaction;
     use solana_sdk::genesis_block::GenesisBlock;
+    use solana_sdk::pubkey::Pubkey;
     use solana_sdk::signature::{Keypair, KeypairUtil};
     use solana_sdk::system_transaction::SystemTransaction;
     use solana_sdk::transaction::Transaction;
@@ -184,7 +185,11 @@ mod tests {
         subscriptions.notify_subscribers(&bank);
 
         // Simulate a block boundary
-        Ok(Arc::new(Bank::new_from_parent(&bank)))
+        Ok(Arc::new(Bank::new_from_parent(
+            &bank,
+            Pubkey::default(),
+            bank.slot() + 1,
+        )))
     }
 
     fn create_session() -> Arc<Session> {

src/staking_utils.rs

@@ -120,6 +120,7 @@ mod tests {
     use hashbrown::HashSet;
     use solana_sdk::genesis_block::GenesisBlock;
     use solana_sdk::hash::Hash;
+    use solana_sdk::pubkey::Pubkey;
     use solana_sdk::signature::{Keypair, KeypairUtil};
     use std::iter::FromIterator;
     use std::sync::Arc;
@@ -131,20 +132,24 @@ mod tests {
         (bank.tick_index(), bank.slot_index(), bank.epoch_height())
     }
 
+    fn new_from_parent(parent: &Arc<Bank>) -> Bank {
+        Bank::new_from_parent(parent, Pubkey::default(), parent.slot() + 1)
+    }
+
     #[test]
     fn test_bank_staked_nodes_at_epoch() {
         let pubkey = Keypair::new().pubkey();
         let bootstrap_tokens = 2;
         let (genesis_block, _) = GenesisBlock::new_with_leader(2, pubkey, bootstrap_tokens);
         let bank = Bank::new(&genesis_block);
-        let bank = Bank::new_from_parent(&Arc::new(bank));
+        let bank = new_from_parent(&Arc::new(bank));
         let ticks_per_offset = bank.stakers_slot_offset() * bank.ticks_per_slot();
         register_ticks(&bank, ticks_per_offset);
         assert_eq!(bank.slot_height(), bank.stakers_slot_offset());
 
         let mut expected = HashMap::new();
         expected.insert(pubkey, bootstrap_tokens - 1);
-        let bank = Bank::new_from_parent(&Arc::new(bank));
+        let bank = new_from_parent(&Arc::new(bank));
         assert_eq!(
             node_stakes_at_slot_extractor(&bank, bank.slot_height(), |s, _| s),
             expected