Integration of the NFT pallet in XCM

[vstam1]

Problem

While working on integrating the pallet-nfts with XCM, I ran into a problem with the CollectionId. The NFTs pallet works with a NextCollectionId storage, where the CollectionId for the next collection is stored. Each time a collection is created, the Id for the next collection is calculated and stored. We require the collectionId to implement the Incrementable trait, so that it's easy to calculate the next value.

To enable derivative NFTs in the NFTs pallet, we should be able to represent a CollectionId as a MultiLocation. For Example, the CollectionId of a derivative of a NFT on the relay chain could be represented as Parent/GeneralIndex(x). However, the current flow of the NextCollectionId does not allow for this structure, as there is no incremental flow for these MultiLocations.

Solution

We add an additional extrinsic that takes a collectionId and bypasses the whole NextCollectionId flow.

pub fn create_with_collection_id (
        origin: OriginFor<T>,
	admin: AccountIdLookupOf<T>,
	config: CollectionConfigFor<T, I>,
        collection_id: T::CollectionId,
) -> DispatchResult 

The NFT pallet should add on top of the CreateOrigin and ForceOrigin config types, also a CreateCollectionIdOrigin (other names are welcome) that gates the Origins that can create collections with custom CollectionIds.

[ggwpez]

However, the current flow of the NextCollectionId does not allow for this structure, as there is no incremental flow for these MultiLocations.

I dont understand this sentence. Is the problem that XCM cannot increment the collection id?
Just to be clear: the id would be Parent/GeneralIndex(x.inc()) for example.

Maybe it would help to configure the CollectionId type as an enum: one part being managed by normal increments and one variant for XCM arbitrary ids. The Incremental trait does not require that the full range of the type is accessible through it AFAIK.

[vstam1]

Is the problem that XCM cannot increment the collection id?

The main problem is that the derivates can come from different origins. So we can have three calls to create a collection with the following CollectionIds:

1. Parent/GeneralIndex(3)
2. Parent/Parachain(1)/GeneralIndex(1)
3. Parent/GeneralIndex(2)

In the current flow, after the creation of a collection with collectionId 1 we set the Id for the next collection. However, we do have no way to know what the next Id should be as the order is arbitrary.

[gavofyork]

we should be able to represent a CollectionId as a MultiLocation

Is this accurate? It sounds like you mean it should be possible to represent a MultiLocation as a CollectionId?

[vstam1]

@gavofyork I think you are right! Just to make it more clear, I want the CollectionIds of the nfts pallet to be a MultiLocation. This is btw currently not possible because MultiLocation does not implement the Incrementable trait, so in this draft I introduced a wrapper for MultiLocation.

Do you think the solution described in this issue is the right approach?

[gavofyork]

I see. So it seems that the NFT pallet is rather overzealous in its trait bounds to the point where it's not really fit for this purpose. In particular it has determined that the key type must be self-generating when under certain usage situations (which the original designer was apparently ignorant of) it need not be, and may not even be practical.

If we were doing it right, the collection creation logic which uses this autokey functionality (the Incrementable trait) should be an optional adjunct, at the very least fallible and ideally contained in a separately configurable module or pallet.

Right now it seems there's two trait Incrementals in the codebase 😕 I would suggest we first ditch one of them (the one actually in pallet-nfts I suppose) and then I would make the remaining trait Incrementable be fallible for both functions. MultiLocation can return None/Err for both. If the auto-keying create is called, it can error out in the case that the auto-key is not available.

[jsidorenko]

I've removed the duplicate from the nfts pallet here #14367