Make instruction data opaque to runtime (solana-labs#6470)

programs/bpf/tests/programs.rs

@@ -3,9 +3,12 @@ mod bpf {
     use solana_runtime::bank::Bank;
     use solana_runtime::bank_client::BankClient;
     use solana_runtime::genesis_utils::{create_genesis_block, GenesisBlockInfo};
-    use solana_runtime::loader_utils::load_program;
+    use solana_runtime::loader_utils::{load_program, run_program};
+    use solana_sdk::bpf_loader;
+    use solana_sdk::instruction::AccountMeta;
     use std::env;
     use std::fs::File;
+    use std::io::Read;
     use std::path::PathBuf;
 
     /// BPF program file extension
@@ -26,11 +29,7 @@ mod bpf {
     #[cfg(feature = "bpf_c")]
     mod bpf_c {
         use super::*;
-        use solana_runtime::loader_utils::create_invoke_instruction;
-        use solana_sdk::bpf_loader;
-        use solana_sdk::client::SyncClient;
         use solana_sdk::signature::KeypairUtil;
-        use std::io::Read;
 
         #[test]
         fn test_program_bpf_c() {
@@ -62,9 +61,14 @@ mod bpf {
 
                 // Call user program
                 let program_id = load_program(&bank_client, &mint_keypair, &bpf_loader::id(), elf);
-                let instruction =
-                    create_invoke_instruction(mint_keypair.pubkey(), program_id, &1u8);
-                let result = bank_client.send_instruction(&mint_keypair, instruction);
+                let account_metas = vec![AccountMeta::new(mint_keypair.pubkey(), true)];
+                let result = run_program(
+                    &bank_client,
+                    &mint_keypair,
+                    &program_id,
+                    account_metas,
+                    &1u8,
+                );
                 if program.1 {
                     assert!(result.is_ok());
                 } else {
@@ -77,14 +81,10 @@ mod bpf {
     #[cfg(feature = "bpf_rust")]
     mod bpf_rust {
         use super::*;
-        use solana_sdk::bpf_loader;
-        use solana_sdk::client::SyncClient;
         use solana_sdk::clock::DEFAULT_SLOTS_PER_EPOCH;
-        use solana_sdk::instruction::{AccountMeta, Instruction};
         use solana_sdk::pubkey::Pubkey;
         use solana_sdk::signature::{Keypair, KeypairUtil};
         use solana_sdk::sysvar::{clock, fees, rent, rewards, slot_hashes, stake_history};
-        use std::io::Read;
         use std::sync::Arc;
 
         #[test]
@@ -133,8 +133,13 @@ mod bpf {
                     AccountMeta::new(stake_history::id(), false),
                     AccountMeta::new(rent::id(), false),
                 ];
-                let instruction = Instruction::new(program_id, &1u8, account_metas);
-                let result = bank_client.send_instruction(&mint_keypair, instruction);
+                let result = run_program(
+                    &bank_client,
+                    &mint_keypair,
+                    &program_id,
+                    account_metas,
+                    &1u8,
+                );
                 if program.1 {
                     assert!(result.is_ok());
                 } else {

programs/bpf_loader_api/src/lib.rs

@@ -167,8 +167,12 @@ pub fn process_instruction(
             }
         }
     } else {
-        warn!("Invalid instruction data: {:?}", ix_data);
-        return Err(InstructionError::GenericError);
+        warn!(
+            "Invalid instruction data ({:?}): {:?}",
+            ix_data.len(),
+            ix_data
+        );
+        return Err(InstructionError::InvalidInstructionData);
     }
     Ok(())
 }

programs/failure_program/tests/failure.rs

@@ -1,8 +1,8 @@
 use solana_runtime::bank::Bank;
 use solana_runtime::bank_client::BankClient;
-use solana_runtime::loader_utils::create_invoke_instruction;
-use solana_sdk::client::SyncClient;
+use solana_runtime::loader_utils::run_program;
 use solana_sdk::genesis_block::create_genesis_block;
+use solana_sdk::instruction::AccountMeta;
 use solana_sdk::instruction::InstructionError;
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::signature::KeypairUtil;
@@ -14,15 +14,20 @@ fn test_program_native_failure() {
     let program_id = Pubkey::new_rand();
     let bank = Bank::new(&genesis_block);
     bank.register_native_instruction_processor("solana_failure_program", &program_id);
+    let bank_client = BankClient::new(bank);
 
     // Call user program
-    let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8);
-    let bank_client = BankClient::new(bank);
+    let account_metas = vec![AccountMeta::new(alice_keypair.pubkey(), true)];
     assert_eq!(
-        bank_client
-            .send_instruction(&alice_keypair, instruction)
-            .unwrap_err()
-            .unwrap(),
+        run_program(
+            &bank_client,
+            &alice_keypair,
+            &program_id,
+            account_metas,
+            &1u8,
+        )
+        .unwrap_err()
+        .unwrap(),
         TransactionError::InstructionError(0, InstructionError::GenericError)
     );
 }

runtime/benches/bank.rs

@@ -5,12 +5,13 @@ extern crate test;
 use log::*;
 use solana_runtime::bank::*;
 use solana_runtime::bank_client::BankClient;
-use solana_runtime::loader_utils::create_invoke_instruction;
 use solana_sdk::account::KeyedAccount;
 use solana_sdk::client::AsyncClient;
 use solana_sdk::client::SyncClient;
 use solana_sdk::genesis_block::create_genesis_block;
+use solana_sdk::instruction::AccountMeta;
 use solana_sdk::instruction::InstructionError;
+use solana_sdk::loader_instruction;
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::signature::{Keypair, KeypairUtil};
 use solana_sdk::transaction::Transaction;
@@ -52,7 +53,8 @@ pub fn create_builtin_transactions(
                 .transfer(10_000, &mint_keypair, &rando0.pubkey())
                 .expect(&format!("{}:{}", line!(), file!()));
 
-            let instruction = create_invoke_instruction(rando0.pubkey(), program_id, &1u8);
+            let account_metas = vec![AccountMeta::new(rando0.pubkey(), true)];
+            let instruction = loader_instruction::invoke_main(&program_id, &1u8, account_metas);
             let (blockhash, _fee_calculator) = bank_client.get_recent_blockhash().unwrap();
             Transaction::new_signed_instructions(&[&rando0], vec![instruction], blockhash)
         })
@@ -74,7 +76,8 @@ pub fn create_native_loader_transactions(
                 .transfer(10_000, &mint_keypair, &rando0.pubkey())
                 .expect(&format!("{}:{}", line!(), file!()));
 
-            let instruction = create_invoke_instruction(rando0.pubkey(), program_id, &1u8);
+            let account_metas = vec![AccountMeta::new(rando0.pubkey(), true)];
+            let instruction = loader_instruction::invoke_main(&program_id, &1u8, account_metas);
             let (blockhash, _fee_calculator) = bank_client.get_recent_blockhash().unwrap();
             Transaction::new_signed_instructions(&[&rando0], vec![instruction], blockhash)
         })

runtime/src/append_vec.rs

@@ -13,8 +13,8 @@ use std::{
     sync::Mutex,
 };
 
-//Data is aligned at the next 64 byte offset. Without alignment loading the memory may
-//crash on some architectures.
+// Data is aligned at the next 64 byte offset. Without alignment loading the memory may
+// crash on some architectures.
 macro_rules! align_up {
     ($addr: expr, $align: expr) => {
         ($addr + ($align - 1)) & !($align - 1)

runtime/src/loader_utils.rs

@@ -1,11 +1,12 @@
 use serde::Serialize;
 use solana_sdk::client::Client;
-use solana_sdk::instruction::{AccountMeta, Instruction};
+use solana_sdk::instruction::AccountMeta;
 use solana_sdk::loader_instruction;
 use solana_sdk::message::Message;
 use solana_sdk::pubkey::Pubkey;
-use solana_sdk::signature::{Keypair, KeypairUtil};
+use solana_sdk::signature::{Keypair, KeypairUtil, Signature};
 use solana_sdk::system_instruction;
+use solana_sdk::transport::Result;
 
 pub fn load_program<T: Client>(
     bank_client: &T,
@@ -27,7 +28,7 @@ pub fn load_program<T: Client>(
         .send_instruction(&from_keypair, instruction)
         .unwrap();
 
-    let chunk_size = 256; // Size of chunk just needs to fit into tx
+    let chunk_size = 256; // Size of the chunk needs to fit into the transaction
     let mut offset = 0;
     for chunk in program.chunks(chunk_size) {
         let instruction =
@@ -48,13 +49,13 @@ pub fn load_program<T: Client>(
     program_pubkey
 }
 
-// Return an Instruction that invokes `program_id` with `data` and required
-// a signature from `from_pubkey`.
-pub fn create_invoke_instruction<T: Serialize>(
-    from_pubkey: Pubkey,
-    program_id: Pubkey,
-    data: &T,
-) -> Instruction {
-    let account_metas = vec![AccountMeta::new(from_pubkey, true)];
-    Instruction::new(program_id, data, account_metas)
+pub fn run_program<T: Client, D: Serialize>(
+    bank_client: &T,
+    from_keypair: &Keypair,
+    loader_pubkey: &Pubkey,
+    account_metas: Vec<AccountMeta>,
+    data: &D,
+) -> Result<Signature> {
+    let instruction = loader_instruction::invoke_main(loader_pubkey, data, account_metas);
+    bank_client.send_instruction(from_keypair, instruction)
 }

runtime/src/message_processor.rs

@@ -6,13 +6,11 @@ use solana_sdk::account::{
 };
 use solana_sdk::instruction::{CompiledInstruction, InstructionError};
 use solana_sdk::instruction_processor_utils;
-use solana_sdk::loader_instruction::LoaderInstruction;
 use solana_sdk::message::Message;
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::system_program;
 use solana_sdk::transaction::TransactionError;
 use std::collections::HashMap;
-use std::io::Write;
 use std::sync::RwLock;
 
 #[cfg(unix)]
@@ -137,27 +135,6 @@ fn verify_instruction(
     Ok(())
 }
 
-/// Return instruction data to pass to process_instruction().
-/// When a loader is detected, the instruction data is wrapped with a LoaderInstruction
-/// to signal to the loader that the instruction data should be used as arguments when
-/// invoking a "main()" function.
-fn get_loader_instruction_data<'a>(
-    loaders: &[(Pubkey, Account)],
-    ix_data: &'a [u8],
-    loader_ix_data: &'a mut Vec<u8>,
-) -> &'a [u8] {
-    if loaders.len() > 1 {
-        let ix = LoaderInstruction::InvokeMain {
-            data: ix_data.to_vec(),
-        };
-        let ix_data = bincode::serialize(&ix).unwrap();
-        loader_ix_data.write_all(&ix_data).unwrap();
-        loader_ix_data
-    } else {
-        ix_data
-    }
-}
-
 pub type ProcessInstruction =
     fn(&Pubkey, &mut [KeyedAccount], &[u8]) -> Result<(), InstructionError>;
 
@@ -206,14 +183,6 @@ impl MessageProcessor {
         program_accounts: &mut [&mut Account],
     ) -> Result<(), InstructionError> {
         let program_id = instruction.program_id(&message.account_keys);
-
-        let mut loader_ix_data = vec![];
-        let ix_data = get_loader_instruction_data(
-            executable_accounts,
-            &instruction.data,
-            &mut loader_ix_data,
-        );
-
         let mut keyed_accounts = create_keyed_credit_only_accounts(executable_accounts);
         let mut keyed_accounts2: Vec<_> = instruction
             .accounts
@@ -247,14 +216,18 @@ impl MessageProcessor {
         let loader_id = keyed_accounts[0].unsigned_key();
         for (id, process_instruction) in &self.instruction_processors {
             if id == loader_id {
-                return process_instruction(&program_id, &mut keyed_accounts[1..], &ix_data);
+                return process_instruction(
+                    &program_id,
+                    &mut keyed_accounts[1..],
+                    &instruction.data,
+                );
             }
         }
 
         native_loader::invoke_entrypoint(
             &program_id,
             &mut keyed_accounts,
-            ix_data,
+            &instruction.data,
             &self.symbol_cache,
         )
     }
@@ -809,37 +782,4 @@ mod tests {
             ))
         );
     }
-
-    #[test]
-    fn test_get_loader_instruction_data() {
-        // First ensure the ix_data is unaffected if not invoking via a loader.
-        let ix_data = [1];
-        let mut loader_ix_data = vec![];
-
-        let native_pubkey = Pubkey::new_rand();
-        let native_loader = (native_pubkey, Account::new(0, 0, &native_pubkey));
-        assert_eq!(
-            get_loader_instruction_data(&[native_loader.clone()], &ix_data, &mut loader_ix_data),
-            &ix_data
-        );
-
-        // Now ensure the ix_data is wrapped when there's a loader present.
-        let acme_pubkey = Pubkey::new_rand();
-        let acme_loader = (acme_pubkey, Account::new(0, 0, &native_pubkey));
-        let expected_ix = LoaderInstruction::InvokeMain {
-            data: ix_data.to_vec(),
-        };
-        let expected_ix_data = bincode::serialize(&expected_ix).unwrap();
-        assert_eq!(
-            get_loader_instruction_data(
-                &[native_loader.clone(), acme_loader.clone()],
-                &ix_data,
-                &mut loader_ix_data
-            ),
-            &expected_ix_data[..]
-        );
-
-        // Note there was an allocation in the input vector.
-        assert_eq!(loader_ix_data, expected_ix_data);
-    }
 }

runtime/tests/noop.rs

@@ -1,8 +1,8 @@
 use solana_runtime::bank::Bank;
 use solana_runtime::bank_client::BankClient;
-use solana_runtime::loader_utils::create_invoke_instruction;
-use solana_sdk::client::SyncClient;
+use solana_runtime::loader_utils::run_program;
 use solana_sdk::genesis_block::create_genesis_block;
+use solana_sdk::instruction::AccountMeta;
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::signature::KeypairUtil;
 
@@ -14,11 +14,16 @@ fn test_program_native_noop() {
     let program_id = Pubkey::new_rand();
     let bank = Bank::new(&genesis_block);
     bank.register_native_instruction_processor("solana_noop_program", &program_id);
+    let bank_client = BankClient::new(bank);
 
     // Call user program
-    let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8);
-    let bank_client = BankClient::new(bank);
-    bank_client
-        .send_instruction(&alice_keypair, instruction)
-        .unwrap();
+    let account_metas = vec![AccountMeta::new(alice_keypair.pubkey(), true)];
+    run_program(
+        &bank_client,
+        &alice_keypair,
+        &program_id,
+        account_metas,
+        &1u8,
+    )
+    .unwrap();
 }