From a730a7233bfc780a95f3e7a07e8d7ed04a8a3b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 16 Nov 2023 20:43:40 +0000 Subject: [PATCH 1/3] Moves modification of feature accounts from Bank::compute_active_feature_set() into Bank::apply_feature_activations(). --- runtime/src/bank.rs | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index bfe3e7083fb5cb..fca087ebbe8d9a 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -7966,6 +7966,19 @@ impl Bank { self.compute_active_feature_set(allow_new_activations); self.feature_set = Arc::new(feature_set); + // Update activation slot of features in `new_feature_activations` + for feature_id in new_feature_activations.iter() { + if let Some(mut account) = self.get_account_with_fixed_root(feature_id) { + if let Some(mut feature) = feature::from_account(&account) { + feature.activated_at = Some(self.slot()); + if feature::to_account(&feature, &mut account).is_some() { + self.store_account(feature_id, &account); + } + info!("Feature {} activated at slot {}", feature_id, self.slot()); + } + } + } + if new_feature_activations.contains(&feature_set::pico_inflation::id()) { *self.inflation.write().unwrap() = Inflation::pico(); self.fee_rate_governor.burn_percent = 50; // 50% fee burn @@ -8035,7 +8048,7 @@ impl Bank { /// Compute the active feature set based on the current bank state, /// and return it together with the set of newly activated features. fn compute_active_feature_set( - &mut self, + &self, allow_new_activations: bool, ) -> (FeatureSet, HashSet) { let mut active = self.feature_set.active.clone(); @@ -8045,27 +8058,19 @@ impl Bank { for feature_id in &self.feature_set.inactive { let mut activated = None; - if let Some(mut account) = self.get_account_with_fixed_root(feature_id) { - if let Some(mut feature) = feature::from_account(&account) { + if let Some(account) = self.get_account_with_fixed_root(feature_id) { + if let Some(feature) = feature::from_account(&account) { match feature.activated_at { - None => { - if allow_new_activations { - // Feature has been requested, activate it now - feature.activated_at = Some(slot); - if feature::to_account(&feature, &mut account).is_some() { - self.store_account(feature_id, &account); - } - newly_activated.insert(*feature_id); - activated = Some(slot); - info!("Feature {} activated at slot {}", feature_id, slot); - } + None if allow_new_activations => { + // Feature activation is pending + newly_activated.insert(*feature_id); + activated = Some(slot); } - Some(activation_slot) => { - if slot >= activation_slot { - // Feature is already active - activated = Some(activation_slot); - } + Some(activation_slot) if slot >= activation_slot => { + // Feature has been activated already + activated = Some(activation_slot); } + _ => {} } } } From c505817d982a57d6bb87a64cf58d3e6b38e78975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 16 Nov 2023 20:51:04 +0000 Subject: [PATCH 2/3] Renames allow_new_activations and newly_activated to include_pending and pending. --- runtime/src/bank.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index fca087ebbe8d9a..871c314554c6ed 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8047,13 +8047,10 @@ impl Bank { /// Compute the active feature set based on the current bank state, /// and return it together with the set of newly activated features. - fn compute_active_feature_set( - &self, - allow_new_activations: bool, - ) -> (FeatureSet, HashSet) { + fn compute_active_feature_set(&self, include_pending: bool) -> (FeatureSet, HashSet) { let mut active = self.feature_set.active.clone(); let mut inactive = HashSet::new(); - let mut newly_activated = HashSet::new(); + let mut pending = HashSet::new(); let slot = self.slot(); for feature_id in &self.feature_set.inactive { @@ -8061,9 +8058,9 @@ impl Bank { if let Some(account) = self.get_account_with_fixed_root(feature_id) { if let Some(feature) = feature::from_account(&account) { match feature.activated_at { - None if allow_new_activations => { + None if include_pending => { // Feature activation is pending - newly_activated.insert(*feature_id); + pending.insert(*feature_id); activated = Some(slot); } Some(activation_slot) if slot >= activation_slot => { @@ -8081,7 +8078,7 @@ impl Bank { } } - (FeatureSet { active, inactive }, newly_activated) + (FeatureSet { active, inactive }, pending) } fn apply_builtin_program_feature_transitions( From 0942241839bb16758d5fa2176746b49c1366dce6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 16 Nov 2023 22:48:05 +0000 Subject: [PATCH 3/3] Fix test_compute_active_feature_set. --- runtime/src/bank/tests.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 5851f3a2bb3d8a..23f3a9c7483594 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7749,25 +7749,26 @@ fn test_compute_active_feature_set() { let feature = Feature::default(); assert_eq!(feature.activated_at, None); bank.store_account(&test_feature, &feature::create_account(&feature, 42)); + let feature = feature::from_account(&bank.get_account(&test_feature).expect("get_account")) + .expect("from_account"); + assert_eq!(feature.activated_at, None); - // Run `compute_active_feature_set` disallowing new activations + // Run `compute_active_feature_set` excluding pending activation let (feature_set, new_activations) = bank.compute_active_feature_set(false); assert!(new_activations.is_empty()); assert!(!feature_set.is_active(&test_feature)); - let feature = feature::from_account(&bank.get_account(&test_feature).expect("get_account")) - .expect("from_account"); - assert_eq!(feature.activated_at, None); - // Run `compute_active_feature_set` allowing new activations - let (feature_set, new_activations) = bank.compute_active_feature_set(true); + // Run `compute_active_feature_set` including pending activation + let (_feature_set, new_activations) = bank.compute_active_feature_set(true); assert_eq!(new_activations.len(), 1); - assert!(feature_set.is_active(&test_feature)); + assert!(new_activations.contains(&test_feature)); + + // Actually activate the pending activation + bank.apply_feature_activations(ApplyFeatureActivationsCaller::NewFromParent, true); let feature = feature::from_account(&bank.get_account(&test_feature).expect("get_account")) .expect("from_account"); assert_eq!(feature.activated_at, Some(1)); - // Running `compute_active_feature_set` will not cause new activations, but - // `test_feature` is now be active let (feature_set, new_activations) = bank.compute_active_feature_set(true); assert!(new_activations.is_empty()); assert!(feature_set.is_active(&test_feature));