gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Refactor: Cleanup InstructionProcessor (#21404)

Browse Source 
* Moves create_message(), native_invoke() and process_cross_program_instruction()
from the InstructionProcessor to the InvokeContext so that they can have a useful "self" parameter.

* Moves InstructionProcessor into InvokeContext and Bank.

* Moves ExecuteDetailsTimings into its own file.

* Moves Executor into invoke_context.rs

* Moves PreAccount into its own file.

* impl AbiExample for BuiltinPrograms
This commit is contained in:
Alexander Meißner
2021-12-01 08:54:42 +01:00
committed by GitHub
parent e922c2da9d
commit b78f5b6032
17 changed files with 548 additions and 623 deletions
Download Patch File Download Diff File Expand all files Collapse all files

124
runtime/src/bank.rs
View File

@ -70,10 +70,12 @@ use rayon::{
use solana_measure::measure::Measure;
use solana_metrics::{inc_new_counter_debug, inc_new_counter_info};
use solana_program_runtime::{
instruction_processor::{ExecuteDetailsTimings, Executor, Executors, InstructionProcessor},
instruction_recorder::InstructionRecorder,
invoke_context::{ComputeMeter, ProcessInstructionWithContext},
invoke_context::{
BuiltinProgram, ComputeMeter, Executor, Executors, ProcessInstructionWithContext,
},
log_collector::LogCollector,
timings::ExecuteDetailsTimings,
};
#[allow(deprecated)]
use solana_sdk::recent_blockhashes_account;
@ -872,6 +874,18 @@ 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
@ -989,8 +1003,8 @@ pub struct Bank {
/// stream for the slot == self.slot
is_delta: AtomicBool,
/// The InstructionProcessor
instruction_processor: InstructionProcessor,
/// The builtin programs
builtin_programs: BuiltinPrograms,
compute_budget: Option<ComputeBudget>,
@ -1149,7 +1163,7 @@ impl Bank {
stakes: RwLock::<Stakes>::default(),
epoch_stakes: HashMap::<Epoch, EpochStakes>::default(),
is_delta: AtomicBool::default(),
instruction_processor: InstructionProcessor::default(),
builtin_programs: BuiltinPrograms::default(),
compute_budget: Option::<ComputeBudget>::default(),
feature_builtins: Arc::<Vec<(Builtin, Pubkey, ActivationType)>>::default(),
rewards: RwLock::<Vec<(Pubkey, RewardInfo)>>::default(),
@ -1389,7 +1403,7 @@ impl Bank {
is_delta: AtomicBool::new(false),
tick_height: AtomicU64::new(parent.tick_height.load(Relaxed)),
signature_count: AtomicU64::new(0),
instruction_processor: parent.instruction_processor.clone(),
builtin_programs: parent.builtin_programs.clone(),
compute_budget: parent.compute_budget,
feature_builtins: parent.feature_builtins.clone(),
hard_forks: parent.hard_forks.clone(),
@ -1598,7 +1612,7 @@ impl Bank {
stakes: RwLock::new(fields.stakes),
epoch_stakes: fields.epoch_stakes,
is_delta: AtomicBool::new(fields.is_delta),
instruction_processor: new(),
builtin_programs: new(),
compute_budget: None,
feature_builtins: new(),
rewards: new(),
@ -3040,7 +3054,7 @@ impl Bank {
&genesis_config.rent,
);
// Add additional native programs specified in the genesis config
// Add additional builtin programs specified in the genesis config
for (name, program_id) in &genesis_config.native_instruction_processors {
self.add_builtin_account(name, program_id, false);
}
@ -3073,10 +3087,10 @@ impl Bank {
});
if must_replace {
// updating native program
// updating builtin program
match &existing_genuine_program {
None => panic!(
"There is no account to replace with native program ({}, {}).",
"There is no account to replace with builtin program ({}, {}).",
name, program_id
),
Some(account) => {
@ -3087,7 +3101,7 @@ impl Bank {
}
}
} else {
// introducing native program
// introducing builtin program
if existing_genuine_program.is_some() {
// The existing account is sufficient
return;
@ -3096,13 +3110,13 @@ impl Bank {
assert!(
!self.freeze_started(),
"Can't change frozen bank by adding not-existing new native program ({}, {}). \
"Can't change frozen bank by adding not-existing new builtin program ({}, {}). \
Maybe, inconsistent program activation is detected on snapshot restore?",
name,
program_id
);
// Add a bogus executable native account, which will be loaded and ignored.
// Add a bogus executable builtin account, which will be loaded and ignored.
let account = native_loader::create_loadable_account_with_fields(
name,
self.inherit_specially_retained_account_fields(&existing_genuine_program),
@ -3867,7 +3881,7 @@ impl Bank {
if let Some(legacy_message) = tx.message().legacy_message() {
process_result = MessageProcessor::process_message(
&self.instruction_processor,
&self.builtin_programs.vec,
legacy_message,
&loaded_transaction.program_indices,
&account_refcells,
@ -5895,12 +5909,23 @@ impl Bank {
&mut self,
name: &str,
program_id: &Pubkey,
process_instruction_with_context: ProcessInstructionWithContext,
process_instruction: ProcessInstructionWithContext,
) {
debug!("Adding program {} under {:?}", name, program_id);
self.add_builtin_account(name, program_id, false);
self.instruction_processor
.add_program(program_id, process_instruction_with_context);
if let Some(entry) = self
.builtin_programs
.vec
.iter_mut()
.find(|entry| entry.program_id == *program_id)
{
entry.process_instruction = process_instruction;
} else {
self.builtin_programs.vec.push(BuiltinProgram {
program_id: *program_id,
process_instruction,
});
}
debug!("Added program {} under {:?}", name, program_id);
}
@ -5909,12 +5934,18 @@ impl Bank {
&mut self,
name: &str,
program_id: &Pubkey,
process_instruction_with_context: ProcessInstructionWithContext,
process_instruction: ProcessInstructionWithContext,
) {
debug!("Replacing program {} under {:?}", name, program_id);
self.add_builtin_account(name, program_id, true);
self.instruction_processor
.add_program(program_id, process_instruction_with_context);
if let Some(entry) = self
.builtin_programs
.vec
.iter_mut()
.find(|entry| entry.program_id == *program_id)
{
entry.process_instruction = process_instruction;
}
debug!("Replaced program {} under {:?}", name, program_id);
}
@ -5923,7 +5954,14 @@ impl Bank {
debug!("Removing program {} under {:?}", name, program_id);
// Don't remove the account since the bank expects the account state to
// be idempotent
self.instruction_processor.remove_program(program_id);
if let Some(position) = self
.builtin_programs
.vec
.iter()
.position(|entry| entry.program_id == *program_id)
{
self.builtin_programs.vec.remove(position);
}
debug!("Removed program {} under {:?}", name, program_id);
}
@ -6787,16 +6825,16 @@ pub(crate) mod tests {
cluster_type: ClusterType::MainnetBeta,
..GenesisConfig::default()
}));
let sysvar_and_native_program_delta0 = 11;
let sysvar_and_builtin_program_delta0 = 11;
assert_eq!(
bank0.capitalization(),
42 * 42 + sysvar_and_native_program_delta0
42 * 42 + sysvar_and_builtin_program_delta0
);
let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), 1);
let sysvar_and_native_program_delta1 = 2;
let sysvar_and_builtin_program_delta1 = 2;
assert_eq!(
bank1.capitalization(),
42 * 42 + sysvar_and_native_program_delta0 + sysvar_and_native_program_delta1,
42 * 42 + sysvar_and_builtin_program_delta0 + sysvar_and_builtin_program_delta1,
);
}
@ -6866,7 +6904,7 @@ pub(crate) mod tests {
bank_with_success_txs.store_account(&keypair5.pubkey(), &account5);
bank_with_success_txs.store_account(&keypair6.pubkey(), &account6);
// Make native instruction loader rent exempt
// Make builtin instruction loader rent exempt
let system_program_id = system_program::id();
let mut system_program_account = bank.get_account(&system_program_id).unwrap();
system_program_account.set_lamports(
@ -7393,10 +7431,10 @@ pub(crate) mod tests {
let current_capitalization = bank.capitalization.load(Relaxed);
// only slot history is newly created
let sysvar_and_native_program_delta =
let sysvar_and_builtin_program_delta =
min_rent_excempt_balance_for_sysvars(&bank, &[sysvar::slot_history::id()]);
assert_eq!(
previous_capitalization - (current_capitalization - sysvar_and_native_program_delta),
previous_capitalization - (current_capitalization - sysvar_and_builtin_program_delta),
burned_portion
);
@ -8531,10 +8569,10 @@ pub(crate) mod tests {
// not being eagerly-collected for exact rewards calculation
bank0.restore_old_behavior_for_fragile_tests();
let sysvar_and_native_program_delta0 = 11;
let sysvar_and_builtin_program_delta0 = 11;
assert_eq!(
bank0.capitalization(),
42 * 1_000_000_000 + sysvar_and_native_program_delta0
42 * 1_000_000_000 + sysvar_and_builtin_program_delta0
);
assert!(bank0.rewards.read().unwrap().is_empty());
@ -8596,9 +8634,9 @@ pub(crate) mod tests {
assert_ne!(bank1.capitalization(), bank0.capitalization());
// verify the inflation is represented in validator_points *
let sysvar_and_native_program_delta1 = 2;
let sysvar_and_builtin_program_delta1 = 2;
let paid_rewards =
bank1.capitalization() - bank0.capitalization() - sysvar_and_native_program_delta1;
bank1.capitalization() - bank0.capitalization() - sysvar_and_builtin_program_delta1;
let rewards = bank1
.get_account(&sysvar::rewards::id())
@ -8665,10 +8703,10 @@ pub(crate) mod tests {
// not being eagerly-collected for exact rewards calculation
bank.restore_old_behavior_for_fragile_tests();
let sysvar_and_native_program_delta = 11;
let sysvar_and_builtin_program_delta = 11;
assert_eq!(
bank.capitalization(),
42 * 1_000_000_000 + sysvar_and_native_program_delta
42 * 1_000_000_000 + sysvar_and_builtin_program_delta
);
assert!(bank.rewards.read().unwrap().is_empty());
@ -9116,9 +9154,9 @@ pub(crate) mod tests {
); // Leader collects fee after the bank is frozen
// verify capitalization
let sysvar_and_native_program_delta = 1;
let sysvar_and_builtin_program_delta = 1;
assert_eq!(
capitalization - expected_fee_burned + sysvar_and_native_program_delta,
capitalization - expected_fee_burned + sysvar_and_builtin_program_delta,
bank.capitalization()
);
@ -10782,7 +10820,7 @@ pub(crate) mod tests {
Err(InstructionError::Custom(42))
}
// Non-native loader accounts can not be used for instruction processing
// Non-builtin loader accounts can not be used for instruction processing
{
let stakes = bank.stakes.read().unwrap();
assert!(stakes.vote_accounts().as_ref().is_empty());
@ -12544,7 +12582,7 @@ pub(crate) mod tests {
assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot);
let bank = Arc::new(new_from_parent(&bank));
// When replacing native_program, name must change to disambiguate from repeated
// When replacing builtin_program, name must change to disambiguate from repeated
// invocations.
assert_capitalization_diff(
&bank,
@ -12608,7 +12646,7 @@ pub(crate) mod tests {
#[test]
#[should_panic(
expected = "Can't change frozen bank by adding not-existing new native \
expected = "Can't change frozen bank by adding not-existing new builtin \
program (mock_program, CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \
Maybe, inconsistent program activation is detected on snapshot restore?"
)]
@ -12631,7 +12669,7 @@ pub(crate) mod tests {
#[test]
#[should_panic(
expected = "There is no account to replace with native program (mock_program, \
expected = "There is no account to replace with builtin program (mock_program, \
CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre)."
)]
fn test_add_builtin_account_replace_none() {
@ -12836,9 +12874,9 @@ pub(crate) mod tests {
// assert that everything gets in order....
assert!(bank1.get_account(&reward_pubkey).is_none());
let sysvar_and_native_program_delta = 1;
let sysvar_and_builtin_program_delta = 1;
assert_eq!(
bank0.capitalization() + 1 + 1_000_000_000 + sysvar_and_native_program_delta,
bank0.capitalization() + 1 + 1_000_000_000 + sysvar_and_builtin_program_delta,
bank1.capitalization()
);
assert_eq!(bank1.capitalization(), bank1.calculate_capitalization(true));
@ -13377,7 +13415,7 @@ pub(crate) mod tests {
.accounts
.remove(&feature_set::deprecate_rewards_sysvar::id());
// intentionally create bogus native programs
// intentionally create bogus builtin programs
#[allow(clippy::unnecessary_wraps)]
fn mock_process_instruction(
_first_instruction_account: usize,

43
runtime/src/message_processor.rs
View File

@ -1,10 +1,10 @@
use serde::{Deserialize, Serialize};
use solana_measure::measure::Measure;
use solana_program_runtime::{
instruction_processor::{ExecuteDetailsTimings, Executors, InstructionProcessor},
instruction_recorder::InstructionRecorder,
invoke_context::{ComputeMeter, InvokeContext, ThisInvokeContext},
invoke_context::{BuiltinProgram, ComputeMeter, Executors, InvokeContext, ThisInvokeContext},
log_collector::LogCollector,
timings::ExecuteDetailsTimings,
};
use solana_sdk::{
account::{AccountSharedData, WritableAccount},
@ -40,7 +40,7 @@ impl MessageProcessor {
/// The accounts are committed back to the bank only if every instruction succeeds.
#[allow(clippy::too_many_arguments)]
pub fn process_message(
instruction_processor: &InstructionProcessor,
builtin_programs: &[BuiltinProgram],
message: &Message,
program_indices: &[Vec<usize>],
accounts: &[(Pubkey, Rc<RefCell<AccountSharedData>>)],
@ -59,7 +59,7 @@ impl MessageProcessor {
let mut invoke_context = ThisInvokeContext::new(
rent,
accounts,
instruction_processor.programs(),
builtin_programs,
sysvars,
log_collector,
compute_budget,
@ -107,8 +107,7 @@ impl MessageProcessor {
let result = invoke_context
.push(message, instruction, program_indices, None)
.and_then(|_| {
instruction_processor
.process_instruction(&instruction.data, &mut invoke_context)?;
invoke_context.process_instruction(&instruction.data)?;
invoke_context.verify(message, instruction, program_indices)?;
timings.accumulate(&invoke_context.timings);
Ok(())
@ -199,8 +198,10 @@ mod tests {
let mock_system_program_id = Pubkey::new(&[2u8; 32]);
let rent_collector = RentCollector::default();
let mut instruction_processor = InstructionProcessor::default();
instruction_processor.add_program(&mock_system_program_id, mock_system_process_instruction);
let builtin_programs = &[BuiltinProgram {
program_id: mock_system_program_id,
process_instruction: mock_system_process_instruction,
}];
let program_account = Rc::new(RefCell::new(create_loadable_account_for_test(
"mock_system_program",
@ -234,7 +235,7 @@ mod tests {
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&program_indices,
&accounts,
@ -264,7 +265,7 @@ mod tests {
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&program_indices,
&accounts,
@ -298,7 +299,7 @@ mod tests {
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&program_indices,
&accounts,
@ -404,8 +405,10 @@ mod tests {
let mock_program_id = Pubkey::new(&[2u8; 32]);
let rent_collector = RentCollector::default();
let mut instruction_processor = InstructionProcessor::default();
instruction_processor.add_program(&mock_program_id, mock_system_process_instruction);
let builtin_programs = &[BuiltinProgram {
program_id: mock_program_id,
process_instruction: mock_system_process_instruction,
}];
let program_account = Rc::new(RefCell::new(create_loadable_account_for_test(
"mock_system_program",
@ -441,7 +444,7 @@ mod tests {
Some(&accounts[0].0),
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&program_indices,
&accounts,
@ -475,7 +478,7 @@ mod tests {
Some(&accounts[0].0),
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&program_indices,
&accounts,
@ -506,7 +509,7 @@ mod tests {
Some(&accounts[0].0),
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&program_indices,
&accounts,
@ -538,8 +541,10 @@ mod tests {
) -> Result<(), InstructionError> {
Err(InstructionError::Custom(0xbabb1e))
}
let mut instruction_processor = InstructionProcessor::default();
instruction_processor.add_program(&mock_program_id, mock_process_instruction);
let builtin_programs = &[BuiltinProgram {
program_id: mock_program_id,
process_instruction: mock_process_instruction,
}];
let secp256k1_account = AccountSharedData::new_ref(1, 0, &native_loader::id());
secp256k1_account.borrow_mut().set_executable(true);
@ -562,7 +567,7 @@ mod tests {
);
let result = MessageProcessor::process_message(
&instruction_processor,
builtin_programs,
&message,
&[vec![0], vec![1]],
&accounts,

1
runtime/src/serde_snapshot.rs
View File

@ -23,7 +23,6 @@ use {
rayon::prelude::*,
serde::{de::DeserializeOwned, Deserialize, Serialize},
solana_measure::measure::Measure,
solana_program_runtime::instruction_processor::InstructionProcessor,
solana_sdk::{
clock::{Epoch, Slot, UnixTimestamp},
epoch_schedule::EpochSchedule,

4
runtime/src/serde_snapshot/future.rs
View File

@ -79,8 +79,6 @@ pub(crate) struct DeserializableVersionedBank {
pub(crate) unused_accounts: UnusedAccounts,
pub(crate) epoch_stakes: HashMap<Epoch, EpochStakes>,
pub(crate) is_delta: bool,
#[allow(dead_code)]
pub(crate) message_processor: InstructionProcessor,
}
impl From<DeserializableVersionedBank> for BankFieldsToDeserialize {
@ -157,7 +155,6 @@ pub(crate) struct SerializableVersionedBank<'a> {
pub(crate) unused_accounts: UnusedAccounts,
pub(crate) epoch_stakes: &'a HashMap<Epoch, EpochStakes>,
pub(crate) is_delta: bool,
pub(crate) message_processor: InstructionProcessor,
}
impl<'a> From<crate::bank::BankFieldsToSerialize<'a>> for SerializableVersionedBank<'a> {
@ -198,7 +195,6 @@ impl<'a> From<crate::bank::BankFieldsToSerialize<'a>> for SerializableVersionedB
unused_accounts: new(),
epoch_stakes: rhs.epoch_stakes,
is_delta: rhs.is_delta,
message_processor: new(),
}
}
}

2
runtime/src/serde_snapshot/tests.rs
View File

@ -312,7 +312,7 @@ mod test_bank_serialize {
// This some what long test harness is required to freeze the ABI of
// Bank's serialization due to versioned nature
#[frozen_abi(digest = "A9KFf8kLJczP3AMbFXRrqzmruoqMjooTPzdvEwZZ4EP7")]
#[frozen_abi(digest = "5vYNtKztrNKfb6TtRktXdLCbU73rJSWhLwguCHLvFujZ")]
#[derive(Serialize, AbiExample)]
pub struct BankAbiTestWrapperFuture {
#[serde(serialize_with = "wrapper_future")]