forked from solana-labs/solana
-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove partitioned reward test CLI and Config #3949
Closed
HaoranYi
wants to merge
1
commit into
anza-xyz:master
from
HaoranYi:remove_partitioned_reward_test_code
+48
−331
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the test CLI will work fine even when the feature is active.