diff --git a/Cargo.lock b/Cargo.lock index d9175d0d9c0682..c6ab0a2fdce279 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7382,6 +7382,8 @@ dependencies = [ "libsecp256k1", "log", "memmap2", + "num-derive", + "num-traits", "num_enum", "pbkdf2 0.11.0", "qstring", diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 002e77b0549061..10cc43a5878619 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -17,7 +17,7 @@ use { account::Account, clock::Slot, commitment_config::CommitmentLevel, - feature_set::FeatureSet, + feature_set::{self, FeatureSet}, hash::Hash, message::{Message, SanitizedMessage}, pubkey::Pubkey, @@ -163,7 +163,13 @@ fn verify_transaction( feature_set: &Arc, ) -> transaction::Result<()> { transaction.verify()?; - transaction.verify_precompiles(feature_set)?; + + let move_precompile_verification_to_svm = + feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id()); + if !move_precompile_verification_to_svm { + transaction.verify_precompiles(feature_set)?; + } + Ok(()) } diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 9965b1c3214c3d..ce2affa78931dd 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -390,11 +390,20 @@ impl Consumer { let check_results = bank.check_transactions(txs, &pre_results, MAX_PROCESSING_AGE, &mut error_counters); // If checks passed, verify pre-compiles and continue processing on success. + let move_precompile_verification_to_svm = bank + .feature_set + .is_active(&feature_set::move_precompile_verification_to_svm::id()); let check_results: Vec<_> = txs .iter() .zip(check_results) .map(|(tx, result)| match result { - Ok(_) => tx.verify_precompiles(&bank.feature_set), + Ok(_) => { + if !move_precompile_verification_to_svm { + tx.verify_precompiles(&bank.feature_set) + } else { + Ok(()) + } + } Err(err) => Err(err), }) .collect(); @@ -419,7 +428,10 @@ impl Consumer { txs: &[SanitizedTransaction], max_slot_ages: &[Slot], ) -> ProcessTransactionBatchOutput { - // Verify pre-compiles. + let move_precompile_verification_to_svm = bank + .feature_set + .is_active(&feature_set::move_precompile_verification_to_svm::id()); + // Need to filter out transactions since they were sanitized earlier. // This means that the transaction may cross and epoch boundary (not allowed), // or account lookup tables may have been closed. @@ -437,7 +449,10 @@ impl Consumer { } } else { // Verify pre-compiles. - tx.verify_precompiles(&bank.feature_set)?; + if !move_precompile_verification_to_svm { + tx.verify_precompiles(&bank.feature_set)?; + } + // Any transaction executed between sanitization time and now may have closed the lookup table(s). // Above re-sanitization already loads addresses, so don't need to re-check in that case. let lookup_tables = tx.message().message_address_table_lookups(); diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 5c732463c9c353..9194f38b714d3b 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -22,10 +22,11 @@ use { bpf_loader_deprecated, clock::Slot, epoch_schedule::EpochSchedule, - feature_set::FeatureSet, + feature_set::{self, FeatureSet}, hash::Hash, instruction::{AccountMeta, InstructionError}, native_loader, + precompiles::Precompile, pubkey::Pubkey, saturating_add_assign, stable_layout::stable_instruction::StableInstruction, @@ -465,6 +466,38 @@ impl<'a> InvokeContext<'a> { .and(self.pop()) } + /// Processes a precompile instruction + pub fn process_precompile<'ix_data>( + &mut self, + precompile: &Precompile, + precompile_instruction_data: &[u8], + message_instruction_datas_iter: impl Iterator, + instruction_accounts: &[InstructionAccount], + program_indices: &[IndexOfAccount], + ) -> Result<(), InstructionError> { + self.transaction_context + .get_next_instruction_context()? + .configure( + program_indices, + instruction_accounts, + precompile_instruction_data, + ); + self.push()?; + + let feature_set = self.get_feature_set(); + let move_precompile_verification_to_svm = + feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id()); + if move_precompile_verification_to_svm { + let instruction_datas: Vec<_> = message_instruction_datas_iter.collect(); + precompile + .verify(precompile_instruction_data, &instruction_datas, feature_set) + .map_err(InstructionError::from) + .and(self.pop()) + } else { + self.pop() + } + } + /// Calls the instruction's program entrypoint method fn process_executable_chain( &mut self, diff --git a/programs/ed25519-tests/tests/process_transaction.rs b/programs/ed25519-tests/tests/process_transaction.rs index 25897f8fd2075a..e8b1c3b85a1c8b 100644 --- a/programs/ed25519-tests/tests/process_transaction.rs +++ b/programs/ed25519-tests/tests/process_transaction.rs @@ -3,6 +3,8 @@ use { solana_program_test::*, solana_sdk::{ ed25519_instruction::new_ed25519_instruction, + instruction::InstructionError, + precompiles::PrecompileError, signature::Signer, transaction::{Transaction, TransactionError}, }, @@ -68,7 +70,9 @@ async fn test_failure() { assert_matches!( client.process_transaction(transaction).await, Err(BanksClientError::TransactionError( - TransactionError::InvalidAccountIndex + TransactionError::InstructionError(0, InstructionError::Custom(3)) )) ); + // this assert is for documenting the matched error code above + assert_eq!(3, PrecompileError::InvalidDataOffsets as u32); } diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 16a475c9d77ac0..2371bce80857e1 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -6191,6 +6191,8 @@ dependencies = [ "libsecp256k1 0.6.0", "log", "memmap2", + "num-derive", + "num-traits", "num_enum", "pbkdf2 0.11.0", "qstring", diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index d62a61ec81fe00..c50bf95330fc1c 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -2252,8 +2252,12 @@ fn verify_transaction( return Err(RpcCustomError::TransactionSignatureVerificationFailure.into()); } - if let Err(e) = transaction.verify_precompiles(feature_set) { - return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into()); + let move_precompile_verification_to_svm = + feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id()); + if !move_precompile_verification_to_svm { + if let Err(e) = transaction.verify_precompiles(feature_set) { + return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into()); + } } Ok(()) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cebf31f2d7f945..18cc3cc40f804a 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5606,9 +5606,13 @@ impl Bank { ) }?; - if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles - || verification_mode == TransactionVerificationMode::FullVerification - { + let move_precompile_verification_to_svm = self + .feature_set + .is_active(&feature_set::move_precompile_verification_to_svm::id()); + if !move_precompile_verification_to_svm && { + verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles + || verification_mode == TransactionVerificationMode::FullVerification + } { sanitized_tx.verify_precompiles(&self.feature_set)?; } diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index 7db5d780c76b07..8aaf0fe1f4280a 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -67,6 +67,8 @@ lazy_static = { workspace = true } libsecp256k1 = { workspace = true, optional = true, features = ["hmac"] } log = { workspace = true } memmap2 = { workspace = true, optional = true } +num-derive = { workspace = true } +num-traits = { workspace = true } num_enum = { workspace = true } pbkdf2 = { workspace = true } qstring = { workspace = true } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 887d2e547f19b2..a136a90ab5413a 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -841,6 +841,10 @@ pub mod vote_only_retransmitter_signed_fec_sets { solana_sdk::declare_id!("RfEcA95xnhuwooVAhUUksEJLZBF7xKCLuqrJoqk4Zph"); } +pub mod move_precompile_verification_to_svm { + solana_sdk::declare_id!("9ypxGLzkMxi89eDerRKXWDXe44UY2z4hBig4mDhNq5Dp"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -1046,6 +1050,7 @@ lazy_static! { (move_stake_and_move_lamports_ixs::id(), "Enable MoveStake and MoveLamports stake program instructions #1610"), (ed25519_precompile_verify_strict::id(), "Use strict verification in ed25519 precompile SIMD-0152"), (vote_only_retransmitter_signed_fec_sets::id(), "vote only on retransmitter signed fec sets"), + (move_precompile_verification_to_svm::id(), "Move precompile verification into SVM SIMD-0159"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/precompiles.rs b/sdk/src/precompiles.rs index 04e1e2ea2ec389..cb16e5ecd86276 100644 --- a/sdk/src/precompiles.rs +++ b/sdk/src/precompiles.rs @@ -3,14 +3,16 @@ #![cfg(feature = "full")] use { - crate::{feature_set::FeatureSet, instruction::CompiledInstruction, pubkey::Pubkey}, + crate::{feature_set::FeatureSet, pubkey::Pubkey}, lazy_static::lazy_static, + num_derive::{FromPrimitive, ToPrimitive}, solana_decode_error::DecodeError, + solana_program::instruction::CompiledInstruction, thiserror::Error, }; /// Precompile errors -#[derive(Error, Debug, Clone, PartialEq, Eq)] +#[derive(Error, Debug, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)] pub enum PrecompileError { #[error("public key is not valid")] InvalidPublicKey, @@ -23,6 +25,7 @@ pub enum PrecompileError { #[error("instruction is incorrect size")] InvalidInstructionDataSize, } + impl DecodeError for PrecompileError { fn type_of() -> &'static str { "PrecompileError" @@ -96,6 +99,16 @@ where .any(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id))) } +/// Find an enabled precompiled program +pub fn get_precompile(program_id: &Pubkey, is_enabled: F) -> Option<&Precompile> +where + F: Fn(&Pubkey) -> bool, +{ + PRECOMPILES + .iter() + .find(|precompile| precompile.check_id(program_id, |feature_id| is_enabled(feature_id))) +} + pub fn get_precompiles<'a>() -> &'a [Precompile] { &PRECOMPILES } diff --git a/svm/src/message_processor.rs b/svm/src/message_processor.rs index 21348bc1ae7d95..81f4b2c6d2551a 100644 --- a/svm/src/message_processor.rs +++ b/svm/src/message_processor.rs @@ -1,9 +1,9 @@ use { - solana_measure::measure::Measure, + solana_measure::measure_us, solana_program_runtime::invoke_context::InvokeContext, solana_sdk::{ account::WritableAccount, - precompiles::is_precompile, + precompiles::get_precompile, saturating_add_assign, sysvar::instructions, transaction::TransactionError, @@ -44,10 +44,6 @@ impl MessageProcessor { .zip(program_indices.iter()) .enumerate() { - let is_precompile = is_precompile(program_id, |id| { - invoke_context.get_feature_set().is_active(id) - }); - // Fixup the special instructions key if present // before the account pre-values are taken care of if let Some(account_index) = invoke_context @@ -87,53 +83,48 @@ impl MessageProcessor { }); } - let result = if is_precompile { - invoke_context - .transaction_context - .get_next_instruction_context() - .map(|instruction_context| { - instruction_context.configure( - program_indices, - &instruction_accounts, - instruction.data, - ); - }) - .and_then(|_| { - invoke_context.transaction_context.push()?; - invoke_context.transaction_context.pop() - }) - } else { - let time = Measure::start("execute_instruction"); - let mut compute_units_consumed = 0; - let result = invoke_context.process_instruction( - instruction.data, - &instruction_accounts, - program_indices, - &mut compute_units_consumed, - execute_timings, - ); - let time = time.end_as_us(); - *accumulated_consumed_units = - accumulated_consumed_units.saturating_add(compute_units_consumed); - execute_timings.details.accumulate_program( - program_id, - time, - compute_units_consumed, - result.is_err(), - ); - invoke_context.timings = { - execute_timings.details.accumulate(&invoke_context.timings); - ExecuteDetailsTimings::default() - }; - saturating_add_assign!( - execute_timings - .execute_accessories - .process_instructions - .total_us, - time - ); - result + let mut compute_units_consumed = 0; + let (result, process_instruction_us) = measure_us!({ + if let Some(precompile) = get_precompile(program_id, |feature_id| { + invoke_context.get_feature_set().is_active(feature_id) + }) { + invoke_context.process_precompile( + precompile, + instruction.data, + message.instructions_iter().map(|ix| ix.data), + &instruction_accounts, + program_indices, + ) + } else { + invoke_context.process_instruction( + instruction.data, + &instruction_accounts, + program_indices, + &mut compute_units_consumed, + execute_timings, + ) + } + }); + + *accumulated_consumed_units = + accumulated_consumed_units.saturating_add(compute_units_consumed); + execute_timings.details.accumulate_program( + program_id, + process_instruction_us, + compute_units_consumed, + result.is_err(), + ); + invoke_context.timings = { + execute_timings.details.accumulate(&invoke_context.timings); + ExecuteDetailsTimings::default() }; + saturating_add_assign!( + execute_timings + .execute_accessories + .process_instructions + .total_us, + process_instruction_us + ); result .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?;