Skip to content

Commit

Permalink
Do not derive Copy for EpochSchedule and Rent (solana-labs#32767)
Browse files Browse the repository at this point in the history
  • Loading branch information
apfitzge authored Dec 1, 2023
1 parent 68928bb commit 2294801
Show file tree
Hide file tree
Showing 18 changed files with 54 additions and 42 deletions.
8 changes: 4 additions & 4 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion core/src/repair/ancestor_hashes_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1347,7 +1347,12 @@ mod test {
fn new(bank_forks: Arc<RwLock<BankForks>>) -> 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,
Expand Down
2 changes: 1 addition & 1 deletion core/src/repair/repair_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion core/src/tvu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions genesis/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ fn main() -> Result<(), Box<dyn error::Error>> {
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(
Expand All @@ -558,7 +558,7 @@ fn main() -> Result<(), Box<dyn error::Error>> {
.unwrap_or(identity_pubkey),
vote_pubkey,
&vote_account,
&rent,
&genesis_config.rent,
bootstrap_validator_stake_lamports,
),
);
Expand Down
8 changes: 4 additions & 4 deletions genesis/src/stakes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Slot> {
Expand Down
8 changes: 5 additions & 3 deletions ledger/src/leader_schedule_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions programs/bpf_loader/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![
Expand Down
2 changes: 1 addition & 1 deletion programs/vote/src/vote_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down
2 changes: 1 addition & 1 deletion rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
16 changes: 8 additions & 8 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!({
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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(|| {
Expand Down
15 changes: 7 additions & 8 deletions runtime/src/bank/fee_distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/snapshot_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion sdk/macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Self>::uninit();
unsafe {
Expand Down
2 changes: 1 addition & 1 deletion sdk/program/src/epoch_schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion sdk/program/src/rent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion test-validator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 2294801

Please sign in to comment.