From c0e2ef06dcc5ade81fc45d4893154118226881c5 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 9 Nov 2020 19:10:09 -0700 Subject: [PATCH] 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 --- .../bank-timestamp-correction.md | 6 +- runtime/src/bank.rs | 11 ++- runtime/src/bank_forks.rs | 79 ++++++++++++++++++- sdk/src/feature_set.rs | 2 +- 4 files changed, 88 insertions(+), 10 deletions(-) diff --git a/docs/src/implemented-proposals/bank-timestamp-correction.md b/docs/src/implemented-proposals/bank-timestamp-correction.md index 74096e88cc..f4ba7a0efb 100644 --- a/docs/src/implemented-proposals/bank-timestamp-correction.md +++ b/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 diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 69e2d88f99..3e5e4ed4ac 100644 --- a/runtime/src/bank.rs +++ b/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(×tamp_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; diff --git a/runtime/src/bank_forks.rs b/runtime/src/bank_forks.rs index 277173b711..21b1e4082f 100644 --- a/runtime/src/bank_forks.rs +++ b/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()); + } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index d24ed861f7..7e6148b177 100644 --- a/sdk/src/feature_set.rs +++ b/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 {