blockworks-foundation
/
solana
mirror of https://github.com/blockworks-foundation/solana.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Port various rent fixes to runtime feature (#12842) 

* Port various rent fixes to runtime feature

* Fix CI

* Use more consistent naming...
This commit is contained in:
Ryo Onodera 2020-10-21 22:19:25 +09:00 committed by GitHub
parent 02bf9ca834
commit 608b81b412
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 84 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
runtime/src/accounts.rs
View File

@ -144,6 +144,8 @@ impl Accounts {
let mut payer_index = None;
let mut tx_rent: TransactionRent = 0;
let mut accounts = Vec::with_capacity(message.account_keys.len());
let rent_fix_enabled = feature_set.cumulative_rent_related_fixes_enabled();
for (i, key) in message.account_keys.iter().enumerate() {
let account = if Self::is_non_loader_key(message, key, i) {
if payer_index.is_none() {
@ -163,7 +165,11 @@ impl Accounts {
.map(|(mut account, _)| {
if message.is_writable(i) {
let rent_due = rent_collector
.collect_from_existing_account(&key, &mut account);
.collect_from_existing_account(
&key,
&mut account,
rent_fix_enabled,
);
(account, rent_due)
} else {
(account, 0)
@ -720,6 +726,8 @@ impl Accounts {
}
/// Store the accounts into the DB
// allow(clippy) needed for various gating flags
#[allow(clippy::too_many_arguments)]
pub fn store_accounts(
&self,
slot: Slot,
@ -730,6 +738,7 @@ impl Accounts {
rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool,
rent_fix_enabled: bool,
) {
let accounts_to_store = self.collect_accounts_to_store(
txs,
@ -739,6 +748,7 @@ impl Accounts {
rent_collector,
last_blockhash_with_fee_calculator,
fix_recent_blockhashes_sysvar_delay,
rent_fix_enabled,
);
self.accounts_db.store(slot, &accounts_to_store);
}
@ -766,6 +776,7 @@ impl Accounts {
rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool,
rent_fix_enabled: bool,
) -> Vec<(&'a Pubkey, &'a Account)> {
let mut accounts = Vec::with_capacity(loaded.len());
for (i, ((raccs, _hash_age_kind), (_, tx))) in loaded
@ -806,7 +817,11 @@ impl Accounts {
);
if message.is_writable(i) {
if account.rent_epoch == 0 {
acc.2 += rent_collector.collect_from_created_account(&key, account);
acc.2 += rent_collector.collect_from_created_account(
&key,
account,
rent_fix_enabled,
);
}
accounts.push((key, &*account));
}
@ -1122,7 +1137,6 @@ mod tests {
lamports_per_byte_year: 42,
..Rent::default()
},
ClusterType::Development,
);
let min_balance = rent_collector.rent.minimum_balance(nonce::State::size());
let fee_calculator = FeeCalculator::new(min_balance);
@ -1799,6 +1813,7 @@ mod tests {
&rent_collector,
&(Hash::default(), FeeCalculator::default()),
true,
true,
);
assert_eq!(collected_accounts.len(), 2);
assert!(collected_accounts

58
runtime/src/bank.rs
View File

@ -807,9 +807,7 @@ impl Bank {
slots_per_year: parent.slots_per_year,
epoch_schedule,
collected_rent: AtomicU64::new(0),
rent_collector: parent
.rent_collector
.clone_with_epoch(epoch, parent.cluster_type()),
rent_collector: parent.rent_collector.clone_with_epoch(epoch),
max_tick_height: (slot + 1) * parent.ticks_per_slot,
block_height: parent.block_height + 1,
fee_calculator: fee_rate_governor.create_fee_calculator(),
@ -927,9 +925,7 @@ impl Bank {
fee_rate_governor: fields.fee_rate_governor,
collected_rent: AtomicU64::new(fields.collected_rent),
// clone()-ing is needed to consider a gated behavior in rent_collector
rent_collector: fields
.rent_collector
.clone_with_epoch(fields.epoch, genesis_config.cluster_type),
rent_collector: fields.rent_collector.clone_with_epoch(fields.epoch),
epoch_schedule: fields.epoch_schedule,
inflation: Arc::new(RwLock::new(fields.inflation)),
stakes: RwLock::new(fields.stakes),
@ -1668,7 +1664,6 @@ impl Bank {
&self.epoch_schedule,
self.slots_per_year,
&genesis_config.rent,
self.cluster_type(),
);
// Add additional native programs specified in the genesis config
@ -2591,6 +2586,7 @@ impl Bank {
&self.rent_collector,
&self.last_blockhash_with_fee_calculator(),
self.fix_recent_blockhashes_sysvar_delay(),
self.cumulative_rent_related_fixes_enabled(),
);
self.collect_rent(executed, loaded_accounts);
@ -2834,9 +2830,11 @@ impl Bank {
// parallelize?
let mut rent = 0;
for (pubkey, mut account) in accounts {
rent += self
.rent_collector
.collect_from_existing_account(&pubkey, &mut account);
rent += self.rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
self.cumulative_rent_related_fixes_enabled(),
);
// Store all of them unconditionally to purge old AppendVec,
// even if collected rent is 0 (= not updated).
self.store_account(&pubkey, &account);
@ -2954,12 +2952,9 @@ impl Bank {
self.get_epoch_and_slot_index(self.parent_slot());
let should_enable = match self.cluster_type() {
ClusterType::Development => true,
ClusterType::Devnet => true,
ClusterType::Testnet => current_epoch >= 97,
ClusterType::MainnetBeta => {
#[cfg(not(test))]
let should_enable = current_epoch >= Epoch::max_value();
let should_enable = self.cumulative_rent_related_fixes_enabled();
// needed for test_rent_eager_across_epoch_with_gap_under_multi_epoch_cycle,
// which depends on ClusterType::MainnetBeta
@ -2968,6 +2963,7 @@ impl Bank {
should_enable
}
_ => self.cumulative_rent_related_fixes_enabled(),
};
let mut partitions = vec![];
@ -3314,21 +3310,19 @@ impl Bank {
pub fn deposit(&self, pubkey: &Pubkey, lamports: u64) -> u64 {
let mut account = self.get_account(pubkey).unwrap_or_default();
let should_be_in_new_behavior = match self.cluster_type() {
ClusterType::Development => true,
ClusterType::Devnet => true,
ClusterType::Testnet => self.epoch() >= 97,
ClusterType::MainnetBeta => self.epoch() >= Epoch::max_value(),
};
let rent_fix_enabled = self.cumulative_rent_related_fixes_enabled();
// don't collect rents if we're in the new behavior;
// in genral, it's not worthwhile to account for rents outside the runtime (transactions)
// there are too many and subtly nuanced modification codepaths
if !should_be_in_new_behavior {
if !rent_fix_enabled {
// previously we're too much collecting rents as if it existed since epoch 0...
self.collected_rent.fetch_add(
self.rent_collector
.collect_from_existing_account(pubkey, &mut account),
self.rent_collector.collect_from_existing_account(
pubkey,
&mut account,
rent_fix_enabled,
),
Relaxed,
);
}
@ -3895,6 +3889,10 @@ impl Bank {
.is_active(&feature_set::no_overflow_rent_distribution::id())
}
pub fn cumulative_rent_related_fixes_enabled(&self) -> bool {
self.feature_set.cumulative_rent_related_fixes_enabled()
}
// This is called from snapshot restore AND for each epoch boundary
// The entire code path herein must be idempotent
fn apply_feature_activations(&mut self, init_finish_or_warp: bool) {
@ -4100,8 +4098,9 @@ mod tests {
use crate::{
accounts_index::{AccountMap, Ancestors},
genesis_utils::{
create_genesis_config_with_leader, create_genesis_config_with_vote_accounts,
GenesisConfigInfo, ValidatorVoteKeypairs, BOOTSTRAP_VALIDATOR_LAMPORTS,
activate_all_features, create_genesis_config_with_leader,
create_genesis_config_with_vote_accounts, GenesisConfigInfo, ValidatorVoteKeypairs,
BOOTSTRAP_VALIDATOR_LAMPORTS,
},
native_loader::NativeLoaderError,
process_instruction::InvokeContext,
@ -5071,7 +5070,8 @@ mod tests {
#[test]
fn test_rent_eager_across_epoch_with_full_gap() {
let (genesis_config, _mint_keypair) = create_genesis_config(1);
let (mut genesis_config, _mint_keypair) = create_genesis_config(1);
activate_all_features(&mut genesis_config);
let mut bank = Arc::new(Bank::new(&genesis_config));
assert_eq!(bank.rent_collection_partitions(), vec![(0, 0, 32)]);
@ -5093,7 +5093,8 @@ mod tests {
#[test]
fn test_rent_eager_across_epoch_with_half_gap() {
let (genesis_config, _mint_keypair) = create_genesis_config(1);
let (mut genesis_config, _mint_keypair) = create_genesis_config(1);
activate_all_features(&mut genesis_config);
let mut bank = Arc::new(Bank::new(&genesis_config));
assert_eq!(bank.rent_collection_partitions(), vec![(0, 0, 32)]);
@ -5641,7 +5642,8 @@ mod tests {
fn test_rent_eager_collect_rent_in_partition() {
solana_logger::setup();
let (genesis_config, _mint_keypair) = create_genesis_config(1);
let (mut genesis_config, _mint_keypair) = create_genesis_config(1);
activate_all_features(&mut genesis_config);
let zero_lamport_pubkey = Pubkey::new_rand();
let rent_due_pubkey = Pubkey::new_rand();

5
runtime/src/feature_set.rs
View File

@ -130,6 +130,11 @@ impl FeatureSet {
pub fn is_active(&self, feature_id: &Pubkey) -> bool {
self.active.contains(feature_id)
}
pub fn cumulative_rent_related_fixes_enabled(&self) -> bool {
self.is_active(&cumulative_rent_related_fixes::id())
}
/// All features enabled, useful for testing
pub fn all_enabled() -> Self {
Self {

72
runtime/src/rent_collector.rs
View File

@ -1,13 +1,7 @@
//! calculate and collect rent from Accounts
use solana_sdk::{
account::Account,
clock::Epoch,
epoch_schedule::EpochSchedule,
genesis_config::{ClusterType, GenesisConfig},
incinerator,
pubkey::Pubkey,
rent::Rent,
sysvar,
account::Account, clock::Epoch, epoch_schedule::EpochSchedule, genesis_config::GenesisConfig,
incinerator, pubkey::Pubkey, rent::Rent, sysvar,
};
#[derive(Serialize, Deserialize, Clone, PartialEq, Debug, AbiExample)]
@ -16,11 +10,6 @@ pub struct RentCollector {
pub epoch_schedule: EpochSchedule,
pub slots_per_year: f64,
pub rent: Rent,
// serde(skip) is needed not to break abi
// Also, wrap this with Option so that we can spot any uninitialized codepath (like
// snapshot restore)
#[serde(skip)]
pub cluster_type: Option<ClusterType>,
}
impl Default for RentCollector {
@ -31,7 +20,6 @@ impl Default for RentCollector {
// derive default value using GenesisConfig::default()
slots_per_year: GenesisConfig::default().slots_per_year(),
rent: Rent::default(),
cluster_type: Option::default(),
}
}
}
@ -42,39 +30,32 @@ impl RentCollector {
epoch_schedule: &EpochSchedule,
slots_per_year: f64,
rent: &Rent,
cluster_type: ClusterType,
) -> Self {
Self {
epoch,
epoch_schedule: *epoch_schedule,
slots_per_year,
rent: *rent,
cluster_type: Some(cluster_type),
}
}
pub fn clone_with_epoch(&self, epoch: Epoch, cluster_type: ClusterType) -> Self {
pub fn clone_with_epoch(&self, epoch: Epoch) -> Self {
Self {
epoch,
cluster_type: Some(cluster_type),
..self.clone()
}
}
fn enable_new_behavior(&self) -> bool {
match self.cluster_type.unwrap() {
ClusterType::Development => true,
ClusterType::Devnet => true,
ClusterType::Testnet => self.epoch >= 97,
ClusterType::MainnetBeta => self.epoch >= Epoch::max_value(),
}
}
// updates this account's lamports and status and returns
// the account rent collected, if any
//
#[must_use = "add to Bank::collected_rent"]
pub fn collect_from_existing_account(&self, address: &Pubkey, account: &mut Account) -> u64 {
pub fn collect_from_existing_account(
&self,
address: &Pubkey,
account: &mut Account,
rent_fix_enabled: bool,
) -> u64 {
if account.executable
|| account.rent_epoch > self.epoch
|| sysvar::check_id(&account.owner)
@ -100,7 +81,7 @@ impl RentCollector {
if exempt || rent_due != 0 {
if account.lamports > rent_due {
account.rent_epoch = self.epoch
+ if self.enable_new_behavior() && exempt {
+ if rent_fix_enabled && exempt {
// Rent isn't collected for the next epoch
// Make sure to check exempt status later in curent epoch again
0
@ -123,10 +104,15 @@ impl RentCollector {
}
#[must_use = "add to Bank::collected_rent"]
pub fn collect_from_created_account(&self, address: &Pubkey, account: &mut Account) -> u64 {
pub fn collect_from_created_account(
&self,
address: &Pubkey,
account: &mut Account,
enable_new_behavior: bool,
) -> u64 {
// initialize rent_epoch as created at this epoch
account.rent_epoch = self.epoch;
self.collect_from_existing_account(address, account)
self.collect_from_existing_account(address, account, enable_new_behavior)
}
}
@ -148,19 +134,24 @@ mod tests {
(account.clone(), account)
};
let rent_collector =
RentCollector::default().clone_with_epoch(new_epoch, ClusterType::Development);
let rent_collector = RentCollector::default().clone_with_epoch(new_epoch);
// collect rent on a newly-created account
let collected =
rent_collector.collect_from_created_account(&Pubkey::new_rand(), &mut created_account);
let collected = rent_collector.collect_from_created_account(
&Pubkey::new_rand(),
&mut created_account,
true,
);
assert!(created_account.lamports < old_lamports);
assert_eq!(created_account.lamports + collected, old_lamports);
assert_ne!(created_account.rent_epoch, old_epoch);
// collect rent on a already-existing account
let collected = rent_collector
.collect_from_existing_account(&Pubkey::new_rand(), &mut existing_account);
let collected = rent_collector.collect_from_existing_account(
&Pubkey::new_rand(),
&mut existing_account,
true,
);
assert!(existing_account.lamports < old_lamports);
assert_eq!(existing_account.lamports + collected, old_lamports);
assert_ne!(existing_account.rent_epoch, old_epoch);
@ -183,11 +174,10 @@ mod tests {
assert_eq!(account.rent_epoch, 0);
// create a tested rent collector
let rent_collector =
RentCollector::default().clone_with_epoch(epoch, ClusterType::Development);
let rent_collector = RentCollector::default().clone_with_epoch(epoch);
// 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!(collected, 0);
@ -195,7 +185,7 @@ mod tests {
account.lamports = tiny_lamports;
// ... 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_ne!(collected, 0);
}