Transfer Hook: Off-chain and on-chain helpers are resolving keys incorrectly

[buffalojoec]

Problem

Token2022's off-chain and on-chain helpers for adding the necessary extra account metas for a transfer instruction that requires a transfer hook are missing a crucial step.

Note: This crucial step is missing only from the helpers, thus the on-chain program itself needs no change.

At the bottom of this issue is a test. In it, we're going to build instructions with extra account metas in three ways:

• Adding extra metas to a TransferChecked instruction using the Token2022 off-chain helper
• Adding extra metas to an Execute instruction using the TLV Account Resolution library's add_to_instruction
• Adding extra metas to an Execute CPI instruction using the TLV Account Resolution library's add_to_cpi_instruction

As you can probably infer, the issue is that we are adding the extra metas to a transfer instruction, rather than an Execute instruction.

Consider the instruction keys for TransferChecked (simplified):

0. [w] Source
1. [] Mint
2. [w] Destination
3. [s] Owner

Now consider the instruction keys as defined in the SPL Transfer Hook Interface's Execute instruction:

0. [] Source
1. [] Mint
2. [] Destination
3. [] Owner
4. [] Validation account

As you can see, Execute requires the validation (extra metas) account, while TransferChecked does not.

When extra account metas are hard-coded addresses, ie. ExtraAccountMeta::new_with_pubkey(..), this is not an issue. However, once any extra account meta uses seed configurations that point to other account keys in the list, this difference in account keys bungles the whole resolution.

The problem here is that, at the time of account resolution, the list of account keys for a transfer instruction and the list of account keys for an Execute instruction will yield entirely different resolutions.

For example, if I have a PDA that's based off of the account at index 4, that will be two different accounts depending on the instruction. No bueno.

Consider the test.

const TRANSFER_HOOK_PROGRAM_ID: Pubkey = Pubkey::new_from_array([1; 32]);

const MINT_PUBKEY: Pubkey = Pubkey::new_from_array([2; 32]);

const MOCK_MINT_STATE: [u8; 234] = [
    0, 0, 0, 0, // COption (4): None = 0
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, // Mint authority (32)
    0, 0, 0, 0, 0, 0, 0, 0, // Supply (8)
    0, // Decimals (1)
    1, // Is initialized (1)
    0, 0, 0, 0, // COption (4): None = 0
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, // Freeze authority (32)
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Padding (83)
    1, // Account type (1): Mint = 1
    14, 0, // Extension type (2): Transfer hook = 14
    64, 0, // Extension length (2): 64
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, // Authority (32)
    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
    1, 1, // Transfer hook program ID (32)
];

const MOCK_EXTRA_METAS_STATE: [u8; 86] = [
    105, 37, 101, 197, 75, 251, 102, 26, // Discriminator for `ExecuteInstruction` (8)
    74, 0, 0, 0, // Length of pod slice (4): 74
    2, 0, 0, 0, // Count of account metas (4): 2
    1, // First account meta discriminator (1): PDA = 1
    3, 0, // First seed: Account key at index 0 (2)
    3, 1, // Second seed: Account key at index 1 (2)
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, // No more seeds (28)
    0, // First account meta is signer (1): false = 0
    0, // First account meta is writable (1): false = 0
    1, // Second account meta discriminator (1): PDA = 1
    3, 4, // First seed: Account key at index 4 (2)
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, // No more seeds (30)
    0, // Second account meta is signer (1): false = 0
    0, // Second account meta is writable (1): false = 0
];

async fn mock_fetch_account_data_fn(address: Pubkey) -> AccountDataResult {
    if address == MINT_PUBKEY {
        Ok(Some(MOCK_MINT_STATE.to_vec()))
    } else if address
        == get_extra_account_metas_address(&MINT_PUBKEY, &TRANSFER_HOOK_PROGRAM_ID)
    {
        Ok(Some(MOCK_EXTRA_METAS_STATE.to_vec()))
    } else {
        Ok(None)
    }
}

#[tokio::test]
async fn test_resolve_extra_transfer_account_metas() {
    let spl_token_2022_program_id = crate::id();
    let transfer_hook_program_id = TRANSFER_HOOK_PROGRAM_ID;

    let source_pubkey = Pubkey::new_unique();
    let mut source_data = vec![0; 165]; // Mock
    let mut source_lamports = 0; // Mock
    let source_account_info = AccountInfo::new(
        &source_pubkey,
        false,
        true,
        &mut source_lamports,
        &mut source_data,
        &spl_token_2022_program_id,
        false,
        0,
    );

    let mint_pubkey = MINT_PUBKEY;
    let mut mint_data = MOCK_MINT_STATE.to_vec();
    let mut mint_lamports = 0; // Mock
    let mint_account_info = AccountInfo::new(
        &mint_pubkey,
        false,
        true,
        &mut mint_lamports,
        &mut mint_data,
        &spl_token_2022_program_id,
        false,
        0,
    );

    let destination_pubkey = Pubkey::new_unique();
    let mut destination_data = vec![0; 165]; // Mock
    let mut destination_lamports = 0; // Mock
    let destination_account_info = AccountInfo::new(
        &destination_pubkey,
        false,
        true,
        &mut destination_lamports,
        &mut destination_data,
        &spl_token_2022_program_id,
        false,
        0,
    );

    let authority_pubkey = Pubkey::new_unique();
    let mut authority_data = vec![]; // Mock
    let mut authority_lamports = 0; // Mock
    let authority_account_info = AccountInfo::new(
        &authority_pubkey,
        false,
        true,
        &mut authority_lamports,
        &mut authority_data,
        &system_program::ID,
        false,
        0,
    );

    let validate_state_pubkey = get_extra_account_metas_address(&mint_pubkey, &transfer_hook_program_id);

    let extra_meta_1_pubkey = Pubkey::find_program_address(
        &[
            &source_pubkey.to_bytes(), // Account key at index 0
            &mint_pubkey.to_bytes(),   // Account key at index 1
        ],
        &transfer_hook_program_id,
    )
    .0;
    let mut extra_meta_1_data = vec![]; // Mock
    let mut extra_meta_1_lamports = 0; // Mock
    let extra_meta_1_account_info = AccountInfo::new(
        &extra_meta_1_pubkey,
        false,
        true,
        &mut extra_meta_1_lamports,
        &mut extra_meta_1_data,
        &transfer_hook_program_id,
        false,
        0,
    );

    let extra_meta_2_pubkey = Pubkey::find_program_address(
        &[
            &validate_state_pubkey.to_bytes(), // Account key at index 4
        ],
        &transfer_hook_program_id,
    )
    .0;
    let mut extra_meta_2_data = vec![]; // Mock
    let mut extra_meta_2_lamports = 0; // Mock
    let extra_meta_2_account_info = AccountInfo::new(
        &extra_meta_2_pubkey,
        false,
        true,
        &mut extra_meta_2_lamports,
        &mut extra_meta_2_data,
        &transfer_hook_program_id,
        false,
        0,
    );

    let mut validate_state_data = MOCK_EXTRA_METAS_STATE.to_vec();
    let mut validate_state_lamports = 0; // Mock
    let validate_state_account_info = AccountInfo::new(
        &validate_state_pubkey,
        false,
        true,
        &mut validate_state_lamports,
        &mut validate_state_data,
        &transfer_hook_program_id,
        false,
        0,
    );

    // First use the resolve function to add the extra account metas to the
    // transfer instruction from offchain
    let mut offchain_transfer_instruction = crate::instruction::transfer_checked(
        &spl_token_2022_program_id,
        &source_pubkey,
        &mint_pubkey,
        &destination_pubkey,
        &authority_pubkey,
        &[],
        2,
        9,
    )
    .unwrap();

    resolve_extra_transfer_account_metas(
        &mut offchain_transfer_instruction,
        mock_fetch_account_data_fn,
        &mint_pubkey,
    )
    .await
    .unwrap();

    // Then use the offchain function to add the extra account metas to the
    // _execute_ instruction from offchain
    let mut offchain_execute_instruction = spl_transfer_hook_interface::instruction::execute(
        &transfer_hook_program_id,
        &source_pubkey,
        &mint_pubkey,
        &destination_pubkey,
        &authority_pubkey,
        &validate_state_pubkey,
        0,
    );

    ExtraAccountMetaList::add_to_instruction::<ExecuteInstruction, _, _>(
        &mut offchain_execute_instruction,
        mock_fetch_account_data_fn,
        &MOCK_EXTRA_METAS_STATE,
    )
    .await
    .unwrap();

    // Finally, use the onchain function to add the extra account metas to
    // the _execute_ CPI instruction from onchain
    let mut onchain_execute_cpi_instruction = spl_transfer_hook_interface::instruction::execute(
        &transfer_hook_program_id,
        &source_pubkey,
        &mint_pubkey,
        &destination_pubkey,
        &authority_pubkey,
        &validate_state_pubkey,
        0,
    );
    let mut onchain_execute_cpi_account_infos = vec![
        source_account_info.clone(),
        mint_account_info.clone(),
        destination_account_info.clone(),
        authority_account_info.clone(),
        validate_state_account_info.clone(),
    ];
    let all_account_infos = &[
        source_account_info.clone(),
        mint_account_info.clone(),
        destination_account_info.clone(),
        authority_account_info.clone(),
        validate_state_account_info.clone(),
        extra_meta_1_account_info.clone(),
        extra_meta_2_account_info.clone(),
    ];

    ExtraAccountMetaList::add_to_cpi_instruction::<ExecuteInstruction>(
        &mut onchain_execute_cpi_instruction,
        &mut onchain_execute_cpi_account_infos,
        &MOCK_EXTRA_METAS_STATE,
        all_account_infos,
    )
    .unwrap();

    // Now let's evaluate the different lists of keys

    // The two `Execute` instructions should have the same accounts
    assert_eq!(
        offchain_execute_instruction.accounts,
        onchain_execute_cpi_instruction.accounts,
    );

    // However, the transfer instruction is going to be missing the
    // the validation account at index 4
    assert_ne!(
        offchain_transfer_instruction.accounts,
        offchain_execute_instruction.accounts,
    );
    assert_ne!(
        offchain_transfer_instruction.accounts[4].pubkey,
        validate_state_pubkey,
    );

    // Even though both execute instructions have the validation account
    // at index 4
    assert_eq!(
        offchain_execute_instruction.accounts[4].pubkey,
        validate_state_pubkey,
    );
    assert_eq!(
        onchain_execute_cpi_instruction.accounts[4].pubkey,
        validate_state_pubkey,
    );

    // Despite the differences in indices, we're also going to have entirely
    // different PDAs in each list, and this is the issue
    assert_ne!(
        offchain_transfer_instruction.accounts[4].pubkey,
        extra_meta_1_pubkey,
    );
    assert_eq!(
        offchain_execute_instruction.accounts[5].pubkey,
        extra_meta_1_pubkey,
    );
    assert_eq!(
        onchain_execute_cpi_instruction.accounts[5].pubkey,
        extra_meta_1_pubkey,
    );
    assert_ne!(
        offchain_transfer_instruction.accounts[5].pubkey,
        extra_meta_2_pubkey,
    );
    assert_eq!(
        offchain_execute_instruction.accounts[6].pubkey,
        extra_meta_2_pubkey,
    );
    assert_eq!(
        onchain_execute_cpi_instruction.accounts[6].pubkey,
        extra_meta_2_pubkey,
    );

    // For even more verbosity
    assert!(offchain_transfer_instruction
        .accounts
        .iter()
        .find(|account_meta| account_meta.pubkey == extra_meta_1_pubkey)
        .is_none());
    assert!(offchain_transfer_instruction
        .accounts
        .iter()
        .find(|account_meta| account_meta.pubkey == extra_meta_2_pubkey)
        .is_none());
}

Proposed Solution

We should be able to insert a few lines of code into the helpers that simply converts a transfer instruction into an Execute instruction before resolving the extra account metas.

Since anyone storing extra account meta configs should be considering the list of accounts as it will arrive in their Transfer Hook program's Execute instruction, it makes sense to resolve for Execute.

After all, the main problem we have here is that we're asking TLV Account Resolution to resolve the extra account metas for an Execute instruction, but we're not actually feeding it a valid Execute instruction.

[jdubpark]

Is there a timeline for publishing v0.3.12? Currently facing 0x7dc8348c (IncorrectAccount) error on using addExtraAccountsToInstruction for the transfer hook. cc @buffalojoec

[buffalojoec]

Is there a timeline for publishing v0.3.12? Currently facing 0x7dc8348c (IncorrectAccount) error on using addExtraAccountsToInstruction for the transfer hook. cc @buffalojoec

https://github.com/solana-labs/solana-program-library/releases/tag/token-js-v0.4.0

[jdubpark]

Love it sir 😎🫡 my tx is still failing with 0x7dc8348c but at least the new package is out :)

Time to go debug more...