Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

token 2022: add new offchain helper #6100

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

buffalojoec
Copy link
Contributor

@buffalojoec buffalojoec commented Jan 10, 2024

This PR adds a new offchain helper for creating a TransferChecked instruction
with the necessary accounts metas for a transfer hook ExecuteInstruction to
Token2022, deprecating the old one.

As described in #6064, the original offchain helper in Token2022 was being used
incorrectly to resolve extra account metas. Rather than providing the SPL
Transfer Hook interface helper with a valid ExecuteInstruction, the original
Token2022 helper was actually providing it with a transfer instruction, causing
erroneous account resolution.

Taking advantage of the more secure SPL Transfer Hook interface offchain helper
provided in #6099, this new offchain helper creates a TransferChecked
instruction and calls add_extra_account_metas_for_execute(..), providing the
keys used to build the transfer instruction.

Note: unlike the deprecated helper in #6099, the deprecated offchain helper in
Token2022 is in fact inaccurately resolving account metas for certain use
cases, thus it should be vigilantly avoided.

@buffalojoec buffalojoec force-pushed the 01-09-token_2022_add_new_offchain_helper branch 2 times, most recently from 9f760e1 to 0c3b21e Compare January 10, 2024 14:16
@buffalojoec buffalojoec requested a review from joncinque January 10, 2024 14:21
@buffalojoec buffalojoec marked this pull request as ready for review January 10, 2024 14:21
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfect! Just a micro-nit to fixup, then this can go in. This is all much more legible too

token/program-2022/src/offchain.rs Outdated Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
joncinque
joncinque previously approved these changes Jan 10, 2024
@buffalojoec buffalojoec force-pushed the 01-09-transfer_hook_add_new_offchain_helper branch from 262663a to 3700ef4 Compare January 11, 2024 16:35
@buffalojoec buffalojoec force-pushed the 01-09-token_2022_add_new_offchain_helper branch from 2fa3ac2 to 8c04474 Compare January 11, 2024 16:35
Copy link
Contributor Author

buffalojoec commented Jan 11, 2024

Merge activity

  • Jan 11, 4:15 PM: @buffalojoec started a stack merge that includes this pull request via Graphite.
  • Jan 11, 4:16 PM: Graphite rebased this pull request as part of a merge.
  • Jan 11, 4:16 PM: @buffalojoec merged this pull request with Graphite.

Base automatically changed from 01-09-transfer_hook_add_new_offchain_helper to master January 11, 2024 21:15
@buffalojoec buffalojoec force-pushed the 01-09-token_2022_add_new_offchain_helper branch from 8c04474 to 006e8fc Compare January 11, 2024 21:15
@mergify mergify bot dismissed joncinque’s stale review January 11, 2024 21:16

Pull request has been modified.

@buffalojoec buffalojoec merged commit 3e6a9b8 into master Jan 11, 2024
33 checks passed
@buffalojoec buffalojoec deleted the 01-09-token_2022_add_new_offchain_helper branch January 11, 2024 21:16
buffalojoec pushed a commit that referenced this pull request Jan 11, 2024
Following up from the new helpers added in #6099 and #6100, there's a transfer
hook test in `program-2022-test` that's using the now-deprecated helper to build
a swap instruction for the test program.

This PR replaces that bit of code to use the new helper from
`spl_transfer_hook_interface`.

Note: The test in question is designed to test Token2022's
`invoke_transfer_checked(..)` on-chain helper, *not* the offchain helpers. So,
it's appropriate to use the offchain helper directly from the interface crate
for this step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants