From 51eaa2b9cc02c6dab1571a023b7c580c602d2a7b Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+taozhu-chicago@users.noreply.github.com> Date: Fri, 12 Jan 2024 14:28:50 -0600 Subject: [PATCH] cleanup feature code after activation (#34695) cleanup feature checking code --- program-runtime/src/invoke_context.rs | 13 +--- program-test/src/lib.rs | 7 +- programs/bpf_loader/src/lib.rs | 21 ++---- programs/loader-v4/src/lib.rs | 8 +-- programs/sbf/tests/programs.rs | 5 -- programs/zk-token-proof/src/lib.rs | 93 ++++++++++----------------- runtime/src/bank/tests.rs | 9 +-- 7 files changed, 43 insertions(+), 113 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 3ac29b5a58..7e930fad16 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -19,7 +19,7 @@ use { solana_sdk::{ account::AccountSharedData, bpf_loader_deprecated, - feature_set::{native_programs_consume_cu, FeatureSet}, + feature_set::FeatureSet, hash::Hash, instruction::{AccountMeta, InstructionError}, native_loader, @@ -62,9 +62,6 @@ macro_rules! declare_process_instruction { $inner let consumption_result = if $cu_to_consume > 0 - && invoke_context - .feature_set - .is_active(&solana_sdk::feature_set::native_programs_consume_cu::id()) { invoke_context.consume_checked($cu_to_consume) } else { @@ -522,13 +519,7 @@ impl<'a> InvokeContext<'a> { let post_remaining_units = self.get_remaining(); *compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); - if builtin_id == program_id - && result.is_ok() - && *compute_units_consumed == 0 - && self - .feature_set - .is_active(&native_programs_consume_cu::id()) - { + if builtin_id == program_id && result.is_ok() && *compute_units_consumed == 0 { return Err(InstructionError::BuiltinProgramsMustConsumeComputeUnits); } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 7b946a371c..32dbb276ee 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -501,17 +501,12 @@ impl Default for ProgramTest { let prefer_bpf = std::env::var("BPF_OUT_DIR").is_ok() || std::env::var("SBF_OUT_DIR").is_ok(); - // deactivate feature `native_program_consume_cu` to continue support existing mock/test - // programs that do not consume units. - let deactivate_feature_set = - HashSet::from([solana_sdk::feature_set::native_programs_consume_cu::id()]); - Self { accounts: vec![], builtin_programs: vec![], compute_max_units: None, prefer_bpf, - deactivate_feature_set, + deactivate_feature_set: HashSet::default(), transaction_account_lock_limit: None, } } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 18ca167607..7526b68f82 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -36,8 +36,8 @@ use { feature_set::{ bpf_account_data_direct_mapping, deprecate_executable_meta_update_in_bpf_loader, disable_bpf_loader_instructions, enable_bpf_loader_extend_program_ix, - enable_bpf_loader_set_authority_checked_ix, native_programs_consume_cu, - remove_bpf_loader_incorrect_program_id, FeatureSet, + enable_bpf_loader_set_authority_checked_ix, remove_bpf_loader_incorrect_program_id, + FeatureSet, }, instruction::{AccountMeta, InstructionError}, loader_instruction::LoaderInstruction, @@ -474,29 +474,18 @@ pub fn process_instruction_inner( let program_account = instruction_context.try_borrow_last_program_account(transaction_context)?; - // Consume compute units if feature `native_programs_consume_cu` is activated - let native_programs_consume_cu = invoke_context - .feature_set - .is_active(&native_programs_consume_cu::id()); - // Program Management Instruction if native_loader::check_id(program_account.get_owner()) { drop(program_account); let program_id = instruction_context.get_last_program_key(transaction_context)?; return if bpf_loader_upgradeable::check_id(program_id) { - if native_programs_consume_cu { - invoke_context.consume_checked(UPGRADEABLE_LOADER_COMPUTE_UNITS)?; - } + invoke_context.consume_checked(UPGRADEABLE_LOADER_COMPUTE_UNITS)?; process_loader_upgradeable_instruction(invoke_context) } else if bpf_loader::check_id(program_id) { - if native_programs_consume_cu { - invoke_context.consume_checked(DEFAULT_LOADER_COMPUTE_UNITS)?; - } + invoke_context.consume_checked(DEFAULT_LOADER_COMPUTE_UNITS)?; process_loader_instruction(invoke_context) } else if bpf_loader_deprecated::check_id(program_id) { - if native_programs_consume_cu { - invoke_context.consume_checked(DEPRECATED_LOADER_COMPUTE_UNITS)?; - } + invoke_context.consume_checked(DEPRECATED_LOADER_COMPUTE_UNITS)?; ic_logger_msg!(log_collector, "Deprecated loader is no longer supported"); Err(InstructionError::UnsupportedProgramId) } else { diff --git a/programs/loader-v4/src/lib.rs b/programs/loader-v4/src/lib.rs index 881dc7a1bb..20b413d23e 100644 --- a/programs/loader-v4/src/lib.rs +++ b/programs/loader-v4/src/lib.rs @@ -21,7 +21,6 @@ use { }, solana_sdk::{ entrypoint::SUCCESS, - feature_set, instruction::InstructionError, loader_v4::{self, LoaderV4State, LoaderV4Status, DEPLOYMENT_COOLDOWN_IN_SLOTS}, loader_v4_instruction::LoaderV4Instruction, @@ -554,12 +553,7 @@ pub fn process_instruction_inner( let instruction_data = instruction_context.get_instruction_data(); let program_id = instruction_context.get_last_program_key(transaction_context)?; if loader_v4::check_id(program_id) { - if invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()) - { - invoke_context.consume_checked(DEFAULT_COMPUTE_UNITS)?; - } + invoke_context.consume_checked(DEFAULT_COMPUTE_UNITS)?; match limited_deserialize(instruction_data)? { LoaderV4Instruction::Write { offset, bytes } => { process_instruction_write(invoke_context, offset, bytes) diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index c4006f2055..d7d1b04fb5 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -2523,11 +2523,6 @@ fn test_program_sbf_disguised_as_sbf_loader() { .. } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); - // disable native_programs_consume_cu feature to allow test program - // not consume units. - let mut feature_set = FeatureSet::all_enabled(); - feature_set.deactivate(&solana_sdk::feature_set::native_programs_consume_cu::id()); - bank.feature_set = Arc::new(feature_set); bank.deactivate_feature( &solana_sdk::feature_set::remove_bpf_loader_incorrect_program_id::id(), ); diff --git a/programs/zk-token-proof/src/lib.rs b/programs/zk-token-proof/src/lib.rs index 530856df9e..a4a8dad956 100644 --- a/programs/zk-token-proof/src/lib.rs +++ b/programs/zk-token-proof/src/lib.rs @@ -135,11 +135,6 @@ fn process_close_proof_context(invoke_context: &mut InvokeContext) -> Result<(), } declare_process_instruction!(Entrypoint, 0, |invoke_context| { - // Consume compute units if feature `native_programs_consume_cu` is activated - let native_programs_consume_cu = invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()); - let enable_zk_transfer_with_fee = invoke_context .feature_set .is_active(&feature_set::enable_zk_transfer_with_fee::id()); @@ -159,38 +154,30 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| { match instruction { ProofInstruction::CloseContextState => { - if native_programs_consume_cu { - invoke_context - .consume_checked(CLOSE_CONTEXT_STATE_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(CLOSE_CONTEXT_STATE_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "CloseContextState"); process_close_proof_context(invoke_context) } ProofInstruction::VerifyZeroBalance => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_ZERO_BALANCE_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_ZERO_BALANCE_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyZeroBalance"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyWithdraw => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_WITHDRAW_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_WITHDRAW_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyWithdraw"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyCiphertextCiphertextEquality => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_CIPHERTEXT_CIPHERTEXT_EQUALITY_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_CIPHERTEXT_CIPHERTEXT_EQUALITY_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyCiphertextCiphertextEquality"); process_verify_proof::< CiphertextCiphertextEqualityProofData, @@ -198,11 +185,9 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| { >(invoke_context) } ProofInstruction::VerifyTransfer => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_TRANSFER_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_TRANSFER_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyTransfer"); process_verify_proof::(invoke_context) } @@ -212,49 +197,39 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| { return Err(InstructionError::InvalidInstructionData); } - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_TRANSFER_WITH_FEE_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_TRANSFER_WITH_FEE_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyTransferWithFee"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyPubkeyValidity => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_PUBKEY_VALIDITY_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_PUBKEY_VALIDITY_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyPubkeyValidity"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyRangeProofU64 => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_RANGE_PROOF_U64_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_RANGE_PROOF_U64_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyRangeProof"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyBatchedRangeProofU64 => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U64_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U64_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyBatchedRangeProof64"); process_verify_proof::( invoke_context, ) } ProofInstruction::VerifyBatchedRangeProofU128 => { - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U128_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U128_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyBatchedRangeProof128"); process_verify_proof::( invoke_context, @@ -266,11 +241,9 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| { return Err(InstructionError::InvalidInstructionData); } - if native_programs_consume_cu { - invoke_context - .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U256_COMPUTE_UNITS) - .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; - } + invoke_context + .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U256_COMPUTE_UNITS) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; ic_msg!(invoke_context, "VerifyBatchedRangeProof256"); process_verify_proof::( invoke_context, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 5377473abe..ce2c52ae38 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -14016,14 +14016,7 @@ fn test_failed_simulation_compute_units() { const TEST_UNITS: u64 = 10_000; const MOCK_BUILTIN_UNITS: u64 = 1; - let expected_consumed_units = if bank - .feature_set - .is_active(&solana_sdk::feature_set::native_programs_consume_cu::id()) - { - TEST_UNITS + MOCK_BUILTIN_UNITS - } else { - TEST_UNITS - }; + let expected_consumed_units = TEST_UNITS + MOCK_BUILTIN_UNITS; declare_process_instruction!(MockBuiltin, MOCK_BUILTIN_UNITS, |invoke_context| { invoke_context.consume_checked(TEST_UNITS).unwrap(); Err(InstructionError::InvalidInstructionData)