Refactor: Remove program_id from process_instruction() (#20540)

* Replaces usage of program_id parameter by invoke_context.get_caller()?. * Removes "pubkey: &Pubkey" parameter from "process_instruction()".
2021-10-10 22:29:18 +02:00
parent c16510152e
commit f30f3bddbb
23 changed files with 110 additions and 175 deletions
--- a/runtime/benches/bank.rs
+++ b/runtime/benches/bank.rs
@ -32,7 +32,6 @@ const NOOP_PROGRAM_ID: [u8; 32] = [

 #[allow(clippy::unnecessary_wraps)]
 fn process_instruction(
-    _program_id: &Pubkey,
    _first_instruction_account: usize,
    _data: &[u8],
    _invoke_context: &mut dyn InvokeContext,
--- a/runtime/src/bank.rs
+++ b/runtime/src/bank.rs
@ -6685,7 +6685,6 @@ pub(crate) mod tests {
    }

    fn mock_process_instruction(
-        _program_id: &Pubkey,
        first_instruction_account: usize,
        data: &[u8],
        invoke_context: &mut dyn InvokeContext,
@ -10333,11 +10332,11 @@ pub(crate) mod tests {
            Pubkey::new(&[42u8; 32])
        }
        fn mock_vote_processor(
-            program_id: &Pubkey,
            _first_instruction_account: usize,
            _instruction_data: &[u8],
-            _invoke_context: &mut dyn InvokeContext,
+            invoke_context: &mut dyn InvokeContext,
        ) -> std::result::Result<(), InstructionError> {
+            let program_id = invoke_context.get_caller()?;
            if mock_vote_program_id() != *program_id {
                return Err(InstructionError::IncorrectProgramId);
            }
@ -10391,7 +10390,6 @@ pub(crate) mod tests {
        let mut bank = Bank::new_for_tests(&genesis_config);

        fn mock_vote_processor(
-            _pubkey: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
@ -10442,7 +10440,6 @@ pub(crate) mod tests {
        let mut bank = Bank::new_for_tests(&genesis_config);

        fn mock_ix_processor(
-            _pubkey: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
@ -11284,7 +11281,6 @@ pub(crate) mod tests {
        let mut bank = Bank::new_for_tests(&genesis_config);

        fn mock_process_instruction(
-            _program_id: &Pubkey,
            first_instruction_account: usize,
            data: &[u8],
            invoke_context: &mut dyn InvokeContext,
@ -11345,7 +11341,6 @@ pub(crate) mod tests {

        #[allow(clippy::unnecessary_wraps)]
        fn mock_process_instruction(
-            _program_id: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
@ -11532,7 +11527,6 @@ pub(crate) mod tests {

    #[allow(clippy::unnecessary_wraps)]
    fn mock_ok_vote_processor(
-        _pubkey: &Pubkey,
        _first_instruction_account: usize,
        _data: &[u8],
        _invoke_context: &mut dyn InvokeContext,
@ -11783,7 +11777,6 @@ pub(crate) mod tests {
    #[test]
    fn test_same_program_id_uses_unqiue_executable_accounts() {
        fn nested_processor(
-            _program_id: &Pubkey,
            first_instruction_account: usize,
            _data: &[u8],
            invoke_context: &mut dyn InvokeContext,
@ -12155,7 +12148,6 @@ pub(crate) mod tests {

        #[allow(clippy::unnecessary_wraps)]
        fn mock_ix_processor(
-            _pubkey: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
@ -12205,7 +12197,6 @@ pub(crate) mod tests {

        #[allow(clippy::unnecessary_wraps)]
        fn mock_ix_processor(
-            _pubkey: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _context: &mut dyn InvokeContext,
@ -12593,7 +12584,6 @@ pub(crate) mod tests {
    impl Executor for TestExecutor {
        fn execute(
            &self,
-            _program_id: &Pubkey,
            _first_instruction_account: usize,
            _instruction_data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
@ -13097,7 +13087,6 @@ pub(crate) mod tests {
        // intentionally create bogus native programs
        #[allow(clippy::unnecessary_wraps)]
        fn mock_process_instruction(
-            _program_id: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
@ -14595,7 +14584,6 @@ pub(crate) mod tests {
        let mut bank = Bank::new_for_tests(&genesis_config);

        fn mock_ix_processor(
-            _pubkey: &Pubkey,
            first_instruction_account: usize,
            _data: &[u8],
            invoke_context: &mut dyn InvokeContext,
@ -14805,7 +14793,6 @@ pub(crate) mod tests {
        let mut bank = Bank::new_for_tests(&genesis_config);

        fn mock_ix_processor(
-            _pubkey: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            invoke_context: &mut dyn InvokeContext,
--- a/runtime/src/builtins.rs
+++ b/runtime/src/builtins.rs
@ -13,7 +13,6 @@ use solana_frozen_abi::abi_example::AbiExample;

 fn process_instruction_with_program_logging(
    process_instruction: ProcessInstructionWithContext,
-    program_id: &Pubkey,
    first_instruction_account: usize,
    instruction_data: &[u8],
    invoke_context: &mut dyn InvokeContext,
@ -21,15 +20,12 @@ fn process_instruction_with_program_logging(
    debug_assert_eq!(first_instruction_account, 1);

    let logger = invoke_context.get_logger();
+    let program_id = invoke_context.get_caller()?;
    stable_log::program_invoke(&logger, program_id, invoke_context.invoke_depth());

-    let result = process_instruction(
-        program_id,
-        first_instruction_account,
-        instruction_data,
-        invoke_context,
-    );
+    let result = process_instruction(first_instruction_account, instruction_data, invoke_context);

+    let program_id = invoke_context.get_caller()?;
    match &result {
        Ok(()) => stable_log::program_success(&logger, program_id),
        Err(err) => stable_log::program_failure(&logger, program_id, err),
@ -39,13 +35,11 @@ fn process_instruction_with_program_logging(

 macro_rules! with_program_logging {
    ($process_instruction:expr) => {
-        |program_id: &Pubkey,
-         first_instruction_account: usize,
+        |first_instruction_account: usize,
         instruction_data: &[u8],
         invoke_context: &mut dyn InvokeContext| {
            process_instruction_with_program_logging(
                $process_instruction,
-                program_id,
                first_instruction_account,
                instruction_data,
                invoke_context,
@ -94,7 +88,7 @@ impl AbiExample for Builtin {
        Self {
            name: String::default(),
            id: Pubkey::default(),
-            process_instruction_with_context: |_, _, _, _| Ok(()),
+            process_instruction_with_context: |_, _, _| Ok(()),
        }
    }
 }
@ -141,7 +135,6 @@ fn genesis_builtins() -> Vec<Builtin> {

 /// place holder for secp256k1, remove when the precompile program is deactivated via feature activation
 fn dummy_process_instruction(
-    _program_id: &Pubkey,
    _first_instruction_account: usize,
    _data: &[u8],
    _invoke_context: &mut dyn InvokeContext,
--- a/runtime/src/message_processor.rs
+++ b/runtime/src/message_processor.rs
@ -550,11 +550,8 @@ impl MessageProcessor {
            let result = invoke_context
                .push(program_id, message, instruction, program_indices, None)
                .and_then(|_| {
-                    instruction_processor.process_instruction(
-                        program_id,
-                        &instruction.data,
-                        &mut invoke_context,
-                    )?;
+                    instruction_processor
+                        .process_instruction(&instruction.data, &mut invoke_context)?;
                    invoke_context.verify(message, instruction, program_indices)?;
                    timings.accumulate(&invoke_context.timings);
                    Ok(())
@ -598,11 +595,11 @@ mod tests {
    }

    fn mock_process_instruction(
-        program_id: &Pubkey,
        first_instruction_account: usize,
        data: &[u8],
        invoke_context: &mut dyn InvokeContext,
    ) -> Result<(), InstructionError> {
+        let program_id = invoke_context.get_caller()?;
        let keyed_accounts = invoke_context.get_keyed_accounts()?;
        assert_eq!(
            *program_id,
@ -821,7 +818,6 @@ mod tests {
        }

        fn mock_system_process_instruction(
-            _program_id: &Pubkey,
            first_instruction_account: usize,
            data: &[u8],
            invoke_context: &mut dyn InvokeContext,
@ -995,7 +991,6 @@ mod tests {
        }

        fn mock_system_process_instruction(
-            _program_id: &Pubkey,
            first_instruction_account: usize,
            data: &[u8],
            invoke_context: &mut dyn InvokeContext,
@ -1523,7 +1518,6 @@ mod tests {
    fn test_precompile() {
        let mock_program_id = Pubkey::new_unique();
        fn mock_process_instruction(
-            _program_id: &Pubkey,
            _first_instruction_account: usize,
            _data: &[u8],
            _invoke_context: &mut dyn InvokeContext,
--- a/runtime/src/system_instruction_processor.rs
+++ b/runtime/src/system_instruction_processor.rs
@ -263,7 +263,6 @@ fn transfer_with_seed(
 }

 pub fn process_instruction(
-    _owner: &Pubkey,
    first_instruction_account: usize,
    instruction_data: &[u8],
    invoke_context: &mut dyn InvokeContext,
@ -522,13 +521,11 @@ mod tests {
            ..Account::default()
        }));
        let mut keyed_accounts = keyed_accounts.to_vec();
-        let processor_id = Pubkey::default();
-        keyed_accounts.insert(0, (false, false, &processor_id, &processor_account));
+        keyed_accounts.insert(0, (false, false, owner, &processor_account));
        super::process_instruction(
-            owner,
            1,
            instruction_data,
-            &mut MockInvokeContext::new(create_keyed_accounts_unified(&keyed_accounts)),
+            &mut MockInvokeContext::new(owner, create_keyed_accounts_unified(&keyed_accounts)),
        )
    }

@ -661,7 +658,7 @@ mod tests {
            Address::create(
                &to,
                Some((&from, seed, &owner)),
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Err(SystemError::AddressWithSeedMismatch.into())
        );
@ -679,7 +676,7 @@ mod tests {
        let to_address = Address::create(
            &to,
            Some((&from, seed, &new_owner)),
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        )
        .unwrap();

@ -692,7 +689,7 @@ mod tests {
                2,
                &new_owner,
                &HashSet::new(),
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Err(InstructionError::MissingRequiredSignature)
        );
@ -719,7 +716,7 @@ mod tests {
                2,
                &new_owner,
                &[from, to].iter().cloned().collect::<HashSet<_>>(),
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Ok(())
        );
@ -751,7 +748,7 @@ mod tests {
            2,
            &new_owner,
            &[from, to].iter().cloned().collect::<HashSet<_>>(),
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into()));
    }
@ -775,7 +772,7 @@ mod tests {
            MAX_PERMITTED_DATA_LENGTH + 1,
            &system_program::id(),
            signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert!(result.is_err());
        assert_eq!(
@ -792,7 +789,7 @@ mod tests {
            MAX_PERMITTED_DATA_LENGTH,
            &system_program::id(),
            signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert!(result.is_ok());
        assert_eq!(to_account.borrow().lamports(), 50);
@ -825,7 +822,7 @@ mod tests {
            2,
            &new_owner,
            signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));

@ -844,7 +841,7 @@ mod tests {
            2,
            &new_owner,
            signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
        let from_lamports = from_account.borrow().lamports();
@ -862,7 +859,7 @@ mod tests {
            2,
            &new_owner,
            signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
        assert_eq!(from_lamports, 100);
@ -890,7 +887,7 @@ mod tests {
            2,
            &new_owner,
            &[owned_key].iter().cloned().collect::<HashSet<_>>(),
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(InstructionError::MissingRequiredSignature));

@ -904,7 +901,7 @@ mod tests {
            2,
            &new_owner,
            &[from].iter().cloned().collect::<HashSet<_>>(),
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(InstructionError::MissingRequiredSignature));

@ -918,7 +915,7 @@ mod tests {
            2,
            &new_owner,
            &[owned_key].iter().cloned().collect::<HashSet<_>>(),
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
    }
@ -944,7 +941,7 @@ mod tests {
            2,
            &sysvar::id(),
            &signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Ok(()));
    }
@ -973,7 +970,7 @@ mod tests {
                disabled_features: vec![feature_set::rent_for_sysvars::id()]
                    .into_iter()
                    .collect(),
-                ..MockInvokeContext::new(vec![])
+                ..MockInvokeContext::new(&Pubkey::default(), vec![])
            },
        );
        assert_eq!(result, Err(SystemError::InvalidProgramId.into()));
@ -1007,7 +1004,7 @@ mod tests {
            2,
            &new_owner,
            &signers,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
    }
@ -1041,7 +1038,7 @@ mod tests {
                0,
                &solana_sdk::pubkey::new_rand(),
                &signers,
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Err(InstructionError::InvalidArgument),
        );
@ -1059,7 +1056,7 @@ mod tests {
                &pubkey.into(),
                &new_owner,
                &HashSet::new(),
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Err(InstructionError::MissingRequiredSignature)
        );
@ -1071,7 +1068,7 @@ mod tests {
                &pubkey.into(),
                &system_program::id(),
                &HashSet::new(),
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Ok(())
        );
@ -1100,7 +1097,7 @@ mod tests {
                &from.into(),
                &new_owner,
                &[from].iter().cloned().collect::<HashSet<_>>(),
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Ok(())
        );
@ -1123,7 +1120,7 @@ mod tests {
                    disabled_features: vec![feature_set::rent_for_sysvars::id()]
                        .into_iter()
                        .collect(),
-                    ..MockInvokeContext::new(vec![])
+                    ..MockInvokeContext::new(&Pubkey::default(), vec![])
                },
            ),
            Err(SystemError::InvalidProgramId.into())
@ -1165,7 +1162,7 @@ mod tests {
            &from_keyed_account,
            &to_keyed_account,
            50,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        )
        .unwrap();
        let from_lamports = from_keyed_account.account.borrow().lamports();
@ -1179,7 +1176,7 @@ mod tests {
            &from_keyed_account,
            &to_keyed_account,
            100,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into()));
        assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
@ -1190,7 +1187,7 @@ mod tests {
            &from_keyed_account,
            &to_keyed_account,
            0,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        )
        .is_ok());
        assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
@ -1204,7 +1201,7 @@ mod tests {
                &from_keyed_account,
                &to_keyed_account,
                0,
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Err(InstructionError::MissingRequiredSignature)
        );
@ -1232,7 +1229,7 @@ mod tests {
            &from_owner,
            &to_keyed_account,
            50,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        )
        .unwrap();
        let from_lamports = from_keyed_account.account.borrow().lamports();
@ -1249,7 +1246,7 @@ mod tests {
            &from_owner,
            &to_keyed_account,
            100,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        );
        assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into()));
        assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
@ -1264,7 +1261,7 @@ mod tests {
            &from_owner,
            &to_keyed_account,
            0,
-            &MockInvokeContext::new(vec![]),
+            &MockInvokeContext::new(&Pubkey::default(), vec![]),
        )
        .is_ok());
        assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
@ -1295,7 +1292,7 @@ mod tests {
                &KeyedAccount::new(&from, true, &from_account),
                &KeyedAccount::new(&to, false, &to_account),
                50,
-                &MockInvokeContext::new(vec![]),
+                &MockInvokeContext::new(&Pubkey::default(), vec![]),
            ),
            Err(InstructionError::InvalidArgument),
        )
@ -1610,12 +1607,13 @@ mod tests {
            (true, false, &owner, &nonce_acc),
            (false, false, &blockhash_id, &new_recent_blockhashes_account),
        ];
-        let mut invoke_context =
-            &mut MockInvokeContext::new(create_keyed_accounts_unified(&keyed_accounts));
+        let mut invoke_context = &mut MockInvokeContext::new(
+            &Pubkey::default(),
+            create_keyed_accounts_unified(&keyed_accounts),
+        );
        invoke_context.blockhash = *blockhash;
        assert_eq!(
            super::process_instruction(
-                &Pubkey::default(),
                1,
                &serialize(&SystemInstruction::AdvanceNonceAccount).unwrap(),
                invoke_context,
@ -2034,12 +2032,13 @@ mod tests {
            (true, false, &owner, &nonce_acc),
            (false, false, &blockhash_id, &new_recent_blockhashes_account),
        ];
-        let mut invoke_context =
-            &mut MockInvokeContext::new(create_keyed_accounts_unified(&keyed_accounts));
+        let mut invoke_context = &mut MockInvokeContext::new(
+            &Pubkey::default(),
+            create_keyed_accounts_unified(&keyed_accounts),
+        );
        invoke_context.blockhash = *blockhash;
        assert_eq!(
            super::process_instruction(
-                &Pubkey::default(),
                1,
                &serialize(&SystemInstruction::AdvanceNonceAccount).unwrap(),
                invoke_context,