clean feature: `prevent_calling_precompiles_as_programs` (#27100)

* clean feature: prevent_calling_precompiles_as_programs

* fix tests

* fix test

* remove comment

* fix test

* feedback
This commit is contained in:
Justin Starry 2022-08-18 06:21:16 +01:00 committed by GitHub
parent d2d4d4a240
commit 7d765e3d67
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 20 additions and 82 deletions

View File

@ -834,7 +834,6 @@ fn check_authorized_program(
instruction_data: &[u8],
invoke_context: &InvokeContext,
) -> Result<(), EbpfError<BpfError>> {
#[allow(clippy::blocks_in_if_conditions)]
if native_loader::check_id(program_id)
|| bpf_loader::check_id(program_id)
|| bpf_loader_deprecated::check_id(program_id)
@ -842,12 +841,9 @@ fn check_authorized_program(
&& !(bpf_loader_upgradeable::is_upgrade_instruction(instruction_data)
|| bpf_loader_upgradeable::is_set_authority_instruction(instruction_data)
|| bpf_loader_upgradeable::is_close_instruction(instruction_data)))
|| (invoke_context
.feature_set
.is_active(&prevent_calling_precompiles_as_programs::id())
&& is_precompile(program_id, |feature_id: &Pubkey| {
invoke_context.feature_set.is_active(feature_id)
}))
|| is_precompile(program_id, |feature_id: &Pubkey| {
invoke_context.feature_set.is_active(feature_id)
})
{
return Err(SyscallError::ProgramNotSupported(*program_id).into());
}

View File

@ -36,8 +36,7 @@ use {
self, blake3_syscall_enabled, check_physical_overlapping, check_slice_translation_size,
curve25519_syscall_enabled, disable_cpi_setting_executable_and_rent_epoch,
disable_fees_sysvar, enable_early_verification_of_account_modifications,
libsecp256k1_0_5_upgrade_enabled, limit_secp256k1_recovery_id,
prevent_calling_precompiles_as_programs, syscall_saturated_math,
libsecp256k1_0_5_upgrade_enabled, limit_secp256k1_recovery_id, syscall_saturated_math,
},
hash::{Hasher, HASH_BYTES},
instruction::{

View File

@ -4,7 +4,6 @@ use {
solana_program_test::*,
solana_sdk::{
ed25519_instruction::new_ed25519_instruction,
feature_set,
signature::Signer,
transaction::{Transaction, TransactionError},
},
@ -60,27 +59,3 @@ async fn test_failure() {
))
);
}
#[tokio::test]
async fn test_success_call_builtin_program() {
let mut program_test = ProgramTest::default();
program_test.deactivate_feature(feature_set::prevent_calling_precompiles_as_programs::id());
let mut context = program_test.start_with_context().await;
let client = &mut context.banks_client;
let payer = &context.payer;
let recent_blockhash = context.last_blockhash;
let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng());
let message_arr = b"hello";
let instruction = new_ed25519_instruction(&privkey, message_arr);
let transaction = Transaction::new_signed_with_payer(
&[instruction],
Some(&payer.pubkey()),
&[payer],
recent_blockhash,
);
assert_matches!(client.process_transaction(transaction).await, Ok(()));
}

View File

@ -14571,25 +14571,25 @@ pub(crate) mod tests {
if bank.slot == 0 {
assert_eq!(
bank.hash().to_string(),
"9tLrxkBoNE7zEUZ2g72ZwE4fTfhUQnhC8A4Xt4EmYhP1"
"5gY6TCgB9NymbbxgFgAjvYLpXjyXiVyyruS1aEwbWKLK"
);
}
if bank.slot == 32 {
assert_eq!(
bank.hash().to_string(),
"AxphC8xDj9gmFosor5gyiovNvPVMydJCFRUTxn2wFiQf"
"6uJ5C4QDXWCN39EjJ5Frcz73nnS2jMJ55KgkQff12Fqp"
);
}
if bank.slot == 64 {
assert_eq!(
bank.hash().to_string(),
"4vZCSbBuL8xjE43rCy9Cm3dCh1BMj45heMiMb6n6qgzA"
"4u8bxZRLYdQBkWRBwmpcwcQVMCJoEpzY7hCuAzxr3kCe"
);
}
if bank.slot == 128 {
assert_eq!(
bank.hash().to_string(),
"46LUpeBdJuisnfwgYisvh4x7jnxzBaLfHF614GtcTs59"
"4c5F8UbcDD8FM7qXcfv6BPPo6nHNYJQmN5gHiCMTdEzX"
);
break;
}
@ -14817,7 +14817,7 @@ pub(crate) mod tests {
// No more slots should be shrunk
assert_eq!(bank2.shrink_candidate_slots(), 0);
// alive_counts represents the count of alive accounts in the three slots 0,1,2
assert_eq!(alive_counts, vec![9, 1, 7]);
assert_eq!(alive_counts, vec![11, 1, 7]);
}
#[test]
@ -14863,7 +14863,7 @@ pub(crate) mod tests {
.map(|_| bank.process_stale_slot_with_budget(0, force_to_return_alive_account))
.sum();
// consumed_budgets represents the count of alive accounts in the three slots 0,1,2
assert_eq!(consumed_budgets, 10);
assert_eq!(consumed_budgets, 12);
}
#[test]

View File

@ -2,10 +2,8 @@
use solana_frozen_abi::abi_example::AbiExample;
use {
crate::system_instruction_processor,
solana_program_runtime::invoke_context::{InvokeContext, ProcessInstructionWithContext},
solana_sdk::{
feature_set, instruction::InstructionError, pubkey::Pubkey, stake, system_program,
},
solana_program_runtime::invoke_context::ProcessInstructionWithContext,
solana_sdk::{feature_set, pubkey::Pubkey, stake, system_program},
std::fmt,
};
@ -141,14 +139,6 @@ fn genesis_builtins() -> Vec<Builtin> {
]
}
/// place holder for precompile programs, remove when the precompile program is deactivated via feature activation
fn dummy_process_instruction(
_first_instruction_account: usize,
_invoke_context: &mut InvokeContext,
) -> Result<(), InstructionError> {
Ok(())
}
/// Dynamic feature transitions for builtin programs
fn builtin_feature_transitions() -> Vec<BuiltinFeatureTransition> {
vec![
@ -160,24 +150,6 @@ fn builtin_feature_transitions() -> Vec<BuiltinFeatureTransition> {
),
feature_id: feature_set::add_compute_budget_program::id(),
},
BuiltinFeatureTransition::RemoveOrRetain {
previously_added_builtin: Builtin::new(
"secp256k1_program",
solana_sdk::secp256k1_program::id(),
dummy_process_instruction,
),
addition_feature_id: feature_set::secp256k1_program_enabled::id(),
removal_feature_id: feature_set::prevent_calling_precompiles_as_programs::id(),
},
BuiltinFeatureTransition::RemoveOrRetain {
previously_added_builtin: Builtin::new(
"ed25519_program",
solana_sdk::ed25519_program::id(),
dummy_process_instruction,
),
addition_feature_id: feature_set::ed25519_program_enabled::id(),
removal_feature_id: feature_set::prevent_calling_precompiles_as_programs::id(),
},
BuiltinFeatureTransition::Add {
builtin: Builtin::new(
"address_lookup_table_program",

View File

@ -27,6 +27,7 @@ pub fn bootstrap_validator_stake_lamports() -> u64 {
// Number of lamports automatically used for genesis accounts
pub const fn genesis_sysvar_and_builtin_program_lamports() -> u64 {
const NUM_BUILTIN_PROGRAMS: u64 = 4;
const NUM_PRECOMPILES: u64 = 2;
const FEES_SYSVAR_MIN_BALANCE: u64 = 946_560;
const STAKE_HISTORY_MIN_BALANCE: u64 = 114_979_200;
const CLOCK_SYSVAR_MIN_BALANCE: u64 = 1_169_280;
@ -41,6 +42,7 @@ pub const fn genesis_sysvar_and_builtin_program_lamports() -> u64 {
+ EPOCH_SCHEDULE_SYSVAR_MIN_BALANCE
+ RECENT_BLOCKHASHES_SYSVAR_MIN_BALANCE
+ NUM_BUILTIN_PROGRAMS
+ NUM_PRECOMPILES
}
pub struct ValidatorVoteKeypairs {

View File

@ -10,7 +10,7 @@ use {
},
solana_sdk::{
account::WritableAccount,
feature_set::{prevent_calling_precompiles_as_programs, FeatureSet},
feature_set::FeatureSet,
hash::Hash,
message::SanitizedMessage,
precompiles::is_precompile,
@ -86,10 +86,8 @@ impl MessageProcessor {
.zip(program_indices.iter())
.enumerate()
{
let is_precompile = invoke_context
.feature_set
.is_active(&prevent_calling_precompiles_as_programs::id())
&& is_precompile(program_id, |id| invoke_context.feature_set.is_active(id));
let is_precompile =
is_precompile(program_id, |id| invoke_context.feature_set.is_active(id));
// Fixup the special instructions key if present
// before the account pre-values are taken care of

View File

@ -201,8 +201,6 @@ pub mod do_support_realloc {
solana_sdk::declare_id!("75m6ysz33AfLA5DDEzWM1obBrnPQRSsdVQ2nRmc8Vuu1");
}
// Note: when this feature is cleaned up, also remove the secp256k1 program from
// the list of builtins and remove its files from /programs
pub mod prevent_calling_precompiles_as_programs {
solana_sdk::declare_id!("4ApgRX3ud6p7LNMJmsuaAcZY5HWctGPr5obAsjB3A54d");
}

View File

@ -4,9 +4,7 @@
use {
crate::{
decode_error::DecodeError,
feature_set::{prevent_calling_precompiles_as_programs, FeatureSet},
instruction::CompiledInstruction,
decode_error::DecodeError, feature_set::FeatureSet, instruction::CompiledInstruction,
pubkey::Pubkey,
},
lazy_static::lazy_static,
@ -81,12 +79,12 @@ lazy_static! {
static ref PRECOMPILES: Vec<Precompile> = vec![
Precompile::new(
crate::secp256k1_program::id(),
Some(prevent_calling_precompiles_as_programs::id()),
None, // always enabled
crate::secp256k1_instruction::verify,
),
Precompile::new(
crate::ed25519_program::id(),
Some(prevent_calling_precompiles_as_programs::id()),
None, // always enabled
crate::ed25519_instruction::verify,
),
];