From 22948019540daae4a165e00755002aaf3b73af45 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 1 Dec 2023 07:57:25 -0800 Subject: [PATCH] Do not derive Copy for EpochSchedule and Rent (#32767) --- accounts-db/src/accounts_db.rs | 8 ++++---- core/src/repair/ancestor_hashes_service.rs | 7 ++++++- core/src/repair/repair_weight.rs | 2 +- core/src/tvu.rs | 7 ++++++- genesis/src/main.rs | 4 ++-- genesis/src/stakes.rs | 8 ++++---- ledger/src/blockstore_processor.rs | 2 +- ledger/src/leader_schedule_cache.rs | 8 +++++--- programs/bpf_loader/src/syscalls/mod.rs | 4 ++-- programs/vote/src/vote_state/mod.rs | 2 +- rpc/src/rpc.rs | 2 +- runtime/src/bank.rs | 16 ++++++++-------- runtime/src/bank/fee_distribution.rs | 15 +++++++-------- runtime/src/snapshot_package.rs | 2 +- sdk/macro/src/lib.rs | 1 - sdk/program/src/epoch_schedule.rs | 2 +- sdk/program/src/rent.rs | 2 +- test-validator/src/lib.rs | 4 +++- 18 files changed, 54 insertions(+), 42 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index ea2faf37a9e7d3..4a5a2be9e1259b 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8989,17 +8989,17 @@ impl AccountsDb { slots.truncate(limit); // get rid of the newer slots and keep just the older } let max_slot = slots.last().cloned().unwrap_or_default(); - let schedule = genesis_config.epoch_schedule; + let schedule = &genesis_config.epoch_schedule; let rent_collector = RentCollector::new( schedule.get_epoch(max_slot), - schedule, + schedule.clone(), genesis_config.slots_per_year(), - genesis_config.rent, + genesis_config.rent.clone(), ); let accounts_data_len = AtomicU64::new(0); let rent_paying_accounts_by_partition = - Mutex::new(RentPayingAccountsByPartition::new(&schedule)); + Mutex::new(RentPayingAccountsByPartition::new(schedule)); // pass == 0 always runs and generates the index // pass == 1 only runs if verify == true. diff --git a/core/src/repair/ancestor_hashes_service.rs b/core/src/repair/ancestor_hashes_service.rs index 978d0c074c3904..e980ddb46b4745 100644 --- a/core/src/repair/ancestor_hashes_service.rs +++ b/core/src/repair/ancestor_hashes_service.rs @@ -1347,7 +1347,12 @@ mod test { fn new(bank_forks: Arc>) -> Self { let ancestor_hashes_request_statuses = Arc::new(DashMap::new()); let ancestor_hashes_request_socket = Arc::new(UdpSocket::bind("0.0.0.0:0").unwrap()); - let epoch_schedule = *bank_forks.read().unwrap().root_bank().epoch_schedule(); + let epoch_schedule = bank_forks + .read() + .unwrap() + .root_bank() + .epoch_schedule() + .clone(); let keypair = Keypair::new(); let requester_cluster_info = Arc::new(ClusterInfo::new( Node::new_localhost_with_pubkey(&keypair.pubkey()).info, diff --git a/core/src/repair/repair_weight.rs b/core/src/repair/repair_weight.rs index 6838021d7574c7..430a02850b30c2 100644 --- a/core/src/repair/repair_weight.rs +++ b/core/src/repair/repair_weight.rs @@ -2553,7 +2553,7 @@ mod test { let stake = 100; let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys_for_tests(10, stake); let mut epoch_stakes = bank.epoch_stakes_map().clone(); - let mut epoch_schedule = *bank.epoch_schedule(); + let mut epoch_schedule = bank.epoch_schedule().clone(); // Simulate epoch boundary at slot 10, where half of the stake deactivates // Additional epoch boundary at slot 20, where 30% of the stake reactivates diff --git a/core/src/tvu.rs b/core/src/tvu.rs index 214fae3dceac0f..2fe7e08dd60f8b 100644 --- a/core/src/tvu.rs +++ b/core/src/tvu.rs @@ -196,7 +196,12 @@ impl Tvu { let (dumped_slots_sender, dumped_slots_receiver) = unbounded(); let (popular_pruned_forks_sender, popular_pruned_forks_receiver) = unbounded(); let window_service = { - let epoch_schedule = *bank_forks.read().unwrap().working_bank().epoch_schedule(); + let epoch_schedule = bank_forks + .read() + .unwrap() + .working_bank() + .epoch_schedule() + .clone(); let repair_info = RepairInfo { bank_forks: bank_forks.clone(), epoch_schedule, diff --git a/genesis/src/main.rs b/genesis/src/main.rs index c254975379c937..6b7efd5e664339 100644 --- a/genesis/src/main.rs +++ b/genesis/src/main.rs @@ -547,7 +547,7 @@ fn main() -> Result<(), Box> { identity_pubkey, identity_pubkey, commission, - VoteState::get_rent_exempt_reserve(&rent).max(1), + VoteState::get_rent_exempt_reserve(&genesis_config.rent).max(1), ); genesis_config.add_account( @@ -558,7 +558,7 @@ fn main() -> Result<(), Box> { .unwrap_or(identity_pubkey), vote_pubkey, &vote_account, - &rent, + &genesis_config.rent, bootstrap_validator_stake_lamports, ), ); diff --git a/genesis/src/stakes.rs b/genesis/src/stakes.rs index 1d7c18f3a034a9..133fdf57f4968b 100644 --- a/genesis/src/stakes.rs +++ b/genesis/src/stakes.rs @@ -246,7 +246,7 @@ mod tests { let total_lamports = staker_reserve + reserve * 2 + 1; create_and_check_stakes( &mut GenesisConfig { - rent, + rent: rent.clone(), ..GenesisConfig::default() }, &StakerInfo { @@ -272,7 +272,7 @@ mod tests { let total_lamports = staker_reserve + reserve * 2 + 1; create_and_check_stakes( &mut GenesisConfig { - rent, + rent: rent.clone(), ..GenesisConfig::default() }, &StakerInfo { @@ -298,7 +298,7 @@ mod tests { let total_lamports = staker_reserve + (granularity + reserve) * 2; create_and_check_stakes( &mut GenesisConfig { - rent, + rent: rent.clone(), ..GenesisConfig::default() }, &StakerInfo { @@ -323,7 +323,7 @@ mod tests { let total_lamports = staker_reserve + (granularity + reserve + 1) * 2; create_and_check_stakes( &mut GenesisConfig { - rent, + rent: rent.clone(), ..GenesisConfig::default() }, &StakerInfo { diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 3e0f6d1d375cc4..2806cfa3d891b5 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3877,7 +3877,7 @@ pub mod tests { AccountSecondaryIndexes::default(), AccountShrinkThreshold::default(), ); - *bank.epoch_schedule() + bank.epoch_schedule().clone() } fn frozen_bank_slots(bank_forks: &BankForks) -> Vec { diff --git a/ledger/src/leader_schedule_cache.rs b/ledger/src/leader_schedule_cache.rs index 733ea3c359befd..a1607ab0cb3a46 100644 --- a/ledger/src/leader_schedule_cache.rs +++ b/ledger/src/leader_schedule_cache.rs @@ -40,7 +40,7 @@ pub struct LeaderScheduleCache { impl LeaderScheduleCache { pub fn new_from_bank(bank: &Bank) -> Self { - Self::new(*bank.epoch_schedule(), bank) + Self::new(bank.epoch_schedule().clone(), bank) } pub fn new(epoch_schedule: EpochSchedule, root_bank: &Bank) -> Self { @@ -56,9 +56,11 @@ impl LeaderScheduleCache { cache.set_root(root_bank); // Calculate the schedule for all epochs between 0 and leader_schedule_epoch(root) - let leader_schedule_epoch = epoch_schedule.get_leader_schedule_epoch(root_bank.slot()); + let leader_schedule_epoch = cache + .epoch_schedule + .get_leader_schedule_epoch(root_bank.slot()); for epoch in 0..leader_schedule_epoch { - let first_slot_in_epoch = epoch_schedule.get_first_slot_in_epoch(epoch); + let first_slot_in_epoch = cache.epoch_schedule.get_first_slot_in_epoch(epoch); cache.slot_leader_at(first_slot_in_epoch, Some(root_bank)); } cache diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 8ae412bc140e95..3e6562b8ed7b8a 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -3203,9 +3203,9 @@ mod tests { let mut sysvar_cache = SysvarCache::default(); sysvar_cache.set_clock(src_clock.clone()); - sysvar_cache.set_epoch_schedule(src_epochschedule); + sysvar_cache.set_epoch_schedule(src_epochschedule.clone()); sysvar_cache.set_fees(src_fees.clone()); - sysvar_cache.set_rent(src_rent); + sysvar_cache.set_rent(src_rent.clone()); sysvar_cache.set_epoch_rewards(src_rewards); let transaction_accounts = vec![ diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index cdc6780d2bc000..775bfef326ed13 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1237,7 +1237,7 @@ mod tests { let processor_account = AccountSharedData::new(0, 0, &solana_sdk::native_loader::id()); let transaction_context = TransactionContext::new( vec![(id(), processor_account), (node_pubkey, vote_account)], - rent, + rent.clone(), 0, 0, ); diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 5e62dff9ce55d3..2878f80b8fe8ca 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -631,7 +631,7 @@ impl JsonRpcRequestProcessor { // Since epoch schedule data comes from the genesis config, any commitment level should be // fine let bank = self.bank(Some(CommitmentConfig::finalized())); - *bank.epoch_schedule() + bank.epoch_schedule().clone() } pub fn get_balance( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 9f4e5387295838..44c3d6b495a7b7 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1342,7 +1342,7 @@ impl Bank { parent.freeze(); assert_ne!(slot, parent.slot()); - let epoch_schedule = parent.epoch_schedule; + let epoch_schedule = parent.epoch_schedule().clone(); let epoch = epoch_schedule.get_epoch(slot); let (rc, bank_rc_creation_time_us) = measure_us!({ @@ -1490,7 +1490,7 @@ impl Bank { ); } else { // Save a snapshot of stakes for use in consensus and stake weighted networking - let leader_schedule_epoch = epoch_schedule.get_leader_schedule_epoch(slot); + let leader_schedule_epoch = new.epoch_schedule().get_leader_schedule_epoch(slot); new.update_epoch_stakes(leader_schedule_epoch); } if new.is_partitioned_rewards_code_enabled() { @@ -2051,7 +2051,7 @@ impl Bank { fee_rate_governor: self.fee_rate_governor.clone(), collected_rent: self.collected_rent.load(Relaxed), rent_collector: self.rent_collector.clone(), - epoch_schedule: self.epoch_schedule, + epoch_schedule: self.epoch_schedule.clone(), inflation: *self.inflation.read().unwrap(), stakes: &self.stakes_cache, epoch_stakes: &self.epoch_stakes, @@ -3926,15 +3926,15 @@ impl Bank { self.max_tick_height = (self.slot + 1) * self.ticks_per_slot; self.slots_per_year = genesis_config.slots_per_year(); - self.epoch_schedule = genesis_config.epoch_schedule; + self.epoch_schedule = genesis_config.epoch_schedule.clone(); self.inflation = Arc::new(RwLock::new(genesis_config.inflation)); self.rent_collector = RentCollector::new( self.epoch, - *self.epoch_schedule(), + self.epoch_schedule().clone(), self.slots_per_year, - genesis_config.rent, + genesis_config.rent.clone(), ); // Add additional builtin programs specified in the genesis config @@ -4929,7 +4929,7 @@ impl Bank { let mut transaction_context = TransactionContext::new( transaction_accounts, - self.rent_collector.rent, + self.rent_collector.rent.clone(), compute_budget.max_invoke_stack_height, compute_budget.max_instruction_trace_length, ); @@ -7184,7 +7184,7 @@ impl Bank { if config.run_in_background { let ancestors = ancestors.clone(); let accounts = Arc::clone(accounts); - let epoch_schedule = *epoch_schedule; + let epoch_schedule = epoch_schedule.clone(); let rent_collector = rent_collector.clone(); let accounts_ = Arc::clone(&accounts); accounts.accounts_db.verify_accounts_hash_in_bg.start(|| { diff --git a/runtime/src/bank/fee_distribution.rs b/runtime/src/bank/fee_distribution.rs index 8252e14bcfb2ea..00f70eb09ac3f6 100644 --- a/runtime/src/bank/fee_distribution.rs +++ b/runtime/src/bank/fee_distribution.rs @@ -102,14 +102,14 @@ impl Bank { return Err(DepositFeeError::InvalidAccountOwner); } - let rent = self.rent_collector().rent; - let recipient_pre_rent_state = RentState::from_account(&account, &rent); + let rent = &self.rent_collector().rent; + let recipient_pre_rent_state = RentState::from_account(&account, rent); let distribution = account.checked_add_lamports(fees); if distribution.is_err() { return Err(DepositFeeError::LamportOverflow); } if options.check_rent_paying { - let recipient_post_rent_state = RentState::from_account(&account, &rent); + let recipient_post_rent_state = RentState::from_account(&account, rent); let rent_state_transition_allowed = recipient_post_rent_state.transition_allowed_from(&recipient_pre_rent_state); if !rent_state_transition_allowed { @@ -572,10 +572,9 @@ pub mod tests { let genesis = create_genesis_config(initial_balance); let pubkey = genesis.mint_keypair.pubkey(); let mut genesis_config = genesis.genesis_config; - let rent = Rent::default(); - genesis_config.rent = rent; // Ensure rent is non-zero, as genesis_utils sets Rent::free by default + genesis_config.rent = Rent::default(); // Ensure rent is non-zero, as genesis_utils sets Rent::free by default let bank = Bank::new_for_tests(&genesis_config); - let min_rent_exempt_balance = rent.minimum_balance(0); + let min_rent_exempt_balance = genesis_config.rent.minimum_balance(0); let deposit_amount = 500; assert!(initial_balance + deposit_amount < min_rent_exempt_balance); @@ -642,7 +641,7 @@ pub mod tests { .unwrap(); } let bank = Bank::new_for_tests(&genesis_config); - let rent = bank.rent_collector().rent; + let rent = &bank.rent_collector().rent; let rent_exempt_minimum = rent.minimum_balance(0); // Make one validator have an empty identity account @@ -680,7 +679,7 @@ pub mod tests { let account = bank .get_account_with_fixed_root(address) .unwrap_or_default(); - RentState::from_account(&account, &rent) + RentState::from_account(&account, rent) }; // Assert starting RentStates diff --git a/runtime/src/snapshot_package.rs b/runtime/src/snapshot_package.rs index 6685cd269daea6..8053a13d2c137e 100644 --- a/runtime/src/snapshot_package.rs +++ b/runtime/src/snapshot_package.rs @@ -148,7 +148,7 @@ impl AccountsPackage { expected_capitalization: bank.capitalization(), accounts_hash_for_testing, accounts: bank.accounts(), - epoch_schedule: *bank.epoch_schedule(), + epoch_schedule: bank.epoch_schedule().clone(), rent_collector: bank.rent_collector().clone(), is_incremental_accounts_hash_feature_enabled, snapshot_info, diff --git a/sdk/macro/src/lib.rs b/sdk/macro/src/lib.rs index f72dcdfcf8eb2f..157592dc37bcaa 100644 --- a/sdk/macro/src/lib.rs +++ b/sdk/macro/src/lib.rs @@ -430,7 +430,6 @@ pub fn derive_clone_zeroed(input: proc_macro::TokenStream) -> proc_macro::TokenS // implementations on `Copy` types are simply wrappers of `Copy`. // This is not the case here, and intentionally so because we want to // guarantee zeroed padding. - #[allow(clippy::incorrect_clone_impl_on_copy_type)] fn clone(&self) -> Self { let mut value = std::mem::MaybeUninit::::uninit(); unsafe { diff --git a/sdk/program/src/epoch_schedule.rs b/sdk/program/src/epoch_schedule.rs index 672b0f15359f6e..b9de3310b76e74 100644 --- a/sdk/program/src/epoch_schedule.rs +++ b/sdk/program/src/epoch_schedule.rs @@ -29,7 +29,7 @@ pub const MAX_LEADER_SCHEDULE_EPOCH_OFFSET: u64 = 3; pub const MINIMUM_SLOTS_PER_EPOCH: u64 = 32; #[repr(C)] -#[derive(Debug, CloneZeroed, Copy, PartialEq, Eq, Deserialize, Serialize, AbiExample)] +#[derive(Debug, CloneZeroed, PartialEq, Eq, Deserialize, Serialize, AbiExample)] #[serde(rename_all = "camelCase")] pub struct EpochSchedule { /// The maximum number of slots in each epoch. diff --git a/sdk/program/src/rent.rs b/sdk/program/src/rent.rs index 7257b9a2073ec7..f2c52a4d5a98ee 100644 --- a/sdk/program/src/rent.rs +++ b/sdk/program/src/rent.rs @@ -8,7 +8,7 @@ use {crate::clock::DEFAULT_SLOTS_PER_EPOCH, solana_sdk_macro::CloneZeroed}; /// Configuration of network rent. #[repr(C)] -#[derive(Serialize, Deserialize, PartialEq, CloneZeroed, Copy, Debug, AbiExample)] +#[derive(Serialize, Deserialize, PartialEq, CloneZeroed, Debug, AbiExample)] pub struct Rent { /// Rental rate in lamports/byte-year. pub lamports_per_byte_year: u64, diff --git a/test-validator/src/lib.rs b/test-validator/src/lib.rs index e807f80c9692f0..9f994eee9a19df 100644 --- a/test-validator/src/lib.rs +++ b/test-validator/src/lib.rs @@ -777,12 +777,14 @@ impl TestValidator { validator_stake_lamports, validator_identity_lamports, config.fee_rate_governor.clone(), - config.rent, + config.rent.clone(), solana_sdk::genesis_config::ClusterType::Development, accounts.into_iter().collect(), ); genesis_config.epoch_schedule = config .epoch_schedule + .as_ref() + .cloned() .unwrap_or_else(EpochSchedule::without_warmup); if let Some(ticks_per_slot) = config.ticks_per_slot {