[token-cli] Add support for transfer-hook account resolution for transfers with the transfer-fee extension.

[tonton-sol]

Per #7059

Adds a new offchain helper offchain::create_transfer_checked_with_fee_instruction_with_extra_metas.
Adds a new onchain helper onchain::invoke_transfer_checked_with_fee

Adds logic to transfer_with_fee in token/client/src/token.rs to add the extra-account-metas using the offchain helper if the token uses the transfer-hook extension.

[tonton-sol]

What tests if any needed to be added?

[buffalojoec]

Nice!! The changes look great to me.

For the tests, you'll want to test the helpers by adding a few cases to the Token-2022 tests under token/program-2022-test in the transfer_hook test module.

The idea is to basically make copies of success_transfer and success_transfers_using_onchain_helper, but add the transfer fee config to the mint. The former is going to test with the transfer hook example program, which should still work fine for this case, since it's really just going to test that the proper extra metas are loaded and passed to the example program in token/transfer-hook/example. The latter is using a mocked-up processor in the test module (see this line here).

Then maybe add a block in the Token CLI's transfer_hook tests to ensure we can parse a transfer instruction with a fee and all the extra transfer hook accounts as arguments.

If you have any questions, let me know, and thanks!

[tonton-sol]

@buffalojoec I am working in token/program-2022-test/transfer_hook.rs and I noticed that rust-analyser/cargo is not working in this directory. Is this a known thing?

[tonton-sol]

And actually, does it make sense to add the combination test here in token/program-2022-test? Shouldn't there then be tests for every arbitrary combination of extensions?

[buffalojoec]

@buffalojoec I am working in token/program-2022-test/transfer_hook.rs and I noticed that rust-analyser/cargo is not working in this directory. Is this a known thing?

Yeah, you need to add test-sbf to Rust Analyzer's default features. Even after doing that, I don't think it ever really worked for me. Sometimes I just comment out the #![cfg(feature = "test-sbf")] at the top, handle all of my compile errors, then add it back.

And actually, does it make sense to add the combination test here in token/program-2022-test? Shouldn't there then be tests for every arbitrary combination of extensions?

I didn't want to dump that whole workload on you. 😅

[tonton-sol]

Looking at the rest of the tests, I actually think its okay to have these mixed extension tests in token/program-2022-test/transfer_hook.rs because of the special treatment needed for the extra metas. So I will go ahead and add tests for the w/ fee conditions.

Also there are no tests here for spl_token_2022::offchain, only spl_transfer_hook_interface. Is this by design? Would it even make sense to test the higher level helper functions here?

[tonton-sol]

Then maybe add a block in the Token CLI's transfer_hook tests to ensure we can parse a transfer instruction with a fee and all the extra transfer hook accounts as arguments.

I am having trouble trying to add the arguments for the expected fee to the transfer test. I have confirmed that I can successfully create the token with both the transfer-hook and transfer-fee extensions in the test, so this is not a problem. Could this be caused by --sign-only and doing process_command() in offline mode?

I assume it's a catch-22 because we need the associated token account to be created meaning it cannot be done in offline mode. If it is online mode, the transfer-hook program would need to be deployed and initialized. Is this correct?

[buffalojoec]

Then maybe add a block in the Token CLI's transfer_hook tests to ensure we can parse a transfer instruction with a fee and all the extra transfer hook accounts as arguments.

I am having trouble trying to add the arguments for the expected fee to the transfer test. I have confirmed that I can successfully create the token with both the transfer-hook and transfer-fee extensions in the test, so this is not a problem. Could this be caused by --sign-only and doing process_command() in offline mode?

I assume it's a catch-22 because we need the associated token account to be created meaning it cannot be done in offline mode. If it is online mode, the transfer-hook program would need to be deployed and initialized. Is this correct?

Yeah that's correct. The test I linked is sending the command to create the token, then disable the transfer hook (set program to None), but the middle bit where made the permalink is doing --sign-only so that it doesn't actually invoke the client but it's testing the parsing of the arguments and validation against the mint's extensions.

Maybe duplicate that test and set the token up with a fee config, similar to this test here, and then add the fee argument to the parsing test block? That way you can also set up the token account required.

[tonton-sol]

Okay I figured it out! In sign-only + offline mode you need to explicitly pass the --program-id or else it will default to Tokenkeg. This is a problem for the transfer-fee extension but not the transfer-hook extension because transfer_checked() checks if it is either Tokenkeg or Token2022 while transfer_checked_with_fee() checks if it is Token2022. I guess this is just a weird quirk of testing this way.

[tonton-sol]

I think this is good to go!

[buffalojoec]

Nice! This is super close, just a nit and comment about the one test, then we can ship.

[buffalojoec]

Nice work!!

[tonton-sol]

Will this auto close and not be added? 😅

[buffalojoec]

Will this auto close and not be added? 😅

Whoops, sorry! I thought you could merge after I approved.

[tonton-sol]

I'd need to be employed to do that right? 😉