-
Notifications
You must be signed in to change notification settings - Fork 293
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
test validator: make feature accounts additive #3766
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,10 @@ use { | |
solana_rpc_client::{nonblocking, rpc_client::RpcClient}, | ||
solana_rpc_client_api::request::MAX_MULTIPLE_ACCOUNTS, | ||
solana_runtime::{ | ||
bank_forks::BankForks, genesis_utils::create_genesis_config_with_leader_ex, | ||
runtime_config::RuntimeConfig, snapshot_config::SnapshotConfig, | ||
bank_forks::BankForks, | ||
genesis_utils::{self, create_genesis_config_with_leader_ex_no_features}, | ||
runtime_config::RuntimeConfig, | ||
snapshot_config::SnapshotConfig, | ||
}, | ||
solana_sdk::{ | ||
account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, | ||
|
@@ -44,6 +46,7 @@ use { | |
commitment_config::CommitmentConfig, | ||
epoch_schedule::EpochSchedule, | ||
exit::Exit, | ||
feature_set::FeatureSet, | ||
fee_calculator::FeeRateGovernor, | ||
instruction::{AccountMeta, Instruction}, | ||
message::Message, | ||
|
@@ -829,7 +832,7 @@ impl TestValidator { | |
); | ||
} | ||
|
||
let mut genesis_config = create_genesis_config_with_leader_ex( | ||
let mut genesis_config = create_genesis_config_with_leader_ex_no_features( | ||
mint_lamports, | ||
&mint_address, | ||
&validator_identity.pubkey(), | ||
|
@@ -852,23 +855,21 @@ impl TestValidator { | |
genesis_config.ticks_per_slot = ticks_per_slot; | ||
} | ||
|
||
// Remove features tagged to deactivate | ||
for deactivate_feature_pk in &config.deactivate_feature_set { | ||
if FEATURE_NAMES.contains_key(deactivate_feature_pk) { | ||
match genesis_config.accounts.remove(deactivate_feature_pk) { | ||
Some(_) => info!("Feature for {:?} deactivated", deactivate_feature_pk), | ||
None => warn!( | ||
"Feature {:?} set for deactivation not found in genesis_config account list, ignored.", | ||
deactivate_feature_pk | ||
), | ||
} | ||
// Only activate features which are not explicitly deactivated. | ||
let mut feature_set = FeatureSet::default().inactive; | ||
for feature in &config.deactivate_feature_set { | ||
if feature_set.remove(feature) { | ||
info!("Feature for {:?} deactivated", feature) | ||
} else { | ||
warn!( | ||
"Feature {:?} set for deactivation is not a known Feature public key", | ||
deactivate_feature_pk | ||
); | ||
feature, | ||
) | ||
} | ||
} | ||
for feature in feature_set { | ||
genesis_utils::activate_feature(&mut genesis_config, feature); | ||
} | ||
|
||
let ledger_path = match &config.ledger_path { | ||
None => create_new_tmp_ledger!(&genesis_config).0, | ||
|
@@ -1193,7 +1194,7 @@ impl Drop for TestValidator { | |
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use {super::*, solana_sdk::feature::Feature}; | ||
|
||
#[test] | ||
fn get_health() { | ||
|
@@ -1217,4 +1218,86 @@ mod test { | |
// `start()` blows up when run within tokio | ||
let (_test_validator, _payer) = TestValidatorGenesis::default().start(); | ||
} | ||
|
||
#[tokio::test] | ||
async fn test_deactivate_features() { | ||
let mut control = FeatureSet::default().inactive; | ||
let mut deactivate_features = Vec::new(); | ||
[ | ||
solana_sdk::feature_set::deprecate_rewards_sysvar::id(), | ||
solana_sdk::feature_set::disable_fees_sysvar::id(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test doesn't seem to exercise the exact behavior that you want, of overriding a feature account with something else. How about adding some account data at one of these addresses? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hah, yeah, good point. Updated! |
||
] | ||
.into_iter() | ||
.for_each(|feature| { | ||
control.remove(&feature); | ||
deactivate_features.push(feature); | ||
}); | ||
|
||
// Convert to `Vec` so we can get a slice. | ||
let control: Vec<Pubkey> = control.into_iter().collect(); | ||
|
||
let (test_validator, _payer) = TestValidatorGenesis::default() | ||
.deactivate_features(&deactivate_features) | ||
.start_async() | ||
.await; | ||
|
||
let rpc_client = test_validator.get_async_rpc_client(); | ||
|
||
// Our deactivated features should be inactive. | ||
let inactive_feature_accounts = rpc_client | ||
.get_multiple_accounts(&deactivate_features) | ||
.await | ||
.unwrap(); | ||
for f in inactive_feature_accounts { | ||
assert!(f.is_none()); | ||
} | ||
|
||
// Everything else should be active. | ||
for chunk in control.chunks(100) { | ||
let active_feature_accounts = rpc_client.get_multiple_accounts(chunk).await.unwrap(); | ||
for f in active_feature_accounts { | ||
let account = f.unwrap(); // Should be `Some`. | ||
let feature_state: Feature = bincode::deserialize(account.data()).unwrap(); | ||
assert!(feature_state.activated_at.is_some()); | ||
} | ||
} | ||
} | ||
|
||
#[tokio::test] | ||
async fn test_override_feature_account() { | ||
let with_deactivate_flag = solana_sdk::feature_set::deprecate_rewards_sysvar::id(); | ||
let without_deactivate_flag = solana_sdk::feature_set::disable_fees_sysvar::id(); | ||
|
||
let owner = Pubkey::new_unique(); | ||
let account = || AccountSharedData::new(100_000, 0, &owner); | ||
|
||
let (test_validator, _payer) = TestValidatorGenesis::default() | ||
.deactivate_features(&[with_deactivate_flag]) // Just deactivate one feature. | ||
.add_accounts([ | ||
(with_deactivate_flag, account()), // But add both accounts. | ||
(without_deactivate_flag, account()), | ||
]) | ||
.start_async() | ||
.await; | ||
|
||
let rpc_client = test_validator.get_async_rpc_client(); | ||
|
||
let our_accounts = rpc_client | ||
.get_multiple_accounts(&[with_deactivate_flag, without_deactivate_flag]) | ||
.await | ||
.unwrap(); | ||
|
||
// The first one, where we provided `--deactivate-feature`, should be | ||
// the account we provided. | ||
let overriden_account = our_accounts[0].as_ref().unwrap(); | ||
assert_eq!(overriden_account.lamports, 100_000); | ||
assert_eq!(overriden_account.data.len(), 0); | ||
assert_eq!(overriden_account.owner, owner); | ||
|
||
// The second one should be a feature account. | ||
let feature_account = our_accounts[1].as_ref().unwrap(); | ||
assert_eq!(feature_account.owner, solana_sdk::feature::id()); | ||
let feature_state: Feature = bincode::deserialize(feature_account.data()).unwrap(); | ||
assert!(feature_state.activated_at.is_some()); | ||
} | ||
} |
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.
This may look a little redundant, but I wanted to be absolutely sure the original behavior for
create_genesis_config_with_leader_ex
was preserved.