diff --git a/Cargo.lock b/Cargo.lock index 2673d69a60babc..3a9553d12024e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4617,16 +4617,6 @@ dependencies = [ "solana-sdk", ] -[[package]] -name = "solana-ed25519-program" -version = "1.8.0" -dependencies = [ - "ed25519-dalek", - "rand 0.7.3", - "solana-logger 1.8.0", - "solana-sdk", -] - [[package]] name = "solana-entry" version = "1.8.0" @@ -5417,6 +5407,7 @@ dependencies = [ "crossbeam-channel", "dashmap", "dir-diff", + "ed25519-dalek", "flate2", "fnv", "itertools 0.10.1", @@ -5434,7 +5425,6 @@ dependencies = [ "solana-bucket-map", "solana-compute-budget-program", "solana-config-program", - "solana-ed25519-program", "solana-frozen-abi 1.8.0", "solana-frozen-abi-macro 1.8.0", "solana-logger 1.8.0", @@ -5443,7 +5433,6 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-sdk", - "solana-secp256k1-program", "solana-stake-program", "solana-vote-program", "symlink", @@ -5535,17 +5524,6 @@ dependencies = [ "syn 1.0.67", ] -[[package]] -name = "solana-secp256k1-program" -version = "1.8.0" -dependencies = [ - "bincode", - "libsecp256k1 0.6.0", - "rand 0.7.3", - "solana-logger 1.8.0", - "solana-sdk", -] - [[package]] name = "solana-send-transaction-service" version = "1.8.0" diff --git a/Cargo.toml b/Cargo.toml index 7f49ba685726d9..434e97e89a7a57 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,11 +46,9 @@ members = [ "programs/bpf_loader", "programs/compute-budget", "programs/config", - "programs/ed25519", "programs/failure", "programs/noop", "programs/ownable", - "programs/secp256k1", "programs/stake", "programs/vote", "rbpf-cli", diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs index 0e511b82ab4241..afb92856467352 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/instruction_processor.rs @@ -335,6 +335,13 @@ impl InstructionProcessor { } } + /// Remove a program + pub fn remove_program(&mut self, program_id: Pubkey) { + if let Some(position) = self.programs.iter().position(|(key, _)| program_id == *key) { + self.programs.remove(position); + } + } + /// Process an instruction /// This method calls the instruction's program entrypoint method pub fn process_instruction( diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index abec636bc84f27..1ccb7500a2292c 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -2934,13 +2934,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "solana-ed25519-program" -version = "1.8.0" -dependencies = [ - "solana-sdk", -] - [[package]] name = "solana-faucet" version = "1.8.0" @@ -3247,7 +3240,6 @@ dependencies = [ "solana-bucket-map", "solana-compute-budget-program", "solana-config-program", - "solana-ed25519-program", "solana-frozen-abi 1.8.0", "solana-frozen-abi-macro 1.8.0", "solana-logger 1.8.0", @@ -3256,7 +3248,6 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-sdk", - "solana-secp256k1-program", "solana-stake-program", "solana-vote-program", "symlink", @@ -3338,13 +3329,6 @@ dependencies = [ "syn 1.0.67", ] -[[package]] -name = "solana-secp256k1-program" -version = "1.8.0" -dependencies = [ - "solana-sdk", -] - [[package]] name = "solana-send-transaction-service" version = "1.8.0" diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index c7807b4f280fa3..58ed806c05cd6b 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -26,8 +26,9 @@ static const uint8_t TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE = 13; static const uint8_t TEST_WRITABLE_DEESCALATION_WRITABLE = 14; static const uint8_t TEST_NESTED_INVOKE_TOO_DEEP = 15; static const uint8_t TEST_EXECUTABLE_LAMPORTS = 16; -static const uint8_t ADD_LAMPORTS = 17; -static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 18; +static const uint8_t TEST_CALL_PRECOMPILE = 17; +static const uint8_t ADD_LAMPORTS = 18; +static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 19; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -40,6 +41,8 @@ static const int DERIVED_KEY2_INDEX = 7; static const int DERIVED_KEY3_INDEX = 8; static const int SYSTEM_PROGRAM_INDEX = 9; static const int FROM_INDEX = 10; +static const int ED25519_PROGRAM_INDEX = 11; +static const int INVOKE_PROGRAM_INDEX = 12; uint64_t do_nested_invokes(uint64_t num_nested_invokes, SolAccountInfo *accounts, uint64_t num_accounts) { @@ -73,7 +76,7 @@ uint64_t do_nested_invokes(uint64_t num_nested_invokes, extern uint64_t entrypoint(const uint8_t *input) { sol_log("Invoke C program"); - SolAccountInfo accounts[12]; + SolAccountInfo accounts[13]; SolParameters params = (SolParameters){.ka = accounts}; if (!sol_deserialize(input, ¶ms, SOL_ARRAY_SIZE(accounts))) { @@ -588,6 +591,16 @@ extern uint64_t entrypoint(const uint8_t *input) { *accounts[ARGUMENT_INDEX].lamports += 1; break; } + case TEST_CALL_PRECOMPILE: { + sol_log("Test calling builtin from cpi"); + SolAccountMeta arguments[] = {}; + uint8_t data[] = {}; + const SolInstruction instruction = {accounts[ED25519_PROGRAM_INDEX].key, + arguments, SOL_ARRAY_SIZE(arguments), + data, SOL_ARRAY_SIZE(data)}; + sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)); + break; + } case ADD_LAMPORTS: { *accounts[0].lamports += 1; break; diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs index 31171c7b0316f3..16d2948320e433 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -9,6 +9,7 @@ use solana_program::{ account_info::AccountInfo, entrypoint, entrypoint::{ProgramResult, MAX_PERMITTED_DATA_INCREASE}, + instruction::Instruction, msg, program::{get_return_data, invoke, invoke_signed, set_return_data}, program_error::ProgramError, @@ -32,7 +33,8 @@ const TEST_PRIVILEGE_DEESCALATION_ESCALATION_WRITABLE: u8 = 13; const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14; const TEST_NESTED_INVOKE_TOO_DEEP: u8 = 15; const TEST_EXECUTABLE_LAMPORTS: u8 = 16; -const ADD_LAMPORTS: u8 = 17; +const TEST_CALL_PRECOMPILE: u8 = 17; +const ADD_LAMPORTS: u8 = 18; // const MINT_INDEX: usize = 0; // unused placeholder const ARGUMENT_INDEX: usize = 1; @@ -45,6 +47,8 @@ const DERIVED_KEY2_INDEX: usize = 7; const DERIVED_KEY3_INDEX: usize = 8; const SYSTEM_PROGRAM_INDEX: usize = 9; const FROM_INDEX: usize = 10; +const ED25519_PROGRAM_INDEX: usize = 11; +const INVOKE_PROGRAM_INDEX: usize = 12; fn do_nested_invokes(num_nested_invokes: u64, accounts: &[AccountInfo]) -> ProgramResult { assert!(accounts[ARGUMENT_INDEX].is_signer); @@ -663,6 +667,12 @@ fn process_instruction( // reset executable account **(*accounts[ARGUMENT_INDEX].lamports).borrow_mut() += 1; } + TEST_CALL_PRECOMPILE => { + msg!("Test calling builtin from cpi"); + let instruction = + Instruction::new_with_bytes(*accounts[ED25519_PROGRAM_INDEX].key, &[], vec![]); + invoke(&instruction, accounts)?; + } ADD_LAMPORTS => { // make sure the total balance is fine **accounts[0].lamports.borrow_mut() += 1; diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 70b311da08af99..d5221fd0042bb4 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -760,7 +760,8 @@ fn test_program_bpf_invoke_sanity() { const TEST_WRITABLE_DEESCALATION_WRITABLE: u8 = 14; const TEST_NESTED_INVOKE_TOO_DEEP: u8 = 15; const TEST_EXECUTABLE_LAMPORTS: u8 = 16; - const TEST_RETURN_DATA_TOO_LARGE: u8 = 18; + const TEST_CALL_PRECOMPILE: u8 = 17; + const TEST_RETURN_DATA_TOO_LARGE: u8 = 19; #[allow(dead_code)] #[derive(Debug)] @@ -835,6 +836,7 @@ fn test_program_bpf_invoke_sanity() { AccountMeta::new_readonly(derived_key3, false), AccountMeta::new_readonly(system_program::id(), false), AccountMeta::new(from_keypair.pubkey(), true), + AccountMeta::new_readonly(solana_sdk::ed25519_program::id(), false), AccountMeta::new_readonly(invoke_program_id, false), ]; @@ -1037,6 +1039,12 @@ fn test_program_bpf_invoke_sanity() { &[invoke_program_id.clone()], ); + do_invoke_failure_test_local( + TEST_CALL_PRECOMPILE, + TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete), + &[], + ); + do_invoke_failure_test_local( TEST_RETURN_DATA_TOO_LARGE, TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete), @@ -1091,7 +1099,7 @@ fn test_program_bpf_invoke_sanity() { result.unwrap_err(), TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete) ); - } + } } #[cfg(feature = "bpf_rust")] diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index cf03ea9fcc2ee4..3193f82b13610f 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -21,8 +21,8 @@ use solana_sdk::{ feature_set::{ allow_native_ids, blake3_syscall_enabled, check_seed_length, close_upgradeable_program_accounts, demote_program_write_locks, disable_fees_sysvar, - libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, return_data_syscall_enabled, - secp256k1_recover_syscall_enabled, + libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, prevent_calling_precompiles_as_programs, + return_data_syscall_enabled, secp256k1_recover_syscall_enabled, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -30,6 +30,7 @@ use solana_sdk::{ keccak, message::Message, native_loader, + precompiles::is_precompile, process_instruction::{self, stable_log, ComputeMeter, InvokeContext, Logger}, program::MAX_RETURN_DATA, pubkey::{Pubkey, PubkeyError, MAX_SEEDS, MAX_SEED_LEN}, @@ -2125,16 +2126,21 @@ fn check_account_infos( fn check_authorized_program( program_id: &Pubkey, instruction_data: &[u8], - close_upgradeable_program_accounts: bool, + invoke_context: &dyn InvokeContext, ) -> Result<(), EbpfError> { + #[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) || (bpf_loader_upgradeable::check_id(program_id) && !(bpf_loader_upgradeable::is_upgrade_instruction(instruction_data) || bpf_loader_upgradeable::is_set_authority_instruction(instruction_data) - || (close_upgradeable_program_accounts + || (invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()) && bpf_loader_upgradeable::is_close_instruction(instruction_data)))) + || (invoke_context.is_feature_active(&prevent_calling_precompiles_as_programs::id()) + && is_precompile(program_id, |feature_id: &Pubkey| { + invoke_context.is_feature_active(feature_id) + })) { return Err(SyscallError::ProgramNotSupported(*program_id).into()); } @@ -2171,11 +2177,7 @@ fn call<'a>( let (message, caller_write_privileges, program_indices) = InstructionProcessor::create_message(&instruction, &signers, &invoke_context) .map_err(SyscallError::InstructionError)?; - check_authorized_program( - &instruction.program_id, - &instruction.data, - invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()), - )?; + check_authorized_program(&instruction.program_id, &instruction.data, *invoke_context)?; let (accounts, account_refs) = syscall.translate_accounts( &message, account_infos_addr, diff --git a/programs/ed25519/Cargo.toml b/programs/ed25519/Cargo.toml deleted file mode 100644 index 18f484de57e299..00000000000000 --- a/programs/ed25519/Cargo.toml +++ /dev/null @@ -1,25 +0,0 @@ -[package] -name = "solana-ed25519-program" -description = "Solana Ed25519 program" -version = "1.8.0" -homepage = "https://solana.com/" -documentation = "https://docs.rs/solana-ed25519-program" -repository = "https://github.com/solana-labs/solana" -authors = ["Solana Maintainers "] -license = "Apache-2.0" -edition = "2018" - -[dependencies] -solana-sdk = { path = "../../sdk", version = "=1.8.0" } - -[dev-dependencies] -ed25519-dalek = "=1.0.1" -rand = "0.7.0" -solana-logger = { path = "../../logger", version = "=1.8.0" } - -[lib] -crate-type = ["lib"] -name = "solana_ed25519_program" - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] diff --git a/programs/ed25519/src/lib.rs b/programs/ed25519/src/lib.rs deleted file mode 100644 index da579fb1d19f8a..00000000000000 --- a/programs/ed25519/src/lib.rs +++ /dev/null @@ -1,55 +0,0 @@ -use solana_sdk::{ - instruction::InstructionError, process_instruction::InvokeContext, pubkey::Pubkey, -}; - -pub fn process_instruction( - _program_id: &Pubkey, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, -) -> Result<(), InstructionError> { - // Should be already checked by now. - Ok(()) -} - -#[cfg(test)] -pub mod test { - use rand::{thread_rng, Rng}; - use solana_sdk::{ - ed25519_instruction::new_ed25519_instruction, - feature_set::FeatureSet, - hash::Hash, - signature::{Keypair, Signer}, - transaction::Transaction, - }; - use std::sync::Arc; - - #[test] - fn test_ed25519() { - solana_logger::setup(); - - let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng()); - let message_arr = b"hello"; - let mut instruction = new_ed25519_instruction(&privkey, message_arr); - let mint_keypair = Keypair::new(); - let feature_set = Arc::new(FeatureSet::all_enabled()); - - let tx = Transaction::new_signed_with_payer( - &[instruction.clone()], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - - assert!(tx.verify_precompiles(&feature_set).is_ok()); - - let index = thread_rng().gen_range(0, instruction.data.len()); - instruction.data[index] = instruction.data[index].wrapping_add(12); - let tx = Transaction::new_signed_with_payer( - &[instruction], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - assert!(tx.verify_precompiles(&feature_set).is_err()); - } -} diff --git a/programs/secp256k1/Cargo.toml b/programs/secp256k1/Cargo.toml deleted file mode 100644 index c8b91cf6b2a40c..00000000000000 --- a/programs/secp256k1/Cargo.toml +++ /dev/null @@ -1,26 +0,0 @@ -[package] -name = "solana-secp256k1-program" -description = "Solana Secp256k1 program" -version = "1.8.0" -homepage = "https://solana.com/" -documentation = "https://docs.rs/solana-secp256k1-program" -repository = "https://github.com/solana-labs/solana" -authors = ["Solana Maintainers "] -license = "Apache-2.0" -edition = "2018" - -[dependencies] -solana-sdk = { path = "../../sdk", version = "=1.8.0" } - -[dev-dependencies] -bincode = "1.3.3" -libsecp256k1 = "0.6.0" -rand = "0.7.0" -solana-logger = { path = "../../logger", version = "=1.8.0" } - -[lib] -crate-type = ["lib"] -name = "solana_secp256k1_program" - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] diff --git a/programs/secp256k1/src/lib.rs b/programs/secp256k1/src/lib.rs deleted file mode 100644 index de879e0924e5b7..00000000000000 --- a/programs/secp256k1/src/lib.rs +++ /dev/null @@ -1,69 +0,0 @@ -use solana_sdk::{ - instruction::InstructionError, process_instruction::InvokeContext, pubkey::Pubkey, -}; - -pub fn process_instruction( - _program_id: &Pubkey, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, -) -> Result<(), InstructionError> { - // Should be already checked by now. - Ok(()) -} - -#[cfg(test)] -pub mod test { - use rand::{thread_rng, Rng}; - use solana_sdk::{ - feature_set, - hash::Hash, - secp256k1_instruction::{ - new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE, - }, - signature::{Keypair, Signer}, - transaction::Transaction, - }; - use std::sync::Arc; - - #[test] - fn test_secp256k1() { - solana_logger::setup(); - let offsets = SecpSignatureOffsets::default(); - assert_eq!( - bincode::serialized_size(&offsets).unwrap() as usize, - SIGNATURE_OFFSETS_SERIALIZED_SIZE - ); - - let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng()); - let message_arr = b"hello"; - let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); - let mint_keypair = Keypair::new(); - let mut feature_set = feature_set::FeatureSet::all_enabled(); - feature_set - .active - .remove(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()); - feature_set - .inactive - .insert(feature_set::libsecp256k1_0_5_upgrade_enabled::id()); - let feature_set = Arc::new(feature_set); - - let tx = Transaction::new_signed_with_payer( - &[secp_instruction.clone()], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - - assert!(tx.verify_precompiles(&feature_set).is_ok()); - - let index = thread_rng().gen_range(0, secp_instruction.data.len()); - secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); - let tx = Transaction::new_signed_with_payer( - &[secp_instruction], - Some(&mint_keypair.pubkey()), - &[&mint_keypair], - Hash::default(), - ); - assert!(tx.verify_precompiles(&feature_set).is_err()); - } -} diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 060cd3cb92d5ea..5617c80de90112 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -34,7 +34,6 @@ serde = { version = "1.0.130", features = ["rc"] } serde_derive = "1.0.103" solana-config-program = { path = "../programs/config", version = "=1.8.0" } solana-compute-budget-program = { path = "../programs/compute-budget", version = "=1.8.0" } -solana-ed25519-program = { path = "../programs/ed25519", version = "=1.8.0" } solana-frozen-abi = { path = "../frozen-abi", version = "=1.8.0" } solana-frozen-abi-macro = { path = "../frozen-abi/macro", version = "=1.8.0" } solana-logger = { path = "../logger", version = "=1.8.0" } @@ -44,7 +43,6 @@ solana-bucket-map = { path = "../bucket_map", version = "=1.8.0" } solana-program-runtime = { path = "../program-runtime", version = "=1.8.0" } solana-rayon-threadlimit = { path = "../rayon-threadlimit", version = "=1.8.0" } solana-sdk = { path = "../sdk", version = "=1.8.0" } -solana-secp256k1-program = { path = "../programs/secp256k1", version = "=1.8.0" } solana-stake-program = { path = "../programs/stake", version = "=1.8.0" } solana-vote-program = { path = "../programs/vote", version = "=1.8.0" } symlink = "0.1.0" @@ -59,6 +57,7 @@ name = "solana_runtime" [dev-dependencies] assert_matches = "1.5.0" +ed25519-dalek = "=1.0.1" [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7607e1ffbb7332..0b90b0d5deeaf3 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -97,6 +97,7 @@ use solana_sdk::{ native_token::sol_to_lamports, nonce, nonce_account, packet::PACKET_DATA_SIZE, + precompiles::get_precompiles, process_instruction::{ComputeMeter, Executor, ProcessInstructionWithContext}, program_utils::limited_deserialize, pubkey::Pubkey, @@ -2583,6 +2584,62 @@ impl Bank { debug!("Added native program {} under {:?}", name, program_id); } + /// Add a precompiled program + pub fn add_precompiled_program(&mut self, program_id: &Pubkey) { + debug!("Adding precompiled program {}", program_id); + let existing_genuine_program = + if let Some(mut account) = self.get_account_with_fixed_root(program_id) { + // The account just needs to be executable + if account.executable() { + Some(account) + } else { + // malicious account is pre-occupying at program_id + // forcibly burn and purge it + + self.capitalization.fetch_sub(account.lamports(), Relaxed); + + // Resetting account balance to 0 is needed to really purge from AccountsDb and + // flush the Stakes cache + account.set_lamports(0); + self.store_account(program_id, &account); + None + } + } else { + None + }; + + // introducing precompiled program + + match &existing_genuine_program { + None => (), // continue to add account + Some(_account) => { + // nop; it seems that we already have account + return; + } + } + + assert!( + !self.freeze_started(), + "Can't change frozen bank by adding not-existing new precompiled program ({}). \ + Maybe, inconsistent program activation is detected on snapshot restore?", + program_id + ); + + // Add a bogus executable account, which will be loaded and ignored. + let (lamports, rent_epoch) = + self.inherit_specially_retained_account_fields(&existing_genuine_program); + let account = AccountSharedData::from(Account { + lamports, + owner: solana_sdk::system_program::id(), + data: vec![], + executable: true, + rent_epoch, + }); + self.store_account_and_update_capitalization(program_id, &account); + + debug!("Added precompiled program {:?}", program_id); + } + pub fn set_rent_burn_percentage(&mut self, burn_percent: u8) { self.rent_collector.rent.burn_percent = burn_percent; } @@ -4590,6 +4647,11 @@ impl Bank { builtin.process_instruction_with_context, ); } + for precompile in get_precompiles() { + if precompile.feature.is_none() { + self.add_precompiled_program(&precompile.program_id); + } + } } self.feature_builtins = Arc::new(builtins.feature_builtins); @@ -5322,6 +5384,12 @@ impl Bank { .add_program(program_id, process_instruction_with_context); } + /// Remove a builtin instruction processor if it already exists + pub fn remove_builtin(&mut self, name: &str, program_id: Pubkey) { + debug!("Removing program {} under {:?}", name, program_id); + self.message_processor.remove_program(program_id); + } + pub fn clean_accounts( &self, skip_last: bool, @@ -5593,9 +5661,18 @@ impl Bank { builtin.id, builtin.process_instruction_with_context, ), + ActivationType::RemoveProgram => self.remove_builtin(&builtin.name, builtin.id), } } } + for precompile in get_precompiles() { + #[allow(clippy::blocks_in_if_conditions)] + if precompile.feature.map_or(false, |ref feature_id| { + self.feature_set.is_active(feature_id) + }) { + self.add_precompiled_program(&precompile.program_id); + } + } } fn apply_spl_token_v2_set_authority_fix(&mut self) { @@ -6025,7 +6102,7 @@ pub(crate) mod tests { cluster_type: ClusterType::MainnetBeta, ..GenesisConfig::default() })); - let sysvar_and_native_program_delta0 = 11; + let sysvar_and_native_program_delta0 = 10; assert_eq!( bank0.capitalization(), 42 * 42 + sysvar_and_native_program_delta0 @@ -7730,7 +7807,7 @@ pub(crate) mod tests { // not being eagerly-collected for exact rewards calculation bank0.restore_old_behavior_for_fragile_tests(); - let sysvar_and_native_program_delta0 = 11; + let sysvar_and_native_program_delta0 = 10; assert_eq!( bank0.capitalization(), 42 * 1_000_000_000 + sysvar_and_native_program_delta0 @@ -7853,7 +7930,7 @@ pub(crate) mod tests { // not being eagerly-collected for exact rewards calculation bank.restore_old_behavior_for_fragile_tests(); - let sysvar_and_native_program_delta = 11; + let sysvar_and_native_program_delta = 10; assert_eq!( bank.capitalization(), 42 * 1_000_000_000 + sysvar_and_native_program_delta @@ -11203,25 +11280,25 @@ pub(crate) mod tests { if bank.slot == 0 { assert_eq!( bank.hash().to_string(), - "DqaWg7EVKzb5Fpe92zNBtXAWqLwcedgHDicYrCBnf3QK" + "7jRygKXT3WYoJWigqwHvnEHY2MGtCpV9DicY51kGjzKW" ); } if bank.slot == 32 { assert_eq!( bank.hash().to_string(), - "AYdhzhKrM74r9XuZBDGcHeFzg2DEtp1boggnEnzDjZSq" + "B8qZwsyx9h9NeUcVTG7uWFLCgxVQBca5fVmziWjNwfec" ); } if bank.slot == 64 { assert_eq!( bank.hash().to_string(), - "EsbPVYzo1qz5reEUH5okKW4ExB6WbcidkVdW5mzpFn7C" + "A893DSezmoRuEAWPQdHNEzShx4AtaY2VFFRKEeh9sx42" ); } if bank.slot == 128 { assert_eq!( bank.hash().to_string(), - "H3DWrQ6FqbLkFNDxbWQ62UKRbw2dbuxf3oVF2VpBk6Ga" + "9qHRKxR56VhJ49rHiCpXcw6AyvYh7r2JT5jDxJyeqDnT" ); break; } @@ -11449,7 +11526,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![10, 1, 7]); + assert_eq!(alive_counts, vec![9, 1, 7]); } #[test] @@ -11497,7 +11574,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, 11); + assert_eq!(consumed_budgets, 10); } #[test] @@ -14372,4 +14449,29 @@ pub(crate) mod tests { ); } } + + #[test] + fn test_call_builtin_program() { + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config_with_leader(42, &Pubkey::new_unique(), 42); + let bank = Bank::new_for_tests(&genesis_config); + + let privkey = ed25519_dalek::Keypair::generate(&mut rand::thread_rng()); + let message_arr = b"hello"; + let instruction = + solana_sdk::ed25519_instruction::new_ed25519_instruction(&privkey, message_arr); + let tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + + // calling the program should be successful when called from the bank + // even if the program itself is not called + bank.process_transaction(&tx).unwrap(); + } } diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index e0e59a2a6965f6..eeb2093927bd2a 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -46,6 +46,7 @@ macro_rules! with_program_logging { pub enum ActivationType { NewProgram, NewVersion, + RemoveProgram, } #[derive(Clone)] @@ -91,7 +92,7 @@ pub struct Builtins { /// Builtin programs that are always available pub genesis_builtins: Vec, - /// Builtin programs activated dynamically by feature + /// Builtin programs activated or deactivated dynamically by feature pub feature_builtins: Vec<(Builtin, Pubkey, ActivationType)>, } @@ -121,11 +122,20 @@ fn genesis_builtins() -> Vec { Builtin::new( "secp256k1_program", solana_sdk::secp256k1_program::id(), - solana_secp256k1_program::process_instruction, + dummy_process_instruction, ), ] } +/// place holder for secp256k1, remove when the precompile program is deactivated via feature activation +fn dummy_process_instruction( + _program_id: &Pubkey, + _data: &[u8], + _invoke_context: &mut dyn InvokeContext, +) -> Result<(), InstructionError> { + Ok(()) +} + /// Builtin programs activated dynamically by feature /// /// Note: If the feature_builtin is intended to replace another builtin program, it must have a new @@ -145,14 +155,17 @@ fn feature_builtins() -> Vec<(Builtin, Pubkey, ActivationType)> { feature_set::tx_wide_compute_cap::id(), ActivationType::NewProgram, ), + // TODO when feature `prevent_calling_precompiles_as_programs` is + // cleaned up also remove "secp256k1_program" from the main builtins + // list ( Builtin::new( - "ed25519_program", - solana_sdk::ed25519_program::id(), - solana_ed25519_program::process_instruction, + "secp256k1_program", + solana_sdk::secp256k1_program::id(), + dummy_process_instruction, ), - feature_set::ed25519_program_enabled::id(), - ActivationType::NewProgram, + feature_set::prevent_calling_precompiles_as_programs::id(), + ActivationType::RemoveProgram, ), ] } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index a2c945975938bb..4262d13e02cb79 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -10,7 +10,8 @@ use solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, compute_budget::ComputeBudget, feature_set::{ - demote_program_write_locks, neon_evm_compute_budget, tx_wide_compute_cap, FeatureSet, + demote_program_write_locks, neon_evm_compute_budget, + prevent_calling_precompiles_as_programs, tx_wide_compute_cap, FeatureSet, }, fee_calculator::FeeCalculator, hash::Hash, @@ -18,6 +19,7 @@ use solana_sdk::{ instruction::{CompiledInstruction, Instruction, InstructionError}, keyed_account::{create_keyed_accounts_unified, KeyedAccount}, message::Message, + precompiles::is_precompile, process_instruction::{ ComputeMeter, Executor, InvokeContext, InvokeContextStackFrame, Logger, ProcessInstructionWithContext, @@ -378,6 +380,11 @@ impl MessageProcessor { .add_program(program_id, process_instruction); } + /// Remove a program. + pub fn remove_program(&mut self, program_id: Pubkey) { + self.instruction_processor.remove_program(program_id); + } + /// Record the initial state of the accounts so that they can be compared /// after the instruction is processed pub fn create_pre_accounts( @@ -500,6 +507,14 @@ impl MessageProcessor { blockhash: &Hash, fee_calculator: &FeeCalculator, ) -> Result<(), InstructionError> { + let program_id = instruction.program_id(&message.account_keys); + if feature_set.is_active(&prevent_calling_precompiles_as_programs::id()) + && is_precompile(program_id, |feature_id| feature_set.is_active(feature_id)) + { + // Precompiled programs don't have an instruction processor + return Ok(()); + } + // Fixup the special instructions key if present // before the account pre-values are taken care of for (pubkey, accont) in accounts.iter().take(message.account_keys.len()) { @@ -513,8 +528,6 @@ impl MessageProcessor { } } - let program_id = instruction.program_id(&message.account_keys); - let mut compute_budget = compute_budget; if feature_set.is_active(&neon_evm_compute_budget::id()) && *program_id == crate::neon_evm_program::id() diff --git a/runtime/src/non_circulating_supply.rs b/runtime/src/non_circulating_supply.rs index 817f6e8c17feca..8df40ffcce6d97 100644 --- a/runtime/src/non_circulating_supply.rs +++ b/runtime/src/non_circulating_supply.rs @@ -273,7 +273,7 @@ mod tests { ..GenesisConfig::default() }; let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); - let sysvar_and_native_program_delta = 11; + let sysvar_and_native_program_delta = 10; assert_eq!( bank.capitalization(), (num_genesis_accounts + num_non_circulating_accounts + num_stake_accounts) * balance diff --git a/sdk/src/ed25519_instruction.rs b/sdk/src/ed25519_instruction.rs index 704b210cbb2adf..052d4617b99fc2 100644 --- a/sdk/src/ed25519_instruction.rs +++ b/sdk/src/ed25519_instruction.rs @@ -170,6 +170,15 @@ fn get_data_slice<'a>( #[cfg(test)] pub mod test { use super::*; + use crate::{ + ed25519_instruction::new_ed25519_instruction, + feature_set::FeatureSet, + hash::Hash, + signature::{Keypair, Signer}, + transaction::Transaction, + }; + use rand::{thread_rng, Rng}; + use std::sync::Arc; fn test_case( num_signatures: u16, @@ -322,4 +331,34 @@ pub mod test { Err(PrecompileError::InvalidDataOffsets) ); } + + #[test] + fn test_ed25519() { + solana_logger::setup(); + + let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng()); + let message_arr = b"hello"; + let mut instruction = new_ed25519_instruction(&privkey, message_arr); + let mint_keypair = Keypair::new(); + let feature_set = Arc::new(FeatureSet::all_enabled()); + + let tx = Transaction::new_signed_with_payer( + &[instruction.clone()], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + + assert!(tx.verify_precompiles(&feature_set).is_ok()); + + let index = thread_rng().gen_range(0, instruction.data.len()); + instruction.data[index] = instruction.data[index].wrapping_add(12); + let tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + assert!(tx.verify_precompiles(&feature_set).is_err()); + } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 6efe4726db95ae..8c4a5d528f655f 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -211,6 +211,12 @@ pub mod reduce_required_deploy_balance { solana_sdk::declare_id!("EBeznQDjcPG8491sFsKZYBi5S5jTVXMpAKNDJMQPS2kq"); } +// 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"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -259,6 +265,7 @@ lazy_static! { (return_data_syscall_enabled::id(), "enable sol_{set,get}_return_data syscall"), (fix_write_privs::id(), "fix native invoke write privileges"), (reduce_required_deploy_balance::id(), "reduce required payer balance for program deploys"), + (prevent_calling_precompiles_as_programs::id(), "Prevent calling precompiles as programs"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/precompiles.rs b/sdk/src/precompiles.rs index 3372fca3c4737a..478618478e8dbc 100644 --- a/sdk/src/precompiles.rs +++ b/sdk/src/precompiles.rs @@ -38,7 +38,7 @@ impl DecodeError for PrecompileError { pub type Verify = fn(&[u8], &[&[u8]], &Arc) -> std::result::Result<(), PrecompileError>; /// Information on a precompiled program -struct Precompile { +pub struct Precompile { /// Program id pub program_id: Pubkey, /// Feature to enable on, `None` indicates always enabled @@ -56,8 +56,13 @@ impl Precompile { } } /// Check if a program id is this precompiled program - pub fn check_id(&self, program_id: &Pubkey, feature_set: &Arc) -> bool { - self.feature.map_or(true, |f| feature_set.is_active(&f)) && self.program_id == *program_id + pub fn check_id(&self, program_id: &Pubkey, is_enabled: F) -> bool + where + F: Fn(&Pubkey) -> bool, + { + self.feature + .map_or(true, |ref feature_id| is_enabled(feature_id)) + && self.program_id == *program_id } /// Verify this precompiled program pub fn verify( @@ -87,10 +92,17 @@ lazy_static! { } /// Check if a program is a precompiled program -pub fn is_precompile(program_id: &Pubkey, feature_set: &Arc) -> bool { +pub fn is_precompile(program_id: &Pubkey, is_enabled: F) -> bool +where + F: Fn(&Pubkey) -> bool, +{ PRECOMPILES .iter() - .any(|precompile| precompile.check_id(program_id, feature_set)) + .any(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id))) +} + +pub fn get_precompiles<'a>() -> &'a [Precompile] { + &PRECOMPILES } /// Check that a program is precompiled and if so verify it @@ -101,7 +113,7 @@ pub fn verify_if_precompile( feature_set: &Arc, ) -> Result<(), PrecompileError> { for precompile in PRECOMPILES.iter() { - if precompile.check_id(program_id, feature_set) { + if precompile.check_id(program_id, |feature_id| feature_set.is_active(feature_id)) { let instruction_datas: Vec<_> = all_instructions .iter() .map(|instruction| instruction.data.as_ref()) diff --git a/sdk/src/secp256k1_instruction.rs b/sdk/src/secp256k1_instruction.rs index 299237b9cbb5d6..1cf471db9b681a 100644 --- a/sdk/src/secp256k1_instruction.rs +++ b/sdk/src/secp256k1_instruction.rs @@ -212,6 +212,17 @@ fn get_data_slice<'a>( #[cfg(test)] pub mod test { use super::*; + use crate::{ + feature_set, + hash::Hash, + secp256k1_instruction::{ + new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE, + }, + signature::{Keypair, Signer}, + transaction::Transaction, + }; + use rand::{thread_rng, Rng}; + use std::sync::Arc; fn test_case( num_signatures: u8, @@ -390,4 +401,46 @@ pub mod test { Err(PrecompileError::InvalidInstructionDataSize) ); } + + #[test] + fn test_secp256k1() { + solana_logger::setup(); + let offsets = SecpSignatureOffsets::default(); + assert_eq!( + bincode::serialized_size(&offsets).unwrap() as usize, + SIGNATURE_OFFSETS_SERIALIZED_SIZE + ); + + let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng()); + let message_arr = b"hello"; + let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); + let mint_keypair = Keypair::new(); + let mut feature_set = feature_set::FeatureSet::all_enabled(); + feature_set + .active + .remove(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()); + feature_set + .inactive + .insert(feature_set::libsecp256k1_0_5_upgrade_enabled::id()); + let feature_set = Arc::new(feature_set); + + let tx = Transaction::new_signed_with_payer( + &[secp_instruction.clone()], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + + assert!(tx.verify_precompiles(&feature_set).is_ok()); + + let index = thread_rng().gen_range(0, secp_instruction.data.len()); + secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); + let tx = Transaction::new_signed_with_payer( + &[secp_instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + ); + assert!(tx.verify_precompiles(&feature_set).is_err()); + } }