Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
don't return zero lamport accounts from 'load' with feature (backport #…
Browse files Browse the repository at this point in the history 
…27793) (backport #27803) (#27818)

* don't return zero lamport accounts from 'load' with feature (backport #27793) (#27803)

* don't return zero lamport accounts from 'load' with feature (#27793)

* don't return zero lamport accounts from 'load'

* add feature

* rename

(cherry picked from commit 138d5ed)

# Conflicts:
#	runtime/src/accounts.rs
#	sdk/src/feature_set.rs

* fix merge errors

Co-authored-by: Jeff Washington (jwash) <jeff.washington@solana.com>
(cherry picked from commit 0dda25f)

# Conflicts:
#	runtime/src/accounts.rs
#	runtime/src/accounts_db.rs
#	sdk/src/feature_set.rs

* fix merge errors

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: jeff washington <jeff.washington@solana.com>
  • Loading branch information
@mergify @jeffwashington
mergify[bot] and jeffwashington authored Sep 15, 2022
1 parent fe27380 commit 84fc345
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 22 deletions.
60 changes: 44 additions & 16 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use {
account_rent_state::{check_rent_state_with_account, RentState},
accounts_db::{
AccountShrinkThreshold, AccountsAddRootTiming, AccountsDb, AccountsDbConfig,
BankHashInfo, LoadHint, LoadedAccount, ScanStorageResult,
BankHashInfo, LoadHint, LoadZeroLamports, LoadedAccount, ScanStorageResult,
ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING,
},
accounts_index::{AccountSecondaryIndexes, IndexKey, ScanConfig, ScanError, ScanResult},
Expand All @@ -31,7 +31,10 @@ use {
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::{BankId, Slot, INITIAL_RENT_EPOCH},
feature_set::{self, add_set_compute_unit_price_ix, tx_wide_compute_cap, FeatureSet},
feature_set::{
self, add_set_compute_unit_price_ix, return_none_for_zero_lamport_accounts,
tx_wide_compute_cap, FeatureSet,
},
fee::FeeStructure,
genesis_config::ClusterType,
hash::Hash,
Expand Down Expand Up @@ -245,6 +248,13 @@ impl Accounts {
feature_set: &FeatureSet,
account_overrides: Option<&AccountOverrides>,
) -> Result<LoadedTransaction> {
let load_zero_lamports =
if feature_set.is_active(&return_none_for_zero_lamport_accounts::id()) {
LoadZeroLamports::None
} else {
LoadZeroLamports::SomeWithZeroLamportAccount
};

// Copy all the accounts
let message = tx.message();
// NOTE: this check will never fail because `tx` is sanitized
Expand Down Expand Up @@ -280,7 +290,7 @@ impl Accounts {
(account_override.clone(), 0)
} else {
self.accounts_db
.load_with_fixed_root(ancestors, key)
.load_with_fixed_root(ancestors, key, load_zero_lamports)
.map(|(mut account, _)| {
if message.is_writable(i) {
let rent_due = rent_collector
Expand Down Expand Up @@ -329,9 +339,12 @@ impl Accounts {
programdata_address,
}) = account.state()
{
if let Some((programdata_account, _)) = self
.accounts_db
.load_with_fixed_root(ancestors, &programdata_address)
if let Some((programdata_account, _)) =
self.accounts_db.load_with_fixed_root(
ancestors,
&programdata_address,
load_zero_lamports,
)
{
account_deps
.push((programdata_address, programdata_account));
Expand Down Expand Up @@ -375,6 +388,7 @@ impl Accounts {
&mut accounts,
instruction.program_id_index as usize,
error_counters,
load_zero_lamports,
)
})
.collect::<Result<Vec<Vec<usize>>>>()?;
Expand Down Expand Up @@ -454,6 +468,7 @@ impl Accounts {
accounts: &mut Vec<TransactionAccount>,
mut program_account_index: usize,
error_counters: &mut TransactionErrorMetrics,
load_zero_lamports: LoadZeroLamports,
) -> Result<Vec<usize>> {
let mut account_indices = Vec::new();
let mut program_id = accounts[program_account_index].0;
Expand All @@ -465,10 +480,11 @@ impl Accounts {
}
depth += 1;

program_account_index = match self
.accounts_db
.load_with_fixed_root(ancestors, &program_id)
{
program_account_index = match self.accounts_db.load_with_fixed_root(
ancestors,
&program_id,
load_zero_lamports,
) {
Some((program_account, _)) => {
let account_index = accounts.len();
accounts.push((program_id, program_account));
Expand All @@ -494,10 +510,11 @@ impl Accounts {
programdata_address,
}) = program.state()
{
let programdata_account_index = match self
.accounts_db
.load_with_fixed_root(ancestors, &programdata_address)
{
let programdata_account_index = match self.accounts_db.load_with_fixed_root(
ancestors,
&programdata_address,
load_zero_lamports,
) {
Some((programdata_account, _)) => {
let account_index = accounts.len();
accounts.push((programdata_address, programdata_account));
Expand Down Expand Up @@ -595,10 +612,15 @@ impl Accounts {
ancestors: &Ancestors,
address_table_lookup: &MessageAddressTableLookup,
slot_hashes: &SlotHashes,
load_zero_lamports: LoadZeroLamports,
) -> std::result::Result<LoadedAddresses, AddressLookupError> {
let table_account = self
.accounts_db
.load_with_fixed_root(ancestors, &address_table_lookup.account_key)
.load_with_fixed_root(
ancestors,
&address_table_lookup.account_key,
load_zero_lamports,
)
.map(|(account, _rent)| account)
.ok_or(AddressLookupError::LookupTableAccountNotFound)?;

Expand Down Expand Up @@ -2036,6 +2058,7 @@ mod tests {
&ancestors,
&address_table_lookup,
&SlotHashes::default(),
LoadZeroLamports::SomeWithZeroLamportAccount,
),
Err(AddressLookupError::LookupTableAccountNotFound),
);
Expand All @@ -2053,7 +2076,8 @@ mod tests {
);

let invalid_table_key = Pubkey::new_unique();
let invalid_table_account = AccountSharedData::default();
let mut invalid_table_account = AccountSharedData::default();
invalid_table_account.set_lamports(1);
accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account);

let address_table_lookup = MessageAddressTableLookup {
Expand All @@ -2067,6 +2091,7 @@ mod tests {
&ancestors,
&address_table_lookup,
&SlotHashes::default(),
LoadZeroLamports::SomeWithZeroLamportAccount,
),
Err(AddressLookupError::InvalidAccountOwner),
);
Expand Down Expand Up @@ -2099,6 +2124,7 @@ mod tests {
&ancestors,
&address_table_lookup,
&SlotHashes::default(),
LoadZeroLamports::SomeWithZeroLamportAccount,
),
Err(AddressLookupError::InvalidAccountData),
);
Expand Down Expand Up @@ -2143,6 +2169,7 @@ mod tests {
&ancestors,
&address_table_lookup,
&SlotHashes::default(),
LoadZeroLamports::SomeWithZeroLamportAccount,
),
Ok(LoadedAddresses {
writable: vec![table_addresses[0]],
Expand Down Expand Up @@ -2439,6 +2466,7 @@ mod tests {
&mut vec![(keypair.pubkey(), account)],
0,
&mut error_counters,
LoadZeroLamports::SomeWithZeroLamportAccount,
),
Err(TransactionError::ProgramAccountNotFound)
);
Expand Down
29 changes: 23 additions & 6 deletions runtime/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,17 @@ const CACHE_VIRTUAL_WRITE_VERSION: StoredMetaWriteVersion = 0;
pub(crate) const CACHE_VIRTUAL_OFFSET: Offset = 0;
const CACHE_VIRTUAL_STORED_SIZE: StoredSize = 0;

/// specifies how to return zero lamport accounts
/// This will only be useful until a feature activation occurs.
#[derive(Clone, Copy)]
pub enum LoadZeroLamports {
/// return None if loaded account has zero lamports
None,
/// return Some(account with zero lamports) if loaded account has zero lamports
/// Today this is the default. With feature activation, this will no longer be possible.
SomeWithZeroLamportAccount,
}

pub const ACCOUNTS_DB_CONFIG_FOR_TESTING: AccountsDbConfig = AccountsDbConfig {
index: Some(ACCOUNTS_INDEX_CONFIG_FOR_TESTING),
accounts_hash_cache_path: None,
Expand Down Expand Up @@ -3349,8 +3360,15 @@ impl AccountsDb {
&self,
ancestors: &Ancestors,
pubkey: &Pubkey,
load_zero_lamports: LoadZeroLamports,
) -> Option<(AccountSharedData, Slot)> {
self.load(ancestors, pubkey, LoadHint::FixedMaxRoot)
.filter(|(account, _)| {
matches!(
load_zero_lamports,
LoadZeroLamports::SomeWithZeroLamportAccount
) || !account.is_zero_lamport()
})
}

pub fn load_without_fixed_root(
Expand Down Expand Up @@ -11691,24 +11709,23 @@ pub mod tests {

assert_eq!(db.read_only_accounts_cache.cache_len(), 0);
let account = db
.load_with_fixed_root(&Ancestors::default(), &account_key)
.load_with_fixed_root(&Ancestors::default(), &account_key, LoadZeroLamports::None)
.map(|(account, _)| account)
.unwrap();
assert_eq!(account.lamports(), 1);
assert_eq!(db.read_only_accounts_cache.cache_len(), 1);
let account = db
.load_with_fixed_root(&Ancestors::default(), &account_key)
.load_with_fixed_root(&Ancestors::default(), &account_key, LoadZeroLamports::None)
.map(|(account, _)| account)
.unwrap();
assert_eq!(account.lamports(), 1);
assert_eq!(db.read_only_accounts_cache.cache_len(), 1);
db.store_cached(2, &[(&account_key, &zero_lamport_account)]);
assert_eq!(db.read_only_accounts_cache.cache_len(), 1);
let account = db
.load_with_fixed_root(&Ancestors::default(), &account_key)
.map(|(account, _)| account)
.unwrap();
assert_eq!(account.lamports(), 0);
.load_with_fixed_root(&Ancestors::default(), &account_key, LoadZeroLamports::None)
.map(|(account, _)| account);
assert!(account.is_none());
assert_eq!(db.read_only_accounts_cache.cache_len(), 1);
}

Expand Down
12 changes: 12 additions & 0 deletions runtime/src/bank/address_lookup_table.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use {
super::Bank,
crate::accounts_db::LoadZeroLamports,
solana_address_lookup_table_program::error::AddressLookupError,
solana_sdk::{
feature_set::return_none_for_zero_lamport_accounts,
message::v0::{LoadedAddresses, MessageAddressTableLookup},
transaction::{AddressLoader, Result as TransactionResult, TransactionError},
},
Expand All @@ -16,6 +18,15 @@ impl AddressLoader for &Bank {
return Err(TransactionError::UnsupportedVersion);
}

let load_zero_lamports = if self
.feature_set
.is_active(&return_none_for_zero_lamport_accounts::id())
{
LoadZeroLamports::None
} else {
LoadZeroLamports::SomeWithZeroLamportAccount
};

let slot_hashes = self
.sysvar_cache
.read()
Expand All @@ -30,6 +41,7 @@ impl AddressLoader for &Bank {
&self.ancestors,
address_table_lookup,
&slot_hashes,
load_zero_lamports,
)
})
.collect::<Result<_, AddressLookupError>>()?)
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ pub mod check_ping_ancestor_requests {
solana_sdk::declare_id!("AXLB87anNaUQtqBSsxkm4gvNzYY985aLtNtpJC94uWLJ");
}

pub mod return_none_for_zero_lamport_accounts {
solana_sdk::declare_id!("7K5HFrS1WAq6ND7RQbShXZXbtAookyTfaDQPTJNuZpze");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -518,6 +522,7 @@ lazy_static! {
(prevent_crediting_accounts_that_end_rent_paying::id(), "prevent crediting rent paying accounts #26606"),
(sign_repair_requests::id(), "sign repair requests #26834"),
(check_ping_ancestor_requests::id(), "ancestor hash repair socket ping/pong support #26963"),
(return_none_for_zero_lamport_accounts::id(), "return none for zero lamport accounts #27800"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit 84fc345

Please sign in to comment.