Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: deploy program on last slot of epoch during environment change #101

Merged
merged 9 commits into from
Mar 22, 2024
Merged

Fix: deploy program on last slot of epoch during environment change #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
17f94d7
Fix: deploy program on last slot of epoch during environment change
pgarg66 Mar 6, 2024
bf97ccd
solana-runtime: deploy at last epoch slot test
alessandrod Mar 5, 2024
2db2892
fix tests
pgarg66 Mar 6, 2024
e438f8d
disable deployment of sol_alloc_free
pgarg66 Mar 6, 2024
edc773e
address review comments
pgarg66 Mar 6, 2024
0744297
address review comments
pgarg66 Mar 12, 2024
2796df2
Move tx-batch-constructor to its own function
pgarg66 Mar 20, 2024
04768d0
fix imports
pgarg66 Mar 20, 2024
6b09337
use new_from_cache
pgarg66 Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions ledger-tool/src/program.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ use {
},
solana_runtime::bank::Bank,
solana_sdk::{
account::AccountSharedData,
account::{create_account_shared_data_for_test, AccountSharedData},
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
pubkey::Pubkey,
slot_history::Slot,
sysvar,
transaction_context::{IndexOfAccount, InstructionAccount},
},
std::{
Expand Down Expand Up @@ -510,13 +511,16 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
program_id, // ID of the loaded program. It can modify accounts with the same owner key
AccountSharedData::new(0, 0, &loader_id),
));
transaction_accounts.push((
sysvar::epoch_schedule::id(),
create_account_shared_data_for_test(bank.epoch_schedule()),
));
let interpreted = matches.value_of("mode").unwrap() != "jit";
with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);

// Adding `DELAY_VISIBILITY_SLOT_OFFSET` to slots to accommodate for delay visibility of the program
let slot = bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET;
let mut loaded_programs =
LoadedProgramsForTxBatch::new(slot, bank.get_runtime_environments_for_slot(slot));
bank.new_program_cache_for_tx_batch_for_slot(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
for key in cached_account_keys {
loaded_programs.replenish(key, bank.load_program(&key, false, bank.epoch()));
debug!("Loaded program {}", key);
Expand Down
35 changes: 33 additions & 2 deletions program-runtime/src/invoke_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use {
crate::{
compute_budget::ComputeBudget,
ic_msg,
loaded_programs::{LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch},
loaded_programs::{
LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch, ProgramRuntimeEnvironments,
},
log_collector::LogCollector,
stable_log,
sysvar_cache::SysvarCache,
Expand All @@ -17,15 +19,18 @@ use {
vm::{Config, ContextObject, EbpfVm},
},
solana_sdk::{
account::AccountSharedData,
account::{create_account_shared_data_for_test, AccountSharedData},
bpf_loader_deprecated,
clock::Slot,
epoch_schedule::EpochSchedule,
feature_set::FeatureSet,
hash::Hash,
instruction::{AccountMeta, InstructionError},
native_loader,
pubkey::Pubkey,
saturating_add_assign,
stable_layout::stable_instruction::StableInstruction,
sysvar,
transaction_context::{
IndexOfAccount, InstructionAccount, TransactionAccount, TransactionContext,
},
Expand Down Expand Up @@ -209,6 +214,17 @@ impl<'a> InvokeContext<'a> {
.or_else(|| self.programs_loaded_for_tx_batch.find(pubkey))
}

pub fn get_environments_for_slot(
&self,
effective_slot: Slot,
) -> Result<&ProgramRuntimeEnvironments, InstructionError> {
let epoch_schedule = self.sysvar_cache.get_epoch_schedule()?;
pgarg66 marked this conversation as resolved.
Show resolved Hide resolved
let epoch = epoch_schedule.get_epoch(effective_slot);
Ok(self
.programs_loaded_for_tx_batch
.get_environments_for_epoch(epoch))
}

/// Push a stack frame onto the invocation stack
pub fn push(&mut self) -> Result<(), InstructionError> {
let instruction_context = self
Expand Down Expand Up @@ -713,6 +729,18 @@ pub fn mock_process_instruction<F: FnMut(&mut InvokeContext), G: FnMut(&mut Invo
program_indices.insert(0, transaction_accounts.len() as IndexOfAccount);
let processor_account = AccountSharedData::new(0, 0, &native_loader::id());
transaction_accounts.push((*loader_id, processor_account));
let pop_epoch_schedule_account = if !transaction_accounts
.iter()
.any(|(key, _)| *key == sysvar::epoch_schedule::id())
{
transaction_accounts.push((
sysvar::epoch_schedule::id(),
create_account_shared_data_for_test(&EpochSchedule::default()),
));
true
} else {
false
};
with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
let mut programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default();
programs_loaded_for_tx_batch.replenish(
Expand All @@ -731,6 +759,9 @@ pub fn mock_process_instruction<F: FnMut(&mut InvokeContext), G: FnMut(&mut Invo
assert_eq!(result, expected_result);
post_adjustments(&mut invoke_context);
let mut transaction_accounts = transaction_context.deconstruct_without_keys().unwrap();
if pop_epoch_schedule_account {
transaction_accounts.pop();
}
transaction_accounts.pop();
transaction_accounts
}
Expand Down
93 changes: 73 additions & 20 deletions program-runtime/src/loaded_programs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,19 +618,61 @@ pub struct LoadedProgramsForTxBatch {
entries: HashMap<Pubkey, Arc<LoadedProgram>>,
slot: Slot,
pub environments: ProgramRuntimeEnvironments,
/// Anticipated replacement for `environments` at the next epoch.
///
/// This is `None` during most of an epoch, and only `Some` around the boundaries (at the end and beginning of an epoch).
/// More precisely, it starts with the recompilation phase a few hundred slots before the epoch boundary,
/// and it ends with the first rerooting after the epoch boundary.
pgarg66 marked this conversation as resolved.
Show resolved Hide resolved
/// Needed when a program is deployed at the last slot of an epoch, becomes effective in the next epoch.
/// So needs to be compiled with the environment for the next epoch.
pub upcoming_environments: Option<ProgramRuntimeEnvironments>,
/// The epoch of the last rerooting
pub latest_root_epoch: Epoch,
pub hit_max_limit: bool,
}

impl LoadedProgramsForTxBatch {
pub fn new(slot: Slot, environments: ProgramRuntimeEnvironments) -> Self {
pub fn new(
slot: Slot,
environments: ProgramRuntimeEnvironments,
upcoming_environments: Option<ProgramRuntimeEnvironments>,
latest_root_epoch: Epoch,
) -> Self {
Self {
entries: HashMap::new(),
slot,
environments,
upcoming_environments,
latest_root_epoch,
hit_max_limit: false,
}
}

pub fn new_from_cache<FG: ForkGraph>(
slot: Slot,
epoch: Epoch,
cache: &ProgramCache<FG>,
) -> Self {
Self {
entries: HashMap::new(),
slot,
environments: cache.get_environments_for_epoch(epoch).clone(),
upcoming_environments: cache.get_upcoming_environments_for_epoch(epoch),
latest_root_epoch: cache.latest_root_epoch,
hit_max_limit: false,
}
}

/// Returns the current environments depending on the given epoch
pub fn get_environments_for_epoch(&self, epoch: Epoch) -> &ProgramRuntimeEnvironments {
if epoch != self.latest_root_epoch {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but I think > instead of != would make the logic clearer (although I know that in other places we use !=, so feel free to leave != if you prefer to be consistent)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to leave it as is for now. I am not 100% sure why we have != check in the other function. Just want to make sure we don't oversee some actual condition.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I chose == and != over <= and > to avoid off-by-one-errors such as writing < instead of <=. That is all, don't see a semantic reason why we need to stick with ==.

if let Some(upcoming_environments) = self.upcoming_environments.as_ref() {
return upcoming_environments;
}
}
&self.environments
}

/// Refill the cache with a single entry. It's typically called during transaction loading, and
/// transaction processing (for program management instructions).
/// It replaces the existing entry (if any) with the provided entry. The return value contains
Expand Down Expand Up @@ -710,6 +752,17 @@ impl<FG: ForkGraph> ProgramCache<FG> {
&self.environments
}

/// Returns the upcoming environments depending on the given epoch
pub fn get_upcoming_environments_for_epoch(
&self,
epoch: Epoch,
) -> Option<ProgramRuntimeEnvironments> {
if epoch == self.latest_root_epoch {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I'd prefer <=

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to leave it as is for now. I am not 100% sure why we have != check in the other function. Just want to make sure we don't oversee some actual condition.

return self.upcoming_environments.clone();
}
None
}

/// Insert a single entry. It's typically called during transaction loading,
/// when the cache doesn't contain the entry corresponding to program `key`.
pub fn assign_program(&mut self, key: Pubkey, entry: Arc<LoadedProgram>) -> bool {
Expand Down Expand Up @@ -2057,7 +2110,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 3)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 4)),
];
let mut extracted = LoadedProgramsForTxBatch::new(22, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(22, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 20, 22));
Expand All @@ -2073,7 +2126,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(15, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(15, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 15));
Expand All @@ -2096,7 +2149,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(18, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(18, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 18));
Expand All @@ -2114,7 +2167,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(23, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(23, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 23));
Expand All @@ -2132,7 +2185,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(11, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(11, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 11));
Expand Down Expand Up @@ -2170,7 +2223,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(21, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(21, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

// Since the fork was pruned, we should not find the entry deployed at slot 20.
Expand All @@ -2187,7 +2240,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(27, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(27, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 27));
Expand Down Expand Up @@ -2219,7 +2272,7 @@ mod tests {
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program4, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(23, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(23, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 23));
Expand Down Expand Up @@ -2274,7 +2327,7 @@ mod tests {
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 12));
Expand All @@ -2294,7 +2347,7 @@ mod tests {
),
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program2, 11, 12));
Expand Down Expand Up @@ -2360,7 +2413,7 @@ mod tests {
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(19, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(19, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 19));
Expand All @@ -2374,7 +2427,7 @@ mod tests {
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(27, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(27, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 27));
Expand All @@ -2388,7 +2441,7 @@ mod tests {
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program3, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(22, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(22, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 20, 22));
Expand Down Expand Up @@ -2469,7 +2522,7 @@ mod tests {
cache.prune(10, 0);

let mut missing = vec![(program1, (LoadedProgramMatchCriteria::NoCriteria, 1))];
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

// The cache should have the program deployed at slot 0
Expand Down Expand Up @@ -2513,7 +2566,7 @@ mod tests {
(program1, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 20));
Expand All @@ -2523,7 +2576,7 @@ mod tests {
(program1, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(6, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(6, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 5, 6));
Expand All @@ -2537,7 +2590,7 @@ mod tests {
(program1, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 20));
Expand All @@ -2547,7 +2600,7 @@ mod tests {
(program1, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(6, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(6, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 6));
Expand All @@ -2561,7 +2614,7 @@ mod tests {
(program1, (LoadedProgramMatchCriteria::NoCriteria, 1)),
(program2, (LoadedProgramMatchCriteria::NoCriteria, 1)),
];
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone());
let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone(), None, 0);
cache.extract(&mut missing, &mut extracted, true);

assert!(match_slot(&extracted, &program1, 0, 20));
Expand Down
Loading
Loading