Skip to content

Commit

Permalink
Clean up Partitioned Epoch Rewards feature logic (#4186)
Browse files Browse the repository at this point in the history
* Remove feature from syscall logic

* Remove is_partitioned_rewards_code_enabled

* Update test_bank_hash_consistency, which runs with all features off

* Fixup test_bank_update_rewards_determinism

* Remove is_partitioned_rewards_feature_enabled

* Remove feature-gate logic

* Remove superfluous total_rewards pass-through

* Start removing old rewards logic: update_rewards_with_thread_pool and PER compare fns

* Remove more old rewards logic: helper fns; and fixup tests

* Remove more old rewards logic: StakesCache helpers

* Remove remaining old rewards logic: type aliases and helper structs

* remove partitioned reward test code

* Remove superfluous test attribute

Co-authored-by: Brooks <brooks@prumo.org>

---------

Co-authored-by: HaoranYi <haoran.yi@anza.xyz>
Co-authored-by: Tyera <tyera@solana.com>
Co-authored-by: Brooks <brooks@prumo.org>
  • Loading branch information
4 people authored Dec 30, 2024
1 parent 7f2bbd3 commit 5662eee
Show file tree
Hide file tree
Showing 16 changed files with 242 additions and 1,166 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 @@ -1964,10 +1966,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 @@ -2030,7 +2028,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
13 changes: 4 additions & 9 deletions programs/bpf_loader/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ use {
bpf_account_data_direct_mapping, curve25519_syscall_enabled,
disable_deploy_of_alloc_free_syscall, disable_fees_sysvar, disable_sbpf_v0_execution,
enable_alt_bn128_compression_syscall, enable_alt_bn128_syscall, enable_big_mod_exp_syscall,
enable_get_epoch_stake_syscall, enable_partitioned_epoch_reward, enable_poseidon_syscall,
enable_get_epoch_stake_syscall, enable_poseidon_syscall,
enable_sbpf_v1_deployment_and_execution, enable_sbpf_v2_deployment_and_execution,
enable_sbpf_v3_deployment_and_execution, get_sysvar_syscall_enabled,
last_restart_slot_sysvar, partitioned_epoch_rewards_superfeature,
reenable_sbpf_v0_execution, remaining_compute_units_syscall_enabled, FeatureSet,
last_restart_slot_sysvar, reenable_sbpf_v0_execution,
remaining_compute_units_syscall_enabled, FeatureSet,
},
solana_log_collector::{ic_logger_msg, ic_msg},
solana_poseidon as poseidon,
Expand Down Expand Up @@ -277,9 +277,6 @@ pub fn create_program_runtime_environment_v1<'a>(
let blake3_syscall_enabled = feature_set.is_active(&blake3_syscall_enabled::id());
let curve25519_syscall_enabled = feature_set.is_active(&curve25519_syscall_enabled::id());
let disable_fees_sysvar = feature_set.is_active(&disable_fees_sysvar::id());
let epoch_rewards_syscall_enabled = feature_set
.is_active(&enable_partitioned_epoch_reward::id())
|| feature_set.is_active(&partitioned_epoch_rewards_superfeature::id());
let disable_deploy_of_alloc_free_syscall = reject_deployment_of_broken_elfs
&& feature_set.is_active(&disable_deploy_of_alloc_free_syscall::id());
let last_restart_slot_syscall_enabled = feature_set.is_active(&last_restart_slot_sysvar::id());
Expand Down Expand Up @@ -416,9 +413,7 @@ pub fn create_program_runtime_environment_v1<'a>(
SyscallGetLastRestartSlotSysvar::vm,
)?;

register_feature_gated_function!(
result,
epoch_rewards_syscall_enabled,
result.register_function(
"sol_get_epoch_rewards_sysvar",
39,
SyscallGetEpochRewardsSysvar::vm,
Expand Down
Loading

0 comments on commit 5662eee

Please sign in to comment.