cleanup feature code after activation (#34695)

cleanup feature checking code
This commit is contained in:
Tao Zhu 2024-01-12 14:28:50 -06:00 committed by GitHub
parent beef8476f8
commit 51eaa2b9cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 43 additions and 113 deletions

View File

@ -19,7 +19,7 @@ use {
solana_sdk::{ solana_sdk::{
account::AccountSharedData, account::AccountSharedData,
bpf_loader_deprecated, bpf_loader_deprecated,
feature_set::{native_programs_consume_cu, FeatureSet}, feature_set::FeatureSet,
hash::Hash, hash::Hash,
instruction::{AccountMeta, InstructionError}, instruction::{AccountMeta, InstructionError},
native_loader, native_loader,
@ -62,9 +62,6 @@ macro_rules! declare_process_instruction {
$inner $inner
let consumption_result = if $cu_to_consume > 0 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) invoke_context.consume_checked($cu_to_consume)
} else { } else {
@ -522,13 +519,7 @@ impl<'a> InvokeContext<'a> {
let post_remaining_units = self.get_remaining(); let post_remaining_units = self.get_remaining();
*compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); *compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units);
if builtin_id == program_id if builtin_id == program_id && result.is_ok() && *compute_units_consumed == 0 {
&& result.is_ok()
&& *compute_units_consumed == 0
&& self
.feature_set
.is_active(&native_programs_consume_cu::id())
{
return Err(InstructionError::BuiltinProgramsMustConsumeComputeUnits); return Err(InstructionError::BuiltinProgramsMustConsumeComputeUnits);
} }

View File

@ -501,17 +501,12 @@ impl Default for ProgramTest {
let prefer_bpf = let prefer_bpf =
std::env::var("BPF_OUT_DIR").is_ok() || std::env::var("SBF_OUT_DIR").is_ok(); 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 { Self {
accounts: vec![], accounts: vec![],
builtin_programs: vec![], builtin_programs: vec![],
compute_max_units: None, compute_max_units: None,
prefer_bpf, prefer_bpf,
deactivate_feature_set, deactivate_feature_set: HashSet::default(),
transaction_account_lock_limit: None, transaction_account_lock_limit: None,
} }
} }

View File

@ -36,8 +36,8 @@ use {
feature_set::{ feature_set::{
bpf_account_data_direct_mapping, deprecate_executable_meta_update_in_bpf_loader, bpf_account_data_direct_mapping, deprecate_executable_meta_update_in_bpf_loader,
disable_bpf_loader_instructions, enable_bpf_loader_extend_program_ix, disable_bpf_loader_instructions, enable_bpf_loader_extend_program_ix,
enable_bpf_loader_set_authority_checked_ix, native_programs_consume_cu, enable_bpf_loader_set_authority_checked_ix, remove_bpf_loader_incorrect_program_id,
remove_bpf_loader_incorrect_program_id, FeatureSet, FeatureSet,
}, },
instruction::{AccountMeta, InstructionError}, instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction, loader_instruction::LoaderInstruction,
@ -474,29 +474,18 @@ pub fn process_instruction_inner(
let program_account = let program_account =
instruction_context.try_borrow_last_program_account(transaction_context)?; 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 // Program Management Instruction
if native_loader::check_id(program_account.get_owner()) { if native_loader::check_id(program_account.get_owner()) {
drop(program_account); drop(program_account);
let program_id = instruction_context.get_last_program_key(transaction_context)?; let program_id = instruction_context.get_last_program_key(transaction_context)?;
return if bpf_loader_upgradeable::check_id(program_id) { 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) process_loader_upgradeable_instruction(invoke_context)
} else if bpf_loader::check_id(program_id) { } 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) process_loader_instruction(invoke_context)
} else if bpf_loader_deprecated::check_id(program_id) { } 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"); ic_logger_msg!(log_collector, "Deprecated loader is no longer supported");
Err(InstructionError::UnsupportedProgramId) Err(InstructionError::UnsupportedProgramId)
} else { } else {

View File

@ -21,7 +21,6 @@ use {
}, },
solana_sdk::{ solana_sdk::{
entrypoint::SUCCESS, entrypoint::SUCCESS,
feature_set,
instruction::InstructionError, instruction::InstructionError,
loader_v4::{self, LoaderV4State, LoaderV4Status, DEPLOYMENT_COOLDOWN_IN_SLOTS}, loader_v4::{self, LoaderV4State, LoaderV4Status, DEPLOYMENT_COOLDOWN_IN_SLOTS},
loader_v4_instruction::LoaderV4Instruction, loader_v4_instruction::LoaderV4Instruction,
@ -554,12 +553,7 @@ pub fn process_instruction_inner(
let instruction_data = instruction_context.get_instruction_data(); let instruction_data = instruction_context.get_instruction_data();
let program_id = instruction_context.get_last_program_key(transaction_context)?; let program_id = instruction_context.get_last_program_key(transaction_context)?;
if loader_v4::check_id(program_id) { if loader_v4::check_id(program_id) {
if invoke_context invoke_context.consume_checked(DEFAULT_COMPUTE_UNITS)?;
.feature_set
.is_active(&feature_set::native_programs_consume_cu::id())
{
invoke_context.consume_checked(DEFAULT_COMPUTE_UNITS)?;
}
match limited_deserialize(instruction_data)? { match limited_deserialize(instruction_data)? {
LoaderV4Instruction::Write { offset, bytes } => { LoaderV4Instruction::Write { offset, bytes } => {
process_instruction_write(invoke_context, offset, bytes) process_instruction_write(invoke_context, offset, bytes)

View File

@ -2523,11 +2523,6 @@ fn test_program_sbf_disguised_as_sbf_loader() {
.. ..
} = create_genesis_config(50); } = create_genesis_config(50);
let mut bank = Bank::new_for_tests(&genesis_config); 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( bank.deactivate_feature(
&solana_sdk::feature_set::remove_bpf_loader_incorrect_program_id::id(), &solana_sdk::feature_set::remove_bpf_loader_incorrect_program_id::id(),
); );

View File

@ -135,11 +135,6 @@ fn process_close_proof_context(invoke_context: &mut InvokeContext) -> Result<(),
} }
declare_process_instruction!(Entrypoint, 0, |invoke_context| { 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 let enable_zk_transfer_with_fee = invoke_context
.feature_set .feature_set
.is_active(&feature_set::enable_zk_transfer_with_fee::id()); .is_active(&feature_set::enable_zk_transfer_with_fee::id());
@ -159,38 +154,30 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| {
match instruction { match instruction {
ProofInstruction::CloseContextState => { ProofInstruction::CloseContextState => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(CLOSE_CONTEXT_STATE_COMPUTE_UNITS)
.consume_checked(CLOSE_CONTEXT_STATE_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "CloseContextState"); ic_msg!(invoke_context, "CloseContextState");
process_close_proof_context(invoke_context) process_close_proof_context(invoke_context)
} }
ProofInstruction::VerifyZeroBalance => { ProofInstruction::VerifyZeroBalance => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_ZERO_BALANCE_COMPUTE_UNITS)
.consume_checked(VERIFY_ZERO_BALANCE_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyZeroBalance"); ic_msg!(invoke_context, "VerifyZeroBalance");
process_verify_proof::<ZeroBalanceProofData, ZeroBalanceProofContext>(invoke_context) process_verify_proof::<ZeroBalanceProofData, ZeroBalanceProofContext>(invoke_context)
} }
ProofInstruction::VerifyWithdraw => { ProofInstruction::VerifyWithdraw => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_WITHDRAW_COMPUTE_UNITS)
.consume_checked(VERIFY_WITHDRAW_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyWithdraw"); ic_msg!(invoke_context, "VerifyWithdraw");
process_verify_proof::<WithdrawData, WithdrawProofContext>(invoke_context) process_verify_proof::<WithdrawData, WithdrawProofContext>(invoke_context)
} }
ProofInstruction::VerifyCiphertextCiphertextEquality => { ProofInstruction::VerifyCiphertextCiphertextEquality => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_CIPHERTEXT_CIPHERTEXT_EQUALITY_COMPUTE_UNITS)
.consume_checked(VERIFY_CIPHERTEXT_CIPHERTEXT_EQUALITY_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyCiphertextCiphertextEquality"); ic_msg!(invoke_context, "VerifyCiphertextCiphertextEquality");
process_verify_proof::< process_verify_proof::<
CiphertextCiphertextEqualityProofData, CiphertextCiphertextEqualityProofData,
@ -198,11 +185,9 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| {
>(invoke_context) >(invoke_context)
} }
ProofInstruction::VerifyTransfer => { ProofInstruction::VerifyTransfer => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_TRANSFER_COMPUTE_UNITS)
.consume_checked(VERIFY_TRANSFER_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyTransfer"); ic_msg!(invoke_context, "VerifyTransfer");
process_verify_proof::<TransferData, TransferProofContext>(invoke_context) process_verify_proof::<TransferData, TransferProofContext>(invoke_context)
} }
@ -212,49 +197,39 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| {
return Err(InstructionError::InvalidInstructionData); return Err(InstructionError::InvalidInstructionData);
} }
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_TRANSFER_WITH_FEE_COMPUTE_UNITS)
.consume_checked(VERIFY_TRANSFER_WITH_FEE_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyTransferWithFee"); ic_msg!(invoke_context, "VerifyTransferWithFee");
process_verify_proof::<TransferWithFeeData, TransferWithFeeProofContext>(invoke_context) process_verify_proof::<TransferWithFeeData, TransferWithFeeProofContext>(invoke_context)
} }
ProofInstruction::VerifyPubkeyValidity => { ProofInstruction::VerifyPubkeyValidity => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_PUBKEY_VALIDITY_COMPUTE_UNITS)
.consume_checked(VERIFY_PUBKEY_VALIDITY_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyPubkeyValidity"); ic_msg!(invoke_context, "VerifyPubkeyValidity");
process_verify_proof::<PubkeyValidityData, PubkeyValidityProofContext>(invoke_context) process_verify_proof::<PubkeyValidityData, PubkeyValidityProofContext>(invoke_context)
} }
ProofInstruction::VerifyRangeProofU64 => { ProofInstruction::VerifyRangeProofU64 => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_RANGE_PROOF_U64_COMPUTE_UNITS)
.consume_checked(VERIFY_RANGE_PROOF_U64_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyRangeProof"); ic_msg!(invoke_context, "VerifyRangeProof");
process_verify_proof::<RangeProofU64Data, RangeProofContext>(invoke_context) process_verify_proof::<RangeProofU64Data, RangeProofContext>(invoke_context)
} }
ProofInstruction::VerifyBatchedRangeProofU64 => { ProofInstruction::VerifyBatchedRangeProofU64 => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U64_COMPUTE_UNITS)
.consume_checked(VERIFY_BATCHED_RANGE_PROOF_U64_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyBatchedRangeProof64"); ic_msg!(invoke_context, "VerifyBatchedRangeProof64");
process_verify_proof::<BatchedRangeProofU64Data, BatchedRangeProofContext>( process_verify_proof::<BatchedRangeProofU64Data, BatchedRangeProofContext>(
invoke_context, invoke_context,
) )
} }
ProofInstruction::VerifyBatchedRangeProofU128 => { ProofInstruction::VerifyBatchedRangeProofU128 => {
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U128_COMPUTE_UNITS)
.consume_checked(VERIFY_BATCHED_RANGE_PROOF_U128_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyBatchedRangeProof128"); ic_msg!(invoke_context, "VerifyBatchedRangeProof128");
process_verify_proof::<BatchedRangeProofU128Data, BatchedRangeProofContext>( process_verify_proof::<BatchedRangeProofU128Data, BatchedRangeProofContext>(
invoke_context, invoke_context,
@ -266,11 +241,9 @@ declare_process_instruction!(Entrypoint, 0, |invoke_context| {
return Err(InstructionError::InvalidInstructionData); return Err(InstructionError::InvalidInstructionData);
} }
if native_programs_consume_cu { invoke_context
invoke_context .consume_checked(VERIFY_BATCHED_RANGE_PROOF_U256_COMPUTE_UNITS)
.consume_checked(VERIFY_BATCHED_RANGE_PROOF_U256_COMPUTE_UNITS) .map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
.map_err(|_| InstructionError::ComputationalBudgetExceeded)?;
}
ic_msg!(invoke_context, "VerifyBatchedRangeProof256"); ic_msg!(invoke_context, "VerifyBatchedRangeProof256");
process_verify_proof::<BatchedRangeProofU256Data, BatchedRangeProofContext>( process_verify_proof::<BatchedRangeProofU256Data, BatchedRangeProofContext>(
invoke_context, invoke_context,

View File

@ -14016,14 +14016,7 @@ fn test_failed_simulation_compute_units() {
const TEST_UNITS: u64 = 10_000; const TEST_UNITS: u64 = 10_000;
const MOCK_BUILTIN_UNITS: u64 = 1; const MOCK_BUILTIN_UNITS: u64 = 1;
let expected_consumed_units = if bank let expected_consumed_units = TEST_UNITS + MOCK_BUILTIN_UNITS;
.feature_set
.is_active(&solana_sdk::feature_set::native_programs_consume_cu::id())
{
TEST_UNITS + MOCK_BUILTIN_UNITS
} else {
TEST_UNITS
};
declare_process_instruction!(MockBuiltin, MOCK_BUILTIN_UNITS, |invoke_context| { declare_process_instruction!(MockBuiltin, MOCK_BUILTIN_UNITS, |invoke_context| {
invoke_context.consume_checked(TEST_UNITS).unwrap(); invoke_context.consume_checked(TEST_UNITS).unwrap();
Err(InstructionError::InvalidInstructionData) Err(InstructionError::InvalidInstructionData)