From 99686355fa76e5a8553cc2691e4044a653b92da1 Mon Sep 17 00:00:00 2001 From: frits-double-eye <31902851+frits-double-eye@users.noreply.github.com> Date: Wed, 24 Nov 2021 14:39:20 +0200 Subject: [PATCH] Revert "Dont call precompiled programs (#19930)" This reverts commit b578765556934ac2733a4faaeae9b5e28528cc4a. --- Cargo.lock | 25 +- Cargo.toml | 2 + program-runtime/src/instruction_processor.rs | 19 +- program-test/src/lib.rs | 4 +- programs/bpf/Cargo.lock | 16 + programs/bpf/benches/bpf_loader.rs | 2 +- programs/bpf/c/src/invoke/invoke.c | 19 +- programs/bpf/rust/invoke/src/lib.rs | 12 +- programs/bpf/tests/programs.rs | 73 ++-- programs/bpf_loader/src/lib.rs | 2 +- programs/bpf_loader/src/syscalls.rs | 20 +- programs/ed25519/Cargo.toml | 25 ++ programs/ed25519/src/lib.rs | 55 +++ programs/failure/tests/failure.rs | 2 +- programs/ownable/src/ownable_processor.rs | 2 +- programs/secp256k1/Cargo.toml | 26 ++ programs/secp256k1/src/lib.rs | 69 ++++ runtime/Cargo.toml | 4 +- runtime/benches/bank.rs | 4 +- runtime/src/bank.rs | 368 +++++-------------- runtime/src/builtins.rs | 27 +- runtime/src/message_processor.rs | 83 +---- runtime/tests/noop.rs | 2 +- sdk/src/ed25519_instruction.rs | 39 -- sdk/src/feature_set.rs | 7 - sdk/src/precompiles.rs | 30 +- sdk/src/secp256k1_instruction.rs | 53 --- 27 files changed, 386 insertions(+), 604 deletions(-) create mode 100644 programs/ed25519/Cargo.toml create mode 100644 programs/ed25519/src/lib.rs create mode 100644 programs/secp256k1/Cargo.toml create mode 100644 programs/secp256k1/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 4cb943aac86295..f0d9ec5eefcfbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4563,6 +4563,16 @@ 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" @@ -5355,12 +5365,10 @@ dependencies = [ "crossbeam-channel", "dashmap", "dir-diff", - "ed25519-dalek", "flate2", "fnv", "itertools 0.10.1", "lazy_static", - "libsecp256k1 0.6.0", "log 0.4.14", "memmap2 0.5.0", "num_cpus", @@ -5374,6 +5382,7 @@ 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", @@ -5382,6 +5391,7 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-sdk", + "solana-secp256k1-program", "solana-stake-program", "solana-vote-program", "symlink", @@ -5473,6 +5483,17 @@ 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 434e97e89a7a57..7f49ba685726d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,9 +46,11 @@ 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 97a086fa18acef..edc8ec8493a553 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/instruction_processor.rs @@ -343,19 +343,12 @@ impl InstructionProcessor { /// Add a static entrypoint to intercept instructions before the dynamic loader. pub fn add_program( &mut self, - program_id: &Pubkey, + program_id: Pubkey, process_instruction: ProcessInstructionWithContext, ) { - match self.programs.iter_mut().find(|(key, _)| program_id == key) { + match self.programs.iter_mut().find(|(key, _)| program_id == *key) { Some((_, processor)) => *processor = process_instruction, - None => self.programs.push((*program_id, process_instruction)), - } - } - - /// 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); + None => self.programs.push((program_id, process_instruction)), } } @@ -615,7 +608,7 @@ impl InstructionProcessor { let mut instruction_processor = InstructionProcessor::default(); for (program_id, process_instruction) in invoke_context.get_programs().iter() { - instruction_processor.add_program(program_id, *process_instruction); + instruction_processor.add_program(*program_id, *process_instruction); } let mut result = instruction_processor.process_instruction( @@ -1097,8 +1090,8 @@ mod tests { Ok(()) } let program_id = solana_sdk::pubkey::new_rand(); - instruction_processor.add_program(&program_id, mock_process_instruction); - instruction_processor.add_program(&program_id, mock_ix_processor); + instruction_processor.add_program(program_id, mock_process_instruction); + instruction_processor.add_program(program_id, mock_ix_processor); assert!(!format!("{:?}", instruction_processor).is_empty()); } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 4219e63e3f847f..69b97f5ecf6010 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -774,7 +774,7 @@ impl ProgramTest { // Add loaders macro_rules! add_builtin { ($b:expr) => { - bank.add_builtin(&$b.0, &$b.1, $b.2) + bank.add_builtin(&$b.0, $b.1, $b.2) }; } add_builtin!(solana_bpf_loader_deprecated_program!()); @@ -795,7 +795,7 @@ impl ProgramTest { for builtin in self.builtins.iter() { bank.add_builtin( &builtin.name, - &builtin.id, + builtin.id, builtin.process_instruction_with_context, ); } diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 4952d3afad1e4f..48c04b77d9987f 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -2952,6 +2952,13 @@ dependencies = [ "winapi", ] +[[package]] +name = "solana-ed25519-program" +version = "1.8.0" +dependencies = [ + "solana-sdk", +] + [[package]] name = "solana-faucet" version = "1.8.0" @@ -3259,6 +3266,7 @@ 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", @@ -3267,6 +3275,7 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-sdk", + "solana-secp256k1-program", "solana-stake-program", "solana-vote-program", "symlink", @@ -3348,6 +3357,13 @@ 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/benches/bpf_loader.rs b/programs/bpf/benches/bpf_loader.rs index 5742013a3d67e4..c1f2054a43ea6b 100644 --- a/programs/bpf/benches/bpf_loader.rs +++ b/programs/bpf/benches/bpf_loader.rs @@ -172,7 +172,7 @@ fn bench_program_execute_noop(bencher: &mut Bencher) { } = create_genesis_config(50); let mut bank = Bank::new_for_benches(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c index 9f62ca3cef216e..c7807b4f280fa3 100644 --- a/programs/bpf/c/src/invoke/invoke.c +++ b/programs/bpf/c/src/invoke/invoke.c @@ -26,9 +26,8 @@ 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 TEST_CALL_PRECOMPILE = 17; -static const uint8_t ADD_LAMPORTS = 18; -static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 19; +static const uint8_t ADD_LAMPORTS = 17; +static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 18; static const int MINT_INDEX = 0; static const int ARGUMENT_INDEX = 1; @@ -41,8 +40,6 @@ 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) { @@ -76,7 +73,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[13]; + SolAccountInfo accounts[12]; SolParameters params = (SolParameters){.ka = accounts}; if (!sol_deserialize(input, ¶ms, SOL_ARRAY_SIZE(accounts))) { @@ -591,16 +588,6 @@ extern uint64_t entrypoint(const uint8_t *input) { *accounts[ARGUMENT_INDEX].lamports += 1; break; } - case TEST_CALL_PRECOMPILE: { - sol_log("Test calling precompile 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 55286a8efcebfd..31171c7b0316f3 100644 --- a/programs/bpf/rust/invoke/src/lib.rs +++ b/programs/bpf/rust/invoke/src/lib.rs @@ -9,7 +9,6 @@ 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, @@ -33,8 +32,7 @@ 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 TEST_CALL_PRECOMPILE: u8 = 17; -const ADD_LAMPORTS: u8 = 18; +const ADD_LAMPORTS: u8 = 17; // const MINT_INDEX: usize = 0; // unused placeholder const ARGUMENT_INDEX: usize = 1; @@ -47,8 +45,6 @@ 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; unused placeholder fn do_nested_invokes(num_nested_invokes: u64, accounts: &[AccountInfo]) -> ProgramResult { assert!(accounts[ARGUMENT_INDEX].is_signer); @@ -667,12 +663,6 @@ fn process_instruction( // reset executable account **(*accounts[ARGUMENT_INDEX].lamports).borrow_mut() += 1; } - TEST_CALL_PRECOMPILE => { - msg!("Test calling precompiled program 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 53b55bf07c5a1b..2be7c2114044cf 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -481,7 +481,7 @@ fn test_program_bpf_sanity() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); // Call user program @@ -526,7 +526,7 @@ fn test_program_bpf_loader_deprecated() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( @@ -566,7 +566,7 @@ fn test_program_bpf_duplicate_accounts() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); let program_id = load_bpf_program(&bank_client, &bpf_loader::id(), &mint_keypair, program); @@ -666,7 +666,7 @@ fn test_program_bpf_error_handling() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program(&bank_client, &bpf_loader::id(), &mint_keypair, program); let account_metas = vec![AccountMeta::new(mint_keypair.pubkey(), true)]; @@ -768,7 +768,7 @@ fn test_return_data_and_log_data_syscall() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -817,9 +817,7 @@ 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_CALL_PRECOMPILE: u8 = 17; - // const ADD_LAMPORTS: u8 = 18; - const TEST_RETURN_DATA_TOO_LARGE: u8 = 19; + const TEST_RETURN_DATA_TOO_LARGE: u8 = 18; #[allow(dead_code)] #[derive(Debug)] @@ -851,7 +849,7 @@ fn test_program_bpf_invoke_sanity() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -894,7 +892,6 @@ 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), ]; @@ -1097,12 +1094,6 @@ 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), @@ -1170,7 +1161,7 @@ fn test_program_bpf_program_id_spoofing() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1223,7 +1214,7 @@ fn test_program_bpf_caller_has_access_to_cpi_program() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1263,7 +1254,7 @@ fn test_program_bpf_ro_modify() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1320,7 +1311,7 @@ fn test_program_bpf_call_depth() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( &bank_client, @@ -1355,7 +1346,7 @@ fn test_program_bpf_compute_budget() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( &bank_client, @@ -1458,7 +1449,7 @@ fn test_program_bpf_instruction_introspection() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1515,7 +1506,7 @@ fn test_program_bpf_test_use_latest_executor() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let panic_id = load_bpf_program( &bank_client, @@ -1610,7 +1601,7 @@ fn test_program_bpf_test_use_latest_executor2() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let invoke_and_error = load_bpf_program( &bank_client, @@ -1740,7 +1731,7 @@ fn test_program_bpf_upgrade() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); // Deploy upgrade program @@ -1834,7 +1825,7 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -1914,9 +1905,9 @@ fn test_program_bpf_invoke_upgradeable_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let invoke_and_return = load_bpf_program( &bank_client, @@ -2029,7 +2020,7 @@ fn test_program_bpf_disguised_as_bpf_loader() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_deprecated_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let program_id = load_bpf_program( @@ -2061,7 +2052,7 @@ fn test_program_bpf_c_dup() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let account_address = Pubkey::new_unique(); let account = AccountSharedData::new_data(42, &[1_u8, 2, 3], &system_program::id()).unwrap(); @@ -2092,9 +2083,9 @@ fn test_program_bpf_upgrade_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); let invoke_and_return = load_bpf_program( &bank_client, @@ -2206,9 +2197,9 @@ fn test_program_bpf_upgrade_self_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); let noop_program_id = load_bpf_program( @@ -2296,9 +2287,9 @@ fn test_program_bpf_set_upgrade_authority_via_cpi() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank_client = BankClient::new(bank); // Deploy CPI invoker program @@ -2389,7 +2380,7 @@ fn test_program_upgradeable_locks() { } = create_genesis_config(2_000_000_000); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2524,7 +2515,7 @@ fn test_program_bpf_finalize() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2586,7 +2577,7 @@ fn test_program_bpf_ro_account_modify() { } = create_genesis_config(50); let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2654,7 +2645,7 @@ fn test_program_bpf_realloc() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); @@ -2910,7 +2901,7 @@ fn test_program_bpf_realloc_invoke() { let mut bank = Bank::new_for_tests(&genesis_config); let (name, id, entrypoint) = solana_bpf_loader_program!(); - bank.add_builtin(&name, &id, entrypoint); + bank.add_builtin(&name, id, entrypoint); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 843cfd1ce38a4e..67ddacbf4ffacb 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1672,7 +1672,7 @@ mod tests { bank.feature_set = Arc::new(FeatureSet::all_enabled()); bank.add_builtin( "solana_bpf_loader_upgradeable_program", - &bpf_loader_upgradeable::id(), + bpf_loader_upgradeable::id(), process_instruction, ); let bank = Arc::new(bank); diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 3cd423cecbf75d..d97ce1f5c8196a 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -22,8 +22,8 @@ use solana_sdk::{ allow_native_ids, blake3_syscall_enabled, check_seed_length, close_upgradeable_program_accounts, demote_program_write_locks, disable_fees_sysvar, do_support_realloc, libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, - prevent_calling_precompiles_as_programs, return_data_syscall_enabled, - secp256k1_recover_syscall_enabled, sol_log_data_syscall_enabled, + return_data_syscall_enabled, secp256k1_recover_syscall_enabled, + sol_log_data_syscall_enabled, }, hash::{Hasher, HASH_BYTES}, ic_msg, @@ -31,7 +31,6 @@ 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}, @@ -2158,21 +2157,16 @@ fn check_account_infos( fn check_authorized_program( program_id: &Pubkey, instruction_data: &[u8], - invoke_context: &dyn InvokeContext, + close_upgradeable_program_accounts: bool, ) -> 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) - || (invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()) + || (close_upgradeable_program_accounts && 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()); } @@ -2210,7 +2204,11 @@ 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)?; + check_authorized_program( + &instruction.program_id, + &instruction.data, + invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()), + )?; let (account_indices, mut accounts) = syscall.translate_accounts( &message, account_infos_addr, diff --git a/programs/ed25519/Cargo.toml b/programs/ed25519/Cargo.toml new file mode 100644 index 00000000000000..18f484de57e299 --- /dev/null +++ b/programs/ed25519/Cargo.toml @@ -0,0 +1,25 @@ +[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 new file mode 100644 index 00000000000000..da579fb1d19f8a --- /dev/null +++ b/programs/ed25519/src/lib.rs @@ -0,0 +1,55 @@ +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/failure/tests/failure.rs b/programs/failure/tests/failure.rs index 54bf7da7acaa65..ba43045492f29a 100644 --- a/programs/failure/tests/failure.rs +++ b/programs/failure/tests/failure.rs @@ -12,7 +12,7 @@ fn test_program_native_failure() { let (genesis_config, alice_keypair) = create_genesis_config(50); let program_id = solana_sdk::pubkey::new_rand(); let bank = Bank::new_for_tests(&genesis_config); - bank.add_builtin_account("solana_failure_program", &program_id, false); + bank.add_native_program("solana_failure_program", &program_id, false); // Call user program let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8); diff --git a/programs/ownable/src/ownable_processor.rs b/programs/ownable/src/ownable_processor.rs index 7254cbb3e223d8..c415f2d5d25978 100644 --- a/programs/ownable/src/ownable_processor.rs +++ b/programs/ownable/src/ownable_processor.rs @@ -74,7 +74,7 @@ mod tests { fn create_bank(lamports: u64) -> (Bank, Keypair) { let (genesis_config, mint_keypair) = create_genesis_config(lamports); let mut bank = Bank::new_for_tests(&genesis_config); - bank.add_builtin("ownable_program", &crate::id(), process_instruction); + bank.add_builtin("ownable_program", crate::id(), process_instruction); (bank, mint_keypair) } diff --git a/programs/secp256k1/Cargo.toml b/programs/secp256k1/Cargo.toml new file mode 100644 index 00000000000000..c8b91cf6b2a40c --- /dev/null +++ b/programs/secp256k1/Cargo.toml @@ -0,0 +1,26 @@ +[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 new file mode 100644 index 00000000000000..de879e0924e5b7 --- /dev/null +++ b/programs/secp256k1/src/lib.rs @@ -0,0 +1,69 @@ +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 893dd6a90862e5..94f0009af96d19 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -34,6 +34,7 @@ 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" } @@ -43,6 +44,7 @@ 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" @@ -56,8 +58,6 @@ crate-type = ["lib"] name = "solana_runtime" [dev-dependencies] -ed25519-dalek = "=1.0.1" -libsecp256k1 = "0.6.0" assert_matches = "1.5.0" [package.metadata.docs.rs] diff --git a/runtime/benches/bank.rs b/runtime/benches/bank.rs index d47ea8f6ecb834..a061f83d9d5188 100644 --- a/runtime/benches/bank.rs +++ b/runtime/benches/bank.rs @@ -127,10 +127,10 @@ fn do_bench_transactions( let mut bank = Bank::new_for_benches(&genesis_config); bank.add_builtin( "builtin_program", - &Pubkey::new(&BUILTIN_PROGRAM_ID), + Pubkey::new(&BUILTIN_PROGRAM_ID), process_instruction, ); - bank.add_builtin_account("solana_noop_program", &Pubkey::new(&NOOP_PROGRAM_ID), false); + bank.add_native_program("solana_noop_program", &Pubkey::new(&NOOP_PROGRAM_ID), false); let bank = Arc::new(bank); let bank_client = BankClient::new_shared(&bank); let transactions = create_transactions(&bank_client, &mint_keypair); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 53e252d415489f..447535ed2b2a4e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -97,7 +97,6 @@ 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, @@ -2501,38 +2500,38 @@ impl Bank { // Add additional native programs specified in the genesis config for (name, program_id) in &genesis_config.native_instruction_processors { - self.add_builtin_account(name, program_id, false); + self.add_native_program(name, program_id, false); } } - fn burn_and_purge_account(&self, program_id: &Pubkey, mut account: AccountSharedData) { - 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); - } - // NOTE: must hold idempotent for the same set of arguments - /// Add a builtin program account - pub fn add_builtin_account(&self, name: &str, program_id: &Pubkey, must_replace: bool) { + pub fn add_native_program(&self, name: &str, program_id: &Pubkey, must_replace: bool) { let existing_genuine_program = - self.get_account_with_fixed_root(program_id) - .and_then(|account| { - // it's very unlikely to be squatted at program_id as non-system account because of burden to - // find victim's pubkey/hash. So, when account.owner is indeed native_loader's, it's - // safe to assume it's a genuine program. - if native_loader::check_id(account.owner()) { - Some(account) - } else { - // malicious account is pre-occupying at program_id - self.burn_and_purge_account(program_id, account); - None - } - }); + if let Some(mut account) = self.get_account_with_fixed_root(program_id) { + // it's very unlikely to be squatted at program_id as non-system account because of burden to + // find victim's pubkey/hash. So, when account.owner is indeed native_loader's, it's + // safe to assume it's a genuine program. + if native_loader::check_id(account.owner()) { + 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 + }; if must_replace { // updating native program + match &existing_genuine_program { None => panic!( "There is no account to replace with native program ({}, {}).", @@ -2540,16 +2539,29 @@ impl Bank { ), Some(account) => { if *name == String::from_utf8_lossy(account.data()) { - // The existing account is well formed + // nop; it seems that already AccountsDb is updated. return; } + // continue to replace account } } } else { // introducing native program - if existing_genuine_program.is_some() { - // The existing account is sufficient - return; + + match &existing_genuine_program { + None => (), // continue to add account + Some(_account) => { + // nop; it seems that we already have account + + // before returning here to retain idempotent just make sure + // the existing native program name is same with what we're + // supposed to add here (but skipping) But I can't: + // following assertion already catches several different names for same + // program_id + // depending on clusters... + // assert_eq!(name.to_owned(), String::from_utf8_lossy(&account.data)); + return; + } } } @@ -2567,37 +2579,8 @@ impl Bank { self.inherit_specially_retained_account_fields(&existing_genuine_program), ); self.store_account_and_update_capitalization(program_id, &account); - } - /// Add a precompiled program account - pub fn add_precompiled_account(&self, program_id: &Pubkey) { - if let Some(account) = self.get_account_with_fixed_root(program_id) { - if account.executable() { - // The account is already executable, that's all we need - return; - } else { - // malicious account is pre-occupying at program_id - self.burn_and_purge_account(program_id, account); - } - }; - - 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(&None); - 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 native program {} under {:?}", name, program_id); } pub fn set_rent_burn_percentage(&mut self, burn_percent: u8) { @@ -4604,15 +4587,10 @@ impl Bank { for builtin in builtins.genesis_builtins { self.add_builtin( &builtin.name, - &builtin.id, + builtin.id, builtin.process_instruction_with_context, ); } - for precompile in get_precompiles() { - if precompile.feature.is_none() { - self.add_precompile(&precompile.program_id); - } - } } self.feature_builtins = Arc::new(builtins.feature_builtins); @@ -5342,43 +5320,26 @@ impl Bank { pub fn add_builtin( &mut self, name: &str, - program_id: &Pubkey, + program_id: Pubkey, process_instruction_with_context: ProcessInstructionWithContext, ) { debug!("Adding program {} under {:?}", name, program_id); - self.add_builtin_account(name, program_id, false); + self.add_native_program(name, &program_id, false); self.message_processor .add_program(program_id, process_instruction_with_context); - debug!("Added program {} under {:?}", name, program_id); } /// Replace a builtin instruction processor if it already exists pub fn replace_builtin( &mut self, name: &str, - program_id: &Pubkey, + program_id: Pubkey, process_instruction_with_context: ProcessInstructionWithContext, ) { debug!("Replacing program {} under {:?}", name, program_id); - self.add_builtin_account(name, program_id, true); + self.add_native_program(name, &program_id, true); self.message_processor .add_program(program_id, process_instruction_with_context); - debug!("Replaced program {} under {:?}", name, program_id); - } - - /// 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); - // Don't remove the account since the bank expects the account state to - // be idempotent - self.message_processor.remove_program(program_id); - debug!("Removed program {} under {:?}", name, program_id); - } - - pub fn add_precompile(&mut self, program_id: &Pubkey) { - debug!("Adding precompiled program {}", program_id); - self.add_precompiled_account(program_id); - debug!("Added precompiled program {:?}", program_id); } pub fn clean_accounts( @@ -5649,28 +5610,17 @@ impl Bank { match activation_type { ActivationType::NewProgram => self.add_builtin( &builtin.name, - &builtin.id, + builtin.id, builtin.process_instruction_with_context, ), ActivationType::NewVersion => self.replace_builtin( &builtin.name, - &builtin.id, + 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_precompile(&precompile.program_id); - } - } } fn apply_spl_token_v2_set_authority_fix(&mut self) { @@ -6392,7 +6342,7 @@ pub(crate) mod tests { ) as u64, ); bank.rent_collector.slots_per_year = 421_812.0; - bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction); + bank.add_builtin("mock_program", mock_program_id, mock_process_instruction); bank } @@ -9878,7 +9828,7 @@ pub(crate) mod tests { assert!(bank.get_account(&mock_vote_program_id()).is_none()); bank.add_builtin( "mock_vote_program", - &mock_vote_program_id(), + mock_vote_program_id(), mock_vote_processor, ); assert!(bank.get_account(&mock_vote_program_id()).is_some()); @@ -9951,7 +9901,7 @@ pub(crate) mod tests { let vote_loader_account = bank.get_account(&solana_vote_program::id()).unwrap(); bank.add_builtin( "solana_vote_program", - &solana_vote_program::id(), + solana_vote_program::id(), mock_vote_processor, ); let new_vote_loader_account = bank.get_account(&solana_vote_program::id()).unwrap(); @@ -10000,8 +9950,8 @@ pub(crate) mod tests { assert!(!bank.stakes.read().unwrap().stake_delegations().is_empty()); assert_eq!(bank.calculate_capitalization(true), bank.capitalization()); - bank.add_builtin("mock_program1", &vote_id, mock_ix_processor); - bank.add_builtin("mock_program2", &stake_id, mock_ix_processor); + bank.add_builtin("mock_program1", vote_id, mock_ix_processor); + bank.add_builtin("mock_program2", stake_id, mock_ix_processor); { let stakes = bank.stakes.read().unwrap(); assert!(stakes.vote_accounts().as_ref().is_empty()); @@ -10020,8 +9970,8 @@ pub(crate) mod tests { // Re-adding builtin programs should be no-op bank.update_accounts_hash(); let old_hash = bank.get_accounts_hash(); - bank.add_builtin("mock_program1", &vote_id, mock_ix_processor); - bank.add_builtin("mock_program2", &stake_id, mock_ix_processor); + bank.add_builtin("mock_program1", vote_id, mock_ix_processor); + bank.add_builtin("mock_program2", stake_id, mock_ix_processor); bank.update_accounts_hash(); let new_hash = bank.get_accounts_hash(); assert_eq!(old_hash, new_hash); @@ -10835,7 +10785,7 @@ pub(crate) mod tests { } let mock_program_id = Pubkey::new(&[2u8; 32]); - bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction); + bank.add_builtin("mock_program", mock_program_id, mock_process_instruction); let from_pubkey = solana_sdk::pubkey::new_rand(); let to_pubkey = solana_sdk::pubkey::new_rand(); @@ -10879,7 +10829,7 @@ pub(crate) mod tests { } let mock_program_id = Pubkey::new(&[2u8; 32]); - bank.add_builtin("mock_program", &mock_program_id, mock_process_instruction); + bank.add_builtin("mock_program", mock_program_id, mock_process_instruction); let from_pubkey = solana_sdk::pubkey::new_rand(); let to_pubkey = solana_sdk::pubkey::new_rand(); @@ -10934,7 +10884,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - &solana_vote_program::id(), + solana_vote_program::id(), mock_ok_vote_processor, ); let result = bank.process_transaction(&tx); @@ -10988,7 +10938,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - &solana_vote_program::id(), + solana_vote_program::id(), mock_ok_vote_processor, ); @@ -11022,7 +10972,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - &solana_vote_program::id(), + solana_vote_program::id(), mock_ok_vote_processor, ); @@ -11079,7 +11029,7 @@ pub(crate) mod tests { bank.add_builtin( "mock_vote", - &solana_vote_program::id(), + solana_vote_program::id(), mock_ok_vote_processor, ); @@ -11114,7 +11064,7 @@ pub(crate) mod tests { .map(|i| { let key = solana_sdk::pubkey::new_rand(); let name = format!("program{:?}", i); - bank.add_builtin(&name, &key, mock_ok_vote_processor); + bank.add_builtin(&name, key, mock_ok_vote_processor); (key, name.as_bytes().to_vec()) }) .collect(); @@ -11323,7 +11273,7 @@ pub(crate) mod tests { // Add a new program let program1_pubkey = solana_sdk::pubkey::new_rand(); - bank.add_builtin("program", &program1_pubkey, nested_processor); + bank.add_builtin("program", program1_pubkey, nested_processor); // Add a new program owned by the first let program2_pubkey = solana_sdk::pubkey::new_rand(); @@ -11690,24 +11640,20 @@ pub(crate) mod tests { )); assert_eq!(bank.get_account_modified_slot(&program_id), None); - Arc::get_mut(&mut bank).unwrap().add_builtin( - "mock_program", - &program_id, - mock_ix_processor, - ); + Arc::get_mut(&mut bank) + .unwrap() + .add_builtin("mock_program", program_id, mock_ix_processor); assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); let mut bank = Arc::new(new_from_parent(&bank)); - Arc::get_mut(&mut bank).unwrap().add_builtin( - "mock_program", - &program_id, - mock_ix_processor, - ); + Arc::get_mut(&mut bank) + .unwrap() + .add_builtin("mock_program", program_id, mock_ix_processor); assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); Arc::get_mut(&mut bank).unwrap().replace_builtin( "mock_program v2", - &program_id, + program_id, mock_ix_processor, ); assert_eq!( @@ -11741,18 +11687,18 @@ pub(crate) mod tests { Arc::get_mut(&mut bank) .unwrap() - .add_builtin("mock_program", &loader_id, mock_ix_processor); + .add_builtin("mock_program", loader_id, mock_ix_processor); assert_eq!(bank.get_account_modified_slot(&loader_id).unwrap().1, slot); let mut bank = Arc::new(new_from_parent(&bank)); Arc::get_mut(&mut bank) .unwrap() - .add_builtin("mock_program", &loader_id, mock_ix_processor); + .add_builtin("mock_program", loader_id, mock_ix_processor); assert_eq!(bank.get_account_modified_slot(&loader_id).unwrap().1, slot); } #[test] - fn test_add_builtin_account() { + fn test_add_native_program() { let (mut genesis_config, _mint_keypair) = create_genesis_config(100_000); activate_all_features(&mut genesis_config); @@ -11768,7 +11714,7 @@ pub(crate) mod tests { assert_capitalization_diff( &bank, - || bank.add_builtin_account("mock_program", &program_id, false), + || bank.add_native_program("mock_program", &program_id, false), |old, new| { assert_eq!(old + 1, new); }, @@ -11779,7 +11725,7 @@ pub(crate) mod tests { let bank = Arc::new(new_from_parent(&bank)); assert_capitalization_diff( &bank, - || bank.add_builtin_account("mock_program", &program_id, false), + || bank.add_native_program("mock_program", &program_id, false), |old, new| assert_eq!(old, new), ); @@ -11790,7 +11736,7 @@ pub(crate) mod tests { // invocations. assert_capitalization_diff( &bank, - || bank.add_builtin_account("mock_program v2", &program_id, true), + || bank.add_native_program("mock_program v2", &program_id, true), |old, new| assert_eq!(old, new), ); @@ -11802,7 +11748,7 @@ pub(crate) mod tests { let bank = Arc::new(new_from_parent(&bank)); assert_capitalization_diff( &bank, - || bank.add_builtin_account("mock_program v2", &program_id, true), + || bank.add_native_program("mock_program v2", &program_id, true), |old, new| assert_eq!(old, new), ); @@ -11814,12 +11760,12 @@ pub(crate) mod tests { } #[test] - fn test_add_builtin_account_inherited_cap_while_replacing() { + fn test_add_native_program_inherited_cap_while_replacing() { let (genesis_config, mint_keypair) = create_genesis_config(100_000); let bank = Bank::new_for_tests(&genesis_config); let program_id = solana_sdk::pubkey::new_rand(); - bank.add_builtin_account("mock_program", &program_id, false); + bank.add_native_program("mock_program", &program_id, false); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); // someone mess with program_id's balance @@ -11828,12 +11774,12 @@ pub(crate) mod tests { bank.deposit(&program_id, 10).unwrap(); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - bank.add_builtin_account("mock_program v2", &program_id, true); + bank.add_native_program("mock_program v2", &program_id, true); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } #[test] - fn test_add_builtin_account_squatted_while_not_replacing() { + fn test_add_native_program_squatted_while_not_replacing() { let (genesis_config, mint_keypair) = create_genesis_config(100_000); let bank = Bank::new_for_tests(&genesis_config); let program_id = solana_sdk::pubkey::new_rand(); @@ -11844,7 +11790,7 @@ pub(crate) mod tests { bank.deposit(&program_id, 10).unwrap(); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - bank.add_builtin_account("mock_program", &program_id, false); + bank.add_native_program("mock_program", &program_id, false); assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } @@ -11854,7 +11800,7 @@ pub(crate) mod tests { program (mock_program, CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \ Maybe, inconsistent program activation is detected on snapshot restore?" )] - fn test_add_builtin_account_after_frozen() { + fn test_add_native_program_after_frozen() { use std::str::FromStr; let (genesis_config, _mint_keypair) = create_genesis_config(100_000); @@ -11868,7 +11814,7 @@ pub(crate) mod tests { ); bank.freeze(); - bank.add_builtin_account("mock_program", &program_id, false); + bank.add_native_program("mock_program", &program_id, false); } #[test] @@ -11876,99 +11822,7 @@ pub(crate) mod tests { expected = "There is no account to replace with native program (mock_program, \ CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre)." )] - fn test_add_builtin_account_replace_none() { - use std::str::FromStr; - let (genesis_config, _mint_keypair) = create_genesis_config(100_000); - - let slot = 123; - let program_id = Pubkey::from_str("CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre").unwrap(); - - let bank = Bank::new_from_parent( - &Arc::new(Bank::new_for_tests(&genesis_config)), - &Pubkey::default(), - slot, - ); - - bank.add_builtin_account("mock_program", &program_id, true); - } - - #[test] - fn test_add_precompiled_account() { - let (mut genesis_config, _mint_keypair) = create_genesis_config(100_000); - activate_all_features(&mut genesis_config); - - let slot = 123; - let program_id = solana_sdk::pubkey::new_rand(); - - let bank = Arc::new(Bank::new_from_parent( - &Arc::new(Bank::new_for_tests(&genesis_config)), - &Pubkey::default(), - slot, - )); - assert_eq!(bank.get_account_modified_slot(&program_id), None); - - assert_capitalization_diff( - &bank, - || bank.add_precompiled_account(&program_id), - |old, new| { - assert_eq!(old + 1, new); - }, - ); - - assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); - - let bank = Arc::new(new_from_parent(&bank)); - assert_capitalization_diff( - &bank, - || bank.add_precompiled_account(&program_id), - |old, new| assert_eq!(old, new), - ); - - assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot); - } - - #[test] - fn test_add_precompiled_account_inherited_cap_while_replacing() { - let (genesis_config, mint_keypair) = create_genesis_config(100_000); - let bank = Bank::new_for_tests(&genesis_config); - let program_id = solana_sdk::pubkey::new_rand(); - - bank.add_precompiled_account(&program_id); - assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - - // someone mess with program_id's balance - bank.withdraw(&mint_keypair.pubkey(), 10).unwrap(); - assert_ne!(bank.capitalization(), bank.calculate_capitalization(true)); - bank.deposit(&program_id, 10).unwrap(); - assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - - bank.add_precompiled_account(&program_id); - assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - } - - #[test] - fn test_add_precompiled_account_squatted_while_not_replacing() { - let (genesis_config, mint_keypair) = create_genesis_config(100_000); - let bank = Bank::new_for_tests(&genesis_config); - let program_id = solana_sdk::pubkey::new_rand(); - - // someone managed to squat at program_id! - bank.withdraw(&mint_keypair.pubkey(), 10).unwrap(); - assert_ne!(bank.capitalization(), bank.calculate_capitalization(true)); - bank.deposit(&program_id, 10).unwrap(); - assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - - bank.add_precompiled_account(&program_id); - assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); - } - - #[test] - #[should_panic( - expected = "Can't change frozen bank by adding not-existing new precompiled \ - program (CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre). \ - Maybe, inconsistent program activation is detected on snapshot restore?" - )] - fn test_add_precompiled_account_after_frozen() { + fn test_add_native_program_replace_none() { use std::str::FromStr; let (genesis_config, _mint_keypair) = create_genesis_config(100_000); @@ -11980,9 +11834,8 @@ pub(crate) mod tests { &Pubkey::default(), slot, ); - bank.freeze(); - bank.add_precompiled_account(&program_id); + bank.add_native_program("mock_program", &program_id, true); } #[test] @@ -14179,7 +14032,7 @@ pub(crate) mod tests { } let program_id = solana_sdk::pubkey::new_rand(); - bank.add_builtin("mock_program1", &program_id, mock_ix_processor); + bank.add_builtin("mock_program1", program_id, mock_ix_processor); let blockhash = bank.last_blockhash(); #[allow(deprecated)] @@ -14391,7 +14244,7 @@ pub(crate) mod tests { Ok(()) } let program_id = solana_sdk::pubkey::new_rand(); - bank.add_builtin("mock_program", &program_id, mock_ix_processor); + bank.add_builtin("mock_program", program_id, mock_ix_processor); let message = Message::new( &[ @@ -14544,47 +14397,4 @@ pub(crate) mod tests { ); } } - - #[test] - fn test_call_precomiled_program() { - let GenesisConfigInfo { - mut genesis_config, - mint_keypair, - .. - } = create_genesis_config_with_leader(42, &Pubkey::new_unique(), 42); - activate_all_features(&mut genesis_config); - let bank = Bank::new_for_tests(&genesis_config); - - // libsecp256k1 - let secp_privkey = libsecp256k1::SecretKey::random(&mut rand::thread_rng()); - let message_arr = b"hello"; - let instruction = solana_sdk::secp256k1_instruction::new_secp256k1_instruction( - &secp_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(); - - // ed25519 - 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 eeb2093927bd2a..e0e59a2a6965f6 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -46,7 +46,6 @@ macro_rules! with_program_logging { pub enum ActivationType { NewProgram, NewVersion, - RemoveProgram, } #[derive(Clone)] @@ -92,7 +91,7 @@ pub struct Builtins { /// Builtin programs that are always available pub genesis_builtins: Vec, - /// Builtin programs activated or deactivated dynamically by feature + /// Builtin programs activated dynamically by feature pub feature_builtins: Vec<(Builtin, Pubkey, ActivationType)>, } @@ -122,20 +121,11 @@ fn genesis_builtins() -> Vec { Builtin::new( "secp256k1_program", solana_sdk::secp256k1_program::id(), - dummy_process_instruction, + solana_secp256k1_program::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 @@ -155,17 +145,14 @@ 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( - "secp256k1_program", - solana_sdk::secp256k1_program::id(), - dummy_process_instruction, + "ed25519_program", + solana_sdk::ed25519_program::id(), + solana_ed25519_program::process_instruction, ), - feature_set::prevent_calling_precompiles_as_programs::id(), - ActivationType::RemoveProgram, + feature_set::ed25519_program_enabled::id(), + ActivationType::NewProgram, ), ] } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index ca97b372c7f266..b793d3cab6d9da 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -11,7 +11,7 @@ use solana_sdk::{ compute_budget::ComputeBudget, feature_set::{ demote_program_write_locks, do_support_realloc, neon_evm_compute_budget, - prevent_calling_precompiles_as_programs, tx_wide_compute_cap, FeatureSet, + tx_wide_compute_cap, FeatureSet, }, fee_calculator::FeeCalculator, hash::Hash, @@ -19,7 +19,6 @@ 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, @@ -401,18 +400,13 @@ impl MessageProcessor { /// Add a static entrypoint to intercept instructions before the dynamic loader. pub fn add_program( &mut self, - program_id: &Pubkey, + program_id: Pubkey, process_instruction: ProcessInstructionWithContext, ) { self.instruction_processor .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( @@ -537,14 +531,6 @@ 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, |id| feature_set.is_active(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()) { @@ -686,8 +672,6 @@ mod tests { message::Message, native_loader::{self, create_loadable_account_for_test}, process_instruction::MockComputeMeter, - secp256k1_instruction::new_secp256k1_instruction, - secp256k1_program, }; #[test] @@ -880,7 +864,7 @@ mod tests { let mock_system_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); let mut message_processor = MessageProcessor::default(); - message_processor.add_program(&mock_system_program_id, mock_system_process_instruction); + message_processor.add_program(mock_system_program_id, mock_system_process_instruction); let program_account = Rc::new(RefCell::new(create_loadable_account_for_test( "mock_system_program", @@ -1068,7 +1052,7 @@ mod tests { let mock_program_id = Pubkey::new(&[2u8; 32]); let rent_collector = RentCollector::default(); let mut message_processor = MessageProcessor::default(); - message_processor.add_program(&mock_program_id, mock_system_process_instruction); + message_processor.add_program(mock_program_id, mock_system_process_instruction); let program_account = Rc::new(RefCell::new(create_loadable_account_for_test( "mock_system_program", @@ -1595,63 +1579,4 @@ mod tests { ); } } - - #[test] - fn test_precompile() { - let mut message_processor = MessageProcessor::default(); - let mock_program_id = Pubkey::new_unique(); - fn mock_process_instruction( - _program_id: &Pubkey, - _data: &[u8], - _invoke_context: &mut dyn InvokeContext, - ) -> Result<(), InstructionError> { - Err(InstructionError::Custom(0xbabb1e)) - } - message_processor.add_program(&mock_program_id, mock_process_instruction); - - let secp256k1_account = AccountSharedData::new_ref(1, 0, &native_loader::id()); - secp256k1_account.borrow_mut().set_executable(true); - let mock_program_account = AccountSharedData::new_ref(1, 0, &native_loader::id()); - mock_program_account.borrow_mut().set_executable(true); - let accounts = vec![ - (secp256k1_program::id(), secp256k1_account), - (mock_program_id, mock_program_account), - ]; - - let message = Message::new( - &[ - new_secp256k1_instruction( - &libsecp256k1::SecretKey::random(&mut rand::thread_rng()), - b"hello", - ), - Instruction::new_with_bytes(mock_program_id, &[], vec![]), - ], - None, - ); - - let result = message_processor.process_message( - &message, - &[vec![0], vec![1]], - &accounts, - &RentCollector::default(), - None, - Rc::new(RefCell::new(Executors::default())), - None, - Arc::new(FeatureSet::all_enabled()), - ComputeBudget::new(), - Rc::new(RefCell::new(MockComputeMeter::default())), - &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &Ancestors::default(), - Hash::default(), - FeeCalculator::default(), - ); - assert_eq!( - result, - Err(TransactionError::InstructionError( - 1, - InstructionError::Custom(0xbabb1e) - )) - ); - } } diff --git a/runtime/tests/noop.rs b/runtime/tests/noop.rs index 3f550323fc3c00..3e26389b82e144 100644 --- a/runtime/tests/noop.rs +++ b/runtime/tests/noop.rs @@ -10,7 +10,7 @@ fn test_program_native_noop() { let (genesis_config, alice_keypair) = create_genesis_config(50); let program_id = solana_sdk::pubkey::new_rand(); let bank = Bank::new_for_tests(&genesis_config); - bank.add_builtin_account("solana_noop_program", &program_id, false); + bank.add_native_program("solana_noop_program", &program_id, false); // Call user program let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8); diff --git a/sdk/src/ed25519_instruction.rs b/sdk/src/ed25519_instruction.rs index 052d4617b99fc2..704b210cbb2adf 100644 --- a/sdk/src/ed25519_instruction.rs +++ b/sdk/src/ed25519_instruction.rs @@ -170,15 +170,6 @@ 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, @@ -331,34 +322,4 @@ 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 e65ff489d4a0f2..502d0b4728af6e 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -223,12 +223,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"); -} - lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -280,7 +274,6 @@ lazy_static! { (sol_log_data_syscall_enabled::id(), "enable sol_log_data syscall"), (stakes_remove_delegation_if_inactive::id(), "remove delegations from stakes cache when inactive"), (do_support_realloc::id(), "support account data reallocation"), - (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 f1fff29213ff28..3372fca3c4737a 100644 --- a/sdk/src/precompiles.rs +++ b/sdk/src/precompiles.rs @@ -5,9 +5,7 @@ use { crate::{ decode_error::DecodeError, - feature_set::{ - ed25519_program_enabled, prevent_calling_precompiles_as_programs, FeatureSet, - }, + feature_set::{ed25519_program_enabled, FeatureSet}, instruction::CompiledInstruction, pubkey::Pubkey, }, @@ -40,7 +38,7 @@ impl DecodeError for PrecompileError { pub type Verify = fn(&[u8], &[&[u8]], &Arc) -> std::result::Result<(), PrecompileError>; /// Information on a precompiled program -pub struct Precompile { +struct Precompile { /// Program id pub program_id: Pubkey, /// Feature to enable on, `None` indicates always enabled @@ -58,13 +56,8 @@ impl Precompile { } } /// Check if a program id is this precompiled program - 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 + 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 } /// Verify this precompiled program pub fn verify( @@ -82,7 +75,7 @@ lazy_static! { static ref PRECOMPILES: Vec = vec![ Precompile::new( crate::secp256k1_program::id(), - Some(prevent_calling_precompiles_as_programs::id()), + None, crate::secp256k1_instruction::verify, ), Precompile::new( @@ -94,17 +87,10 @@ lazy_static! { } /// Check if a program is a precompiled program -pub fn is_precompile(program_id: &Pubkey, is_enabled: F) -> bool -where - F: Fn(&Pubkey) -> bool, -{ +pub fn is_precompile(program_id: &Pubkey, feature_set: &Arc) -> bool { PRECOMPILES .iter() - .any(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id))) -} - -pub fn get_precompiles<'a>() -> &'a [Precompile] { - &PRECOMPILES + .any(|precompile| precompile.check_id(program_id, feature_set)) } /// Check that a program is precompiled and if so verify it @@ -115,7 +101,7 @@ pub fn verify_if_precompile( feature_set: &Arc, ) -> Result<(), PrecompileError> { for precompile in PRECOMPILES.iter() { - if precompile.check_id(program_id, |feature_id| feature_set.is_active(feature_id)) { + if precompile.check_id(program_id, feature_set) { 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 1cf471db9b681a..299237b9cbb5d6 100644 --- a/sdk/src/secp256k1_instruction.rs +++ b/sdk/src/secp256k1_instruction.rs @@ -212,17 +212,6 @@ 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, @@ -401,46 +390,4 @@ 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()); - } }