diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 03bbdd98387679..8cf6b2a402f071 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -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}, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/accounts-db/src/partitioned_rewards.rs b/accounts-db/src/partitioned_rewards.rs index 4ced408c6495e9..85758a4df84f0f 100644 --- a/accounts-db/src/partitioned_rewards.rs +++ b/accounts-db/src/partitioned_rewards.rs @@ -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, } } } diff --git a/ledger-tool/src/args.rs b/ledger-tool/src/args.rs index bc80abcfa7b113..1d0505968d822d 100644 --- a/ledger-tool/src/args.rs +++ b/ledger-tool/src/args.rs @@ -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::{ @@ -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) @@ -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, diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 4580c0b7731abe..8e0cdc5a019895 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -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") diff --git a/runtime/src/bank/partitioned_epoch_rewards/calculation.rs b/runtime/src/bank/partitioned_epoch_rewards/calculation.rs index f6c84557cda0a4..4fba65b604aa84 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/calculation.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/calculation.rs @@ -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; diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 32ba70e0eee282..2d2e21aace90e3 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -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)] @@ -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, @@ -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, @@ -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, diff --git a/validator/src/cli.rs b/validator/src/cli.rs index aed7a3bffc1d9f..526870faa0670d 100644 --- a/validator/src/cli.rs +++ b/validator/src/cli.rs @@ -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") diff --git a/validator/src/main.rs b/validator/src/main.rs index a7de615b3be9ac..f4eb2fe21e86b8 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -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, @@ -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 { @@ -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,