Clean up feature `add_new_reserved_account_keys` (#6387)

reserved-account-keys/src/lib.rs

@@ -49,7 +49,7 @@ impl ReservedAccountKeys {
     /// designate a feature id, it's already activated and should be inserted
     /// into the active set. If it does have a feature id, insert the key and
     /// its feature id into the inactive map.
-    fn new(reserved_accounts: &[ReservedAccount]) -> Self {
+    pub fn new(reserved_accounts: &[ReservedAccount]) -> Self {
         Self {
             active: reserved_accounts
                 .iter()
@@ -113,20 +113,20 @@ impl ReservedAccountKeys {
 /// write-lockable by transactions. If a feature id is set, the account will
 /// become read-only only after the feature has been activated.
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
-struct ReservedAccount {
+pub struct ReservedAccount {
     key: Pubkey,
     feature_id: Option<Pubkey>,
 }
 
 impl ReservedAccount {
-    fn new_pending(key: Pubkey, feature_id: Pubkey) -> Self {
+    pub fn new_pending(key: Pubkey, feature_id: Pubkey) -> Self {
         Self {
             key,
             feature_id: Some(feature_id),
         }
     }
 
-    fn new_active(key: Pubkey) -> Self {
+    pub fn new_active(key: Pubkey) -> Self {
         Self {
             key,
             feature_id: None,
@@ -141,31 +141,16 @@ static RESERVED_ACCOUNTS: std::sync::LazyLock<Vec<ReservedAccount>> =
     std::sync::LazyLock::new(|| {
         vec![
             // builtin programs
-            ReservedAccount::new_pending(
-                address_lookup_table::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(address_lookup_table::id()),
             ReservedAccount::new_active(bpf_loader::id()),
             ReservedAccount::new_active(bpf_loader_deprecated::id()),
             ReservedAccount::new_active(bpf_loader_upgradeable::id()),
-            ReservedAccount::new_pending(
-                compute_budget::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(compute_budget::id()),
             ReservedAccount::new_active(config::id()),
-            ReservedAccount::new_pending(
-                ed25519_program::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(ed25519_program::id()),
             ReservedAccount::new_active(feature::id()),
-            ReservedAccount::new_pending(
-                loader_v4::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
-            ReservedAccount::new_pending(
-                secp256k1_program::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(loader_v4::id()),
+            ReservedAccount::new_active(secp256k1_program::id()),
             ReservedAccount::new_pending(
                 secp256r1_program::id(),
                 feature_set::enable_secp256r1_precompile::id(),
@@ -175,28 +160,16 @@ static RESERVED_ACCOUNTS: std::sync::LazyLock<Vec<ReservedAccount>> =
             ReservedAccount::new_active(stake::id()),
             ReservedAccount::new_active(system_program::id()),
             ReservedAccount::new_active(vote::id()),
-            ReservedAccount::new_pending(
-                zk_elgamal_proof_program::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
-            ReservedAccount::new_pending(
-                zk_token_proof_program::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(zk_elgamal_proof_program::id()),
+            ReservedAccount::new_active(zk_token_proof_program::id()),
             // sysvars
             ReservedAccount::new_active(sysvar::clock::id()),
-            ReservedAccount::new_pending(
-                sysvar::epoch_rewards::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(sysvar::epoch_rewards::id()),
             ReservedAccount::new_active(sysvar::epoch_schedule::id()),
             #[allow(deprecated)]
             ReservedAccount::new_active(sysvar::fees::id()),
             ReservedAccount::new_active(sysvar::instructions::id()),
-            ReservedAccount::new_pending(
-                sysvar::last_restart_slot::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(sysvar::last_restart_slot::id()),
             #[allow(deprecated)]
             ReservedAccount::new_active(sysvar::recent_blockhashes::id()),
             ReservedAccount::new_active(sysvar::rent::id()),
@@ -206,17 +179,13 @@ static RESERVED_ACCOUNTS: std::sync::LazyLock<Vec<ReservedAccount>> =
             ReservedAccount::new_active(sysvar::stake_history::id()),
             // other
             ReservedAccount::new_active(native_loader::id()),
-            ReservedAccount::new_pending(
-                sysvar::id(),
-                feature_set::add_new_reserved_account_keys::id(),
-            ),
+            ReservedAccount::new_active(sysvar::id()),
         ]
     });
 
 #[cfg(test)]
 mod tests {
-    #![allow(deprecated)]
-    use {super::*, solana_message::legacy::BUILTIN_PROGRAMS_KEYS, solana_sysvar::ALL_IDS};
+    use super::*;
 
     #[test]
     fn test_is_reserved() {
@@ -277,15 +246,4 @@ mod tests {
         assert!(reserved_account_keys.is_reserved(&pending_reserved_keys[0]));
         assert!(reserved_account_keys.is_reserved(&pending_reserved_keys[1]));
     }
-
-    #[test]
-    fn test_static_list_compat() {
-        let mut static_set = HashSet::new();
-        static_set.extend(ALL_IDS.iter().cloned());
-        static_set.extend(BUILTIN_PROGRAMS_KEYS.iter().cloned());
-
-        let initial_active_set = ReservedAccountKeys::default().active;
-
-        assert_eq!(initial_active_set, static_set);
-    }
 }

runtime/src/bank/tests.rs

@@ -21,7 +21,9 @@ use {
         status_cache::MAX_CACHE_ENTRIES,
     },
     agave_feature_set::{self as feature_set, FeatureSet},
+    agave_reserved_account_keys::ReservedAccount,
     agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET,
+    ahash::AHashMap,
     assert_matches::assert_matches,
     crossbeam_channel::{bounded, unbounded},
     ed25519_dalek::ed25519::signature::Signer as EdSigner,
@@ -8047,25 +8049,32 @@ fn test_compute_active_feature_set() {
 fn test_reserved_account_keys() {
     let (bank0, _bank_forks) = create_simple_test_arc_bank(100_000);
     let mut bank = Bank::new_from_parent(bank0, &Pubkey::default(), 1);
-    bank.feature_set = Arc::new(FeatureSet::default());
+    let test_feature_id = Pubkey::new_unique();
+    bank.feature_set = Arc::new(FeatureSet::new(
+        AHashMap::new(),
+        AHashSet::from([test_feature_id]),
+    ));
+    bank.reserved_account_keys = Arc::new(ReservedAccountKeys::new(&[
+        ReservedAccount::new_active(system_program::id()),
+        ReservedAccount::new_pending(Pubkey::new_unique(), test_feature_id),
+    ]));
 
     assert_eq!(
         bank.get_reserved_account_keys().len(),
-        20,
-        "before activating the new feature, bank should already have active reserved keys"
+        1,
+        "before activating the test feature, bank should already have an active reserved key"
     );
 
-    // Activate `add_new_reserved_account_keys` feature
     bank.store_account(
-        &feature_set::add_new_reserved_account_keys::id(),
+        &test_feature_id,
         &feature::create_account(&Feature::default(), 42),
     );
     bank.apply_feature_activations(ApplyFeatureActivationsCaller::NewFromParent, true);
 
     assert_eq!(
         bank.get_reserved_account_keys().len(),
-        30,
-        "after activating the new feature, bank should have new active reserved keys"
+        2,
+        "after activating the test feature, bank should have another active reserved key"
     );
 }