gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Re-do rent collection check on rent-exempt account (#11349)

Browse Source 
* wip: re-do rent collection check on rent-exempt account

* Let's see how the ci goes

* Restore previous code

* Well, almost all new changes are revertable

* Update doc

* Add test and gating

* Fix tests

* Fix tests, especially avoid to change abi...

* Fix more tests...

* Fix snapshot restore

* Align to _new_ with better uninitialized detection
This commit is contained in:
Ryo Onodera
2020-08-17 14:22:16 +09:00
committed by GitHub
parent 6c5b8f324a
commit 23fa84b322
9 changed files with 138 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
runtime/src/accounts.rs
View File

@ -778,6 +778,7 @@ mod tests {
account::Account,
epoch_schedule::EpochSchedule,
fee_calculator::FeeCalculator,
genesis_config::OperatingMode,
hash::Hash,
instruction::CompiledInstruction,
message::Message,
@ -1011,6 +1012,7 @@ mod tests {
lamports_per_byte_year: 42,
..Rent::default()
},
OperatingMode::Development,
);
let min_balance = rent_collector.rent.minimum_balance(nonce::State::size());
let fee_calculator = FeeCalculator::new(min_balance);

19
runtime/src/bank.rs
View File

@ -519,7 +519,9 @@ 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),
rent_collector: parent
.rent_collector
.clone_with_epoch(epoch, parent.operating_mode()),
max_tick_height: (slot + 1) * parent.ticks_per_slot,
block_height: parent.block_height + 1,
fee_calculator: fee_rate_governor.create_fee_calculator(),
@ -640,7 +642,10 @@ impl Bank {
fee_calculator: fields.fee_calculator,
fee_rate_governor: fields.fee_rate_governor,
collected_rent: AtomicU64::new(fields.collected_rent),
rent_collector: fields.rent_collector,
// clone()-ing is needed to consider a gated behavior in rent_collector
rent_collector: fields
.rent_collector
.clone_with_epoch(fields.epoch, genesis_config.operating_mode),
epoch_schedule: fields.epoch_schedule,
inflation: Arc::new(RwLock::new(fields.inflation)),
stakes: RwLock::new(fields.stakes),
@ -689,6 +694,7 @@ impl Bank {
&bank.epoch_schedule,
bank.slots_per_year,
&genesis_config.rent,
genesis_config.operating_mode,
)
);
@ -1198,6 +1204,7 @@ impl Bank {
&self.epoch_schedule,
self.slots_per_year,
&genesis_config.rent,
self.operating_mode(),
);
// Add additional native programs specified in the genesis config
@ -4748,7 +4755,7 @@ mod tests {
bank.get_account(&rent_exempt_pubkey).unwrap().lamports,
large_lamports
);
assert_eq!(bank.get_account(&rent_exempt_pubkey).unwrap().rent_epoch, 6);
assert_eq!(bank.get_account(&rent_exempt_pubkey).unwrap().rent_epoch, 5);
assert_eq!(
bank.slots_by_pubkey(&rent_due_pubkey, &ancestors),
vec![genesis_slot, some_slot]
@ -7889,19 +7896,19 @@ mod tests {
if bank.slot == 32 {
assert_eq!(
bank.hash().to_string(),
"5yZDar5HaXypoeNnE9mEfVwJEEyDCP5W1pLwhuouPjqN"
"GDH7kUpcQuMT23pPeU9vZdmyMSPQPwzoqdNgFaLga7x3"
);
}
if bank.slot == 64 {
assert_eq!(
bank.hash().to_string(),
"FmPBRC6AAZgXu1QiqZ445FhLYZXun9KTtjMMKqCim9Hd"
"J4L6bT3KnMMXSufcUSy6Lg9TNi2pFVsYNvQ1Fzms2j1Z"
);
}
if bank.slot == 128 {
assert_eq!(
bank.hash().to_string(),
"Aqi2QcSoxaYu2QJHKaMHi9G8J2vNEtjpuKzjj5rHP9wr"
"BiCUyj8PsbsLW79waf1ifr3wDuZSFwLBhTkdbgHFjrtJ"
);
break;
}

76
runtime/src/rent_collector.rs
View File

@ -1,7 +1,13 @@
//! calculate and collect rent from Accounts
use solana_sdk::{
account::Account, clock::Epoch, epoch_schedule::EpochSchedule, genesis_config::GenesisConfig,
incinerator, pubkey::Pubkey, rent::Rent, sysvar,
account::Account,
clock::Epoch,
epoch_schedule::EpochSchedule,
genesis_config::{GenesisConfig, OperatingMode},
incinerator,
pubkey::Pubkey,
rent::Rent,
sysvar,
};
#[derive(Serialize, Deserialize, Clone, PartialEq, Debug, AbiExample)]
@ -10,6 +16,11 @@ 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 operating_mode: Option<OperatingMode>,
}
impl Default for RentCollector {
@ -20,6 +31,7 @@ impl Default for RentCollector {
// derive default value using GenesisConfig::default()
slots_per_year: GenesisConfig::default().slots_per_year(),
rent: Rent::default(),
operating_mode: Option::default(),
}
}
}
@ -30,21 +42,33 @@ impl RentCollector {
epoch_schedule: &EpochSchedule,
slots_per_year: f64,
rent: &Rent,
operating_mode: OperatingMode,
) -> Self {
Self {
epoch,
epoch_schedule: *epoch_schedule,
slots_per_year,
rent: *rent,
operating_mode: Some(operating_mode),
}
}
pub fn clone_with_epoch(&self, epoch: Epoch) -> Self {
pub fn clone_with_epoch(&self, epoch: Epoch, operating_mode: OperatingMode) -> Self {
Self {
epoch,
operating_mode: Some(operating_mode),
..self.clone()
}
}
fn enable_new_behavior(&self) -> bool {
match self.operating_mode.unwrap() {
OperatingMode::Development => true,
OperatingMode::Preview => self.epoch >= Epoch::max_value(),
OperatingMode::Stable => self.epoch >= Epoch::max_value(),
}
}
// updates this account's lamports and status and returns
// the account rent collected, if any
//
@ -74,7 +98,15 @@ impl RentCollector {
if exempt || rent_due != 0 {
if account.lamports > rent_due {
account.rent_epoch = self.epoch + 1;
account.rent_epoch = self.epoch
+ if self.enable_new_behavior() && exempt {
// Rent isn't collected for the next epoch
// Make sure to check exempt status later in curent epoch again
0
} else {
// Rent is collected for next epoch
1
};
account.lamports -= rent_due;
rent_due
} else {
@ -115,21 +147,55 @@ mod tests {
(account.clone(), account)
};
let rent_collector = RentCollector::default().clone_with_epoch(new_epoch);
let rent_collector =
RentCollector::default().clone_with_epoch(new_epoch, OperatingMode::Development);
// collect rent on a newly-created account
let collected =
rent_collector.collect_from_created_account(&Pubkey::new_rand(), &mut created_account);
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);
assert!(existing_account.lamports < old_lamports);
assert_eq!(existing_account.lamports + collected, old_lamports);
assert_ne!(existing_account.rent_epoch, old_epoch);
// newly created account should be collected for less rent; thus more remaining balance
assert!(created_account.lamports > existing_account.lamports);
assert_eq!(created_account.rent_epoch, existing_account.rent_epoch);
}
#[test]
fn test_rent_exempt_temporal_escape() {
let mut account = Account::default();
let epoch = 3;
let huge_lamports = 123_456_789_012;
let tiny_lamports = 789_012;
let mut collected;
let pubkey = Pubkey::new_rand();
account.lamports = huge_lamports;
assert_eq!(account.rent_epoch, 0);
// create a tested rent collector
let rent_collector =
RentCollector::default().clone_with_epoch(epoch, OperatingMode::Development);
// first mark account as being collected while being rent-exempt
collected = rent_collector.collect_from_existing_account(&pubkey, &mut account);
assert_eq!(account.lamports, huge_lamports);
assert_eq!(collected, 0);
// decrease the balance not to be rent-exempt
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);
assert_eq!(account.lamports, tiny_lamports - collected);
assert_ne!(collected, 0);
}
}