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

Introduce nonfungibles v2 adapter #1242

Closed
wants to merge 6 commits into from

Conversation

vstam1
Copy link
Contributor

@vstam1 vstam1 commented Aug 29, 2023

This pr introduces the NonFungiblesV2Adapter as a new type for the XCM executor's AssetTransactor type, designed to work with the NFTs pallet.

Introduces:

NonFungiblesV2Adapter
MultiLocationCollectionId: Wrapper for MultiLocation that implements the Incrementable trait. The trait implementation allow the NFT's pallet CollectionId type to be a wrapped MultiLocation.
Replaces the uniques pallet with the NFTs pallet in the xcm-simulator parachain runtime.

@vstam1 vstam1 added R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM. A3-backport Pull request is already reviewed well in another branch. labels Aug 29, 2023
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3516263

@franciscoaguirre franciscoaguirre self-assigned this Sep 9, 2023
@bkontur
Copy link
Contributor

bkontur commented Apr 3, 2024

@franciscoaguirre @Szegoo do we still need this NonFungiblesV2Adapter? Something was merged here: #2924 and for uniques here: #2796

@vstam1
Copy link
Contributor Author

vstam1 commented Apr 3, 2024

@franciscoaguirre @Szegoo do we still need this NonFungiblesV2Adapter? Something was merged here: #2924 and for uniques here: #2796

Think this PR can be closed. I initially introduced it to work with the Uniques pallet. But I see that there is already some other implementation merged.

@liamaharon liamaharon closed this Apr 4, 2024
@franciscoaguirre
Copy link
Contributor

This was closed, but we still don't have an adapter for the nonfungible v2 traits. The merged PRs were for nonfungible v1 traits. Without this, pallet-nfts can't be used with XCM. What do you think about nonfungible v1 and v2 @mrshiposha ? Is it worth it to revive this, or should we create a better set of traits for NFTs?

@mrshiposha
Copy link
Contributor

I believe we need a better set of traits that will be more general and granular than the existing ones. Also, I think we need more granular NFT adapters.
For instance, the old pallet-uniques allow you to burn and then re-mint an NFT without losing any data (the property the corresponding XCM adapter uses). However, the pallet-nfts while preserving the attributes may remove the NFT metadata when it burns an NFT.

There could also be different approaches to NFT derivatives (if we consider the reserve-based model).
That's why I think we need more granular NFT adapters that use more general NFT traits.

I want to draft a PR about this (the same PR I proposed in the comment here). You'll see the concrete code there, and I will provide reasons for it in the PR description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A3-backport Pull request is already reviewed well in another branch. R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants