feat: move precompile verification to SVM

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<FeatureSet>,
 ) -> 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(())
 }
 

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();

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<Item = &'ix_data [u8]>,
+        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,

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);
 }

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(())

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)?;
         }
 

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 }

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<Pubkey, &'static str> = [
@@ -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()

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<T> DecodeError<T> 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<F>(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
 }

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))?;