solana-genesis: deactivate specific features for development clusters #2246
Conversation
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
gregcusack
changed the title
steviez
reviewed
Jul 24, 2024
gregcusack
force-pushed
the
genesis-deactivate-features
branch
from
c67dff9 to
456552e
Compare
steviez
reviewed
Jul 24, 2024
runtime/src/genesis_utils.rs
Outdated
| @@ -200,6 +201,20 @@ pub fn activate_all_features(genesis_config: &mut GenesisConfig) { | |||
| } | |||
| } | |||
|
|
|||
| pub fn deactivate_features(genesis_config: &mut GenesisConfig, features_to_skip: &Vec<Pubkey>) { | |||
There was a problem hiding this comment.
nit: Since the function name is now deactivate_features, can you change the parameter name to features_to_deactivate ?
steviez
reviewed
Jul 25, 2024
genesis/src/main.rs
Outdated
| let features_to_deactivate = pubkeys_of(&matches, "deactivate_feature").unwrap_or_default(); | ||
|
|
||
| if cluster_type != ClusterType::Development && !features_to_deactivate.is_empty() { | ||
| eprintln!("Error: The --deactivate-feature-set argument cannot be used with --cluster-type={cluster_type:?}"); |
There was a problem hiding this comment.
nit: Update the help text to match the new flag name
steviez
reviewed
Jul 25, 2024
genesis/src/main.rs
Outdated
| .value_name("FEATURE_PUBKEY") | ||
| .validator(is_pubkey) | ||
| .multiple(true) | ||
| .help("deactivate this feature in genesis. Compatable with ClusterType::Development"), |
There was a problem hiding this comment.
typo: compatible
nit: ClusterType::Development is the name of the type in code, but I think it is possibly more clear to use the string that a user would pass on the CLI (development). I think your wording is likely fine as-is, but could also adjust wording to more strongly say only valid when using --cluster-type development or something along those lines
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
It's not very easy to deactivate specific features in development clusters. This is a step towards increasing feature activation granularity
Summary of Changes
--deactivate-feature-set<feature-pubkey0> <feature-pubkey1> ... <feature-pubkeyN>ClusterType::Developmentclusters, all features are activated, this lets us deactivate specific ones. Example: in a development cluster we do not want to activatetimely_vote_creditsfor a cluster running both v1.17 and v1.18 validatorsNote: naming convention is kept in line with
test-validatorgenesis