Skip to content

Commit

Permalink
cleanup feature code after activation (solana-labs#34695)
Browse files Browse the repository at this point in the history 
cleanup feature checking code
  • Loading branch information
@tao-stones
tao-stones authored Jan 12, 2024
1 parent beef847 commit 51eaa2b
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 113 deletions.
13 changes: 2 additions & 11 deletions program-runtime/src/invoke_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
7 changes: 1 addition & 6 deletions program-test/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down
21 changes: 5 additions & 16 deletions programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 1 addition & 7 deletions programs/loader-v4/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions programs/sbf/tests/programs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
Expand Down
93 changes: 33 additions & 60 deletions programs/zk-token-proof/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -159,50 +154,40 @@ 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::<ZeroBalanceProofData, ZeroBalanceProofContext>(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::<WithdrawData, WithdrawProofContext>(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,
CiphertextCiphertextEqualityProofContext,
>(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::<TransferData, TransferProofContext>(invoke_context)
}
Expand All @@ -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::<TransferWithFeeData, TransferWithFeeProofContext>(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::<PubkeyValidityData, PubkeyValidityProofContext>(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::<RangeProofU64Data, RangeProofContext>(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::<BatchedRangeProofU64Data, BatchedRangeProofContext>(
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::<BatchedRangeProofU128Data, BatchedRangeProofContext>(
invoke_context,
Expand All @@ -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::<BatchedRangeProofU256Data, BatchedRangeProofContext>(
invoke_context,
Expand Down
9 changes: 1 addition & 8 deletions runtime/src/bank/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 51eaa2b

Please sign in to comment.