diff --git a/cli-output/src/display.rs b/cli-output/src/display.rs index 4961feb0f6..f8f7e8274a 100644 --- a/cli-output/src/display.rs +++ b/cli-output/src/display.rs @@ -140,7 +140,7 @@ fn format_account_mode(message: &Message, index: usize) -> String { } else { "-" }, - if message.is_writable(index, /*demote_sysvar_write_locks=*/ true) { + if message.is_writable(index) { "w" // comment for consistent rust fmt (no joking; lol) } else { "-" diff --git a/core/src/cost_model.rs b/core/src/cost_model.rs index 0f16981ea4..416e62f3dd 100644 --- a/core/src/cost_model.rs +++ b/core/src/cost_model.rs @@ -27,7 +27,6 @@ pub const ACCOUNT_MAX_COST: u64 = 100_000_000; pub const BLOCK_MAX_COST: u64 = 2_500_000_000; const MAX_WRITABLE_ACCOUNTS: usize = 256; -const DEMOTE_SYSVAR_WRITE_LOCKS: bool = true; // cost of transaction is made of account_access_cost and instruction execution_cost // where @@ -97,7 +96,7 @@ impl CostModel { let message = transaction.message(); message.account_keys.iter().enumerate().for_each(|(i, k)| { let is_signer = message.is_signer(i); - let is_writable = message.is_writable(i, DEMOTE_SYSVAR_WRITE_LOCKS); + let is_writable = message.is_writable(i); if is_signer && is_writable { self.transaction_cost.writable_accounts.push(*k); diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index bb01626149..636fa68d60 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -19,7 +19,6 @@ use { clock::{Clock, Slot}, entrypoint::{ProgramResult, SUCCESS}, epoch_schedule::EpochSchedule, - feature_set::demote_sysvar_write_locks, fee_calculator::{FeeCalculator, FeeRateGovernor}, genesis_config::{ClusterType, GenesisConfig}, hash::Hash, @@ -259,14 +258,12 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { } panic!("Program id {} wasn't found in account_infos", program_id); }; - let demote_sysvar_write_locks = - invoke_context.is_feature_active(&demote_sysvar_write_locks::id()); // TODO don't have the caller's keyed_accounts so can't validate writer or signer escalation or deescalation yet let caller_privileges = message .account_keys .iter() .enumerate() - .map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks)) + .map(|(i, _)| message.is_writable(i)) .collect::>(); stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth()); @@ -337,7 +334,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { // Copy writeable account modifications back into the caller's AccountInfos for (i, (pubkey, account)) in accounts.iter().enumerate().take(message.account_keys.len()) { - if !message.is_writable(i, true) { + if !message.is_writable(i) { continue; } for account_info in account_infos { diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 9ad7f9e259..fde574c5cb 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -19,9 +19,8 @@ use solana_sdk::{ entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - blake3_syscall_enabled, cpi_data_cost, demote_sysvar_write_locks, - enforce_aligned_host_addrs, keccak256_syscall_enabled, memory_ops_syscalls, - sysvar_via_syscall, update_data_on_realloc, + blake3_syscall_enabled, cpi_data_cost, enforce_aligned_host_addrs, + keccak256_syscall_enabled, memory_ops_syscalls, sysvar_via_syscall, update_data_on_realloc, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -2187,14 +2186,7 @@ fn call<'a>( signers_seeds_len: u64, memory_mapping: &MemoryMapping, ) -> Result> { - let ( - message, - executables, - accounts, - account_refs, - caller_write_privileges, - demote_sysvar_write_locks, - ) = { + let (message, executables, accounts, account_refs, caller_write_privileges) = { let invoke_context = syscall.get_context()?; invoke_context @@ -2285,7 +2277,6 @@ fn call<'a>( accounts, account_refs, caller_write_privileges, - invoke_context.is_feature_active(&demote_sysvar_write_locks::id()), ) }; @@ -2311,7 +2302,7 @@ fn call<'a>( for (i, ((_key, account), account_ref)) in accounts.iter().zip(account_refs).enumerate() { let account = account.borrow(); if let Some(mut account_ref) = account_ref { - if message.is_writable(i, demote_sysvar_write_locks) && !account.executable() { + if message.is_writable(i) && !account.executable() { *account_ref.lamports = account.lamports(); *account_ref.owner = *account.owner(); if account_ref.data.len() != account.data().len() { diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 0db5583ba7..24bb8c4d74 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -110,9 +110,8 @@ impl TransactionStatusService { }) .expect("FeeCalculator must exist"); let fee = fee_calculator.calculate_fee(transaction.message()); - let (writable_keys, readonly_keys) = transaction - .message - .get_account_keys_by_lock_type(bank.demote_sysvar_write_locks()); + let (writable_keys, readonly_keys) = + transaction.message.get_account_keys_by_lock_type(); let inner_instructions = inner_instructions.map(|inner_instructions| { inner_instructions diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 216701f1db..10ea2aa322 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -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, - demote_sysvar_write_locks: bool, - ) -> Vec> { + pub fn lock_accounts<'a>(&self, txs: impl Iterator) -> Vec> { use solana_sdk::sanitize::Sanitize; let keys: Vec> = 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, 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 diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7db90c7c6c..80855b9719 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2590,20 +2590,15 @@ impl Bank { tick_height % self.ticks_per_slot == 0 } - pub fn demote_sysvar_write_locks(&self) -> bool { - self.feature_set - .is_active(&feature_set::demote_sysvar_write_locks::id()) - } - pub fn prepare_batch<'a, 'b>( &'a self, txs: impl Iterator, ) -> TransactionBatch<'a, 'b> { let hashed_txs: Vec = txs.map(HashedTransaction::from).collect(); - let lock_results = self.rc.accounts.lock_accounts( - hashed_txs.as_transactions_iter(), - self.demote_sysvar_write_locks(), - ); + let lock_results = self + .rc + .accounts + .lock_accounts(hashed_txs.as_transactions_iter()); TransactionBatch::new(lock_results, self, Cow::Owned(hashed_txs)) } @@ -2611,10 +2606,10 @@ impl Bank { &'a self, hashed_txs: &'b [HashedTransaction], ) -> TransactionBatch<'a, 'b> { - let lock_results = self.rc.accounts.lock_accounts( - hashed_txs.as_transactions_iter(), - self.demote_sysvar_write_locks(), - ); + let lock_results = self + .rc + .accounts + .lock_accounts(hashed_txs.as_transactions_iter()); TransactionBatch::new(lock_results, self, Cow::Borrowed(hashed_txs)) } @@ -2693,11 +2688,9 @@ impl Bank { pub fn unlock_accounts(&self, batch: &mut TransactionBatch) { if batch.needs_unlock { batch.needs_unlock = false; - self.rc.accounts.unlock_accounts( - batch.transactions_iter(), - batch.lock_results(), - self.demote_sysvar_write_locks(), - ) + self.rc + .accounts + .unlock_accounts(batch.transactions_iter(), batch.lock_results()) } } @@ -3417,7 +3410,6 @@ impl Bank { &self.rent_collector, &self.last_blockhash_with_fee_calculator(), self.fix_recent_blockhashes_sysvar_delay(), - self.demote_sysvar_write_locks(), ); let rent_debits = self.collect_rent(executed, loaded_txs); @@ -7714,13 +7706,14 @@ pub(crate) mod tests { assert_eq!(bank.get_balance(&sysvar_pubkey), 1); bank.transfer(500, &mint_keypair, &normal_pubkey).unwrap(); - bank.transfer(500, &mint_keypair, &sysvar_pubkey).unwrap(); + bank.transfer(500, &mint_keypair, &sysvar_pubkey) + .unwrap_err(); assert_eq!(bank.get_balance(&normal_pubkey), 500); - assert_eq!(bank.get_balance(&sysvar_pubkey), 501); + assert_eq!(bank.get_balance(&sysvar_pubkey), 1); let bank = Arc::new(new_from_parent(&bank)); assert_eq!(bank.get_balance(&normal_pubkey), 500); - assert_eq!(bank.get_balance(&sysvar_pubkey), 501); + assert_eq!(bank.get_balance(&sysvar_pubkey), 1); } #[test] diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 039a008e79..c1999559cc 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -9,7 +9,7 @@ use solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, - feature_set::{demote_sysvar_write_locks, instructions_sysvar_enabled, FeatureSet}, + feature_set::{instructions_sysvar_enabled, FeatureSet}, ic_logger_msg, ic_msg, instruction::{CompiledInstruction, Instruction, InstructionError}, keyed_account::{create_keyed_accounts_unified, keyed_account_at_index, KeyedAccount}, @@ -299,7 +299,6 @@ impl<'a> ThisInvokeContext<'a> { instruction, executable_accounts, accounts, - feature_set.is_active(&demote_sysvar_write_locks::id()), ); let mut invoke_context = Self { invoke_stack: Vec::with_capacity(bpf_compute_budget.max_invoke_depth), @@ -410,7 +409,6 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { &self.rent, caller_write_privileges, &mut self.timings, - self.feature_set.is_active(&demote_sysvar_write_locks::id()), logger, ) } @@ -613,7 +611,6 @@ impl MessageProcessor { instruction: &'a CompiledInstruction, executable_accounts: &'a [(Pubkey, Rc>)], accounts: &'a [(Pubkey, Rc>)], - demote_sysvar_write_locks: bool, ) -> Vec<(bool, bool, &'a Pubkey, &'a RefCell)> { executable_accounts .iter() @@ -622,7 +619,7 @@ impl MessageProcessor { let index = *index as usize; ( message.is_signer(index), - message.is_writable(index, demote_sysvar_write_locks), + message.is_writable(index), &accounts[index].0, &accounts[index].1 as &RefCell, ) @@ -759,7 +756,6 @@ impl MessageProcessor { accounts, keyed_account_indices_reordered, caller_write_privileges, - demote_sysvar_write_locks, ) = { let invoke_context = invoke_context.borrow(); @@ -852,7 +848,6 @@ impl MessageProcessor { accounts, keyed_account_indices_reordered, caller_write_privileges, - invoke_context.is_feature_active(&demote_sysvar_write_locks::id()), ) }; @@ -877,9 +872,7 @@ impl MessageProcessor { { let dst_keyed_account = &keyed_accounts[dst_keyed_account_index]; let src_keyed_account = account.borrow(); - if message.is_writable(src_keyed_account_index, demote_sysvar_write_locks) - && !src_keyed_account.executable() - { + if message.is_writable(src_keyed_account_index) && !src_keyed_account.executable() { if dst_keyed_account.data_len()? != src_keyed_account.data().len() && dst_keyed_account.data_len()? != 0 { @@ -928,13 +921,8 @@ impl MessageProcessor { )?; // Construct keyed accounts - let keyed_accounts = Self::create_keyed_accounts( - message, - instruction, - executable_accounts, - accounts, - invoke_context.is_feature_active(&demote_sysvar_write_locks::id()), - ); + let keyed_accounts = + Self::create_keyed_accounts(message, instruction, executable_accounts, accounts); // Invoke callee invoke_context.push(program_id, &keyed_accounts)?; @@ -1006,7 +994,6 @@ impl MessageProcessor { accounts: &[(Pubkey, Rc>)], rent: &Rent, timings: &mut ExecuteDetailsTimings, - demote_sysvar_write_locks: bool, logger: Rc>, ) -> Result<(), InstructionError> { // Verify all executable accounts have zero outstanding refs @@ -1028,7 +1015,7 @@ impl MessageProcessor { pre_accounts[unique_index] .verify( program_id, - message.is_writable(account_index, demote_sysvar_write_locks), + message.is_writable(account_index), rent, &account, timings, @@ -1068,7 +1055,6 @@ impl MessageProcessor { rent: &Rent, caller_write_privileges: Option<&[bool]>, timings: &mut ExecuteDetailsTimings, - demote_sysvar_write_locks: bool, logger: Rc>, ) -> Result<(), InstructionError> { // Verify the per-account instruction results @@ -1079,7 +1065,7 @@ impl MessageProcessor { let is_writable = if let Some(caller_write_privileges) = caller_write_privileges { caller_write_privileges[account_index] } else { - message.is_writable(account_index, demote_sysvar_write_locks) + message.is_writable(account_index) }; // Find the matching PreAccount for pre_account in pre_accounts.iter_mut() { @@ -1137,7 +1123,6 @@ impl MessageProcessor { feature_set: Arc, bpf_compute_budget: BpfComputeBudget, timings: &mut ExecuteDetailsTimings, - demote_sysvar_write_locks: bool, account_db: Arc, ancestors: &Ancestors, ) -> Result<(), InstructionError> { @@ -1182,7 +1167,6 @@ impl MessageProcessor { accounts, &rent_collector.rent, timings, - demote_sysvar_write_locks, invoke_context.get_logger(), )?; @@ -1211,7 +1195,6 @@ impl MessageProcessor { account_db: Arc, ancestors: &Ancestors, ) -> Result<(), TransactionError> { - let demote_sysvar_write_locks = feature_set.is_active(&demote_sysvar_write_locks::id()); for (instruction_index, instruction) in message.instructions.iter().enumerate() { let mut time = Measure::start("execute_instruction"); let instruction_recorder = instruction_recorders @@ -1231,7 +1214,6 @@ impl MessageProcessor { feature_set.clone(), bpf_compute_budget, timings, - demote_sysvar_write_locks, account_db.clone(), ancestors, ) @@ -2249,13 +2231,11 @@ mod tests { ); // not owned account modified by the caller (before the invoke) - let demote_sysvar_write_locks = - invoke_context.is_feature_active(&demote_sysvar_write_locks::id()); let caller_write_privileges = message .account_keys .iter() .enumerate() - .map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks)) + .map(|(i, _)| message.is_writable(i)) .collect::>(); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( @@ -2310,7 +2290,7 @@ mod tests { .account_keys .iter() .enumerate() - .map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks)) + .map(|(i, _)| message.is_writable(i)) .collect::>(); assert_eq!( MessageProcessor::process_cross_program_instruction( diff --git a/sdk/benches/serialize_instructions.rs b/sdk/benches/serialize_instructions.rs index 1367c0474b..dae549c98e 100644 --- a/sdk/benches/serialize_instructions.rs +++ b/sdk/benches/serialize_instructions.rs @@ -27,9 +27,7 @@ fn bench_manual_instruction_serialize(b: &mut Bencher) { let instructions = make_instructions(); let message = Message::new(&instructions, None); b.iter(|| { - test::black_box(message.serialize_instructions( - true, // demote_sysvar_write_locks - )); + test::black_box(message.serialize_instructions()); }); } @@ -46,9 +44,7 @@ fn bench_bincode_instruction_deserialize(b: &mut Bencher) { fn bench_manual_instruction_deserialize(b: &mut Bencher) { let instructions = make_instructions(); let message = Message::new(&instructions, None); - let serialized = message.serialize_instructions( - true, // demote_sysvar_write_locks - ); + let serialized = message.serialize_instructions(); b.iter(|| { for i in 0..instructions.len() { test::black_box(instructions::load_instruction_at(i, &serialized).unwrap()); @@ -60,9 +56,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) { fn bench_manual_instruction_deserialize_single(b: &mut Bencher) { let instructions = make_instructions(); let message = Message::new(&instructions, None); - let serialized = message.serialize_instructions( - true, // demote_sysvar_write_locks - ); + let serialized = message.serialize_instructions(); b.iter(|| { test::black_box(instructions::load_instruction_at(3, &serialized).unwrap()); }); diff --git a/sdk/program/src/message.rs b/sdk/program/src/message.rs index 3b21246112..28b44eec37 100644 --- a/sdk/program/src/message.rs +++ b/sdk/program/src/message.rs @@ -391,7 +391,7 @@ impl Message { self.program_position(i).is_some() } - pub fn is_writable(&self, i: usize, demote_sysvar_write_locks: bool) -> bool { + pub fn is_writable(&self, i: usize) -> bool { (i < (self.header.num_required_signatures - self.header.num_readonly_signed_accounts) as usize || (i >= self.header.num_required_signatures as usize @@ -399,8 +399,7 @@ impl Message { - self.header.num_readonly_unsigned_accounts as usize)) && !{ let key = self.account_keys[i]; - demote_sysvar_write_locks - && (sysvar::is_sysvar_id(&key) || BUILTIN_PROGRAMS_KEYS.contains(&key)) + sysvar::is_sysvar_id(&key) || BUILTIN_PROGRAMS_KEYS.contains(&key) } } @@ -408,14 +407,11 @@ impl Message { i < self.header.num_required_signatures as usize } - pub fn get_account_keys_by_lock_type( - &self, - demote_sysvar_write_locks: bool, - ) -> (Vec<&Pubkey>, Vec<&Pubkey>) { + pub fn get_account_keys_by_lock_type(&self) -> (Vec<&Pubkey>, Vec<&Pubkey>) { let mut writable_keys = vec![]; let mut readonly_keys = vec![]; for (i, key) in self.account_keys.iter().enumerate() { - if self.is_writable(i, demote_sysvar_write_locks) { + if self.is_writable(i) { writable_keys.push(key); } else { readonly_keys.push(key); @@ -437,7 +433,7 @@ impl Message { // 35..67 - program_id // 67..69 - data len - u16 // 69..data_len - data - pub fn serialize_instructions(&self, demote_sysvar_write_locks: bool) -> Vec { + pub fn serialize_instructions(&self) -> Vec { // 64 bytes is a reasonable guess, calculating exactly is slower in benchmarks let mut data = Vec::with_capacity(self.instructions.len() * (32 * 2)); append_u16(&mut data, self.instructions.len() as u16); @@ -452,7 +448,7 @@ impl Message { for account_index in &instruction.accounts { let account_index = *account_index as usize; let is_signer = self.is_signer(account_index); - let is_writable = self.is_writable(account_index, demote_sysvar_write_locks); + let is_writable = self.is_writable(account_index); let mut meta_byte = 0; if is_signer { meta_byte |= 1 << Self::IS_SIGNER_BIT; @@ -891,13 +887,12 @@ mod tests { recent_blockhash: Hash::default(), instructions: vec![], }; - let demote_sysvar_write_locks = true; - assert!(message.is_writable(0, demote_sysvar_write_locks)); - assert!(!message.is_writable(1, demote_sysvar_write_locks)); - assert!(!message.is_writable(2, demote_sysvar_write_locks)); - assert!(message.is_writable(3, demote_sysvar_write_locks)); - assert!(message.is_writable(4, demote_sysvar_write_locks)); - assert!(!message.is_writable(5, demote_sysvar_write_locks)); + assert!(message.is_writable(0)); + assert!(!message.is_writable(1)); + assert!(!message.is_writable(2)); + assert!(message.is_writable(3)); + assert!(message.is_writable(4)); + assert!(!message.is_writable(5)); } #[test] @@ -925,9 +920,7 @@ mod tests { Some(&id1), ); assert_eq!( - message.get_account_keys_by_lock_type( - true, // demote_sysvar_write_locks - ), + message.get_account_keys_by_lock_type(), (vec![&id1, &id0], vec![&id3, &id2, &program_id]) ); } @@ -957,9 +950,7 @@ mod tests { ]; let message = Message::new(&instructions, Some(&id1)); - let serialized = message.serialize_instructions( - true, // demote_sysvar_write_locks - ); + let serialized = message.serialize_instructions(); for (i, instruction) in instructions.iter().enumerate() { assert_eq!( Message::deserialize_instruction(i, &serialized).unwrap(), @@ -980,9 +971,7 @@ mod tests { ]; let message = Message::new(&instructions, Some(&id1)); - let serialized = message.serialize_instructions( - true, // demote_sysvar_write_locks - ); + let serialized = message.serialize_instructions(); assert_eq!( Message::deserialize_instruction(instructions.len(), &serialized).unwrap_err(), SanitizeError::IndexOutOfBounds, diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index c210e804a8..cff5fe5b23 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -103,10 +103,6 @@ pub mod upgradeable_close_instruction { solana_sdk::declare_id!("FsPaByos3gA9bUEhp3EimQpQPCoSvCEigHod496NmABQ"); } -pub mod demote_sysvar_write_locks { - solana_sdk::declare_id!("86LJYRuq2zgtHuL3FccR6hqFJQMQkFoun4knAxcPiF1P"); -} - pub mod sysvar_via_syscall { solana_sdk::declare_id!("7411E6gFQLDhQkdRjmpXwM1hzHMMoYQUjHicmvGPC1Nf"); } @@ -177,7 +173,6 @@ lazy_static! { (require_stake_for_gossip::id(), "require stakes for propagating crds values through gossip #15561"), (cpi_data_cost::id(), "charge the compute budget for data passed via CPI"), (upgradeable_close_instruction::id(), "close upgradeable buffer accounts"), - (demote_sysvar_write_locks::id(), "demote builtins and sysvar write locks to readonly #15497"), (sysvar_via_syscall::id(), "provide sysvars via syscalls"), (enforce_aligned_host_addrs::id(), "enforce aligned host addresses"), (update_data_on_realloc::id(), "Retain updated data values modified after realloc via CPI"), diff --git a/transaction-status/src/parse_accounts.rs b/transaction-status/src/parse_accounts.rs index 20bd74a56b..ebef018e71 100644 --- a/transaction-status/src/parse_accounts.rs +++ b/transaction-status/src/parse_accounts.rs @@ -13,7 +13,7 @@ pub fn parse_accounts(message: &Message) -> Vec { for (i, account_key) in message.account_keys.iter().enumerate() { accounts.push(ParsedAccount { pubkey: account_key.to_string(), - writable: message.is_writable(i, /*demote_sysvar_write_locks=*/ true), + writable: message.is_writable(i), signer: message.is_signer(i), }); }