tests: Use mollusk on process instruction

[febo]

Problem

PR #17 moved processors tests to the tests folder but instructions are executed directly against the program Processor.

Solution

Reimplement both do_process_instruction and do_process_instruction_dups to use mollusk to execute instructions.

While the intention was not to introduce any changes to existing tests, there were a couple of changes required:

• Few tests that were using the Processor directly were updated to use do_process_instruction_dups
• assert after the account is closed tests if the data is empty ontest_close_account

[joncinque]

Just tiny things, this looks really good!

I had to look at only the second commit because for some reason the first commit still has everything from #17

[joncinque]

Totally separate, but I just realized, with this change, we can remove the solana-program dependency from spl-token and make it much easier for downstream users

[joncinque]

nit: can the second be an into_iter() to avoid the clone in the next line?

[buffalojoec]

Mollusk will vend you back the exact same accounts in the same order, but with any post-processing changes. So you can just do something like this:

accounts = result.resulting_accounts.into_iter().unzip().1;

[febo]

But they will be a "copy" of the input, right? In this case we have Vec<&mut SolanaAccount>, so we need to update those references, not the whole Vec.

We are giving mollusk a copy of the input here:

let instruction_accounts: Vec<(Pubkey, AccountSharedData)> = instruction
        .accounts
        .iter()
        .zip(&accounts)
        .map(|(account_meta, account)| {
            (
                account_meta.pubkey,
                AccountSharedData::from((*account).clone()),
            )
        })
        .collect();

[buffalojoec]

Yes, they're a copy of the input but with any changes from the instruction(s) applied. So basically the same as what you are doing already.

[febo]

This loop is updating the original references:

for (original, (_, updated)) in accounts
        .iter_mut()
        .zip(result.resulting_accounts.into_iter())
{
    original.data = updated.data().to_vec();
    original.lamports = updated.lamports();
    original.owner = *updated.owner();
}

I think I need to keep this loop, since those references are used in the caller test code.

[buffalojoec]

Looking good!

This is a completely separate note, but going through this and thinking about p-token, I just realized you can write a pretty nifty benchmark that:

1. Clones down SPL-Token from mainnet-beta
2. Runs all fixtures on both programs using process_fixture (no validation)
3. Collects each program's compute unit usage from the results.
4. Compares them.

[buffalojoec]

Hm, interesting. This is required since if the user provides duplicate transaction accounts, the TransactionContext doesn't pilfer the duplicates out? I wonder if there's an elegant way to handle this within the Mollusk harness somehow.

Works for me if not, though.

[febo]

The main reason for it is that the function takes an AccountInfo and Mollusk expects an AccountSharedData for each account. Then during this conversion, we only convert one "copy" of each account.

[febo]

Looking good!

This is a completely separate note, but going through this and thinking about p-token, I just realized you can write a pretty nifty benchmark that:

1. Clones down SPL-Token from mainnet-beta
2. Runs all fixtures on both programs using process_fixture (no validation)
3. Collects each program's compute unit usage from the results.
4. Compares them.

Oooh, that is nice! We can do that once we get everything merged.

[febo]

I had to look at only the second commit because for some reason the first commit still has everything from #17

I think I was missing a rebase - now it looks fine.

[joncinque]

Looks good to me!

[buffalojoec]

I think we want this in here still, right?

[febo]

Not sure, if we don't restrict the branches, we can have CI run in stacked PRs.