XCM NFT: pallet-nfts Westend Asset Hub#9537
Open
mrshiposha wants to merge 14 commits intoparitytech:masterfrom
Open
XCM NFT: pallet-nfts Westend Asset Hub#9537mrshiposha wants to merge 14 commits intoparitytech:masterfrom
mrshiposha wants to merge 14 commits intoparitytech:masterfrom
Conversation
…ssetTransactor for pallet-nfts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR implements the new NFT traits from #5620 for
pallet-nftsand uses the XCM NFT utilities from #4300 to add the corresponding Asset Transactors to the Westend Asset Hub.Also, the
pallet-nftsinstance is made a host for derivative NFTs in Westend AH using thepallet-derivativesinstances.pallet-nftswas chosen to host the NFT derivatives instead ofpallet-uniquesfor the following reasons:pallet-nftsis a newer and more feature-rich pallet. Thus, dApps will likely choose to work with it rather than withpallet-uniques. It would be easier for dApps to support exactly one pallet to work with all the NFTs instead of supporting two different pallets (pallet-nftsandpallet-uniques).pallet-nftshas an integration withpallet-nft-fractionalization. This means only its tokens can be fractionalized. So, if derivative NFTs are deployed there, they can be fractionalized as well.pallet-nfts NFT XCM-related peculiarities
Avoiding losing NFTs metadata
Usually, XCM burns the asset when executing
WithdrawAsset, and then mints it when executingDepositAsset.However, this approach can't be used for
pallet-nfts.pallet-nftsmight save the metadata when an NFT is burned. However, it doesn't always do this. If an NFT has theUnlockMetadatasetting set, the metadata will be cleared when the token is destroyed.So, to preserve the NFT metadata in all cases, the pallet-nfts transactor transfers the NFT to the Treasury on withdrawal and transfers from the Treasury on depositing.
Reusing the existing pallet-nfts instance for derivatives
We can't host another
pallet-nftswithCollectionId = AssetIdandItemId = AssetInstancefor derivative NFTs (similar topallet-assets) sinceCollectionIdmust satisfy the Incrementable bound.Also, if we were to create a separate
pallet-nftsinstance for derivatives, we wouldn't be able to use the existing integration with thepallet-nft-fractionalizationon Westend AH for derivative NFTs.Thus, to support the NFT derivatives, two instances of
pallet-derivativeswere added: 1)DerivativeNftCollectionsfor registering the NFT collections and storing the mapping between XCM AssetId and CollectionId; 2)DerivativeNftsfor holding the mapping between the XCM NFT ID and the local NFT ID.Review Notes
<pallet-nfts-src>/asset_ops/{collection,item}.rs.pallet-nftsrelated asset strategies are added to the<pallet-nfts-src>/types.rsinto theasset_strategiesmodule.tests.rs.pallet-derivativespath in the workspaceCargo.tomlQuestions and TODOs
GeneralAdminand seems rather limiting. May we lower the origin requirements (maybe even down to justEnsureSigned) at least for the testnets until a standard XCM way of registering derivative collections is devised? Lowering the origin requirements would greatly simplify experimenting with XCM NFTs on testnets.TODO: benchmark pallet-derivatives for Westend Asset Hub