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

Revert "collect rent from the accounts (#6181)" (#6356)

Browse Source 
automerge
This commit is contained in:
Rob Walker
2019-10-14 16:16:44 -07:00
committed by Grimes
parent 6a161c740d
commit 9803f167bd
3 changed files with 59 additions and 617 deletions
Download Patch File Download Diff File Expand all files Collapse all files

223
runtime/src/accounts.rs
View File

@@ -27,7 +27,6 @@ use crate::transaction_utils::OrderedIterator;
struct CreditOnlyLock {
credits: AtomicU64,
lock_count: Mutex<u64>,
rent_debtor: bool,
}
/// This structure handles synchronization for db
@@ -40,7 +39,7 @@ pub struct Accounts {
account_locks: Mutex<HashSet<Pubkey>>,
/// Set of credit-only accounts which are currently in the pipeline, caching account balance
/// number of locks and whether or not account owes rent. On commit_credits(), we do a take() on the option so that the hashmap
/// and number of locks. On commit_credits(), we do a take() on the option so that the hashmap
/// is no longer available to be written to.
credit_only_account_locks: Arc<RwLock<Option<HashMap<Pubkey, CreditOnlyLock>>>>,
}
@@ -96,7 +95,6 @@ impl Accounts {
fee: u64,
error_counters: &mut ErrorCounters,
rent_collector: &RentCollector,
w_credit_only_account_locks: &mut RwLockWriteGuard<Option<HashMap<Pubkey, CreditOnlyLock>>>,
) -> Result<(TransactionAccounts, TransactionCredits, TransactionRents)> {
// Copy all the accounts
let message = tx.message();
@@ -109,33 +107,20 @@ impl Accounts {
return Err(TransactionError::AccountLoadedTwice);
}
let credit_only_account_locks = w_credit_only_account_locks.as_mut().unwrap();
let mut credit_only_keys: HashSet<Pubkey> = HashSet::new();
// There is no way to predict what program will execute without an error
// If a fee can pay for execution then the program will be scheduled
let mut accounts: TransactionAccounts = vec![];
let mut credits: TransactionCredits = vec![];
let mut rents: TransactionRents = vec![];
for (i, key) in message
for key in message
.account_keys
.iter()
.filter(|key| !message.program_ids().contains(&key))
.enumerate()
{
let (account, rent) = AccountsDB::load(storage, ancestors, accounts_index, key)
.and_then(
|(mut account, _)| match rent_collector.update(&mut account) {
(Some(_), rent_due) => Some((account, rent_due)),
(None, rent_due) => Some((Account::default(), rent_due)),
},
)
.and_then(|(account, _)| rent_collector.update(account))
.unwrap_or_default();
if !message.is_debitable(i) {
credit_only_keys.insert(*key);
}
accounts.push(account);
credits.push(0);
rents.push(rent);
@@ -152,11 +137,6 @@ impl Accounts {
Err(TransactionError::InsufficientFundsForFee)
} else {
accounts[0].lamports -= fee;
for key in credit_only_keys {
credit_only_account_locks.entry(key).and_modify(|mut lock| {
lock.rent_debtor = true;
});
}
Ok((accounts, credits, rents))
}
}
@@ -249,7 +229,6 @@ impl Accounts {
//TODO: two locks usually leads to deadlocks, should this be one structure?
let accounts_index = self.accounts_db.accounts_index.read().unwrap();
let storage = self.accounts_db.storage.read().unwrap();
let mut w_credit_only_account_locks = self.credit_only_account_locks.write().unwrap();
OrderedIterator::new(txs, txs_iteration_order)
.zip(lock_results.into_iter())
.map(|etx| match etx {
@@ -267,7 +246,6 @@ impl Accounts {
fee,
error_counters,
rent_collector,
&mut w_credit_only_account_locks,
)?;
let loaders = Self::load_loaders(
&storage,
@@ -442,7 +420,6 @@ impl Accounts {
CreditOnlyLock {
credits: AtomicU64::new(0),
lock_count: Mutex::new(1),
rent_debtor: false,
},
);
}
@@ -573,97 +550,49 @@ impl Accounts {
// so will fail the lock
// 2) Any transaction that grabs a lock and then commit_credits clears the HashMap will find
// the HashMap is None on unlock_accounts, and will perform a no-op.
pub fn commit_credits_and_rents(
&self,
rent_collector: &RentCollector,
ancestors: &HashMap<Fork, usize>,
fork: Fork,
) -> u64 {
pub fn commit_credits(&self, ancestors: &HashMap<Fork, usize>, fork: Fork) {
// Clear the credit only hashmap so that no further transactions can modify it
let credit_only_account_locks = Self::take_credit_only(&self.credit_only_account_locks)
.expect("Credit only locks didn't exist in commit_credits_and_rents");
self.store_credit_only_credits_and_rents(
credit_only_account_locks,
rent_collector,
ancestors,
fork,
)
.expect("Credit only locks didn't exist in commit_credits");
self.store_credit_only_credits(credit_only_account_locks, ancestors, fork);
}
/// Used only for tests to store credit-only accounts after every transaction
pub fn commit_credits_and_rents_unsafe(
&self,
rent_collector: &RentCollector,
ancestors: &HashMap<Fork, usize>,
fork: Fork,
) -> u64 {
pub fn commit_credits_unsafe(&self, ancestors: &HashMap<Fork, usize>, fork: Fork) {
// Clear the credit only hashmap so that no further transactions can modify it
let mut w_credit_only_account_locks = self.credit_only_account_locks.write().unwrap();
let w_credit_only_account_locks =
Self::get_write_access_credit_only(&mut w_credit_only_account_locks)
.expect("Credit only locks didn't exist in commit_credits");
self.store_credit_only_credits_and_rents(
w_credit_only_account_locks.drain(),
rent_collector,
ancestors,
fork,
)
self.store_credit_only_credits(w_credit_only_account_locks.drain(), ancestors, fork);
}
fn store_credit_only_credits_and_rents<I>(
fn store_credit_only_credits<I>(
&self,
credit_only_account_locks: I,
rent_collector: &RentCollector,
ancestors: &HashMap<Fork, usize>,
fork: Fork,
) -> u64
where
) where
I: IntoIterator<Item = (Pubkey, CreditOnlyLock)>,
{
let mut accounts: HashMap<Pubkey, Account> = HashMap::new();
let mut total_rent_collected = 0;
{
let accounts_index = self.accounts_db.accounts_index.read().unwrap();
let storage = self.accounts_db.storage.read().unwrap();
for (pubkey, lock) in credit_only_account_locks {
let lock_count = *lock.lock_count.lock().unwrap();
if lock_count != 0 {
warn!(
"dropping credit-only lock on {}, still has {} locks",
pubkey, lock_count
);
}
let credit = lock.credits.load(Ordering::Relaxed);
let (mut account, _) =
AccountsDB::load(&storage, ancestors, &accounts_index, &pubkey)
.unwrap_or_default();
if lock.rent_debtor {
let (rent_debtor_account, rent_collected) =
match rent_collector.update(&mut account) {
(Some(_), rent_due) => (account, rent_due),
(None, rent_due) => (Account::default(), rent_due),
};
total_rent_collected += rent_collected;
account = rent_debtor_account;
}
for (pubkey, lock) in credit_only_account_locks {
let lock_count = *lock.lock_count.lock().unwrap();
if lock_count != 0 {
warn!(
"dropping credit-only lock on {}, still has {} locks",
pubkey, lock_count
);
}
let credit = lock.credits.load(Ordering::Relaxed);
if credit > 0 {
let mut account = self
.load_slow(ancestors, &pubkey)
.map(|(account, _)| account)
.unwrap_or_default();
account.lamports += credit;
accounts.insert(pubkey, account);
self.store_slow(fork, &pubkey, &account);
}
}
let account_to_store: Vec<(&Pubkey, &Account)> = accounts
.iter()
.map(|(key, account)| (key, account))
.collect();
self.accounts_db.store(fork, &account_to_store);
total_rent_collected
}
fn collect_accounts_to_store<'a>(
@@ -735,7 +664,6 @@ mod tests {
use solana_sdk::fee_calculator::FeeCalculator;
use solana_sdk::hash::Hash;
use solana_sdk::instruction::CompiledInstruction;
use solana_sdk::rent_calculator::RentCalculator;
use solana_sdk::signature::{Keypair, KeypairUtil};
use solana_sdk::sysvar;
use solana_sdk::transaction::Transaction;
@@ -744,11 +672,10 @@ mod tests {
use std::{thread, time};
use tempfile::TempDir;
fn load_accounts_with_fee_and_rent(
fn load_accounts_with_fee(
tx: Transaction,
ka: &Vec<(Pubkey, Account)>,
fee_calculator: &FeeCalculator,
rent_collector: &RentCollector,
error_counters: &mut ErrorCounters,
) -> Vec<Result<TransactionLoadResult>> {
let mut hash_queue = BlockhashQueue::new(100);
@@ -759,6 +686,7 @@ mod tests {
}
let ancestors = vec![(0, 0)].into_iter().collect();
let rent_collector = RentCollector::default();
let res = accounts.load_accounts(
&ancestors,
&[tx],
@@ -777,8 +705,7 @@ mod tests {
error_counters: &mut ErrorCounters,
) -> Vec<Result<TransactionLoadResult>> {
let fee_calculator = FeeCalculator::default();
let rent_collector: RentCollector = RentCollector::default();
load_accounts_with_fee_and_rent(tx, ka, &fee_calculator, &rent_collector, error_counters)
load_accounts_with_fee(tx, ka, &fee_calculator, error_counters)
}
#[test]
@@ -882,15 +809,8 @@ mod tests {
let fee_calculator = FeeCalculator::new(10, 0);
assert_eq!(fee_calculator.calculate_fee(tx.message()), 10);
let rent_collector = RentCollector::default();
let loaded_accounts = load_accounts_with_fee_and_rent(
tx,
&accounts,
&fee_calculator,
&rent_collector,
&mut error_counters,
);
let loaded_accounts =
load_accounts_with_fee(tx, &accounts, &fee_calculator, &mut error_counters);
assert_eq!(error_counters.insufficient_funds, 1);
assert_eq!(loaded_accounts.len(), 1);
@@ -1494,29 +1414,17 @@ mod tests {
}
#[test]
fn test_commit_credits_and_rents() {
fn test_commit_credits() {
let pubkey0 = Pubkey::new_rand();
let pubkey1 = Pubkey::new_rand();
let pubkey2 = Pubkey::new_rand();
let overdue_account_pubkey = Pubkey::new_rand();
let account0 = Account::new(1, 1, &Pubkey::default());
let account1 = Account::new(2, 1, &Pubkey::default());
let overdue_account = Account::new(1, 2, &Pubkey::default());
let account0 = Account::new(1, 0, &Pubkey::default());
let account1 = Account::new(2, 0, &Pubkey::default());
let accounts = Accounts::new(None);
accounts.store_slow(0, &pubkey0, &account0);
accounts.store_slow(0, &pubkey1, &account1);
accounts.store_slow(0, &overdue_account_pubkey, &overdue_account);
let mut rent_collector = RentCollector::default();
rent_collector.epoch = 2;
rent_collector.slots_per_year = 400_f64;
rent_collector.rent_calculator = RentCalculator {
burn_percent: 10,
exemption_threshold: 8.0,
lamports_per_byte_year: 1,
};
{
let mut credit_only_account_locks = accounts.credit_only_account_locks.write().unwrap();
@@ -1524,9 +1432,8 @@ mod tests {
credit_only_account_locks.insert(
pubkey0,
CreditOnlyLock {
credits: AtomicU64::new(1),
credits: AtomicU64::new(0),
lock_count: Mutex::new(1),
rent_debtor: true,
},
);
credit_only_account_locks.insert(
@@ -1534,7 +1441,6 @@ mod tests {
CreditOnlyLock {
credits: AtomicU64::new(5),
lock_count: Mutex::new(1),
rent_debtor: true,
},
);
credit_only_account_locks.insert(
@@ -1542,74 +1448,24 @@ mod tests {
CreditOnlyLock {
credits: AtomicU64::new(10),
lock_count: Mutex::new(1),
rent_debtor: false,
},
);
credit_only_account_locks.insert(
overdue_account_pubkey,
CreditOnlyLock {
credits: AtomicU64::new(3),
lock_count: Mutex::new(1),
rent_debtor: true,
},
);
}
let ancestors = vec![(0, 0)].into_iter().collect();
accounts.commit_credits_unsafe(&ancestors, 0);
// This account has data length of 2
assert_eq!(
accounts
.load_slow(&ancestors, &overdue_account_pubkey)
.unwrap()
.0
.data
.len(),
2
);
// Total rent collected should be: rent from pubkey0(1) + rent from pubkey1(1)
assert_eq!(
accounts.commit_credits_and_rents_unsafe(&rent_collector, &ancestors, 0),
3
);
// New balance should be previous balance plus CreditOnlyLock credits - rent
// Balance(1) - Rent(1) + Credit(1)
// No change when CreditOnlyLock credits are 0
assert_eq!(
accounts.load_slow(&ancestors, &pubkey0).unwrap().0.lamports,
1
);
// New balance should equal previous balance plus CreditOnlyLock credits - rent
// Balance(2) - Rent(1) + Credit(5)
// New balance should equal previous balance plus CreditOnlyLock credits
assert_eq!(
accounts.load_slow(&ancestors, &pubkey1).unwrap().0.lamports,
6
7
);
// Rent overdue account, will be reseted and it's lamport will be consumed
// Balance(1) - Rent(1)(Due to insufficient balance) + Credit(3)
assert_eq!(
accounts
.load_slow(&ancestors, &overdue_account_pubkey)
.unwrap()
.0
.lamports,
3
);
// The reseted account should have data length of zero
assert_eq!(
accounts
.load_slow(&ancestors, &overdue_account_pubkey)
.unwrap()
.0
.data
.len(),
0
);
// New account should be created and no rent should be charged
// New account should be created
assert_eq!(
accounts.load_slow(&ancestors, &pubkey2).unwrap().0.lamports,
10
@@ -1695,7 +1551,6 @@ mod tests {
CreditOnlyLock {
credits: AtomicU64::new(0),
lock_count: Mutex::new(1),
rent_debtor: false,
},
);
}