Skip to content

Commit

Permalink
remove partitioned reward test code
Browse files Browse the repository at this point in the history
  • Loading branch information
HaoranYi authored and CriesofCarrots committed Dec 20, 2024
1 parent d29cd8e commit f88bda1
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 177 deletions.
16 changes: 7 additions & 9 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ use {
cache_hash_data::{CacheHashData, DeletionPolicy as CacheHashDeletionPolicy},
contains::Contains,
epoch_accounts_hash::EpochAccountsHashManager,
partitioned_rewards::{PartitionedEpochRewardsConfig, TestPartitionedEpochRewards},
partitioned_rewards::{
PartitionedEpochRewardsConfig, DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
},
read_only_accounts_cache::ReadOnlyAccountsCache,
sorted_storages::SortedStorages,
storable_accounts::{StorableAccounts, StorableAccountsBySlot},
Expand Down Expand Up @@ -506,7 +508,7 @@ pub const ACCOUNTS_DB_CONFIG_FOR_TESTING: AccountsDbConfig = AccountsDbConfig {
skip_initial_hash_calc: false,
exhaustively_verify_refcounts: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::CompareResults,
partitioned_epoch_rewards_config: DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
test_skip_rewrites_but_include_in_bank_hash: false,
storage_access: StorageAccess::Mmap,
scan_filter_for_shrinking: ScanFilter::OnlyAbnormalWithVerify,
Expand All @@ -532,7 +534,7 @@ pub const ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS: AccountsDbConfig = AccountsDbConfig
skip_initial_hash_calc: false,
exhaustively_verify_refcounts: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::None,
partitioned_epoch_rewards_config: DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
test_skip_rewrites_but_include_in_bank_hash: false,
storage_access: StorageAccess::Mmap,
scan_filter_for_shrinking: ScanFilter::OnlyAbnormalWithVerify,
Expand Down Expand Up @@ -661,7 +663,7 @@ pub struct AccountsDbConfig {
pub exhaustively_verify_refcounts: bool,
/// how to create ancient storages
pub create_ancient_storage: CreateAncientStorage,
pub test_partitioned_epoch_rewards: TestPartitionedEpochRewards,
pub partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig,
pub storage_access: StorageAccess,
pub scan_filter_for_shrinking: ScanFilter,
pub enable_experimental_accumulator_hash: bool,
Expand Down Expand Up @@ -1969,10 +1971,6 @@ impl AccountsDb {
accounts_hash_cache_path
});

let test_partitioned_epoch_rewards = accounts_db_config.test_partitioned_epoch_rewards;
let partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig =
PartitionedEpochRewardsConfig::new(test_partitioned_epoch_rewards);

let read_cache_size = accounts_db_config.read_cache_limit_bytes.unwrap_or((
Self::DEFAULT_MAX_READ_ONLY_CACHE_DATA_SIZE_LO,
Self::DEFAULT_MAX_READ_ONLY_CACHE_DATA_SIZE_HI,
Expand Down Expand Up @@ -2035,7 +2033,7 @@ impl AccountsDb {
Self::READ_ONLY_CACHE_MS_TO_SKIP_LRU_UPDATE,
),
write_cache_limit_bytes: accounts_db_config.write_cache_limit_bytes,
partitioned_epoch_rewards_config,
partitioned_epoch_rewards_config: accounts_db_config.partitioned_epoch_rewards_config,
exhaustively_verify_refcounts: accounts_db_config.exhaustively_verify_refcounts,
test_skip_rewrites_but_include_in_bank_hash: accounts_db_config
.test_skip_rewrites_but_include_in_bank_hash,
Expand Down
93 changes: 17 additions & 76 deletions accounts-db/src/partitioned_rewards.rs
Original file line number Diff line number Diff line change
@@ -1,98 +1,39 @@
//! Code related to partitioned rewards distribution
#[derive(Debug)]
/// # stake accounts to store in one block during partitioned reward interval
/// Target to store 64 rewards per entry/tick in a block. A block has a minimum of 64
/// entries/tick. This gives 4096 total rewards to store in one block.
/// This constant affects consensus.
const MAX_PARTITIONED_REWARDS_PER_BLOCK: u64 = 4096;

#[derive(Debug, Clone, Copy)]
/// Configuration options for partitioned epoch rewards.
/// This struct allows various forms of testing, especially prior to feature activation.
pub struct PartitionedEpochRewardsConfig {
/// number of stake accounts to store in one block during partitioned reward interval
/// normally, this is a number tuned for reasonable performance, such as 4096 accounts/block
/// if force_one_slot_partitioned_rewards, this will usually be u64::MAX so that all stake accounts are written in the first block
pub stake_account_stores_per_block: u64,
/// if true, end of epoch bank rewards will force using partitioned rewards distribution.
/// see `set_test_enable_partitioned_rewards`
pub test_enable_partitioned_rewards: bool,
/// if true, end of epoch non-partitioned bank rewards will test the partitioned rewards distribution vote and stake accounts
/// This has a significant performance impact on the first slot in each new epoch.
pub test_compare_partitioned_epoch_rewards: bool,
}

/// Convenient constant for default partitioned epoch rewards configuration
/// used for benchmarks and tests.
pub const DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG: PartitionedEpochRewardsConfig =
PartitionedEpochRewardsConfig {
stake_account_stores_per_block: MAX_PARTITIONED_REWARDS_PER_BLOCK,
};

impl Default for PartitionedEpochRewardsConfig {
fn default() -> Self {
Self {
// # stake accounts to store in one block during partitioned reward interval
// Target to store 64 rewards per entry/tick in a block. A block has a minimum of 64
// entries/tick. This gives 4096 total rewards to store in one block.
// This constant affects consensus.
stake_account_stores_per_block: 4096,
test_enable_partitioned_rewards: false,
test_compare_partitioned_epoch_rewards: false,
stake_account_stores_per_block: MAX_PARTITIONED_REWARDS_PER_BLOCK,
}
}
}

#[derive(Debug, Default, Clone, Copy)]
pub enum TestPartitionedEpochRewards {
#[default]
None,
CompareResults,
ForcePartitionedEpochRewardsInOneBlock,
PartitionedEpochRewardsConfigRewardBlocks {
stake_account_stores_per_block: u64,
},
}

impl PartitionedEpochRewardsConfig {
pub fn new(test: TestPartitionedEpochRewards) -> Self {
match test {
TestPartitionedEpochRewards::None => Self::default(),
TestPartitionedEpochRewards::CompareResults => {
Self::set_test_compare_partitioned_epoch_rewards()
}
TestPartitionedEpochRewards::ForcePartitionedEpochRewardsInOneBlock => {
Self::set_test_enable_partitioned_rewards()
}
TestPartitionedEpochRewards::PartitionedEpochRewardsConfigRewardBlocks {
stake_account_stores_per_block,
} => {
Self::set_test_enable_partitioned_rewards_with_custom_number_of_stake_accounts_per_block(
stake_account_stores_per_block
)
}
}
}

/// All rewards will be distributed in the first block in the epoch, matching
/// consensus for the non-partitioned rewards, but running all the partitioned rewards
/// code.
fn set_test_enable_partitioned_rewards() -> Self {
Self {
stake_account_stores_per_block: u64::MAX,
test_enable_partitioned_rewards: true,
// irrelevant if we are not running old code path
test_compare_partitioned_epoch_rewards: false,
}
}

/// All rewards will be distributed in the first block in the epoch as normal.
/// Then, the partitioned rewards code will calculate expected results and compare to
/// the old code path's results.
fn set_test_compare_partitioned_epoch_rewards() -> Self {
Self {
test_compare_partitioned_epoch_rewards: true,
..PartitionedEpochRewardsConfig::default()
}
}

/// A method that configures how many reward reward calculation blocks and how many stake
/// accounts to store per reward block.
fn set_test_enable_partitioned_rewards_with_custom_number_of_stake_accounts_per_block(
stake_account_stores_per_block: u64,
) -> Self {
/// Only for tests and benchmarks
pub fn new_for_test(stake_account_stores_per_block: u64) -> Self {
Self {
stake_account_stores_per_block,
test_enable_partitioned_rewards: true,
// irrelevant if we are not running old code path
test_compare_partitioned_epoch_rewards: false,
}
}
}
11 changes: 0 additions & 11 deletions ledger-tool/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use {
accounts_db::{AccountsDb, AccountsDbConfig, CreateAncientStorage},
accounts_file::StorageAccess,
accounts_index::{AccountsIndexConfig, IndexLimitMb, ScanFilter},
partitioned_rewards::TestPartitionedEpochRewards,
utils::create_and_canonicalize_directories,
},
solana_clap_utils::{
Expand Down Expand Up @@ -301,15 +300,6 @@ pub fn get_accounts_db_config(
..AccountsIndexConfig::default()
};

let test_partitioned_epoch_rewards =
if arg_matches.is_present("partitioned_epoch_rewards_compare_calculation") {
TestPartitionedEpochRewards::CompareResults
} else if arg_matches.is_present("partitioned_epoch_rewards_force_enable_single_slot") {
TestPartitionedEpochRewards::ForcePartitionedEpochRewardsInOneBlock
} else {
TestPartitionedEpochRewards::None
};

let accounts_hash_cache_path = arg_matches
.value_of("accounts_hash_cache_path")
.map(Into::into)
Expand Down Expand Up @@ -388,7 +378,6 @@ pub fn get_accounts_db_config(
.ok(),
exhaustively_verify_refcounts: arg_matches.is_present("accounts_db_verify_refcounts"),
skip_initial_hash_calc: arg_matches.is_present("accounts_db_skip_initial_hash_calculation"),
test_partitioned_epoch_rewards,
test_skip_rewrites_but_include_in_bank_hash: arg_matches
.is_present("accounts_db_test_skip_rewrites"),
create_ancient_storage,
Expand Down
23 changes: 0 additions & 23 deletions ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,29 +1136,6 @@ fn main() {
tasks and assert.",
),
)
.arg(
Arg::with_name("partitioned_epoch_rewards_compare_calculation")
.long("partitioned-epoch-rewards-compare-calculation")
.takes_value(false)
.help(
"Do normal epoch rewards distribution, but also calculate rewards \
using the partitioned rewards code path and compare the resulting \
vote and stake accounts",
)
.hidden(hidden_unless_forced()),
)
.arg(
Arg::with_name("partitioned_epoch_rewards_force_enable_single_slot")
.long("partitioned-epoch-rewards-force-enable-single-slot")
.takes_value(false)
.help(
"Force the partitioned rewards distribution, but distribute all \
rewards in the first slot in the epoch. This should match consensus \
with the normal rewards distribution.",
)
.conflicts_with("partitioned_epoch_rewards_compare_calculation")
.hidden(hidden_unless_forced()),
)
.arg(
Arg::with_name("print_accounts_stats")
.long("print-accounts-stats")
Expand Down
7 changes: 1 addition & 6 deletions runtime/src/bank/partitioned_epoch_rewards/calculation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,7 @@ impl Bank {

let slot = self.slot();
let distribution_starting_block_height =
// For live-cluster testing pre-activation
if self.force_partition_rewards_in_first_block_of_epoch() {
self.block_height()
} else {
self.block_height() + REWARD_CALCULATION_NUM_BLOCKS
};
self.block_height() + REWARD_CALCULATION_NUM_BLOCKS;

let num_partitions = stake_rewards_by_partition.len() as u64;

Expand Down
23 changes: 5 additions & 18 deletions runtime/src/bank/partitioned_epoch_rewards/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,6 @@ impl Bank {
pub fn force_reward_interval_end_for_tests(&mut self) {
self.epoch_reward_status = EpochRewardStatus::Inactive;
}

pub(super) fn force_partition_rewards_in_first_block_of_epoch(&self) -> bool {
self.partitioned_epoch_rewards_config()
.test_enable_partitioned_rewards
&& self.partitioned_rewards_stake_account_stores_per_block() == u64::MAX
}
}

#[cfg(test)]
Expand All @@ -244,10 +238,7 @@ mod tests {
runtime_config::RuntimeConfig,
},
assert_matches::assert_matches,
solana_accounts_db::{
accounts_db::{AccountsDbConfig, ACCOUNTS_DB_CONFIG_FOR_TESTING},
partitioned_rewards::TestPartitionedEpochRewards,
},
solana_accounts_db::accounts_db::{AccountsDbConfig, ACCOUNTS_DB_CONFIG_FOR_TESTING},
solana_sdk::{
account::Account,
account_utils::StateMut,
Expand Down Expand Up @@ -362,10 +353,8 @@ mod tests {
genesis_config.epoch_schedule = EpochSchedule::new(SLOTS_PER_EPOCH);

let mut accounts_db_config: AccountsDbConfig = ACCOUNTS_DB_CONFIG_FOR_TESTING.clone();
accounts_db_config.test_partitioned_epoch_rewards =
TestPartitionedEpochRewards::PartitionedEpochRewardsConfigRewardBlocks {
stake_account_stores_per_block,
};
accounts_db_config.partitioned_epoch_rewards_config =
PartitionedEpochRewardsConfig::new_for_test(stake_account_stores_per_block);

let bank = Bank::new_with_paths(
&genesis_config,
Expand Down Expand Up @@ -459,10 +448,8 @@ mod tests {

// Config stake reward distribution to be 10 per block
let mut accounts_db_config: AccountsDbConfig = ACCOUNTS_DB_CONFIG_FOR_TESTING.clone();
accounts_db_config.test_partitioned_epoch_rewards =
TestPartitionedEpochRewards::PartitionedEpochRewardsConfigRewardBlocks {
stake_account_stores_per_block: 10,
};
accounts_db_config.partitioned_epoch_rewards_config =
PartitionedEpochRewardsConfig::new_for_test(10);

let bank = Bank::new_with_paths(
&genesis_config,
Expand Down
23 changes: 0 additions & 23 deletions validator/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1453,29 +1453,6 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> {
.takes_value(true)
.help("Number of bins to divide the accounts index into"),
)
.arg(
Arg::with_name("partitioned_epoch_rewards_compare_calculation")
.long("partitioned-epoch-rewards-compare-calculation")
.takes_value(false)
.help(
"Do normal epoch rewards distribution, but also calculate rewards using the \
partitioned rewards code path and compare the resulting vote and stake \
accounts",
)
.hidden(hidden_unless_forced()),
)
.arg(
Arg::with_name("partitioned_epoch_rewards_force_enable_single_slot")
.long("partitioned-epoch-rewards-force-enable-single-slot")
.takes_value(false)
.help(
"Force the partitioned rewards distribution, but distribute all rewards in \
the first slot in the epoch. This should match consensus with the normal \
rewards distribution.",
)
.conflicts_with("partitioned_epoch_rewards_compare_calculation")
.hidden(hidden_unless_forced()),
)
.arg(
Arg::with_name("accounts_index_path")
.long("accounts-index-path")
Expand Down
11 changes: 0 additions & 11 deletions validator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use {
AccountIndex, AccountSecondaryIndexes, AccountSecondaryIndexesIncludeExclude,
AccountsIndexConfig, IndexLimitMb, ScanFilter,
},
partitioned_rewards::TestPartitionedEpochRewards,
utils::{
create_all_accounts_run_and_snapshot_dirs, create_and_canonicalize_directories,
create_and_canonicalize_directory,
Expand Down Expand Up @@ -1229,15 +1228,6 @@ pub fn main() {
accounts_index_config.bins = Some(bins);
}

let test_partitioned_epoch_rewards =
if matches.is_present("partitioned_epoch_rewards_compare_calculation") {
TestPartitionedEpochRewards::CompareResults
} else if matches.is_present("partitioned_epoch_rewards_force_enable_single_slot") {
TestPartitionedEpochRewards::ForcePartitionedEpochRewardsInOneBlock
} else {
TestPartitionedEpochRewards::None
};

accounts_index_config.index_limit_mb = if matches.is_present("disable_accounts_disk_index") {
IndexLimitMb::InMemOnly
} else {
Expand Down Expand Up @@ -1362,7 +1352,6 @@ pub fn main() {
.ok(),
exhaustively_verify_refcounts: matches.is_present("accounts_db_verify_refcounts"),
create_ancient_storage,
test_partitioned_epoch_rewards,
test_skip_rewrites_but_include_in_bank_hash: matches
.is_present("accounts_db_test_skip_rewrites"),
storage_access,
Expand Down

0 comments on commit f88bda1

Please sign in to comment.