From cd96d5b6c3fcb80905648261567e893a94e16f5a Mon Sep 17 00:00:00 2001 From: steviez Date: Tue, 22 Aug 2023 22:33:03 +0200 Subject: [PATCH] Only update last restart slot sysvar if value has changed (#32925) Read account first to check for change in value --- runtime/src/bank.rs | 26 ++++++++++--- runtime/src/bank/tests.rs | 77 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cf025b477..4722e6519 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2116,6 +2116,16 @@ impl Bank { .is_active(&feature_set::last_restart_slot_sysvar::id()); if feature_flag { + // First, see what the currently stored last restart slot is. This + // account may not exist yet if the feature was just activated. + let current_last_restart_slot = self + .get_account(&sysvar::last_restart_slot::id()) + .and_then(|account| { + let lrs: Option = from_account(&account); + lrs + }) + .map(|account| account.last_restart_slot); + let last_restart_slot = { let slot = self.slot; let hard_forks_r = self.hard_forks.read().unwrap(); @@ -2129,12 +2139,16 @@ impl Bank { .map(|(slot, _)| *slot) .unwrap_or(0) }; - self.update_sysvar_account(&sysvar::last_restart_slot::id(), |account| { - create_account( - &LastRestartSlot { last_restart_slot }, - self.inherit_specially_retained_account_fields(account), - ) - }); + + // Only need to write if the last restart has changed + if current_last_restart_slot != Some(last_restart_slot) { + self.update_sysvar_account(&sysvar::last_restart_slot::id(), |account| { + create_account( + &LastRestartSlot { last_restart_slot }, + self.inherit_specially_retained_account_fields(account), + ) + }); + } } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index aa8cde079..e4dcc0254 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -13632,3 +13632,80 @@ fn test_calculate_stake_vote_rewards() { expected_reward_info, ); } + +#[test] +fn test_last_restart_slot() { + fn last_restart_slot_dirty(bank: &Bank) -> bool { + let (dirty_accounts, _, _) = bank + .rc + .accounts + .accounts_db + .get_pubkey_hash_for_slot(bank.slot()); + let dirty_accounts: HashSet<_> = dirty_accounts + .into_iter() + .map(|(pubkey, _hash)| pubkey) + .collect(); + dirty_accounts.contains(&sysvar::last_restart_slot::id()) + } + + fn get_last_restart_slot(bank: &Bank) -> Option { + bank.get_account(&sysvar::last_restart_slot::id()) + .and_then(|account| { + let lrs: Option = from_account(&account); + lrs + }) + .map(|account| account.last_restart_slot) + } + + let mint_lamports = 100; + let validator_stake_lamports = 10; + let leader_pubkey = Pubkey::default(); + let GenesisConfigInfo { + mut genesis_config, .. + } = create_genesis_config_with_leader(mint_lamports, &leader_pubkey, validator_stake_lamports); + // Remove last restart slot account so we can simluate its' activation + genesis_config + .accounts + .remove(&feature_set::last_restart_slot_sysvar::id()) + .unwrap(); + + let bank0 = Arc::new(Bank::new_for_tests(&genesis_config)); + // Register a hard fork in the future so last restart slot will update + bank0.register_hard_fork(6); + assert!(!last_restart_slot_dirty(&bank0)); + assert_eq!(get_last_restart_slot(&bank0), None); + + let mut bank1 = Arc::new(Bank::new_from_parent(bank0, &Pubkey::default(), 1)); + assert!(!last_restart_slot_dirty(&bank1)); + assert_eq!(get_last_restart_slot(&bank1), None); + + // Activate the feature in slot 1, it will get initialized in slot 1's children + Arc::get_mut(&mut bank1) + .unwrap() + .activate_feature(&feature_set::last_restart_slot_sysvar::id()); + let bank2 = Arc::new(Bank::new_from_parent(bank1.clone(), &Pubkey::default(), 2)); + assert!(last_restart_slot_dirty(&bank2)); + assert_eq!(get_last_restart_slot(&bank2), Some(0)); + let bank3 = Arc::new(Bank::new_from_parent(bank1, &Pubkey::default(), 3)); + assert!(last_restart_slot_dirty(&bank3)); + assert_eq!(get_last_restart_slot(&bank3), Some(0)); + + // Not dirty in children where the last restart slot has not changed + let bank4 = Arc::new(Bank::new_from_parent(bank2, &Pubkey::default(), 4)); + assert!(!last_restart_slot_dirty(&bank4)); + assert_eq!(get_last_restart_slot(&bank4), Some(0)); + let bank5 = Arc::new(Bank::new_from_parent(bank3, &Pubkey::default(), 5)); + assert!(!last_restart_slot_dirty(&bank5)); + assert_eq!(get_last_restart_slot(&bank5), Some(0)); + + // Last restart slot has now changed so it will be dirty again + let bank6 = Arc::new(Bank::new_from_parent(bank4, &Pubkey::default(), 6)); + assert!(last_restart_slot_dirty(&bank6)); + assert_eq!(get_last_restart_slot(&bank6), Some(6)); + + // Last restart will not change for a hard fork that has not occurred yet + bank6.register_hard_fork(10); + let bank7 = Arc::new(Bank::new_from_parent(bank6, &Pubkey::default(), 7)); + assert!(!last_restart_slot_dirty(&bank7)); + assert_eq!(get_last_restart_slot(&bank7), Some(6)); +}