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

Add Token-2022 support #17

Merged
merged 70 commits into from
Dec 21, 2023
Merged

Add Token-2022 support #17

merged 70 commits into from
Dec 21, 2023

Conversation

febo
Copy link
Contributor

@febo febo commented Jul 20, 2023

PR to add support to Token-2022 accounts in the unified handlers. The changes consist of:

  • using StateWithExtensions to unpack SPL Token Account and Mint
  • all SPL Token instructions used are from spl-token-2022, which support both SPL Token versions
  • use of test_case on bpf tests so tests run using both SPL Token versions

All unified instructions are updated:

  • Burn
  • Create
  • Delegate
  • Lock
  • Mint
  • Print
  • Revoke
  • Transfer
  • Unlock
  • Unverify
  • Update
  • Verify

These now can be used with both SPL Token programs.

The changes depend on the following PRs:

Additionally, validation of mint and token accounts extensions were added to the Create, Mint, Print and Transfer instructions.

Co-authored-by: Jon Cinque me@jonc.dev (@joncinque)

@dr497
Copy link

dr497 commented Nov 28, 2023

@febo I was wondering if there have been any updates or insights from the security audit regarding the implications of closing mint accounts. This feature is highly anticipated in our operations. Any information on the progress or expected timelines would be greatly appreciated 🙏

@febo
Copy link
Contributor Author

febo commented Dec 1, 2023

@febo I was wondering if there have been any updates or insights from the security audit regarding the implications of closing mint accounts. This feature is highly anticipated in our operations. Any information on the progress or expected timelines would be greatly appreciated 🙏

@dr497 Can you provide a bit more details of the use case for burning mint accounts (apart from the rent reclaim aspect of it)? An issue of closing mint accounts is that in the scenario where the keypair used to create it is kept, it would be possible to recreate the mint account and the whole collective of the metadata accounts again.

@dr497
Copy link

dr497 commented Dec 4, 2023

@febo Thank you for your feedback on the potential risks associated with closing mint accounts. While it's true that deleting mint accounts allows for the possibility of recreating certain accounts with the same public keys, the actual risk this poses is relatively moderate and aligns with the existing risk profile in the NFT space. Currently, the recreation of NFTs with different public keys, but identical metadata, is a risk that exists and is accepted. The risk introduced by allowing mint accounts to be burned and potentially recreated is, in essence, a similar risk level to what already exists.

The specific use case for our request is directly tied to our operations with the Solana Name Service domain ↔ NFT bridge (https://github.com/Bonfida/name-tokenizer). Our objective is to migrate to pNFTs, which requires the creation of master editions. This step necessitates transferring the mint authority of each NFT to the Metaplex smart contract. However, the irreversible nature of transferring mint authority under the current system would effectively make the bridge one-way.

Burning the mint would elegantly solve this by allowing the recreation of NFTs with the same public keys, which is essential for the domain ↔ NFT bridge. This feature isn't merely a convenience but a critical requirement for our use case.

To address your concerns about security, perhaps we could implement a feature where only mints from whitelisted collections are eligible for burning. This approach could mitigate the risk of unauthorized recreations while still providing the functionality we need.

I believe that this feature, with the suggested safeguard and added transparency measures, would be a valuable addition to the Metaplex token metadata smart contract, not only for our project but potentially for others in the community facing similar challenges

@febo
Copy link
Contributor Author

febo commented Dec 6, 2023

Currently, the recreation of NFTs with different public keys, but identical metadata, is a risk that exists and is accepted. The risk introduced by allowing mint accounts to be burned and potentially recreated is, in essence, a similar risk level to what already exists.

Not exactly, unfortunately. Having the same metadata but a different mint is a clear way to differentiate them. There are applications that still rely on mint lists to validate collection membership and allowing a burned mint to be recreated breaks the current assumption that once you burn an NFT you cannot recreate the exactly same one.

An alternative solution for your bridge scenario is to not burn the NFT but transfer the ownership to a PDA (e.g., the bridge). So the bridge can still function as a two-way bridge: when a domain name is tokenized, the domain ownership is transferred to a PDA that will be holding the domain while it's tokenized and the NFT goes to the user; when the domain is redeemed, the NFT is transferred to a PDA in exchange of transferring the domain to the user.

Having said that, we are still evaluating the option of burning mint accounts.

@dr497
Copy link

dr497 commented Dec 6, 2023

Currently, the recreation of NFTs with different public keys, but identical metadata, is a risk that exists and is accepted. The risk introduced by allowing mint accounts to be burned and potentially recreated is, in essence, a similar risk level to what already exists.

Not exactly, unfortunately. Having the same metadata but a different mint is a clear way to differentiate them. There are applications that still rely on mint lists to validate collection membership and allowing a burned mint to be recreated breaks the current assumption that once you burn an NFT you cannot recreate the exactly same one.

An alternative solution for your bridge scenario is to not burn the NFT but transfer the ownership to a PDA (e.g., the bridge). So the bridge can still function as a two-way bridge: when a domain name is tokenized, the domain ownership is transferred to a PDA that will be holding the domain while it's tokenized and the NFT goes to the user; when the domain is redeemed, the NFT is transferred to a PDA in exchange of transferring the domain to the user.

Having said that, we are still evaluating the option of burning mint accounts.

I understand your concerns regarding the implications of allowing a burned mint to be recreated, particularly in terms of mint lists used for validating collection membership. It's a valid point that once an NFT is burned, the current assumption is that it cannot be recreated as the exact same entity. However, I'd like to offer a different perspective on this.

If an NFT was originally whitelisted, signifying its authenticity, the same criteria could logically apply if it were to be burned and recreated. The integrity of the NFT is maintained throughout its lifecycle, whether in its original or recreated form. We could adapt mint list management to track the lifecycle of an NFT, including any burning and recreating events, ensuring the integrity and uniqueness of NFTs in collections while allowing for dynamic use cases. Additionally, it's important to note that anyone currently relying on this behavior would not be impacted, as this change would only affect new Token 2022 collections. Therefore, all existing collections would continue to operate under current expectations, ensuring that there is no disruption to their established mechanisms.

Moreover, the alternative of transferring ownership to a PDA does not align with our project’s requirement for maintaining asset-backed integrity in the bridging process. The essence of our bridge's functionality is based on ensuring that each asset on one side of the bridge is directly and fully backed by a corresponding asset on the other side. Deviating from this principle by not burning the bridged tokens could lead to the creation of unbacked assets, similar to how when people bridge USDC from ETH to Solana and back, the expectation is that the Solana bridged tokens are burned to maintain parity.

To illustrate the broader potential impact of the feature we are proposing, consider a blockchain-based video game that features unique in-game assets like a legendary sword represented as an NFT. This sword can be earned, redeemed for rewards, and then 'reforged' for another player. For such a narrative and game mechanics to work seamlessly, the ability to burn and then recreate the NFT (with the same mint account) becomes crucial. It maintains the sword's legacy and continuity in the game world while ensuring its uniqueness.

Just as in the gaming scenario, where the ability to burn and recreate an NFT maintains the unique identity and continuity of an in-game asset, this concept is equally vital for real-world asset (RWA) tokenization. Take the example of limited-edition consumer goods like designer watches or exclusive artwork, each represented as a unique NFT. If an owner redeems their NFT, the ability to 'burn' the NFT and later retokenize it with the same mint for the next owner is crucial. This process ensures that each tokenized asset remains unique and preserves its history, while also enabling a seamless transfer of ownership in the physical and digital realms.

We believe that this feature could serve various innovative applications in the NFT space, offering new possibilities for projects that face similar challenges as ours. Implementing a whitelist approach could provide the perfect middle ground, balancing the need for security with the desire for greater composability and flexibility in the NFT ecosystem.

blockiosaurus
blockiosaurus previously approved these changes Dec 20, 2023
Copy link
Contributor

@blockiosaurus blockiosaurus left a comment

Choose a reason for hiding this comment

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

This looks great!

danenbm
danenbm previously approved these changes Dec 20, 2023
Copy link
Contributor

@danenbm danenbm left a comment

Choose a reason for hiding this comment

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

Everything LGTM! Just had a couple minor comments/questions

programs/token-metadata/program/tests/verify.rs Outdated Show resolved Hide resolved
clients/rust/tests/create.rs Outdated Show resolved Hide resolved
clients/rust/tests/create.rs Outdated Show resolved Hide resolved
@febo febo dismissed stale reviews from danenbm and blockiosaurus via 16b6bd5 December 21, 2023 00:36
Copy link
Contributor

@danenbm danenbm left a comment

Choose a reason for hiding this comment

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

LGTM !!!

@febo febo merged commit 4e5bcca into main Dec 21, 2023
9 checks passed
@febo febo deleted the febo/token-2022 branch December 21, 2023 00:50
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.

4 participants