Remove sysvar special cases for rent and assign

This commit is contained in:
Ryo Onodera 2021-05-18 00:30:48 +09:00 committed by Michael Vines
parent fd574dcb3b
commit f029af0fca
7 changed files with 513 additions and 33 deletions

View File

@ -1138,6 +1138,7 @@ mod tests {
programs: vec![], programs: vec![],
accounts: vec![], accounts: vec![],
sysvars: vec![], sysvars: vec![],
feature_active: false,
}; };
assert_eq!( assert_eq!(
Err(InstructionError::ProgramFailedToComplete), Err(InstructionError::ProgramFailedToComplete),

View File

@ -210,6 +210,8 @@ impl Accounts {
let mut account_deps = Vec::with_capacity(message.account_keys.len()); let mut account_deps = Vec::with_capacity(message.account_keys.len());
let mut key_check = MessageProgramIdsCache::new(message); let mut key_check = MessageProgramIdsCache::new(message);
let mut rent_debits = RentDebits::default(); let mut rent_debits = RentDebits::default();
let rent_for_sysvars = feature_set.is_active(&feature_set::rent_for_sysvars::id());
for (i, key) in message.account_keys.iter().enumerate() { for (i, key) in message.account_keys.iter().enumerate() {
let account = if key_check.is_non_loader_key(key, i) { let account = if key_check.is_non_loader_key(key, i) {
if payer_index.is_none() { if payer_index.is_none() {
@ -229,8 +231,11 @@ impl Accounts {
.load_with_fixed_root(ancestors, key) .load_with_fixed_root(ancestors, key)
.map(|(mut account, _)| { .map(|(mut account, _)| {
if message.is_writable(i) { if message.is_writable(i) {
let rent_due = rent_collector let rent_due = rent_collector.collect_from_existing_account(
.collect_from_existing_account(key, &mut account); key,
&mut account,
rent_for_sysvars,
);
(account, rent_due) (account, rent_due)
} else { } else {
(account, 0) (account, 0)
@ -912,6 +917,7 @@ impl Accounts {
rent_collector: &RentCollector, rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool, fix_recent_blockhashes_sysvar_delay: bool,
rent_for_sysvars: bool,
) { ) {
let accounts_to_store = self.collect_accounts_to_store( let accounts_to_store = self.collect_accounts_to_store(
txs, txs,
@ -920,6 +926,7 @@ impl Accounts {
rent_collector, rent_collector,
last_blockhash_with_fee_calculator, last_blockhash_with_fee_calculator,
fix_recent_blockhashes_sysvar_delay, fix_recent_blockhashes_sysvar_delay,
rent_for_sysvars,
); );
self.accounts_db.store_cached(slot, &accounts_to_store); self.accounts_db.store_cached(slot, &accounts_to_store);
} }
@ -944,6 +951,7 @@ impl Accounts {
rent_collector: &RentCollector, rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool, fix_recent_blockhashes_sysvar_delay: bool,
rent_for_sysvars: bool,
) -> Vec<(&'a Pubkey, &'a AccountSharedData)> { ) -> Vec<(&'a Pubkey, &'a AccountSharedData)> {
let mut accounts = Vec::with_capacity(loaded.len()); let mut accounts = Vec::with_capacity(loaded.len());
for (i, ((raccs, _nonce_rollback), tx)) in loaded.iter_mut().zip(txs).enumerate() { for (i, ((raccs, _nonce_rollback), tx)) in loaded.iter_mut().zip(txs).enumerate() {
@ -1005,7 +1013,11 @@ impl Accounts {
} }
} }
if account.rent_epoch() == INITIAL_RENT_EPOCH { if account.rent_epoch() == INITIAL_RENT_EPOCH {
let rent = rent_collector.collect_from_created_account(key, account); let rent = rent_collector.collect_from_created_account(
key,
account,
rent_for_sysvars,
);
loaded_transaction.rent += rent; loaded_transaction.rent += rent;
loaded_transaction loaded_transaction
.rent_debits .rent_debits
@ -2001,6 +2013,7 @@ mod tests {
&rent_collector, &rent_collector,
&(Hash::default(), FeeCalculator::default()), &(Hash::default(), FeeCalculator::default()),
true, true,
true,
); );
assert_eq!(collected_accounts.len(), 2); assert_eq!(collected_accounts.len(), 2);
assert!(collected_accounts assert!(collected_accounts
@ -2377,6 +2390,7 @@ mod tests {
&rent_collector, &rent_collector,
&(next_blockhash, FeeCalculator::default()), &(next_blockhash, FeeCalculator::default()),
true, true,
true,
); );
assert_eq!(collected_accounts.len(), 2); assert_eq!(collected_accounts.len(), 2);
assert_eq!( assert_eq!(
@ -2493,6 +2507,7 @@ mod tests {
&rent_collector, &rent_collector,
&(next_blockhash, FeeCalculator::default()), &(next_blockhash, FeeCalculator::default()),
true, true,
true,
); );
assert_eq!(collected_accounts.len(), 1); assert_eq!(collected_accounts.len(), 1);
let collected_nonce_account = collected_accounts let collected_nonce_account = collected_accounts

View File

@ -1517,8 +1517,27 @@ impl Bank {
where where
F: Fn(&Option<AccountSharedData>) -> AccountSharedData, F: Fn(&Option<AccountSharedData>) -> AccountSharedData,
{ {
let old_account = self.get_sysvar_account_with_fixed_root(pubkey); let old_account = if !self.rent_for_sysvars() {
let new_account = updater(&old_account); // This old behavior is being retired for simpler reasoning for the benefits of all.
// Specifically, get_sysvar_account_with_fixed_root() doesn't work nicely with eager
// rent collection, which becomes applicable for sysvars after rent_for_sysvars
// activation. That's because get_sysvar_account_with_fixed_root() called by both
// update_slot_history() and update_recent_blockhashes() ignores any updates
// done by eager rent collection in this slot.
// Also, it turned out that get_sysvar_account_with_fixed_root()'s special
// behavior (idempotent) isn't needed to begin with, because we're fairly certain that
// we don't call new_from_parent() with same child slot multiple times in the
// production code...
self.get_sysvar_account_with_fixed_root(pubkey)
} else {
self.get_account_with_fixed_root(pubkey)
};
let mut new_account = updater(&old_account);
if self.rent_for_sysvars() {
// always re-calculate for possible data size change
self.adjust_sysvar_balance_for_rent(&mut new_account);
}
self.store_account_and_update_capitalization(pubkey, &new_account); self.store_account_and_update_capitalization(pubkey, &new_account);
} }
@ -1529,7 +1548,16 @@ impl Bank {
) -> InheritableAccountFields { ) -> InheritableAccountFields {
( (
old_account.as_ref().map(|a| a.lamports()).unwrap_or(1), old_account.as_ref().map(|a| a.lamports()).unwrap_or(1),
INITIAL_RENT_EPOCH, if !self.rent_for_sysvars() {
INITIAL_RENT_EPOCH
} else {
// start to inherit rent_epoch updated by rent collection to be consistent with
// other normal accounts
old_account
.as_ref()
.map(|a| a.rent_epoch())
.unwrap_or(INITIAL_RENT_EPOCH)
},
) )
} }
@ -3407,6 +3435,7 @@ impl Bank {
&self.rent_collector, &self.rent_collector,
&self.last_blockhash_with_fee_calculator(), &self.last_blockhash_with_fee_calculator(),
self.fix_recent_blockhashes_sysvar_delay(), self.fix_recent_blockhashes_sysvar_delay(),
self.rent_for_sysvars(),
); );
let rent_debits = self.collect_rent(executed, loaded_txs); let rent_debits = self.collect_rent(executed, loaded_txs);
@ -3678,12 +3707,15 @@ impl Bank {
let account_count = accounts.len(); let account_count = accounts.len();
// parallelize? // parallelize?
let rent_for_sysvars = self.rent_for_sysvars();
let mut total_rent = 0; let mut total_rent = 0;
let mut rent_debits = RentDebits::default(); let mut rent_debits = RentDebits::default();
for (pubkey, mut account) in accounts { for (pubkey, mut account) in accounts {
let rent = self let rent = self.rent_collector.collect_from_existing_account(
.rent_collector &pubkey,
.collect_from_existing_account(&pubkey, &mut account); &mut account,
rent_for_sysvars,
);
total_rent += rent; total_rent += rent;
// Store all of them unconditionally to purge old AppendVec, // Store all of them unconditionally to purge old AppendVec,
// even if collected rent is 0 (= not updated). // even if collected rent is 0 (= not updated).
@ -4182,16 +4214,31 @@ impl Bank {
if let Some(old_account) = self.get_account_with_fixed_root(pubkey) { if let Some(old_account) = self.get_account_with_fixed_root(pubkey) {
match new_account.lamports().cmp(&old_account.lamports()) { match new_account.lamports().cmp(&old_account.lamports()) {
std::cmp::Ordering::Greater => { std::cmp::Ordering::Greater => {
self.capitalization let increased = new_account.lamports() - old_account.lamports();
.fetch_add(new_account.lamports() - old_account.lamports(), Relaxed); trace!(
"store_account_and_update_capitalization: increased: {} {}",
pubkey,
increased
);
self.capitalization.fetch_add(increased, Relaxed);
} }
std::cmp::Ordering::Less => { std::cmp::Ordering::Less => {
self.capitalization let decreased = old_account.lamports() - new_account.lamports();
.fetch_sub(old_account.lamports() - new_account.lamports(), Relaxed); trace!(
"store_account_and_update_capitalization: decreased: {} {}",
pubkey,
decreased
);
self.capitalization.fetch_sub(decreased, Relaxed);
} }
std::cmp::Ordering::Equal => {} std::cmp::Ordering::Equal => {}
} }
} else { } else {
trace!(
"store_account_and_update_capitalization: created: {} {}",
pubkey,
new_account.lamports()
);
self.capitalization self.capitalization
.fetch_add(new_account.lamports(), Relaxed); .fetch_add(new_account.lamports(), Relaxed);
} }
@ -4335,6 +4382,8 @@ impl Bank {
// multiple times with the same parent_slot in the case of forking. // multiple times with the same parent_slot in the case of forking.
// //
// Generally, all of sysvar update granularity should be slot boundaries. // Generally, all of sysvar update granularity should be slot boundaries.
//
// This behavior is deprecated... See comment in update_sysvar_account() for details
fn get_sysvar_account_with_fixed_root(&self, pubkey: &Pubkey) -> Option<AccountSharedData> { fn get_sysvar_account_with_fixed_root(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
let mut ancestors = self.ancestors.clone(); let mut ancestors = self.ancestors.clone();
ancestors.remove(&self.slot()); ancestors.remove(&self.slot());
@ -5088,6 +5137,14 @@ impl Bank {
self.rewrite_stakes(); self.rewrite_stakes();
} }
if new_feature_activations.contains(&feature_set::rent_for_sysvars::id()) {
// when this feature is activated, immediately all of existing sysvars are susceptible
// to rent collection and account data removal due to insufficient balance due to only
// having 1 lamport.
// so before any is accessed, reset the balance to be rent-exempt here at the same
// timing when perpetual balance adjustment is started in update_sysvar_account().
self.reset_all_sysvar_balances();
}
if !debug_do_not_add_builtins { if !debug_do_not_add_builtins {
self.ensure_feature_builtins(init_finish_or_warp, &new_feature_activations); self.ensure_feature_builtins(init_finish_or_warp, &new_feature_activations);
@ -5096,6 +5153,41 @@ impl Bank {
self.ensure_no_storage_rewards_pool(); self.ensure_no_storage_rewards_pool();
} }
fn reset_all_sysvar_balances(&self) {
for sysvar_id in &[
sysvar::clock::id(),
sysvar::epoch_schedule::id(),
sysvar::fees::id(),
sysvar::recent_blockhashes::id(),
sysvar::rent::id(),
sysvar::rewards::id(),
sysvar::slot_hashes::id(),
sysvar::slot_history::id(),
sysvar::stake_history::id(),
] {
if let Some(mut account) = self.get_account(sysvar_id) {
let (old_data_len, old_lamports) = (account.data().len(), account.lamports());
self.adjust_sysvar_balance_for_rent(&mut account);
info!(
"reset_all_sysvar_balances (slot: {}): {} ({} bytes) is reset from {} to {}",
self.slot(),
sysvar_id,
old_data_len,
old_lamports,
account.lamports()
);
self.store_account_and_update_capitalization(sysvar_id, &account);
}
}
}
fn adjust_sysvar_balance_for_rent(&self, account: &mut AccountSharedData) {
account.set_lamports(
self.get_minimum_balance_for_rent_exemption(account.data().len())
.max(account.lamports()),
);
}
// Compute the active feature set based on the current bank state, and return the set of newly activated features // Compute the active feature set based on the current bank state, and return the set of newly activated features
fn compute_active_feature_set(&mut self, allow_new_activations: bool) -> HashSet<Pubkey> { fn compute_active_feature_set(&mut self, allow_new_activations: bool) -> HashSet<Pubkey> {
let mut active = self.feature_set.active.clone(); let mut active = self.feature_set.active.clone();
@ -5275,6 +5367,11 @@ impl Bank {
.is_active(&feature_set::consistent_recent_blockhashes_sysvar::id()), .is_active(&feature_set::consistent_recent_blockhashes_sysvar::id()),
} }
} }
fn rent_for_sysvars(&self) -> bool {
self.feature_set
.is_active(&feature_set::rent_for_sysvars::id())
}
} }
impl Drop for Bank { impl Drop for Bank {
@ -5887,6 +5984,19 @@ pub(crate) mod tests {
assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true));
} }
fn assert_capitalization_diff_with_new_bank(
bank: &Bank,
updater: impl Fn() -> Bank,
asserter: impl Fn(u64, u64),
) -> Bank {
let old = bank.capitalization();
let bank = updater();
let new = bank.capitalization();
asserter(old, new);
assert_eq!(bank.capitalization(), bank.calculate_capitalization());
bank
}
#[test] #[test]
fn test_store_account_and_update_capitalization_missing() { fn test_store_account_and_update_capitalization_missing() {
let (genesis_config, _mint_keypair) = create_genesis_config(0); let (genesis_config, _mint_keypair) = create_genesis_config(0);
@ -6174,7 +6284,9 @@ pub(crate) mod tests {
let current_capitalization = bank.capitalization.load(Relaxed); let current_capitalization = bank.capitalization.load(Relaxed);
let sysvar_and_native_proram_delta = 1; // only slot history is newly created
let sysvar_and_native_proram_delta =
min_rent_excempt_balance_for_sysvars(&bank, &[sysvar::slot_history::id()]);
assert_eq!( assert_eq!(
previous_capitalization - (current_capitalization - sysvar_and_native_proram_delta), previous_capitalization - (current_capitalization - sysvar_and_native_proram_delta),
burned_portion burned_portion
@ -7086,6 +7198,11 @@ pub(crate) mod tests {
.map(|(slot, _)| *slot) .map(|(slot, _)| *slot)
.collect::<Vec<Slot>>() .collect::<Vec<Slot>>()
} }
fn first_slot_in_next_epoch(&self) -> Slot {
self.epoch_schedule()
.get_first_slot_in_epoch(self.epoch() + 1)
}
} }
#[test] #[test]
@ -8657,7 +8774,7 @@ pub(crate) mod tests {
let (mut genesis_config, _mint_keypair) = create_genesis_config(500); let (mut genesis_config, _mint_keypair) = create_genesis_config(500);
let expected_previous_slot = 3; let expected_previous_slot = 3;
let expected_next_slot = expected_previous_slot + 1; let mut expected_next_slot = expected_previous_slot + 1;
// First, initialize the clock sysvar // First, initialize the clock sysvar
activate_all_features(&mut genesis_config); activate_all_features(&mut genesis_config);
@ -8688,7 +8805,10 @@ pub(crate) mod tests {
assert_eq!(dummy_rent_epoch, current_account.rent_epoch()); assert_eq!(dummy_rent_epoch, current_account.rent_epoch());
}, },
|old, new| { |old, new| {
assert_eq!(old + 1, new); assert_eq!(
old + min_rent_excempt_balance_for_sysvars(&bank1, &[sysvar::clock::id()]),
new
);
}, },
); );
@ -8696,7 +8816,7 @@ pub(crate) mod tests {
&bank1, &bank1,
|| { || {
bank1.update_sysvar_account(&dummy_clock_id, |optional_account| { bank1.update_sysvar_account(&dummy_clock_id, |optional_account| {
assert!(optional_account.is_none()); assert!(optional_account.is_some());
create_account( create_account(
&Clock { &Clock {
@ -8737,7 +8857,9 @@ pub(crate) mod tests {
expected_next_slot, expected_next_slot,
from_account::<Clock, _>(&current_account).unwrap().slot from_account::<Clock, _>(&current_account).unwrap().slot
); );
assert_eq!(INITIAL_RENT_EPOCH, current_account.rent_epoch()); // inherit_specially_retained_account_fields() now starts to inherit rent_epoch too
// with rent_for_sysvars
assert_eq!(dummy_rent_epoch, current_account.rent_epoch());
}, },
|old, new| { |old, new| {
// if existing, capitalization shouldn't change // if existing, capitalization shouldn't change
@ -8745,8 +8867,9 @@ pub(crate) mod tests {
}, },
); );
// Updating again should give bank1's sysvar to the closure not bank2's. // Updating again should give bank2's sysvar to the closure not bank1's.
// Thus, assert with same expected_next_slot as previously // Thus, increment expected_next_slot accordingly
expected_next_slot += 1;
assert_capitalization_diff( assert_capitalization_diff(
&bank2, &bank2,
|| { || {
@ -11599,6 +11722,253 @@ pub(crate) mod tests {
bank.store_account(vote_pubkey, &vote_account); bank.store_account(vote_pubkey, &vote_account);
} }
fn min_rent_excempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 {
sysvar_ids
.iter()
.map(|sysvar_id| {
trace!("min_rent_excempt_balance_for_sysvars: {}", sysvar_id);
bank.get_minimum_balance_for_rent_exemption(
bank.get_account(sysvar_id).unwrap().data().len(),
)
})
.sum()
}
fn expected_cap_delta_after_sysvar_reset(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 {
min_rent_excempt_balance_for_sysvars(bank, sysvar_ids) - sysvar_ids.len() as u64
}
#[test]
fn test_adjust_sysvar_balance_for_rent() {
let (genesis_config, _mint_keypair) = create_genesis_config(0);
let bank = Bank::new(&genesis_config);
let mut sample_sysvar = bank.get_account(&sysvar::clock::id()).unwrap();
assert_eq!(sample_sysvar.lamports(), 1);
bank.adjust_sysvar_balance_for_rent(&mut sample_sysvar);
let rent_exempt_balance_for_sysvar_clock = 1169280;
assert_eq!(
sample_sysvar.lamports(),
rent_exempt_balance_for_sysvar_clock
);
// excess lamports shouldn't be reduced by adjust_sysvar_balance_for_rent()
let excess_lamports = 9_999_999;
sample_sysvar.set_lamports(excess_lamports);
bank.adjust_sysvar_balance_for_rent(&mut sample_sysvar);
assert_eq!(sample_sysvar.lamports(), excess_lamports);
}
#[test]
fn test_no_deletion_due_to_rent_upon_rent_for_sysvar_activation() {
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config(0);
let feature_balance =
std::cmp::max(genesis_config.rent.minimum_balance(Feature::size_of()), 1);
// activate all features but rent_for_sysvars
activate_all_features(&mut genesis_config);
genesis_config
.accounts
.remove(&feature_set::rent_for_sysvars::id());
let bank0 = Bank::new(&genesis_config);
let bank1 = Arc::new(new_from_parent(&Arc::new(bank0)));
// schedule activation of simple capitalization
bank1.store_account_and_update_capitalization(
&feature_set::rent_for_sysvars::id(),
&feature::create_account(&Feature { activated_at: None }, feature_balance),
);
let bank2 =
Bank::new_from_parent(&bank1, &Pubkey::default(), bank1.first_slot_in_next_epoch());
assert_eq!(bank2.get_program_accounts(&sysvar::id()).len(), 8);
// force rent collection for sysvars
bank2.collect_rent_in_partition((0, 0, 1)); // all range
// no sysvar should be deleted due to rent
assert_eq!(bank2.get_program_accounts(&sysvar::id()).len(), 8);
}
#[test]
fn test_rent_for_sysvars_adjustment_minimum_genesis_set() {
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config(0);
let feature_balance =
std::cmp::max(genesis_config.rent.minimum_balance(Feature::size_of()), 1);
// inhibit deprecated rewards sysvar creation altogether
genesis_config.accounts.insert(
feature_set::deprecate_rewards_sysvar::id(),
Account::from(feature::create_account(
&Feature {
activated_at: Some(0),
},
feature_balance,
)),
);
let bank0 = Bank::new(&genesis_config);
let bank1 = Arc::new(new_from_parent(&Arc::new(bank0)));
// schedule activation of rent_for_sysvars
bank1.store_account_and_update_capitalization(
&feature_set::rent_for_sysvars::id(),
&feature::create_account(&Feature { activated_at: None }, feature_balance),
);
{
let sysvars = bank1.get_program_accounts(&sysvar::id());
assert_eq!(sysvars.len(), 8);
assert!(sysvars
.iter()
.map(|(_pubkey, account)| account.lamports())
.all(|l| l == 1));
}
// 8 sysvars should be reset by reset_all_sysvar_balances()
let bank2 = assert_capitalization_diff_with_new_bank(
&bank1,
|| Bank::new_from_parent(&bank1, &Pubkey::default(), bank1.first_slot_in_next_epoch()),
|old, new| {
assert_eq!(
old + expected_cap_delta_after_sysvar_reset(
&bank1,
&[
sysvar::clock::id(),
sysvar::epoch_schedule::id(),
sysvar::fees::id(),
sysvar::recent_blockhashes::id(),
sysvar::rent::id(),
sysvar::slot_hashes::id(),
sysvar::slot_history::id(),
sysvar::stake_history::id(),
]
),
new
)
},
);
{
let sysvars = bank2.get_program_accounts(&sysvar::id());
assert_eq!(sysvars.len(), 8);
assert!(sysvars
.iter()
.map(|(_pubkey, account)| account.lamports())
.all(|l| l > 1));
}
}
#[test]
fn test_rent_for_sysvars_adjustment_full_set() {
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config(0);
let feature_balance =
std::cmp::max(genesis_config.rent.minimum_balance(Feature::size_of()), 1);
// activate all features but rent_for_sysvars
activate_all_features(&mut genesis_config);
genesis_config
.accounts
.remove(&feature_set::rent_for_sysvars::id());
// intentionally create deprecated rewards sysvar creation
genesis_config
.accounts
.remove(&feature_set::deprecate_rewards_sysvar::id());
// intentionally create bogus native programs
#[allow(clippy::unnecessary_wraps)]
fn mock_process_instruction(
_program_id: &Pubkey,
_data: &[u8],
_invoke_context: &mut dyn InvokeContext,
) -> std::result::Result<(), solana_sdk::instruction::InstructionError> {
Ok(())
}
let builtins = Builtins {
genesis_builtins: vec![
Builtin::new(
"mock bpf",
solana_sdk::bpf_loader::id(),
mock_process_instruction,
),
Builtin::new(
"mock bpf",
solana_sdk::bpf_loader_deprecated::id(),
mock_process_instruction,
),
],
feature_builtins: (vec![]),
};
let bank0 = Arc::new(Bank::new_with_paths(
&genesis_config,
Vec::new(),
&[],
None,
Some(&builtins),
AccountSecondaryIndexes::default(),
false,
));
// move to next epoch to create now deprecated rewards sysvar intentionally
let bank1 = Arc::new(Bank::new_from_parent(
&bank0,
&Pubkey::default(),
bank0.first_slot_in_next_epoch(),
));
// schedule activation of simple capitalization
bank1.store_account_and_update_capitalization(
&feature_set::rent_for_sysvars::id(),
&feature::create_account(&Feature { activated_at: None }, feature_balance),
);
{
let sysvars = bank1.get_program_accounts(&sysvar::id());
assert_eq!(sysvars.len(), 9);
assert!(sysvars
.iter()
.map(|(_pubkey, account)| account.lamports())
.all(|l| l == 1));
}
// 9 sysvars should be reset by reset_all_sysvar_balances()
let bank2 = assert_capitalization_diff_with_new_bank(
&bank1,
|| Bank::new_from_parent(&bank1, &Pubkey::default(), bank1.first_slot_in_next_epoch()),
|old, new| {
assert_eq!(
old + expected_cap_delta_after_sysvar_reset(
&bank1,
&[
sysvar::clock::id(),
sysvar::epoch_schedule::id(),
sysvar::fees::id(),
sysvar::recent_blockhashes::id(),
sysvar::rent::id(),
sysvar::rewards::id(),
sysvar::slot_hashes::id(),
sysvar::slot_history::id(),
sysvar::stake_history::id(),
]
),
new
)
},
);
{
let sysvars = bank2.get_program_accounts(&sysvar::id());
assert_eq!(sysvars.len(), 9);
assert!(sysvars
.iter()
.map(|(_pubkey, account)| account.lamports())
.all(|l| l > 1));
}
}
#[test] #[test]
fn test_update_clock_timestamp() { fn test_update_clock_timestamp() {
let leader_pubkey = solana_sdk::pubkey::new_rand(); let leader_pubkey = solana_sdk::pubkey::new_rand();

View File

@ -60,10 +60,11 @@ impl RentCollector {
&self, &self,
address: &Pubkey, address: &Pubkey,
account: &mut AccountSharedData, account: &mut AccountSharedData,
rent_for_sysvars: bool,
) -> u64 { ) -> u64 {
if account.executable() // executable accounts must be rent-exempt balance if account.executable() // executable accounts must be rent-exempt balance
|| account.rent_epoch() > self.epoch || account.rent_epoch() > self.epoch
|| sysvar::check_id(account.owner()) || (!rent_for_sysvars && sysvar::check_id(&account.owner()))
|| *address == incinerator::id() || *address == incinerator::id()
{ {
0 0
@ -115,10 +116,11 @@ impl RentCollector {
&self, &self,
address: &Pubkey, address: &Pubkey,
account: &mut AccountSharedData, account: &mut AccountSharedData,
rent_for_sysvars: bool,
) -> u64 { ) -> u64 {
// initialize rent_epoch as created at this epoch // initialize rent_epoch as created at this epoch
account.set_rent_epoch(self.epoch); account.set_rent_epoch(self.epoch);
self.collect_from_existing_account(address, account) self.collect_from_existing_account(address, account, rent_for_sysvars)
} }
} }
@ -146,15 +148,21 @@ mod tests {
let rent_collector = RentCollector::default().clone_with_epoch(new_epoch); let rent_collector = RentCollector::default().clone_with_epoch(new_epoch);
// collect rent on a newly-created account // collect rent on a newly-created account
let collected = rent_collector let collected = rent_collector.collect_from_created_account(
.collect_from_created_account(&solana_sdk::pubkey::new_rand(), &mut created_account); &solana_sdk::pubkey::new_rand(),
&mut created_account,
true,
);
assert!(created_account.lamports() < old_lamports); assert!(created_account.lamports() < old_lamports);
assert_eq!(created_account.lamports() + collected, old_lamports); assert_eq!(created_account.lamports() + collected, old_lamports);
assert_ne!(created_account.rent_epoch(), old_epoch); assert_ne!(created_account.rent_epoch(), old_epoch);
// collect rent on a already-existing account // collect rent on a already-existing account
let collected = rent_collector let collected = rent_collector.collect_from_existing_account(
.collect_from_existing_account(&solana_sdk::pubkey::new_rand(), &mut existing_account); &solana_sdk::pubkey::new_rand(),
&mut existing_account,
true,
);
assert!(existing_account.lamports() < old_lamports); assert!(existing_account.lamports() < old_lamports);
assert_eq!(existing_account.lamports() + collected, old_lamports); assert_eq!(existing_account.lamports() + collected, old_lamports);
assert_ne!(existing_account.rent_epoch(), old_epoch); assert_ne!(existing_account.rent_epoch(), old_epoch);
@ -180,7 +188,7 @@ mod tests {
let rent_collector = RentCollector::default().clone_with_epoch(epoch); let rent_collector = RentCollector::default().clone_with_epoch(epoch);
// first mark account as being collected while being rent-exempt // first mark account as being collected while being rent-exempt
collected = rent_collector.collect_from_existing_account(&pubkey, &mut account); collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, true);
assert_eq!(account.lamports(), huge_lamports); assert_eq!(account.lamports(), huge_lamports);
assert_eq!(collected, 0); assert_eq!(collected, 0);
@ -188,8 +196,33 @@ mod tests {
account.set_lamports(tiny_lamports); account.set_lamports(tiny_lamports);
// ... and trigger another rent collection on the same epoch and check that rent is working // ... and trigger another rent collection on the same epoch and check that rent is working
collected = rent_collector.collect_from_existing_account(&pubkey, &mut account); collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, true);
assert_eq!(account.lamports(), tiny_lamports - collected); assert_eq!(account.lamports(), tiny_lamports - collected);
assert_ne!(collected, 0); assert_ne!(collected, 0);
} }
#[test]
fn test_rent_exempt_sysvar() {
let tiny_lamports = 1;
let mut account = AccountSharedData::default();
account.set_owner(sysvar::id());
account.set_lamports(tiny_lamports);
let pubkey = solana_sdk::pubkey::new_rand();
assert_eq!(account.rent_epoch(), 0);
let epoch = 3;
let rent_collector = RentCollector::default().clone_with_epoch(epoch);
// old behavior: sysvars are special-cased
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, false);
assert_eq!(account.lamports(), tiny_lamports);
assert_eq!(collected, 0);
// new behavior: sysvars are NOT special-cased
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, true);
assert_eq!(account.lamports(), 0);
assert_eq!(collected, 1);
}
} }

View File

@ -120,8 +120,13 @@ fn assign(
return Err(InstructionError::MissingRequiredSignature); return Err(InstructionError::MissingRequiredSignature);
} }
// bpf programs are allowed to do this; so this is inconsistent...
// Thus, we're starting to remove this restricition from system instruction
// processor for consistency and fewer special casing by piggybacking onto
// the related feature gate..
let rent_for_sysvars = invoke_context.is_feature_active(&feature_set::rent_for_sysvars::id());
if !rent_for_sysvars && sysvar::check_id(owner) {
// guard against sysvars being made // guard against sysvars being made
if sysvar::check_id(owner) {
ic_msg!(invoke_context, "Assign: cannot assign to sysvar, {}", owner); ic_msg!(invoke_context, "Assign: cannot assign to sysvar, {}", owner);
return Err(SystemError::InvalidProgramId.into()); return Err(SystemError::InvalidProgramId.into());
} }
@ -890,7 +895,7 @@ mod tests {
} }
#[test] #[test]
fn test_create_sysvar_invalid_id() { fn test_create_sysvar_invalid_id_with_feature() {
// Attempt to create system account in account already owned by another program // Attempt to create system account in account already owned by another program
let from = solana_sdk::pubkey::new_rand(); let from = solana_sdk::pubkey::new_rand();
let from_account = AccountSharedData::new_ref(100, 0, &system_program::id()); let from_account = AccountSharedData::new_ref(100, 0, &system_program::id());
@ -912,7 +917,34 @@ mod tests {
&signers, &signers,
&MockInvokeContext::new(vec![]), &MockInvokeContext::new(vec![]),
); );
assert_eq!(result, Ok(()));
}
#[test]
fn test_create_sysvar_invalid_id_without_feature() {
// Attempt to create system account in account already owned by another program
let from = solana_sdk::pubkey::new_rand();
let from_account = AccountSharedData::new_ref(100, 0, &system_program::id());
let to = solana_sdk::pubkey::new_rand();
let to_account = AccountSharedData::new_ref(0, 0, &system_program::id());
let signers = [from, to].iter().cloned().collect::<HashSet<_>>();
let to_address = to.into();
let result = create_account(
&KeyedAccount::new(&from, true, &from_account),
&KeyedAccount::new(&to, false, &to_account),
&to_address,
50,
2,
&sysvar::id(),
&signers,
&MockInvokeContext {
feature_active: false,
..MockInvokeContext::new(vec![])
},
);
assert_eq!(result, Err(SystemError::InvalidProgramId.into())); assert_eq!(result, Err(SystemError::InvalidProgramId.into()));
} }
@ -1025,7 +1057,7 @@ mod tests {
} }
#[test] #[test]
fn test_assign_to_sysvar() { fn test_assign_to_sysvar_with_feature() {
let new_owner = sysvar::id(); let new_owner = sysvar::id();
let from = solana_sdk::pubkey::new_rand(); let from = solana_sdk::pubkey::new_rand();
@ -1039,6 +1071,28 @@ mod tests {
&[from].iter().cloned().collect::<HashSet<_>>(), &[from].iter().cloned().collect::<HashSet<_>>(),
&MockInvokeContext::new(vec![]), &MockInvokeContext::new(vec![]),
), ),
Ok(())
);
}
#[test]
fn test_assign_to_sysvar_without_feature() {
let new_owner = sysvar::id();
let from = solana_sdk::pubkey::new_rand();
let mut from_account = AccountSharedData::new(100, 0, &system_program::id());
assert_eq!(
assign(
&mut from_account,
&from.into(),
&new_owner,
&[from].iter().cloned().collect::<HashSet<_>>(),
&MockInvokeContext {
feature_active: false,
..MockInvokeContext::new(vec![])
},
),
Err(SystemError::InvalidProgramId.into()) Err(SystemError::InvalidProgramId.into())
); );
} }

View File

@ -163,6 +163,10 @@ pub mod neon_evm_compute_budget {
solana_sdk::declare_id!("GLrVvDPkQi5PMYUrsYWT9doZhSHr1BVZXqj5DbFps3rS"); solana_sdk::declare_id!("GLrVvDPkQi5PMYUrsYWT9doZhSHr1BVZXqj5DbFps3rS");
} }
pub mod rent_for_sysvars {
solana_sdk::declare_id!("BKCPBQQBZqggVnFso5nQ8rQ4RwwogYwjuUt9biBjxwNF");
}
lazy_static! { lazy_static! {
/// Map of feature identifiers to user-visible description /// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [ pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -203,6 +207,7 @@ lazy_static! {
(vote_stake_checked_instructions::id(), "vote/state program checked instructions #18345"), (vote_stake_checked_instructions::id(), "vote/state program checked instructions #18345"),
(updated_verify_policy::id(), "Update verify policy"), (updated_verify_policy::id(), "Update verify policy"),
(neon_evm_compute_budget::id(), "bump neon_evm's compute budget"), (neon_evm_compute_budget::id(), "bump neon_evm's compute budget"),
(rent_for_sysvars::id(), "collect rent from accounts owned by sysvars"),
/*************** ADD NEW FEATURES HERE ***************/ /*************** ADD NEW FEATURES HERE ***************/
] ]
.iter() .iter()

View File

@ -334,6 +334,7 @@ pub struct MockInvokeContext<'a> {
pub programs: Vec<(Pubkey, ProcessInstructionWithContext)>, pub programs: Vec<(Pubkey, ProcessInstructionWithContext)>,
pub accounts: Vec<(Pubkey, Rc<RefCell<AccountSharedData>>)>, pub accounts: Vec<(Pubkey, Rc<RefCell<AccountSharedData>>)>,
pub sysvars: Vec<(Pubkey, Option<Rc<Vec<u8>>>)>, pub sysvars: Vec<(Pubkey, Option<Rc<Vec<u8>>>)>,
pub feature_active: bool,
} }
impl<'a> MockInvokeContext<'a> { impl<'a> MockInvokeContext<'a> {
pub fn new(keyed_accounts: Vec<KeyedAccount<'a>>) -> Self { pub fn new(keyed_accounts: Vec<KeyedAccount<'a>>) -> Self {
@ -348,6 +349,7 @@ impl<'a> MockInvokeContext<'a> {
programs: vec![], programs: vec![],
accounts: vec![], accounts: vec![],
sysvars: vec![], sysvars: vec![],
feature_active: true,
}; };
invoke_context invoke_context
.invoke_stack .invoke_stack
@ -442,7 +444,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
} }
fn record_instruction(&self, _instruction: &Instruction) {} fn record_instruction(&self, _instruction: &Instruction) {}
fn is_feature_active(&self, _feature_id: &Pubkey) -> bool { fn is_feature_active(&self, _feature_id: &Pubkey) -> bool {
true self.feature_active
} }
fn get_account(&self, pubkey: &Pubkey) -> Option<Rc<RefCell<AccountSharedData>>> { fn get_account(&self, pubkey: &Pubkey) -> Option<Rc<RefCell<AccountSharedData>>> {
for (key, account) in self.accounts.iter() { for (key, account) in self.accounts.iter() {