Fix Bank accounts hash mismatch related to Clock::unix_timestamp (#13477)

* Test for different ancestors with mismatch bank hash

* Test cleanup

* Remove nondeterministic ancestor check

* Update timestamp bounding feature key

* Update design doc

* Filter recent_timestamps to nodes voting within the last epoch

Co-authored-by: Stephen Akridge <sakridge@gmail.com>

docs/src/implemented-proposals/bank-timestamp-correction.md

@@ -36,10 +36,8 @@ correction.
 In order to calculate the estimated timestamp for a particular Bank, the runtime
 first needs to get the most recent vote timestamps from the active validator
 set. The `Bank::vote_accounts()` method provides the vote accounts state, and
-these can be filtered to all accounts whose most recent timestamp is for an
-ancestor slot back to the current root. This should guarantee 2/3+ of the
-current cluster stake is represented, since by definition, roots must be
-confirmed by 2/3+ stake.
+these can be filtered to all accounts whose most recent timestamp was provided
+within the last epoch.
 
 From each vote timestamp, an estimate for the current Bank is calculated using
 the epoch's target ns_per_slot for any delta between the Bank slot and the

runtime/src/bank.rs

@@ -1548,7 +1548,8 @@ impl Bank {
                     if (self
                         .feature_set
                         .is_active(&feature_set::timestamp_bounding::id())
-                        && self.ancestors.contains_key(&timestamp_slot))
+                        && self.slot().checked_sub(timestamp_slot)?
+                            <= self.epoch_schedule().slots_per_epoch)
                         || self.slot().checked_sub(timestamp_slot)?
                             <= DEPRECATED_TIMESTAMP_SLOT_RANGE as u64
                     {
@@ -4259,7 +4260,7 @@ pub fn goto_end_of_slot(bank: &mut Bank) {
 }
 
 #[cfg(test)]
-mod tests {
+pub(crate) mod tests {
     use super::*;
     use crate::{
         accounts_index::{AccountMap, Ancestors},
@@ -9847,7 +9848,11 @@ mod tests {
         assert_eq!(bank.capitalization(), original_capitalization - 100);
     }
 
-    fn update_vote_account_timestamp(timestamp: BlockTimestamp, bank: &Bank, vote_pubkey: &Pubkey) {
+    pub fn update_vote_account_timestamp(
+        timestamp: BlockTimestamp,
+        bank: &Bank,
+        vote_pubkey: &Pubkey,
+    ) {
         let mut vote_account = bank.get_account(vote_pubkey).unwrap_or_default();
         let mut vote_state = VoteState::from(&vote_account).unwrap_or_default();
         vote_state.last_timestamp = timestamp;

runtime/src/bank_forks.rs

@@ -292,9 +292,21 @@ impl BankForks {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::genesis_utils::{create_genesis_config, GenesisConfigInfo};
+    use crate::{
+        bank::tests::update_vote_account_timestamp,
+        genesis_utils::{
+            create_genesis_config, create_genesis_config_with_leader, GenesisConfigInfo,
+        },
+    };
     use solana_sdk::hash::Hash;
-    use solana_sdk::pubkey::Pubkey;
+    use solana_sdk::{
+        clock::UnixTimestamp,
+        pubkey::Pubkey,
+        signature::{Keypair, Signer},
+        stake_weighted_timestamp::DEPRECATED_TIMESTAMP_SLOT_RANGE,
+        sysvar::epoch_schedule::EpochSchedule,
+    };
+    use solana_vote_program::vote_state::BlockTimestamp;
 
     #[test]
     fn test_bank_forks_new() {
@@ -378,4 +390,67 @@ mod tests {
         bank_forks.insert(child_bank);
         assert_eq!(bank_forks.active_banks(), vec![1]);
     }
+
+    #[test]
+    fn test_bank_forks_different_set_root() {
+        solana_logger::setup();
+        let leader_keypair = Keypair::new();
+        let GenesisConfigInfo {
+            mut genesis_config,
+            mint_keypair: _,
+            voting_keypair,
+        } = create_genesis_config_with_leader(10_000, &leader_keypair.pubkey(), 1_000);
+        let slots_in_epoch = 32;
+        genesis_config.epoch_schedule = EpochSchedule::new(slots_in_epoch);
+
+        let bank0 = Bank::new(&genesis_config);
+        let mut bank_forks0 = BankForks::new(bank0);
+        bank_forks0.set_root(0, &None, None);
+
+        let bank1 = Bank::new(&genesis_config);
+        let mut bank_forks1 = BankForks::new(bank1);
+
+        let additional_timestamp_secs = 2;
+
+        let num_slots = slots_in_epoch + 1 // Advance past first epoch boundary
+            + DEPRECATED_TIMESTAMP_SLOT_RANGE as u64 + 1; // ... and past deprecated slot range
+        for slot in 1..num_slots {
+            // Just after the epoch boundary, timestamp a vote that will shift
+            // Clock::unix_timestamp from Bank::unix_timestamp_from_genesis()
+            let update_timestamp_case = slot == slots_in_epoch;
+
+            let child1 = Bank::new_from_parent(&bank_forks0[slot - 1], &Pubkey::default(), slot);
+            let child2 = Bank::new_from_parent(&bank_forks1[slot - 1], &Pubkey::default(), slot);
+
+            if update_timestamp_case {
+                for child in &[&child1, &child2] {
+                    let recent_timestamp: UnixTimestamp = child.unix_timestamp_from_genesis();
+                    update_vote_account_timestamp(
+                        BlockTimestamp {
+                            slot: child.slot(),
+                            timestamp: recent_timestamp + additional_timestamp_secs,
+                        },
+                        &child,
+                        &voting_keypair.pubkey(),
+                    );
+                }
+            }
+
+            // Set root in bank_forks0 to truncate the ancestor history
+            bank_forks0.insert(child1);
+            bank_forks0.set_root(slot, &None, None);
+
+            // Don't set root in bank_forks1 to keep the ancestor history
+            bank_forks1.insert(child2);
+        }
+        let child1 = &bank_forks0.working_bank();
+        let child2 = &bank_forks1.working_bank();
+
+        child1.freeze();
+        child2.freeze();
+
+        info!("child0.ancestors: {:?}", child1.ancestors);
+        info!("child1.ancestors: {:?}", child2.ancestors);
+        assert_eq!(child1.hash(), child2.hash());
+    }
 }

sdk/src/feature_set.rs

@@ -79,7 +79,7 @@ pub mod pull_request_ping_pong_check {
 }
 
 pub mod timestamp_bounding {
-    solana_sdk::declare_id!("8FyEA6ABYiMxX7Az6AopQN3mavLD8Rz3N4bvKnbbBFFq");
+    solana_sdk::declare_id!("2cGj3HJYPhBrtQizd7YbBxEsifFs5qhzabyFjUAp6dBa");
 }
 
 pub mod stake_program_v2 {