Refactor - BuiltinPrograms

ledger/src/builtins.rs

@@ -1,13 +1,20 @@
-use solana_runtime::builtins::{Builtin, BuiltinFeatureTransition, Builtins};
+use {
+    solana_program_runtime::builtin_program::BuiltinProgram,
+    solana_runtime::builtins::{BuiltinFeatureTransition, Builtins},
+};
 
 macro_rules! to_builtin {
     ($b:expr) => {
-        Builtin::new(&$b.0, $b.1, $b.2)
+        BuiltinProgram {
+            name: $b.0.to_string(),
+            program_id: $b.1,
+            process_instruction: $b.2,
+        }
     };
 }
 
 /// Builtin programs that are always available
-fn genesis_builtins(bpf_jit: bool) -> Vec<Builtin> {
+fn genesis_builtins(bpf_jit: bool) -> Vec<BuiltinProgram> {
     // Currently JIT is not supported on the SBF VM:
     // !x86_64: https://github.com/qmonnet/rbpf/issues/48
     // Windows: https://github.com/solana-labs/rbpf/issues/217

program-runtime/src/builtin_program.rs

@@ -0,0 +1,64 @@
+#[cfg(RUSTC_WITH_SPECIALIZATION)]
+use {crate::declare_process_instruction, solana_frozen_abi::abi_example::AbiExample};
+use {
+    crate::invoke_context::InvokeContext, solana_rbpf::vm::BuiltInFunction,
+    solana_sdk::pubkey::Pubkey,
+};
+
+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,
+        }
+    }
+}
+
+#[derive(Debug, Clone, Default)]
+pub struct BuiltinPrograms {
+    pub vec: Vec<BuiltinProgram>,
+}
+
+#[cfg(RUSTC_WITH_SPECIALIZATION)]
+impl AbiExample for BuiltinPrograms {
+    fn example() -> Self {
+        Self::default()
+    }
+}
+
+impl BuiltinPrograms {
+    pub fn new_mock(
+        program_id: Pubkey,
+        process_instruction: ProcessInstructionWithContext,
+    ) -> Self {
+        Self {
+            vec: vec![BuiltinProgram {
+                name: "mock instruction processor".to_string(),
+                program_id,
+                process_instruction,
+            }],
+        }
+    }
+}

program-runtime/src/invoke_context.rs

@@ -1,6 +1,7 @@
 use {
     crate::{
         accounts_data_meter::AccountsDataMeter,
+        builtin_program::{BuiltinPrograms, ProcessInstructionWithContext},
         compute_budget::ComputeBudget,
         executor_cache::TransactionExecutorCache,
         ic_logger_msg, ic_msg,
@@ -14,7 +15,7 @@ use {
     solana_rbpf::{
         ebpf::MM_HEAP_START,
         memory_region::MemoryMapping,
-        vm::{BuiltInFunction, Config, ContextObject, ProgramResult},
+        vm::{Config, ContextObject, ProgramResult},
     },
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount},
@@ -82,24 +83,6 @@ macro_rules! declare_process_instruction {
     };
 }
 
-pub type ProcessInstructionWithContext = BuiltInFunction<InvokeContext<'static>>;
-
-#[derive(Clone)]
-pub struct BuiltinProgram {
-    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,
-            "{}: {:p}",
-            self.program_id, self.process_instruction as *const ProcessInstructionWithContext,
-        )
-    }
-}
-
 impl<'a> ContextObject for InvokeContext<'a> {
     fn trace(&mut self, state: [u64; 12]) {
         self.syscall_context
@@ -169,7 +152,7 @@ pub struct InvokeContext<'a> {
     pub transaction_context: &'a mut TransactionContext,
     rent: Rent,
     pre_accounts: Vec<PreAccount>,
-    builtin_programs: &'a [BuiltinProgram],
+    builtin_programs: &'a BuiltinPrograms,
     sysvar_cache: &'a SysvarCache,
     log_collector: Option<Rc<RefCell<LogCollector>>>,
     compute_budget: ComputeBudget,
@@ -189,7 +172,7 @@ impl<'a> InvokeContext<'a> {
     pub fn new(
         transaction_context: &'a mut TransactionContext,
         rent: Rent,
-        builtin_programs: &'a [BuiltinProgram],
+        builtin_programs: &'a BuiltinPrograms,
         sysvar_cache: &'a SysvarCache,
         log_collector: Option<Rc<RefCell<LogCollector>>>,
         compute_budget: ComputeBudget,
@@ -731,7 +714,7 @@ impl<'a> InvokeContext<'a> {
             }
         };
 
-        for entry in self.builtin_programs {
+        for entry in self.builtin_programs.vec.iter() {
             if entry.program_id == builtin_id {
                 let program_id =
                     *instruction_context.get_last_program_key(self.transaction_context)?;
@@ -894,9 +877,9 @@ macro_rules! with_mock_invoke_context {
             },
             std::{cell::RefCell, rc::Rc, sync::Arc},
             $crate::{
-                compute_budget::ComputeBudget, executor_cache::TransactionExecutorCache,
-                invoke_context::InvokeContext, log_collector::LogCollector,
-                sysvar_cache::SysvarCache,
+                builtin_program::BuiltinPrograms, compute_budget::ComputeBudget,
+                executor_cache::TransactionExecutorCache, invoke_context::InvokeContext,
+                log_collector::LogCollector, sysvar_cache::SysvarCache,
             },
         };
         let compute_budget = ComputeBudget::default();
@@ -907,6 +890,7 @@ macro_rules! with_mock_invoke_context {
             compute_budget.max_instruction_trace_length,
         );
         $transaction_context.enable_cap_accounts_data_allocations_per_transaction();
+        let builtin_programs = BuiltinPrograms::default();
         let mut sysvar_cache = SysvarCache::default();
         sysvar_cache.fill_missing_entries(|pubkey, callback| {
             for index in 0..$transaction_context.get_number_of_accounts() {
@@ -928,7 +912,7 @@ macro_rules! with_mock_invoke_context {
         let mut $invoke_context = InvokeContext::new(
             &mut $transaction_context,
             Rent::default(),
-            &[],
+            &builtin_programs,
             &sysvar_cache,
             Some(LogCollector::new_ref()),
             compute_budget,
@@ -980,11 +964,8 @@ pub fn mock_process_instruction<F: FnMut(&mut InvokeContext), G: FnMut(&mut Invo
     let processor_account = AccountSharedData::new(0, 0, &native_loader::id());
     transaction_accounts.push((*loader_id, processor_account));
     with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
-    let builtin_programs = &[BuiltinProgram {
-        program_id: *loader_id,
-        process_instruction,
-    }];
-    invoke_context.builtin_programs = builtin_programs;
+    let builtin_programs = BuiltinPrograms::new_mock(*loader_id, process_instruction);
+    invoke_context.builtin_programs = &builtin_programs;
     pre_adjustments(&mut invoke_context);
     let result = invoke_context.process_instruction(
         instruction_data,
@@ -1233,11 +1214,8 @@ mod tests {
             })
             .collect::<Vec<_>>();
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
-        let builtin_programs = &[BuiltinProgram {
-            program_id: callee_program_id,
-            process_instruction,
-        }];
-        invoke_context.builtin_programs = builtin_programs;
+        let builtin_programs = BuiltinPrograms::new_mock(callee_program_id, process_instruction);
+        invoke_context.builtin_programs = &builtin_programs;
 
         // Account modification tests
         let cases = vec![
@@ -1378,11 +1356,8 @@ mod tests {
             },
         ];
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
-        let builtin_programs = &[BuiltinProgram {
-            program_id: program_key,
-            process_instruction,
-        }];
-        invoke_context.builtin_programs = builtin_programs;
+        let builtin_programs = BuiltinPrograms::new_mock(program_key, process_instruction);
+        invoke_context.builtin_programs = &builtin_programs;
 
         // Test: Resize the account to *the same size*, so not consuming any additional size; this must succeed
         {

program-runtime/src/lib.rs

@@ -11,6 +11,7 @@ extern crate solana_metrics;
 
 pub use solana_rbpf;
 pub mod accounts_data_meter;
+pub mod builtin_program;
 pub mod compute_budget;
 pub mod executor_cache;
 pub mod invoke_context;

program-test/src/lib.rs

@@ -11,14 +11,15 @@ use {
     solana_banks_server::banks_server::start_local_server,
     solana_bpf_loader_program::serialization::serialize_parameters,
     solana_program_runtime::{
-        compute_budget::ComputeBudget, ic_msg, invoke_context::ProcessInstructionWithContext,
-        stable_log, timings::ExecuteTimings,
+        builtin_program::{BuiltinProgram, BuiltinPrograms, ProcessInstructionWithContext},
+        compute_budget::ComputeBudget,
+        ic_msg, stable_log,
+        timings::ExecuteTimings,
     },
     solana_runtime::{
         accounts_background_service::{AbsRequestSender, SnapshotRequestType},
         bank::Bank,
         bank_forks::BankForks,
-        builtins::Builtin,
         commitment::BlockCommitmentCache,
         epoch_accounts_hash::EpochAccountsHash,
         genesis_utils::{create_genesis_config_with_leader_ex, GenesisConfigInfo},
@@ -436,7 +437,7 @@ pub fn read_file<P: AsRef<Path>>(path: P) -> Vec<u8> {
 
 pub struct ProgramTest {
     accounts: Vec<(Pubkey, AccountSharedData)>,
-    builtins: Vec<Builtin>,
+    builtin_programs: BuiltinPrograms,
     compute_max_units: Option<u64>,
     prefer_bpf: bool,
     use_bpf_jit: bool,
@@ -474,7 +475,7 @@ impl Default for ProgramTest {
 
         Self {
             accounts: vec![],
-            builtins: vec![],
+            builtin_programs: BuiltinPrograms::default(),
             compute_max_units: None,
             prefer_bpf,
             use_bpf_jit: false,
@@ -628,12 +629,6 @@ impl ProgramTest {
             );
         };
 
-        let add_native = |this: &mut ProgramTest, process_fn: ProcessInstructionWithContext| {
-            info!("\"{}\" program loaded as native code", program_name);
-            this.builtins
-                .push(Builtin::new(program_name, program_id, process_fn));
-        };
-
         let warn_invalid_program_name = || {
             let valid_program_names = default_shared_object_dirs()
                 .iter()
@@ -679,7 +674,9 @@ impl ProgramTest {
             // processor function as is.
             //
             // TODO: figure out why tests hang if a processor panics when running native code.
-            (false, _, Some(process)) => add_native(self, process),
+            (false, _, Some(process)) => {
+                self.add_builtin_program(program_name, program_id, process)
+            }
 
             // Invalid: `test-sbf` invocation with no matching SBF shared object.
             (true, None, _) => {
@@ -704,8 +701,11 @@ impl ProgramTest {
         process_instruction: ProcessInstructionWithContext,
     ) {
         info!("\"{}\" builtin program", program_name);
-        self.builtins
-            .push(Builtin::new(program_name, program_id, process_instruction));
+        self.builtin_programs.vec.push(BuiltinProgram {
+            name: program_name.to_string(),
+            program_id,
+            process_instruction,
+        });
     }
 
     /// Deactivate a runtime feature.
@@ -816,11 +816,11 @@ impl ProgramTest {
         }
 
         // User-supplied additional builtins
-        for builtin in self.builtins.iter() {
+        for builtin in self.builtin_programs.vec.iter() {
             bank.add_builtin(
                 &builtin.name,
-                &builtin.id,
-                builtin.process_instruction_with_context,
+                &builtin.program_id,
+                builtin.process_instruction,
             );
         }
 

runtime/src/bank.rs

@@ -94,9 +94,9 @@ use {
     solana_perf::perf_libs,
     solana_program_runtime::{
         accounts_data_meter::MAX_ACCOUNTS_DATA_LEN,
+        builtin_program::{BuiltinProgram, BuiltinPrograms, ProcessInstructionWithContext},
         compute_budget::{self, ComputeBudget},
         executor_cache::{BankExecutorCache, TransactionExecutorCache, MAX_CACHED_EXECUTORS},
-        invoke_context::{BuiltinProgram, ProcessInstructionWithContext},
         loaded_programs::{LoadedProgram, LoadedProgramType, LoadedPrograms, WorkingSlot},
         log_collector::LogCollector,
         sysvar_cache::SysvarCache,
@@ -887,18 +887,6 @@ impl AbiExample for OptionalDropCallback {
     }
 }
 
-#[derive(Debug, Clone, Default)]
-pub struct BuiltinPrograms {
-    pub vec: Vec<BuiltinProgram>,
-}
-
-#[cfg(RUSTC_WITH_SPECIALIZATION)]
-impl AbiExample for BuiltinPrograms {
-    fn example() -> Self {
-        Self::default()
-    }
-}
-
 /// Manager for the state of all accounts and programs after processing its entries.
 /// AbiExample is needed even without Serialize/Deserialize; actual (de-)serialization
 /// are implemented elsewhere for versioning
@@ -4282,7 +4270,7 @@ impl Bank {
 
         let mut process_message_time = Measure::start("process_message_time");
         let process_result = MessageProcessor::process_message(
-            &self.builtin_programs.vec,
+            &self.builtin_programs,
             tx.message(),
             &loaded_transaction.program_indices,
             &mut transaction_context,
@@ -6631,8 +6619,8 @@ impl Bank {
             for builtin in builtins.genesis_builtins {
                 self.add_builtin(
                     &builtin.name,
-                    &builtin.id,
-                    builtin.process_instruction_with_context,
+                    &builtin.program_id,
+                    builtin.process_instruction,
                 );
             }
             for precompile in get_precompiles() {
@@ -7674,6 +7662,7 @@ impl Bank {
             entry.process_instruction = process_instruction;
         } else {
             self.builtin_programs.vec.push(BuiltinProgram {
+                name: name.to_string(),
                 program_id: *program_id,
                 process_instruction,
             });
@@ -7944,8 +7933,8 @@ impl Bank {
                 match builtin_action {
                     BuiltinAction::Add(builtin) => self.add_builtin(
                         &builtin.name,
-                        &builtin.id,
-                        builtin.process_instruction_with_context,
+                        &builtin.program_id,
+                        builtin.process_instruction,
                     ),
                     BuiltinAction::Remove(program_id) => self.remove_builtin(&program_id),
                 }