Skip to content

Commit

Permalink
Deprecate is_sysvar_id function (solana-labs#789)
Browse files Browse the repository at this point in the history
Deprecate is_sysvar_id
  • Loading branch information
jstarry authored and buffalojoec committed Apr 16, 2024
1 parent 782e308 commit fff1b60
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 16 deletions.
2 changes: 1 addition & 1 deletion ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
12 changes: 6 additions & 6 deletions ledger-tool/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S>(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4369,7 +4369,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"
);

Expand Down
1 change: 1 addition & 0 deletions sdk/program/src/message/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions sdk/program/src/sysvar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
29 changes: 21 additions & 8 deletions storage-bigtable/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
Expand Down Expand Up @@ -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;
Expand All @@ -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()
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down

0 comments on commit fff1b60

Please sign in to comment.