From 056bfcede7270ed1f38869cebcab93619b9a045c Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Tue, 16 Apr 2024 06:22:52 +0800 Subject: [PATCH] Deprecate `is_sysvar_id` function (#789) Deprecate is_sysvar_id --- ledger-tool/src/main.rs | 2 +- ledger-tool/src/output.rs | 12 ++++++------ runtime/src/bank/tests.rs | 2 +- sdk/program/src/message/legacy.rs | 1 + sdk/program/src/sysvar/mod.rs | 4 ++++ storage-bigtable/src/lib.rs | 29 +++++++++++++++++++++-------- 6 files changed, 34 insertions(+), 16 deletions(-) diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 3c1e23f6d..c03c4c670 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -2778,7 +2778,7 @@ fn main() { for (pubkey, warped_account) in all_accounts { // Don't output sysvars; it's always updated but not related to // inflation. - if solana_sdk::sysvar::is_sysvar_id(&pubkey) { + if solana_sdk::sysvar::check_id(warped_account.owner()) { continue; } diff --git a/ledger-tool/src/output.rs b/ledger-tool/src/output.rs index 2de702ef4..ceb79ec99 100644 --- a/ledger-tool/src/output.rs +++ b/ledger-tool/src/output.rs @@ -643,9 +643,9 @@ struct AccountsScanner { impl AccountsScanner { /// Returns true if this account should be included in the output - fn should_process_account(&self, account: &AccountSharedData, pubkey: &Pubkey) -> bool { + fn should_process_account(&self, account: &AccountSharedData) -> bool { solana_accounts_db::accounts::Accounts::is_loadable(account.lamports()) - && (self.config.include_sysvars || !solana_sdk::sysvar::is_sysvar_id(pubkey)) + && (self.config.include_sysvars || !solana_sdk::sysvar::check_id(account.owner())) } fn maybe_output_account( @@ -688,8 +688,8 @@ impl AccountsScanner { }; let scan_func = |account_tuple: Option<(&Pubkey, AccountSharedData, Slot)>| { - if let Some((pubkey, account, slot)) = account_tuple - .filter(|(pubkey, account, _)| self.should_process_account(account, pubkey)) + if let Some((pubkey, account, slot)) = + account_tuple.filter(|(_, account, _)| self.should_process_account(account)) { total_accounts_stats.accumulate_account(pubkey, &account, rent_collector); self.maybe_output_account( @@ -710,7 +710,7 @@ impl AccountsScanner { if let Some((account, slot)) = self .bank .get_account_modified_slot_with_fixed_root(pubkey) - .filter(|(account, _)| self.should_process_account(account, pubkey)) + .filter(|(account, _)| self.should_process_account(account)) { total_accounts_stats.accumulate_account(pubkey, &account, rent_collector); self.maybe_output_account( @@ -727,7 +727,7 @@ impl AccountsScanner { .get_program_accounts(program_pubkey, &ScanConfig::default()) .unwrap() .iter() - .filter(|(pubkey, account)| self.should_process_account(account, pubkey)) + .filter(|(_, account)| self.should_process_account(account)) .for_each(|(pubkey, account)| { total_accounts_stats.accumulate_account(pubkey, account, rent_collector); self.maybe_output_account( diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 80deca700..5a3b6c767 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -4366,7 +4366,7 @@ fn test_bank_get_program_accounts() { assert!( genesis_accounts .iter() - .any(|(pubkey, _, _)| solana_sdk::sysvar::is_sysvar_id(pubkey)), + .any(|(_, account, _)| solana_sdk::sysvar::check_id(account.owner())), "no sysvars found" ); diff --git a/sdk/program/src/message/legacy.rs b/sdk/program/src/message/legacy.rs index 3d20e9808..561de13f0 100644 --- a/sdk/program/src/message/legacy.rs +++ b/sdk/program/src/message/legacy.rs @@ -58,6 +58,7 @@ lazy_static! { }; } +#[allow(deprecated)] pub fn is_builtin_key_or_sysvar(key: &Pubkey) -> bool { if MAYBE_BUILTIN_KEY_OR_SYSVAR[key.0[0] as usize] { return sysvar::is_sysvar_id(key) || BUILTIN_PROGRAMS_KEYS.contains(key); diff --git a/sdk/program/src/sysvar/mod.rs b/sdk/program/src/sysvar/mod.rs index 4aabbce33..4f37f3c5b 100644 --- a/sdk/program/src/sysvar/mod.rs +++ b/sdk/program/src/sysvar/mod.rs @@ -118,6 +118,10 @@ lazy_static! { } /// Returns `true` of the given `Pubkey` is a sysvar account. +#[deprecated( + since = "2.0.0", + note = "please check the account's owner or use solana_sdk::reserved_account_keys::ReservedAccountKeys instead" +)] pub fn is_sysvar_id(id: &Pubkey) -> bool { ALL_IDS.iter().any(|key| key == id) } diff --git a/storage-bigtable/src/lib.rs b/storage-bigtable/src/lib.rs index 85c714c63..240ae44c3 100644 --- a/storage-bigtable/src/lib.rs +++ b/storage-bigtable/src/lib.rs @@ -10,8 +10,8 @@ use { deserialize_utils::default_on_eof, message::v0::LoadedAddresses, pubkey::Pubkey, + reserved_account_keys::ReservedAccountKeys, signature::Signature, - sysvar::is_sysvar_id, timing::AtomicInterval, transaction::{TransactionError, VersionedTransaction}, }, @@ -938,6 +938,7 @@ impl LedgerStorage { entries, } = confirmed_block; + let reserved_account_keys = ReservedAccountKeys::new_all_activated(); let mut tx_cells = Vec::with_capacity(confirmed_block.transactions.len()); for (index, transaction_with_meta) in confirmed_block.transactions.iter().enumerate() { let VersionedTransactionWithStatusMeta { meta, transaction } = transaction_with_meta; @@ -947,7 +948,11 @@ impl LedgerStorage { let memo = extract_and_fmt_memos(transaction_with_meta); for address in transaction_with_meta.account_keys().iter() { - if !is_sysvar_id(address) { + // Historical note that previously only a set of sysvar ids were + // skipped from being uploaded. Now we skip uploaded for the set + // of all reserved account keys which will continue to grow in + // the future. + if !reserved_account_keys.is_reserved(address) { by_addr .entry(address) .or_default() @@ -1083,9 +1088,13 @@ impl LedgerStorage { let err = None; for address in transaction.message.account_keys.iter() { - if !is_sysvar_id(address) { - addresses.insert(address); - } + // We could skip deleting addresses that are known + // reserved keys but it's hard to be sure whether we + // previously uploaded rows for reserved keys or not. So + // to ensure everything is deleted properly, we attempt + // to delete rows for all addresses even if they might + // not have been uploaded. + addresses.insert(address); } expected_tx_infos.insert( @@ -1100,9 +1109,13 @@ impl LedgerStorage { let err = meta.status.clone().err(); for address in tx_with_meta.account_keys().iter() { - if !is_sysvar_id(address) { - addresses.insert(address); - } + // We could skip deleting addresses that are known + // reserved keys but it's hard to be sure whether we + // previously uploaded rows for reserved keys or not. So + // to ensure everything is deleted properly, we attempt + // to delete rows for all addresses even if they might + // not have been uploaded. + addresses.insert(address); } expected_tx_infos.insert(