Skip to content

Commit

Permalink
Fix - Bank::compute_active_feature_set() and `Bank::apply_feature_a…
Browse files Browse the repository at this point in the history
…ctivations()` (#34124)

* Moves modification of feature accounts from Bank::compute_active_feature_set() into Bank::apply_feature_activations().

* Renames allow_new_activations and newly_activated to include_pending and pending.

* Fix test_compute_active_feature_set.

(cherry picked from commit 6b85450)
  • Loading branch information
Lichtso authored and mergify[bot] committed Nov 17, 2023
1 parent a9d5a5b commit 3157d63
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 33 deletions.
50 changes: 26 additions & 24 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7892,6 +7892,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
Expand Down Expand Up @@ -7960,38 +7973,27 @@ 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,
allow_new_activations: bool,
) -> (FeatureSet, HashSet<Pubkey>) {
fn compute_active_feature_set(&self, include_pending: bool) -> (FeatureSet, HashSet<Pubkey>) {
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 {
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 include_pending => {
// Feature activation is pending
pending.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);
}
_ => {}
}
}
}
Expand All @@ -8002,7 +8004,7 @@ impl Bank {
}
}

(FeatureSet { active, inactive }, newly_activated)
(FeatureSet { active, inactive }, pending)
}

fn apply_builtin_program_feature_transitions(
Expand Down
19 changes: 10 additions & 9 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7740,25 +7740,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));
Expand Down

0 comments on commit 3157d63

Please sign in to comment.