From 63d1c3efbb0ea3e5a304de1287e1acb9b49f489e Mon Sep 17 00:00:00 2001 From: Jack May Date: Fri, 18 Feb 2022 09:29:06 -0800 Subject: [PATCH] consistent owner across builtins and precompiles + nits --- runtime/src/bank.rs | 16 +++--- runtime/src/builtins.rs | 118 +++++++++++++++++++--------------------- 2 files changed, 63 insertions(+), 71 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 034b6171ebe926..fedfd6d61e43ee 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3220,7 +3220,7 @@ impl Bank { let (lamports, rent_epoch) = self.inherit_specially_retained_account_fields(&None); let account = AccountSharedData::from(Account { lamports, - owner: solana_sdk::system_program::id(), + owner: native_loader::id(), data: vec![], executable: true, rent_epoch, @@ -6432,9 +6432,9 @@ impl Bank { } if !debug_do_not_add_builtins { - let apply_transitions_for_old_features = init_finish_or_warp; + let apply_transitions_for_new_features = !init_finish_or_warp; self.apply_builtin_program_feature_transitions( - apply_transitions_for_old_features, + apply_transitions_for_new_features, &new_feature_activations, ); self.reconfigure_token2_native_mint(); @@ -6495,15 +6495,15 @@ impl Bank { fn apply_builtin_program_feature_transitions( &mut self, - apply_transitions_for_old_features: bool, + apply_transitions_for_new_features: bool, new_feature_activations: &HashSet, ) { let feature_set = self.feature_set.clone(); let should_apply_action_for_feature = |feature_id: &Pubkey| -> bool { - if apply_transitions_for_old_features { - feature_set.is_active(feature_id) - } else { + if apply_transitions_for_new_features { new_feature_activations.contains(feature_id) + } else { + feature_set.is_active(feature_id) } }; @@ -6516,7 +6516,7 @@ impl Bank { &builtin.id, builtin.process_instruction_with_context, ), - BuiltinAction::Remove { program_id } => self.remove_builtin(&program_id), + BuiltinAction::Remove(program_id) => self.remove_builtin(&program_id), } } } diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index 226f8bf055da31..f19db3fff3c324 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -51,63 +51,6 @@ macro_rules! with_program_logging { }; } -/// State transition enum used for adding and removing builtin programs through -/// feature activations. -#[derive(Debug, Clone)] -pub enum BuiltinFeatureTransition { - /// Add a builtin program if a feature is activated. - Add { - builtin: Builtin, - feature_id: Pubkey, - }, - /// Remove a builtin program if a feature is activated or - /// retain a previously added builtin. - RemoveOrRetain { - previous_builtin: Builtin, - removal_feature_id: Pubkey, - }, -} - -/// Actions taken by a bank when managing the list of active builtin programs. -#[derive(Debug, Clone)] -pub enum BuiltinAction { - Add(Builtin), - Remove { program_id: Pubkey }, -} - -impl BuiltinFeatureTransition { - pub fn to_action( - &self, - should_apply_action_for_feature: &impl Fn(&Pubkey) -> bool, - ) -> Option { - match self { - Self::Add { - builtin, - feature_id, - } => { - if should_apply_action_for_feature(feature_id) { - Some(BuiltinAction::Add(builtin.clone())) - } else { - None - } - } - Self::RemoveOrRetain { - previous_builtin, - removal_feature_id, - } => { - if should_apply_action_for_feature(removal_feature_id) { - Some(BuiltinAction::Remove { - program_id: previous_builtin.id, - }) - } else { - // Retaining is no different from adding a new builtin. - Some(BuiltinAction::Add(previous_builtin.clone())) - } - } - } - } -} - #[derive(Clone)] pub struct Builtin { pub name: String, @@ -155,6 +98,61 @@ pub struct Builtins { pub feature_transitions: Vec, } +/// Actions taken by a bank when managing the list of active builtin programs. +#[derive(Debug, Clone)] +pub enum BuiltinAction { + Add(Builtin), + Remove(Pubkey), +} + +/// State transition enum used for adding and removing builtin programs through +/// feature activations. +#[derive(Debug, Clone)] +pub enum BuiltinFeatureTransition { + /// Add a builtin program if a feature is activated. + Add { + builtin: Builtin, + feature_id: Pubkey, + }, + /// Remove a builtin program if a feature is activated or + /// retain a previously added builtin. + RemoveOrRetain { + previous_builtin: Builtin, + removal_feature_id: Pubkey, + }, +} + +impl BuiltinFeatureTransition { + pub fn to_action( + &self, + should_apply_action_for_feature: &impl Fn(&Pubkey) -> bool, + ) -> Option { + match self { + Self::Add { + builtin, + feature_id, + } => { + if should_apply_action_for_feature(feature_id) { + Some(BuiltinAction::Add(builtin.clone())) + } else { + None + } + } + Self::RemoveOrRetain { + previous_builtin, + removal_feature_id, + } => { + if should_apply_action_for_feature(removal_feature_id) { + Some(BuiltinAction::Remove(previous_builtin.id)) + } else { + // Retaining is no different from adding a new builtin. + Some(BuiltinAction::Add(previous_builtin.clone())) + } + } + } + } +} + /// Builtin programs that are always available fn genesis_builtins() -> Vec { vec![ @@ -201,9 +199,6 @@ fn builtin_feature_transitions() -> Vec { ), feature_id: feature_set::add_compute_budget_program::id(), }, - // TODO when feature `prevent_calling_precompiles_as_programs` is - // cleaned up also remove "secp256k1_program" from the main builtins - // list BuiltinFeatureTransition::RemoveOrRetain { previous_builtin: Builtin::new( "secp256k1_program", @@ -212,9 +207,6 @@ fn builtin_feature_transitions() -> Vec { ), removal_feature_id: feature_set::prevent_calling_precompiles_as_programs::id(), }, - // TODO when feature `prevent_calling_precompiles_as_programs` is - // cleaned up also remove "ed25519_program" from the main builtins - // list BuiltinFeatureTransition::RemoveOrRetain { previous_builtin: Builtin::new( "ed25519_program",