Skip to content

Commit

Permalink
Fix - program loading with effective slot at epoch boundary (#35283)
Browse files Browse the repository at this point in the history
* Always limit effective slot to the begin of the current epoch.

* Adds comments.

* Optimizes to avoid having two entries if there is no relevant feature activation.

* Adds test_feature_activation_loaded_programs_epoch_transition().

(cherry picked from commit 2891ce8)
  • Loading branch information
Lichtso committed Feb 23, 2024
1 parent 5b24097 commit 60d4446
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 4 deletions.
19 changes: 15 additions & 4 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4811,11 +4811,22 @@ impl Bank {

let mut timings = ExecuteDetailsTimings::default();
load_program_metrics.submit_datapoint(&mut timings);
if !Arc::ptr_eq(
&environments.program_runtime_v1,
&loaded_programs_cache.environments.program_runtime_v1,
) || !Arc::ptr_eq(
&environments.program_runtime_v2,
&loaded_programs_cache.environments.program_runtime_v2,
) {
// There can be two entries per program when the environment changes.
// One for the old environment before the epoch boundary and one for the new environment after the epoch boundary.
// These two entries have the same deployment slot, so they must differ in their effective slot instead.
// This is done by setting the effective slot of the entry for the new environment to the epoch boundary.
loaded_program.effective_slot = loaded_program
.effective_slot
.max(self.epoch_schedule.get_first_slot_in_epoch(effective_epoch));
}
if let Some(recompile) = recompile {
loaded_program.effective_slot = loaded_program.effective_slot.max(
self.epoch_schedule()
.get_first_slot_in_epoch(effective_epoch),
);
loaded_program.tx_usage_counter =
AtomicU64::new(recompile.tx_usage_counter.load(Ordering::Relaxed));
loaded_program.ix_usage_counter =
Expand Down
54 changes: 54 additions & 0 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11970,6 +11970,60 @@ fn test_feature_activation_loaded_programs_recompilation_phase() {
);
}

#[test]
fn test_feature_activation_loaded_programs_epoch_transition() {
solana_logger::setup();

// Bank Setup
let (mut genesis_config, mint_keypair) = create_genesis_config(1_000_000 * LAMPORTS_PER_SOL);
genesis_config
.accounts
.remove(&feature_set::reject_callx_r10::id());
let (root_bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// Program Setup
let program_keypair = Keypair::new();
let program_data = include_bytes!("../../../programs/bpf_loader/test_elfs/out/noop_aligned.so");
let program_account = AccountSharedData::from(Account {
lamports: Rent::default().minimum_balance(program_data.len()).min(1),
data: program_data.to_vec(),
owner: bpf_loader::id(),
executable: true,
rent_epoch: 0,
});
root_bank.store_account(&program_keypair.pubkey(), &program_account);

// Compose message using the desired program.
let instruction = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new());
let message = Message::new(&[instruction], Some(&mint_keypair.pubkey()));
let binding = mint_keypair.insecure_clone();
let signers = vec![&binding];

// Advance the bank so that the program becomes effective.
goto_end_of_slot(root_bank.clone());
let bank = new_from_parent_with_fork_next_slot(root_bank, bank_forks.as_ref());

// Load the program with the old environment.
let transaction = Transaction::new(&signers, message.clone(), bank.last_blockhash());
assert!(bank.process_transaction(&transaction).is_ok());

// Schedule feature activation to trigger a change of environment at the epoch boundary.
let feature_account_balance =
std::cmp::max(genesis_config.rent.minimum_balance(Feature::size_of()), 1);
bank.store_account(
&feature_set::reject_callx_r10::id(),
&feature::create_account(&Feature { activated_at: None }, feature_account_balance),
);

// Advance the bank to cross the epoch boundary and activate the feature.
goto_end_of_slot(bank.clone());
let bank = new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), 33);

// Load the program with the new environment.
let transaction = Transaction::new(&signers, message, bank.last_blockhash());
assert!(bank.process_transaction(&transaction).is_ok());
}

#[test]
fn test_bank_verify_accounts_hash_with_base() {
let GenesisConfigInfo {
Expand Down

0 comments on commit 60d4446

Please sign in to comment.