Skip to content

Commit

Permalink
SVM: Cleanup transaction processor construction (anza-xyz#974)
Browse files Browse the repository at this point in the history
  • Loading branch information
pgarg66 authored Apr 22, 2024
1 parent 0f939c1 commit bbe4705
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 64 deletions.
2 changes: 1 addition & 1 deletion core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ impl Consumer {
let fee_payer = message.fee_payer();
let budget_limits =
process_compute_budget_instructions(message.program_instructions_iter())?.into();
let fee = bank.fee_structure.calculate_fee(
let fee = bank.fee_structure().calculate_fee(
message,
bank.get_lamports_per_signature(),
&budget_limits,
Expand Down
3 changes: 1 addition & 2 deletions program-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,8 +837,7 @@ impl ProgramTest {
let mut builtin_programs = Vec::new();
std::mem::swap(&mut self.builtin_programs, &mut builtin_programs);
for (program_id, name, builtin) in builtin_programs.into_iter() {
bank.get_transaction_processor()
.add_builtin(&bank, program_id, name, builtin);
bank.add_builtin(program_id, name, builtin);
}

for (address, account) in self.accounts.iter() {
Expand Down
2 changes: 1 addition & 1 deletion programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3692,7 +3692,7 @@ fn test_program_fees() {
let mut bank = Bank::new_for_tests(&genesis_config);
let fee_structure =
FeeStructure::new(0.000005, 0.0, vec![(200, 0.0000005), (1400000, 0.000005)]);
bank.fee_structure = fee_structure.clone();
bank.set_fee_structure(&fee_structure);
let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
let mut bank_client = BankClient::new_shared(bank);
let authority_keypair = Keypair::new();
Expand Down
78 changes: 33 additions & 45 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,6 @@ impl PartialEq for Bank {
epoch_stakes,
is_delta,
// TODO: Confirm if all these fields are intentionally ignored!
runtime_config: _,
rewards: _,
cluster_type: _,
lazy_rent_collection: _,
Expand All @@ -584,7 +583,6 @@ impl PartialEq for Bank {
accounts_data_size_initial: _,
accounts_data_size_delta_on_chain: _,
accounts_data_size_delta_off_chain: _,
fee_structure: _,
incremental_snapshot_persistence: _,
epoch_reward_status: _,
transaction_processor: _,
Expand Down Expand Up @@ -773,9 +771,6 @@ pub struct Bank {
/// stream for the slot == self.slot
is_delta: AtomicBool,

/// Optional config parameters that can override runtime behavior
pub(crate) runtime_config: Arc<RuntimeConfig>,

/// Protocol-level rewards that were distributed by this bank
pub rewards: RwLock<Vec<(Pubkey, RewardInfo)>>,

Expand Down Expand Up @@ -820,9 +815,6 @@ pub struct Bank {
/// the account hash of the accounts that would have been rewritten as bank hash expects.
skipped_rewrites: Mutex<HashMap<Pubkey, AccountHash>>,

/// Transaction fee structure
pub fee_structure: FeeStructure,

pub incremental_snapshot_persistence: Option<BankIncrementalSnapshotPersistence>,

epoch_reward_status: EpochRewardStatus,
Expand Down Expand Up @@ -927,7 +919,6 @@ impl Bank {
stakes_cache: StakesCache::default(),
epoch_stakes: HashMap::<Epoch, EpochStakes>::default(),
is_delta: AtomicBool::default(),
runtime_config: Arc::<RuntimeConfig>::default(),
rewards: RwLock::<Vec<(Pubkey, RewardInfo)>>::default(),
cluster_type: Option::<ClusterType>::default(),
lazy_rent_collection: AtomicBool::default(),
Expand All @@ -945,7 +936,6 @@ impl Bank {
accounts_data_size_initial: 0,
accounts_data_size_delta_on_chain: AtomicI64::new(0),
accounts_data_size_delta_off_chain: AtomicI64::new(0),
fee_structure: FeeStructure::default(),
epoch_reward_status: EpochRewardStatus::default(),
transaction_processor: TransactionBatchProcessor::default(),
check_program_modification_slot: false,
Expand All @@ -956,8 +946,7 @@ impl Bank {
bank.slot,
bank.epoch,
bank.epoch_schedule.clone(),
bank.fee_structure.clone(),
bank.runtime_config.clone(),
Arc::new(RuntimeConfig::default()),
Arc::new(RwLock::new(ProgramCache::new(
Slot::default(),
Epoch::default(),
Expand Down Expand Up @@ -999,7 +988,7 @@ impl Bank {
let mut bank = Self::default_with_accounts(accounts);
bank.ancestors = Ancestors::from(vec![bank.slot()]);
bank.transaction_debug_keys = debug_keys;
bank.runtime_config = runtime_config;
bank.transaction_processor.runtime_config = runtime_config;
bank.cluster_type = Some(genesis_config.cluster_type);

#[cfg(not(feature = "dev-context-only-utils"))]
Expand Down Expand Up @@ -1115,12 +1104,9 @@ impl Bank {

let (epoch_stakes, epoch_stakes_time_us) = measure_us!(parent.epoch_stakes.clone());

let (builtin_program_ids, builtin_program_ids_time_us) = measure_us!(parent
.transaction_processor
.builtin_program_ids
.read()
.unwrap()
.clone());
let (transaction_processor, builtin_program_ids_time_us) = measure_us!(
TransactionBatchProcessor::new_from(&parent.transaction_processor, slot, epoch)
);

let (rewards_pool_pubkeys, rewards_pool_pubkeys_time_us) =
measure_us!(parent.rewards_pool_pubkeys.clone());
Expand Down Expand Up @@ -1178,7 +1164,6 @@ impl Bank {
is_delta: AtomicBool::new(false),
tick_height: AtomicU64::new(parent.tick_height.load(Relaxed)),
signature_count: AtomicU64::new(0),
runtime_config: parent.runtime_config.clone(),
hard_forks: parent.hard_forks.clone(),
rewards: RwLock::new(vec![]),
cluster_type: parent.cluster_type,
Expand All @@ -1203,23 +1188,12 @@ impl Bank {
accounts_data_size_initial,
accounts_data_size_delta_on_chain: AtomicI64::new(0),
accounts_data_size_delta_off_chain: AtomicI64::new(0),
fee_structure: parent.fee_structure.clone(),
epoch_reward_status: parent.epoch_reward_status.clone(),
transaction_processor: TransactionBatchProcessor::default(),
transaction_processor,
check_program_modification_slot: false,
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
};

new.transaction_processor = TransactionBatchProcessor::new(
new.slot,
new.epoch,
new.epoch_schedule.clone(),
new.fee_structure.clone(),
new.runtime_config.clone(),
parent.transaction_processor.program_cache.clone(),
builtin_program_ids,
);

let (_, ancestors_time_us) = measure_us!({
let mut ancestors = Vec::with_capacity(1 + new.parents().len());
ancestors.push(new.slot());
Expand Down Expand Up @@ -1287,13 +1261,13 @@ impl Bank {
let mut program_cache = new.transaction_processor.program_cache.write().unwrap();
let program_runtime_environment_v1 = create_program_runtime_environment_v1(
&feature_set,
&new.runtime_config.compute_budget.unwrap_or_default(),
&new.runtime_config().compute_budget.unwrap_or_default(),
false, /* deployment */
false, /* debugging_features */
)
.unwrap();
let program_runtime_environment_v2 = create_program_runtime_environment_v2(
&new.runtime_config.compute_budget.unwrap_or_default(),
&new.runtime_config().compute_budget.unwrap_or_default(),
false, /* debugging_features */
);
let mut upcoming_environments = program_cache.environments.clone();
Expand Down Expand Up @@ -1643,7 +1617,6 @@ impl Bank {
stakes_cache: StakesCache::new(stakes),
epoch_stakes: fields.epoch_stakes,
is_delta: AtomicBool::new(fields.is_delta),
runtime_config,
rewards: RwLock::new(vec![]),
cluster_type: Some(genesis_config.cluster_type),
lazy_rent_collection: AtomicBool::default(),
Expand All @@ -1661,7 +1634,6 @@ impl Bank {
accounts_data_size_initial,
accounts_data_size_delta_on_chain: AtomicI64::new(0),
accounts_data_size_delta_off_chain: AtomicI64::new(0),
fee_structure: FeeStructure::default(),
epoch_reward_status: fields.epoch_reward_status,
transaction_processor: TransactionBatchProcessor::default(),
check_program_modification_slot: false,
Expand All @@ -1673,8 +1645,7 @@ impl Bank {
bank.slot,
bank.epoch,
bank.epoch_schedule.clone(),
bank.fee_structure.clone(),
bank.runtime_config.clone(),
runtime_config,
Arc::new(RwLock::new(ProgramCache::new(fields.slot, fields.epoch))),
HashSet::default(),
);
Expand Down Expand Up @@ -3163,7 +3134,7 @@ impl Bank {
message: &SanitizedMessage,
lamports_per_signature: u64,
) -> u64 {
self.fee_structure.calculate_fee(
self.fee_structure().calculate_fee(
message,
lamports_per_signature,
&process_compute_budget_instructions(message.program_instructions_iter())
Expand Down Expand Up @@ -3324,7 +3295,7 @@ impl Bank {
/// Get the max number of accounts that a transaction may lock in this block
pub fn get_transaction_account_lock_limit(&self) -> usize {
if let Some(transaction_account_lock_limit) =
self.runtime_config.transaction_account_lock_limit
self.runtime_config().transaction_account_lock_limit
{
transaction_account_lock_limit
} else if self
Expand Down Expand Up @@ -4027,7 +3998,7 @@ impl Bank {
)?;

if !FeeStructure::to_clear_transaction_fee(lamports_per_signature) {
let fee_details = self.fee_structure.calculate_fee_details(
let fee_details = self.fee_structure().calculate_fee_details(
message,
&process_compute_budget_instructions(message.program_instructions_iter())
.unwrap_or_default()
Expand Down Expand Up @@ -5223,15 +5194,15 @@ impl Bank {
program_cache.environments.program_runtime_v1 = Arc::new(
create_program_runtime_environment_v1(
&self.feature_set,
&self.runtime_config.compute_budget.unwrap_or_default(),
&self.runtime_config().compute_budget.unwrap_or_default(),
false, /* deployment */
false, /* debugging_features */
)
.unwrap(),
);
program_cache.environments.program_runtime_v2 =
Arc::new(create_program_runtime_environment_v2(
&self.runtime_config.compute_budget.unwrap_or_default(),
&self.runtime_config().compute_budget.unwrap_or_default(),
false, /* debugging_features */
));
}
Expand Down Expand Up @@ -6751,8 +6722,17 @@ impl Bank {
.load_program_with_pubkey(self, pubkey, reload, effective_epoch)
}

pub fn get_transaction_processor(&self) -> &TransactionBatchProcessor<BankForks> {
&self.transaction_processor
pub fn fee_structure(&self) -> &FeeStructure {
&self.transaction_processor.fee_structure
}

pub fn runtime_config(&self) -> &RuntimeConfig {
&self.transaction_processor.runtime_config
}

pub fn add_builtin(&self, program_id: Pubkey, name: &str, builtin: ProgramCacheEntry) {
self.transaction_processor
.add_builtin(self, program_id, name, builtin)
}
}

Expand Down Expand Up @@ -7038,6 +7018,14 @@ impl Bank {
&self.transaction_processor.program_cache.read().unwrap(),
)
}

pub fn get_transaction_processor(&self) -> &TransactionBatchProcessor<BankForks> {
&self.transaction_processor
}

pub fn set_fee_structure(&mut self, fee_structure: &FeeStructure) {
self.transaction_processor.fee_structure = fee_structure.clone();
}
}

/// Compute how much an account has changed size. This function is useful when the data size delta
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank/builtins/core_bpf_migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl Bank {
// environment, as well as the two `ProgramCacheForTxBatch`
// instances configured above, then invoke the loader.
{
let compute_budget = self.runtime_config.compute_budget.unwrap_or_default();
let compute_budget = self.runtime_config().compute_budget.unwrap_or_default();
let mut sysvar_cache = SysvarCache::default();
sysvar_cache.fill_missing_entries(|pubkey, set_sysvar| {
if let Some(account) = self.get_account(pubkey) {
Expand Down
4 changes: 2 additions & 2 deletions runtime/src/bank/fee_distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ impl Bank {
fee_budget_limits: &FeeBudgetLimits,
) -> u64 {
let (reward, _burn) = if self.feature_set.is_active(&reward_full_priority_fee::id()) {
let fee_details = self.fee_structure.calculate_fee_details(
let fee_details = self.fee_structure().calculate_fee_details(
transaction.message(),
fee_budget_limits,
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
);
self.calculate_reward_and_burn_fee_details(&CollectorFeeDetails::from(fee_details))
} else {
let fee = self.fee_structure.calculate_fee(
let fee = self.fee_structure().calculate_fee(
transaction.message(),
5_000, // this just needs to be non-zero
fee_budget_limits,
Expand Down
12 changes: 6 additions & 6 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2648,7 +2648,7 @@ fn test_bank_tx_compute_unit_fee() {
.fee_rate_governor
.create_fee_calculator()
.lamports_per_signature,
&bank.fee_structure,
bank.fee_structure(),
);

let (expected_fee_collected, expected_fee_burned) =
Expand Down Expand Up @@ -2773,7 +2773,7 @@ fn test_bank_blockhash_fee_structure() {
let cheap_fee = calculate_test_fee(
&new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))),
cheap_lamports_per_signature,
&bank.fee_structure,
bank.fee_structure(),
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
Expand All @@ -2789,7 +2789,7 @@ fn test_bank_blockhash_fee_structure() {
let expensive_fee = calculate_test_fee(
&new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))),
expensive_lamports_per_signature,
&bank.fee_structure,
bank.fee_structure(),
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
Expand Down Expand Up @@ -2835,7 +2835,7 @@ fn test_bank_blockhash_compute_unit_fee_structure() {
let cheap_fee = calculate_test_fee(
&new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))),
cheap_lamports_per_signature,
&bank.fee_structure,
bank.fee_structure(),
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
Expand All @@ -2851,7 +2851,7 @@ fn test_bank_blockhash_compute_unit_fee_structure() {
let expensive_fee = calculate_test_fee(
&new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))),
expensive_lamports_per_signature,
&bank.fee_structure,
bank.fee_structure(),
);
assert_eq!(
bank.get_balance(&mint_keypair.pubkey()),
Expand Down Expand Up @@ -2965,7 +2965,7 @@ fn test_filter_program_errors_and_collect_compute_unit_fee() {
.fee_rate_governor
.create_fee_calculator()
.lamports_per_signature,
&bank.fee_structure,
bank.fee_structure(),
) * 2
)
.0
Expand Down
Loading

0 comments on commit bbe4705

Please sign in to comment.