token-js: added extra account resolution for transfer hook extension

[wjthieme]

resolves #5108

Couple notes:

• Not sure exactly how multisigners is implemented for extra account resolution in the program since it changes the ordering of the account keys. Does it mean that if a user wants multisigners to work they explicitly need to code this into their transfer hook program?
• What is done by tlv-account-resolution in rust is now baked into @solana/spl-token for js. Might be a nice improvement to separate that into its own package?
• I have written a few tests to the best of my understanding of what exactly the data structure of the extra accounts data account is. Might be worth for @joncinque or @buffalojoec to check if my implementation actually matches that of the rust client.
• The way transferCheckedWithHook and addExtraAccountsToInstruction are structured is a little bit made up by me. Let me know if it matches your expectations @joncinque

[buffalojoec]

Nice work mate!

The Rust lib separates serialization/deserialization of the seed configs into the seeds module, since it has to be used to pack and unpack both on-chain and off-chain, but JavaScript doesn't need that! I appreciate your condensed version of the seed resolution, doing both deserialization and resolution in one step 🤌🏼

I left a handful of comments, but this is shaping up nicely!

[buffalojoec]

• Not sure exactly how multisigners is implemented for extra account resolution in the program since it changes the ordering of the account keys. Does it mean that if a user wants multisigners to work they explicitly need to code this into their transfer hook program?

Multisigs are evaluated at the Token-2022 level. The TransferChecked instruction expects - if you're working with multisig - the multisig account first and then the signers immediately preceding.

solana-program-library/token/program-2022/src/instruction.rs

Lines 319 to 330 in 737a37a

	/// * Single owner/delegate
	/// 0. `[writable]` The source account.
	/// 1. `[]` The token mint.
	/// 2. `[writable]` The destination account.
	/// 3. `[signer]` The source account's owner/delegate.
	///
	/// * Multisignature owner/delegate
	/// 0. `[writable]` The source account.
	/// 1. `[]` The token mint.
	/// 2. `[writable]` The destination account.
	/// 3. `[]` The source account's multisignature owner/delegate.
	/// 4. ..4+M `[signer]` M signer accounts.

You can see here we're reading the data of the authority account and then using validate_owner to evaluate the mutisigs from the remaining accounts.

solana-program-library/token/program-2022/src/processor.rs

Lines 396 to 402 in 737a37a

	Self::validate_owner(
	program_id,
	&source_account.base.owner,
	authority_info,
	authority_info_data_len,
	account_info_iter.as_slice(),
	)?;

Then your extra metas would come after!

• What is done by tlv-account-resolution in rust is now baked into @solana/spl-token for js. Might be a nice improvement to separate that into its own package?

We could separate it, but I mentioned in my comment above that it's actually not necessary at the moment for the JavaScript library to be able to handle (de)serialization of the seed configs and TLV state. However, the more I think about it, the more I think we will eventually want a JavaScript TLV library so we don't have to keep writing the same count and values structures with buffer layout. Perhaps just a split-out TLV JS library?

• I have written a few tests to the best of my understanding of what exactly the data structure of the extra accounts data account is. Might be worth for @joncinque or @buffalojoec to check if my implementation actually matches that of the rust client.

Actually you're pretty damn close, I just had the comment about little-endian. It's all facilitated by the PodSlice type in the spl-type-length-value library, which uses bytemuck zero-copy serde to do exactly what you're doing, iterate over deterministic bytes in a slice with a u32 length at the front to know where to stop.

• The way transferCheckedWithHook and addExtraAccountsToInstruction are structured is a little bit made up by me. Let me know if it matches your expectations @joncinque

I think these look pretty good! I don't think we'll really be able to test these handlers properly until we have the instructions from the transfer hook interface baked into the JS client, so we can tell a program to pack some extra metas and then try to transfer with them.

Once there, we can probably pluck these commands out of the package.json and into a shell script so we can also deploy a hook program and maybe even another CPI program like the Rust tests do.

solana-program-library/token/js/package.json

Lines 44 to 47 in 737a37a

	"test:e2e-built": "start-server-and-test 'solana-test-validator --bpf-program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA ../../target/deploy/spl_token.so --reset --quiet' http://127.0.0.1:8899/health 'mocha test/e2e'",
	"test:e2e-2022": "TEST_PROGRAM_ID=TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb start-server-and-test 'solana-test-validator --bpf-program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL ../../target/deploy/spl_associated_token_account.so --bpf-program TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb ../../target/deploy/spl_token_2022.so --reset --quiet' http://127.0.0.1:8899/health 'mocha test/e2e*'",
	"test:e2e-native": "start-server-and-test 'solana-test-validator --reset --quiet' http://127.0.0.1:8899/health 'mocha test/e2e'",
	"test:build-programs": "cargo build-sbf --manifest-path ../program/Cargo.toml && cargo build-sbf --manifest-path ../program-2022/Cargo.toml && cargo build-sbf --manifest-path ../../associated-token-account/program/Cargo.toml",

[wjthieme]

Multisigs are evaluated at the Token-2022 level.

Awesome! So the js implementation for this is correct and a transfer hook program would need to take these extra multisig accounts into consideration if they want to support it.

Perhaps just a split-out TLV JS library?

Yeah I think that would make more sense than a tlv-account-resolution library. Perhaps we can create a issue for it but guess it is not that important for now.

so we can tell a program to pack some extra metas and then try to transfer with them

Yeah was thinking about that as well but figured that was beyond the scope of this PR/issue

[wjthieme]

Extra clarification question

Is it correct that tlv-account-resolution has a limitation that accounts using other accounts as seeds need to be ordered after the account that needs them? At least that is what I am seeing in the rust implementation so just copied that logic into js as well.

i.e. if you are resolving the account at index 3 it can only use accounts 0..2 as seed inputs.

[buffalojoec]

Extra clarification question

Is it correct that tlv-account-resolution has a limitation that accounts using other accounts as seeds need to be ordered after the account that needs them? At least that is what I am seeing in the rust implementation so just copied that logic into js as well.

i.e. if you are resolving the account at index 3 it can only use accounts 0..2 as seed inputs.

Yep that's correct. I have an implementation where this constraint is no longer required but it was a bit complicated. If we ever need it, I'll pull that bad boy out

[buffalojoec]

Awesome! So the js implementation for this is correct and a transfer hook program would need to take these extra multisig accounts into consideration if they want to support it.

Yep

Perhaps just a split-out TLV JS library?

Yeah I think that would make more sense than a tlv-account-resolution library. Perhaps we can create a issue for it but guess it is not that important for now.

Agreed I swore I made one already but I'll throw one up now #5127

so we can tell a program to pack some extra metas and then try to transfer with them

Yeah was thinking about that as well but figured that was beyond the scope of this PR/issue

Agreed not in this PR. Also made an issue for this! #5128

[joncinque]

Really great work! This is pretty close to ready, thanks so much for contributing. We'll just need to fixup the decoding bit and clean up a few nits, then it should be good to go!

[joncinque]

This is extremely close, just a few last little things! Thanks for your patience and quick responses

[joncinque]

This looks good to me, thanks so much for taking this on! You:

I'll give @buffalojoec a chance to add any other comments

[buffalojoec]

Beast! Ship it 🚢