Refactor - Delay visibility of program un-/re-/deployment (#29654)

* Use three separate HashMaps instead of the enum TxBankExecutorCacheDiff.

* Replaces all places which deploy programs by a macro.

* Adds a feature gate.

* Adjust tests.

* Makes undeployment visible immediately.
This commit is contained in:
Alexander Meißner 2023-02-11 11:18:25 +01:00 committed by GitHub
parent 85af23613e
commit 6558c8fdc9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 337 additions and 307 deletions

View File

@ -9,87 +9,77 @@ use {
ops::Div, ops::Div,
sync::{ sync::{
atomic::{AtomicU64, Ordering::Relaxed}, atomic::{AtomicU64, Ordering::Relaxed},
Arc, RwLock, Arc,
}, },
}, },
}; };
/// Relation between a TransactionExecutorCacheEntry and its matching BankExecutorCacheEntry
#[repr(u8)]
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum TxBankExecutorCacheDiff {
/// The TransactionExecutorCacheEntry did not change and is the same as the BankExecutorCacheEntry.
None,
/// The TransactionExecutorCacheEntry was inserted, no matching BankExecutorCacheEntry exists, so it needs to be inserted.
Inserted,
/// The TransactionExecutorCacheEntry was replaced, the matching BankExecutorCacheEntry needs to be updated.
Updated,
}
/// An entry of the TransactionExecutorCache
#[derive(Debug)]
pub struct TransactionExecutorCacheEntry {
executor: Arc<dyn Executor>,
difference: TxBankExecutorCacheDiff,
}
/// A subset of the BankExecutorCache containing only the executors relevant to one transaction /// A subset of the BankExecutorCache containing only the executors relevant to one transaction
/// ///
/// The BankExecutorCache can not be updated directly as transaction batches are /// The BankExecutorCache can not be updated directly as transaction batches are
/// processed in parallel, which would cause a race condition. /// processed in parallel, which would cause a race condition.
#[derive(Default, Debug)] #[derive(Default, Debug)]
pub struct TransactionExecutorCache { pub struct TransactionExecutorCache {
pub executors: HashMap<Pubkey, TransactionExecutorCacheEntry>, /// Executors or tombstones which are visible during the transaction
pub visible: HashMap<Pubkey, Option<Arc<dyn Executor>>>,
/// Executors of programs which were re-/deploymed during the transaction
pub deployments: HashMap<Pubkey, Arc<dyn Executor>>,
/// Executors which were missing in the cache and not re-/deploymed during the transaction
pub add_to_cache: HashMap<Pubkey, Arc<dyn Executor>>,
} }
impl TransactionExecutorCache { impl TransactionExecutorCache {
pub fn new(executable_keys: impl Iterator<Item = (Pubkey, Arc<dyn Executor>)>) -> Self { pub fn new(executable_keys: impl Iterator<Item = (Pubkey, Arc<dyn Executor>)>) -> Self {
Self { Self {
executors: executable_keys visible: executable_keys
.map(|(key, executor)| { .map(|(id, executor)| (id, Some(executor)))
let entry = TransactionExecutorCacheEntry {
executor,
difference: TxBankExecutorCacheDiff::None,
};
(key, entry)
})
.collect(), .collect(),
deployments: HashMap::new(),
add_to_cache: HashMap::new(),
} }
} }
pub fn get(&self, key: &Pubkey) -> Option<Arc<dyn Executor>> { pub fn get(&self, key: &Pubkey) -> Option<Option<Arc<dyn Executor>>> {
self.executors.get(key).map(|entry| entry.executor.clone()) self.visible.get(key).cloned()
} }
pub fn set(&mut self, key: Pubkey, executor: Arc<dyn Executor>, replacement: bool) { pub fn set_tombstone(&mut self, key: Pubkey) {
let difference = if replacement { self.visible.insert(key, None);
TxBankExecutorCacheDiff::Updated
} else {
TxBankExecutorCacheDiff::Inserted
};
let entry = TransactionExecutorCacheEntry {
executor,
difference,
};
let _was_replaced = self.executors.insert(key, entry).is_some();
} }
pub fn update_global_cache( pub fn set(
&self, &mut self,
global_cache: &RwLock<BankExecutorCache>, key: Pubkey,
selector: impl Fn(TxBankExecutorCacheDiff) -> bool, executor: Arc<dyn Executor>,
upgrade: bool,
delay_visibility_of_program_deployment: bool,
) { ) {
let executors_delta: Vec<_> = self if upgrade {
.executors if delay_visibility_of_program_deployment {
.iter() // Place a tombstone in the cache so that
.filter_map(|(key, entry)| { // we don't load the new version from the database as it should remain invisible
selector(entry.difference).then(|| (key, entry.executor.clone())) self.set_tombstone(key);
}) } else {
.collect(); self.visible.insert(key, Some(executor.clone()));
if !executors_delta.is_empty() { }
global_cache.write().unwrap().put(&executors_delta); self.deployments.insert(key, executor);
} else {
self.visible.insert(key, Some(executor.clone()));
self.add_to_cache.insert(key, executor);
} }
} }
pub fn get_executors_added_to_the_cache(&mut self) -> HashMap<Pubkey, Arc<dyn Executor>> {
let mut executors = HashMap::new();
std::mem::swap(&mut executors, &mut self.add_to_cache);
executors
}
pub fn get_executors_which_were_deployed(&mut self) -> HashMap<Pubkey, Arc<dyn Executor>> {
let mut executors = HashMap::new();
std::mem::swap(&mut executors, &mut self.deployments);
executors
}
} }
/// Capacity of `BankExecutorCache` /// Capacity of `BankExecutorCache`
@ -197,18 +187,17 @@ impl BankExecutorCache {
} }
} }
fn put(&mut self, executors: &[(&Pubkey, Arc<dyn Executor>)]) { pub fn put(&mut self, executors: impl Iterator<Item = (Pubkey, Arc<dyn Executor>)>) {
let mut new_executors: Vec<_> = executors let mut new_executors: Vec<_> = executors
.iter()
.filter_map(|(key, executor)| { .filter_map(|(key, executor)| {
if let Some(mut entry) = self.remove(key) { if let Some(mut entry) = self.remove(&key) {
self.stats.replacements.fetch_add(1, Relaxed); self.stats.replacements.fetch_add(1, Relaxed);
entry.executor = executor.clone(); entry.executor = executor.clone();
let _ = self.executors.insert(**key, entry); let _ = self.executors.insert(key, entry);
None None
} else { } else {
self.stats.insertions.fetch_add(1, Relaxed); self.stats.insertions.fetch_add(1, Relaxed);
Some((*key, executor)) Some((key, executor))
} }
}) })
.collect(); .collect();
@ -251,7 +240,7 @@ impl BankExecutorCache {
executor: executor.clone(), executor: executor.clone(),
hit_count: AtomicU64::new(1), hit_count: AtomicU64::new(1),
}; };
let _ = self.executors.insert(*key, entry); let _ = self.executors.insert(key, entry);
} }
} }
} }
@ -388,9 +377,9 @@ mod tests {
let executor: Arc<dyn Executor> = Arc::new(TestExecutor {}); let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
let mut cache = BankExecutorCache::new(3, 0); let mut cache = BankExecutorCache::new(3, 0);
cache.put(&[(&key1, executor.clone())]); cache.put([(key1, executor.clone())].into_iter());
cache.put(&[(&key2, executor.clone())]); cache.put([(key2, executor.clone())].into_iter());
cache.put(&[(&key3, executor.clone())]); cache.put([(key3, executor.clone())].into_iter());
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
assert!(cache.get(&key2).is_some()); assert!(cache.get(&key2).is_some());
assert!(cache.get(&key3).is_some()); assert!(cache.get(&key3).is_some());
@ -398,7 +387,7 @@ mod tests {
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
assert!(cache.get(&key2).is_some()); assert!(cache.get(&key2).is_some());
cache.put(&[(&key4, executor.clone())]); cache.put([(key4, executor.clone())].into_iter());
assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some());
let num_retained = [&key1, &key2, &key3] let num_retained = [&key1, &key2, &key3]
.iter() .iter()
@ -409,7 +398,7 @@ mod tests {
assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some());
assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some());
assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some());
cache.put(&[(&key3, executor.clone())]); cache.put([(key3, executor.clone())].into_iter());
assert!(cache.get(&key3).is_some()); assert!(cache.get(&key3).is_some());
let num_retained = [&key1, &key2, &key4] let num_retained = [&key1, &key2, &key4]
.iter() .iter()
@ -428,9 +417,9 @@ mod tests {
let mut cache = BankExecutorCache::new(3, 0); let mut cache = BankExecutorCache::new(3, 0);
assert!(cache.current_epoch == 0); assert!(cache.current_epoch == 0);
cache.put(&[(&key1, executor.clone())]); cache.put([(key1, executor.clone())].into_iter());
cache.put(&[(&key2, executor.clone())]); cache.put([(key2, executor.clone())].into_iter());
cache.put(&[(&key3, executor.clone())]); cache.put([(key3, executor.clone())].into_iter());
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
@ -441,7 +430,7 @@ mod tests {
assert!(cache.get(&key2).is_some()); assert!(cache.get(&key2).is_some());
assert!(cache.get(&key2).is_some()); assert!(cache.get(&key2).is_some());
assert!(cache.get(&key3).is_some()); assert!(cache.get(&key3).is_some());
cache.put(&[(&key4, executor.clone())]); cache.put([(key4, executor.clone())].into_iter());
assert!(cache.get(&key4).is_some()); assert!(cache.get(&key4).is_some());
let num_retained = [&key1, &key2, &key3] let num_retained = [&key1, &key2, &key3]
@ -450,8 +439,8 @@ mod tests {
.count(); .count();
assert_eq!(num_retained, 2); assert_eq!(num_retained, 2);
cache.put(&[(&key1, executor.clone())]); cache.put([(key1, executor.clone())].into_iter());
cache.put(&[(&key3, executor.clone())]); cache.put([(key3, executor.clone())].into_iter());
assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some());
assert!(cache.get(&key3).is_some()); assert!(cache.get(&key3).is_some());
let num_retained = [&key2, &key4] let num_retained = [&key2, &key4]
@ -463,7 +452,7 @@ mod tests {
cache = BankExecutorCache::new_from_parent_bank_executors(&cache, 2); cache = BankExecutorCache::new_from_parent_bank_executors(&cache, 2);
assert!(cache.current_epoch == 2); assert!(cache.current_epoch == 2);
cache.put(&[(&key3, executor.clone())]); cache.put([(key3, executor.clone())].into_iter());
assert!(cache.get(&key3).is_some()); assert!(cache.get(&key3).is_some());
} }
@ -475,11 +464,11 @@ mod tests {
let executor: Arc<dyn Executor> = Arc::new(TestExecutor {}); let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
let mut cache = BankExecutorCache::new(2, 0); let mut cache = BankExecutorCache::new(2, 0);
cache.put(&[(&key1, executor.clone())]); cache.put([(key1, executor.clone())].into_iter());
for _ in 0..5 { for _ in 0..5 {
let _ = cache.get(&key1); let _ = cache.get(&key1);
} }
cache.put(&[(&key2, executor.clone())]); cache.put([(key2, executor.clone())].into_iter());
// make key1's use-count for sure greater than key2's // make key1's use-count for sure greater than key2's
let _ = cache.get(&key1); let _ = cache.get(&key1);
@ -491,7 +480,7 @@ mod tests {
entries.sort_by_key(|(_, v)| *v); entries.sort_by_key(|(_, v)| *v);
assert!(entries[0].1 < entries[1].1); assert!(entries[0].1 < entries[1].1);
cache.put(&[(&key3, executor.clone())]); cache.put([(key3, executor.clone())].into_iter());
assert!(cache.get(&entries[0].0).is_none()); assert!(cache.get(&entries[0].0).is_none());
assert!(cache.get(&entries[1].0).is_some()); assert!(cache.get(&entries[1].0).is_some());
} }
@ -508,10 +497,10 @@ mod tests {
assert_eq!(cache.stats.one_hit_wonders.load(Relaxed), 0); assert_eq!(cache.stats.one_hit_wonders.load(Relaxed), 0);
// add our one-hit-wonder // add our one-hit-wonder
cache.put(&[(&one_hit_wonder, executor.clone())]); cache.put([(one_hit_wonder, executor.clone())].into_iter());
assert_eq!(cache.executors[&one_hit_wonder].hit_count.load(Relaxed), 1); assert_eq!(cache.executors[&one_hit_wonder].hit_count.load(Relaxed), 1);
// displace the one-hit-wonder with "popular program" // displace the one-hit-wonder with "popular program"
cache.put(&[(&popular, executor.clone())]); cache.put([(popular, executor.clone())].into_iter());
assert_eq!(cache.executors[&popular].hit_count.load(Relaxed), 1); assert_eq!(cache.executors[&popular].hit_count.load(Relaxed), 1);
// one-hit-wonder counter incremented // one-hit-wonder counter incremented
@ -522,7 +511,7 @@ mod tests {
assert_eq!(cache.executors[&popular].hit_count.load(Relaxed), 2); assert_eq!(cache.executors[&popular].hit_count.load(Relaxed), 2);
// evict "popular program" // evict "popular program"
cache.put(&[(&one_hit_wonder, executor.clone())]); cache.put([(one_hit_wonder, executor.clone())].into_iter());
assert_eq!(cache.executors[&one_hit_wonder].hit_count.load(Relaxed), 1); assert_eq!(cache.executors[&one_hit_wonder].hit_count.load(Relaxed), 1);
// one-hit-wonder counter not incremented // one-hit-wonder counter not incremented
@ -593,13 +582,13 @@ mod tests {
assert_eq!(ComparableStats::from(&cache.stats), expected_stats,); assert_eq!(ComparableStats::from(&cache.stats), expected_stats,);
// insert some executors // insert some executors
cache.put(&[(&program_id1, executor.clone())]); cache.put([(program_id1, executor.clone())].into_iter());
cache.put(&[(&program_id2, executor.clone())]); cache.put([(program_id2, executor.clone())].into_iter());
expected_stats.insertions += 2; expected_stats.insertions += 2;
assert_eq!(ComparableStats::from(&cache.stats), expected_stats); assert_eq!(ComparableStats::from(&cache.stats), expected_stats);
// replace a one-hit-wonder executor // replace a one-hit-wonder executor
cache.put(&[(&program_id1, executor.clone())]); cache.put([(program_id1, executor.clone())].into_iter());
expected_stats.replacements += 1; expected_stats.replacements += 1;
expected_stats.one_hit_wonders += 1; expected_stats.one_hit_wonders += 1;
assert_eq!(ComparableStats::from(&cache.stats), expected_stats); assert_eq!(ComparableStats::from(&cache.stats), expected_stats);
@ -617,7 +606,7 @@ mod tests {
assert_eq!(ComparableStats::from(&cache.stats), expected_stats); assert_eq!(ComparableStats::from(&cache.stats), expected_stats);
// evict an executor // evict an executor
cache.put(&[(&Pubkey::new_unique(), executor.clone())]); cache.put([(Pubkey::new_unique(), executor.clone())].into_iter());
expected_stats.insertions += 1; expected_stats.insertions += 1;
expected_stats.evictions.insert(program_id2, 1); expected_stats.evictions.insert(program_id2, 1);
assert_eq!(ComparableStats::from(&cache.stats), expected_stats); assert_eq!(ComparableStats::from(&cache.stats), expected_stats);

View File

@ -41,9 +41,10 @@ use {
entrypoint::{HEAP_LENGTH, SUCCESS}, entrypoint::{HEAP_LENGTH, SUCCESS},
feature_set::{ feature_set::{
cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts, cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts,
check_slice_translation_size, disable_deploy_of_alloc_free_syscall, check_slice_translation_size, delay_visibility_of_program_deployment,
enable_bpf_loader_extend_program_ix, enable_bpf_loader_set_authority_checked_ix, disable_deploy_of_alloc_free_syscall, enable_bpf_loader_extend_program_ix,
enable_program_redeployment_cooldown, limit_max_instruction_trace_length, FeatureSet, enable_bpf_loader_set_authority_checked_ix, enable_program_redeployment_cooldown,
limit_max_instruction_trace_length, FeatureSet,
}, },
instruction::{AccountMeta, InstructionError}, instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction, loader_instruction::LoaderInstruction,
@ -228,8 +229,14 @@ pub fn create_executor_from_account(
}; };
if let Some(ref tx_executor_cache) = tx_executor_cache { if let Some(ref tx_executor_cache) = tx_executor_cache {
if let Some(executor) = tx_executor_cache.get(program.get_key()) { match tx_executor_cache.get(program.get_key()) {
return Ok((executor, None)); // Executor exists and is cached, use it
Some(Some(executor)) => return Ok((executor, None)),
// We cached that the Executor does not exist, abort
// This case can only happen once delay_visibility_of_program_deployment is active.
Some(None) => return Err(InstructionError::InvalidAccountData),
// Nothing cached, try to load from account instead
None => {}
} }
} }
@ -250,11 +257,44 @@ pub fn create_executor_from_account(
false, /* reject_deployment_of_broken_elfs */ false, /* reject_deployment_of_broken_elfs */
)?; )?;
if let Some(mut tx_executor_cache) = tx_executor_cache { if let Some(mut tx_executor_cache) = tx_executor_cache {
tx_executor_cache.set(*program.get_key(), executor.clone(), false); tx_executor_cache.set(
*program.get_key(),
executor.clone(),
false,
feature_set.is_active(&delay_visibility_of_program_deployment::id()),
);
} }
Ok((executor, Some(create_executor_metrics))) Ok((executor, Some(create_executor_metrics)))
} }
macro_rules! deploy_program {
($invoke_context:expr, $use_jit:expr, $program_id:expr,
$drop:expr, $new_programdata:expr $(,)?) => {{
let delay_visibility_of_program_deployment = $invoke_context
.feature_set
.is_active(&delay_visibility_of_program_deployment::id());
let mut create_executor_metrics = CreateMetrics::default();
let executor = create_executor_from_bytes(
&$invoke_context.feature_set,
$invoke_context.get_compute_budget(),
$invoke_context.get_log_collector(),
&mut create_executor_metrics,
$new_programdata,
$use_jit,
true,
)?;
$drop
create_executor_metrics.program_id = $program_id.to_string();
create_executor_metrics.submit_datapoint(&mut $invoke_context.timings);
$invoke_context.tx_executor_cache.borrow_mut().set(
$program_id,
executor,
true,
delay_visibility_of_program_deployment,
);
}};
}
fn write_program_data( fn write_program_data(
program_account_index: IndexOfAccount, program_account_index: IndexOfAccount,
program_data_offset: usize, program_data_offset: usize,
@ -642,26 +682,18 @@ fn process_loader_upgradeable_instruction(
let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_context = transaction_context.get_current_instruction_context()?;
let buffer = let buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 3)?; instruction_context.try_borrow_instruction_account(transaction_context, 3)?;
let mut create_executor_metrics = CreateMetrics::default(); deploy_program!(
let executor = create_executor_from_bytes( invoke_context,
&invoke_context.feature_set, use_jit,
invoke_context.get_compute_budget(), new_program_id,
invoke_context.get_log_collector(), {
&mut create_executor_metrics, drop(buffer);
},
buffer buffer
.get_data() .get_data()
.get(buffer_data_offset..) .get(buffer_data_offset..)
.ok_or(InstructionError::AccountDataTooSmall)?, .ok_or(InstructionError::AccountDataTooSmall)?,
use_jit, );
true,
)?;
drop(buffer);
create_executor_metrics.program_id = new_program_id.to_string();
create_executor_metrics.submit_datapoint(&mut invoke_context.timings);
invoke_context
.tx_executor_cache
.borrow_mut()
.set(new_program_id, executor, true);
let transaction_context = &invoke_context.transaction_context; let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_context = transaction_context.get_current_instruction_context()?;
@ -835,26 +867,18 @@ fn process_loader_upgradeable_instruction(
// Load and verify the program bits // Load and verify the program bits
let buffer = let buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 2)?; instruction_context.try_borrow_instruction_account(transaction_context, 2)?;
let mut create_executor_metrics = CreateMetrics::default(); deploy_program!(
let executor = create_executor_from_bytes( invoke_context,
&invoke_context.feature_set, use_jit,
invoke_context.get_compute_budget(), new_program_id,
invoke_context.get_log_collector(), {
&mut create_executor_metrics, drop(buffer);
},
buffer buffer
.get_data() .get_data()
.get(buffer_data_offset..) .get(buffer_data_offset..)
.ok_or(InstructionError::AccountDataTooSmall)?, .ok_or(InstructionError::AccountDataTooSmall)?,
use_jit, );
true,
)?;
drop(buffer);
create_executor_metrics.program_id = new_program_id.to_string();
create_executor_metrics.submit_datapoint(&mut invoke_context.timings);
invoke_context
.tx_executor_cache
.borrow_mut()
.set(new_program_id, executor, true);
let transaction_context = &invoke_context.transaction_context; let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_context = transaction_context.get_current_instruction_context()?;
@ -1142,6 +1166,15 @@ fn process_loader_upgradeable_instruction(
instruction_context, instruction_context,
&log_collector, &log_collector,
)?; )?;
if invoke_context
.feature_set
.is_active(&delay_visibility_of_program_deployment::id())
{
invoke_context
.tx_executor_cache
.borrow_mut()
.set_tombstone(program_key);
}
} }
_ => { _ => {
ic_logger_msg!(log_collector, "Invalid Program account"); ic_logger_msg!(log_collector, "Invalid Program account");
@ -1358,22 +1391,13 @@ fn process_loader_instruction(
ic_msg!(invoke_context, "key[0] did not sign the transaction"); ic_msg!(invoke_context, "key[0] did not sign the transaction");
return Err(InstructionError::MissingRequiredSignature); return Err(InstructionError::MissingRequiredSignature);
} }
let mut create_executor_metrics = CreateMetrics::default(); deploy_program!(
let executor = create_executor_from_bytes( invoke_context,
&invoke_context.feature_set,
invoke_context.get_compute_budget(),
invoke_context.get_log_collector(),
&mut create_executor_metrics,
program.get_data(),
use_jit, use_jit,
true, *program.get_key(),
)?; {},
create_executor_metrics.program_id = program.get_key().to_string(); program.get_data(),
create_executor_metrics.submit_datapoint(&mut invoke_context.timings); );
invoke_context
.tx_executor_cache
.borrow_mut()
.set(*program.get_key(), executor, true);
program.set_executable(true)?; program.set_executable(true)?;
ic_msg!(invoke_context, "Finalized account {:?}", program.get_key()); ic_msg!(invoke_context, "Finalized account {:?}", program.get_key());
} }

View File

@ -1824,14 +1824,11 @@ fn test_program_sbf_invoke_in_same_tx_as_deployment() {
) )
.unwrap(); .unwrap();
// Deployment is invisible to top-level-instructions but visible to CPI instructions // Deployment is invisible to both top-level-instructions and CPI instructions
for (invoke_instruction, expected) in [ for (index, invoke_instruction) in [invoke_instruction, indirect_invoke_instruction]
( .into_iter()
invoke_instruction, .enumerate()
Err(TransactionError::ProgramAccountNotFound), {
),
(indirect_invoke_instruction, Ok(())),
] {
let mut instructions = deployment_instructions.clone(); let mut instructions = deployment_instructions.clone();
instructions.push(invoke_instruction); instructions.push(invoke_instruction);
let tx = Transaction::new( let tx = Transaction::new(
@ -1839,11 +1836,18 @@ fn test_program_sbf_invoke_in_same_tx_as_deployment() {
Message::new(&instructions, Some(&mint_keypair.pubkey())), Message::new(&instructions, Some(&mint_keypair.pubkey())),
bank.last_blockhash(), bank.last_blockhash(),
); );
let results = execute_transactions(&bank, vec![tx]); if index == 0 {
if let Err(err) = expected { let results = execute_transactions(&bank, vec![tx]);
assert_eq!(results[0].as_ref().unwrap_err(), &err); assert_eq!(
results[0].as_ref().unwrap_err(),
&TransactionError::ProgramAccountNotFound,
);
} else { } else {
assert!(results[0].is_ok()); let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(2, InstructionError::InvalidAccountData),
);
} }
} }
} }
@ -1889,98 +1893,6 @@ fn test_program_sbf_invoke_in_same_tx_as_redeployment() {
"solana_sbf_rust_invoke_and_return", "solana_sbf_rust_invoke_and_return",
); );
let invoke_instruction =
Instruction::new_with_bytes(program_id, &[0], vec![AccountMeta::new(clock::id(), false)]);
let indirect_invoke_instruction = Instruction::new_with_bytes(
indirect_program_keypair.pubkey(),
&[0],
vec![
AccountMeta::new_readonly(program_id, false),
AccountMeta::new_readonly(clock::id(), false),
],
);
// Prepare redeployment
load_upgradeable_buffer(
&bank_client,
&mint_keypair,
&buffer_keypair,
&authority_keypair,
"solana_sbf_rust_panic",
);
let redeployment_instruction = bpf_loader_upgradeable::upgrade(
&program_id,
&buffer_keypair.pubkey(),
&authority_keypair.pubkey(),
&mint_keypair.pubkey(),
);
// Redeployment is visible to both top-level-instructions and CPI instructions
for invoke_instruction in [invoke_instruction, indirect_invoke_instruction] {
// Call upgradeable program
let result =
bank_client.send_and_confirm_instruction(&mint_keypair, invoke_instruction.clone());
assert!(result.is_ok());
// Upgrade the program and invoke in same tx
let message = Message::new(
&[redeployment_instruction.clone(), invoke_instruction],
Some(&mint_keypair.pubkey()),
);
let tx = Transaction::new(
&[&mint_keypair, &authority_keypair],
message.clone(),
bank.last_blockhash(),
);
let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(1, InstructionError::ProgramFailedToComplete),
);
}
}
#[test]
#[cfg(feature = "sbf_rust")]
fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
solana_logger::setup();
let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = create_genesis_config(50);
let mut bank = Bank::new_for_tests(&genesis_config);
let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!();
bank.add_builtin(&name, &id, entrypoint);
let bank = Arc::new(bank);
let bank_client = BankClient::new_shared(&bank);
// Deploy upgradeable program
let buffer_keypair = Keypair::new();
let program_keypair = Keypair::new();
let program_id = program_keypair.pubkey();
let authority_keypair = Keypair::new();
load_upgradeable_program(
&bank_client,
&mint_keypair,
&buffer_keypair,
&program_keypair,
&authority_keypair,
"solana_sbf_rust_noop",
);
// Deploy indirect invocation program
let indirect_program_keypair = Keypair::new();
load_upgradeable_program(
&bank_client,
&mint_keypair,
&buffer_keypair,
&indirect_program_keypair,
&authority_keypair,
"solana_sbf_rust_invoke_and_return",
);
// Deploy panic program // Deploy panic program
let panic_program_keypair = Keypair::new(); let panic_program_keypair = Keypair::new();
load_upgradeable_program( load_upgradeable_program(
@ -2002,8 +1914,99 @@ fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
AccountMeta::new_readonly(clock::id(), false), AccountMeta::new_readonly(clock::id(), false),
], ],
); );
let panic_instruction =
Instruction::new_with_bytes(panic_program_keypair.pubkey(), &[], vec![]); // Prepare redeployment
let buffer_keypair = Keypair::new();
load_upgradeable_buffer(
&bank_client,
&mint_keypair,
&buffer_keypair,
&authority_keypair,
"solana_sbf_rust_panic",
);
let redeployment_instruction = bpf_loader_upgradeable::upgrade(
&program_id,
&buffer_keypair.pubkey(),
&authority_keypair.pubkey(),
&mint_keypair.pubkey(),
);
// Redeployment causes programs to be unavailable to both top-level-instructions and CPI instructions
for invoke_instruction in [invoke_instruction, indirect_invoke_instruction] {
// Call upgradeable program
let result =
bank_client.send_and_confirm_instruction(&mint_keypair, invoke_instruction.clone());
assert!(result.is_ok());
// Upgrade the program and invoke in same tx
let message = Message::new(
&[redeployment_instruction.clone(), invoke_instruction],
Some(&mint_keypair.pubkey()),
);
let tx = Transaction::new(
&[&mint_keypair, &authority_keypair],
message.clone(),
bank.last_blockhash(),
);
let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(1, InstructionError::InvalidAccountData),
);
}
}
#[test]
#[cfg(feature = "sbf_rust")]
fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
solana_logger::setup();
let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = create_genesis_config(50);
let mut bank = Bank::new_for_tests(&genesis_config);
let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!();
bank.add_builtin(&name, &id, entrypoint);
let bank = Arc::new(bank);
let bank_client = BankClient::new_shared(&bank);
// Deploy upgradeable program
let buffer_keypair = Keypair::new();
let program_keypair = Keypair::new();
let program_id = program_keypair.pubkey();
let authority_keypair = Keypair::new();
load_upgradeable_program(
&bank_client,
&mint_keypair,
&buffer_keypair,
&program_keypair,
&authority_keypair,
"solana_sbf_rust_noop",
);
// Deploy indirect invocation program
let indirect_program_keypair = Keypair::new();
load_upgradeable_program(
&bank_client,
&mint_keypair,
&buffer_keypair,
&indirect_program_keypair,
&authority_keypair,
"solana_sbf_rust_invoke_and_return",
);
let invoke_instruction =
Instruction::new_with_bytes(program_id, &[0], vec![AccountMeta::new(clock::id(), false)]);
let indirect_invoke_instruction = Instruction::new_with_bytes(
indirect_program_keypair.pubkey(),
&[0],
vec![
AccountMeta::new_readonly(program_id, false),
AccountMeta::new_readonly(clock::id(), false),
],
);
// Prepare undeployment // Prepare undeployment
let (programdata_address, _) = Pubkey::find_program_address( let (programdata_address, _) = Pubkey::find_program_address(
@ -2017,7 +2020,7 @@ fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
Some(&program_id), Some(&program_id),
); );
// Undeployment is invisible to both top-level-instructions and CPI instructions // Undeployment is visible to both top-level-instructions and CPI instructions
for invoke_instruction in [invoke_instruction, indirect_invoke_instruction] { for invoke_instruction in [invoke_instruction, indirect_invoke_instruction] {
// Call upgradeable program // Call upgradeable program
let result = let result =
@ -2026,11 +2029,7 @@ fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
// Upgrade the program and invoke in same tx // Upgrade the program and invoke in same tx
let message = Message::new( let message = Message::new(
&[ &[undeployment_instruction.clone(), invoke_instruction],
undeployment_instruction.clone(),
invoke_instruction,
panic_instruction.clone(), // Make sure the TX fails, so we don't have to deploy another program
],
Some(&mint_keypair.pubkey()), Some(&mint_keypair.pubkey()),
); );
let tx = Transaction::new( let tx = Transaction::new(
@ -2041,7 +2040,7 @@ fn test_program_sbf_invoke_in_same_tx_as_undeployment() {
let (result, _, _) = process_transaction_and_record_inner(&bank, tx); let (result, _, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!( assert_eq!(
result.unwrap_err(), result.unwrap_err(),
TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete), TransactionError::InstructionError(1, InstructionError::InvalidAccountData),
); );
} }
} }

View File

@ -93,10 +93,7 @@ use {
accounts_data_meter::MAX_ACCOUNTS_DATA_LEN, accounts_data_meter::MAX_ACCOUNTS_DATA_LEN,
compute_budget::{self, ComputeBudget}, compute_budget::{self, ComputeBudget},
executor::Executor, executor::Executor,
executor_cache::{ executor_cache::{BankExecutorCache, TransactionExecutorCache, MAX_CACHED_EXECUTORS},
BankExecutorCache, TransactionExecutorCache, TxBankExecutorCacheDiff,
MAX_CACHED_EXECUTORS,
},
invoke_context::{BuiltinProgram, ProcessInstructionWithContext}, invoke_context::{BuiltinProgram, ProcessInstructionWithContext},
loaded_programs::{LoadedPrograms, WorkingSlot}, loaded_programs::{LoadedPrograms, WorkingSlot},
log_collector::LogCollector, log_collector::LogCollector,
@ -4027,22 +4024,36 @@ impl Bank {
Rc::new(RefCell::new(tx_executor_cache)) Rc::new(RefCell::new(tx_executor_cache))
} }
/// Add executors back to the bank's cache if they were missing and not updated /// Add executors back to the bank's cache if they were missing and not re-/deployed
fn store_missing_executors(&self, tx_executor_cache: &RefCell<TransactionExecutorCache>) { fn store_executors_which_added_to_the_cache(
tx_executor_cache &self,
.borrow() tx_executor_cache: &RefCell<TransactionExecutorCache>,
.update_global_cache(&self.executor_cache, |difference| { ) {
difference == TxBankExecutorCacheDiff::Inserted let executors = tx_executor_cache
}); .borrow_mut()
.get_executors_added_to_the_cache();
if !executors.is_empty() {
self.executor_cache
.write()
.unwrap()
.put(executors.into_iter());
}
} }
/// Add updated executors back to the bank's cache /// Add re-/deployed executors to the bank's cache
fn store_updated_executors(&self, tx_executor_cache: &RefCell<TransactionExecutorCache>) { fn store_executors_which_were_deployed(
tx_executor_cache &self,
.borrow() tx_executor_cache: &RefCell<TransactionExecutorCache>,
.update_global_cache(&self.executor_cache, |difference| { ) {
difference == TxBankExecutorCacheDiff::Updated let executors = tx_executor_cache
}); .borrow_mut()
.get_executors_which_were_deployed();
if !executors.is_empty() {
self.executor_cache
.write()
.unwrap()
.put(executors.into_iter());
}
} }
#[allow(dead_code)] // Preparation for BankExecutorCache rework #[allow(dead_code)] // Preparation for BankExecutorCache rework
@ -4219,12 +4230,13 @@ impl Bank {
process_message_time.as_us() process_message_time.as_us()
); );
let mut store_missing_executors_time = Measure::start("store_missing_executors_time"); let mut store_executors_which_added_to_the_cache_time =
self.store_missing_executors(&tx_executor_cache); Measure::start("store_executors_which_added_to_the_cache_time");
store_missing_executors_time.stop(); self.store_executors_which_added_to_the_cache(&tx_executor_cache);
store_executors_which_added_to_the_cache_time.stop();
saturating_add_assign!( saturating_add_assign!(
timings.execute_accessories.update_executors_us, timings.execute_accessories.update_executors_us,
store_missing_executors_time.as_us() store_executors_which_added_to_the_cache_time.as_us()
); );
let status = process_result let status = process_result
@ -4903,7 +4915,8 @@ impl Bank {
sanitized_txs.len() sanitized_txs.len()
); );
let mut store_updated_executors_time = Measure::start("store_updated_executors_time"); let mut store_executors_which_were_deployed_time =
Measure::start("store_executors_which_were_deployed_time");
for execution_result in &execution_results { for execution_result in &execution_results {
if let TransactionExecutionResult::Executed { if let TransactionExecutionResult::Executed {
details, details,
@ -4911,14 +4924,14 @@ impl Bank {
} = execution_result } = execution_result
{ {
if details.status.is_ok() { if details.status.is_ok() {
self.store_updated_executors(tx_executor_cache); self.store_executors_which_were_deployed(tx_executor_cache);
} }
} }
} }
store_updated_executors_time.stop(); store_executors_which_were_deployed_time.stop();
saturating_add_assign!( saturating_add_assign!(
timings.execute_accessories.update_executors_us, timings.execute_accessories.update_executors_us,
store_updated_executors_time.as_us() store_executors_which_were_deployed_time.as_us()
); );
let accounts_data_len_delta = execution_results let accounts_data_len_delta = execution_results

View File

@ -7708,42 +7708,42 @@ fn test_bank_executor_cache() {
let executors = let executors =
TransactionExecutorCache::new((0..4).map(|i| (accounts[i].0, executor.clone()))); TransactionExecutorCache::new((0..4).map(|i| (accounts[i].0, executor.clone())));
let executors = Rc::new(RefCell::new(executors)); let executors = Rc::new(RefCell::new(executors));
bank.store_missing_executors(&executors); bank.store_executors_which_added_to_the_cache(&executors);
bank.store_updated_executors(&executors); bank.store_executors_which_were_deployed(&executors);
let stored_executors = bank.get_tx_executor_cache(accounts); let stored_executors = bank.get_tx_executor_cache(accounts);
assert_eq!(stored_executors.borrow().executors.len(), 0); assert_eq!(stored_executors.borrow().visible.len(), 0);
// do work // do work
let mut executors = let mut executors =
TransactionExecutorCache::new((2..3).map(|i| (accounts[i].0, executor.clone()))); TransactionExecutorCache::new((2..3).map(|i| (accounts[i].0, executor.clone())));
executors.set(key1, executor.clone(), false); executors.set(key1, executor.clone(), false, true);
executors.set(key2, executor.clone(), false); executors.set(key2, executor.clone(), false, true);
executors.set(key3, executor.clone(), true); executors.set(key3, executor.clone(), true, true);
executors.set(key4, executor.clone(), false); executors.set(key4, executor.clone(), false, true);
let executors = Rc::new(RefCell::new(executors)); let executors = Rc::new(RefCell::new(executors));
// store Missing // store Missing
bank.store_missing_executors(&executors); bank.store_executors_which_added_to_the_cache(&executors);
let stored_executors = bank.get_tx_executor_cache(accounts); let stored_executors = bank.get_tx_executor_cache(accounts);
assert_eq!(stored_executors.borrow().executors.len(), 2); assert_eq!(stored_executors.borrow().visible.len(), 2);
assert!(stored_executors.borrow().executors.contains_key(&key1)); assert!(stored_executors.borrow().visible.contains_key(&key1));
assert!(stored_executors.borrow().executors.contains_key(&key2)); assert!(stored_executors.borrow().visible.contains_key(&key2));
// store Updated // store Updated
bank.store_updated_executors(&executors); bank.store_executors_which_were_deployed(&executors);
let stored_executors = bank.get_tx_executor_cache(accounts); let stored_executors = bank.get_tx_executor_cache(accounts);
assert_eq!(stored_executors.borrow().executors.len(), 3); assert_eq!(stored_executors.borrow().visible.len(), 3);
assert!(stored_executors.borrow().executors.contains_key(&key1)); assert!(stored_executors.borrow().visible.contains_key(&key1));
assert!(stored_executors.borrow().executors.contains_key(&key2)); assert!(stored_executors.borrow().visible.contains_key(&key2));
assert!(stored_executors.borrow().executors.contains_key(&key3)); assert!(stored_executors.borrow().visible.contains_key(&key3));
// Check inheritance // Check inheritance
let bank = Bank::new_from_parent(&Arc::new(bank), &solana_sdk::pubkey::new_rand(), 1); let bank = Bank::new_from_parent(&Arc::new(bank), &solana_sdk::pubkey::new_rand(), 1);
let stored_executors = bank.get_tx_executor_cache(accounts); let stored_executors = bank.get_tx_executor_cache(accounts);
assert_eq!(stored_executors.borrow().executors.len(), 3); assert_eq!(stored_executors.borrow().visible.len(), 3);
assert!(stored_executors.borrow().executors.contains_key(&key1)); assert!(stored_executors.borrow().visible.contains_key(&key1));
assert!(stored_executors.borrow().executors.contains_key(&key2)); assert!(stored_executors.borrow().visible.contains_key(&key2));
assert!(stored_executors.borrow().executors.contains_key(&key3)); assert!(stored_executors.borrow().visible.contains_key(&key3));
// Force compilation of an executor // Force compilation of an executor
let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so").unwrap(); let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so").unwrap();
@ -7784,7 +7784,7 @@ fn test_bank_executor_cache() {
bank.remove_executor(&key3); bank.remove_executor(&key3);
bank.remove_executor(&key4); bank.remove_executor(&key4);
let stored_executors = bank.get_tx_executor_cache(accounts); let stored_executors = bank.get_tx_executor_cache(accounts);
assert_eq!(stored_executors.borrow().executors.len(), 0); assert_eq!(stored_executors.borrow().visible.len(), 0);
} }
#[test] #[test]
@ -7810,36 +7810,36 @@ fn test_bank_executor_cow() {
// add one to root bank // add one to root bank
let mut executors = TransactionExecutorCache::default(); let mut executors = TransactionExecutorCache::default();
executors.set(key1, executor.clone(), false); executors.set(key1, executor.clone(), false, true);
let executors = Rc::new(RefCell::new(executors)); let executors = Rc::new(RefCell::new(executors));
root.store_missing_executors(&executors); root.store_executors_which_added_to_the_cache(&executors);
let executors = root.get_tx_executor_cache(accounts); let executors = root.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 1); assert_eq!(executors.borrow().visible.len(), 1);
let fork1 = Bank::new_from_parent(&root, &Pubkey::default(), 1); let fork1 = Bank::new_from_parent(&root, &Pubkey::default(), 1);
let fork2 = Bank::new_from_parent(&root, &Pubkey::default(), 2); let fork2 = Bank::new_from_parent(&root, &Pubkey::default(), 2);
let executors = fork1.get_tx_executor_cache(accounts); let executors = fork1.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 1); assert_eq!(executors.borrow().visible.len(), 1);
let executors = fork2.get_tx_executor_cache(accounts); let executors = fork2.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 1); assert_eq!(executors.borrow().visible.len(), 1);
let mut executors = TransactionExecutorCache::default(); let mut executors = TransactionExecutorCache::default();
executors.set(key2, executor.clone(), false); executors.set(key2, executor.clone(), false, true);
let executors = Rc::new(RefCell::new(executors)); let executors = Rc::new(RefCell::new(executors));
fork1.store_missing_executors(&executors); fork1.store_executors_which_added_to_the_cache(&executors);
let executors = fork1.get_tx_executor_cache(accounts); let executors = fork1.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 2); assert_eq!(executors.borrow().visible.len(), 2);
let executors = fork2.get_tx_executor_cache(accounts); let executors = fork2.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 1); assert_eq!(executors.borrow().visible.len(), 1);
fork1.remove_executor(&key1); fork1.remove_executor(&key1);
let executors = fork1.get_tx_executor_cache(accounts); let executors = fork1.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 1); assert_eq!(executors.borrow().visible.len(), 1);
let executors = fork2.get_tx_executor_cache(accounts); let executors = fork2.get_tx_executor_cache(accounts);
assert_eq!(executors.borrow().executors.len(), 1); assert_eq!(executors.borrow().visible.len(), 1);
} }
#[test] #[test]

View File

@ -610,6 +610,10 @@ pub mod prevent_rent_paying_rent_recipients {
solana_sdk::declare_id!("Fab5oP3DmsLYCiQZXdjyqT3ukFFPrsmqhXU4WU1AWVVF"); solana_sdk::declare_id!("Fab5oP3DmsLYCiQZXdjyqT3ukFFPrsmqhXU4WU1AWVVF");
} }
pub mod delay_visibility_of_program_deployment {
solana_sdk::declare_id!("GmuBvtFb2aHfSfMXpuFeWZGHyDeCLPS79s48fmCWCfM5");
}
lazy_static! { lazy_static! {
/// Map of feature identifiers to user-visible description /// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [ pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -757,6 +761,7 @@ lazy_static! {
(remove_congestion_multiplier_from_fee_calculation::id(), "Remove congestion multiplier from transaction fee calculation #29881"), (remove_congestion_multiplier_from_fee_calculation::id(), "Remove congestion multiplier from transaction fee calculation #29881"),
(enable_request_heap_frame_ix::id(), "Enable transaction to request heap frame using compute budget instruction #30076"), (enable_request_heap_frame_ix::id(), "Enable transaction to request heap frame using compute budget instruction #30076"),
(prevent_rent_paying_rent_recipients::id(), "prevent recipients of rent rewards from ending in rent-paying state #30151"), (prevent_rent_paying_rent_recipients::id(), "prevent recipients of rent rewards from ending in rent-paying state #30151"),
(delay_visibility_of_program_deployment::id(), "delay visibility of program upgrades #30085"),
/*************** ADD NEW FEATURES HERE ***************/ /*************** ADD NEW FEATURES HERE ***************/
] ]
.iter() .iter()