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

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

Merged
mergify merged 3 commits into from
Aug 17, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2188b7
Re-do rent collection check on rent-exempt account (#11349)
ryoqun Aug 17, 2020
241fd7b
Fix conflicts
ryoqun Aug 17, 2020
341490a
Add missing comment
ryoqun Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/src/rpc_pubsub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ mod tests {
"lamports": 51,
"data": bs58::encode(expected_data).into_string(),
"executable": false,
"rentEpoch": 1,
"rentEpoch": 0,
},
},
"subscription": 0,
Expand Down Expand Up @@ -720,7 +720,7 @@ mod tests {
"lamports": 100,
"data": expected_data,
"executable": false,
"rentEpoch": 1,
"rentEpoch": 0,
},
},
"subscription": 0,
Expand Down Expand Up @@ -899,7 +899,7 @@ mod tests {
"lamports": 100,
"data": "",
"executable": false,
"rentEpoch": 1,
"rentEpoch": 0,
},
},
"subscription": 0,
Expand Down
8 changes: 4 additions & 4 deletions core/src/rpc_subscriptions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Budget1111111111111111111111111111111111111",
"rentEpoch": 1,
"rentEpoch": 0,
},
},
"subscription": 0,
Expand Down Expand Up @@ -1185,7 +1185,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Budget1111111111111111111111111111111111111",
"rentEpoch": 1,
"rentEpoch": 0,
},
"pubkey": alice.pubkey().to_string(),
},
Expand Down Expand Up @@ -1607,7 +1607,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Budget1111111111111111111111111111111111111",
"rentEpoch": 1,
"rentEpoch": 0,
},
},
"subscription": 0,
Expand Down Expand Up @@ -1641,7 +1641,7 @@ pub(crate) mod tests {
"executable": false,
"lamports": 1,
"owner": "Budget1111111111111111111111111111111111111",
"rentEpoch": 1,
"rentEpoch": 0,
},
},
"subscription": 1,
Expand Down
66 changes: 46 additions & 20 deletions core/tests/bank_forks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,32 @@
// Long-running bank_forks tests

macro_rules! DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS {
($x:ident) => {
($x:ident, $y:ident, $z:ident) => {
#[allow(non_snake_case)]
mod $x {
mod $z {
use super::*;

const SNAPSHOT_VERSION: SnapshotVersion = SnapshotVersion::$x;
const OPERATING_MODE: OperatingMode = OperatingMode::$y;

#[test]
fn test_bank_forks_status_cache_snapshot_n() {
run_test_bank_forks_status_cache_snapshot_n(SNAPSHOT_VERSION)
run_test_bank_forks_status_cache_snapshot_n(SNAPSHOT_VERSION, OPERATING_MODE)
}

#[test]
fn test_bank_forks_snapshot_n() {
run_test_bank_forks_snapshot_n(SNAPSHOT_VERSION)
run_test_bank_forks_snapshot_n(SNAPSHOT_VERSION, OPERATING_MODE)
}

#[test]
fn test_concurrent_snapshot_packaging() {
run_test_concurrent_snapshot_packaging(SNAPSHOT_VERSION)
run_test_concurrent_snapshot_packaging(SNAPSHOT_VERSION, OPERATING_MODE)
}

#[test]
fn test_slots_to_snapshot() {
run_test_slots_to_snapshot(SNAPSHOT_VERSION)
run_test_slots_to_snapshot(SNAPSHOT_VERSION, OPERATING_MODE)
}
}
};
Expand All @@ -52,7 +53,7 @@ mod tests {
};
use solana_sdk::{
clock::Slot,
genesis_config::GenesisConfig,
genesis_config::{GenesisConfig, OperatingMode},
hash::hashv,
pubkey::Pubkey,
signature::{Keypair, Signer},
Expand All @@ -61,8 +62,12 @@ mod tests {
use std::{fs, path::PathBuf, sync::atomic::AtomicBool, sync::mpsc::channel, sync::Arc};
use tempfile::TempDir;

DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_1_0);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_2_0);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_1_0, Development, V1_1_0_Development);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_1_0, Preview, V1_1_0_Preview);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_1_0, Stable, V1_1_0_Stable);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_2_0, Development, V1_2_0_Development);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_2_0, Preview, V1_2_0_Preview);
DEFINE_SNAPSHOT_VERSION_PARAMETERIZED_TEST_FUNCTIONS!(V1_2_0, Stable, V1_2_0_Stable);

struct SnapshotTestConfig {
accounts_dir: TempDir,
Expand All @@ -76,12 +81,14 @@ mod tests {
impl SnapshotTestConfig {
fn new(
snapshot_version: SnapshotVersion,
operating_mode: OperatingMode,
snapshot_interval_slots: u64,
) -> SnapshotTestConfig {
let accounts_dir = TempDir::new().unwrap();
let snapshot_dir = TempDir::new().unwrap();
let snapshot_output_path = TempDir::new().unwrap();
let genesis_config_info = create_genesis_config(10_000);
let mut genesis_config_info = create_genesis_config(10_000);
genesis_config_info.genesis_config.operating_mode = operating_mode;
let bank0 = Bank::new_with_paths(
&genesis_config_info.genesis_config,
vec![accounts_dir.path().to_path_buf()],
Expand Down Expand Up @@ -113,6 +120,7 @@ mod tests {
fn restore_from_snapshot(
old_bank_forks: &BankForks,
old_last_slot: Slot,
old_genesis_config: &GenesisConfig,
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun Aug 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is extracted from #10581

account_paths: &[PathBuf],
) {
let (snapshot_path, snapshot_package_output_path) = old_bank_forks
Expand All @@ -137,7 +145,7 @@ mod tests {
&CompressionType::Bzip2,
),
CompressionType::Bzip2,
&GenesisConfig::default(),
old_genesis_config,
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun Aug 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spent some time for me to figure out why the test has started to failing.....

)
.unwrap();

Expand All @@ -161,6 +169,7 @@ mod tests {
// `last_slot` bank
fn run_bank_forks_snapshot_n<F>(
snapshot_version: SnapshotVersion,
operating_mode: OperatingMode,
last_slot: Slot,
f: F,
set_root_interval: u64,
Expand All @@ -169,10 +178,9 @@ mod tests {
{
solana_logger::setup();
// Set up snapshotting config
let mut snapshot_test_config = SnapshotTestConfig::new(snapshot_version, 1);
let mut snapshot_test_config = SnapshotTestConfig::new(snapshot_version, operating_mode, 1);

let bank_forks = &mut snapshot_test_config.bank_forks;
let accounts_dir = &snapshot_test_config.accounts_dir;
let mint_keypair = &snapshot_test_config.genesis_config_info.mint_keypair;

let (s, _r) = channel();
Expand All @@ -188,6 +196,7 @@ mod tests {
bank_forks.set_root(bank.slot(), &sender, None);
}
}

// Generate a snapshot package for last bank
let last_bank = bank_forks.get(last_slot).unwrap();
let snapshot_config = &snapshot_test_config.snapshot_config;
Expand All @@ -206,17 +215,23 @@ mod tests {
snapshot_version,
)
.unwrap();

snapshot_utils::archive_snapshot_package(&snapshot_package).unwrap();

restore_from_snapshot(bank_forks, last_slot, &[accounts_dir.path().to_path_buf()]);
// Restore bank from snapshot
let account_paths = &[snapshot_test_config.accounts_dir.path().to_path_buf()];
let genesis_config = &snapshot_test_config.genesis_config_info.genesis_config;
restore_from_snapshot(bank_forks, last_slot, genesis_config, account_paths);
}

fn run_test_bank_forks_snapshot_n(snapshot_version: SnapshotVersion) {
fn run_test_bank_forks_snapshot_n(
snapshot_version: SnapshotVersion,
operating_mode: OperatingMode,
) {
// create banks upto slot 4 and create 1 new account in each bank. test that bank 4 snapshots
// and restores correctly
run_bank_forks_snapshot_n(
snapshot_version,
operating_mode,
4,
|bank, mint_keypair| {
let key1 = Keypair::new().pubkey();
Expand Down Expand Up @@ -247,11 +262,14 @@ mod tests {
}
}

fn run_test_concurrent_snapshot_packaging(snapshot_version: SnapshotVersion) {
fn run_test_concurrent_snapshot_packaging(
snapshot_version: SnapshotVersion,
operating_mode: OperatingMode,
) {
solana_logger::setup();

// Set up snapshotting config
let mut snapshot_test_config = SnapshotTestConfig::new(snapshot_version, 1);
let mut snapshot_test_config = SnapshotTestConfig::new(snapshot_version, operating_mode, 1);

let bank_forks = &mut snapshot_test_config.bank_forks;
let accounts_dir = &snapshot_test_config.accounts_dir;
Expand Down Expand Up @@ -396,7 +414,10 @@ mod tests {
);
}

fn run_test_slots_to_snapshot(snapshot_version: SnapshotVersion) {
fn run_test_slots_to_snapshot(
snapshot_version: SnapshotVersion,
operating_mode: OperatingMode,
) {
solana_logger::setup();
let num_set_roots = MAX_CACHE_ENTRIES * 2;

Expand All @@ -405,6 +426,7 @@ mod tests {
// Make sure this test never clears bank.slots_since_snapshot
let mut snapshot_test_config = SnapshotTestConfig::new(
snapshot_version,
operating_mode,
(*add_root_interval * num_set_roots * 2) as u64,
);
let mut current_bank = snapshot_test_config.bank_forks[0].clone();
Expand Down Expand Up @@ -438,7 +460,10 @@ mod tests {
}
}

fn run_test_bank_forks_status_cache_snapshot_n(snapshot_version: SnapshotVersion) {
fn run_test_bank_forks_status_cache_snapshot_n(
snapshot_version: SnapshotVersion,
operating_mode: OperatingMode,
) {
// create banks upto slot (MAX_CACHE_ENTRIES * 2) + 1 while transferring 1 lamport into 2 different accounts each time
// this is done to ensure the AccountStorageEntries keep getting cleaned up as the root moves
// ahead. Also tests the status_cache purge and status cache snapshotting.
Expand All @@ -448,6 +473,7 @@ mod tests {
for set_root_interval in &[1, 4] {
run_bank_forks_snapshot_n(
snapshot_version,
operating_mode,
(MAX_CACHE_ENTRIES * 2 + 1) as u64,
|bank, mint_keypair| {
let tx = system_transaction::transfer(
Expand Down
6 changes: 3 additions & 3 deletions docs/src/implemented-proposals/rent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ Even if those processes are out of scope of rent collection, all of manipulated

## Actual processing of collecting rent

Rent is due for one epoch's worth of time, and accounts always have `Account::rent_epoch` of `current_epoch + 1`.
Rent is due for one epoch's worth of time, and accounts have `Account::rent_epoch` of `current_epoch` or `current_epoch + 1` depending on the rent regime.

If the account is in the exempt regime, `Account::rent_epoch` is simply pushed to `current_epoch + 1`.
If the account is in the exempt regime, `Account::rent_epoch` is simply updated to `current_epoch`.

If the account is non-exempt, the difference between the next epoch and `Account::rent_epoch` is used to calculate the amount of rent owed by this account \(via `Rent::due()`\). Any fractional lamports of the calculation are truncated. Rent due is deducted from `Account::lamports` and `Account::rent_epoch` is updated to the next epoch. If the amount of rent due is less than one lamport, no changes are made to the account.
If the account is non-exempt, the difference between the next epoch and `Account::rent_epoch` is used to calculate the amount of rent owed by this account \(via `Rent::due()`\). Any fractional lamports of the calculation are truncated. Rent due is deducted from `Account::lamports` and `Account::rent_epoch` is updated to `current_epoch + 1` (= next epoch). If the amount of rent due is less than one lamport, no changes are made to the account.

Accounts whose balance is insufficient to satisfy the rent that would be due simply fail to load.

Expand Down
1 change: 1 addition & 0 deletions ledger/src/snapshot_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ where

bank.rc = bankrc;
bank.operating_mode = Some(genesis_config.operating_mode);
bank.init_rent_collector_after_deserialize(genesis_config);
bank.finish_init();
Ok(bank)
})?;
Expand Down
6 changes: 3 additions & 3 deletions programs/bpf/c/src/invoked/invoked.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
sol_assert(accounts[ARGUMENT_INDEX].data_len == 100);
sol_assert(accounts[ARGUMENT_INDEX].is_signer);
sol_assert(accounts[ARGUMENT_INDEX].is_writable);
sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 1);
sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 0);
sol_assert(!accounts[ARGUMENT_INDEX].executable);
for (int i = 0; i < accounts[ARGUMENT_INDEX].data_len; i++) {
sol_assert(accounts[ARGUMENT_INDEX].data[i] == i);
Expand All @@ -48,7 +48,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].data_len == 10);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_signer);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_writable);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 1);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 0);
sol_assert(!accounts[INVOKED_ARGUMENT_INDEX].executable);

sol_assert(
Expand All @@ -57,7 +57,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
&bpf_loader_id));
sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_signer);
sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_writable);
sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == 1);
sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == 0);
sol_assert(accounts[INVOKED_PROGRAM_INDEX].executable);

sol_assert(SolPubkey_same(accounts[INVOKED_PROGRAM_INDEX].key,
Expand Down
6 changes: 3 additions & 3 deletions programs/bpf/rust/invoked/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn process_instruction(
assert_eq!(accounts[ARGUMENT_INDEX].data_len(), 100);
assert!(accounts[ARGUMENT_INDEX].is_signer);
assert!(accounts[ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, 1);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, 0);
assert!(!accounts[ARGUMENT_INDEX].executable);
{
let data = accounts[ARGUMENT_INDEX].try_borrow_data()?;
Expand All @@ -59,14 +59,14 @@ fn process_instruction(
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].data_len(), 10);
assert!(accounts[INVOKED_ARGUMENT_INDEX].is_signer);
assert!(accounts[INVOKED_ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, 1);
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, 0);
assert!(!accounts[INVOKED_ARGUMENT_INDEX].executable);

assert_eq!(accounts[INVOKED_PROGRAM_INDEX].key, program_id);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].owner, &bpf_loader::id());
assert!(!accounts[INVOKED_PROGRAM_INDEX].is_signer);
assert!(!accounts[INVOKED_PROGRAM_INDEX].is_writable);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, 1);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, 0);
assert!(accounts[INVOKED_PROGRAM_INDEX].executable);

assert_eq!(
Expand Down
2 changes: 2 additions & 0 deletions runtime/src/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,7 @@ mod tests {
account::Account,
epoch_schedule::EpochSchedule,
fee_calculator::FeeCalculator,
genesis_config::OperatingMode,
hash::Hash,
instruction::CompiledInstruction,
message::Message,
Expand Down Expand Up @@ -1027,6 +1028,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);
Expand Down
Loading