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 js: create new offchain helper #6108

Merged
merged 5 commits into from
Jan 11, 2024

Conversation

buffalojoec
Copy link
Contributor

@buffalojoec buffalojoec commented Jan 10, 2024

This is the final PR to round off the changes required to fix #6064.

Previously, the offchain helpers for adding extra metas to instructions have
been replaced with new ones in the SPL Transfer Hook interface and Token2022.

This PR follows suit and adds a new helper to SPL Token JS.

The new helper, addExtraAccountMetasForExecute(..), mirrors the Rust helper in
SPL Transfer Hook interface, requiring the parameters for an
ExecuteInstruction to be passed into the function directly.

This change also adds a public function for creating an ExecuteInstruction, in
case developers wish to create such an instruction for directly sending
instructions to their transfer hook program.

These existing functions have been updated to use the new helper:

  • createTransferCheckedWithTransferHookInstruction(..)
  • createTransferCheckedWithFeeAndTransferHookInstruction(..)

Closes #6064

@buffalojoec buffalojoec force-pushed the 01-10-token_client_refactor_transfer_to_use_new_offchain_helper branch from 8df6a4b to 8bda3ed Compare January 10, 2024 21:18
@buffalojoec buffalojoec force-pushed the 01-10-token_js_create_new_offchain_helper branch from b835aee to 9b33a88 Compare January 10, 2024 21:18
@buffalojoec buffalojoec changed the base branch from 01-10-token_client_refactor_transfer_to_use_new_offchain_helper to 01-10-token_2022_test_refactor_transfer_hook_test_to_use_new_offchain_helper January 10, 2024 21:18
@buffalojoec buffalojoec requested a review from joncinque January 10, 2024 21:18
@buffalojoec buffalojoec force-pushed the 01-10-token_2022_test_refactor_transfer_hook_test_to_use_new_offchain_helper branch from 7de4df7 to 0b076b1 Compare January 10, 2024 22:04
@buffalojoec buffalojoec force-pushed the 01-10-token_js_create_new_offchain_helper branch from 9b33a88 to 7bf9077 Compare January 10, 2024 22:04
@buffalojoec buffalojoec changed the base branch from 01-10-token_2022_test_refactor_transfer_hook_test_to_use_new_offchain_helper to 01-10-token_2022_repair_onchain_helper January 10, 2024 22:04
@buffalojoec buffalojoec force-pushed the 01-10-token_2022_repair_onchain_helper branch from ac35de5 to b47b84e Compare January 10, 2024 23:30
@buffalojoec buffalojoec force-pushed the 01-10-token_js_create_new_offchain_helper branch from 7bf9077 to 2b6b769 Compare January 10, 2024 23:30
@buffalojoec buffalojoec force-pushed the 01-10-token_2022_repair_onchain_helper branch from b47b84e to a7e399c Compare January 11, 2024 01:02
@buffalojoec buffalojoec force-pushed the 01-10-token_js_create_new_offchain_helper branch from 2b6b769 to c7e787b Compare January 11, 2024 01:02
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.

Just nits really, the whole thing looks great!

token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/src/extensions/transferHook/instructions.ts Outdated Show resolved Hide resolved
token/js/test/unit/transferHook.test.ts Show resolved Hide resolved
token/js/test/unit/transferHook.test.ts Outdated Show resolved Hide resolved
token/js/test/unit/transferHook.test.ts Outdated Show resolved Hide resolved
@buffalojoec buffalojoec force-pushed the 01-10-token_2022_repair_onchain_helper branch from a7e399c to ed3fca4 Compare January 11, 2024 16:38
joncinque
joncinque previously approved these changes Jan 11, 2024
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.

Just one last little nit, but otherwise feel free to merge once it's resolved!

token/js/test/unit/transferHook.test.ts Outdated Show resolved Hide resolved
@buffalojoec buffalojoec force-pushed the 01-10-token_js_create_new_offchain_helper branch 2 times, most recently from 55dfa61 to dc2441d Compare January 11, 2024 20:55
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:25 PM: Graphite rebased this pull request as part of a merge.
  • Jan 11, 4:26 PM: @buffalojoec merged this pull request with Graphite.

@buffalojoec buffalojoec force-pushed the 01-10-token_2022_repair_onchain_helper branch from ed3fca4 to 367eedb Compare January 11, 2024 21:22
Base automatically changed from 01-10-token_2022_repair_onchain_helper to master January 11, 2024 21:24
@buffalojoec buffalojoec force-pushed the 01-10-token_js_create_new_offchain_helper branch from dc2441d to 07b2e6b Compare January 11, 2024 21:25
@mergify mergify bot dismissed joncinque’s stale review January 11, 2024 21:25

Pull request has been modified.

@buffalojoec buffalojoec merged commit de2e356 into master Jan 11, 2024
17 checks passed
@buffalojoec buffalojoec deleted the 01-10-token_js_create_new_offchain_helper branch January 11, 2024 21:26
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.

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