Only add hashes for completed blocks to recent blockhashes (#24389)

* Only add hashes for completed blocks to recent blockhashes

* feedback

core/src/replay_stage.rs

@@ -5879,7 +5879,7 @@ pub mod tests {
 
         // Simulate landing a vote for slot 0 landing in slot 1
         let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
-        bank1.fill_bank_with_ticks();
+        bank1.fill_bank_with_ticks_for_tests();
         tower.record_bank_vote(&bank0, &my_vote_pubkey);
         ReplayStage::push_vote(
             &bank0,
@@ -5919,7 +5919,7 @@ pub mod tests {
         // Trying to refresh the vote for bank 0 in bank 1 or bank 2 won't succeed because
         // the last vote has landed already
         let bank2 = Arc::new(Bank::new_from_parent(&bank1, &Pubkey::default(), 2));
-        bank2.fill_bank_with_ticks();
+        bank2.fill_bank_with_ticks_for_tests();
         bank2.freeze();
         for refresh_bank in &[&bank1, &bank2] {
             ReplayStage::refresh_last_vote(
@@ -6001,13 +6001,19 @@ pub mod tests {
         assert_eq!(tower.last_voted_slot().unwrap(), 1);
 
         // Create a bank where the last vote transaction will have expired
-        let expired_bank = Arc::new(Bank::new_from_parent(
-            &bank2,
-            &Pubkey::default(),
-            bank2.slot() + MAX_PROCESSING_AGE as Slot,
-        ));
-        expired_bank.fill_bank_with_ticks();
-        expired_bank.freeze();
+        let expired_bank = {
+            let mut parent_bank = bank2.clone();
+            for _ in 0..MAX_PROCESSING_AGE {
+                parent_bank = Arc::new(Bank::new_from_parent(
+                    &parent_bank,
+                    &Pubkey::default(),
+                    parent_bank.slot() + 1,
+                ));
+                parent_bank.fill_bank_with_ticks_for_tests();
+                parent_bank.freeze();
+            }
+            parent_bank
+        };
 
         // Now trying to refresh the vote for slot 1 will succeed because the recent blockhash
         // of the last vote transaction has expired
@@ -6070,7 +6076,7 @@ pub mod tests {
             vote_account.vote_state().as_ref().unwrap().tower(),
             vec![0, 1]
         );
-        expired_bank_child.fill_bank_with_ticks();
+        expired_bank_child.fill_bank_with_ticks_for_tests();
         expired_bank_child.freeze();
 
         // Trying to refresh the vote on a sibling bank where:
@@ -6082,7 +6088,7 @@ pub mod tests {
             &Pubkey::default(),
             expired_bank_child.slot() + 1,
         ));
-        expired_bank_sibling.fill_bank_with_ticks();
+        expired_bank_sibling.fill_bank_with_ticks_for_tests();
         expired_bank_sibling.freeze();
         // Set the last refresh to now, shouldn't refresh because the last refresh just happened.
         last_vote_refresh_time.last_refresh_time = Instant::now();

ledger/src/blockstore_processor.rs

@@ -3951,7 +3951,7 @@ pub mod tests {
     }
 
     #[test]
-    fn test_confirm_slot_entries() {
+    fn test_confirm_slot_entries_without_fix() {
         const HASHES_PER_TICK: u64 = 10;
         const TICKS_PER_SLOT: u64 = 2;
 
@@ -3966,7 +3966,9 @@ pub mod tests {
         genesis_config.ticks_per_slot = TICKS_PER_SLOT;
         let genesis_hash = genesis_config.hash();
 
-        let slot_0_bank = Arc::new(Bank::new_for_tests(&genesis_config));
+        let mut slot_0_bank = Bank::new_for_tests(&genesis_config);
+        slot_0_bank.deactivate_feature(&feature_set::fix_recent_blockhashes::id());
+        let slot_0_bank = Arc::new(slot_0_bank);
         assert_eq!(slot_0_bank.slot(), 0);
         assert_eq!(slot_0_bank.tick_height(), 0);
         assert_eq!(slot_0_bank.max_tick_height(), 2);
@@ -4033,4 +4035,124 @@ pub mod tests {
         assert_eq!(slot_2_bank.get_hash_age(&slot_1_hash), Some(1));
         assert_eq!(slot_2_bank.get_hash_age(&slot_2_hash), Some(0));
     }
+
+    #[test]
+    fn test_confirm_slot_entries_with_fix() {
+        const HASHES_PER_TICK: u64 = 10;
+        const TICKS_PER_SLOT: u64 = 2;
+
+        let collector_id = Pubkey::new_unique();
+
+        let GenesisConfigInfo {
+            mut genesis_config,
+            mint_keypair,
+            ..
+        } = create_genesis_config(10_000);
+        genesis_config.poh_config.hashes_per_tick = Some(HASHES_PER_TICK);
+        genesis_config.ticks_per_slot = TICKS_PER_SLOT;
+        let genesis_hash = genesis_config.hash();
+
+        let slot_0_bank = Arc::new(Bank::new_for_tests(&genesis_config));
+        assert_eq!(slot_0_bank.slot(), 0);
+        assert_eq!(slot_0_bank.tick_height(), 0);
+        assert_eq!(slot_0_bank.max_tick_height(), 2);
+        assert_eq!(slot_0_bank.last_blockhash(), genesis_hash);
+        assert_eq!(slot_0_bank.get_hash_age(&genesis_hash), Some(0));
+
+        let slot_0_entries = entry::create_ticks(TICKS_PER_SLOT, HASHES_PER_TICK, genesis_hash);
+        let slot_0_hash = slot_0_entries.last().unwrap().hash;
+        confirm_slot_entries_for_tests(&slot_0_bank, slot_0_entries, true, genesis_hash).unwrap();
+        assert_eq!(slot_0_bank.tick_height(), slot_0_bank.max_tick_height());
+        assert_eq!(slot_0_bank.last_blockhash(), slot_0_hash);
+        assert_eq!(slot_0_bank.get_hash_age(&genesis_hash), Some(1));
+        assert_eq!(slot_0_bank.get_hash_age(&slot_0_hash), Some(0));
+
+        let slot_2_bank = Arc::new(Bank::new_from_parent(&slot_0_bank, &collector_id, 2));
+        assert_eq!(slot_2_bank.slot(), 2);
+        assert_eq!(slot_2_bank.tick_height(), 2);
+        assert_eq!(slot_2_bank.max_tick_height(), 6);
+        assert_eq!(slot_2_bank.last_blockhash(), slot_0_hash);
+
+        let slot_1_entries = entry::create_ticks(TICKS_PER_SLOT, HASHES_PER_TICK, slot_0_hash);
+        let slot_1_hash = slot_1_entries.last().unwrap().hash;
+        confirm_slot_entries_for_tests(&slot_2_bank, slot_1_entries, false, slot_0_hash).unwrap();
+        assert_eq!(slot_2_bank.tick_height(), 4);
+        assert_eq!(slot_2_bank.last_blockhash(), slot_0_hash);
+        assert_eq!(slot_2_bank.get_hash_age(&genesis_hash), Some(1));
+        assert_eq!(slot_2_bank.get_hash_age(&slot_0_hash), Some(0));
+
+        struct TestCase {
+            recent_blockhash: Hash,
+            expected_result: result::Result<(), BlockstoreProcessorError>,
+        }
+
+        let test_cases = [
+            TestCase {
+                recent_blockhash: slot_1_hash,
+                expected_result: Err(BlockstoreProcessorError::InvalidTransaction(
+                    TransactionError::BlockhashNotFound,
+                )),
+            },
+            TestCase {
+                recent_blockhash: slot_0_hash,
+                expected_result: Ok(()),
+            },
+        ];
+
+        // Check that slot 2 transactions can only use hashes for completed blocks.
+        for TestCase {
+            recent_blockhash,
+            expected_result,
+        } in test_cases
+        {
+            let slot_2_entries = {
+                let to_pubkey = Pubkey::new_unique();
+                let mut prev_entry_hash = slot_1_hash;
+                let mut remaining_entry_hashes = HASHES_PER_TICK;
+
+                let tx =
+                    system_transaction::transfer(&mint_keypair, &to_pubkey, 1, recent_blockhash);
+                remaining_entry_hashes = remaining_entry_hashes.checked_sub(1).unwrap();
+                let mut entries = vec![next_entry_mut(&mut prev_entry_hash, 1, vec![tx])];
+
+                entries.push(next_entry_mut(
+                    &mut prev_entry_hash,
+                    remaining_entry_hashes,
+                    vec![],
+                ));
+                entries.push(next_entry_mut(
+                    &mut prev_entry_hash,
+                    HASHES_PER_TICK,
+                    vec![],
+                ));
+
+                entries
+            };
+
+            let slot_2_hash = slot_2_entries.last().unwrap().hash;
+            let result =
+                confirm_slot_entries_for_tests(&slot_2_bank, slot_2_entries, true, slot_1_hash);
+            match (result, expected_result) {
+                (Ok(()), Ok(())) => {
+                    assert_eq!(slot_2_bank.tick_height(), slot_2_bank.max_tick_height());
+                    assert_eq!(slot_2_bank.last_blockhash(), slot_2_hash);
+                    assert_eq!(slot_2_bank.get_hash_age(&genesis_hash), Some(2));
+                    assert_eq!(slot_2_bank.get_hash_age(&slot_0_hash), Some(1));
+                    assert_eq!(slot_2_bank.get_hash_age(&slot_2_hash), Some(0));
+                }
+                (
+                    Err(BlockstoreProcessorError::InvalidTransaction(err)),
+                    Err(BlockstoreProcessorError::InvalidTransaction(expected_err)),
+                ) => {
+                    assert_eq!(err, expected_err);
+                }
+                (result, expected_result) => {
+                    panic!(
+                        "actual result {:?} != expected result {:?}",
+                        result, expected_result
+                    );
+                }
+            }
+        }
+    }
 }

poh/src/poh_recorder.rs

@@ -1127,7 +1127,7 @@ mod tests {
                 Arc::new(AtomicBool::default()),
             );
 
-            bank0.fill_bank_with_ticks();
+            bank0.fill_bank_with_ticks_for_tests();
             let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
 
             // Set a working bank
@@ -1239,7 +1239,7 @@ mod tests {
                 Arc::new(AtomicBool::default()),
             );
 
-            bank0.fill_bank_with_ticks();
+            bank0.fill_bank_with_ticks_for_tests();
             let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
             poh_recorder.set_bank(&bank1);
             // Let poh_recorder tick up to bank1.tick_height() - 1
@@ -1324,7 +1324,7 @@ mod tests {
                 Arc::new(AtomicBool::default()),
             );
 
-            bank0.fill_bank_with_ticks();
+            bank0.fill_bank_with_ticks_for_tests();
             let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
             poh_recorder.set_bank(&bank1);
 
@@ -1420,7 +1420,7 @@ mod tests {
                 Arc::new(AtomicBool::default()),
             );
 
-            bank0.fill_bank_with_ticks();
+            bank0.fill_bank_with_ticks_for_tests();
             let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
             poh_recorder.set_bank(&bank1);
 
@@ -1962,12 +1962,13 @@ mod tests {
             );
             //create a new bank
             let bank = Arc::new(Bank::new_from_parent(&bank, &Pubkey::default(), 2));
-            //put 2 slots worth of virtual ticks into poh
-            for _ in 0..(bank.ticks_per_slot() * 2) {
+            // add virtual ticks into poh for slots 0, 1, and 2
+            for _ in 0..(bank.ticks_per_slot() * 3) {
                 poh_recorder.tick();
             }
             poh_recorder.set_bank(&bank);
-            assert!(!bank.is_hash_valid_for_age(&genesis_hash, 1));
+            assert!(!bank.is_hash_valid_for_age(&genesis_hash, 0));
+            assert!(bank.is_hash_valid_for_age(&genesis_hash, 1));
         }
     }