test validator: make feature accounts additive #3766
Conversation
| for feature in feature_set { | ||
| genesis_utils::activate_feature(&mut genesis_config, feature); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The change looks good to me! just a suggestion on the test.
Just to be totally sure, with this change, if you want to add account data at a feature address, you'll need to specify both --account ... and --deactivate-feature ..., correct?
We could eventually return an error if the pubkey on a provided --account is a feature and the --deactivate-feature option is not provided, but as you said, this is a pretty niche use-case.
| 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.
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.
Hah, yeah, good point. Updated!
I can add this if you think we should include it here, but for now you'd just get your account overwritten like it does now. I think that's okay, but happy to add the check if need be. |
buffalojoec
force-pushed
the
test-validator-genesis-features
branch
from
a641488 to
12ff917
Compare
buffalojoec
added
the
automerge
Problem
In a situation where you may want to place a certain account at one of the feature IDs in the SDK's feature set, the test validator will always overwrite your provided account with a feature account. You can provide the
--deactivate-featureoption, but the account is still overwritten before it is then removed, rendering the user no way to place an account at that particular address.This may not be especially useful to typical program developers, however it is definitely useful to test runtime behavior, such as that of the nascent Feature Gate Core BPF program.
Summary of Changes
Simply reorder how the test validator pops deactivated features out of the genesis accounts list. Rather than populating the account store with a feature account at every feature ID and then removing the declared deactivated features, instead only create accounts for the active ones, allowing provided accounts for deactivated features to make it into genesis.