From 763d44d6b7193f7cf076c181933df954e2c89314 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 20 Aug 2020 23:43:07 +0900 Subject: [PATCH 01/23] Implement Debug for MessageProcessor --- runtime/src/message_processor.rs | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index d4cf26c4d48a59..adced81361260f 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -293,6 +293,55 @@ pub struct MessageProcessor { #[serde(skip)] compute_budget: u64, } + +impl std::fmt::Debug for MessageProcessor { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + #[derive(Debug)] + struct MessageProcessor<'a> { + programs: Vec, + loaders: Vec, + native_loader: &'a NativeLoader, + is_cross_program_supported: bool, + } + // rustc doesn't compile due to bug without this work around + // https://github.com/rust-lang/rust/issues/50280 + // https://users.rust-lang.org/t/display-function-pointer/17073/2 + let processor = MessageProcessor { + programs: self + .programs + .iter() + .map(|(pubkey, instruction)| { + let erased_instruction: fn( + &'static Pubkey, + &'static [KeyedAccount<'static>], + &'static [u8], + ) + -> Result<(), InstructionError> = *instruction; + format!("{}: {:p}", pubkey, erased_instruction) + }) + .collect::>(), + loaders: self + .loaders + .iter() + .map(|(pubkey, instruction)| { + let erased_instruction: fn( + &'static Pubkey, + &'static [KeyedAccount<'static>], + &'static [u8], + &'static mut dyn InvokeContext, + ) + -> Result<(), InstructionError> = *instruction; + format!("{}: {:p}", pubkey, erased_instruction) + }) + .collect::>(), + native_loader: &self.native_loader, + is_cross_program_supported: self.is_cross_program_supported, + }; + + write!(f, "{:?}", processor) + } +} + impl Default for MessageProcessor { fn default() -> Self { Self { From 95b1936f46ba647716d226e6bc670da8ffe4d228 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 20 Aug 2020 23:44:27 +0900 Subject: [PATCH 02/23] Switch from delta-based gating to whole-set gating --- genesis-programs/src/lib.rs | 133 +++++++++++++---------------- genesis/src/main.rs | 2 +- local-cluster/src/local_cluster.rs | 1 - runtime/src/bank.rs | 34 ++++---- runtime/src/builtins.rs | 21 +++-- 5 files changed, 90 insertions(+), 101 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index c89a789f0749a8..bb75958e66643a 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -52,94 +52,89 @@ enum Program { BuiltinLoader((String, Pubkey, ProcessInstructionWithContext)), } -fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Option> { +// given operating_mode and epoch, return the entire set of enabled programs +fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { + let mut programs = vec![]; + match operating_mode { OperatingMode::Development => { - if epoch == 0 { - // Programs used for testing - Some(vec![ - Program::BuiltinLoader(solana_bpf_loader_program!()), - Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()), - Program::Native(solana_vest_program!()), - Program::Native(solana_budget_program!()), - Program::Native(solana_exchange_program!()), - ]) - } else if epoch == std::u64::MAX { + // Programs used for testing + programs.extend(vec![ + Program::BuiltinLoader(solana_bpf_loader_program!()), + Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()), + Program::Native(solana_vest_program!()), + Program::Native(solana_budget_program!()), + Program::Native(solana_exchange_program!()), + ]); + + if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected // to be reduced in a future network update. - Some(vec![Program::BuiltinLoader(solana_bpf_loader_program!())]) - } else { - None + programs.extend(vec![Program::BuiltinLoader(solana_bpf_loader_program!())]); } } - OperatingMode::Stable => { - if epoch == std::u64::MAX { + OperatingMode::Preview => { + if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected // to be reduced in a future network update. - Some(vec![ + programs.extend(vec![ Program::BuiltinLoader(solana_bpf_loader_program!()), Program::Native(solana_vest_program!()), ]) - } else { - None } } - OperatingMode::Preview => { - if epoch == std::u64::MAX { + OperatingMode::Stable => { + // at which epoch, bpf_loader_program is enabled?? + + if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected // to be reduced in a future network update. - Some(vec![ + programs.extend(vec![ Program::BuiltinLoader(solana_bpf_loader_program!()), Program::Native(solana_vest_program!()), - ]) - } else { - None + ]); } } - } + }; + + programs } -pub fn get_native_programs( - operating_mode: OperatingMode, - epoch: Epoch, -) -> Option> { - match get_programs(operating_mode, epoch) { - Some(programs) => { - let mut native_programs = vec![]; - for program in programs { - if let Program::Native((string, key)) = program { - native_programs.push((string, key)); - } - } - Some(native_programs) +pub fn get_native_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec<(String, Pubkey)> { + let mut native_programs = vec![]; + for program in get_programs(operating_mode, epoch) { + if let Program::Native((string, key)) = program { + native_programs.push((string, key)); } - None => None, + } + native_programs +} + +fn recheck_cross_program_support(bank: &mut Bank) { + if OperatingMode::Stable == bank.operating_mode() { + bank.set_cross_program_support(bank.epoch() >= 63); + } else { + bank.set_cross_program_support(true); } } pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpochCallback { Box::new(move |bank: &mut Bank| { + // Be careful to add arbitrary logic here; this should be idempotent and can be called + // at arbitrary point in an epoch not only epoch boundaries. + // This is because this closure need to be executed immediately after snapshot restoration, + // in addition to usual epoch boundaries if let Some(inflation) = get_inflation(operating_mode, bank.epoch()) { info!("Entering new epoch with inflation {:?}", inflation); bank.set_inflation(inflation); } - if let Some(programs) = get_programs(operating_mode, bank.epoch()) { - for program in programs { - match program { - Program::Native((name, program_id)) => { - bank.add_native_program(&name, &program_id); - } - Program::BuiltinLoader(( - name, - program_id, - process_instruction_with_context, - )) => { - bank.add_builtin_loader( - &name, - program_id, - process_instruction_with_context, - ); - } + for program in get_programs(operating_mode, bank.epoch()) { + match program { + Program::Native((name, program_id)) => { + bank.add_native_program(&name, &program_id); + } + Program::BuiltinLoader((name, program_id, process_instruction_with_context)) => { + bank.add_builtin_loader(&name, program_id, process_instruction_with_context); } } } @@ -151,6 +146,8 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch bank.set_max_invoke_depth(DEFAULT_MAX_INVOKE_DEPTH); bank.set_compute_budget(DEFAULT_COMPUTE_BUDGET); + + recheck_cross_program_support(bank); }) } @@ -162,7 +159,7 @@ mod tests { #[test] fn test_id_uniqueness() { let mut unique = HashSet::new(); - let programs = get_programs(OperatingMode::Development, 0).unwrap(); + let programs = get_programs(OperatingMode::Development, 0); for program in programs { match program { Program::Native((name, id)) => assert!(unique.insert((name, id))), @@ -182,22 +179,14 @@ mod tests { #[test] fn test_development_programs() { - assert_eq!( - get_programs(OperatingMode::Development, 0).unwrap().len(), - 5 - ); - assert!(get_programs(OperatingMode::Development, 1).is_none()); + assert_eq!(get_programs(OperatingMode::Development, 0).len(), 5); + assert_eq!(get_programs(OperatingMode::Development, 1).len(), 5); } #[test] fn test_native_development_programs() { - assert_eq!( - get_native_programs(OperatingMode::Development, 0) - .unwrap() - .len(), - 3 - ); - assert!(get_native_programs(OperatingMode::Development, 1).is_none()); + assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3); + assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3); } #[test] @@ -215,7 +204,7 @@ mod tests { #[test] fn test_softlaunch_programs() { - assert!(get_programs(OperatingMode::Stable, 1).is_none()); - assert!(get_programs(OperatingMode::Stable, std::u64::MAX).is_some()); + assert!(get_programs(OperatingMode::Stable, 1).is_empty()); + assert!(!get_programs(OperatingMode::Stable, std::u64::MAX).is_empty()); } } diff --git a/genesis/src/main.rs b/genesis/src/main.rs index 57dc3d556387c2..3530d7d34336cf 100644 --- a/genesis/src/main.rs +++ b/genesis/src/main.rs @@ -468,7 +468,7 @@ fn main() -> Result<(), Box> { ); let native_instruction_processors = - solana_genesis_programs::get_native_programs(operating_mode, 0).unwrap_or_else(Vec::new); + solana_genesis_programs::get_native_programs(operating_mode, 0); let inflation = solana_genesis_programs::get_inflation(operating_mode, 0).unwrap(); let mut genesis_config = GenesisConfig { diff --git a/local-cluster/src/local_cluster.rs b/local-cluster/src/local_cluster.rs index 2b0f69dce87fdc..7bd16d477449f6 100644 --- a/local-cluster/src/local_cluster.rs +++ b/local-cluster/src/local_cluster.rs @@ -173,7 +173,6 @@ impl LocalCluster { OperatingMode::Stable | OperatingMode::Preview => { genesis_config.native_instruction_processors = solana_genesis_programs::get_native_programs(genesis_config.operating_mode, 0) - .unwrap_or_default() } _ => (), } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 651abe64e10562..ed6bc69f8c7ec9 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -10,7 +10,7 @@ use crate::{ accounts_db::{ErrorCounters, SnapshotStorages}, accounts_index::Ancestors, blockhash_queue::BlockhashQueue, - builtins::{get_builtins, get_epoch_activated_builtins}, + builtins::get_builtins, epoch_stakes::{EpochStakes, NodeVoteAccounts}, log_collector::LogCollector, message_processor::MessageProcessor, @@ -557,17 +557,7 @@ impl Bank { let leader_schedule_epoch = epoch_schedule.get_leader_schedule_epoch(slot); if parent.epoch() < new.epoch() { - if let Some(entered_epoch_callback) = - parent.entered_epoch_callback.read().unwrap().as_ref() - { - entered_epoch_callback(&mut new) - } - - if let Some(builtins) = get_epoch_activated_builtins(new.operating_mode(), new.epoch) { - for program in builtins.iter() { - new.add_builtin(&program.name, program.id, program.entrypoint); - } - } + new.refresh_programs_and_inflation(); } new.update_epoch_stakes(leader_schedule_epoch); @@ -584,6 +574,7 @@ impl Bank { if !new.fix_recent_blockhashes_sysvar_delay() { new.update_recent_blockhashes(); } + dbg!(&new.message_processor); new } @@ -2607,10 +2598,7 @@ impl Bank { } pub fn finish_init(&mut self) { - let builtins = get_builtins(); - for program in builtins.iter() { - self.add_builtin(&program.name, program.id, program.entrypoint); - } + self.refresh_programs_and_inflation(); } pub fn set_parent(&mut self, parent: &Arc) { @@ -3187,6 +3175,20 @@ impl Bank { consumed_budget.saturating_sub(budget_recovery_delta) } + // This is called from snapshot restore and for each epoch boundary + // The entire code path herein must be idempotent + pub fn refresh_programs_and_inflation(&mut self) { + if let Some(entered_epoch_callback) = + self.entered_epoch_callback.clone().read().unwrap().as_ref() + { + entered_epoch_callback(self) + } + + for program in get_builtins(self.operating_mode(), self.epoch()) { + self.add_builtin(&program.name, program.id, program.entrypoint); + } + } + fn fix_recent_blockhashes_sysvar_delay(&self) -> bool { let activation_slot = match self.operating_mode() { OperatingMode::Development => 0, diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index 50534d79d3efb6..e1ee49b3dc7684 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -4,9 +4,11 @@ use crate::{ }; use solana_sdk::{clock::Epoch, genesis_config::OperatingMode, system_program}; -/// All builtin programs that should be active at the given (operating_mode, epoch) -pub fn get_builtins() -> Vec { - vec![ +/// The entire set of available builtin programs that should be active at the given (operating_mode, epoch) +pub fn get_builtins(_operating_mode: OperatingMode, _epoch: Epoch) -> Vec { + let mut builtins = vec![]; + + builtins.extend(vec![ Builtin::new( "system_program", system_program::id(), @@ -27,13 +29,10 @@ pub fn get_builtins() -> Vec { solana_vote_program::id(), Entrypoint::Program(solana_vote_program::vote_instruction::process_instruction), ), - ] -} + ]); + + // if we ever add gated builtins, add here like this + // if _epoch >= 10 { builtins.extend(....) } -/// Builtin programs that activate at the given (operating_mode, epoch) -pub fn get_epoch_activated_builtins( - _operating_mode: OperatingMode, - _epoch: Epoch, -) -> Option> { - None + builtins } From 1a4d0a52e34c80b20337833d8f3ff22ec54ec18c Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 00:09:51 +0900 Subject: [PATCH 03/23] Remove dbg! --- runtime/src/bank.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ed6bc69f8c7ec9..5379b1fb3dc52b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -574,7 +574,6 @@ impl Bank { if !new.fix_recent_blockhashes_sysvar_delay() { new.update_recent_blockhashes(); } - dbg!(&new.message_processor); new } From d66723a8945647c0cb5fff295b1b42c240719bc6 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 00:24:08 +0900 Subject: [PATCH 04/23] Fix clippy --- runtime/src/message_processor.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index adced81361260f..c350b71d5128a5 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -311,12 +311,13 @@ impl std::fmt::Debug for MessageProcessor { .programs .iter() .map(|(pubkey, instruction)| { - let erased_instruction: fn( + type ErasedProcessInstruction = fn( &'static Pubkey, &'static [KeyedAccount<'static>], &'static [u8], ) - -> Result<(), InstructionError> = *instruction; + -> Result<(), InstructionError>; + let erased_instruction: ErasedProcessInstruction = *instruction; format!("{}: {:p}", pubkey, erased_instruction) }) .collect::>(), @@ -324,13 +325,14 @@ impl std::fmt::Debug for MessageProcessor { .loaders .iter() .map(|(pubkey, instruction)| { - let erased_instruction: fn( + type ErasedProcessInstructionWithContext = fn( &'static Pubkey, &'static [KeyedAccount<'static>], &'static [u8], &'static mut dyn InvokeContext, ) - -> Result<(), InstructionError> = *instruction; + -> Result<(), InstructionError>; + let erased_instruction: ErasedProcessInstructionWithContext = *instruction; format!("{}: {:p}", pubkey, erased_instruction) }) .collect::>(), From de46b1c7d29f1be855ed8f0d4fecff188a42ecc9 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 00:34:16 +0900 Subject: [PATCH 05/23] Clippy --- genesis-programs/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index bb75958e66643a..d3cc2050d2c4f0 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -67,6 +67,7 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { Program::Native(solana_exchange_program!()), ]); + #[allow(clippy::absurd_extreme_comparisons)] if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected // to be reduced in a future network update. @@ -74,6 +75,7 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { } } OperatingMode::Preview => { + #[allow(clippy::absurd_extreme_comparisons)] if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected // to be reduced in a future network update. @@ -85,7 +87,7 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { } OperatingMode::Stable => { // at which epoch, bpf_loader_program is enabled?? - + #[allow(clippy::absurd_extreme_comparisons)] if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected // to be reduced in a future network update. From c83cc7a1802e140edb32cc7cb6539baa2c5734e8 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 01:47:31 +0900 Subject: [PATCH 06/23] Add test --- genesis-programs/src/lib.rs | 10 ---------- runtime/src/bank.rs | 23 +++++++++++++++++++++++ runtime/src/message_processor.rs | 4 ++++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index d3cc2050d2c4f0..cd802e0aff539a 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -112,14 +112,6 @@ pub fn get_native_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec<( native_programs } -fn recheck_cross_program_support(bank: &mut Bank) { - if OperatingMode::Stable == bank.operating_mode() { - bank.set_cross_program_support(bank.epoch() >= 63); - } else { - bank.set_cross_program_support(true); - } -} - pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpochCallback { Box::new(move |bank: &mut Bank| { // Be careful to add arbitrary logic here; this should be idempotent and can be called @@ -148,8 +140,6 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch bank.set_max_invoke_depth(DEFAULT_MAX_INVOKE_DEPTH); bank.set_compute_budget(DEFAULT_COMPUTE_BUDGET); - - recheck_cross_program_support(bank); }) } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5379b1fb3dc52b..6d522e0b804dd1 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3186,6 +3186,16 @@ impl Bank { for program in get_builtins(self.operating_mode(), self.epoch()) { self.add_builtin(&program.name, program.id, program.entrypoint); } + + self.recheck_cross_program_support(); + } + + fn recheck_cross_program_support(self: &mut Bank) { + if OperatingMode::Stable == self.operating_mode() { + self.set_cross_program_support(self.epoch() >= 63); + } else { + self.set_cross_program_support(true); + } } fn fix_recent_blockhashes_sysvar_delay(&self) -> bool { @@ -8101,4 +8111,17 @@ mod tests { ); assert_eq!(bank.get_balance(&mint_keypair.pubkey()), 496); // no transaction fee charged } + + #[test] + fn test_finish_init() { + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + let mut bank = Bank::new(&genesis_config); + bank.message_processor = MessageProcessor::default(); + bank.message_processor.set_cross_program_support(false); + + // simulate bank is just after deserialized from snapshot + bank.finish_init(); + + assert_eq!(bank.message_processor.get_cross_program_support(), true); + } } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index c350b71d5128a5..723cbf2edc50f8 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -411,6 +411,10 @@ impl MessageProcessor { self.compute_budget = compute_budget; } + pub fn get_cross_program_support(&mut self) -> bool { + self.is_cross_program_supported + } + /// Create the KeyedAccounts that will be passed to the program fn create_keyed_accounts<'a>( message: &'a Message, From 2dd6a83a8f20d098e6a82f2e2514a9cb39066aa6 Mon Sep 17 00:00:00 2001 From: Jack May Date: Thu, 20 Aug 2020 12:21:14 -0700 Subject: [PATCH 07/23] add loader to stable operating mode at proper epoch --- genesis-programs/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index cd802e0aff539a..0043b3b0354c10 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -66,13 +66,6 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { Program::Native(solana_budget_program!()), Program::Native(solana_exchange_program!()), ]); - - #[allow(clippy::absurd_extreme_comparisons)] - if epoch >= std::u64::MAX { - // The epoch of std::u64::MAX is a placeholder and is expected - // to be reduced in a future network update. - programs.extend(vec![Program::BuiltinLoader(solana_bpf_loader_program!())]); - } } OperatingMode::Preview => { #[allow(clippy::absurd_extreme_comparisons)] @@ -86,6 +79,11 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { } } OperatingMode::Stable => { + if epoch >= 34 { + programs.extend(vec![Program::BuiltinLoader( + solana_bpf_loader_deprecated_program!(), + )]); + } // at which epoch, bpf_loader_program is enabled?? #[allow(clippy::absurd_extreme_comparisons)] if epoch >= std::u64::MAX { From c87169697dd2c344eee0d15876ca2bad61ed19c2 Mon Sep 17 00:00:00 2001 From: Jack May Date: Thu, 20 Aug 2020 14:29:44 -0700 Subject: [PATCH 08/23] refresh_programs_and_inflation after ancestor setup --- runtime/src/bank.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6d522e0b804dd1..7a89130dc7c879 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -556,15 +556,14 @@ impl Bank { ); let leader_schedule_epoch = epoch_schedule.get_leader_schedule_epoch(slot); - if parent.epoch() < new.epoch() { - new.refresh_programs_and_inflation(); - } - new.update_epoch_stakes(leader_schedule_epoch); new.ancestors.insert(new.slot(), 0); new.parents().iter().enumerate().for_each(|(i, p)| { new.ancestors.insert(p.slot(), i + 1); }); + if parent.epoch() < new.epoch() { + new.refresh_programs_and_inflation(); + } new.update_slot_hashes(); new.update_rewards(parent.epoch()); @@ -574,6 +573,7 @@ impl Bank { if !new.fix_recent_blockhashes_sysvar_delay() { new.update_recent_blockhashes(); } + new } From 08ce44ac5560bb7e1809eb4ede745021e465d361 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 18:07:43 +0900 Subject: [PATCH 09/23] Callback via snapshot; avoid account re-add; Debug --- genesis-programs/src/lib.rs | 25 ++++++++++- ledger/src/blockstore_processor.rs | 8 ++-- runtime/src/bank.rs | 67 +++++++++++++++++------------- runtime/src/message_processor.rs | 16 +------ sdk/src/entrypoint_native.rs | 14 +++++++ 5 files changed, 82 insertions(+), 48 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index 0043b3b0354c10..38c467f9a8e650 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -13,8 +13,11 @@ use solana_runtime::{ message_processor::{DEFAULT_COMPUTE_BUDGET, DEFAULT_MAX_INVOKE_DEPTH}, }; use solana_sdk::{ - clock::Epoch, entrypoint_native::ProcessInstructionWithContext, genesis_config::OperatingMode, - inflation::Inflation, pubkey::Pubkey, + clock::Epoch, + entrypoint_native::{ErasedProcessInstructionWithContext, ProcessInstructionWithContext}, + genesis_config::OperatingMode, + inflation::Inflation, + pubkey::Pubkey, }; pub fn get_inflation(operating_mode: OperatingMode, epoch: Epoch) -> Option { @@ -52,6 +55,24 @@ enum Program { BuiltinLoader((String, Pubkey, ProcessInstructionWithContext)), } +impl std::fmt::Debug for Program { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + #[derive(Debug)] + enum Program { + Native((String, Pubkey)), + BuiltinLoader((String, Pubkey, String)), + } + let program = match self { + crate::Program::Native((string, pubkey)) => Program::Native((string.clone(), *pubkey)), + crate::Program::BuiltinLoader((string, pubkey, instruction)) => { + let erased: ErasedProcessInstructionWithContext = *instruction; + Program::BuiltinLoader((string.clone(), *pubkey, format!("{:p}", erased))) + } + }; + write!(f, "{:?}", program) + } +} + // given operating_mode and epoch, return the entire set of enabled programs fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { let mut programs = vec![]; diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 92da01c57a1c6f..7f1a5ee38970ed 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -340,7 +340,7 @@ pub fn process_blockstore( pub fn process_blockstore_from_root( genesis_config: &GenesisConfig, blockstore: &Blockstore, - bank: Arc, + mut bank: Arc, opts: &ProcessOptions, recyclers: &VerifyRecyclers, transaction_status_sender: Option, @@ -355,9 +355,9 @@ pub fn process_blockstore_from_root( let now = Instant::now(); let mut root = start_slot; - bank.set_entered_epoch_callback(solana_genesis_programs::get_entered_epoch_callback( - genesis_config.operating_mode, - )); + Arc::get_mut(&mut bank).unwrap().set_entered_epoch_callback( + solana_genesis_programs::get_entered_epoch_callback(genesis_config.operating_mode), + ); if let Some(ref new_hard_forks) = opts.new_hard_forks { let hard_forks = bank.hard_forks(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7a89130dc7c879..4e4371d440a50d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -562,7 +562,7 @@ impl Bank { new.ancestors.insert(p.slot(), i + 1); }); if parent.epoch() < new.epoch() { - new.refresh_programs_and_inflation(); + new.apply_feature_activations(); } new.update_slot_hashes(); @@ -1201,8 +1201,27 @@ impl Bank { } pub fn add_native_program(&self, name: &str, program_id: &Pubkey) { - let account = native_loader::create_loadable_account(name); - self.store_account(program_id, &account); + match self.get_account(&program_id) { + Some(account) => { + assert_eq!( + account.owner, + native_loader::id(), + "Cannot overwrite non-native account" + ); + } + None => { + assert!( + !self.is_frozen(), + "Can't change frozen bank by adding not-existing new native 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. + let account = native_loader::create_loadable_account(name); + self.store_account(&program_id, &account); + } + } debug!("Added native program {} under {:?}", name, program_id); } @@ -2597,7 +2616,7 @@ impl Bank { } pub fn finish_init(&mut self) { - self.refresh_programs_and_inflation(); + self.apply_feature_activations(); } pub fn set_parent(&mut self, parent: &Arc) { @@ -2612,8 +2631,9 @@ impl Bank { self.hard_forks.clone() } - pub fn set_entered_epoch_callback(&self, entered_epoch_callback: EnteredEpochCallback) { + pub fn set_entered_epoch_callback(&mut self, entered_epoch_callback: EnteredEpochCallback) { *self.entered_epoch_callback.write().unwrap() = Some(entered_epoch_callback); + self.apply_feature_activations(); } pub fn get_account(&self, pubkey: &Pubkey) -> Option { @@ -3064,20 +3084,7 @@ impl Bank { /// Add an instruction processor to intercept instructions before the dynamic loader. pub fn add_builtin(&mut self, name: &str, program_id: Pubkey, entrypoint: Entrypoint) { - match self.get_account(&program_id) { - Some(account) => { - assert_eq!( - account.owner, - native_loader::id(), - "Cannot overwrite non-native account" - ); - } - None => { - // Add a bogus executable native account, which will be loaded and ignored. - let account = native_loader::create_loadable_account(name); - self.store_account(&program_id, &account); - } - } + self.add_native_program(name, &program_id); match entrypoint { Entrypoint::Program(process_instruction) => { self.message_processor @@ -3176,7 +3183,7 @@ impl Bank { // This is called from snapshot restore and for each epoch boundary // The entire code path herein must be idempotent - pub fn refresh_programs_and_inflation(&mut self) { + pub fn apply_feature_activations(&mut self) { if let Some(entered_epoch_callback) = self.entered_epoch_callback.clone().read().unwrap().as_ref() { @@ -6360,16 +6367,18 @@ mod tests { #[test] fn test_bank_entered_epoch_callback() { let (genesis_config, _) = create_genesis_config(500); - let bank0 = Arc::new(Bank::new(&genesis_config)); + let mut bank0 = Arc::new(Bank::new(&genesis_config)); let callback_count = Arc::new(AtomicU64::new(0)); - bank0.set_entered_epoch_callback({ - let callback_count = callback_count.clone(); - //Box::new(move |_bank: &mut Bank| { - Box::new(move |_| { - callback_count.fetch_add(1, Ordering::SeqCst); - }) - }); + Arc::get_mut(&mut bank0) + .unwrap() + .set_entered_epoch_callback({ + let callback_count = callback_count.clone(); + //Box::new(move |_bank: &mut Bank| { + Box::new(move |_| { + callback_count.fetch_add(1, Ordering::SeqCst); + }) + }); let _bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), bank0.get_slots_in_epoch(0) - 1); @@ -7892,6 +7901,8 @@ mod tests { #[test] fn test_bank_hash_consistency() { + solana_logger::setup(); + let mut genesis_config = GenesisConfig::new( &[( Pubkey::new(&[42; 32]), diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 723cbf2edc50f8..08a3c684795b36 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -7,7 +7,8 @@ use solana_sdk::{ account::{create_keyed_readonly_accounts, Account, KeyedAccount}, clock::Epoch, entrypoint_native::{ - ComputeMeter, InvokeContext, Logger, ProcessInstruction, ProcessInstructionWithContext, + ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext, InvokeContext, + Logger, ProcessInstruction, ProcessInstructionWithContext, }, instruction::{CompiledInstruction, InstructionError}, message::Message, @@ -311,12 +312,6 @@ impl std::fmt::Debug for MessageProcessor { .programs .iter() .map(|(pubkey, instruction)| { - type ErasedProcessInstruction = fn( - &'static Pubkey, - &'static [KeyedAccount<'static>], - &'static [u8], - ) - -> Result<(), InstructionError>; let erased_instruction: ErasedProcessInstruction = *instruction; format!("{}: {:p}", pubkey, erased_instruction) }) @@ -325,13 +320,6 @@ impl std::fmt::Debug for MessageProcessor { .loaders .iter() .map(|(pubkey, instruction)| { - type ErasedProcessInstructionWithContext = fn( - &'static Pubkey, - &'static [KeyedAccount<'static>], - &'static [u8], - &'static mut dyn InvokeContext, - ) - -> Result<(), InstructionError>; let erased_instruction: ErasedProcessInstructionWithContext = *instruction; format!("{}: {:p}", pubkey, erased_instruction) }) diff --git a/sdk/src/entrypoint_native.rs b/sdk/src/entrypoint_native.rs index a689640be85f85..07a68dc0c88f85 100644 --- a/sdk/src/entrypoint_native.rs +++ b/sdk/src/entrypoint_native.rs @@ -175,6 +175,20 @@ pub type ProcessInstruction = fn(&Pubkey, &[KeyedAccount], &[u8]) -> Result<(), pub type ProcessInstructionWithContext = fn(&Pubkey, &[KeyedAccount], &[u8], &mut dyn InvokeContext) -> Result<(), InstructionError>; +// These are just type aliases for work around of Debug-ing above function pointers +pub type ErasedProcessInstructionWithContext = fn( + &'static Pubkey, + &'static [KeyedAccount<'static>], + &'static [u8], + &'static mut dyn InvokeContext, +) -> Result<(), InstructionError>; + +pub type ErasedProcessInstruction = fn( + &'static Pubkey, + &'static [KeyedAccount<'static>], + &'static [u8], +) -> Result<(), InstructionError>; + /// Invocation context passed to loaders pub trait InvokeContext { /// Push a program ID on to the invocation stack From 113433bdb55b240793d8e937f4b0bdd71a89e1c1 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 18:45:39 +0900 Subject: [PATCH 10/23] Fix test --- ledger/src/blockstore_processor.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 7f1a5ee38970ed..8875fb726c8a26 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -2573,7 +2573,11 @@ pub mod tests { blockstore.set_roots(&[3, 5]).unwrap(); // Set up bank1 - let bank0 = Arc::new(Bank::new(&genesis_config)); + let mut bank0 = Bank::new(&genesis_config); + let callback = + solana_genesis_programs::get_entered_epoch_callback(genesis_config.operating_mode); + callback(&mut bank0); + let bank0 = Arc::new(bank0); let opts = ProcessOptions { poh_verify: true, ..ProcessOptions::default() From 1e4c9e87aa00c3c4a84a067b9e69c6bcae95ee3f Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 19:17:50 +0900 Subject: [PATCH 11/23] Fix test and fix the past history --- genesis-programs/src/lib.rs | 10 ++++++++-- runtime/src/bank.rs | 7 +++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index 38c467f9a8e650..00c9f229b6772a 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -89,6 +89,13 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { ]); } OperatingMode::Preview => { + // tds enabled async cluster restart with smart contract being enabled + // at slot 2196960 (midway epoch 17) with v1.0.1 on Mar 1, 2020 + if epoch >= 17 { + programs.extend(vec![Program::BuiltinLoader( + solana_bpf_loader_deprecated_program!(), + )]); + } #[allow(clippy::absurd_extreme_comparisons)] if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected @@ -105,7 +112,6 @@ fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { solana_bpf_loader_deprecated_program!(), )]); } - // at which epoch, bpf_loader_program is enabled?? #[allow(clippy::absurd_extreme_comparisons)] if epoch >= std::u64::MAX { // The epoch of std::u64::MAX is a placeholder and is expected @@ -197,7 +203,7 @@ mod tests { #[test] fn test_native_development_programs() { assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3); - assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3); + assert_eq!(get_native_programs(OperatingMode::Development, 1).len(), 3); } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4e4371d440a50d..8daf4a5ea27ba1 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -6380,14 +6380,17 @@ mod tests { }) }); + // set_entered_eepoc_callbak fires the initial call + assert_eq!(callback_count.load(Ordering::SeqCst), 1); + let _bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), bank0.get_slots_in_epoch(0) - 1); // No callback called while within epoch 0 - assert_eq!(callback_count.load(Ordering::SeqCst), 0); + assert_eq!(callback_count.load(Ordering::SeqCst), 1); let _bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), bank0.get_slots_in_epoch(0)); // Callback called as bank1 is in epoch 1 - assert_eq!(callback_count.load(Ordering::SeqCst), 1); + assert_eq!(callback_count.load(Ordering::SeqCst), 2); callback_count.store(0, Ordering::SeqCst); let _bank1 = Bank::new_from_parent( From b6ebb717eaf71e8916a70bc60021a1877999b319 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 21:46:56 +0900 Subject: [PATCH 12/23] Make callback management stricter and cleaner --- ledger/src/blockstore_processor.rs | 61 +++++++++++++++++++++++------- runtime/src/bank.rs | 13 +++++-- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 8875fb726c8a26..62317f45b85300 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -309,6 +309,14 @@ pub struct ProcessOptions { pub frozen_accounts: Vec, } +fn initiate_callback(mut bank: &mut Arc, genesis_config: &GenesisConfig) { + Arc::get_mut(&mut bank) + .unwrap() + .initiate_entered_epoch_callback(solana_genesis_programs::get_entered_epoch_callback( + genesis_config.operating_mode, + )); +} + pub fn process_blockstore( genesis_config: &GenesisConfig, blockstore: &Blockstore, @@ -325,25 +333,54 @@ pub fn process_blockstore( } // Setup bank for slot 0 - let mut bank0 = Bank::new_with_paths(&genesis_config, account_paths, &opts.frozen_accounts); - let callback = - solana_genesis_programs::get_entered_epoch_callback(genesis_config.operating_mode); - callback(&mut bank0); - let bank0 = Arc::new(bank0); + let mut bank0 = Arc::new(Bank::new_with_paths( + &genesis_config, + account_paths, + &opts.frozen_accounts, + )); + initiate_callback(&mut bank0, genesis_config); info!("processing ledger for slot 0..."); let recyclers = VerifyRecyclers::default(); process_bank_0(&bank0, blockstore, &opts, &recyclers)?; - process_blockstore_from_root(genesis_config, blockstore, bank0, &opts, &recyclers, None) + do_process_blockstore_from_root( + genesis_config, + blockstore, + bank0, + &opts, + &recyclers, + None, + false, + ) } // Process blockstore from a known root bank pub fn process_blockstore_from_root( + genesis_config: &GenesisConfig, + blockstore: &Blockstore, + bank: Arc, + opts: &ProcessOptions, + recyclers: &VerifyRecyclers, + transaction_status_sender: Option, +) -> BlockstoreProcessorResult { + do_process_blockstore_from_root( + genesis_config, + blockstore, + bank, + opts, + recyclers, + transaction_status_sender, + true, + ) +} + +fn do_process_blockstore_from_root( genesis_config: &GenesisConfig, blockstore: &Blockstore, mut bank: Arc, opts: &ProcessOptions, recyclers: &VerifyRecyclers, transaction_status_sender: Option, + enable_callback: bool, ) -> BlockstoreProcessorResult { info!("processing ledger from slot {}...", bank.slot()); let allocated = thread_mem_usage::Allocatedp::default(); @@ -355,9 +392,9 @@ pub fn process_blockstore_from_root( let now = Instant::now(); let mut root = start_slot; - Arc::get_mut(&mut bank).unwrap().set_entered_epoch_callback( - solana_genesis_programs::get_entered_epoch_callback(genesis_config.operating_mode), - ); + if enable_callback { + initiate_callback(&mut bank, genesis_config); + } if let Some(ref new_hard_forks) = opts.new_hard_forks { let hard_forks = bank.hard_forks(); @@ -2573,11 +2610,7 @@ pub mod tests { blockstore.set_roots(&[3, 5]).unwrap(); // Set up bank1 - let mut bank0 = Bank::new(&genesis_config); - let callback = - solana_genesis_programs::get_entered_epoch_callback(genesis_config.operating_mode); - callback(&mut bank0); - let bank0 = Arc::new(bank0); + let bank0 = Arc::new(Bank::new(&genesis_config)); let opts = ProcessOptions { poh_verify: true, ..ProcessOptions::default() diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 8daf4a5ea27ba1..dd56620a7e1c75 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2631,8 +2631,15 @@ impl Bank { self.hard_forks.clone() } - pub fn set_entered_epoch_callback(&mut self, entered_epoch_callback: EnteredEpochCallback) { - *self.entered_epoch_callback.write().unwrap() = Some(entered_epoch_callback); + pub fn initiate_entered_epoch_callback( + &mut self, + entered_epoch_callback: EnteredEpochCallback, + ) { + { + let mut callback_w = self.entered_epoch_callback.write().unwrap(); + assert!(callback_w.is_none(), "Already callback has been initiated"); + *callback_w = Some(entered_epoch_callback); + } self.apply_feature_activations(); } @@ -6372,7 +6379,7 @@ mod tests { Arc::get_mut(&mut bank0) .unwrap() - .set_entered_epoch_callback({ + .initiate_entered_epoch_callback({ let callback_count = callback_count.clone(); //Box::new(move |_bank: &mut Bank| { Box::new(move |_| { From 920ca8013b0f699a28676cb92550954962674082 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 21 Aug 2020 23:05:59 +0900 Subject: [PATCH 13/23] Fix test --- ledger/src/blockstore_processor.rs | 6 ++++-- runtime/src/bank.rs | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 62317f45b85300..f59c12e8d1a9b9 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -2610,7 +2610,8 @@ pub mod tests { blockstore.set_roots(&[3, 5]).unwrap(); // Set up bank1 - let bank0 = Arc::new(Bank::new(&genesis_config)); + let mut bank0 = Arc::new(Bank::new(&genesis_config)); + initiate_callback(&mut bank0, &genesis_config); let opts = ProcessOptions { poh_verify: true, ..ProcessOptions::default() @@ -2631,13 +2632,14 @@ pub mod tests { bank1.squash(); // Test process_blockstore_from_root() from slot 1 onwards - let (bank_forks, _leader_schedule) = process_blockstore_from_root( + let (bank_forks, _leader_schedule) = do_process_blockstore_from_root( &genesis_config, &blockstore, bank1, &opts, &recyclers, None, + false, ) .unwrap(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index dd56620a7e1c75..d74e4e7b6aa5a7 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -561,6 +561,8 @@ impl Bank { new.parents().iter().enumerate().for_each(|(i, p)| { new.ancestors.insert(p.slot(), i + 1); }); + + // Following code may touch AccountsDB, requiring proper ancestors if parent.epoch() < new.epoch() { new.apply_feature_activations(); } From ba8f59facadff4623beae6d13b59fa6f26b6a475 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 22 Aug 2020 03:47:54 +0900 Subject: [PATCH 14/23] Test overwrite and frozen for native programs --- runtime/src/bank.rs | 128 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d74e4e7b6aa5a7..33a2f8817bc8e4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8147,4 +8147,132 @@ mod tests { assert_eq!(bank.message_processor.get_cross_program_support(), true); } + + #[test] + fn test_add_builtin_program_no_overwrite() { + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + + fn mock_ix_processor( + _pubkey: &Pubkey, + _ka: &[KeyedAccount], + _data: &[u8], + ) -> std::result::Result<(), InstructionError> { + Err(InstructionError::Custom(42)) + } + + let slot = 123; + let program_id = Pubkey::new_rand(); + + let mut bank = Arc::new(Bank::new_from_parent( + &Arc::new(Bank::new(&genesis_config)), + &Pubkey::default(), + slot, + )); + assert_eq!(bank.get_account_modified_slot(&program_id), None); + + Arc::get_mut(&mut bank).unwrap().add_builtin_program( + "mock_program", + program_id, + mock_ix_processor, + ); + assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); + + let mut bank = Arc::new(new_from_parent(&bank)); + Arc::get_mut(&mut bank).unwrap().add_builtin_program( + "mock_program", + program_id, + mock_ix_processor, + ); + assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); + } + + #[test] + fn test_add_builtin_loader_no_overwrite() { + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + + fn mock_ix_processor( + _pubkey: &Pubkey, + _ka: &[KeyedAccount], + _data: &[u8], + _context: &mut dyn solana_sdk::entrypoint_native::InvokeContext, + ) -> std::result::Result<(), InstructionError> { + Err(InstructionError::Custom(42)) + } + + let slot = 123; + let loader_id = Pubkey::new_rand(); + + let mut bank = Arc::new(Bank::new_from_parent( + &Arc::new(Bank::new(&genesis_config)), + &Pubkey::default(), + slot, + )); + assert_eq!(bank.get_account_modified_slot(&loader_id), None); + + Arc::get_mut(&mut bank).unwrap().add_builtin_loader( + "mock_program", + loader_id, + mock_ix_processor, + ); + assert_eq!(bank.get_account_modified_slot(&loader_id).unwrap().1, slot); + + let mut bank = Arc::new(new_from_parent(&bank)); + Arc::get_mut(&mut bank).unwrap().add_builtin_loader( + "mock_program", + loader_id, + mock_ix_processor, + ); + assert_eq!(bank.get_account_modified_slot(&loader_id).unwrap().1, slot); + } + + #[test] + fn test_add_native_program_no_overwrite() { + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + + let slot = 123; + let program_id = Pubkey::new_rand(); + + let mut bank = Arc::new(Bank::new_from_parent( + &Arc::new(Bank::new(&genesis_config)), + &Pubkey::default(), + slot, + )); + assert_eq!(bank.get_account_modified_slot(&program_id), None); + + Arc::get_mut(&mut bank) + .unwrap() + .add_native_program("mock_program", &program_id); + assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); + + let mut bank = Arc::new(new_from_parent(&bank)); + Arc::get_mut(&mut bank) + .unwrap() + .add_native_program("mock_program", &program_id); + assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); + } + + #[test] + #[should_panic( + expected = "Can't change frozen bank by adding not-existing new native \ + program (mock_program, CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \ + Maybe, inconsistent program activation is detected on snapshot restore?" + )] + fn test_add_native_program_after_frozen() { + use std::str::FromStr; + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + + let slot = 123; + let program_id = Pubkey::from_str("CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre").unwrap(); + + let mut bank = Arc::new(Bank::new_from_parent( + &Arc::new(Bank::new(&genesis_config)), + &Pubkey::default(), + slot, + )); + bank.freeze(); + + Arc::get_mut(&mut bank) + .unwrap() + .add_native_program("mock_program", &program_id); + } } From 9160dc6d1df4878fa1f38c6f9ec82fae86ccf4dd Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 22 Aug 2020 04:59:17 +0900 Subject: [PATCH 15/23] Test epoch callback with genesis-programs --- ledger/src/blockstore_processor.rs | 88 ++++++++++++++++++++++++++++++ runtime/src/bank.rs | 14 ++++- runtime/src/message_processor.rs | 5 ++ 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index f59c12e8d1a9b9..b68b08caacef79 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3104,4 +3104,92 @@ pub mod tests { run_test_process_blockstore_with_supermajority_root(None); run_test_process_blockstore_with_supermajority_root(Some(1)) } + + #[test] + fn test_process_blockstore_feature_activations_since_genesis() { + let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(123); + + let (ledger_path, _blockhash) = create_new_tmp_ledger!(&genesis_config); + let blockstore = Blockstore::open(&ledger_path).unwrap(); + + let opts = ProcessOptions::default(); + let (bank_forks, _leader_schedule) = + process_blockstore(&genesis_config, &blockstore, vec![], opts).unwrap(); + + assert_eq!(bank_forks.working_bank().slot(), 0); + assert_eq!( + bank_forks.working_bank().loader_program_ids(), + vec![ + solana_sdk::bpf_loader::id(), + solana_sdk::bpf_loader_deprecated::id() + ] + ); + } + + #[test] + fn test_process_blockstore_feature_activations_from_snapshot() { + let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(123); + + let (ledger_path, _blockhash) = create_new_tmp_ledger!(&genesis_config); + let blockstore = Blockstore::open(&ledger_path).unwrap(); + + // Set up bank1 + let mut bank0 = Arc::new(Bank::new(&genesis_config)); + initiate_callback(&mut bank0, &genesis_config); + let recyclers = VerifyRecyclers::default(); + let opts = ProcessOptions::default(); + process_bank_0(&bank0, &blockstore, &opts, &recyclers).unwrap(); + let restored_slot = genesis_config.epoch_schedule.get_first_slot_in_epoch(1); + let mut bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), restored_slot); + bank1.squash(); + + // this is similar to snapshot deserialization + bank1.reset_callback_and_message_processor(); + assert_eq!(bank1.loader_program_ids(), vec![]); + + let bank1 = Arc::new(bank1); + let (bank_forks, _leader_schedule) = process_blockstore_from_root( + &genesis_config, + &blockstore, + bank1, + &opts, + &recyclers, + None, + ) + .unwrap(); + assert_eq!(bank_forks.working_bank().slot(), restored_slot); + assert_eq!( + bank_forks.working_bank().loader_program_ids(), + vec![ + solana_sdk::bpf_loader::id(), + solana_sdk::bpf_loader_deprecated::id() + ] + ); + } + + #[test] + fn test_process_blockstore_feature_activations_into_epoch_with_activation() { + let GenesisConfigInfo { + mut genesis_config, .. + } = create_genesis_config(123); + + genesis_config.operating_mode = solana_sdk::genesis_config::OperatingMode::Stable; + let (ledger_path, _blockhash) = create_new_tmp_ledger!(&genesis_config); + let blockstore = Blockstore::open(&ledger_path).unwrap(); + + let opts = ProcessOptions::default(); + let (bank_forks, _leader_schedule) = + process_blockstore(&genesis_config, &blockstore, vec![], opts).unwrap(); + let bank = bank_forks.working_bank(); + assert_eq!(bank.loader_program_ids(), vec![]); + + let restored_slot = genesis_config.epoch_schedule.get_first_slot_in_epoch(34); + let bank = Bank::new_from_parent(&bank, &Pubkey::default(), restored_slot); + + assert_eq!(bank.slot(), restored_slot); + assert_eq!( + bank.loader_program_ids(), + vec![solana_sdk::bpf_loader_deprecated::id()] + ); + } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 33a2f8817bc8e4..d5e083d4c31996 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -190,6 +190,7 @@ impl StatusCacheRc { } pub type EnteredEpochCallback = Box; +type WrappedEnteredEpochCallback = Arc>>; pub type TransactionProcessResult = (Result<()>, Option); pub struct TransactionResults { @@ -416,7 +417,7 @@ pub struct Bank { /// Callback to be notified when a bank enters a new Epoch /// (used to adjust cluster features over time) - entered_epoch_callback: Arc>>, + entered_epoch_callback: WrappedEnteredEpochCallback, /// Last time when the cluster info vote listener has synced with this bank pub last_vote_sync: AtomicU64, @@ -3223,6 +3224,17 @@ impl Bank { self.slot() >= activation_slot } + + // only used for testing + pub fn loader_program_ids(&self) -> Vec { + self.message_processor.loader_program_ids() + } + + // only used for testing + pub fn reset_callback_and_message_processor(&mut self) { + self.entered_epoch_callback = WrappedEnteredEpochCallback::default(); + self.message_processor = MessageProcessor::default(); + } } impl Drop for Bank { diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 08a3c684795b36..ae004ae03d42c9 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -691,6 +691,11 @@ impl MessageProcessor { } Ok(()) } + + // only used for testing + pub fn loader_program_ids(&self) -> Vec { + self.loaders.iter().map(|a| a.0).collect::>() + } } #[cfg(test)] From 7c266989cbe3dea195c3111a9adee57ac32d0444 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 22 Aug 2020 17:00:42 +0900 Subject: [PATCH 16/23] Add assertions for parent bank --- ledger/src/blockstore_processor.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index b68b08caacef79..ec7593cc40fc5f 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3180,15 +3180,18 @@ pub mod tests { let opts = ProcessOptions::default(); let (bank_forks, _leader_schedule) = process_blockstore(&genesis_config, &blockstore, vec![], opts).unwrap(); - let bank = bank_forks.working_bank(); - assert_eq!(bank.loader_program_ids(), vec![]); + let bank0 = bank_forks.working_bank(); + assert_eq!(bank0.loader_program_ids(), vec![]); let restored_slot = genesis_config.epoch_schedule.get_first_slot_in_epoch(34); - let bank = Bank::new_from_parent(&bank, &Pubkey::default(), restored_slot); + let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), restored_slot); - assert_eq!(bank.slot(), restored_slot); + assert_eq!(bank0.slot(), 0); + assert_eq!(bank0.loader_program_ids(), vec![]); + + assert_eq!(bank1.slot(), restored_slot); assert_eq!( - bank.loader_program_ids(), + bank1.loader_program_ids(), vec![solana_sdk::bpf_loader_deprecated::id()] ); } From 5699303e754fc7c9f27a0af62ff63ac561126763 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Mon, 24 Aug 2020 14:46:07 +0900 Subject: [PATCH 17/23] Add tests and some minor cleaning --- genesis-programs/src/lib.rs | 16 +++-- ledger/src/blockstore_processor.rs | 12 ++-- runtime/src/bank.rs | 12 ++-- runtime/src/builtins.rs | 110 ++++++++++++++++++++++++++++- runtime/src/message_processor.rs | 8 ++- 5 files changed, 137 insertions(+), 21 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index 00c9f229b6772a..e4766d28380de7 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -143,11 +143,18 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch // at arbitrary point in an epoch not only epoch boundaries. // This is because this closure need to be executed immediately after snapshot restoration, // in addition to usual epoch boundaries - if let Some(inflation) = get_inflation(operating_mode, bank.epoch()) { + let (epoch, slot_index) = bank.get_epoch_and_slot_index(bank.slot()); + if slot_index != 0 { + // bank should be frozen (restored from snapshot) unless entering into a new epoch + assert!(bank.is_frozen()); + } + // we must initialize some skip(serde) fields here, regardless frozen or not + + if let Some(inflation) = get_inflation(operating_mode, epoch) { info!("Entering new epoch with inflation {:?}", inflation); bank.set_inflation(inflation); } - for program in get_programs(operating_mode, bank.epoch()) { + for program in get_programs(operating_mode, epoch) { match program { Program::Native((name, program_id)) => { bank.add_native_program(&name, &program_id); @@ -157,11 +164,6 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch } } } - if OperatingMode::Stable == operating_mode { - bank.set_cross_program_support(bank.epoch() >= 63); - } else { - bank.set_cross_program_support(true); - } bank.set_max_invoke_depth(DEFAULT_MAX_INVOKE_DEPTH); bank.set_compute_budget(DEFAULT_COMPUTE_BUDGET); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index ec7593cc40fc5f..f35e3853bca9aa 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3118,7 +3118,7 @@ pub mod tests { assert_eq!(bank_forks.working_bank().slot(), 0); assert_eq!( - bank_forks.working_bank().loader_program_ids(), + bank_forks.working_bank().builtin_loader_ids(), vec![ solana_sdk::bpf_loader::id(), solana_sdk::bpf_loader_deprecated::id() @@ -3145,7 +3145,7 @@ pub mod tests { // this is similar to snapshot deserialization bank1.reset_callback_and_message_processor(); - assert_eq!(bank1.loader_program_ids(), vec![]); + assert_eq!(bank1.builtin_loader_ids(), vec![]); let bank1 = Arc::new(bank1); let (bank_forks, _leader_schedule) = process_blockstore_from_root( @@ -3159,7 +3159,7 @@ pub mod tests { .unwrap(); assert_eq!(bank_forks.working_bank().slot(), restored_slot); assert_eq!( - bank_forks.working_bank().loader_program_ids(), + bank_forks.working_bank().builtin_loader_ids(), vec![ solana_sdk::bpf_loader::id(), solana_sdk::bpf_loader_deprecated::id() @@ -3181,17 +3181,17 @@ pub mod tests { let (bank_forks, _leader_schedule) = process_blockstore(&genesis_config, &blockstore, vec![], opts).unwrap(); let bank0 = bank_forks.working_bank(); - assert_eq!(bank0.loader_program_ids(), vec![]); + assert_eq!(bank0.builtin_loader_ids(), vec![]); let restored_slot = genesis_config.epoch_schedule.get_first_slot_in_epoch(34); let bank1 = Bank::new_from_parent(&bank0, &Pubkey::default(), restored_slot); assert_eq!(bank0.slot(), 0); - assert_eq!(bank0.loader_program_ids(), vec![]); + assert_eq!(bank0.builtin_loader_ids(), vec![]); assert_eq!(bank1.slot(), restored_slot); assert_eq!( - bank1.loader_program_ids(), + bank1.builtin_loader_ids(), vec![solana_sdk::bpf_loader_deprecated::id()] ); } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d5e083d4c31996..c7340ac40cc6ae 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3191,7 +3191,7 @@ impl Bank { consumed_budget.saturating_sub(budget_recovery_delta) } - // This is called from snapshot restore and for each epoch boundary + // This is called from snapshot restore AND for each epoch boundary // The entire code path herein must be idempotent pub fn apply_feature_activations(&mut self) { if let Some(entered_epoch_callback) = @@ -3226,8 +3226,13 @@ impl Bank { } // only used for testing - pub fn loader_program_ids(&self) -> Vec { - self.message_processor.loader_program_ids() + pub fn builtin_loader_ids(&self) -> Vec { + self.message_processor.builtin_loader_ids() + } + + // only used for testing + pub fn builtin_program_ids(&self) -> Vec { + self.message_processor.builtin_program_ids() } // only used for testing @@ -6395,7 +6400,6 @@ mod tests { .unwrap() .initiate_entered_epoch_callback({ let callback_count = callback_count.clone(); - //Box::new(move |_bank: &mut Bank| { Box::new(move |_| { callback_count.fetch_add(1, Ordering::SeqCst); }) diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index e1ee49b3dc7684..7c7710cc77b39a 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -4,8 +4,11 @@ use crate::{ }; use solana_sdk::{clock::Epoch, genesis_config::OperatingMode, system_program}; +use log::*; + /// The entire set of available builtin programs that should be active at the given (operating_mode, epoch) -pub fn get_builtins(_operating_mode: OperatingMode, _epoch: Epoch) -> Vec { +pub fn get_builtins(operating_mode: OperatingMode, epoch: Epoch) -> Vec { + trace!("get_builtins: {:?}, {:?}", operating_mode, epoch); let mut builtins = vec![]; builtins.extend(vec![ @@ -31,8 +34,109 @@ pub fn get_builtins(_operating_mode: OperatingMode, _epoch: Epoch) -> Vec= 10 { builtins.extend(....) } + #[cfg(test)] + if operating_mode == OperatingMode::Development && epoch >= 2 { + use solana_sdk::instruction::InstructionError; + use solana_sdk::{account::KeyedAccount, pubkey::Pubkey}; + use std::str::FromStr; + fn mock_ix_processor( + _pubkey: &Pubkey, + _ka: &[KeyedAccount], + _data: &[u8], + ) -> std::result::Result<(), InstructionError> { + Err(InstructionError::Custom(42)) + } + let program_id = Pubkey::from_str("7saCc6X5a2syoYANA5oUUnPZLcLMfKoSjiDhFU5fbpoK").unwrap(); + builtins.extend(vec![Builtin::new( + "mock", + program_id, + Entrypoint::Program(mock_ix_processor), + )]); + } builtins } + +#[cfg(test)] +mod tests { + use super::*; + use crate::bank::Bank; + use solana_sdk::{genesis_config::create_genesis_config, pubkey::Pubkey}; + + use std::str::FromStr; + use std::sync::Arc; + + #[test] + fn test_get_builtins() { + let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + let bank0 = Arc::new(Bank::new(&genesis_config)); + + let restored_slot1 = genesis_config.epoch_schedule.get_first_slot_in_epoch(2); + let bank1 = Arc::new(Bank::new_from_parent( + &bank0, + &Pubkey::default(), + restored_slot1, + )); + + let restored_slot2 = genesis_config.epoch_schedule.get_first_slot_in_epoch(3); + let bank2 = Arc::new(Bank::new_from_parent( + &bank1, + &Pubkey::default(), + restored_slot2, + )); + + let warped_slot = genesis_config.epoch_schedule.get_first_slot_in_epoch(999); + let warped_bank = Arc::new(Bank::new_from_parent( + &bank0, + &Pubkey::default(), + warped_slot, + )); + + assert_eq!(bank0.slot(), 0); + assert_eq!( + bank0.builtin_program_ids(), + vec![ + system_program::id(), + solana_config_program::id(), + solana_stake_program::id(), + solana_vote_program::id(), + ] + ); + + assert_eq!(bank1.slot(), restored_slot1); + assert_eq!( + bank1.builtin_program_ids(), + vec![ + system_program::id(), + solana_config_program::id(), + solana_stake_program::id(), + solana_vote_program::id(), + Pubkey::from_str("7saCc6X5a2syoYANA5oUUnPZLcLMfKoSjiDhFU5fbpoK").unwrap(), + ] + ); + + assert_eq!(bank2.slot(), restored_slot2); + assert_eq!( + bank2.builtin_program_ids(), + vec![ + system_program::id(), + solana_config_program::id(), + solana_stake_program::id(), + solana_vote_program::id(), + Pubkey::from_str("7saCc6X5a2syoYANA5oUUnPZLcLMfKoSjiDhFU5fbpoK").unwrap(), + ] + ); + + assert_eq!(warped_bank.slot(), warped_slot); + assert_eq!( + warped_bank.builtin_program_ids(), + vec![ + system_program::id(), + solana_config_program::id(), + solana_stake_program::id(), + solana_vote_program::id(), + Pubkey::from_str("7saCc6X5a2syoYANA5oUUnPZLcLMfKoSjiDhFU5fbpoK").unwrap(), + ] + ); + } +} diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index ae004ae03d42c9..fa488e3a6f44d3 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -399,6 +399,7 @@ impl MessageProcessor { self.compute_budget = compute_budget; } + #[cfg(test)] pub fn get_cross_program_support(&mut self) -> bool { self.is_cross_program_supported } @@ -693,9 +694,14 @@ impl MessageProcessor { } // only used for testing - pub fn loader_program_ids(&self) -> Vec { + pub fn builtin_loader_ids(&self) -> Vec { self.loaders.iter().map(|a| a.0).collect::>() } + + // only used for testing + pub fn builtin_program_ids(&self) -> Vec { + self.programs.iter().map(|a| a.0).collect::>() + } } #[cfg(test)] From 9529fc469fc016ad93e83250c59bae9c8b2e422d Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Mon, 24 Aug 2020 16:19:32 +0900 Subject: [PATCH 18/23] Remove unsteady assertion... --- genesis-programs/src/lib.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index e4766d28380de7..a9667cc78296d5 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -143,18 +143,14 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch // at arbitrary point in an epoch not only epoch boundaries. // This is because this closure need to be executed immediately after snapshot restoration, // in addition to usual epoch boundaries - let (epoch, slot_index) = bank.get_epoch_and_slot_index(bank.slot()); - if slot_index != 0 { - // bank should be frozen (restored from snapshot) unless entering into a new epoch - assert!(bank.is_frozen()); - } - // we must initialize some skip(serde) fields here, regardless frozen or not + // In other words, this callback initializes some skip(serde) fields, regardless + // frozen or not - if let Some(inflation) = get_inflation(operating_mode, epoch) { + if let Some(inflation) = get_inflation(operating_mode, bank.epoch()) { info!("Entering new epoch with inflation {:?}", inflation); bank.set_inflation(inflation); } - for program in get_programs(operating_mode, epoch) { + for program in get_programs(operating_mode, bank.epoch()) { match program { Program::Native((name, program_id)) => { bank.add_native_program(&name, &program_id); From 19bee7904b9b3b92a757d243de3a37b26e80a5c6 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Mon, 24 Aug 2020 17:16:34 +0900 Subject: [PATCH 19/23] Fix test... --- runtime/src/builtins.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index 7c7710cc77b39a..b3c436828f2ea8 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -34,8 +34,9 @@ pub fn get_builtins(operating_mode: OperatingMode, epoch: Epoch) -> Vec ), ]); + // repurpose Preview for test_get_builtins because the Development is overloaded... #[cfg(test)] - if operating_mode == OperatingMode::Development && epoch >= 2 { + if operating_mode == OperatingMode::Preview && epoch >= 2 { use solana_sdk::instruction::InstructionError; use solana_sdk::{account::KeyedAccount, pubkey::Pubkey}; use std::str::FromStr; @@ -61,14 +62,18 @@ pub fn get_builtins(operating_mode: OperatingMode, epoch: Epoch) -> Vec mod tests { use super::*; use crate::bank::Bank; - use solana_sdk::{genesis_config::create_genesis_config, pubkey::Pubkey}; + use solana_sdk::{ + genesis_config::{create_genesis_config, OperatingMode}, + pubkey::Pubkey, + }; use std::str::FromStr; use std::sync::Arc; #[test] fn test_get_builtins() { - let (genesis_config, _mint_keypair) = create_genesis_config(100_000); + let (mut genesis_config, _mint_keypair) = create_genesis_config(100_000); + genesis_config.operating_mode = OperatingMode::Preview; let bank0 = Arc::new(Bank::new(&genesis_config)); let restored_slot1 = genesis_config.epoch_schedule.get_first_slot_in_epoch(2); From c1ba3d24a4ed170c29b9b671e310c7b816985edf Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Mon, 24 Aug 2020 23:55:37 +0900 Subject: [PATCH 20/23] Fix DOS --- runtime/src/bank.rs | 81 +++++++++++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c7340ac40cc6ae..9be432b37fe119 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1204,27 +1204,37 @@ impl Bank { } pub fn add_native_program(&self, name: &str, program_id: &Pubkey) { - match self.get_account(&program_id) { - Some(account) => { - assert_eq!( - account.owner, - native_loader::id(), - "Cannot overwrite non-native account" - ); - } - None => { - assert!( - !self.is_frozen(), - "Can't change frozen bank by adding not-existing new native 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. - let account = native_loader::create_loadable_account(name); + let mut already_genuine_program_exists = false; + if let Some(mut account) = self.get_account(&program_id) { + already_genuine_program_exists = native_loader::check_id(&account.owner); + + if !already_genuine_program_exists { + // malicious account is pre-occupying at program_id + // forcibly burn and purge it + + self.capitalization + .fetch_sub(account.lamports, Ordering::Relaxed); + + // Resetting account balance to 0 is needed to really purge from AccountsDB and + // flush the Stakes cache + account.lamports = 0; self.store_account(&program_id, &account); } } + + if !already_genuine_program_exists { + assert!( + !self.is_frozen(), + "Can't change frozen bank by adding not-existing new native 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. + let account = native_loader::create_loadable_account(name); + self.store_account(&program_id, &account); + } debug!("Added native program {} under {:?}", name, program_id); } @@ -6860,9 +6870,8 @@ mod tests { } #[test] - #[should_panic] - fn test_add_instruction_processor_for_invalid_account() { - let (genesis_config, mint_keypair) = create_genesis_config(500); + fn test_add_instruction_processor_for_existing_unrelated_accounts() { + let (genesis_config, _mint_keypair) = create_genesis_config(500); let mut bank = Bank::new(&genesis_config); fn mock_ix_processor( @@ -6874,8 +6883,36 @@ mod tests { } // Non-native loader accounts can not be used for instruction processing - bank.add_builtin_program("mock_program", mint_keypair.pubkey(), mock_ix_processor); + assert!(bank.stakes.read().unwrap().vote_accounts().is_empty()); + assert!(bank.stakes.read().unwrap().stake_delegations().is_empty()); + assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + + let ((vote_id, vote_account), (stake_id, stake_account)) = + crate::stakes::tests::create_staked_node_accounts(1_0000); + bank.capitalization.fetch_add( + vote_account.lamports + stake_account.lamports, + Ordering::Relaxed, + ); + bank.store_account(&vote_id, &vote_account); + bank.store_account(&stake_id, &stake_account); + assert!(!bank.stakes.read().unwrap().vote_accounts().is_empty()); + assert!(!bank.stakes.read().unwrap().stake_delegations().is_empty()); + assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + + bank.add_builtin_program("mock_program1", vote_id, mock_ix_processor); + bank.add_builtin_program("mock_program2", stake_id, mock_ix_processor); + assert!(bank.stakes.read().unwrap().vote_accounts().is_empty()); + assert!(bank.stakes.read().unwrap().stake_delegations().is_empty()); + assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + + // Re-adding builtin programs should be no-op + bank.add_builtin_program("mock_program1", vote_id, mock_ix_processor); + bank.add_builtin_program("mock_program2", stake_id, mock_ix_processor); + assert!(bank.stakes.read().unwrap().vote_accounts().is_empty()); + assert!(bank.stakes.read().unwrap().stake_delegations().is_empty()); + assert_eq!(bank.calculate_capitalization(), bank.capitalization()); } + #[test] fn test_recent_blockhashes_sysvar() { let (genesis_config, _mint_keypair) = create_genesis_config(500); From ebba5bd5daa794741f7c2254df862b0451f42412 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 25 Aug 2020 01:05:40 +0900 Subject: [PATCH 21/23] Skip ensuring account by dual (whole/delta) gating --- genesis-programs/src/lib.rs | 125 +++++++++++++++++------------ genesis/src/main.rs | 2 +- local-cluster/src/local_cluster.rs | 4 +- runtime/src/bank.rs | 38 ++++++--- runtime/src/builtins.rs | 72 +++++++++-------- sdk/src/clock.rs | 2 + 6 files changed, 144 insertions(+), 99 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index a9667cc78296d5..3540963b2319ec 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -13,7 +13,7 @@ use solana_runtime::{ message_processor::{DEFAULT_COMPUTE_BUDGET, DEFAULT_MAX_INVOKE_DEPTH}, }; use solana_sdk::{ - clock::Epoch, + clock::{Epoch, GENESIS_EPOCH}, entrypoint_native::{ErasedProcessInstructionWithContext, ProcessInstructionWithContext}, genesis_config::OperatingMode, inflation::Inflation, @@ -74,71 +74,80 @@ impl std::fmt::Debug for Program { } // given operating_mode and epoch, return the entire set of enabled programs -fn get_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec { +fn get_programs(operating_mode: OperatingMode) -> Vec<(Program, Epoch)> { let mut programs = vec![]; match operating_mode { OperatingMode::Development => { // Programs used for testing - programs.extend(vec![ - Program::BuiltinLoader(solana_bpf_loader_program!()), - Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()), - Program::Native(solana_vest_program!()), - Program::Native(solana_budget_program!()), - Program::Native(solana_exchange_program!()), - ]); + programs.extend( + vec![ + Program::BuiltinLoader(solana_bpf_loader_program!()), + Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()), + Program::Native(solana_vest_program!()), + Program::Native(solana_budget_program!()), + Program::Native(solana_exchange_program!()), + ] + .into_iter() + .map(|program| (program, 0)) + .collect::>(), + ); } OperatingMode::Preview => { // tds enabled async cluster restart with smart contract being enabled // at slot 2196960 (midway epoch 17) with v1.0.1 on Mar 1, 2020 - if epoch >= 17 { - programs.extend(vec![Program::BuiltinLoader( - solana_bpf_loader_deprecated_program!(), - )]); - } - #[allow(clippy::absurd_extreme_comparisons)] - if epoch >= std::u64::MAX { - // The epoch of std::u64::MAX is a placeholder and is expected - // to be reduced in a future network update. - programs.extend(vec![ + programs.extend(vec![( + Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()), + 17, + )]); + // The epoch of Epoch::max_value() is a placeholder and is expected + // to be reduced in a future network update. + programs.extend( + vec![ Program::BuiltinLoader(solana_bpf_loader_program!()), Program::Native(solana_vest_program!()), - ]) - } + ] + .into_iter() + .map(|program| (program, Epoch::MAX)) + .collect::>(), + ); } OperatingMode::Stable => { - if epoch >= 34 { - programs.extend(vec![Program::BuiltinLoader( - solana_bpf_loader_deprecated_program!(), - )]); - } - #[allow(clippy::absurd_extreme_comparisons)] - if epoch >= std::u64::MAX { - // The epoch of std::u64::MAX is a placeholder and is expected - // to be reduced in a future network update. - programs.extend(vec![ + programs.extend(vec![( + Program::BuiltinLoader(solana_bpf_loader_deprecated_program!()), + 34, + )]); + // The epoch of std::u64::MAX is a placeholder and is expected + // to be reduced in a future network update. + programs.extend( + vec![ Program::BuiltinLoader(solana_bpf_loader_program!()), Program::Native(solana_vest_program!()), - ]); - } + ] + .into_iter() + .map(|program| (program, Epoch::MAX)) + .collect::>(), + ); } }; programs } -pub fn get_native_programs(operating_mode: OperatingMode, epoch: Epoch) -> Vec<(String, Pubkey)> { +pub fn get_native_programs_for_genesis(operating_mode: OperatingMode) -> Vec<(String, Pubkey)> { let mut native_programs = vec![]; - for program in get_programs(operating_mode, epoch) { + for (program, start_epoch) in get_programs(operating_mode) { if let Program::Native((string, key)) = program { - native_programs.push((string, key)); + if start_epoch == GENESIS_EPOCH { + native_programs.push((string, key)); + } } } native_programs } pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpochCallback { - Box::new(move |bank: &mut Bank| { + Box::new(move |bank: &mut Bank, initial: bool| { // Be careful to add arbitrary logic here; this should be idempotent and can be called // at arbitrary point in an epoch not only epoch boundaries. // This is because this closure need to be executed immediately after snapshot restoration, @@ -150,13 +159,25 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch info!("Entering new epoch with inflation {:?}", inflation); bank.set_inflation(inflation); } - for program in get_programs(operating_mode, bank.epoch()) { - match program { - Program::Native((name, program_id)) => { - bank.add_native_program(&name, &program_id); - } - Program::BuiltinLoader((name, program_id, process_instruction_with_context)) => { - bank.add_builtin_loader(&name, program_id, process_instruction_with_context); + for (program, start_epoch) in get_programs(operating_mode) { + let should_populate = + initial && bank.epoch() >= start_epoch || !initial && bank.epoch() == start_epoch; + if should_populate { + match program { + Program::Native((name, program_id)) => { + bank.add_native_program(&name, &program_id); + } + Program::BuiltinLoader(( + name, + program_id, + process_instruction_with_context, + )) => { + bank.add_builtin_loader( + &name, + program_id, + process_instruction_with_context, + ); + } } } } @@ -174,8 +195,8 @@ mod tests { #[test] fn test_id_uniqueness() { let mut unique = HashSet::new(); - let programs = get_programs(OperatingMode::Development, 0); - for program in programs { + let programs = get_programs(OperatingMode::Development); + for (program, _start_epoch) in programs { match program { Program::Native((name, id)) => assert!(unique.insert((name, id))), Program::BuiltinLoader((name, id, _)) => assert!(unique.insert((name, id))), @@ -194,14 +215,15 @@ mod tests { #[test] fn test_development_programs() { - assert_eq!(get_programs(OperatingMode::Development, 0).len(), 5); - assert_eq!(get_programs(OperatingMode::Development, 1).len(), 5); + assert_eq!(get_programs(OperatingMode::Development).len(), 5); } #[test] fn test_native_development_programs() { - assert_eq!(get_native_programs(OperatingMode::Development, 0).len(), 3); - assert_eq!(get_native_programs(OperatingMode::Development, 1).len(), 3); + assert_eq!( + get_native_programs_for_genesis(OperatingMode::Development).len(), + 3 + ); } #[test] @@ -219,7 +241,6 @@ mod tests { #[test] fn test_softlaunch_programs() { - assert!(get_programs(OperatingMode::Stable, 1).is_empty()); - assert!(!get_programs(OperatingMode::Stable, std::u64::MAX).is_empty()); + assert!(!get_programs(OperatingMode::Stable).is_empty()); } } diff --git a/genesis/src/main.rs b/genesis/src/main.rs index 3530d7d34336cf..c086e956fbecec 100644 --- a/genesis/src/main.rs +++ b/genesis/src/main.rs @@ -468,7 +468,7 @@ fn main() -> Result<(), Box> { ); let native_instruction_processors = - solana_genesis_programs::get_native_programs(operating_mode, 0); + solana_genesis_programs::get_native_programs_for_genesis(operating_mode); let inflation = solana_genesis_programs::get_inflation(operating_mode, 0).unwrap(); let mut genesis_config = GenesisConfig { diff --git a/local-cluster/src/local_cluster.rs b/local-cluster/src/local_cluster.rs index 7bd16d477449f6..828a0500b91df6 100644 --- a/local-cluster/src/local_cluster.rs +++ b/local-cluster/src/local_cluster.rs @@ -172,7 +172,9 @@ impl LocalCluster { match genesis_config.operating_mode { OperatingMode::Stable | OperatingMode::Preview => { genesis_config.native_instruction_processors = - solana_genesis_programs::get_native_programs(genesis_config.operating_mode, 0) + solana_genesis_programs::get_native_programs_for_genesis( + genesis_config.operating_mode, + ) } _ => (), } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 9be432b37fe119..0cbf56e9e0c8f7 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -189,7 +189,7 @@ impl StatusCacheRc { } } -pub type EnteredEpochCallback = Box; +pub type EnteredEpochCallback = Box; type WrappedEnteredEpochCallback = Arc>>; pub type TransactionProcessResult = (Result<()>, Option); @@ -565,7 +565,7 @@ impl Bank { // Following code may touch AccountsDB, requiring proper ancestors if parent.epoch() < new.epoch() { - new.apply_feature_activations(); + new.apply_feature_activations(false, false); } new.update_slot_hashes(); @@ -587,6 +587,7 @@ impl Bank { /// * Freezes the new bank, assuming that the user will `Bank::new_from_parent` from this bank pub fn warp_from_parent(parent: &Arc, collector_id: &Pubkey, slot: Slot) -> Self { let mut new = Bank::new_from_parent(parent, collector_id, slot); + new.apply_feature_activations(true, true); new.update_epoch_stakes(new.epoch_schedule().get_epoch(slot)); new.tick_height .store(new.max_tick_height(), Ordering::Relaxed); @@ -2629,7 +2630,7 @@ impl Bank { } pub fn finish_init(&mut self) { - self.apply_feature_activations(); + self.apply_feature_activations(true, false); } pub fn set_parent(&mut self, parent: &Arc) { @@ -2653,7 +2654,8 @@ impl Bank { assert!(callback_w.is_none(), "Already callback has been initiated"); *callback_w = Some(entered_epoch_callback); } - self.apply_feature_activations(); + // immedaitely fire the callback as initial invocation + self.reinvoke_entered_epoch_callback(true); } pub fn get_account(&self, pubkey: &Pubkey) -> Option { @@ -3203,18 +3205,28 @@ impl Bank { // This is called from snapshot restore AND for each epoch boundary // The entire code path herein must be idempotent - pub fn apply_feature_activations(&mut self) { + fn apply_feature_activations(&mut self, init_finish_or_warp: bool, initiate_callback: bool) { + self.ensure_builtins(init_finish_or_warp); + self.reinvoke_entered_epoch_callback(initiate_callback); + self.recheck_cross_program_support(); + } + + fn ensure_builtins(&mut self, init_or_warp: bool) { + for (program, start_epoch) in get_builtins(self.operating_mode()) { + let should_populate = init_or_warp && self.epoch() >= start_epoch + || !init_or_warp && self.epoch() == start_epoch; + if should_populate { + self.add_builtin(&program.name, program.id, program.entrypoint); + } + } + } + + fn reinvoke_entered_epoch_callback(&mut self, initiate: bool) { if let Some(entered_epoch_callback) = self.entered_epoch_callback.clone().read().unwrap().as_ref() { - entered_epoch_callback(self) - } - - for program in get_builtins(self.operating_mode(), self.epoch()) { - self.add_builtin(&program.name, program.id, program.entrypoint); + entered_epoch_callback(self, initiate) } - - self.recheck_cross_program_support(); } fn recheck_cross_program_support(self: &mut Bank) { @@ -6410,7 +6422,7 @@ mod tests { .unwrap() .initiate_entered_epoch_callback({ let callback_count = callback_count.clone(); - Box::new(move |_| { + Box::new(move |_, _| { callback_count.fetch_add(1, Ordering::SeqCst); }) }); diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index b3c436828f2ea8..d7d400acdceab5 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -2,41 +2,50 @@ use crate::{ bank::{Builtin, Entrypoint}, system_instruction_processor, }; -use solana_sdk::{clock::Epoch, genesis_config::OperatingMode, system_program}; +use solana_sdk::{ + clock::{Epoch, GENESIS_EPOCH}, + genesis_config::OperatingMode, + system_program, +}; use log::*; -/// The entire set of available builtin programs that should be active at the given (operating_mode, epoch) -pub fn get_builtins(operating_mode: OperatingMode, epoch: Epoch) -> Vec { - trace!("get_builtins: {:?}, {:?}", operating_mode, epoch); +/// The entire set of available builtin programs that should be active at the given operating_mode +pub fn get_builtins(operating_mode: OperatingMode) -> Vec<(Builtin, Epoch)> { + trace!("get_builtins: {:?}", operating_mode); let mut builtins = vec![]; - builtins.extend(vec![ - Builtin::new( - "system_program", - system_program::id(), - Entrypoint::Program(system_instruction_processor::process_instruction), - ), - Builtin::new( - "config_program", - solana_config_program::id(), - Entrypoint::Program(solana_config_program::config_processor::process_instruction), - ), - Builtin::new( - "stake_program", - solana_stake_program::id(), - Entrypoint::Program(solana_stake_program::stake_instruction::process_instruction), - ), - Builtin::new( - "vote_program", - solana_vote_program::id(), - Entrypoint::Program(solana_vote_program::vote_instruction::process_instruction), - ), - ]); + builtins.extend( + vec![ + Builtin::new( + "system_program", + system_program::id(), + Entrypoint::Program(system_instruction_processor::process_instruction), + ), + Builtin::new( + "config_program", + solana_config_program::id(), + Entrypoint::Program(solana_config_program::config_processor::process_instruction), + ), + Builtin::new( + "stake_program", + solana_stake_program::id(), + Entrypoint::Program(solana_stake_program::stake_instruction::process_instruction), + ), + Builtin::new( + "vote_program", + solana_vote_program::id(), + Entrypoint::Program(solana_vote_program::vote_instruction::process_instruction), + ), + ] + .into_iter() + .map(|program| (program, GENESIS_EPOCH)) + .collect::>(), + ); // repurpose Preview for test_get_builtins because the Development is overloaded... #[cfg(test)] - if operating_mode == OperatingMode::Preview && epoch >= 2 { + if operating_mode == OperatingMode::Preview { use solana_sdk::instruction::InstructionError; use solana_sdk::{account::KeyedAccount, pubkey::Pubkey}; use std::str::FromStr; @@ -48,10 +57,9 @@ pub fn get_builtins(operating_mode: OperatingMode, epoch: Epoch) -> Vec Err(InstructionError::Custom(42)) } let program_id = Pubkey::from_str("7saCc6X5a2syoYANA5oUUnPZLcLMfKoSjiDhFU5fbpoK").unwrap(); - builtins.extend(vec![Builtin::new( - "mock", - program_id, - Entrypoint::Program(mock_ix_processor), + builtins.extend(vec![( + Builtin::new("mock", program_id, Entrypoint::Program(mock_ix_processor)), + 2, )]); } @@ -91,7 +99,7 @@ mod tests { )); let warped_slot = genesis_config.epoch_schedule.get_first_slot_in_epoch(999); - let warped_bank = Arc::new(Bank::new_from_parent( + let warped_bank = Arc::new(Bank::warp_from_parent( &bank0, &Pubkey::default(), warped_slot, diff --git a/sdk/src/clock.rs b/sdk/src/clock.rs index 0aa4e8068f70ea..f576a9bb61663c 100644 --- a/sdk/src/clock.rs +++ b/sdk/src/clock.rs @@ -57,6 +57,8 @@ pub type Slot = u64; /// some number of Slots. pub type Epoch = u64; +pub const GENESIS_EPOCH: Epoch = 0; + /// SlotIndex is an index to the slots of a epoch pub type SlotIndex = u64; From 39201b870ea292fbafbec2099146f3d3e0a53315 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 25 Aug 2020 03:27:27 +0900 Subject: [PATCH 22/23] Fix frozen abi implementation... --- sdk/src/abi_example.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sdk/src/abi_example.rs b/sdk/src/abi_example.rs index f2126c32ab575c..81692d2388f201 100644 --- a/sdk/src/abi_example.rs +++ b/sdk/src/abi_example.rs @@ -305,6 +305,13 @@ impl AbiExample for Box { } } +impl AbiExample for Box { + fn example() -> Self { + info!("AbiExample for (Box): {}", type_name::()); + Box::new(move |_t: &mut T, _u: U| {}) + } +} + impl AbiExample for Box<[T]> { fn example() -> Self { info!("AbiExample for (Box<[T]>): {}", type_name::()); From b6615fbb91d016650098101e1e56cbadb11fda78 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 25 Aug 2020 15:36:37 +0900 Subject: [PATCH 23/23] Move compute budget constatnt init back into bank --- genesis-programs/src/lib.rs | 8 +------- runtime/src/bank.rs | 4 +++- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/genesis-programs/src/lib.rs b/genesis-programs/src/lib.rs index 3540963b2319ec..2b1277d6a20fee 100644 --- a/genesis-programs/src/lib.rs +++ b/genesis-programs/src/lib.rs @@ -8,10 +8,7 @@ extern crate solana_exchange_program; extern crate solana_vest_program; use log::*; -use solana_runtime::{ - bank::{Bank, EnteredEpochCallback}, - message_processor::{DEFAULT_COMPUTE_BUDGET, DEFAULT_MAX_INVOKE_DEPTH}, -}; +use solana_runtime::bank::{Bank, EnteredEpochCallback}; use solana_sdk::{ clock::{Epoch, GENESIS_EPOCH}, entrypoint_native::{ErasedProcessInstructionWithContext, ProcessInstructionWithContext}, @@ -181,9 +178,6 @@ pub fn get_entered_epoch_callback(operating_mode: OperatingMode) -> EnteredEpoch } } } - - bank.set_max_invoke_depth(DEFAULT_MAX_INVOKE_DEPTH); - bank.set_compute_budget(DEFAULT_COMPUTE_BUDGET); }) } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 0cbf56e9e0c8f7..6380308738342c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -13,7 +13,7 @@ use crate::{ builtins::get_builtins, epoch_stakes::{EpochStakes, NodeVoteAccounts}, log_collector::LogCollector, - message_processor::MessageProcessor, + message_processor::{MessageProcessor, DEFAULT_COMPUTE_BUDGET, DEFAULT_MAX_INVOKE_DEPTH}, nonce_utils, rent_collector::RentCollector, stakes::Stakes, @@ -3209,6 +3209,8 @@ impl Bank { self.ensure_builtins(init_finish_or_warp); self.reinvoke_entered_epoch_callback(initiate_callback); self.recheck_cross_program_support(); + self.set_max_invoke_depth(DEFAULT_MAX_INVOKE_DEPTH); + self.set_compute_budget(DEFAULT_COMPUTE_BUDGET); } fn ensure_builtins(&mut self, init_or_warp: bool) {