sdk: refactor: clean up entrypoint

[andreisilviudragnea]

Problem

• entrypoint! macro unnecessarily creates references for program_id and instruction_data, even if they are already references.

Summary of Changes

• I removed redundant references from entrypoint! macro.

[joncinque]

Thanks for the contribution! Can you separate the PR into two? One for the program-test changes, and another for the entrypoint change?

[joncinque]

The comment here is very important, I spent a couple of hours chasing this bug down a few years ago! This can't be removed

[andreisilviudragnea]

Does not the comment refer to the next line after the deleted line?

let instruction_context = transaction_context.get_current_instruction_context()?;

How can the CPI call invalidate the reference created by:

let transaction_context = &invoke_context.transaction_context;

[andreisilviudragnea]

I moved this change to #2694. I don't have strong feelings about this line, I am just curious to understand the bug, since it looks like also removing the line:

let instruction_context = transaction_context.get_current_instruction_context()?;

does not make any test from program-test crate fail.

pub fn invoke_builtin_function(
    builtin_function: solana_sdk::entrypoint::ProcessInstruction,
    invoke_context: &mut InvokeContext,
) -> Result<u64, Box<dyn std::error::Error>> {
    set_invoke_context(invoke_context);

The invoke_context: &mut InvokeContext reference which is used across the invoke_builtin_function should always be valid I think.

[joncinque]

If you CPI into a program that's also a fake builtin function, set_invoke_context will overwrite the previous invoke context, potentially invalidating your reference. It's a classic problem of doing unsafe code incorrectly 😅

[joncinque]

Thanks for the contribution!