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

Remove feature switch for demoting sysvar write locks (#18373)

Browse Source
This commit is contained in:
Justin Starry
2021-07-06 16:22:22 -05:00
committed by GitHub
parent 5dd399dafa
commit 100fabf469
12 changed files with 71 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
runtime/src/accounts.rs
View File

@@ -178,11 +178,8 @@ impl Accounts {
false
}
fn construct_instructions_account(
message: &Message,
demote_sysvar_write_locks: bool,
) -> AccountSharedData {
let mut data = message.serialize_instructions(demote_sysvar_write_locks);
fn construct_instructions_account(message: &Message) -> AccountSharedData {
let mut data = message.serialize_instructions();
// add room for current instruction index.
data.resize(data.len() + 2, 0);
AccountSharedData::from(Account {
@@ -211,8 +208,6 @@ impl Accounts {
let mut tx_rent: TransactionRent = 0;
let mut accounts = Vec::with_capacity(message.account_keys.len());
let mut account_deps = Vec::with_capacity(message.account_keys.len());
let demote_sysvar_write_locks =
feature_set.is_active(&feature_set::demote_sysvar_write_locks::id());
let mut key_check = MessageProgramIdsCache::new(message);
let mut rent_debits = RentDebits::default();
for (i, key) in message.account_keys.iter().enumerate() {
@@ -224,16 +219,16 @@ impl Accounts {
if solana_sdk::sysvar::instructions::check_id(key)
&& feature_set.is_active(&feature_set::instructions_sysvar_enabled::id())
{
if message.is_writable(i, demote_sysvar_write_locks) {
if message.is_writable(i) {
return Err(TransactionError::InvalidAccountIndex);
}
Self::construct_instructions_account(message, demote_sysvar_write_locks)
Self::construct_instructions_account(message)
} else {
let (account, rent) = self
.accounts_db
.load_with_fixed_root(ancestors, key)
.map(|(mut account, _)| {
if message.is_writable(i, demote_sysvar_write_locks) {
if message.is_writable(i) {
let rent_due = rent_collector
.collect_from_existing_account(key, &mut account);
(account, rent_due)
@@ -862,11 +857,7 @@ impl Accounts {
/// same time
#[must_use]
#[allow(clippy::needless_collect)]
pub fn lock_accounts<'a>(
&self,
txs: impl Iterator<Item = &'a Transaction>,
demote_sysvar_write_locks: bool,
) -> Vec<Result<()>> {
pub fn lock_accounts<'a>(&self, txs: impl Iterator<Item = &'a Transaction>) -> Vec<Result<()>> {
use solana_sdk::sanitize::Sanitize;
let keys: Vec<Result<_>> = txs
.map(|tx| {
@@ -876,9 +867,7 @@ impl Accounts {
return Err(TransactionError::AccountLoadedTwice);
}
Ok(tx
.message()
.get_account_keys_by_lock_type(demote_sysvar_write_locks))
Ok(tx.message().get_account_keys_by_lock_type())
})
.collect();
let mut account_locks = &mut self.account_locks.lock().unwrap();
@@ -898,7 +887,6 @@ impl Accounts {
&self,
txs: impl Iterator<Item = &'a Transaction>,
results: &[Result<()>],
demote_sysvar_write_locks: bool,
) {
let keys: Vec<_> = txs
.zip(results)
@@ -906,10 +894,7 @@ impl Accounts {
Err(TransactionError::AccountInUse) => None,
Err(TransactionError::SanitizeFailure) => None,
Err(TransactionError::AccountLoadedTwice) => None,
_ => Some(
tx.message
.get_account_keys_by_lock_type(demote_sysvar_write_locks),
),
_ => Some(tx.message.get_account_keys_by_lock_type()),
})
.collect();
let mut account_locks = self.account_locks.lock().unwrap();
@@ -931,7 +916,6 @@ impl Accounts {
rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool,
demote_sysvar_write_locks: bool,
) {
let accounts_to_store = self.collect_accounts_to_store(
txs,
@@ -940,7 +924,6 @@ impl Accounts {
rent_collector,
last_blockhash_with_fee_calculator,
fix_recent_blockhashes_sysvar_delay,
demote_sysvar_write_locks,
);
self.accounts_db.store_cached(slot, &accounts_to_store);
}
@@ -965,7 +948,6 @@ impl Accounts {
rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool,
demote_sysvar_write_locks: bool,
) -> Vec<(&'a Pubkey, &'a AccountSharedData)> {
let mut accounts = Vec::with_capacity(loaded.len());
for (i, ((raccs, _nonce_rollback), tx)) in loaded.iter_mut().zip(txs).enumerate() {
@@ -1009,7 +991,7 @@ impl Accounts {
fee_payer_index = Some(i);
}
let is_fee_payer = Some(i) == fee_payer_index;
if message.is_writable(i, demote_sysvar_write_locks)
if message.is_writable(i)
&& (res.is_ok()
|| (maybe_nonce_rollback.is_some() && (is_nonce_account || is_fee_payer)))
{
@@ -1774,10 +1756,7 @@ mod tests {
instructions,
);
let tx = Transaction::new(&[&keypair0], message, Hash::default());
let results0 = accounts.lock_accounts(
[tx.clone()].iter(),
true, // demote_sysvar_write_locks
);
let results0 = accounts.lock_accounts([tx.clone()].iter());
assert!(results0[0].is_ok());
assert_eq!(
@@ -1812,10 +1791,7 @@ mod tests {
);
let tx1 = Transaction::new(&[&keypair1], message, Hash::default());
let txs = vec![tx0, tx1];
let results1 = accounts.lock_accounts(
txs.iter(),
true, // demote_sysvar_write_locks
);
let results1 = accounts.lock_accounts(txs.iter());
assert!(results1[0].is_ok()); // Read-only account (keypair1) can be referenced multiple times
assert!(results1[1].is_err()); // Read-only account (keypair1) cannot also be locked as writable
@@ -1830,16 +1806,8 @@ mod tests {
2
);
accounts.unlock_accounts(
[tx].iter(),
&results0,
true, // demote_sysvar_write_locks
);
accounts.unlock_accounts(
txs.iter(),
&results1,
true, // demote_sysvar_write_locks
);
accounts.unlock_accounts([tx].iter(), &results0);
accounts.unlock_accounts(txs.iter(), &results1);
let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])];
let message = Message::new_with_compiled_instructions(
1,
@@ -1850,10 +1818,7 @@ mod tests {
instructions,
);
let tx = Transaction::new(&[&keypair1], message, Hash::default());
let results2 = accounts.lock_accounts(
[tx].iter(),
true, // demote_sysvar_write_locks
);
let results2 = accounts.lock_accounts([tx].iter());
assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable
// Check that read-only lock with zero references is deleted
@@ -1922,20 +1887,13 @@ mod tests {
let exit_clone = exit_clone.clone();
loop {
let txs = vec![writable_tx.clone()];
let results = accounts_clone.clone().lock_accounts(
txs.iter(),
true, // demote_sysvar_write_locks
);
let results = accounts_clone.clone().lock_accounts(txs.iter());
for result in results.iter() {
if result.is_ok() {
counter_clone.clone().fetch_add(1, Ordering::SeqCst);
}
}
accounts_clone.unlock_accounts(
txs.iter(),
&results,
true, // demote_sysvar_write_locks
);
accounts_clone.unlock_accounts(txs.iter(), &results);
if exit_clone.clone().load(Ordering::Relaxed) {
break;
}
@@ -1944,20 +1902,13 @@ mod tests {
let counter_clone = counter;
for _ in 0..5 {
let txs = vec![readonly_tx.clone()];
let results = accounts_arc.clone().lock_accounts(
txs.iter(),
true, // demote_sysvar_write_locks
);
let results = accounts_arc.clone().lock_accounts(txs.iter());
if results[0].is_ok() {
let counter_value = counter_clone.clone().load(Ordering::SeqCst);
thread::sleep(time::Duration::from_millis(50));
assert_eq!(counter_value, counter_clone.clone().load(Ordering::SeqCst));
}
accounts_arc.unlock_accounts(
txs.iter(),
&results,
true, // demote_sysvar_write_locks
);
accounts_arc.unlock_accounts(txs.iter(), &results);
thread::sleep(time::Duration::from_millis(50));
}
exit.store(true, Ordering::Relaxed);
@@ -2054,7 +2005,6 @@ mod tests {
&rent_collector,
&(Hash::default(), FeeCalculator::default()),
true,
true, // demote_sysvar_write_locks
);
assert_eq!(collected_accounts.len(), 2);
assert!(collected_accounts
@@ -2431,7 +2381,6 @@ mod tests {
&rent_collector,
&(next_blockhash, FeeCalculator::default()),
true,
true, // demote_sysvar_write_locks
);
assert_eq!(collected_accounts.len(), 2);
assert_eq!(
@@ -2548,7 +2497,6 @@ mod tests {
&rent_collector,
&(next_blockhash, FeeCalculator::default()),
true,
true, // demote_sysvar_write_locks
);
assert_eq!(collected_accounts.len(), 1);
let collected_nonce_account = collected_accounts