From 562254ef56b850f74ed2ad1a49514421c76be2e9 Mon Sep 17 00:00:00 2001 From: Yihau Chen Date: Fri, 5 Apr 2024 16:51:30 +0800 Subject: [PATCH 01/12] remove InetAddr from streamer/src/recvmmsg.rs (#558) * remove InetAddr from streamer/src/recvmmsg.rs * remove 'allow deprecated' * add ref link --- streamer/src/recvmmsg.rs | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/streamer/src/recvmmsg.rs b/streamer/src/recvmmsg.rs index 72c7d3c51cdba1..b06ab0c43fc6a0 100644 --- a/streamer/src/recvmmsg.rs +++ b/streamer/src/recvmmsg.rs @@ -1,8 +1,5 @@ //! The `recvmmsg` module provides recvmmsg() API implementation -#[cfg(target_os = "linux")] -#[allow(deprecated)] -use nix::sys::socket::InetAddr; pub use solana_perf::packet::NUM_RCVMMSGS; use { crate::packet::{Meta, Packet}, @@ -12,7 +9,11 @@ use { use { itertools::izip, libc::{iovec, mmsghdr, sockaddr_storage, socklen_t, AF_INET, AF_INET6, MSG_WAITFORONE}, - std::{mem, os::unix::io::AsRawFd}, + std::{ + mem, + net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6}, + os::unix::io::AsRawFd, + }, }; #[cfg(not(target_os = "linux"))] @@ -43,22 +44,31 @@ pub fn recv_mmsg(socket: &UdpSocket, packets: &mut [Packet]) -> io::Result Option { +fn cast_socket_addr(addr: &sockaddr_storage, hdr: &mmsghdr) -> Option { use libc::{sa_family_t, sockaddr_in, sockaddr_in6}; const SOCKADDR_IN_SIZE: usize = std::mem::size_of::(); const SOCKADDR_IN6_SIZE: usize = std::mem::size_of::(); if addr.ss_family == AF_INET as sa_family_t && hdr.msg_hdr.msg_namelen == SOCKADDR_IN_SIZE as socklen_t { - let addr = addr as *const _ as *const sockaddr_in; - return Some(unsafe { InetAddr::V4(*addr) }); + // ref: https://github.com/rust-lang/socket2/blob/65085d9dff270e588c0fbdd7217ec0b392b05ef2/src/sockaddr.rs#L167-L172 + let addr = unsafe { &*(addr as *const _ as *const sockaddr_in) }; + return Some(SocketAddr::V4(SocketAddrV4::new( + Ipv4Addr::from(addr.sin_addr.s_addr.to_ne_bytes()), + u16::from_be(addr.sin_port), + ))); } if addr.ss_family == AF_INET6 as sa_family_t && hdr.msg_hdr.msg_namelen == SOCKADDR_IN6_SIZE as socklen_t { - let addr = addr as *const _ as *const sockaddr_in6; - return Some(unsafe { InetAddr::V6(*addr) }); + // ref: https://github.com/rust-lang/socket2/blob/65085d9dff270e588c0fbdd7217ec0b392b05ef2/src/sockaddr.rs#L174-L189 + let addr = unsafe { &*(addr as *const _ as *const sockaddr_in6) }; + return Some(SocketAddr::V6(SocketAddrV6::new( + Ipv6Addr::from(addr.sin6_addr.s6_addr), + u16::from_be(addr.sin6_port), + addr.sin6_flowinfo, + addr.sin6_scope_id, + ))); } error!( "recvmmsg unexpected ss_family:{} msg_namelen:{}", @@ -118,7 +128,7 @@ pub fn recv_mmsg(sock: &UdpSocket, packets: &mut [Packet]) -> io::Result Date: Fri, 5 Apr 2024 13:03:18 +0200 Subject: [PATCH 02/12] Fix - `FailedVerification` and `Closed` tombstones (#419) * Only the verifier can cause FailedVerification, everything else is Closed * Removes the environments parameter from load_program_accounts(). * cargo fmt * Simplify invocation of deployed program * Attempt to invoke a program before it is deployed * Attempt to invoke a buffer before it is used in a deployment * Escalates Option return value of load_program_accounts() to load_program_with_pubkey(). * Review feedback --- ledger-tool/src/program.rs | 6 +- program-runtime/src/loaded_programs.rs | 25 +- runtime/src/bank.rs | 25 +- runtime/src/bank/tests.rs | 82 ++++--- svm/src/transaction_processor.rs | 320 ++++++++++++------------- 5 files changed, 245 insertions(+), 213 deletions(-) diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index d4ecb0d4694b76..7a5f5cc6492e6b 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -522,7 +522,11 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { let mut loaded_programs = bank.new_program_cache_for_tx_batch_for_slot(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET); for key in cached_account_keys { - loaded_programs.replenish(key, bank.load_program(&key, false, bank.epoch())); + loaded_programs.replenish( + key, + bank.load_program(&key, false, bank.epoch()) + .expect("Couldn't find program account"), + ); debug!("Loaded program {}", key); } invoke_context.programs_loaded_for_tx_batch = &loaded_programs; diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 99c77bf9430ba6..419ae7330b410e 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -63,11 +63,11 @@ pub trait ForkGraph { /// Actual payload of [LoadedProgram]. #[derive(Default)] pub enum LoadedProgramType { - /// Tombstone for programs which did not pass the verifier. - /// - /// These can potentially come back alive if the environment changes. + /// Tombstone for programs which currently do not pass the verifier but could if the feature set changed. FailedVerification(ProgramRuntimeEnvironment), - /// Tombstone for programs which were explicitly undeployed / closed. + /// Tombstone for programs that were either explicitly closed or never deployed. + /// + /// It's also used for accounts belonging to program loaders, that don't actually contain program code (e.g. buffer accounts for LoaderV3 programs). #[default] Closed, /// Tombstone for programs which have recently been modified but the new version is not visible yet. @@ -776,12 +776,17 @@ impl ProgramCache { Ok(index) => { let existing = slot_versions.get_mut(index).unwrap(); match (&existing.program, &entry.program) { + // Add test for Closed => Loaded transition in same slot (LoadedProgramType::Builtin(_), LoadedProgramType::Builtin(_)) + | (LoadedProgramType::Closed, LoadedProgramType::LegacyV0(_)) + | (LoadedProgramType::Closed, LoadedProgramType::LegacyV1(_)) + | (LoadedProgramType::Closed, LoadedProgramType::Typed(_)) | (LoadedProgramType::Unloaded(_), LoadedProgramType::LegacyV0(_)) | (LoadedProgramType::Unloaded(_), LoadedProgramType::LegacyV1(_)) | (LoadedProgramType::Unloaded(_), LoadedProgramType::Typed(_)) => {} #[cfg(test)] - (LoadedProgramType::Unloaded(_), LoadedProgramType::TestLoaded(_)) => {} + (LoadedProgramType::Closed, LoadedProgramType::TestLoaded(_)) + | (LoadedProgramType::Unloaded(_), LoadedProgramType::TestLoaded(_)) => {} _ => { // Something is wrong, I can feel it ... error!("ProgramCache::assign_program() failed key={:?} existing={:?} entry={:?}", key, slot_versions, entry); @@ -1680,7 +1685,6 @@ mod tests { #[test_matrix( ( LoadedProgramType::FailedVerification(Arc::new(BuiltinProgram::new_mock())), - LoadedProgramType::Closed, LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())), ), ( @@ -1692,7 +1696,10 @@ mod tests { ) )] #[test_matrix( - (LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())),), + ( + LoadedProgramType::Closed, + LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())), + ), ( LoadedProgramType::FailedVerification(Arc::new(BuiltinProgram::new_mock())), LoadedProgramType::Closed, @@ -1739,6 +1746,10 @@ mod tests { ); } + #[test_case( + LoadedProgramType::Closed, + LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())) + )] #[test_case( LoadedProgramType::Unloaded(Arc::new(BuiltinProgram::new_mock())), LoadedProgramType::TestLoaded(Arc::new(BuiltinProgram::new_mock())) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1571ff11e154c1..a05cf04bf4185c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1253,16 +1253,19 @@ impl Bank { { let effective_epoch = program_cache.latest_root_epoch.saturating_add(1); drop(program_cache); - let recompiled = new.load_program(&key, false, effective_epoch); - recompiled - .tx_usage_counter - .fetch_add(program_to_recompile.tx_usage_counter.load(Relaxed), Relaxed); - recompiled - .ix_usage_counter - .fetch_add(program_to_recompile.ix_usage_counter.load(Relaxed), Relaxed); - let mut program_cache = - new.transaction_processor.program_cache.write().unwrap(); - program_cache.assign_program(key, recompiled); + if let Some(recompiled) = new.load_program(&key, false, effective_epoch) { + recompiled.tx_usage_counter.fetch_add( + program_to_recompile.tx_usage_counter.load(Relaxed), + Relaxed, + ); + recompiled.ix_usage_counter.fetch_add( + program_to_recompile.ix_usage_counter.load(Relaxed), + Relaxed, + ); + let mut program_cache = + new.transaction_processor.program_cache.write().unwrap(); + program_cache.assign_program(key, recompiled); + } } } else if new.epoch() != program_cache.latest_root_epoch || slot_index.saturating_add(slots_in_recompilation_phase) >= slots_in_epoch @@ -6886,7 +6889,7 @@ impl Bank { pubkey: &Pubkey, reload: bool, effective_epoch: Epoch, - ) -> Arc { + ) -> Option> { self.transaction_processor .load_program_with_pubkey(self, pubkey, reload, effective_epoch) } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 31e189e07960a6..9e8b44ffbe625e 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -39,11 +39,7 @@ use { compute_budget::ComputeBudget, compute_budget_processor::{self, MAX_COMPUTE_UNIT_LIMIT}, declare_process_instruction, - invoke_context::mock_process_instruction, - loaded_programs::{ - LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch, - DELAY_VISIBILITY_SLOT_OFFSET, - }, + loaded_programs::{LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch}, prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType}, timings::ExecuteTimings, }, @@ -7141,7 +7137,7 @@ fn test_bank_load_program() { programdata_account.set_rent_epoch(1); bank.store_account_and_update_capitalization(&key1, &program_account); bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); - let program = bank.load_program(&key1, false, bank.epoch()); + let program = bank.load_program(&key1, false, bank.epoch()).unwrap(); assert_matches!(program.program, LoadedProgramType::LegacyV1(_)); assert_eq!( program.account_size, @@ -7167,6 +7163,26 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { ); let upgrade_authority_keypair = Keypair::new(); + // Invoke not yet deployed program + let instruction = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new()); + let invocation_message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); + let binding = mint_keypair.insecure_clone(); + let transaction = Transaction::new( + &[&binding], + invocation_message.clone(), + bank.last_blockhash(), + ); + assert_eq!( + bank.process_transaction(&transaction), + Err(TransactionError::ProgramAccountNotFound), + ); + { + // Make sure it is not in the cache because the account owner is not a loader + let program_cache = bank.transaction_processor.program_cache.read().unwrap(); + let slot_versions = program_cache.get_slot_versions_for_tests(&program_keypair.pubkey()); + assert!(slot_versions.is_empty()); + } + // Load program file let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so") .expect("file open failed"); @@ -7214,6 +7230,28 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { &bpf_loader_upgradeable::id(), ); + // Test buffer invocation + bank.store_account(&buffer_address, &buffer_account); + let instruction = Instruction::new_with_bytes(buffer_address, &[], Vec::new()); + let message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); + let transaction = Transaction::new(&[&binding], message, bank.last_blockhash()); + assert_eq!( + bank.process_transaction(&transaction), + Err(TransactionError::InstructionError( + 0, + InstructionError::InvalidAccountData, + )), + ); + { + let program_cache = bank.transaction_processor.program_cache.read().unwrap(); + let slot_versions = program_cache.get_slot_versions_for_tests(&buffer_address); + assert_eq!(slot_versions.len(), 1); + assert!(matches!( + slot_versions[0].program, + LoadedProgramType::Closed, + )); + } + // Test successful deploy let payer_base_balance = LAMPORTS_PER_SOL; let deploy_fees = { @@ -7231,7 +7269,6 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { &system_program::id(), ), ); - bank.store_account(&buffer_address, &buffer_account); bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default()); bank.store_account(&programdata_address, &AccountSharedData::default()); let message = Message::new( @@ -7296,30 +7333,15 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { assert_eq!(*elf.get(i).unwrap(), *byte); } - let loaded_program = bank.load_program(&program_keypair.pubkey(), false, bank.epoch()); + // Advance the bank so that the program becomes effective + goto_end_of_slot(bank.clone()); + let bank = bank_client + .advance_slot(1, bank_forks.as_ref(), &mint_keypair.pubkey()) + .unwrap(); - // Invoke deployed program - mock_process_instruction( - &bpf_loader_upgradeable::id(), - vec![0, 1], - &[], - vec![ - (programdata_address, post_programdata_account), - (program_keypair.pubkey(), post_program_account), - ], - Vec::new(), - Ok(()), - solana_bpf_loader_program::Entrypoint::vm, - |invoke_context| { - invoke_context - .programs_modified_by_tx - .set_slot_for_tests(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET); - invoke_context - .programs_modified_by_tx - .replenish(program_keypair.pubkey(), loaded_program.clone()); - }, - |_invoke_context| {}, - ); + // Invoke the deployed program + let transaction = Transaction::new(&[&binding], invocation_message, bank.last_blockhash()); + assert!(bank.process_transaction(&transaction).is_ok()); // Test initialized program account bank.clear_signatures(); diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index cc84347fa0d426..d827695e159f4b 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -20,7 +20,7 @@ use { loaded_programs::{ ForkGraph, LoadProgramMetrics, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, LoadedProgramsForTxBatch, ProgramCache, ProgramRuntimeEnvironment, - ProgramRuntimeEnvironments, DELAY_VISIBILITY_SLOT_OFFSET, + DELAY_VISIBILITY_SLOT_OFFSET, }, log_collector::LogCollector, runtime_config::RuntimeConfig, @@ -112,8 +112,7 @@ pub trait TransactionProcessingCallback { #[derive(Debug)] enum ProgramAccountLoadResult { - AccountNotFound, - InvalidAccountData(ProgramRuntimeEnvironment), + InvalidAccountData, ProgramOfLoaderV1orV2(AccountSharedData), ProgramOfLoaderV3(AccountSharedData, AccountSharedData, Slot), ProgramOfLoaderV4(AccountSharedData, Slot), @@ -385,15 +384,18 @@ impl TransactionBatchProcessor { result } - /// Load program with a specific pubkey from program cache, and - /// update the program's access slot as a side-effect. + /// Loads the program with the given pubkey. + /// + /// If the account doesn't exist it returns `None`. If the account does exist, it must be a program + /// account (belong to one of the program loaders). Returns `Some(InvalidAccountData)` if the program + /// account is `Closed`, contains invalid data or any of the programdata accounts are invalid. pub fn load_program_with_pubkey( &self, callbacks: &CB, pubkey: &Pubkey, reload: bool, effective_epoch: Epoch, - ) -> Arc { + ) -> Option> { let program_cache = self.program_cache.read().unwrap(); let environments = program_cache.get_environments_for_epoch(effective_epoch); let mut load_program_metrics = LoadProgramMetrics { @@ -401,74 +403,69 @@ impl TransactionBatchProcessor { ..LoadProgramMetrics::default() }; - let mut loaded_program = - match self.load_program_accounts(callbacks, pubkey, environments) { - ProgramAccountLoadResult::AccountNotFound => Ok(LoadedProgram::new_tombstone( - self.slot, - LoadedProgramType::Closed, - )), - - ProgramAccountLoadResult::InvalidAccountData(env) => Err((self.slot, env)), + let mut loaded_program = match self.load_program_accounts(callbacks, pubkey)? { + ProgramAccountLoadResult::InvalidAccountData => Ok(LoadedProgram::new_tombstone( + self.slot, + LoadedProgramType::Closed, + )), + + ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account) => { + Self::load_program_from_bytes( + &mut load_program_metrics, + program_account.data(), + program_account.owner(), + program_account.data().len(), + 0, + environments.program_runtime_v1.clone(), + reload, + ) + .map_err(|_| (0, environments.program_runtime_v1.clone())) + } - ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account) => { + ProgramAccountLoadResult::ProgramOfLoaderV3( + program_account, + programdata_account, + slot, + ) => programdata_account + .data() + .get(UpgradeableLoaderState::size_of_programdata_metadata()..) + .ok_or(Box::new(InstructionError::InvalidAccountData).into()) + .and_then(|programdata| { Self::load_program_from_bytes( &mut load_program_metrics, - program_account.data(), + programdata, program_account.owner(), - program_account.data().len(), - 0, + program_account + .data() + .len() + .saturating_add(programdata_account.data().len()), + slot, environments.program_runtime_v1.clone(), reload, ) - .map_err(|_| (0, environments.program_runtime_v1.clone())) - } + }) + .map_err(|_| (slot, environments.program_runtime_v1.clone())), - ProgramAccountLoadResult::ProgramOfLoaderV3( - program_account, - programdata_account, - slot, - ) => programdata_account - .data() - .get(UpgradeableLoaderState::size_of_programdata_metadata()..) - .ok_or(Box::new(InstructionError::InvalidAccountData).into()) - .and_then(|programdata| { - Self::load_program_from_bytes( - &mut load_program_metrics, - programdata, - program_account.owner(), - program_account - .data() - .len() - .saturating_add(programdata_account.data().len()), - slot, - environments.program_runtime_v1.clone(), - reload, - ) - }) - .map_err(|_| (slot, environments.program_runtime_v1.clone())), - - ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot) => { - program_account - .data() - .get(LoaderV4State::program_data_offset()..) - .ok_or(Box::new(InstructionError::InvalidAccountData).into()) - .and_then(|elf_bytes| { - Self::load_program_from_bytes( - &mut load_program_metrics, - elf_bytes, - &loader_v4::id(), - program_account.data().len(), - slot, - environments.program_runtime_v2.clone(), - reload, - ) - }) - .map_err(|_| (slot, environments.program_runtime_v2.clone())) - } - } - .unwrap_or_else(|(slot, env)| { - LoadedProgram::new_tombstone(slot, LoadedProgramType::FailedVerification(env)) - }); + ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot) => program_account + .data() + .get(LoaderV4State::program_data_offset()..) + .ok_or(Box::new(InstructionError::InvalidAccountData).into()) + .and_then(|elf_bytes| { + Self::load_program_from_bytes( + &mut load_program_metrics, + elf_bytes, + &loader_v4::id(), + program_account.data().len(), + slot, + environments.program_runtime_v2.clone(), + reload, + ) + }) + .map_err(|_| (slot, environments.program_runtime_v2.clone())), + } + .unwrap_or_else(|(slot, env)| { + LoadedProgram::new_tombstone(slot, LoadedProgramType::FailedVerification(env)) + }); let mut timings = ExecuteDetailsTimings::default(); load_program_metrics.submit_datapoint(&mut timings); @@ -488,7 +485,7 @@ impl TransactionBatchProcessor { .max(self.epoch_schedule.get_first_slot_in_epoch(effective_epoch)); } loaded_program.update_access_slot(self.slot); - Arc::new(loaded_program) + Some(Arc::new(loaded_program)) } fn replenish_program_cache( @@ -553,7 +550,9 @@ impl TransactionBatchProcessor { if let Some((key, count)) = program_to_load { // Load, verify and compile one program. - let program = self.load_program_with_pubkey(callback, &key, false, self.epoch); + let program = self + .load_program_with_pubkey(callback, &key, false, self.epoch) + .expect("called load_program_with_pubkey() with nonexistent account"); program.tx_usage_counter.store(count, Ordering::Relaxed); program_to_store = Some((key, program)); } else if missing_programs.is_empty() { @@ -839,56 +838,48 @@ impl TransactionBatchProcessor { &self, callbacks: &CB, pubkey: &Pubkey, - environments: &ProgramRuntimeEnvironments, - ) -> ProgramAccountLoadResult { - let program_account = match callbacks.get_account_shared_data(pubkey) { - None => return ProgramAccountLoadResult::AccountNotFound, - Some(account) => account, - }; - - debug_assert!(solana_bpf_loader_program::check_loader_id( - program_account.owner() - )); + ) -> Option { + let program_account = callbacks.get_account_shared_data(pubkey)?; if loader_v4::check_id(program_account.owner()) { - return solana_loader_v4_program::get_state(program_account.data()) - .ok() - .and_then(|state| { - (!matches!(state.status, LoaderV4Status::Retracted)).then_some(state.slot) - }) - .map(|slot| ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot)) - .unwrap_or(ProgramAccountLoadResult::InvalidAccountData( - environments.program_runtime_v2.clone(), - )); + return Some( + solana_loader_v4_program::get_state(program_account.data()) + .ok() + .and_then(|state| { + (!matches!(state.status, LoaderV4Status::Retracted)).then_some(state.slot) + }) + .map(|slot| ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot)) + .unwrap_or(ProgramAccountLoadResult::InvalidAccountData), + ); } if !bpf_loader_upgradeable::check_id(program_account.owner()) { - return ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account); + return Some(ProgramAccountLoadResult::ProgramOfLoaderV1orV2( + program_account, + )); } if let Ok(UpgradeableLoaderState::Program { programdata_address, }) = program_account.state() { - let programdata_account = match callbacks.get_account_shared_data(&programdata_address) - { - None => return ProgramAccountLoadResult::AccountNotFound, - Some(account) => account, - }; - - if let Ok(UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address: _, - }) = programdata_account.state() + if let Some(programdata_account) = + callbacks.get_account_shared_data(&programdata_address) { - return ProgramAccountLoadResult::ProgramOfLoaderV3( - program_account, - programdata_account, + if let Ok(UpgradeableLoaderState::ProgramData { slot, - ); + upgrade_authority_address: _, + }) = programdata_account.state() + { + return Some(ProgramAccountLoadResult::ProgramOfLoaderV3( + program_account, + programdata_account, + slot, + )); + } } } - ProgramAccountLoadResult::InvalidAccountData(environments.program_runtime_v1.clone()) + Some(ProgramAccountLoadResult::InvalidAccountData) } /// Extract the InnerInstructionsList from a TransactionContext @@ -970,7 +961,8 @@ mod tests { use { super::*, solana_program_runtime::{ - loaded_programs::BlockRelation, solana_rbpf::program::BuiltinProgram, + loaded_programs::{BlockRelation, ProgramRuntimeEnvironments}, + solana_rbpf::program::BuiltinProgram, }, solana_sdk::{ account::{create_account_shared_data_for_test, WritableAccount}, @@ -1087,12 +1079,10 @@ mod tests { fn test_load_program_accounts_account_not_found() { let mut mock_bank = MockBankCallback::default(); let key = Pubkey::new_unique(); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); - - assert!(matches!(result, ProgramAccountLoadResult::AccountNotFound)); + let result = batch_processor.load_program_accounts(&mock_bank, &key); + assert!(result.is_none()); let mut account_data = AccountSharedData::default(); account_data.set_owner(bpf_loader_upgradeable::id()); @@ -1104,17 +1094,20 @@ mod tests { .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); - assert!(matches!(result, ProgramAccountLoadResult::AccountNotFound)); + let result = batch_processor.load_program_accounts(&mock_bank, &key); + assert!(matches!( + result, + Some(ProgramAccountLoadResult::InvalidAccountData) + )); account_data.set_data(Vec::new()); mock_bank.account_shared_data.insert(key, account_data); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); assert!(matches!( result, - ProgramAccountLoadResult::InvalidAccountData(_) + Some(ProgramAccountLoadResult::InvalidAccountData) )); } @@ -1124,26 +1117,25 @@ mod tests { let mut mock_bank = MockBankCallback::default(); let mut account_data = AccountSharedData::default(); account_data.set_owner(loader_v4::id()); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); mock_bank .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); assert!(matches!( result, - ProgramAccountLoadResult::InvalidAccountData(_) + Some(ProgramAccountLoadResult::InvalidAccountData) )); account_data.set_data(vec![0; 64]); mock_bank .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); assert!(matches!( result, - ProgramAccountLoadResult::InvalidAccountData(_) + Some(ProgramAccountLoadResult::InvalidAccountData) )); let loader_data = LoaderV4State { @@ -1161,10 +1153,10 @@ mod tests { .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); match result { - ProgramAccountLoadResult::ProgramOfLoaderV4(data, slot) => { + Some(ProgramAccountLoadResult::ProgramOfLoaderV4(data, slot)) => { assert_eq!(data, account_data); assert_eq!(slot, 25); } @@ -1179,15 +1171,14 @@ mod tests { let mut mock_bank = MockBankCallback::default(); let mut account_data = AccountSharedData::default(); account_data.set_owner(bpf_loader::id()); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); mock_bank .account_shared_data .insert(key, account_data.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key); match result { - ProgramAccountLoadResult::ProgramOfLoaderV1orV2(data) => { + Some(ProgramAccountLoadResult::ProgramOfLoaderV1orV2(data)) => { assert_eq!(data, account_data); } _ => panic!("Invalid result"), @@ -1199,7 +1190,6 @@ mod tests { let key1 = Pubkey::new_unique(); let key2 = Pubkey::new_unique(); let mut mock_bank = MockBankCallback::default(); - let environment = ProgramRuntimeEnvironments::default(); let batch_processor = TransactionBatchProcessor::::default(); let mut account_data = AccountSharedData::default(); @@ -1223,10 +1213,10 @@ mod tests { .account_shared_data .insert(key2, account_data2.clone()); - let result = batch_processor.load_program_accounts(&mock_bank, &key1, &environment); + let result = batch_processor.load_program_accounts(&mock_bank, &key1); match result { - ProgramAccountLoadResult::ProgramOfLoaderV3(data1, data2, slot) => { + Some(ProgramAccountLoadResult::ProgramOfLoaderV3(data1, data2, slot)) => { assert_eq!(data1, account_data); assert_eq!(data2, account_data2); assert_eq!(slot, 25); @@ -1291,9 +1281,7 @@ mod tests { let batch_processor = TransactionBatchProcessor::::default(); let result = batch_processor.load_program_with_pubkey(&mock_bank, &key, false, 50); - - let loaded_program = LoadedProgram::new_tombstone(0, LoadedProgramType::Closed); - assert_eq!(result, Arc::new(loaded_program)); + assert!(result.is_none()); } #[test] @@ -1321,7 +1309,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); } #[test] @@ -1349,7 +1337,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); let buffer = load_test_program(); account_data.set_data(buffer); @@ -1371,7 +1359,7 @@ mod tests { false, ); - assert_eq!(result, Arc::new(expected.unwrap())); + assert_eq!(result.unwrap(), Arc::new(expected.unwrap())); } #[test] @@ -1416,7 +1404,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); let mut buffer = load_test_program(); let mut header = bincode::serialize(&state).unwrap(); @@ -1451,7 +1439,7 @@ mod tests { environments.program_runtime_v1.clone(), false, ); - assert_eq!(result, Arc::new(expected.unwrap())); + assert_eq!(result.unwrap(), Arc::new(expected.unwrap())); } #[test] @@ -1490,7 +1478,7 @@ mod tests { .program_runtime_v1, ), ); - assert_eq!(result, Arc::new(loaded_program)); + assert_eq!(result.unwrap(), Arc::new(loaded_program)); let mut header = account_data.data().to_vec(); let mut complement = @@ -1523,7 +1511,7 @@ mod tests { environments.program_runtime_v1.clone(), false, ); - assert_eq!(result, Arc::new(expected.unwrap())); + assert_eq!(result.unwrap(), Arc::new(expected.unwrap())); } #[test] @@ -1546,7 +1534,7 @@ mod tests { let result = batch_processor.load_program_with_pubkey(&mock_bank, &key, false, 20); let slot = batch_processor.epoch_schedule.get_first_slot_in_epoch(20); - assert_eq!(result.effective_slot, slot); + assert_eq!(result.unwrap().effective_slot, slot); } #[test] @@ -1825,47 +1813,51 @@ mod tests { assert_eq!(error_metrics.instruction_error, 1); } + #[test] + #[should_panic = "called load_program_with_pubkey() with nonexistent account"] + fn test_replenish_program_cache_with_nonexistent_accounts() { + let mock_bank = MockBankCallback::default(); + let batch_processor = TransactionBatchProcessor::::default(); + batch_processor.program_cache.write().unwrap().fork_graph = + Some(Arc::new(RwLock::new(TestForkGraph {}))); + let key = Pubkey::new_unique(); + let owner = Pubkey::new_unique(); + + let mut account_maps: HashMap = HashMap::new(); + account_maps.insert(key, (&owner, 4)); + + batch_processor.replenish_program_cache(&mock_bank, &account_maps, true); + } + #[test] fn test_replenish_program_cache() { - // Case 1 let mut mock_bank = MockBankCallback::default(); let batch_processor = TransactionBatchProcessor::::default(); batch_processor.program_cache.write().unwrap().fork_graph = Some(Arc::new(RwLock::new(TestForkGraph {}))); - let key1 = Pubkey::new_unique(); - let key2 = Pubkey::new_unique(); + let key = Pubkey::new_unique(); let owner = Pubkey::new_unique(); let mut account_data = AccountSharedData::default(); account_data.set_owner(bpf_loader::id()); - mock_bank.account_shared_data.insert(key2, account_data); + mock_bank.account_shared_data.insert(key, account_data); let mut account_maps: HashMap = HashMap::new(); - account_maps.insert(key1, (&owner, 2)); - - account_maps.insert(key2, (&owner, 4)); - let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, false); + account_maps.insert(key, (&owner, 4)); - let program1 = result.find(&key1).unwrap(); - assert!(matches!(program1.program, LoadedProgramType::Closed)); - assert!(!result.hit_max_limit); - let program2 = result.find(&key2).unwrap(); - assert!(matches!( - program2.program, - LoadedProgramType::FailedVerification(_) - )); - - // Case 2 - let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, true); - - let program1 = result.find(&key1).unwrap(); - assert!(matches!(program1.program, LoadedProgramType::Closed)); - assert!(!result.hit_max_limit); - let program2 = result.find(&key2).unwrap(); - assert!(matches!( - program2.program, - LoadedProgramType::FailedVerification(_) - )); + for limit_to_load_programs in [false, true] { + let result = batch_processor.replenish_program_cache( + &mock_bank, + &account_maps, + limit_to_load_programs, + ); + assert!(!result.hit_max_limit); + let program = result.find(&key).unwrap(); + assert!(matches!( + program.program, + LoadedProgramType::FailedVerification(_) + )); + } } #[test] From de9e999335262947699d608ad218074200ddb879 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Fri, 5 Apr 2024 10:38:11 -0500 Subject: [PATCH 03/12] group remove_dead_accounts by slot (#578) --- accounts-db/src/accounts_db.rs | 50 ++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 713b60f8b6794d..c9fa66551df91d 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -7874,38 +7874,46 @@ impl AccountsDb { .entry(*slot) .or_default() .insert(account_info.offset()); - if let Some(expected_slot) = expected_slot { - assert_eq!(*slot, expected_slot); - } + } + if let Some(expected_slot) = expected_slot { + assert_eq!(reclaimed_offsets.len(), 1); + assert!(reclaimed_offsets.contains_key(&expected_slot)); + } + + reclaimed_offsets.iter().for_each(|(slot, offsets)| { + let mut check_for_shrink = true; if let Some(store) = self .storage - .get_account_storage_entry(*slot, account_info.store_id()) + .get_slot_storage_entry(*slot) { assert_eq!( *slot, store.slot(), "AccountsDB::accounts_index corrupted. Storage pointed to: {}, expected: {}, should only point to one slot", store.slot(), *slot ); - let offset = account_info.offset(); - let account = store.accounts.get_account(offset).unwrap(); - let stored_size = account.0.stored_size(); - let count = store.remove_account(stored_size, reset_accounts); - if count == 0 { - self.dirty_stores.insert(*slot, store.clone()); - dead_slots.insert(*slot); - } else if Self::is_shrinking_productive(*slot, &store) - && self.is_candidate_for_shrink(&store, false) - { - // Checking that this single storage entry is ready for shrinking, - // should be a sufficient indication that the slot is ready to be shrunk - // because slots should only have one storage entry, namely the one that was - // created by `flush_slot_cache()`. + let mut offsets = offsets.iter().cloned().collect::>(); + // sort so offsets are in order. This improves efficiency of loading the accounts. + offsets.sort_unstable(); + offsets.iter().for_each(|offset| { + let account = store.accounts.get_account(*offset).unwrap(); + let stored_size = account.0.stored_size(); + let count = store.remove_account(stored_size, reset_accounts); + if count == 0 { + self.dirty_stores.insert(*slot, store.clone()); + dead_slots.insert(*slot); + } else if check_for_shrink && Self::is_shrinking_productive(*slot, &store) + && self.is_candidate_for_shrink(&store, false) { - new_shrink_candidates.insert(*slot); + // Checking that this single storage entry is ready for shrinking, + // should be a sufficient indication that the slot is ready to be shrunk + // because slots should only have one storage entry, namely the one that was + // created by `flush_slot_cache()`. + new_shrink_candidates.insert(*slot); + check_for_shrink = false; } - } + }); } - } + }); measure.stop(); self.clean_accounts_stats .remove_dead_accounts_remove_us From 9253c465d1fab1aac32f22ba6ab46e9174004f63 Mon Sep 17 00:00:00 2001 From: Tyera Date: Fri, 5 Apr 2024 10:13:50 -0600 Subject: [PATCH 04/12] Persist EpochRewards sysvar (#572) * Persist EpochRewards sysvar between reward intervals * Adjust initial EpochRewards balance to ensure it is not debited out of existence * Set EpochRewards::active = false at end of distribution * Fix tests * Extend test to 2 epochs, assert sysvar still exists * Stop adjusting EpochRewards balance based on rewards * Fix tests * Review suggestions --- program-test/tests/sysvar.rs | 7 +- .../partitioned_epoch_rewards/distribution.rs | 21 ++-- .../src/bank/partitioned_epoch_rewards/mod.rs | 109 ++++++++++++++---- .../bank/partitioned_epoch_rewards/sysvar.rs | 70 +++++------ 4 files changed, 137 insertions(+), 70 deletions(-) diff --git a/program-test/tests/sysvar.rs b/program-test/tests/sysvar.rs index 95db7f1a17113d..0bb3292def88da 100644 --- a/program-test/tests/sysvar.rs +++ b/program-test/tests/sysvar.rs @@ -65,10 +65,11 @@ fn epoch_reward_sysvar_getter_process_instruction( // input[0] == 1 indicates the bank is in reward period. if input[0] == 0 { // epoch rewards sysvar should not exist for banks that are not in reward period - let epoch_rewards = EpochRewards::get(); - assert!(epoch_rewards.is_err()); + let epoch_rewards = EpochRewards::get()?; + assert!(!epoch_rewards.active); } else { - let _epoch_rewards = EpochRewards::get()?; + let epoch_rewards = EpochRewards::get()?; + assert!(epoch_rewards.active); } Ok(()) diff --git a/runtime/src/bank/partitioned_epoch_rewards/distribution.rs b/runtime/src/bank/partitioned_epoch_rewards/distribution.rs index 6975119a8bb409..22297a8abf431b 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/distribution.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/distribution.rs @@ -45,7 +45,7 @@ impl Bank { EpochRewardStatus::Active(_) )); self.epoch_reward_status = EpochRewardStatus::Inactive; - self.destroy_epoch_rewards_sysvar(); + self.set_epoch_rewards_sysvar_to_inactive(); } } @@ -211,7 +211,10 @@ mod tests { let total_rewards = 1_000_000_000; bank.create_epoch_rewards_sysvar(total_rewards, 0, 42); let pre_epoch_rewards_account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap(); - assert_eq!(pre_epoch_rewards_account.lamports(), total_rewards); + let expected_balance = + bank.get_minimum_balance_for_rent_exemption(pre_epoch_rewards_account.data().len()); + // Expected balance is the sysvar rent-exempt balance + assert_eq!(pre_epoch_rewards_account.lamports(), expected_balance); // Set up a partition of rewards to distribute let expected_num = 100; @@ -230,22 +233,18 @@ mod tests { bank.distribute_epoch_rewards_in_partition(&all_rewards, 0); let post_cap = bank.capitalization(); let post_epoch_rewards_account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap(); - let expected_epoch_rewards_sysvar_lamports_remaining = - total_rewards - rewards_to_distribute; - // Assert that epoch rewards sysvar lamports decreases by the distributed rewards - assert_eq!( - post_epoch_rewards_account.lamports(), - expected_epoch_rewards_sysvar_lamports_remaining - ); + // Assert that epoch rewards sysvar lamports balance does not change + assert_eq!(post_epoch_rewards_account.lamports(), expected_balance); let epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(&post_epoch_rewards_account).unwrap(); assert_eq!(epoch_rewards.total_rewards, total_rewards); assert_eq!(epoch_rewards.distributed_rewards, rewards_to_distribute,); - // Assert that the bank total capital didn't change - assert_eq!(pre_cap, post_cap); + // Assert that the bank total capital changed by the amount of rewards + // distributed + assert_eq!(pre_cap + rewards_to_distribute, post_cap); } /// Test partitioned credits and reward history updates of epoch rewards do cover all the rewards diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 3af20f0ddc190c..312de4e987e611 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -379,15 +379,19 @@ mod tests { .map(|_| ValidatorVoteKeypairs::new_rand()) .collect::>(); - let GenesisConfigInfo { genesis_config, .. } = create_genesis_config_with_vote_accounts( + let GenesisConfigInfo { + mut genesis_config, .. + } = create_genesis_config_with_vote_accounts( 1_000_000_000, &validator_keypairs, vec![2_000_000_000; expected_num_delegations], ); + let slots_per_epoch = 32; + genesis_config.epoch_schedule = EpochSchedule::new(slots_per_epoch); let bank0 = Bank::new_for_tests(&genesis_config); let num_slots_in_epoch = bank0.get_slots_in_epoch(bank0.epoch()); - assert_eq!(num_slots_in_epoch, 32); + assert_eq!(num_slots_in_epoch, slots_per_epoch); let mut previous_bank = Arc::new(Bank::new_from_parent( Arc::new(bank0), @@ -396,7 +400,7 @@ mod tests { )); // simulate block progress - for slot in 2..=num_slots_in_epoch + 2 { + for slot in 2..=(2 * slots_per_epoch) + 2 { let pre_cap = previous_bank.capitalization(); let curr_bank = Bank::new_from_parent(previous_bank, &Pubkey::default(), slot); let post_cap = curr_bank.capitalization(); @@ -425,7 +429,7 @@ mod tests { curr_bank.store_account_and_update_capitalization(&vote_id, &vote_account); } - if slot == num_slots_in_epoch { + if slot % num_slots_in_epoch == 0 { // This is the first block of epoch 1. Reward computation should happen in this block. // assert reward compute status activated at epoch boundary assert_matches!( @@ -433,24 +437,49 @@ mod tests { RewardInterval::InsideInterval ); - // cap should increase because of new epoch rewards - assert!(post_cap > pre_cap); - } else if slot == num_slots_in_epoch + 1 || slot == num_slots_in_epoch + 2 { - // 1. when curr_slot == num_slots_in_epoch + 1, the 2nd block of epoch 1, reward distribution should happen in this block. - // however, all stake rewards are paid at the this block therefore reward_status should have transitioned to inactive. And since - // rewards are transferred from epoch_rewards sysvar to stake accounts. The cap should stay the same. - // 2. when curr_slot == num_slots_in_epoch+2, the 3rd block of epoch 1. reward distribution should have already completed. Therefore, - // reward_status should stay inactive and cap should stay the same. + if slot == num_slots_in_epoch { + // cap should increase because of new epoch rewards + assert!(post_cap > pre_cap); + } else { + assert_eq!(post_cap, pre_cap); + } + } else if slot == num_slots_in_epoch + 1 { + // 1. when curr_slot == num_slots_in_epoch + 1, the 2nd block of + // epoch 1, reward distribution should happen in this block. + // however, all stake rewards are paid at this block therefore + // reward_status should have transitioned to inactive. The cap + // should increase accordingly. assert_matches!( curr_bank.get_reward_interval(), RewardInterval::OutsideInterval ); - assert_eq!(post_cap, pre_cap); + let account = curr_bank + .get_account(&solana_sdk::sysvar::epoch_rewards::id()) + .unwrap(); + let epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards = + solana_sdk::account::from_account(&account).unwrap(); + assert_eq!(post_cap, pre_cap + epoch_rewards.distributed_rewards); } else { + // 2. when curr_slot == num_slots_in_epoch+2, the 3rd block of + // epoch 1 (or any other slot). reward distribution should have + // already completed. Therefore, reward_status should stay + // inactive and cap should stay the same. + assert_matches!( + curr_bank.get_reward_interval(), + RewardInterval::OutsideInterval + ); + // slot is not in rewards, cap should not change assert_eq!(post_cap, pre_cap); } + // EpochRewards sysvar is created in the first block of epoch 1. + // Ensure the sysvar persists thereafter. + if slot >= num_slots_in_epoch { + let epoch_rewards_lamports = + curr_bank.get_balance(&solana_sdk::sysvar::epoch_rewards::id()); + assert!(epoch_rewards_lamports > 0); + } previous_bank = Arc::new(curr_bank); } } @@ -513,6 +542,14 @@ mod tests { // simulate block progress for slot in 2..=num_slots_in_epoch + 3 { let pre_cap = previous_bank.capitalization(); + + let pre_sysvar_account = previous_bank + .get_account(&solana_sdk::sysvar::epoch_rewards::id()) + .unwrap_or_default(); + let pre_epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards = + solana_sdk::account::from_account(&pre_sysvar_account).unwrap_or_default(); + let pre_distributed_rewards = pre_epoch_rewards.distributed_rewards; + let curr_bank = Bank::new_from_parent(previous_bank, &Pubkey::default(), slot); let post_cap = curr_bank.capitalization(); @@ -551,27 +588,53 @@ mod tests { // cap should increase because of new epoch rewards assert!(post_cap > pre_cap); } else if slot == num_slots_in_epoch + 1 { - // When curr_slot == num_slots_in_epoch + 1, the 2nd block of epoch 1, reward distribution should happen in this block. - // however, since rewards are transferred from epoch_rewards sysvar to stake accounts. The cap should stay the same. + // When curr_slot == num_slots_in_epoch + 1, the 2nd block of + // epoch 1, reward distribution should happen in this block. The + // cap should increase accordingly. assert_matches!( curr_bank.get_reward_interval(), RewardInterval::InsideInterval ); - assert_eq!(post_cap, pre_cap); - } else if slot == num_slots_in_epoch + 2 || slot == num_slots_in_epoch + 3 { - // 1. when curr_slot == num_slots_in_epoch + 2, the 3nd block of epoch 1, reward distribution should happen in this block. - // however, all stake rewards are paid at the this block therefore reward_status should have transitioned to inactive. And since - // rewards are transferred from epoch_rewards sysvar to stake accounts. The cap should stay the same. - // 2. when curr_slot == num_slots_in_epoch+2, the 3rd block of epoch 1. reward distribution should have already completed. Therefore, - // reward_status should stay inactive and cap should stay the same. + let account = curr_bank + .get_account(&solana_sdk::sysvar::epoch_rewards::id()) + .unwrap(); + let epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards = + solana_sdk::account::from_account(&account).unwrap(); + assert_eq!( + post_cap, + pre_cap + epoch_rewards.distributed_rewards - pre_distributed_rewards + ); + } else if slot == num_slots_in_epoch + 2 { + // When curr_slot == num_slots_in_epoch + 2, the 3nd block of + // epoch 1, reward distribution should happen in this block. + // however, all stake rewards are paid at the this block + // therefore reward_status should have transitioned to inactive. + // The cap should increase accordingly. assert_matches!( curr_bank.get_reward_interval(), RewardInterval::OutsideInterval ); - assert_eq!(post_cap, pre_cap); + let account = curr_bank + .get_account(&solana_sdk::sysvar::epoch_rewards::id()) + .unwrap(); + let epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards = + solana_sdk::account::from_account(&account).unwrap(); + assert_eq!( + post_cap, + pre_cap + epoch_rewards.distributed_rewards - pre_distributed_rewards + ); } else { + // When curr_slot == num_slots_in_epoch + 3, the 4th block of + // epoch 1 (or any other slot). reward distribution should have + // already completed. Therefore, reward_status should stay + // inactive and cap should stay the same. + assert_matches!( + curr_bank.get_reward_interval(), + RewardInterval::OutsideInterval + ); + // slot is not in rewards, cap should not change assert_eq!(post_cap, pre_cap); } diff --git a/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs b/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs index 23eb5c986c3512..36e69a51468732 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs @@ -33,6 +33,8 @@ impl Bank { ) { assert!(self.is_partitioned_rewards_code_enabled()); + assert!(total_rewards >= distributed_rewards); + let epoch_rewards = sysvar::epoch_rewards::EpochRewards { total_rewards, distributed_rewards, @@ -42,13 +44,10 @@ impl Bank { }; self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| { - let mut inherited_account_fields = - self.inherit_specially_retained_account_fields(account); - - assert!(total_rewards >= distributed_rewards); - // set the account lamports to the undistributed rewards - inherited_account_fields.0 = total_rewards - distributed_rewards; - create_account(&epoch_rewards, inherited_account_fields) + create_account( + &epoch_rewards, + self.inherit_specially_retained_account_fields(account), + ) }); self.log_epoch_rewards_sysvar("create"); @@ -66,30 +65,37 @@ impl Bank { epoch_rewards.distribute(distributed); self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| { - let mut inherited_account_fields = - self.inherit_specially_retained_account_fields(account); - - let lamports = inherited_account_fields.0; - assert!(lamports >= distributed); - inherited_account_fields.0 = lamports - distributed; - create_account(&epoch_rewards, inherited_account_fields) + create_account( + &epoch_rewards, + self.inherit_specially_retained_account_fields(account), + ) }); self.log_epoch_rewards_sysvar("update"); } - pub(in crate::bank::partitioned_epoch_rewards) fn destroy_epoch_rewards_sysvar(&self) { - if let Some(account) = self.get_account(&sysvar::epoch_rewards::id()) { - if account.lamports() > 0 { - info!( - "burning {} extra lamports in EpochRewards sysvar account at slot {}", - account.lamports(), - self.slot() - ); - self.log_epoch_rewards_sysvar("burn"); - self.burn_and_purge_account(&sysvar::epoch_rewards::id(), account); - } - } + /// Update EpochRewards sysvar with distributed rewards + pub(in crate::bank::partitioned_epoch_rewards) fn set_epoch_rewards_sysvar_to_inactive(&self) { + let mut epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account( + &self + .get_account(&sysvar::epoch_rewards::id()) + .unwrap_or_default(), + ) + .unwrap_or_default(); + assert_eq!( + epoch_rewards.distributed_rewards, + epoch_rewards.total_rewards + ); + epoch_rewards.active = false; + + self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| { + create_account( + &epoch_rewards, + self.inherit_specially_retained_account_fields(account), + ) + }); + + self.log_epoch_rewards_sysvar("set_inactive"); } } @@ -129,14 +135,17 @@ mod tests { bank.create_epoch_rewards_sysvar(total_rewards, 10, 42); let account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap(); - assert_eq!(account.lamports(), total_rewards - 10); + let expected_balance = bank.get_minimum_balance_for_rent_exemption(account.data().len()); + // Expected balance is the sysvar rent-exempt balance + assert_eq!(account.lamports(), expected_balance); let epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(&account).unwrap(); assert_eq!(epoch_rewards, expected_epoch_rewards); // make a distribution from epoch rewards sysvar bank.update_epoch_rewards_sysvar(10); let account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap(); - assert_eq!(account.lamports(), total_rewards - 20); + // Balance should not change + assert_eq!(account.lamports(), expected_balance); let epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(&account).unwrap(); let expected_epoch_rewards = sysvar::epoch_rewards::EpochRewards { distribution_starting_block_height: 42, @@ -148,10 +157,5 @@ mod tests { active: true, }; assert_eq!(epoch_rewards, expected_epoch_rewards); - - // burn epoch rewards sysvar - bank.burn_and_purge_account(&sysvar::epoch_rewards::id(), account); - let account = bank.get_account(&sysvar::epoch_rewards::id()); - assert!(account.is_none()); } } From e09541a7620d7c2dcbcdfea739f843deb6212acb Mon Sep 17 00:00:00 2001 From: Brooks Date: Fri, 5 Apr 2024 13:36:36 -0400 Subject: [PATCH 05/12] Adds metrics for how long it takes to evict from the accounts read cache (#585) --- accounts-db/src/accounts_db.rs | 6 +++++ accounts-db/src/read_only_accounts_cache.rs | 26 +++++++++++++-------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index c9fa66551df91d..2d6ddc1f593565 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8229,6 +8229,7 @@ impl AccountsDb { read_only_cache_misses, read_only_cache_evicts, read_only_cache_load_us, + read_only_cache_evict_us, ) = self.read_only_accounts_cache.get_and_reset_stats(); datapoint_info!( "accounts_db_store_timings", @@ -8300,6 +8301,11 @@ impl AccountsDb { read_only_cache_load_us, i64 ), + ( + "read_only_accounts_cache_evict_us", + read_only_cache_evict_us, + i64 + ), ( "calc_stored_meta_us", self.stats.calc_stored_meta.swap(0, Ordering::Relaxed), diff --git a/accounts-db/src/read_only_accounts_cache.rs b/accounts-db/src/read_only_accounts_cache.rs index 89cd5928f16786..d3f105e7b9145c 100644 --- a/accounts-db/src/read_only_accounts_cache.rs +++ b/accounts-db/src/read_only_accounts_cache.rs @@ -36,6 +36,7 @@ struct ReadOnlyCacheStats { misses: AtomicU64, evicts: AtomicU64, load_us: AtomicU64, + evict_us: AtomicU64, } impl ReadOnlyCacheStats { @@ -44,15 +45,17 @@ impl ReadOnlyCacheStats { self.misses.store(0, Ordering::Relaxed); self.evicts.store(0, Ordering::Relaxed); self.load_us.store(0, Ordering::Relaxed); + self.evict_us.store(0, Ordering::Relaxed); } - fn get_and_reset_stats(&self) -> (u64, u64, u64, u64) { + fn get_and_reset_stats(&self) -> (u64, u64, u64, u64, u64) { let hits = self.hits.swap(0, Ordering::Relaxed); let misses = self.misses.swap(0, Ordering::Relaxed); let evicts = self.evicts.swap(0, Ordering::Relaxed); let load_us = self.load_us.swap(0, Ordering::Relaxed); + let evict_us = self.evict_us.swap(0, Ordering::Relaxed); - (hits, misses, evicts, load_us) + (hits, misses, evicts, load_us, evict_us) } } @@ -162,14 +165,17 @@ impl ReadOnlyAccountsCache { }; // Evict entries from the front of the queue. let mut num_evicts = 0; - while self.data_size.load(Ordering::Relaxed) > self.max_data_size { - let Some(&(pubkey, slot)) = self.queue.lock().unwrap().get_first() else { - break; - }; - num_evicts += 1; - self.remove(pubkey, slot); - } + let (_, evict_us) = measure_us!({ + while self.data_size.load(Ordering::Relaxed) > self.max_data_size { + let Some(&(pubkey, slot)) = self.queue.lock().unwrap().get_first() else { + break; + }; + num_evicts += 1; + self.remove(pubkey, slot); + } + }); self.stats.evicts.fetch_add(num_evicts, Ordering::Relaxed); + self.stats.evict_us.fetch_add(evict_us, Ordering::Relaxed); } /// true if any pubkeys could have ever been stored into the cache at `slot` @@ -208,7 +214,7 @@ impl ReadOnlyAccountsCache { self.data_size.load(Ordering::Relaxed) } - pub(crate) fn get_and_reset_stats(&self) -> (u64, u64, u64, u64) { + pub(crate) fn get_and_reset_stats(&self) -> (u64, u64, u64, u64, u64) { self.stats.get_and_reset_stats() } } From 948caebfa90c204e0c36c91cd4ff7566ce37b5d3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 5 Apr 2024 19:06:52 +0000 Subject: [PATCH 06/12] build(deps): bump chrono from 0.4.34 to 0.4.37 (#603) * build(deps): bump chrono from 0.4.34 to 0.4.37 Bumps [chrono](https://github.com/chronotope/chrono) from 0.4.34 to 0.4.37. - [Release notes](https://github.com/chronotope/chrono/releases) - [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md) - [Commits](https://github.com/chronotope/chrono/compare/v0.4.34...v0.4.37) --- updated-dependencies: - dependency-name: chrono dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * [auto-commit] Update all Cargo lock files * update deprecated functions * add test_unix_timestamp_to_string --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite Co-authored-by: yihau --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- cli-output/src/display.rs | 13 ++++++++----- programs/sbf/Cargo.lock | 4 ++-- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ba69acd0787f5..7e0d167478637f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1277,9 +1277,9 @@ checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" [[package]] name = "chrono" -version = "0.4.34" +version = "0.4.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bc015644b92d5890fab7489e49d21f879d5c990186827d42ec511919404f38b" +checksum = "8a0d04d43504c61aa6c7531f1871dd0d418d91130162063b789da00fd7057a5e" dependencies = [ "android-tzdata", "iana-time-zone", diff --git a/Cargo.toml b/Cargo.toml index 0f85ef090cfbc1..581ee2efeec769 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -173,7 +173,7 @@ bzip2 = "0.4.4" caps = "0.5.5" cargo_metadata = "0.15.4" cc = "1.0.83" -chrono = { version = "0.4.34", default-features = false } +chrono = { version = "0.4.37", default-features = false } chrono-humanize = "0.2.3" clap = "2.33.1" console = "0.15.8" diff --git a/cli-output/src/display.rs b/cli-output/src/display.rs index c3465e493b7cf1..e68d51b480dbe5 100644 --- a/cli-output/src/display.rs +++ b/cli-output/src/display.rs @@ -1,7 +1,7 @@ use { crate::cli_output::CliSignatureVerificationStatus, base64::{prelude::BASE64_STANDARD, Engine}, - chrono::{Local, NaiveDateTime, SecondsFormat, TimeZone, Utc}, + chrono::{DateTime, Local, SecondsFormat, TimeZone, Utc}, console::style, indicatif::{ProgressBar, ProgressStyle}, solana_cli_config::SettingType, @@ -715,10 +715,8 @@ pub fn new_spinner_progress_bar() -> ProgressBar { } pub fn unix_timestamp_to_string(unix_timestamp: UnixTimestamp) -> String { - match NaiveDateTime::from_timestamp_opt(unix_timestamp, 0) { - Some(ndt) => Utc - .from_utc_datetime(&ndt) - .to_rfc3339_opts(SecondsFormat::Secs, true), + match DateTime::from_timestamp(unix_timestamp, 0) { + Some(ndt) => ndt.to_rfc3339_opts(SecondsFormat::Secs, true), None => format!("UnixTimestamp {unix_timestamp}"), } } @@ -976,4 +974,9 @@ Rewards: "abcdefghijklmnopqrstuvwxyz12345 (1111..1111)" ); } + + #[test] + fn test_unix_timestamp_to_string() { + assert_eq!(unix_timestamp_to_string(1628633791), "2021-08-10T22:16:31Z"); + } } diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index f3279c33612893..417e8a388ecea6 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -1015,9 +1015,9 @@ checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" [[package]] name = "chrono" -version = "0.4.34" +version = "0.4.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bc015644b92d5890fab7489e49d21f879d5c990186827d42ec511919404f38b" +checksum = "8a0d04d43504c61aa6c7531f1871dd0d418d91130162063b789da00fd7057a5e" dependencies = [ "android-tzdata", "iana-time-zone", From ca24f6a44da93788e090f9b3b3af27e92a80cb0f Mon Sep 17 00:00:00 2001 From: Brooks Date: Fri, 5 Apr 2024 16:25:31 -0400 Subject: [PATCH 07/12] Adds metrics for how long it takes to store into the accounts read cache (#610) --- accounts-db/src/accounts_db.rs | 6 ++++++ accounts-db/src/read_only_accounts_cache.rs | 14 ++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 2d6ddc1f593565..7b2d8d6a40d7c0 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8229,6 +8229,7 @@ impl AccountsDb { read_only_cache_misses, read_only_cache_evicts, read_only_cache_load_us, + read_only_cache_store_us, read_only_cache_evict_us, ) = self.read_only_accounts_cache.get_and_reset_stats(); datapoint_info!( @@ -8301,6 +8302,11 @@ impl AccountsDb { read_only_cache_load_us, i64 ), + ( + "read_only_accounts_cache_store_us", + read_only_cache_store_us, + i64 + ), ( "read_only_accounts_cache_evict_us", read_only_cache_evict_us, diff --git a/accounts-db/src/read_only_accounts_cache.rs b/accounts-db/src/read_only_accounts_cache.rs index d3f105e7b9145c..5d10608e266ad9 100644 --- a/accounts-db/src/read_only_accounts_cache.rs +++ b/accounts-db/src/read_only_accounts_cache.rs @@ -3,7 +3,7 @@ use { dashmap::{mapref::entry::Entry, DashMap}, index_list::{Index, IndexList}, - solana_measure::measure_us, + solana_measure::{measure::Measure, measure_us}, solana_sdk::{ account::{AccountSharedData, ReadableAccount}, clock::Slot, @@ -36,6 +36,7 @@ struct ReadOnlyCacheStats { misses: AtomicU64, evicts: AtomicU64, load_us: AtomicU64, + store_us: AtomicU64, evict_us: AtomicU64, } @@ -45,17 +46,19 @@ impl ReadOnlyCacheStats { self.misses.store(0, Ordering::Relaxed); self.evicts.store(0, Ordering::Relaxed); self.load_us.store(0, Ordering::Relaxed); + self.store_us.store(0, Ordering::Relaxed); self.evict_us.store(0, Ordering::Relaxed); } - fn get_and_reset_stats(&self) -> (u64, u64, u64, u64, u64) { + fn get_and_reset_stats(&self) -> (u64, u64, u64, u64, u64, u64) { let hits = self.hits.swap(0, Ordering::Relaxed); let misses = self.misses.swap(0, Ordering::Relaxed); let evicts = self.evicts.swap(0, Ordering::Relaxed); let load_us = self.load_us.swap(0, Ordering::Relaxed); + let store_us = self.store_us.swap(0, Ordering::Relaxed); let evict_us = self.evict_us.swap(0, Ordering::Relaxed); - (hits, misses, evicts, load_us, evict_us) + (hits, misses, evicts, load_us, store_us, evict_us) } } @@ -139,6 +142,7 @@ impl ReadOnlyAccountsCache { } pub(crate) fn store(&self, pubkey: Pubkey, slot: Slot, account: AccountSharedData) { + let measure_store = Measure::start(""); self.highest_slot_stored.fetch_max(slot, Ordering::Release); let key = (pubkey, slot); let account_size = self.account_size(&account); @@ -174,8 +178,10 @@ impl ReadOnlyAccountsCache { self.remove(pubkey, slot); } }); + let store_us = measure_store.end_as_us(); self.stats.evicts.fetch_add(num_evicts, Ordering::Relaxed); self.stats.evict_us.fetch_add(evict_us, Ordering::Relaxed); + self.stats.store_us.fetch_add(store_us, Ordering::Relaxed); } /// true if any pubkeys could have ever been stored into the cache at `slot` @@ -214,7 +220,7 @@ impl ReadOnlyAccountsCache { self.data_size.load(Ordering::Relaxed) } - pub(crate) fn get_and_reset_stats(&self) -> (u64, u64, u64, u64, u64) { + pub(crate) fn get_and_reset_stats(&self) -> (u64, u64, u64, u64, u64, u64) { self.stats.get_and_reset_stats() } } From b443cfb0c7b223e8249219db32be5ed8fa83c629 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Fri, 5 Apr 2024 13:49:23 -0700 Subject: [PATCH 08/12] Show staked vs nonstaked packets sent down/throttled (#600) * Show staked vs nonstaked packets sent down * add metrics on throttled staked vs non-staked --- streamer/src/nonblocking/quic.rs | 25 +++++++++++++++++++++++++ streamer/src/quic.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index feb9bd2db65a3e..ccd6f8a7b46fd4 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -795,6 +795,18 @@ async fn handle_connection( >= max_streams_per_throttling_interval { stats.throttled_streams.fetch_add(1, Ordering::Relaxed); + match params.peer_type { + ConnectionPeerType::Unstaked => { + stats + .throttled_unstaked_streams + .fetch_add(1, Ordering::Relaxed); + } + ConnectionPeerType::Staked(_) => { + stats + .throttled_staked_streams + .fetch_add(1, Ordering::Relaxed); + } + } let _ = stream.stop(VarInt::from_u32(STREAM_STOP_CODE_THROTTLING)); continue; } @@ -963,6 +975,19 @@ async fn handle_chunk( .total_chunks_sent_for_batching .fetch_add(chunks_sent, Ordering::Relaxed); + match peer_type { + ConnectionPeerType::Unstaked => { + stats + .total_unstaked_packets_sent_for_batching + .fetch_add(1, Ordering::Relaxed); + } + ConnectionPeerType::Staked(_) => { + stats + .total_staked_packets_sent_for_batching + .fetch_add(1, Ordering::Relaxed); + } + } + trace!("sent {} byte packet for batching", bytes_sent); } } else { diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index 3b1b6b21adf468..9b68ab1eea01ef 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -177,6 +177,10 @@ pub struct StreamStats { pub(crate) stream_load_capacity_overflow: AtomicUsize, pub(crate) process_sampled_packets_us_hist: Mutex, pub(crate) perf_track_overhead_us: AtomicU64, + pub(crate) total_staked_packets_sent_for_batching: AtomicUsize, + pub(crate) total_unstaked_packets_sent_for_batching: AtomicUsize, + pub(crate) throttled_staked_streams: AtomicUsize, + pub(crate) throttled_unstaked_streams: AtomicUsize, } impl StreamStats { @@ -338,6 +342,18 @@ impl StreamStats { .swap(0, Ordering::Relaxed), i64 ), + ( + "staked_packets_sent_for_batching", + self.total_staked_packets_sent_for_batching + .swap(0, Ordering::Relaxed), + i64 + ), + ( + "unstaked_packets_sent_for_batching", + self.total_unstaked_packets_sent_for_batching + .swap(0, Ordering::Relaxed), + i64 + ), ( "bytes_sent_for_batching", self.total_bytes_sent_for_batching @@ -434,6 +450,16 @@ impl StreamStats { self.stream_load_capacity_overflow.load(Ordering::Relaxed), i64 ), + ( + "throttled_unstaked_streams", + self.throttled_unstaked_streams.swap(0, Ordering::Relaxed), + i64 + ), + ( + "throttled_staked_streams", + self.throttled_staked_streams.swap(0, Ordering::Relaxed), + i64 + ), ( "process_sampled_packets_us_90pct", process_sampled_packets_us_hist From de8e9e6850644074ad977f5ef10f6fb7194c5063 Mon Sep 17 00:00:00 2001 From: carllin Date: Fri, 5 Apr 2024 20:38:13 -0400 Subject: [PATCH 09/12] Add all validators as entrypoint to local cluster (#567) --- local-cluster/src/cluster.rs | 4 +-- local-cluster/src/local_cluster.rs | 46 +++++++++++++++++++----------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/local-cluster/src/cluster.rs b/local-cluster/src/cluster.rs index 5fab8df55205df..99f31bf93504f7 100644 --- a/local-cluster/src/cluster.rs +++ b/local-cluster/src/cluster.rs @@ -59,10 +59,10 @@ pub trait Cluster { &mut self, pubkey: &Pubkey, cluster_validator_info: &mut ClusterValidatorInfo, - ) -> (Node, Option); + ) -> (Node, Vec); fn restart_node_with_context( cluster_validator_info: ClusterValidatorInfo, - restart_context: (Node, Option), + restart_context: (Node, Vec), socket_addr_space: SocketAddrSpace, ) -> ClusterValidatorInfo; fn add_node(&mut self, pubkey: &Pubkey, cluster_validator_info: ClusterValidatorInfo); diff --git a/local-cluster/src/local_cluster.rs b/local-cluster/src/local_cluster.rs index 3d8df638fbbb81..fe27a4a1aed21c 100644 --- a/local-cluster/src/local_cluster.rs +++ b/local-cluster/src/local_cluster.rs @@ -586,7 +586,7 @@ impl LocalCluster { let alive_node_contact_infos = self.discover_nodes(socket_addr_space, test_name); info!( "{} looking minimum root {} on all nodes", - min_root, test_name + test_name, min_root ); cluster_tests::check_min_slot_is_rooted( min_root, @@ -907,23 +907,36 @@ impl Cluster for LocalCluster { &mut self, pubkey: &Pubkey, cluster_validator_info: &mut ClusterValidatorInfo, - ) -> (Node, Option) { + ) -> (Node, Vec) { // Update the stored ContactInfo for this node let node = Node::new_localhost_with_pubkey(pubkey); cluster_validator_info.info.contact_info = node.info.clone(); cluster_validator_info.config.rpc_addrs = Some((node.info.rpc().unwrap(), node.info.rpc_pubsub().unwrap())); - let entry_point_info = { - if pubkey == self.entry_point_info.pubkey() { - self.entry_point_info = node.info.clone(); - None - } else { - Some(self.entry_point_info.clone()) - } - }; + if pubkey == self.entry_point_info.pubkey() { + self.entry_point_info = node.info.clone(); + } + + let mut is_entrypoint_alive = false; + let mut entry_point_infos: Vec = self + .validators + .values() + .map(|validator| { + // Should not be restarting a validator that is still alive + assert!(validator.info.contact_info.pubkey() != pubkey); + if validator.info.contact_info.pubkey() == self.entry_point_info.pubkey() { + is_entrypoint_alive = true; + } + validator.info.contact_info.clone() + }) + .collect(); - (node, entry_point_info) + if !is_entrypoint_alive { + entry_point_infos.push(self.entry_point_info.clone()); + } + + (node, entry_point_infos) } fn set_entry_point(&mut self, entry_point_info: ContactInfo) { @@ -951,7 +964,7 @@ impl Cluster for LocalCluster { fn restart_node_with_context( mut cluster_validator_info: ClusterValidatorInfo, - (node, entry_point_info): (Node, Option), + (node, entry_point_infos): (Node, Vec), socket_addr_space: SocketAddrSpace, ) -> ClusterValidatorInfo { // Restart the node @@ -966,11 +979,10 @@ impl Cluster for LocalCluster { &validator_info.ledger_path, &validator_info.voting_keypair.pubkey(), Arc::new(RwLock::new(vec![validator_info.voting_keypair.clone()])), - entry_point_info - .map(|entry_point_info| { - vec![LegacyContactInfo::try_from(&entry_point_info).unwrap()] - }) - .unwrap_or_default(), + entry_point_infos + .into_iter() + .map(|entry_point_info| LegacyContactInfo::try_from(&entry_point_info).unwrap()) + .collect(), &safe_clone_config(&cluster_validator_info.config), true, // should_check_duplicate_instance None, // rpc_to_plugin_manager_receiver From c207274e11c083bcb2dd9fff413f9f56e7148686 Mon Sep 17 00:00:00 2001 From: Sammy Harris <41593264+stegaBOB@users.noreply.github.com> Date: Sat, 6 Apr 2024 04:27:10 -0500 Subject: [PATCH 10/12] feat: include replaced blockhash in RPC simulation response (#380) * feat: include replaced blockhash in RPC simulation response * rename blockhash field to `replacement_blockhash` * add tests to ensure replacement_blockhash is returning correctly * fixed tests * fixed tests again for real this time? --- rpc-client-api/src/response.rs | 1 + rpc-client/src/mock_sender.rs | 1 + rpc-test/tests/rpc.rs | 47 +++++++++++++++++++++++++++++++++- rpc/src/rpc.rs | 36 +++++++++++++++++++++++--- 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/rpc-client-api/src/response.rs b/rpc-client-api/src/response.rs index fa70e89b6b88ee..f9d3085e83c2d9 100644 --- a/rpc-client-api/src/response.rs +++ b/rpc-client-api/src/response.rs @@ -424,6 +424,7 @@ pub struct RpcSimulateTransactionResult { pub units_consumed: Option, pub return_data: Option, pub inner_instructions: Option>, + pub replacement_blockhash: Option, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] diff --git a/rpc-client/src/mock_sender.rs b/rpc-client/src/mock_sender.rs index 44ab26359c3f32..ec093461c96909 100644 --- a/rpc-client/src/mock_sender.rs +++ b/rpc-client/src/mock_sender.rs @@ -351,6 +351,7 @@ impl RpcSender for MockSender { units_consumed: None, return_data: None, inner_instructions: None, + replacement_blockhash: None }, })?, "getMinimumBalanceForRentExemption" => json![20], diff --git a/rpc-test/tests/rpc.rs b/rpc-test/tests/rpc.rs index d0245608d172d1..19bf50d2e4341b 100644 --- a/rpc-test/tests/rpc.rs +++ b/rpc-test/tests/rpc.rs @@ -14,7 +14,7 @@ use { solana_rpc_client::rpc_client::RpcClient, solana_rpc_client_api::{ client_error::{ErrorKind as ClientErrorKind, Result as ClientResult}, - config::{RpcAccountInfoConfig, RpcSignatureSubscribeConfig}, + config::{RpcAccountInfoConfig, RpcSignatureSubscribeConfig, RpcSimulateTransactionConfig}, request::RpcError, response::{Response as RpcResponse, RpcSignatureResult, SlotUpdate}, }, @@ -139,6 +139,51 @@ fn test_rpc_send_tx() { info!("{:?}", json["result"]["value"]); } +#[test] +fn test_simulation_replaced_blockhash() -> ClientResult<()> { + solana_logger::setup(); + + let alice = Keypair::new(); + let validator = TestValidator::with_no_fees(alice.pubkey(), None, SocketAddrSpace::Unspecified); + let rpc_client = RpcClient::new(validator.rpc_url()); + + let bob = Keypair::new(); + let lamports = 50; + + let res = rpc_client.simulate_transaction_with_config( + &system_transaction::transfer(&alice, &bob.pubkey(), lamports, Hash::default()), + RpcSimulateTransactionConfig { + replace_recent_blockhash: true, + ..Default::default() + }, + )?; + assert!( + res.value.replacement_blockhash.is_some(), + "replaced_blockhash response is None" + ); + let blockhash = res.value.replacement_blockhash.unwrap(); + // ensure nothing weird is going on + assert_ne!( + blockhash.blockhash, + Hash::default().to_string(), + "replaced_blockhash is default" + ); + + let res = rpc_client.simulate_transaction_with_config( + &system_transaction::transfer(&alice, &bob.pubkey(), lamports, Hash::default()), + RpcSimulateTransactionConfig { + replace_recent_blockhash: false, + ..Default::default() + }, + )?; + assert!( + res.value.replacement_blockhash.is_none(), + "replaced_blockhash is Some when nothing should be replaced" + ); + + Ok(()) +} + #[test] fn test_rpc_invalid_requests() { solana_logger::setup(); diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index e6b4a28398a707..ae10b6d3865419 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3723,6 +3723,7 @@ pub mod rpc_full { units_consumed: Some(units_consumed), return_data: return_data.map(|return_data| return_data.into()), inner_instructions: None, + replacement_blockhash: None, }, } .into()); @@ -3768,15 +3769,24 @@ pub mod rpc_full { commitment, min_context_slot, })?; + let mut blockhash: Option = None; if replace_recent_blockhash { if sig_verify { return Err(Error::invalid_params( "sigVerify may not be used with replaceRecentBlockhash", )); } + let recent_blockhash = bank.last_blockhash(); unsanitized_tx .message - .set_recent_blockhash(bank.last_blockhash()); + .set_recent_blockhash(recent_blockhash); + let last_valid_block_height = bank + .get_blockhash_last_valid_block_height(&recent_blockhash) + .expect("bank blockhash queue should contain blockhash"); + blockhash.replace(RpcBlockhash { + blockhash: recent_blockhash.to_string(), + last_valid_block_height, + }); } let transaction = sanitize_transaction(unsanitized_tx, bank)?; @@ -3857,6 +3867,7 @@ pub mod rpc_full { units_consumed: Some(units_consumed), return_data: return_data.map(|return_data| return_data.into()), inner_instructions, + replacement_blockhash: blockhash, }, )) } @@ -6009,6 +6020,7 @@ pub mod tests { "Program 11111111111111111111111111111111 invoke [1]", "Program 11111111111111111111111111111111 success" ], + "replacementBlockhash": null, "returnData":null, "unitsConsumed":150, } @@ -6094,6 +6106,7 @@ pub mod tests { "Program 11111111111111111111111111111111 invoke [1]", "Program 11111111111111111111111111111111 success" ], + "replacementBlockhash": null, "returnData":null, "unitsConsumed":150, } @@ -6123,7 +6136,8 @@ pub mod tests { "Program 11111111111111111111111111111111 invoke [1]", "Program 11111111111111111111111111111111 success" ], - "returnData":null, + "replacementBlockhash": null, + "returnData": null, "unitsConsumed":150, } }, @@ -6173,7 +6187,8 @@ pub mod tests { "accounts":null, "innerInstructions":null, "logs":[], - "returnData":null, + "replacementBlockhash": null, + "returnData": null, "unitsConsumed":0, } }, @@ -6191,6 +6206,11 @@ pub mod tests { r#"{{"jsonrpc":"2.0","id":1,"method":"simulateTransaction","params":["{tx_invalid_recent_blockhash}", {{"replaceRecentBlockhash": true}}]}}"#, ); let res = io.handle_request_sync(&req, meta.clone()); + let latest_blockhash = bank.confirmed_last_blockhash(); + let expiry_slot = bank + .get_blockhash_last_valid_block_height(&latest_blockhash) + .expect("blockhash exists"); + let expected = json!({ "jsonrpc": "2.0", "result": { @@ -6203,6 +6223,10 @@ pub mod tests { "Program 11111111111111111111111111111111 invoke [1]", "Program 11111111111111111111111111111111 success" ], + "replacementBlockhash": { + "blockhash": latest_blockhash.to_string(), + "lastValidBlockHeight": expiry_slot + }, "returnData":null, "unitsConsumed":150, } @@ -6347,6 +6371,7 @@ pub mod tests { "Program 11111111111111111111111111111111 invoke [1]", "Program 11111111111111111111111111111111 success" ], + "replacementBlockhash": null, "returnData": null, "unitsConsumed": 150, } @@ -6427,6 +6452,7 @@ pub mod tests { "Program 11111111111111111111111111111111 success", "Program AddressLookupTab1e1111111111111111111111111 success" ], + "replacementBlockhash": null, "returnData":null, "unitsConsumed":1200, } @@ -6470,6 +6496,7 @@ pub mod tests { "Program 11111111111111111111111111111111 success", "Program AddressLookupTab1e1111111111111111111111111 success" ], + "replacementBlockhash": null, "returnData":null, "unitsConsumed":1200, } @@ -6556,6 +6583,7 @@ pub mod tests { "Program 11111111111111111111111111111111 success", "Program AddressLookupTab1e1111111111111111111111111 success" ], + "replacementBlockhash": null, "returnData":null, "unitsConsumed":1200, } @@ -6931,7 +6959,7 @@ pub mod tests { assert_eq!( res, Some( - r#"{"jsonrpc":"2.0","error":{"code":-32002,"message":"Transaction simulation failed: Blockhash not found","data":{"accounts":null,"err":"BlockhashNotFound","innerInstructions":null,"logs":[],"returnData":null,"unitsConsumed":0}},"id":1}"#.to_string(), + r#"{"jsonrpc":"2.0","error":{"code":-32002,"message":"Transaction simulation failed: Blockhash not found","data":{"accounts":null,"err":"BlockhashNotFound","innerInstructions":null,"logs":[],"replacementBlockhash":null,"returnData":null,"unitsConsumed":0}},"id":1}"#.to_string(), ) ); From be09b497cba00863a202cc15b5876780518cb098 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Sat, 6 Apr 2024 22:21:44 +0800 Subject: [PATCH 11/12] changelog: add entry for new simulate tx replacement blockhash (#623) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b304c367fcabc8..9250bf3885278b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ Release channels have their own copy of this changelog: * `solana-rpc-client-api`: `RpcFilterError` depends on `base64` version 0.22, so users may need to upgrade to `base64` version 0.22 * Changed default value for `--health-check-slot-distance` from 150 to 128 * CLI: Can specify `--with-compute-unit-price` and `--max-sign-attempts` during program deployment + * RPC's `simulateTransaction` now returns an extra `replacementBlockhash` field in the response + when the `replaceRecentBlockhash` config param is `true` (#380) ## [1.18.0] * Changes From 01460ef5ccaf230547301f9b65781e3be2c9df84 Mon Sep 17 00:00:00 2001 From: Yihau Chen Date: Sun, 7 Apr 2024 00:35:31 +0800 Subject: [PATCH 12/12] remove InetAddr from streamer/src/sendmmsg.rs (#557) * remove InetAddr from streamer/src/sendmmsg.rs * add ref links * use SocketAddr conversion directly --- streamer/src/sendmmsg.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/streamer/src/sendmmsg.rs b/streamer/src/sendmmsg.rs index 459d868a2ed0c8..1cb619151f6ff9 100644 --- a/streamer/src/sendmmsg.rs +++ b/streamer/src/sendmmsg.rs @@ -1,8 +1,5 @@ //! The `sendmmsg` module provides sendmmsg() API implementation -#[cfg(target_os = "linux")] -#[allow(deprecated)] -use nix::sys::socket::InetAddr; #[cfg(target_os = "linux")] use { itertools::izip, @@ -76,21 +73,26 @@ fn mmsghdr_for_packet( hdr.msg_hdr.msg_iovlen = 1; hdr.msg_hdr.msg_name = addr as *mut _ as *mut _; - #[allow(deprecated)] - match InetAddr::from_std(dest) { - InetAddr::V4(dest) => { + match dest { + SocketAddr::V4(socket_addr_v4) => { unsafe { - std::ptr::write(addr as *mut _ as *mut _, dest); + std::ptr::write( + addr as *mut _ as *mut _, + *nix::sys::socket::SockaddrIn::from(*socket_addr_v4).as_ref(), + ); } hdr.msg_hdr.msg_namelen = SIZE_OF_SOCKADDR_IN as u32; } - InetAddr::V6(dest) => { + SocketAddr::V6(socket_addr_v6) => { unsafe { - std::ptr::write(addr as *mut _ as *mut _, dest); + std::ptr::write( + addr as *mut _ as *mut _, + *nix::sys::socket::SockaddrIn6::from(*socket_addr_v6).as_ref(), + ); } hdr.msg_hdr.msg_namelen = SIZE_OF_SOCKADDR_IN6 as u32; } - }; + } } #[cfg(target_os = "linux")]