Refactoring - Remove redundant definition of BuiltinProgram (#31429)

* Replaces BuiltinProgram in the program-runtime with the one from solana_rbpf.

* Adjusts the runtimes built-ins to use Arc<LoadedProgram>.

* Adjusts the tests and benchmarks.

ledger/src/blockstore_processor.rs

@@ -1809,7 +1809,7 @@ pub mod tests {
         matches::assert_matches,
         rand::{thread_rng, Rng},
         solana_entry::entry::{create_ticks, next_entry, next_entry_mut},
-        solana_program_runtime::declare_process_instruction,
+        solana_program_runtime::{builtin_program::create_builtin, declare_process_instruction},
         solana_runtime::{
             genesis_utils::{
                 self, create_genesis_config_with_vote_accounts, ValidatorVoteKeypairs,
@@ -2971,7 +2971,10 @@ pub mod tests {
         let mock_program_id = solana_sdk::pubkey::new_rand();
 
         let mut bank = Bank::new_for_tests(&genesis_config);
-        bank.add_builtin("mock_processor", &mock_program_id, mock_processor_ok);
+        bank.add_builtin(
+            mock_program_id,
+            create_builtin("mockup".to_string(), mock_processor_ok),
+        );
 
         let tx = Transaction::new_signed_with_payer(
             &[Instruction::new_with_bincode(
@@ -3012,7 +3015,10 @@ pub mod tests {
 
         (0..get_instruction_errors().len()).for_each(|err| {
             let mut bank = Bank::new_for_tests(&genesis_config);
-            bank.add_builtin("mock_processor", &mock_program_id, mock_processor_err);
+            bank.add_builtin(
+                mock_program_id,
+                create_builtin("mockup".to_string(), mock_processor_err),
+            );
 
             let tx = Transaction::new_signed_with_payer(
                 &[Instruction::new_with_bincode(

program-runtime/src/builtin_program.rs

@@ -1,44 +1,28 @@
 #[cfg(RUSTC_WITH_SPECIALIZATION)]
-use {crate::declare_process_instruction, solana_frozen_abi::abi_example::AbiExample};
+use solana_frozen_abi::abi_example::AbiExample;
 use {
-    crate::invoke_context::InvokeContext, solana_rbpf::vm::BuiltInFunction,
+    crate::{invoke_context::InvokeContext, loaded_programs::LoadedProgram},
+    solana_rbpf::vm::{BuiltInFunction, BuiltInProgram},
     solana_sdk::pubkey::Pubkey,
+    std::sync::Arc,
 };
 
 pub type ProcessInstructionWithContext = BuiltInFunction<InvokeContext<'static>>;
 
-#[derive(Clone)]
-pub struct BuiltinProgram {
-    pub name: String,
-    pub program_id: Pubkey,
-    pub process_instruction: ProcessInstructionWithContext,
-}
-
-impl std::fmt::Debug for BuiltinProgram {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "Builtin [name={}, id={}]", self.name, self.program_id)
-    }
-}
-
-#[cfg(RUSTC_WITH_SPECIALIZATION)]
-impl AbiExample for BuiltinProgram {
-    fn example() -> Self {
-        declare_process_instruction!(empty_mock_process_instruction, 1, |_invoke_context| {
-            // Do nothing
-            Ok(())
-        });
-
-        Self {
-            name: String::default(),
-            program_id: Pubkey::default(),
-            process_instruction: empty_mock_process_instruction,
-        }
-    }
+pub fn create_builtin(
+    name: String,
+    process_instruction: ProcessInstructionWithContext,
+) -> Arc<LoadedProgram> {
+    let mut program = BuiltInProgram::default();
+    program
+        .register_function_by_name("entrypoint", process_instruction)
+        .unwrap();
+    Arc::new(LoadedProgram::new_builtin(name, 0, program))
 }
 
 #[derive(Debug, Clone, Default)]
 pub struct BuiltinPrograms {
-    pub vec: Vec<BuiltinProgram>,
+    pub vec: Vec<(Pubkey, Arc<LoadedProgram>)>,
 }
 
 #[cfg(RUSTC_WITH_SPECIALIZATION)]
@@ -54,11 +38,10 @@ impl BuiltinPrograms {
         process_instruction: ProcessInstructionWithContext,
     ) -> Self {
         Self {
-            vec: vec![BuiltinProgram {
-                name: "mock instruction processor".to_string(),
+            vec: vec![(
                 program_id,
-                process_instruction,
-            }],
+                create_builtin("mockup".to_string(), process_instruction),
+            )],
         }
     }
 }

program-runtime/src/invoke_context.rs

@@ -5,6 +5,7 @@ use {
         compute_budget::ComputeBudget,
         executor_cache::TransactionExecutorCache,
         ic_logger_msg, ic_msg,
+        loaded_programs::LoadedProgramType,
         log_collector::LogCollector,
         pre_account::PreAccount,
         stable_log,
@@ -40,7 +41,7 @@ use {
         cell::RefCell,
         fmt::{self, Debug},
         rc::Rc,
-        sync::Arc,
+        sync::{atomic::Ordering, Arc},
     },
 };
 
@@ -719,19 +720,29 @@ impl<'a> InvokeContext<'a> {
         };
 
         for entry in self.builtin_programs.vec.iter() {
-            if entry.program_id == builtin_id {
+            if entry.0 == builtin_id {
+                // The Murmur3 hash value (used by RBPF) of the string "entrypoint"
+                const ENTRYPOINT_KEY: u32 = 0x71E3CF81;
+                let process_instruction = match &entry.1.program {
+                    LoadedProgramType::Builtin(_name, program) => program
+                        .lookup_function(ENTRYPOINT_KEY)
+                        .map(|(_name, process_instruction)| process_instruction),
+                    _ => None,
+                }
+                .ok_or(InstructionError::GenericError)?;
+                entry.1.usage_counter.fetch_add(1, Ordering::Relaxed);
+
                 let program_id =
                     *instruction_context.get_last_program_key(self.transaction_context)?;
                 self.transaction_context
                     .set_return_data(program_id, Vec::new())?;
-
                 let logger = self.get_log_collector();
                 stable_log::program_invoke(&logger, &program_id, self.get_stack_height());
                 let pre_remaining_units = self.get_remaining();
                 let mock_config = Config::default();
                 let mut mock_memory_mapping = MemoryMapping::new(Vec::new(), &mock_config).unwrap();
                 let mut result = ProgramResult::Ok(0);
-                (entry.process_instruction)(
+                process_instruction(
                     // Removes lifetime tracking
                     unsafe { std::mem::transmute::<&mut InvokeContext, &mut InvokeContext>(self) },
                     0,

program-runtime/src/loaded_programs.rs

@@ -72,7 +72,7 @@ pub enum LoadedProgramType {
     Typed(VerifiedExecutable<RequisiteVerifier, InvokeContext<'static>>),
     #[cfg(test)]
     TestLoaded,
-    BuiltIn(BuiltInProgram<InvokeContext<'static>>),
+    Builtin(String, BuiltInProgram<InvokeContext<'static>>),
 }
 
 impl Debug for LoadedProgramType {
@@ -89,7 +89,9 @@ impl Debug for LoadedProgramType {
             LoadedProgramType::Typed(_) => write!(f, "LoadedProgramType::Typed"),
             #[cfg(test)]
             LoadedProgramType::TestLoaded => write!(f, "LoadedProgramType::TestLoaded"),
-            LoadedProgramType::BuiltIn(_) => write!(f, "LoadedProgramType::BuiltIn"),
+            LoadedProgramType::Builtin(name, _) => {
+                write!(f, "LoadedProgramType::Builtin({name})")
+            }
         }
     }
 }
@@ -215,7 +217,8 @@ impl LoadedProgram {
     }
 
     /// Creates a new built-in program
-    pub fn new_built_in(
+    pub fn new_builtin(
+        name: String,
         deployment_slot: Slot,
         program: BuiltInProgram<InvokeContext<'static>>,
     ) -> Self {
@@ -225,7 +228,7 @@ impl LoadedProgram {
             effective_slot: deployment_slot.saturating_add(1),
             maybe_expiration_slot: None,
             usage_counter: AtomicU64::new(0),
-            program: LoadedProgramType::BuiltIn(program),
+            program: LoadedProgramType::Builtin(name, program),
         }
     }
 
@@ -294,16 +297,6 @@ impl LoadedProgramsForTxBatch {
     }
 }
 
-#[cfg(RUSTC_WITH_SPECIALIZATION)]
-impl solana_frozen_abi::abi_example::AbiExample for LoadedPrograms {
-    fn example() -> Self {
-        // Delegate AbiExample impl to Default before going deep and stuck with
-        // not easily impl-able Arc<dyn Executor> due to rust's coherence issue
-        // This is safe because LoadedPrograms isn't serializable by definition.
-        Self::default()
-    }
-}
-
 pub enum LoadedProgramMatchCriteria {
     DeployedOnOrAfterSlot(Slot),
     Closed,
@@ -474,7 +467,7 @@ impl LoadedPrograms {
                         LoadedProgramType::FailedVerification
                         | LoadedProgramType::Closed
                         | LoadedProgramType::DelayVisibility
-                        | LoadedProgramType::BuiltIn(_) => None,
+                        | LoadedProgramType::Builtin(_, _) => None,
                     })
             })
             .sorted_by_cached_key(|(order, (_id, program))| {
@@ -561,6 +554,22 @@ impl LoadedPrograms {
     }
 }
 
+#[cfg(RUSTC_WITH_SPECIALIZATION)]
+impl solana_frozen_abi::abi_example::AbiExample for LoadedProgram {
+    fn example() -> Self {
+        // LoadedProgram isn't serializable by definition.
+        Self::default()
+    }
+}
+
+#[cfg(RUSTC_WITH_SPECIALIZATION)]
+impl solana_frozen_abi::abi_example::AbiExample for LoadedPrograms {
+    fn example() -> Self {
+        // LoadedPrograms isn't serializable by definition.
+        Self::default()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use {
@@ -582,7 +591,7 @@ mod tests {
 
     fn new_test_builtin_program(deployment_slot: Slot, effective_slot: Slot) -> Arc<LoadedProgram> {
         Arc::new(LoadedProgram {
-            program: LoadedProgramType::BuiltIn(BuiltInProgram::default()),
+            program: LoadedProgramType::Builtin("mockup".to_string(), BuiltInProgram::default()),
             account_size: 0,
             deployment_slot,
             effective_slot,

program-test/src/lib.rs

@@ -11,7 +11,7 @@ use {
     solana_banks_server::banks_server::start_local_server,
     solana_bpf_loader_program::serialization::serialize_parameters,
     solana_program_runtime::{
-        builtin_program::{BuiltinProgram, BuiltinPrograms, ProcessInstructionWithContext},
+        builtin_program::{create_builtin, BuiltinPrograms, ProcessInstructionWithContext},
         compute_budget::ComputeBudget,
         ic_msg, stable_log,
         timings::ExecuteTimings,
@@ -692,11 +692,10 @@ impl ProgramTest {
         process_instruction: ProcessInstructionWithContext,
     ) {
         info!("\"{}\" builtin program", program_name);
-        self.builtin_programs.vec.push(BuiltinProgram {
-            name: program_name.to_string(),
+        self.builtin_programs.vec.push((
             program_id,
-            process_instruction,
-        });
+            create_builtin(program_name.to_string(), process_instruction),
+        ));
     }
 
     /// Deactivate a runtime feature.
@@ -791,12 +790,8 @@ impl ProgramTest {
         }
 
         // User-supplied additional builtins
-        for builtin in self.builtin_programs.vec.iter() {
-            bank.add_builtin(
-                &builtin.name,
-                &builtin.program_id,
-                builtin.process_instruction,
-            );
+        for (program_id, builtin) in self.builtin_programs.vec.iter() {
+            bank.add_builtin(*program_id, builtin.clone());
         }
 
         for (address, account) in self.accounts.iter() {

runtime/benches/bank.rs

@@ -5,7 +5,7 @@ extern crate test;
 
 use {
     log::*,
-    solana_program_runtime::declare_process_instruction,
+    solana_program_runtime::{builtin_program::create_builtin, declare_process_instruction},
     solana_runtime::{
         bank::{test_utils::goto_end_of_slot, *},
         bank_client::BankClient,
@@ -132,9 +132,8 @@ fn do_bench_transactions(
 
     let mut bank = Bank::new_from_parent(&Arc::new(bank), &Pubkey::default(), 1);
     bank.add_builtin(
-        "builtin_program",
-        &Pubkey::from(BUILTIN_PROGRAM_ID),
-        process_instruction,
+        Pubkey::from(BUILTIN_PROGRAM_ID),
+        create_builtin("mockup".to_string(), process_instruction),
     );
     bank.add_builtin_account("solana_noop_program", &Pubkey::from(NOOP_PROGRAM_ID), false);
     let bank = Arc::new(bank);