From 90778615f6624c4ef320a0af697dea00872a1242 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Fri, 23 Oct 2020 15:06:57 -0600 Subject: [PATCH] Use bounded timestamp-correction when feature enabled --- core/src/rpc.rs | 1 + runtime/src/bank.rs | 162 ++++++++++++++++++++++++++-- sdk/src/stake_weighted_timestamp.rs | 27 +++-- 3 files changed, 172 insertions(+), 18 deletions(-) diff --git a/core/src/rpc.rs b/core/src/rpc.rs index be257d0148..b944535c82 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -2703,6 +2703,7 @@ pub mod tests { message::Message, nonce, rpc_port, signature::{Keypair, Signer}, + stake_weighted_timestamp::EstimateType, system_program, system_transaction, timing::slot_duration_from_slots_per_year, transaction::{self, TransactionError}, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index fe69435a17..fbc4a826cb 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1081,8 +1081,32 @@ impl Bank { .feature_set .is_active(&feature_set::timestamp_correction::id()) { + let (estimate_type, epoch_start_timestamp) = + if let Some(timestamp_bounding_activation_slot) = self + .feature_set + .activated_slot(&feature_set::timestamp_bounding::id()) + { + // This check avoids a chicken-egg problem with epoch_start_timestamp, which is + // needed for timestamp bounding, but isn't yet corrected for the activation slot + let epoch_start_timestamp = if self.slot() > timestamp_bounding_activation_slot + { + let epoch = if let Some(epoch) = parent_epoch { + epoch + } else { + self.epoch() + }; + let first_slot_in_epoch = + self.epoch_schedule.get_first_slot_in_epoch(epoch); + Some((first_slot_in_epoch, self.clock().epoch_start_timestamp)) + } else { + None + }; + (EstimateType::Bounded, epoch_start_timestamp) + } else { + (EstimateType::Unbounded, None) + }; if let Some(timestamp_estimate) = - self.get_timestamp_estimate(EstimateType::Unbounded, None) + self.get_timestamp_estimate(estimate_type, epoch_start_timestamp) { if timestamp_estimate > unix_timestamp { datapoint_info!( @@ -9729,6 +9753,10 @@ mod tests { .accounts .remove(&feature_set::timestamp_correction::id()) .unwrap(); + genesis_config + .accounts + .remove(&feature_set::timestamp_bounding::id()) + .unwrap(); let bank = Bank::new(&genesis_config); let recent_timestamp: UnixTimestamp = bank.unix_timestamp_from_genesis(); @@ -9744,10 +9772,10 @@ mod tests { // Bank::new_from_parent should not adjust timestamp before feature activation let mut bank = new_from_parent(&Arc::new(bank)); - let clock = - from_account::(&bank.get_account(&sysvar::clock::id()).unwrap()) - .unwrap(); - assert_eq!(clock.unix_timestamp, bank.unix_timestamp_from_genesis()); + assert_eq!( + bank.clock().unix_timestamp, + bank.unix_timestamp_from_genesis() + ); // Request `timestamp_correction` activation bank.store_account( @@ -9763,15 +9791,131 @@ mod tests { // Now Bank::new_from_parent should adjust timestamp let bank = Arc::new(new_from_parent(&Arc::new(bank))); - let clock = - from_account::(&bank.get_account(&sysvar::clock::id()).unwrap()) - .unwrap(); assert_eq!( - clock.unix_timestamp, + bank.clock().unix_timestamp, bank.unix_timestamp_from_genesis() + additional_secs ); } + #[test] + fn test_timestamp_bounding_feature() { + let leader_pubkey = solana_sdk::pubkey::new_rand(); + let GenesisConfigInfo { + mut genesis_config, + voting_keypair, + .. + } = create_genesis_config_with_leader(5, &leader_pubkey, 3); + let slots_in_epoch = 32; + genesis_config + .accounts + .remove(&feature_set::timestamp_bounding::id()) + .unwrap(); + genesis_config.epoch_schedule = EpochSchedule::new(slots_in_epoch); + let bank = Bank::new(&genesis_config); + + let recent_timestamp: UnixTimestamp = bank.unix_timestamp_from_genesis(); + let additional_secs = 1; + update_vote_account_timestamp( + BlockTimestamp { + slot: bank.slot(), + timestamp: recent_timestamp + additional_secs, + }, + &bank, + &voting_keypair.pubkey(), + ); + + // Bank::new_from_parent should allow unbounded timestamp before activation + let mut bank = new_from_parent(&Arc::new(bank)); + assert_eq!( + bank.clock().unix_timestamp, + bank.unix_timestamp_from_genesis() + additional_secs + ); + + // Bank::new_from_parent should not allow epoch_start_timestamp to be set before activation + bank.update_clock(Some(0)); + assert_eq!( + bank.clock().epoch_start_timestamp, + Bank::get_unused_from_slot(bank.slot(), bank.unused) as i64 + ); + + // Request `timestamp_bounding` activation + let feature = Feature { activated_at: None }; + bank.store_account( + &feature_set::timestamp_bounding::id(), + &feature.create_account(42), + ); + for _ in 0..30 { + bank = new_from_parent(&Arc::new(bank)); + } + + // Refresh vote timestamp + let recent_timestamp: UnixTimestamp = bank.unix_timestamp_from_genesis(); + let additional_secs = 1; + update_vote_account_timestamp( + BlockTimestamp { + slot: bank.slot(), + timestamp: recent_timestamp + additional_secs, + }, + &bank, + &voting_keypair.pubkey(), + ); + + // Advance to epoch boundary to activate + bank = new_from_parent(&Arc::new(bank)); + + // Bank::new_from_parent is bounding, but should not use epoch_start_timestamp in activation slot + assert_eq!( + bank.clock().unix_timestamp, + bank.unix_timestamp_from_genesis() + additional_secs + ); + + assert_eq!( + bank.clock().epoch_start_timestamp, + bank.unix_timestamp_from_genesis() + additional_secs + ); + + // Past activation slot, bounding should use epoch_start_timestamp in activation slot + bank = new_from_parent(&Arc::new(bank)); + assert_eq!( + bank.clock().unix_timestamp, + bank.unix_timestamp_from_genesis() + ); + + for _ in 0..30 { + bank = new_from_parent(&Arc::new(bank)); + } + + // Refresh vote timestamp + let recent_timestamp: UnixTimestamp = bank.unix_timestamp_from_genesis(); + let additional_secs = 20; + update_vote_account_timestamp( + BlockTimestamp { + slot: bank.slot(), + timestamp: recent_timestamp + additional_secs, + }, + &bank, + &voting_keypair.pubkey(), + ); + + // Advance to epoch boundary + bank = new_from_parent(&Arc::new(bank)); + + // Past activation slot, bounding should use previous epoch_start_timestamp on epoch boundary slots + assert_eq!( + bank.clock().unix_timestamp, + bank.unix_timestamp_from_genesis() // Plus estimated offset + 25% + + ((slots_in_epoch as u32 * Duration::from_nanos(bank.ns_per_slot as u64)) + .as_secs() + * 25 + / 100) as i64, + ); + + assert_eq!( + bank.clock().epoch_start_timestamp, + bank.clock().unix_timestamp + ); + } + #[test] fn test_update_clock_timestamp() { let leader_pubkey = solana_sdk::pubkey::new_rand(); diff --git a/sdk/src/stake_weighted_timestamp.rs b/sdk/src/stake_weighted_timestamp.rs index c7417ef2b4..54733844fc 100644 --- a/sdk/src/stake_weighted_timestamp.rs +++ b/sdk/src/stake_weighted_timestamp.rs @@ -110,13 +110,22 @@ fn calculate_bounded_stake_weighted_timestamp( let poh_estimate_offset = slot.saturating_sub(epoch_start_slot) as u32 * slot_duration; let estimate_offset = Duration::from_secs(estimate.saturating_sub(epoch_start_timestamp) as u64); - let delta = if estimate_offset > poh_estimate_offset { - estimate_offset - poh_estimate_offset - } else { - poh_estimate_offset - estimate_offset - }; - if delta > poh_estimate_offset * MAX_ALLOWABLE_DRIFT_PERCENTAGE / 100 { - estimate = epoch_start_timestamp + poh_estimate_offset.as_secs() as i64; + let max_allowable_drift = poh_estimate_offset * MAX_ALLOWABLE_DRIFT_PERCENTAGE / 100; + if estimate_offset > poh_estimate_offset + && estimate_offset - poh_estimate_offset > max_allowable_drift + { + // estimate offset since the start of the epoch is higher than + // `MAX_ALLOWABLE_DRIFT_PERCENTAGE` + estimate = epoch_start_timestamp + + poh_estimate_offset.as_secs() as i64 + + max_allowable_drift.as_secs() as i64; + } else if estimate_offset < poh_estimate_offset + && poh_estimate_offset - estimate_offset > max_allowable_drift + { + // estimate offset since the start of the epoch is lower than + // `MAX_ALLOWABLE_DRIFT_PERCENTAGE` + estimate = epoch_start_timestamp + poh_estimate_offset.as_secs() as i64 + - max_allowable_drift.as_secs() as i64; } } Some(estimate) @@ -503,7 +512,7 @@ pub mod tests { Some((0, epoch_start_timestamp)), ) .unwrap(); - assert_eq!(bounded, poh_estimate); + assert_eq!(bounded, poh_estimate + acceptable_delta); // Test when stake-weighted median is too low let unique_timestamps: HashMap = [ @@ -523,7 +532,7 @@ pub mod tests { Some((0, epoch_start_timestamp)), ) .unwrap(); - assert_eq!(bounded, poh_estimate); + assert_eq!(bounded, poh_estimate - acceptable_delta); // Test stake-weighted median within bounds let unique_timestamps: HashMap = [