From 6a3389a5cf52fd29a7310ef91f0a3bad5ba17a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:00:00 +0000 Subject: [PATCH 01/14] vote_withdraw_authority_may_change_authorized_voter --- programs/vote/src/vote_processor.rs | 8 -------- programs/vote/src/vote_state/mod.rs | 9 ++------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index 179a925a4b5a9d..c8f3c71ebfdf33 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -1066,14 +1066,6 @@ mod tests { instruction_accounts.clone(), Ok(()), ); - - // should fail, if vote_withdraw_authority_may_change_authorized_voter is disabled - process_instruction_disabled_features( - &instruction_data, - transaction_accounts, - instruction_accounts, - Err(InstructionError::MissingRequiredSignature), - ); } #[test] diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 00668d1ddbd574..9e5d52084f9a68 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -801,13 +801,8 @@ pub fn authorize( match vote_authorize { VoteAuthorize::Voter => { - let authorized_withdrawer_signer = if feature_set - .is_active(&feature_set::vote_withdraw_authority_may_change_authorized_voter::id()) - { - verify_authorized_signer(&vote_state.authorized_withdrawer, signers).is_ok() - } else { - false - }; + let authorized_withdrawer_signer = + verify_authorized_signer(&vote_state.authorized_withdrawer, signers).is_ok(); vote_state.set_new_authorized_voter( authorized, From 331b7f0c73f375737ff7b343d8d9fd70c4413b3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:02:57 +0000 Subject: [PATCH 02/14] limit_secp256k1_recovery_id --- programs/bpf_loader/src/syscalls/mod.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 449668a3a10e8e..73f4ccb1681cf4 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -38,9 +38,8 @@ use { disable_deploy_of_alloc_free_syscall, disable_fees_sysvar, enable_alt_bn128_syscall, enable_big_mod_exp_syscall, enable_early_verification_of_account_modifications, error_on_syscall_bpf_function_hash_collisions, libsecp256k1_0_5_upgrade_enabled, - limit_secp256k1_recovery_id, reject_callx_r10, - stop_sibling_instruction_search_at_parent, stop_truncating_strings_in_syscalls, - switch_to_new_elf_parser, + reject_callx_r10, stop_sibling_instruction_search_at_parent, + stop_truncating_strings_in_syscalls, switch_to_new_elf_parser, }, hash::{Hasher, HASH_BYTES}, instruction::{ @@ -873,18 +872,11 @@ declare_syscall!( return Ok(Secp256k1RecoverError::InvalidHash.into()); } }; - let adjusted_recover_id_val = if invoke_context - .feature_set - .is_active(&limit_secp256k1_recovery_id::id()) - { - match recovery_id_val.try_into() { - Ok(adjusted_recover_id_val) => adjusted_recover_id_val, - Err(_) => { - return Ok(Secp256k1RecoverError::InvalidRecoveryId.into()); - } + let adjusted_recover_id_val = match recovery_id_val.try_into() { + Ok(adjusted_recover_id_val) => adjusted_recover_id_val, + Err(_) => { + return Ok(Secp256k1RecoverError::InvalidRecoveryId.into()); } - } else { - recovery_id_val as u8 }; let recovery_id = match libsecp256k1::RecoveryId::parse(adjusted_recover_id_val) { Ok(id) => id, From 10043164f1fd3c3d66184c0de15d8f5a14221e25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:03:24 +0000 Subject: [PATCH 03/14] spl_token_v3_4_0 --- runtime/src/bank.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 0854575bf71bae..ba01c21ee975b1 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -7478,14 +7478,6 @@ impl Bank { self.rent_collector.rent.burn_percent = 50; // 50% rent burn } - if new_feature_activations.contains(&feature_set::spl_token_v3_4_0::id()) { - self.replace_program_account( - &inline_spl_token::id(), - &inline_spl_token::program_v3_4_0::id(), - "bank-apply_spl_token_v3_4_0", - ); - } - if new_feature_activations.contains(&feature_set::spl_associated_token_account_v1_1_0::id()) { self.replace_program_account( From 1df8470c40f539a5a5015e0daa16b282c119e378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:03:39 +0000 Subject: [PATCH 04/14] spl_associated_token_account_v1_1_0 --- runtime/src/bank.rs | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ba01c21ee975b1..2fb7accca3d16c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -60,7 +60,7 @@ use { cost_tracker::CostTracker, epoch_accounts_hash::{self, EpochAccountsHash}, epoch_stakes::{EpochStakes, NodeVoteAccounts}, - inline_spl_associated_token_account, inline_spl_token, + inline_spl_token, message_processor::MessageProcessor, rent_collector::{CollectedInfo, RentCollector}, rent_debits::RentDebits, @@ -4080,14 +4080,6 @@ impl Bank { balances } - /// Unload a program from the bank's cache - fn remove_program_from_cache(&self, pubkey: &Pubkey) { - self.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*pubkey].into_iter()); - } - fn program_modification_slot(&self, pubkey: &Pubkey) -> Result { let program = self .get_account_with_fixed_root(pubkey) @@ -7478,15 +7470,6 @@ impl Bank { self.rent_collector.rent.burn_percent = 50; // 50% rent burn } - if new_feature_activations.contains(&feature_set::spl_associated_token_account_v1_1_0::id()) - { - self.replace_program_account( - &inline_spl_associated_token_account::id(), - &inline_spl_associated_token_account::program_v1_1_0::id(), - "bank-apply_spl_associated_token_account_v1_1_0", - ); - } - if !debug_do_not_add_builtins { self.apply_builtin_program_feature_transitions( allow_new_activations, @@ -7602,6 +7585,8 @@ impl Bank { } } + /// Use to replace programs by feature activation + #[allow(dead_code)] fn replace_program_account( &mut self, old_address: &Pubkey, @@ -7622,7 +7607,11 @@ impl Bank { // Clear new account self.store_account(new_address, &AccountSharedData::default()); - self.remove_program_from_cache(old_address); + // Unload a program from the bank's cache + self.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*old_address].into_iter()); self.calculate_and_update_accounts_data_size_delta_off_chain( old_account.data().len(), From 477b1f06f33ddbbbd4a2fb5bd53a6ef1a8a112d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:09:37 +0000 Subject: [PATCH 05/14] disable_fee_calculator --- runtime/src/bank.rs | 36 ++++------------------------- runtime/src/serde_snapshot/tests.rs | 12 ++++------ 2 files changed, 10 insertions(+), 38 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2fb7accca3d16c..81aca2797554db 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -123,7 +123,7 @@ use { epoch_schedule::EpochSchedule, feature, feature_set::{ - self, add_set_tx_loaded_accounts_data_size_instruction, disable_fee_calculator, + self, add_set_tx_loaded_accounts_data_size_instruction, enable_early_verification_of_account_modifications, enable_request_heap_frame_ix, include_loaded_accounts_data_size_in_fee_calculation, remove_congestion_multiplier_from_fee_calculation, remove_deprecated_request_unit_ix, @@ -766,7 +766,6 @@ impl PartialEq for Bank { block_height, collector_id, collector_fees, - fee_calculator, fee_rate_governor, collected_rent, rent_collector, @@ -824,7 +823,6 @@ impl PartialEq for Bank { && block_height == &other.block_height && collector_id == &other.collector_id && collector_fees.load(Relaxed) == other.collector_fees.load(Relaxed) - && fee_calculator == &other.fee_calculator && fee_rate_governor == &other.fee_rate_governor && collected_rent.load(Relaxed) == other.collected_rent.load(Relaxed) && rent_collector == &other.rent_collector @@ -986,10 +984,6 @@ pub struct Bank { /// Fees that have been collected collector_fees: AtomicU64, - /// Deprecated, do not use - /// Latest transaction fees for transactions processed by this bank - pub(crate) fee_calculator: FeeCalculator, - /// Track cluster signature throughput and adjust fee rate pub(crate) fee_rate_governor: FeeRateGovernor, @@ -1267,7 +1261,6 @@ impl Bank { block_height: u64::default(), collector_id: Pubkey::default(), collector_fees: AtomicU64::default(), - fee_calculator: FeeCalculator::default(), fee_rate_governor: FeeRateGovernor::default(), collected_rent: AtomicU64::default(), rent_collector: RentCollector::default(), @@ -1469,17 +1462,9 @@ impl Bank { let (status_cache, status_cache_time_us) = measure_us!(Arc::clone(&parent.status_cache)); - let ((fee_rate_governor, fee_calculator), fee_components_time_us) = measure_us!({ - let fee_rate_governor = - FeeRateGovernor::new_derived(&parent.fee_rate_governor, parent.signature_count()); - - let fee_calculator = if parent.feature_set.is_active(&disable_fee_calculator::id()) { - FeeCalculator::default() - } else { - fee_rate_governor.create_fee_calculator() - }; - (fee_rate_governor, fee_calculator) - }); + let (fee_rate_governor, fee_components_time_us) = measure_us!( + FeeRateGovernor::new_derived(&parent.fee_rate_governor, parent.signature_count()) + ); let bank_id = rc.bank_id_generator.fetch_add(1, Relaxed) + 1; let (blockhash_queue, blockhash_queue_time_us) = @@ -1527,7 +1512,6 @@ impl Bank { rent_collector: Self::get_rent_collector_from(&parent.rent_collector, epoch), max_tick_height: (slot + 1) * parent.ticks_per_slot, block_height: parent.block_height + 1, - fee_calculator, fee_rate_governor, capitalization: AtomicU64::new(parent.capitalization()), vote_only_bank, @@ -1883,7 +1867,6 @@ impl Bank { block_height: fields.block_height, collector_id: fields.collector_id, collector_fees: AtomicU64::new(fields.collector_fees), - fee_calculator: fields.fee_calculator, fee_rate_governor: fields.fee_rate_governor, collected_rent: AtomicU64::new(fields.collected_rent), // clone()-ing is needed to consider a gated behavior in rent_collector @@ -1951,14 +1934,6 @@ impl Bank { ); assert_eq!(bank.epoch_schedule, genesis_config.epoch_schedule); assert_eq!(bank.epoch, bank.epoch_schedule.get_epoch(bank.slot)); - if !bank.feature_set.is_active(&disable_fee_calculator::id()) { - bank.fee_rate_governor.lamports_per_signature = - bank.fee_calculator.lamports_per_signature; - assert_eq!( - bank.fee_rate_governor.create_fee_calculator(), - bank.fee_calculator - ); - } datapoint_info!( "bank-new-from-fields", @@ -2008,7 +1983,7 @@ impl Bank { block_height: self.block_height, collector_id: self.collector_id, collector_fees: self.collector_fees.load(Relaxed), - fee_calculator: self.fee_calculator, + fee_calculator: FeeCalculator::default(), fee_rate_governor: self.fee_rate_governor.clone(), collected_rent: self.collected_rent.load(Relaxed), rent_collector: self.rent_collector.clone(), @@ -3301,7 +3276,6 @@ impl Bank { fn process_genesis_config(&mut self, genesis_config: &GenesisConfig) { // Bootstrap validator collects fees until `new_from_parent` is called. self.fee_rate_governor = genesis_config.fee_rate_governor.clone(); - self.fee_calculator = self.fee_rate_governor.create_fee_calculator(); for (pubkey, account) in genesis_config.accounts.iter() { assert!( diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index eca735a43751bf..ee85df4d5366fc 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -12,7 +12,7 @@ use { accounts_hash::{AccountsDeltaHash, AccountsHash}, bank::{Bank, BankTestConfig}, epoch_accounts_hash, - genesis_utils::{self, activate_all_features, activate_feature}, + genesis_utils::{activate_all_features, activate_feature}, snapshot_utils::{ create_tmp_accounts_dir_for_tests, get_storages_to_serialize, ArchiveFormat, }, @@ -23,7 +23,7 @@ use { solana_sdk::{ account::{AccountSharedData, ReadableAccount}, clock::Slot, - feature_set::{self, disable_fee_calculator}, + feature_set, genesis_config::{create_genesis_config, ClusterType}, hash::Hash, pubkey::Pubkey, @@ -236,7 +236,7 @@ fn test_bank_serialize_style( ) { solana_logger::setup(); let (mut genesis_config, _) = create_genesis_config(500); - genesis_utils::activate_feature(&mut genesis_config, feature_set::epoch_accounts_hash::id()); + activate_feature(&mut genesis_config, feature_set::epoch_accounts_hash::id()); genesis_config.epoch_schedule = EpochSchedule::custom(400, 400, false); let bank0 = Arc::new(Bank::new_for_tests(&genesis_config)); let eah_start_slot = epoch_accounts_hash::calculation_start(&bank0); @@ -509,8 +509,7 @@ fn add_root_and_flush_write_cache(bank: &Bank) { #[test] fn test_extra_fields_eof() { solana_logger::setup(); - let (mut genesis_config, _) = create_genesis_config(500); - activate_feature(&mut genesis_config, disable_fee_calculator::id()); + let (genesis_config, _) = create_genesis_config(500); let bank0 = Arc::new(Bank::new_for_tests_with_config( &genesis_config, @@ -646,8 +645,7 @@ fn test_extra_fields_full_snapshot_archive() { #[test] fn test_blank_extra_fields() { solana_logger::setup(); - let (mut genesis_config, _) = create_genesis_config(500); - activate_feature(&mut genesis_config, disable_fee_calculator::id()); + let (genesis_config, _) = create_genesis_config(500); let bank0 = Arc::new(Bank::new_for_tests_with_config( &genesis_config, From dd8af1b81a7c3330b691a4e2819a88cbec71ba81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:10:27 +0000 Subject: [PATCH 06/14] vote_authorize_with_seed --- programs/vote/src/vote_processor.rs | 41 ++--------------------------- 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index c8f3c71ebfdf33..e003c62437cfc2 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -28,12 +28,6 @@ fn process_authorize_with_seed_instruction( current_authority_derived_key_owner: &Pubkey, current_authority_derived_key_seed: &str, ) -> Result<(), InstructionError> { - if !invoke_context - .feature_set - .is_active(&feature_set::vote_authorize_with_seed::id()) - { - return Err(InstructionError::InvalidInstructionData); - } let clock = get_sysvar_with_account_check::clock(invoke_context, instruction_context, 1)?; let mut expected_authority_keys: HashSet = HashSet::default(); if instruction_context.is_instruction_account_signer(2)? { @@ -1348,22 +1342,6 @@ mod tests { // Can change authority when base key signs for related derived key. process_instruction( - &serialize(&VoteInstruction::AuthorizeWithSeed( - VoteAuthorizeWithSeedArgs { - authorization_type, - current_authority_derived_key_owner: current_authority_owner, - current_authority_derived_key_seed: current_authority_seed.clone(), - new_authority: new_authority_pubkey, - }, - )) - .unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Ok(()), - ); - - // Should fail when the `vote_authorize_with_seed` feature is disabled - process_instruction_disabled_features( &serialize(&VoteInstruction::AuthorizeWithSeed( VoteAuthorizeWithSeedArgs { authorization_type, @@ -1375,7 +1353,7 @@ mod tests { .unwrap(), transaction_accounts, instruction_accounts, - Err(InstructionError::InvalidInstructionData), + Ok(()), ); } @@ -1489,21 +1467,6 @@ mod tests { // Can change authority when base key signs for related derived key and new authority signs. process_instruction( - &serialize(&VoteInstruction::AuthorizeCheckedWithSeed( - VoteAuthorizeCheckedWithSeedArgs { - authorization_type, - current_authority_derived_key_owner: current_authority_owner, - current_authority_derived_key_seed: current_authority_seed.clone(), - }, - )) - .unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Ok(()), - ); - - // Should fail when the `vote_authorize_with_seed` feature is disabled - process_instruction_disabled_features( &serialize(&VoteInstruction::AuthorizeCheckedWithSeed( VoteAuthorizeCheckedWithSeedArgs { authorization_type, @@ -1514,7 +1477,7 @@ mod tests { .unwrap(), transaction_accounts, instruction_accounts, - Err(InstructionError::InvalidInstructionData), + Ok(()), ); } From 835e7c8f68440da582ffa3680db5d774651e0417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:15:50 +0000 Subject: [PATCH 07/14] merge_nonce_error_into_system_error --- cli/src/nonce.rs | 102 +++------------------- programs/system/src/system_instruction.rs | 45 ++-------- sdk/program/src/system_instruction.rs | 45 +++------- 3 files changed, 26 insertions(+), 166 deletions(-) diff --git a/cli/src/nonce.rs b/cli/src/nonce.rs index 533f64139ff37a..399ffa64f7363d 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -2,11 +2,10 @@ use { crate::{ checks::{check_account_for_fee_with_commitment, check_unique_pubkeys}, cli::{ - log_instruction_custom_error, log_instruction_custom_error_ex, CliCommand, - CliCommandInfo, CliConfig, CliError, ProcessResult, + log_instruction_custom_error, CliCommand, CliCommandInfo, CliConfig, CliError, + ProcessResult, }, compute_unit_price::WithComputeUnitPrice, - feature::get_feature_is_active, memo::WithMemo, spend_utils::{resolve_spend_tx_and_check_account_balance, SpendAmount}, }, @@ -25,19 +24,17 @@ use { solana_rpc_client_nonce_utils::*, solana_sdk::{ account::Account, - feature_set::merge_nonce_error_into_system_error, hash::Hash, - instruction::InstructionError, message::Message, nonce::{self, State}, pubkey::Pubkey, system_instruction::{ advance_nonce_account, authorize_nonce_account, create_nonce_account, - create_nonce_account_with_seed, instruction_to_nonce_error, upgrade_nonce_account, - withdraw_nonce_account, NonceError, SystemError, + create_nonce_account_with_seed, upgrade_nonce_account, withdraw_nonce_account, + SystemError, }, system_program, - transaction::{Transaction, TransactionError}, + transaction::Transaction, }, std::sync::Arc, }; @@ -427,21 +424,9 @@ pub fn process_authorize_nonce_account( &tx.message, config.commitment, )?; - let merge_errors = - get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?; let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx); - if merge_errors { - log_instruction_custom_error::(result, config) - } else { - log_instruction_custom_error_ex::(result, config, |ix_error| { - if let InstructionError::Custom(_) = ix_error { - instruction_to_nonce_error(ix_error, merge_errors) - } else { - None - } - }) - } + log_instruction_custom_error::(result, config) } pub fn process_create_nonce_account( @@ -524,40 +509,9 @@ pub fn process_create_nonce_account( let mut tx = Transaction::new_unsigned(message); tx.try_sign(&config.signers, latest_blockhash)?; - let merge_errors = - get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?; let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx); - let err_ix_index = if let Err(err) = &result { - err.get_transaction_error().and_then(|tx_err| { - if let TransactionError::InstructionError(ix_index, _) = tx_err { - Some(ix_index) - } else { - None - } - }) - } else { - None - }; - - match err_ix_index { - // SystemInstruction::InitializeNonceAccount failed - Some(1) => { - if merge_errors { - log_instruction_custom_error::(result, config) - } else { - log_instruction_custom_error_ex::(result, config, |ix_error| { - if let InstructionError::Custom(_) = ix_error { - instruction_to_nonce_error(ix_error, merge_errors) - } else { - None - } - }) - } - } - // SystemInstruction::CreateAccount{,WithSeed} failed - _ => log_instruction_custom_error::(result, config), - } + log_instruction_custom_error::(result, config) } pub fn process_get_nonce( @@ -611,21 +565,9 @@ pub fn process_new_nonce( &tx.message, config.commitment, )?; - let merge_errors = - get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?; let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx); - if merge_errors { - log_instruction_custom_error::(result, config) - } else { - log_instruction_custom_error_ex::(result, config, |ix_error| { - if let InstructionError::Custom(_) = ix_error { - instruction_to_nonce_error(ix_error, merge_errors) - } else { - None - } - }) - } + log_instruction_custom_error::(result, config) } pub fn process_show_nonce_account( @@ -688,21 +630,9 @@ pub fn process_withdraw_from_nonce_account( &tx.message, config.commitment, )?; - let merge_errors = - get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?; let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx); - if merge_errors { - log_instruction_custom_error::(result, config) - } else { - log_instruction_custom_error_ex::(result, config, |ix_error| { - if let InstructionError::Custom(_) = ix_error { - instruction_to_nonce_error(ix_error, merge_errors) - } else { - None - } - }) - } + log_instruction_custom_error::(result, config) } pub(crate) fn process_upgrade_nonce_account( @@ -725,20 +655,8 @@ pub(crate) fn process_upgrade_nonce_account( &tx.message, config.commitment, )?; - let merge_errors = - get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?; let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx); - if merge_errors { - log_instruction_custom_error::(result, config) - } else { - log_instruction_custom_error_ex::(result, config, |ix_error| { - if let InstructionError::Custom(_) = ix_error { - instruction_to_nonce_error(ix_error, merge_errors) - } else { - None - } - }) - } + log_instruction_custom_error::(result, config) } #[cfg(test)] diff --git a/programs/system/src/system_instruction.rs b/programs/system/src/system_instruction.rs index c0e2eb0f35df9b..95860379fb17a0 100644 --- a/programs/system/src/system_instruction.rs +++ b/programs/system/src/system_instruction.rs @@ -1,7 +1,6 @@ use { solana_program_runtime::{ic_msg, invoke_context::InvokeContext}, solana_sdk::{ - feature_set, instruction::{checked_add, InstructionError}, nonce::{ self, @@ -9,7 +8,7 @@ use { State, }, pubkey::Pubkey, - system_instruction::{nonce_to_instruction_error, NonceError}, + system_instruction::SystemError, sysvar::rent::Rent, transaction_context::{ BorrowedAccount, IndexOfAccount, InstructionContext, TransactionContext, @@ -23,10 +22,6 @@ pub fn advance_nonce_account( signers: &HashSet, invoke_context: &InvokeContext, ) -> Result<(), InstructionError> { - let merge_nonce_error_into_system_error = invoke_context - .feature_set - .is_active(&feature_set::merge_nonce_error_into_system_error::id()); - if !account.is_writable() { ic_msg!( invoke_context, @@ -53,10 +48,7 @@ pub fn advance_nonce_account( invoke_context, "Advance nonce account: nonce can only advance once per slot" ); - return Err(nonce_to_instruction_error( - NonceError::NotExpired, - merge_nonce_error_into_system_error, - )); + return Err(SystemError::NonceBlockhashNotExpired.into()); } let new_data = nonce::state::Data::new( @@ -72,10 +64,7 @@ pub fn advance_nonce_account( "Advance nonce account: Account {} state is invalid", account.get_key() ); - Err(nonce_to_instruction_error( - NonceError::BadAccountState, - merge_nonce_error_into_system_error, - )) + Err(InstructionError::InvalidAccountData) } } } @@ -92,10 +81,6 @@ pub fn withdraw_nonce_account( ) -> Result<(), InstructionError> { let mut from = instruction_context .try_borrow_instruction_account(transaction_context, from_account_index)?; - let merge_nonce_error_into_system_error = invoke_context - .feature_set - .is_active(&feature_set::merge_nonce_error_into_system_error::id()); - if !from.is_writable() { ic_msg!( invoke_context, @@ -127,10 +112,7 @@ pub fn withdraw_nonce_account( invoke_context, "Withdraw nonce account: nonce can only advance once per slot" ); - return Err(nonce_to_instruction_error( - NonceError::NotExpired, - merge_nonce_error_into_system_error, - )); + return Err(SystemError::NonceBlockhashNotExpired.into()); } from.set_state(&Versions::new(State::Uninitialized))?; } else { @@ -174,10 +156,6 @@ pub fn initialize_nonce_account( rent: &Rent, invoke_context: &InvokeContext, ) -> Result<(), InstructionError> { - let merge_nonce_error_into_system_error = invoke_context - .feature_set - .is_active(&feature_set::merge_nonce_error_into_system_error::id()); - if !account.is_writable() { ic_msg!( invoke_context, @@ -214,10 +192,7 @@ pub fn initialize_nonce_account( "Initialize nonce account: Account {} state is invalid", account.get_key() ); - Err(nonce_to_instruction_error( - NonceError::BadAccountState, - merge_nonce_error_into_system_error, - )) + Err(InstructionError::InvalidAccountData) } } } @@ -228,10 +203,6 @@ pub fn authorize_nonce_account( signers: &HashSet, invoke_context: &InvokeContext, ) -> Result<(), InstructionError> { - let merge_nonce_error_into_system_error = invoke_context - .feature_set - .is_active(&feature_set::merge_nonce_error_into_system_error::id()); - if !account.is_writable() { ic_msg!( invoke_context, @@ -251,10 +222,7 @@ pub fn authorize_nonce_account( "Authorize nonce account: Account {} state is invalid", account.get_key() ); - Err(nonce_to_instruction_error( - NonceError::BadAccountState, - merge_nonce_error_into_system_error, - )) + Err(InstructionError::InvalidAccountData) } Err(AuthorizeNonceError::MissingRequiredSignature(account_authority)) => { ic_msg!( @@ -278,7 +246,6 @@ mod test { hash::hash, nonce::{self, State}, nonce_account::{create_account, verify_nonce_account}, - system_instruction::SystemError, system_program, transaction_context::InstructionAccount, }, diff --git a/sdk/program/src/system_instruction.rs b/sdk/program/src/system_instruction.rs index 7500274f458120..1fddf8b7690eaf 100644 --- a/sdk/program/src/system_instruction.rs +++ b/sdk/program/src/system_instruction.rs @@ -128,21 +128,12 @@ impl From for NonceError { } } -pub fn nonce_to_instruction_error(error: NonceError, use_system_variant: bool) -> InstructionError { - if use_system_variant { - match error { - NonceError::NoRecentBlockhashes => SystemError::NonceNoRecentBlockhashes.into(), - NonceError::NotExpired => SystemError::NonceBlockhashNotExpired.into(), - NonceError::UnexpectedValue => SystemError::NonceUnexpectedBlockhashValue.into(), - NonceError::BadAccountState => InstructionError::InvalidAccountData, - } - } else { - match error { - NonceError::NoRecentBlockhashes => NonceErrorAdapter::NoRecentBlockhashes.into(), - NonceError::NotExpired => NonceErrorAdapter::NotExpired.into(), - NonceError::UnexpectedValue => NonceErrorAdapter::UnexpectedValue.into(), - NonceError::BadAccountState => NonceErrorAdapter::BadAccountState.into(), - } +pub fn nonce_to_instruction_error(error: NonceError) -> InstructionError { + match error { + NonceError::NoRecentBlockhashes => SystemError::NonceNoRecentBlockhashes.into(), + NonceError::NotExpired => SystemError::NonceBlockhashNotExpired.into(), + NonceError::UnexpectedValue => SystemError::NonceUnexpectedBlockhashValue.into(), + NonceError::BadAccountState => InstructionError::InvalidAccountData, } } @@ -1945,35 +1936,19 @@ mod tests { #[test] fn test_nonce_to_instruction_error() { assert_eq!( - nonce_to_instruction_error(NonceError::NoRecentBlockhashes, false), - NonceError::NoRecentBlockhashes.into(), - ); - assert_eq!( - nonce_to_instruction_error(NonceError::NotExpired, false), - NonceError::NotExpired.into(), - ); - assert_eq!( - nonce_to_instruction_error(NonceError::UnexpectedValue, false), - NonceError::UnexpectedValue.into(), - ); - assert_eq!( - nonce_to_instruction_error(NonceError::BadAccountState, false), - NonceError::BadAccountState.into(), - ); - assert_eq!( - nonce_to_instruction_error(NonceError::NoRecentBlockhashes, true), + nonce_to_instruction_error(NonceError::NoRecentBlockhashes), SystemError::NonceNoRecentBlockhashes.into(), ); assert_eq!( - nonce_to_instruction_error(NonceError::NotExpired, true), + nonce_to_instruction_error(NonceError::NotExpired), SystemError::NonceBlockhashNotExpired.into(), ); assert_eq!( - nonce_to_instruction_error(NonceError::UnexpectedValue, true), + nonce_to_instruction_error(NonceError::UnexpectedValue), SystemError::NonceUnexpectedBlockhashValue.into(), ); assert_eq!( - nonce_to_instruction_error(NonceError::BadAccountState, true), + nonce_to_instruction_error(NonceError::BadAccountState), InstructionError::InvalidAccountData, ); } From b96251e470dbca2fbbc87f761ae85a9863f81c83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:27:11 +0000 Subject: [PATCH 08/14] instructions_sysvar_owned_by_sysvar --- runtime/src/accounts.rs | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index a53e44682b0522..e10aa743651d03 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -55,7 +55,6 @@ use { pubkey::Pubkey, saturating_add_assign, slot_hashes::SlotHashes, - system_program, sysvar::{self, instructions::construct_instructions_data}, transaction::{Result, SanitizedTransaction, TransactionAccountLocks, TransactionError}, transaction_context::{IndexOfAccount, TransactionAccount}, @@ -224,19 +223,10 @@ impl Accounts { } } - fn construct_instructions_account( - message: &SanitizedMessage, - is_owned_by_sysvar: bool, - ) -> AccountSharedData { - let data = construct_instructions_data(&message.decompile_instructions()); - let owner = if is_owned_by_sysvar { - sysvar::id() - } else { - system_program::id() - }; + fn construct_instructions_account(message: &SanitizedMessage) -> AccountSharedData { AccountSharedData::from(Account { - data, - owner, + data: construct_instructions_data(&message.decompile_instructions()), + owner: sysvar::id(), ..Account::default() }) } @@ -382,11 +372,7 @@ impl Accounts { let mut account_found = true; #[allow(clippy::collapsible_else_if)] let account = if solana_sdk::sysvar::instructions::check_id(key) { - Self::construct_instructions_account( - message, - feature_set - .is_active(&feature_set::instructions_sysvar_owned_by_sysvar::id()), - ) + Self::construct_instructions_account(message) } else { let instruction_account = u8::try_from(i) .map(|i| instruction_accounts.contains(&&i)) From 731dd277c541de8b2b171daeaebb0fec412bdecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:35:11 +0000 Subject: [PATCH 09/14] require_static_program_ids_in_transaction --- banks-server/src/banks_server.rs | 2 - core/src/banking_stage/consumer.rs | 1 - core/src/read_write_account_set.rs | 1 - entry/benches/entry_sigverify.rs | 2 - entry/src/entry.rs | 1 - ledger-tool/src/main.rs | 1 - ledger/src/blockstore.rs | 11 +-- rpc/src/rpc.rs | 10 +-- rpc/src/transaction_status_service.rs | 1 - runtime/src/bank.rs | 20 +---- runtime/src/cost_tracker.rs | 1 - sdk/program/src/message/versions/mod.rs | 4 +- sdk/program/src/message/versions/sanitized.rs | 2 +- sdk/program/src/message/versions/v0/mod.rs | 78 +++++-------------- sdk/src/transaction/sanitized.rs | 3 +- sdk/src/transaction/versioned/mod.rs | 7 +- transaction-status/src/lib.rs | 8 +- 17 files changed, 33 insertions(+), 120 deletions(-) diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 21491038f68182..d9c59bdc5cea54 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -178,7 +178,6 @@ fn simulate_transaction( MessageHash::Compute, Some(false), // is_simple_vote_tx bank, - true, // require_static_program_ids ) { Err(err) => { return BanksTransactionResultWithSimulation { @@ -330,7 +329,6 @@ impl Banks for BanksServer { MessageHash::Compute, Some(false), // is_simple_vote_tx bank.as_ref(), - true, // require_static_program_ids ) { Ok(tx) => tx, Err(err) => return Some(Err(err)), diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 05a0a7382dd423..bc26a119fa44ff 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -1656,7 +1656,6 @@ mod tests { MessageHash::Compute, Some(false), bank.as_ref(), - true, // require_static_program_ids ) .unwrap(); diff --git a/core/src/read_write_account_set.rs b/core/src/read_write_account_set.rs index a0a8b41ca30819..455ae4804b8706 100644 --- a/core/src/read_write_account_set.rs +++ b/core/src/read_write_account_set.rs @@ -157,7 +157,6 @@ mod tests { MessageHash::Compute, Some(false), bank, - true, // require_static_program_ids ) .unwrap() } diff --git a/entry/benches/entry_sigverify.rs b/entry/benches/entry_sigverify.rs index 1f4abf9182a9ef..b3a1b7b5cdb3e6 100644 --- a/entry/benches/entry_sigverify.rs +++ b/entry/benches/entry_sigverify.rs @@ -40,7 +40,6 @@ fn bench_gpusigverify(bencher: &mut Bencher) { message_hash, None, SimpleAddressLoader::Disabled, - true, // require_static_program_ids ) }?; @@ -82,7 +81,6 @@ fn bench_cpusigverify(bencher: &mut Bencher) { message_hash, None, SimpleAddressLoader::Disabled, - true, // require_static_program_ids ) }?; diff --git a/entry/src/entry.rs b/entry/src/entry.rs index 8d7304b7d2dac7..8db0f739dba5f7 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -1041,7 +1041,6 @@ mod tests { message_hash, None, SimpleAddressLoader::Disabled, - true, // require_static_program_ids ) }?; diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index adca0249785114..92345e5f50b619 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -922,7 +922,6 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> MessageHash::Compute, None, SimpleAddressLoader::Disabled, - true, // require_static_program_ids ) .map_err(|err| { warn!("Failed to compute cost of transaction: {:?}", err); diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 3e9ac1cfa330d2..b06198d122f817 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -1984,12 +1984,7 @@ impl Blockstore { .into_iter() .flat_map(|entry| entry.transactions) .map(|transaction| { - if let Err(err) = transaction.sanitize( - // Don't enable additional sanitization checks until - // all clusters have activated the static program id - // feature gate so that bigtable upload isn't affected - false, // require_static_program_ids - ) { + if let Err(err) = transaction.sanitize() { warn!( "Blockstore::get_block sanitize failed: {:?}, \ slot: {:?}, \ @@ -2376,9 +2371,7 @@ impl Blockstore { .cloned() .flat_map(|entry| entry.transactions) .map(|transaction| { - if let Err(err) = transaction.sanitize( - true, // require_static_program_ids - ) { + if let Err(err) = transaction.sanitize() { warn!( "Blockstore::find_transaction_in_slot sanitize failed: {:?}, \ slot: {:?}, \ diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 188199969c71f0..0422d7bf7702d3 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -4516,14 +4516,8 @@ fn sanitize_transaction( transaction: VersionedTransaction, address_loader: impl AddressLoader, ) -> Result { - SanitizedTransaction::try_create( - transaction, - MessageHash::Compute, - None, - address_loader, - true, // require_static_program_ids - ) - .map_err(|err| Error::invalid_params(format!("invalid transaction: {err}"))) + SanitizedTransaction::try_create(transaction, MessageHash::Compute, None, address_loader) + .map_err(|err| Error::invalid_params(format!("invalid transaction: {err}"))) } pub fn create_validator_exit(exit: &Arc) -> Arc> { diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index e47673123ec86b..ed8f8dfc01da0d 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -350,7 +350,6 @@ pub(crate) mod tests { MessageHash::Compute, None, SimpleAddressLoader::Disabled, - true, // require_static_program_ids ) .unwrap(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 81aca2797554db..a71c247d22e9cb 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3719,16 +3719,7 @@ impl Bank { pub fn prepare_entry_batch(&self, txs: Vec) -> Result { let sanitized_txs = txs .into_iter() - .map(|tx| { - SanitizedTransaction::try_create( - tx, - MessageHash::Compute, - None, - self, - self.feature_set - .is_active(&feature_set::require_static_program_ids_in_transaction::ID), - ) - }) + .map(|tx| SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self)) .collect::>>()?; let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_results = self @@ -6807,14 +6798,7 @@ impl Bank { tx.message.hash() }; - SanitizedTransaction::try_create( - tx, - message_hash, - None, - self, - self.feature_set - .is_active(&feature_set::require_static_program_ids_in_transaction::ID), - ) + SanitizedTransaction::try_create(tx, message_hash, None, self) }?; if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index 08991a854cb075..5e98af47633e3f 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -367,7 +367,6 @@ mod tests { MessageHash::Compute, Some(true), SimpleAddressLoader::Disabled, - true, // require_static_program_ids ) .unwrap(); let mut tx_cost = TransactionCost::new_with_capacity(1); diff --git a/sdk/program/src/message/versions/mod.rs b/sdk/program/src/message/versions/mod.rs index fd95bd85f80b08..ccef565fd51022 100644 --- a/sdk/program/src/message/versions/mod.rs +++ b/sdk/program/src/message/versions/mod.rs @@ -39,10 +39,10 @@ pub enum VersionedMessage { } impl VersionedMessage { - pub fn sanitize(&self, require_static_program_ids: bool) -> Result<(), SanitizeError> { + pub fn sanitize(&self) -> Result<(), SanitizeError> { match self { Self::Legacy(message) => message.sanitize(), - Self::V0(message) => message.sanitize(require_static_program_ids), + Self::V0(message) => message.sanitize(), } } diff --git a/sdk/program/src/message/versions/sanitized.rs b/sdk/program/src/message/versions/sanitized.rs index 38264fc30778e9..5d44f7ed4829eb 100644 --- a/sdk/program/src/message/versions/sanitized.rs +++ b/sdk/program/src/message/versions/sanitized.rs @@ -18,7 +18,7 @@ impl TryFrom for SanitizedVersionedMessage { impl SanitizedVersionedMessage { pub fn try_new(message: VersionedMessage) -> Result { - message.sanitize(true /* require_static_program_ids */)?; + message.sanitize()?; Ok(Self { message }) } diff --git a/sdk/program/src/message/versions/v0/mod.rs b/sdk/program/src/message/versions/v0/mod.rs index f264e286ad1c5e..c2f87cf508c3f0 100644 --- a/sdk/program/src/message/versions/v0/mod.rs +++ b/sdk/program/src/message/versions/v0/mod.rs @@ -89,7 +89,7 @@ pub struct Message { impl Message { /// Sanitize message fields and compiled instruction indexes - pub fn sanitize(&self, reject_dynamic_program_ids: bool) -> Result<(), SanitizeError> { + pub fn sanitize(&self) -> Result<(), SanitizeError> { let num_static_account_keys = self.account_keys.len(); if usize::from(self.header.num_required_signatures) .saturating_add(usize::from(self.header.num_readonly_unsigned_accounts)) @@ -143,18 +143,15 @@ impl Message { .checked_sub(1) .expect("message doesn't contain any account keys"); - // switch to rejecting program ids loaded from lookup tables so that - // static analysis on program instructions can be performed without - // loading on-chain data from a bank - let max_program_id_ix = if reject_dynamic_program_ids { + // reject program ids loaded from lookup tables so that + // static analysis on program instructions can be performed + // without loading on-chain data from a bank + let max_program_id_ix = // `expect` is safe because of earlier check that // `num_static_account_keys` is non-zero num_static_account_keys .checked_sub(1) - .expect("message doesn't contain any static account keys") - } else { - max_account_ix - }; + .expect("message doesn't contain any static account keys"); for ci in &self.instructions { if usize::from(ci.program_id_index) > max_program_id_ix { @@ -375,9 +372,7 @@ mod tests { account_keys: vec![Pubkey::new_unique()], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_ok()); } @@ -396,9 +391,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_ok()); } @@ -417,9 +410,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_ok()); } @@ -444,13 +435,7 @@ mod tests { ..Message::default() }; - assert!(message.sanitize( - false, // require_static_program_ids - ).is_ok()); - - assert!(message.sanitize( - true, // require_static_program_ids - ).is_err()); + assert!(message.sanitize().is_err()); } #[test] @@ -473,9 +458,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_ok()); } @@ -486,9 +469,7 @@ mod tests { account_keys: vec![Pubkey::new_unique()], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_err()); } @@ -503,9 +484,7 @@ mod tests { account_keys: vec![Pubkey::new_unique()], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_err()); } @@ -524,9 +503,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_err()); } @@ -540,9 +517,7 @@ mod tests { account_keys: (0..=u8::MAX).map(|_| Pubkey::new_unique()).collect(), ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_ok()); } @@ -556,9 +531,7 @@ mod tests { account_keys: (0..=256).map(|_| Pubkey::new_unique()).collect(), ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_err()); } @@ -577,9 +550,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_ok()); } @@ -598,9 +569,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_err()); } @@ -625,12 +594,7 @@ mod tests { ..Message::default() }; - assert!(message - .sanitize(true /* require_static_program_ids */) - .is_err()); - assert!(message - .sanitize(false /* require_static_program_ids */) - .is_err()); + assert!(message.sanitize().is_err()); } #[test] @@ -653,9 +617,7 @@ mod tests { }], ..Message::default() } - .sanitize( - true, // require_static_program_ids - ) + .sanitize() .is_err()); } diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index 006400779057c9..62ef6dcd6053bf 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -95,9 +95,8 @@ impl SanitizedTransaction { message_hash: impl Into, is_simple_vote_tx: Option, address_loader: impl AddressLoader, - require_static_program_ids: bool, ) -> Result { - tx.sanitize(require_static_program_ids)?; + tx.sanitize()?; let message_hash = match message_hash.into() { MessageHash::Compute => tx.message.hash(), diff --git a/sdk/src/transaction/versioned/mod.rs b/sdk/src/transaction/versioned/mod.rs index 29e385e6279d7e..9faecf2dceb7eb 100644 --- a/sdk/src/transaction/versioned/mod.rs +++ b/sdk/src/transaction/versioned/mod.rs @@ -115,11 +115,8 @@ impl VersionedTransaction { }) } - pub fn sanitize( - &self, - require_static_program_ids: bool, - ) -> std::result::Result<(), SanitizeError> { - self.message.sanitize(require_static_program_ids)?; + pub fn sanitize(&self) -> std::result::Result<(), SanitizeError> { + self.message.sanitize()?; self.sanitize_signatures()?; Ok(()) } diff --git a/transaction-status/src/lib.rs b/transaction-status/src/lib.rs index 7c9877a26beeea..bb1ddd42d00155 100644 --- a/transaction-status/src/lib.rs +++ b/transaction-status/src/lib.rs @@ -1131,13 +1131,7 @@ impl EncodedTransaction { .and_then(|bytes| bincode::deserialize(&bytes).ok()), }; - transaction.filter(|transaction| { - transaction - .sanitize( - true, // require_static_program_ids - ) - .is_ok() - }) + transaction.filter(|transaction| transaction.sanitize().is_ok()) } } From e913167312de10a4454450ce3bc56f2ba17b51f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:39:40 +0000 Subject: [PATCH 10/14] include_account_index_in_rent_error --- runtime/src/account_rent_state.rs | 13 ++++--------- runtime/src/accounts.rs | 4 +--- runtime/src/bank/transaction_account_state_info.rs | 5 ----- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/runtime/src/account_rent_state.rs b/runtime/src/account_rent_state.rs index 0c971adebdd8e8..284dca2bc39be4 100644 --- a/runtime/src/account_rent_state.rs +++ b/runtime/src/account_rent_state.rs @@ -78,7 +78,6 @@ pub(crate) fn check_rent_state( post_rent_state: Option<&RentState>, transaction_context: &TransactionContext, index: IndexOfAccount, - include_account_index_in_err: bool, ) -> Result<()> { if let Some((pre_rent_state, post_rent_state)) = pre_rent_state.zip(post_rent_state) { let expect_msg = "account must exist at TransactionContext index if rent-states are Some"; @@ -92,7 +91,7 @@ pub(crate) fn check_rent_state( .get_account_at_index(index) .expect(expect_msg) .borrow(), - include_account_index_in_err.then_some(index), + index, )?; } Ok(()) @@ -103,7 +102,7 @@ pub(crate) fn check_rent_state_with_account( post_rent_state: &RentState, address: &Pubkey, account_state: &AccountSharedData, - account_index: Option, + account_index: IndexOfAccount, ) -> Result<()> { submit_rent_state_metrics(pre_rent_state, post_rent_state); if !solana_sdk::incinerator::check_id(address) @@ -113,12 +112,8 @@ pub(crate) fn check_rent_state_with_account( "Account {} not rent exempt, state {:?}", address, account_state, ); - if let Some(account_index) = account_index { - let account_index = account_index as u8; - Err(TransactionError::InsufficientFundsForRent { account_index }) - } else { - Err(TransactionError::InvalidRentPayingAccount) - } + let account_index = account_index as u8; + Err(TransactionError::InsufficientFundsForRent { account_index }) } else { Ok(()) } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index e10aa743651d03..0c1f23a121d106 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -623,9 +623,7 @@ impl Accounts { &payer_post_rent_state, payer_address, payer_account, - feature_set - .is_active(&feature_set::include_account_index_in_rent_error::ID) - .then_some(payer_index), + payer_index, ) } diff --git a/runtime/src/bank/transaction_account_state_info.rs b/runtime/src/bank/transaction_account_state_info.rs index 92b84d0a176f96..45f5d59e4cbc69 100644 --- a/runtime/src/bank/transaction_account_state_info.rs +++ b/runtime/src/bank/transaction_account_state_info.rs @@ -5,7 +5,6 @@ use { }, solana_sdk::{ account::ReadableAccount, - feature_set, message::SanitizedMessage, native_loader, transaction::Result, @@ -61,9 +60,6 @@ impl Bank { post_state_infos: &[TransactionAccountStateInfo], transaction_context: &TransactionContext, ) -> Result<()> { - let include_account_index_in_err = self - .feature_set - .is_active(&feature_set::include_account_index_in_rent_error::id()); for (i, (pre_state_info, post_state_info)) in pre_state_infos.iter().zip(post_state_infos).enumerate() { @@ -72,7 +68,6 @@ impl Bank { post_state_info.rent_state.as_ref(), transaction_context, i as IndexOfAccount, - include_account_index_in_err, )?; } Ok(()) From 586dd7a7c06a259c858a9f5d7c2cdfa2d37dea0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:41:44 +0000 Subject: [PATCH 11/14] filter_votes_outside_slot_hashes --- programs/vote/src/vote_state/mod.rs | 123 ++++++++-------------------- 1 file changed, 34 insertions(+), 89 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 9e5d52084f9a68..0e07c687290394 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -10,7 +10,7 @@ use { account::{AccountSharedData, ReadableAccount, WritableAccount}, clock::{Epoch, Slot, UnixTimestamp}, epoch_schedule::EpochSchedule, - feature_set::{self, filter_votes_outside_slot_hashes, FeatureSet}, + feature_set::{self, FeatureSet}, hash::Hash, instruction::InstructionError, pubkey::Pubkey, @@ -727,26 +727,23 @@ pub fn process_vote( vote: &Vote, slot_hashes: &[SlotHash], epoch: Epoch, - feature_set: Option<&FeatureSet>, + filter_vote_slots: bool, ) -> Result<(), VoteError> { if vote.slots.is_empty() { return Err(VoteError::EmptySlots); } - let filtered_vote_slots = feature_set.and_then(|feature_set| { - if feature_set.is_active(&filter_votes_outside_slot_hashes::id()) { - let earliest_slot_in_history = - slot_hashes.last().map(|(slot, _hash)| *slot).unwrap_or(0); - Some( - vote.slots - .iter() - .filter(|slot| **slot >= earliest_slot_in_history) - .cloned() - .collect::>(), - ) - } else { - None - } - }); + let filtered_vote_slots = if filter_vote_slots { + let earliest_slot_in_history = slot_hashes.last().map(|(slot, _hash)| *slot).unwrap_or(0); + Some( + vote.slots + .iter() + .filter(|slot| **slot >= earliest_slot_in_history) + .cloned() + .collect::>(), + ) + } else { + None + }; let vote_slots = filtered_vote_slots.as_ref().unwrap_or(&vote.slots); if vote_slots.is_empty() { @@ -769,7 +766,7 @@ pub fn process_vote_unchecked(vote_state: &mut VoteState, vote: Vote) { &vote, &slot_hashes, vote_state.current_epoch(), - None, + false, ); } @@ -1016,13 +1013,7 @@ pub fn process_vote_with_account( ) -> Result<(), InstructionError> { let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?; - process_vote( - &mut vote_state, - vote, - slot_hashes, - clock.epoch, - Some(feature_set), - )?; + process_vote(&mut vote_state, vote, slot_hashes, clock.epoch, true)?; if let Some(timestamp) = vote.timestamp { vote.slots .iter() @@ -1179,19 +1170,19 @@ mod tests { vote_state.increment_credits(0, 100); assert_eq!( vote_state - .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 0, 1, |_pubkey| Ok(()),), + .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 0, 1, |_pubkey| Ok(())), Ok(()) ); vote_state.increment_credits(1, 200); assert_eq!( vote_state - .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 1, 2, |_pubkey| Ok(()),), + .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 1, 2, |_pubkey| Ok(())), Ok(()) ); vote_state.increment_credits(2, 300); assert_eq!( vote_state - .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 2, 3, |_pubkey| Ok(()),), + .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 2, 3, |_pubkey| Ok(())), Ok(()) ); // Simulate votes having occurred @@ -1493,23 +1484,11 @@ mod tests { let slot_hashes: Vec<_> = vote.slots.iter().rev().map(|x| (*x, vote.hash)).collect(); assert_eq!( - process_vote( - &mut vote_state_a, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state_a, &vote, &slot_hashes, 0, true), Ok(()) ); assert_eq!( - process_vote( - &mut vote_state_b, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state_b, &vote, &slot_hashes, 0, true), Ok(()) ); assert_eq!(recent_votes(&vote_state_a), recent_votes(&vote_state_b)); @@ -1522,24 +1501,12 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(0, vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true), Ok(()) ); let recent = recent_votes(&vote_state); assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true), Err(VoteError::VoteTooOld) ); assert_eq!(recent, recent_votes(&vote_state)); @@ -1599,13 +1566,7 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true), Ok(()) ); assert_eq!( @@ -1621,13 +1582,7 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true), Ok(()) ); @@ -1646,13 +1601,7 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true), Ok(()) ); @@ -1669,7 +1618,7 @@ mod tests { let vote = Vote::new(vec![], Hash::default()); assert_eq!( - process_vote(&mut vote_state, &vote, &[], 0, Some(&FeatureSet::default())), + process_vote(&mut vote_state, &vote, &[], 0, true), Err(VoteError::EmptySlots) ); } @@ -1787,7 +1736,7 @@ mod tests { let current_epoch = vote_state1.current_epoch(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::TooManyVotes) ); } @@ -1848,7 +1797,7 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::ZeroConfirmations) ); @@ -1859,7 +1808,7 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::ZeroConfirmations) ); } @@ -2012,7 +1961,7 @@ mod tests { // Slot 7 should have expired slot 0 assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::NewVoteStateLockoutMismatch) ); } @@ -2315,10 +2264,6 @@ mod tests { #[test] fn test_filter_old_votes() { - // Enable feature - let mut feature_set = FeatureSet::default(); - feature_set.activate(&filter_votes_outside_slot_hashes::id(), 0); - let mut vote_state = VoteState::default(); let old_vote_slot = 1; let vote = Vote::new(vec![old_vote_slot], Hash::default()); @@ -2327,7 +2272,7 @@ mod tests { // error with `VotesTooOldAllFiltered` let slot_hashes = vec![(3, Hash::new_unique()), (2, Hash::new_unique())]; assert_eq!( - process_vote(&mut vote_state, &vote, &slot_hashes, 0, Some(&feature_set),), + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true), Err(VoteError::VotesTooOldAllFiltered) ); @@ -2341,7 +2286,7 @@ mod tests { .1; let vote = Vote::new(vec![old_vote_slot, vote_slot], vote_slot_hash); - process_vote(&mut vote_state, &vote, &slot_hashes, 0, Some(&feature_set)).unwrap(); + process_vote(&mut vote_state, &vote, &slot_hashes, 0, true).unwrap(); assert_eq!( vote_state .votes @@ -2374,7 +2319,7 @@ mod tests { &Vote::new(vote_slots, vote_hash), slot_hashes, 0, - None, + false, ) .unwrap(); } From 04071f56fe56c9affeaad4671d5fe437e8fb025a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:47:23 +0000 Subject: [PATCH 12/14] reject_vote_account_close_unless_zero_credit_epoch --- programs/vote/src/vote_processor.rs | 35 +++-------------------------- programs/vote/src/vote_state/mod.rs | 9 ++++---- 2 files changed, 8 insertions(+), 36 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index e003c62437cfc2..88e258174ac00f 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -218,15 +218,7 @@ declare_process_instruction!(process_instruction, 2100, |invoke_context| { VoteInstruction::Withdraw(lamports) => { instruction_context.check_number_of_instruction_accounts(2)?; let rent_sysvar = invoke_context.get_sysvar_cache().get_rent()?; - - let clock_if_feature_active = if invoke_context - .feature_set - .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()) - { - Some(invoke_context.get_sysvar_cache().get_clock()?) - } else { - None - }; + let clock_sysvar = invoke_context.get_sysvar_cache().get_clock()?; drop(me); vote_state::withdraw( @@ -237,7 +229,7 @@ declare_process_instruction!(process_instruction, 2100, |invoke_context| { 1, &signers, &rent_sysvar, - clock_if_feature_active.as_deref(), + &clock_sysvar, &invoke_context.feature_set, ) } @@ -1224,31 +1216,10 @@ mod tests { // full withdraw, without 0 credit epoch instruction_accounts[0].pubkey = vote_pubkey_2; process_instruction( - &serialize(&VoteInstruction::Withdraw(lamports)).unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Err(VoteError::ActiveVoteAccountClose.into()), - ); - - // Following features disabled: - // reject_vote_account_close_unless_zero_credit_epoch - - // full withdraw, with 0 credit epoch - instruction_accounts[0].pubkey = vote_pubkey_1; - process_instruction_disabled_features( - &serialize(&VoteInstruction::Withdraw(lamports)).unwrap(), - transaction_accounts.clone(), - instruction_accounts.clone(), - Ok(()), - ); - - // full withdraw, without 0 credit epoch - instruction_accounts[0].pubkey = vote_pubkey_2; - process_instruction_disabled_features( &serialize(&VoteInstruction::Withdraw(lamports)).unwrap(), transaction_accounts, instruction_accounts, - Ok(()), + Err(VoteError::ActiveVoteAccountClose.into()), ); } diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 0e07c687290394..c17ece5cccc49d 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -905,7 +905,7 @@ pub fn withdraw( to_account_index: IndexOfAccount, signers: &HashSet, rent_sysvar: &Rent, - clock: Option<&Clock>, + clock: &Clock, feature_set: &FeatureSet, ) -> Result<(), InstructionError> { let mut vote_account = instruction_context @@ -922,9 +922,10 @@ pub fn withdraw( .ok_or(InstructionError::InsufficientFunds)?; if remaining_balance == 0 { - let reject_active_vote_account_close = clock - .zip(vote_state.epoch_credits.last()) - .map(|(clock, (last_epoch_with_credits, _, _))| { + let reject_active_vote_account_close = vote_state + .epoch_credits + .last() + .map(|(last_epoch_with_credits, _, _)| { let current_epoch = clock.epoch; // if current_epoch - last_epoch_with_credits < 2 then the validator has received credits // either in the current epoch or the previous epoch. If it's >= 2 then it has been at least From dd5c55a7cb5d3275bb8071d6da76e52d3474fd4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:48:53 +0000 Subject: [PATCH 13/14] bank_transaction_count_fix --- runtime/src/bank/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 59fad31f23daad..fa3cdc2bec1ac2 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -2663,8 +2663,7 @@ fn test_insufficient_funds() { fn test_executed_transaction_count_post_bank_transaction_count_fix() { let mint_amount = sol_to_lamports(1.); let (genesis_config, mint_keypair) = create_genesis_config(mint_amount); - let mut bank = Bank::new_for_tests(&genesis_config); - bank.activate_feature(&feature_set::bank_transaction_count_fix::id()); + let bank = Bank::new_for_tests(&genesis_config); let pubkey = solana_sdk::pubkey::new_rand(); let amount = genesis_config.rent.minimum_balance(0); bank.transfer(amount, &mint_keypair, &pubkey).unwrap(); From e95f53026e067037182408ac581221347b00a9a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 22 May 2023 17:49:34 +0000 Subject: [PATCH 14/14] check_syscall_outputs_do_not_overlap --- programs/bpf_loader/src/syscalls/mod.rs | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 73f4ccb1681cf4..ab40a9e2a69106 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -33,10 +33,10 @@ use { feature_set::bpf_account_data_direct_mapping, feature_set::FeatureSet, feature_set::{ - self, blake3_syscall_enabled, check_syscall_outputs_do_not_overlap, - curve25519_syscall_enabled, disable_cpi_setting_executable_and_rent_epoch, - disable_deploy_of_alloc_free_syscall, disable_fees_sysvar, enable_alt_bn128_syscall, - enable_big_mod_exp_syscall, enable_early_verification_of_account_modifications, + self, blake3_syscall_enabled, curve25519_syscall_enabled, + disable_cpi_setting_executable_and_rent_epoch, disable_deploy_of_alloc_free_syscall, + disable_fees_sysvar, enable_alt_bn128_syscall, enable_big_mod_exp_syscall, + enable_early_verification_of_account_modifications, error_on_syscall_bpf_function_hash_collisions, libsecp256k1_0_5_upgrade_enabled, reject_callx_r10, stop_sibling_instruction_search_at_parent, stop_truncating_strings_in_syscalls, switch_to_new_elf_parser, @@ -685,10 +685,7 @@ declare_syscall!( std::mem::size_of_val(bump_seed_ref), address.as_ptr() as usize, std::mem::size_of::(), - ) && invoke_context - .feature_set - .is_active(&check_syscall_outputs_do_not_overlap::id()) - { + ) { return Err(SyscallError::CopyOverlapping.into()); } *bump_seed_ref = bump_seed[0]; @@ -1438,10 +1435,7 @@ declare_syscall!( length as usize, program_id_result as *const _ as usize, std::mem::size_of::(), - ) && invoke_context - .feature_set - .is_active(&check_syscall_outputs_do_not_overlap::id()) - { + ) { return Err(SyscallError::CopyOverlapping.into()); } @@ -1530,7 +1524,7 @@ declare_syscall!( invoke_context.get_check_size(), )?; - if (!is_nonoverlapping( + if !is_nonoverlapping( result_header as *const _ as usize, std::mem::size_of::(), program_id as *const _ as usize, @@ -1563,10 +1557,7 @@ declare_syscall!( accounts.as_ptr() as usize, std::mem::size_of::() .saturating_mul(result_header.accounts_len as usize), - )) && invoke_context - .feature_set - .is_active(&check_syscall_outputs_do_not_overlap::id()) - { + ) { return Err(SyscallError::CopyOverlapping.into()); }