Decide if we use another nft pallet

[tonimateos]

As a LAOS dev I want to understand if integrating an existing nft pallet would save a lot of work (compared to writing all methods by ourselves), and how I could integrate it.

Worth having a look at using it via traits here

ACCEPTANCE:

• an analysis is presented to the team, and a decision is made (devs + PO) about using existing nft pallet

[ghost]

criteria	External pallet (nfts, uniques)	Own pallet
Effort	Less effort required	Will have to build from scratch, "re-invent the bicycle"
Customizability	Won't be able to customize it	Can customize it however we want, according to our needs
Maintenance	Minimal effort required, only version upgrades	Maintain it as any other local pallet (migrations, version upgrades, etc.)
Audit	Less effort required during the audit, even if the pallet itself is not audited, it still comes from reputable team paritytech	Audit will require more effort and more resources

Some notes for now

[ghost]

Problems with pallet_nfts:

1. not possible to set collection attributes with traits.

This means that we can't set BaseURI for a new collection and token_uri call won't work.

Mutate trait has set_collection_attribute which works, but it's not possible to read the collection attribute after it was set. This is a bug that I will open an issue. But it seems like they are already going in that direction already with a separate trait for Metadata
paritytech/polkadot-sdk#1561

More context:
https://substrate.stackexchange.com/questions/9974/setting-metadata-of-an-item-of-the-nfts-pallet-in-a-custom-pallet

2. it's possible to hide extrinsics, but we can't hide events.

I.e when we create collection or transfer through evm all the pallet_nfts events will be emitted on substrate side. There's no way to hide it because, pallet_nfts::Config uses RuntimeEvent

3. another dependency to substrate (similar to parity-bridges-common)

we add another dependency to our list and if we encounter any bugs, issues or want to change the behaviour, we should make PR to substrate and wait until it's merged. And some proposals may actually get rejected from the substrate side. In short, it might slow down the development.

[ghost]

this is how our precompiles would look like (on a high-level) if we were to use pallet-nfts

#67

[ghost]

Pallet NFTs

Docs: https://github.com/paritytech/polkadot-sdk/tree/master/substrate/frame/nfts#nfts-pallet

Potential parts of the code to be changed.

1. Get owner of an item
2. Mint an item

[jsidorenko]

but it's not possible to read the collection attribute after it was set.

Hi, actually, there is a way to read the collection's metadata and attributes, it's the collection_attribute() method on the Inspect trait. You can see it here: https://github.com/paritytech/polkadot-sdk/blob/e3892745f7c58e877d8838587e1471a93796ead4/substrate/frame/nfts/src/impl_nonfungibles.rs#L100C5-L100C25

[ghost]

but it's not possible to read the collection attribute after it was set.

Hi, actually, there is a way to read the collection's metadata and attributes, it's the collection_attribute() method on the Inspect trait. You can see it here: https://github.com/paritytech/polkadot-sdk/blob/e3892745f7c58e877d8838587e1471a93796ead4/substrate/frame/nfts/src/impl_nonfungibles.rs#L100C5-L100C25

hey @jsidorenko

thanks for reaching out here.

yes, we are aware of the collection_attribute() function. But it's currently not possible to read the collection_attribute once we set it, because when setting AttributeNamespace::Pallet is used and when reading AttributeNamespace::CollectionOwner is used for the value of namespace.

I think this is a bug, since users who use the pallet-nfts through nonfungibles_v2 traits can't read back the attribute they set.

I was actually thinking of making a PR for this, just want to confirm with you if this is actually a bug or there is something I am missing?

[jsidorenko]

Oh, I see! The easiest solution would be to change that line: https://github.com/paritytech/polkadot-sdk/blob/e3892745f7c58e877d8838587e1471a93796ead4/substrate/frame/nfts/src/impl_nonfungibles.rs#L87 and make the item param optional.

How would you solve that issue?

[ghost]

Oh, I see! The easiest solution would be to change that line: https://github.com/paritytech/polkadot-sdk/blob/e3892745f7c58e877d8838587e1471a93796ead4/substrate/frame/nfts/src/impl_nonfungibles.rs#L87 and make the item param optional.

How would you solve that issue?

yes, that's what I was going to propose

[jsidorenko]

Cool, I can open a PR later today, or feel free to submit it first :)