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: repair offchain extra metas helper #6088

Closed

Conversation

buffalojoec
Copy link
Contributor

This stack introduces the necessary repairs to Token2022's extra account metas
helpers in order to solve the issue reported in #6064.

In this particular PR, Token2022's offchain helper is being repaired by first
adding the necessary steps to build an Execute instruction instead of
resolving on a transfer instruction. Then, this introduces a new function that
takes all of the parameters for instruction::transfer_checked(..), to ensure
we are not getting the wrong instruction, as suggested by @joncinque in this PR
comment
.

The commit flow is as follows:

  1. Repair the original offchain helper, refactor the client and CLI, and
    refactor a transfer hook test in program-2022-test.
  2. Add the new helper, deprecating the old one.
  3. Add tests for both the deprecated and the new helper.

Copy link
Contributor Author

buffalojoec commented Jan 9, 2024

@buffalojoec buffalojoec force-pushed the 01-08-token-2022_repair_offchain_extra_metas_helper branch from eaf4c89 to 73724a8 Compare January 9, 2024 05:43
@buffalojoec buffalojoec force-pushed the 01-08-token-2022_repair_offchain_extra_metas_helper branch from 73724a8 to ce7a81e Compare January 9, 2024 06:04
@buffalojoec buffalojoec force-pushed the 01-08-token-2022_repair_offchain_extra_metas_helper branch from ce7a81e to c03c8df Compare January 9, 2024 06:34
@buffalojoec buffalojoec force-pushed the 01-08-token-2022_repair_offchain_extra_metas_helper branch from c03c8df to a9cda55 Compare January 9, 2024 06:48
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.

The guts look great! I think we might be able to make the interface clearer, and otherwise, mostly small points

token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
token/program-2022/src/offchain.rs Show resolved Hide resolved
@buffalojoec
Copy link
Contributor Author

Thanks for the review @joncinque! If you're not opposed, I think I'll rewrite some of this commit history to shuffle around changes, rather than push up several new ones. Lmk.

@joncinque
Copy link
Contributor

Yep, works for me!

@buffalojoec
Copy link
Contributor Author

Closing in favor of the new stack at #6099 - all comments here resolved there.

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.

None yet

2 participants