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

Granular NFT traits and new XCM NFT types #4300

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

mrshiposha
Copy link
Contributor

@mrshiposha mrshiposha commented Apr 25, 2024

Overview

This PR introduces a new set of traits that represent different asset operations in a granular and abstract way.
Also, it provides new XCM adapters to work with NFT-like entities that use the abovementioned traits.

The new abstractions provide a granular and general interface for both non-fungible collections and tokens for use in general and XCM contexts.
Though, this PR focuses more on the XCM context.

The new XCM adapters and utility types to work with NFTs can be considered the main deliverable of the XCM NFT proposal. The new types use a more general approach, making integrating into any chain with various NFT implementations easier.

For instance, different implementations could use:

  • different ID assignment approaches
    • predefined IDs - pallet-uniques, pallet-nfts
    • auto (for collections) / derived IDs (NFT IDs derived from collection IDs) - Unique Network, ORML/Acala, Aventus
  • classless (collection-less) tokens - CoreTime NFTs, Aventus NFT Manager NFTs
  • in-class (in-collection) tokens - Unique Network, pallet-uniques, pallet-nfts, ORML/Acala
  • different approaches to storing associated data:
    • data is stored entirely separately from tokens - pallet-uniques
    • data is stored partially or entirely within tokens (i.e., burning a token means destroying it with its data) - pallet-nfts (partially), Unique Network, ORML/Acala

With new types, these differences can be either abstracted away or worked around (see the example with pallet-nfts below).

Moreover, with new types, it becomes more apparent how to introduce support for derivative NFTs (see the possible solution for pallet-nfts below). The new utility types are flexible so that different chains can fine-tune the support for their needs.

Also, this is the PR I mentioned in the #4073 issue, as it can be viewed as the solution. In particular, the new adapter requires the Transfer operation with the FromTo strategy. This brings the attention of both a developer and a reviewer to the FromTo strategy (meaning that the transfer checks if the asset can be transferred from a given account to another account) both at trait bound and at the call site without sacrificing the flexibility of the NFT traits.

The following section gives a gist of the operations and strategies.
A detailed description of the operations and strategies is given in the doc-comments.

New frame-support traits

The new asset_ops module is added to frame_support::traits::tokens.
It defines several "operations" on two "asset kinds":

  • Class - class-like entities. For example, a collection of non-fungible tokens.
  • Instance - instances of something. For example, a single non-fungible token.

Additional asset kinds can be defined, though this is outside the scope of this PR, which is NFT-focused.

We avoid duplicating the interfaces with the same idea by providing the possibility to implement the same operations on different asset kinds, e.g., creating Collections/NFTs, transferring their ownership, managing their metadata, etc.

The following "operations" are defined:

Each operation can be implemented multiple times using different strategies associated with this operation (see the example below).

Example: operations and strategies

Let's consider the InspectMetadata and the Transfer operations.

InspectMetadata

We can implement the InspectMetadata to inspect if an NFT (i.e., an Instance) can be transferred by utilizing the CanTransfer strategy (a generic type; check out its docs) from the common_strategies module:

impl InspectMetadata<Instance, CanTransfer> for NftEngine {
	fn inspect_metadata(
		instance_id: &Self::Id,
		_can_transfer: CanTransfer,
	) -> Result<bool, DispatchError> {
		todo!()
	}
}

We can do a similar thing to inspect if the ownership of an NFT collection (i.e., a Class) can be transferred:

impl InspectMetadata<Class, CanTransfer> for NftEngine {
	fn inspect_metadata(
		class_id: &Self::Id,
		_can_transfer: CanTransfer,
	) -> Result<bool, DispatchError> {
		todo!()
	}
}

Another example of inspecting the metadata is reading raw bytes. We can utilize the Bytes strategy (also a generic type):

impl InspectMetadata<Instance, Bytes> for NftEngine {
	fn inspect_metadata(
		instance_id: &Self::Id,
		_bytes: Bytes,
	) -> Result<Vec<u8>, DispatchError> {
		todo!()
	}
}

In this PR, pallet-uniques introduce their own flavor of the Bytes strategy - Attribute. Thus, we can inspect raw bytes under a specific key:

// See the actual code here: https://github.com/UniqueNetwork/polkadot-sdk/blob/45855287b8647f34a4b3015facc714232c2ebe3e/substrate/frame/uniques/src/impl_asset_ops/instance.rs#L44-L59

impl<'a> InspectMetadata<Instance, Bytes<Attribute<'a>>> for NftEngine {
	fn inspect_metadata(
		instance_id: &Self::Id,
		strategy: Bytes<Attribute>,
	) -> Result<Vec<u8>, DispatchError> {
		let Bytes(Attribute(attribute_key)) = strategy;

		todo!()
	}
}

Transfer

To transfer an NFT with an extra check that a particular user owns the token before the transfer, we can do this:

impl Transfer<Instance, FromTo<AccountId>> for NftEngine {
	fn transfer(instance_id: &Self::Id, strategy: FromTo<AccountId>) -> DispatchResult {
		let FromTo(from, to) = strategy;

		todo!()
	}
}

Of course, as with the InspectMetadata, we can define a similar Transfer operation for Classes. But let's omit that for brevity.

We would want to transfer without additional checks (e.g., in some internal system, we want to "just" transfer the asset).
We can do that using the JustDo strategy:

impl Transfer<Instance, JustDo<AccountId>> for NftEngine {
	fn transfer(instance_id: &Self::Id, strategy: JustDo<AccountId>) -> DispatchResult {
		let JustDo(dest) = strategy;

		todo!()
	}
}

Or we would want to "just" transfer after the origin check. So, we can combine the JustDo with the WithOrigin strategy:

impl Transfer<Instance, WithOrigin<RuntimeOrigin, JustDo<AccountId>>> for NftEngine {
	fn transfer(
		instance_id: &Self::Id,
		strategy: WithOrigin<RuntimeOrigin, JustDo<AccountId>>,
	) -> DispatchResult {
		let WithOrigin(origin, JustDo(dest)) = strategy;

		todo!("check origin")
		todo!("transfer")
	}
}

Note: WithOrigin can be combined with any strategy and used with any operation.

For further examples, see how pallet-uniques implements these operations for classes and instances.

New types for xcm-builder and xcm-executor

This PR introduces several new XCM types. Describing them by example seems more straightforward than listing them. Yet, the most important ones should be outlined.

UniqueInstancesAdapter is the new TransactAsset adapter that supersedes both NonFungibleAdapter and NonFungiblesAdapter (for reserve-based transfers only, as teleports can't be implemented appropriately without transferring the NFT metadata alongside it; no standard solution exists for that yet. Hopefully, the XCM RFC 50 (PR, text) will help with that).

Thanks to the new Matcher types, the new adapter can be used instead of both NonFungibleAdapter and NonFungiblesAdapter:

Example: superseding the old adapters for pallet-uniques

Here is how the new UniqueInstancesAdapter in Westend Asset Hub replaces the NonFungiblesAdapter:

/// Means for transacting unique assets.
pub type UniquesTransactor = UniqueInstancesAdapter<
	AccountId,
	LocationToAccountId,
	MatchInClassInstances<UniquesConvertedConcreteId>,
	Uniques,
>;

MatchInClassInstances allows us to reuse the already existing UniquesConvertedConcreteId matcher. As pallet-uniques already implements the needed traits and can destroy and re-create NFTs without losing their data, we just plugged the Uniques pallet instance directly into the adapter.

So, migrating from the old adapter to the new one regarding runtime config changes is easy.

Example: declarative modification of an NFT engine

For simplicity, this PR doesn't include the implementation of the new trait for pallet-nfts.
However, a separate example PR is created in the fork to illustrate how pallet-nfts might be used in this new setup.

Also, pallet-nfts is a good example of an NFT engine that can't destroy and then re-create an NFT without losing its data in general (as is ORML/Acala, Unique Network, Aventus, and possibly more).

Yet, the UniqueInstancesAdapter requires an NFT engine to be able to create and destroy NFTs.
We will avoid implementing the new functionality for the NFT engine directly. Instead, we will illustrate how to model the "create" and "destroy" operations using other operations. Essentially, we will declaratively create a new NFT engine right in the runtime config.
Here is the code in the example PR (Westend Asset Hub):

pub type NftsConvertedConcreteId = assets_common::NftsConvertedConcreteId<NftsPalletLocation>;

type NftsStash = SimpleStash<TreasuryAccount, Nfts>;

// Means for transacting nft assets.
type NftsTransactor = UniqueInstancesAdapter<
	AccountId,
	LocationToAccountId,
	MatchInClassInstances<NftsConvertedConcreteId>,
	UniqueInstancesOps<RestoreOnCreate<NftsStash>, Nfts, StashOnDestroy<NftsStash>>,
>;

The main actor here is UniqueInstancesOps.
It accepts implementations of the "create", "transfer", and "destroy" operations and merges them into one "NFT engine" that implements all three.

The RestoreOnCreate predictably uses the Restore operation when asked to create an NFT. StashOnDestroy works similarly by using the Stash operation.

The pallet-nfts doesn't implement the Restore and Stash operations, however. So, we utilize SimpleStash, which takes a stash account and an NFT engine capable of transferring assets using the FromTo strategy. It transfers an NFT to the stash account when stashing and from the stash account when restoring.

To sum things up, this example illustrates how the existing NFT engine can be declaratively modified to fit into the XCM adapter if needed.

Example: supporting derivative NFTs (reserve-based model)

This example is outdated and will be modified (simplified)

the old text

Different implementations of the derivative NFTs support for different NFT engines could exist.
Each team might develop different solutions. However, some basic stuff can be generalized, so this PR introduces several basic types and a tiny pallet to facilitate derivatives support.

This example describes one of the possible solutions to give a glimpse of how these utilities can be applied.

Let's consider a situation in which we can't make assumptions about the format of the foreign NFT IDs (we don't know if they can be bi-directionally converted to and from IDs used in our chain). This is the most general setup, implying a more complex solution. Due to its generality, it should be more suitable as an example.

In the example, we will use pallet-nfts to support the derivative NFTs and modify the above example. The code of this example can be found in the fork's PR in the Rococo Asset Hub runtime.

Let's gradually formulate the solution.

  1. pallet-nfts will contain:
    • both original and derivative collections (correspond to foreign collections on another chain).
    • both original and derivative NFTs (they correspond to concrete foreign NFTs).

Note: We could surely use two different instances of pallet-nfts. But let's discuss the case when we use the same pallet instance. For uniformity reasons, for example.

  1. As we can't make assumptions about the format of the foreign NFT IDs, we need a mapping between XCM AssetInstance and our derivative NFT IDs

This PR adds the trait DerivativesRegistry to xcm-builder to abstract such mappings.

pub trait DerivativesRegistry<Original, Derivative> {
	fn try_register_derivative(original: &Original, derivative: &Derivative) -> DispatchResult;

	fn try_deregister_derivative(derivative: &Derivative) -> DispatchResult;

	fn get_derivative(original: &Original) -> Option<Derivative>;

	fn get_original(derivative: &Derivative) -> Option<Original>;
}

For step 2, we need its implementation with the following:

  • Original = NonFungibleAsset, i.e. (AssetId, AssetInstance)
  • Derivative = (CollectionId, ItemId)

To easily introduce such a registry into our chain, we can use the tiny pallet this PR provides — pallet-xnft.

Here is its config in Rococo Asset Hub in the fork's PR:

impl pallet_xnft::Config for Runtime {
	type RuntimeEvent = RuntimeEvent;

	type DerivativeIdParams = CollectionId;
	type DerivativeId = (CollectionId, ItemId);
}

Please ignore the DerivativeIdParams for now. We will discuss this when we get to registering new derivative NFTs on our chain.
Let's focus on working with already registered derivatives NFTs.

Working with registered derivative NFTs

We can extend the Matcher of the adapter from the above example to support registered derivatives.

This PR adds the MatchDerivativeInstances type, which matches a foreign NFT if it has a derivative NFT in the supplied Registry.
So, our Matcher for derivatives will look like this:

MatchDerivativeInstances<DerivativeNftsRegitry>

Where DerivativeNftsRegitry is pallet_xnft::DerivativeInstancesRegistry<Xnft> and Xnft is an instance of pallet-xnft in the Runtime.

We would've added this Matcher into the previous example's adapter right away in a tuple with the Matcher for our original NFTs. However, we need to address the fact that our original NFT and derivatives share the ID space on our chain (because all of them are in the same pallet).
For instance, consider a registered derivative NFT (CollectionId(42), ItemId(256)) that corresponds to the original NFT (AssetId(<reserve-location>), AssetInstance(...)). If we do nothing, the Matcher from the previous example's adapter will be triggered if we identify the derivative using local identification:

Asset {
	id: (PalletInstance(52), GeneralIndex(42)).into(),
	fun: AssetInstance::Index(256).into(),
}

This is bad since our chain will act as the reserve location for derivative NFTs. We must guarantee that derivatives will get matched only if they were identified by their original XCM ID and not by our local IDs.

To avoid this situation, we can use the EnsureNotDerivativeInstance type. We need to wrap the Matcher from the previous example in this type:

type MatchAssetHubOriginalInstances = EnsureNotDerivativeInstance<
	DerivativeNftsRegitry,
	MatchInClassInstances<NftsConvertedConcreteId>,
>;

Now, we can write the final code for working with registered derivatives.
Note that only the Matcher has been changed. Everything else is the same as in the previous example.

type NftsTransactor = UniqueInstancesAdapter<
	AccountId,
	LocationToAccountId,
	(MatchAssetHubOriginalInstances, MatchDerivativeInstances<DerivativeNftsRegitry>),
	UniqueInstancesOps<RestoreOnCreate<NftsStash>, Nfts, StashOnDestroy<NftsStash>>,
>;

Deriving NFT IDs

Before discussing how we could register new derivatives in pallet-nfts (i.e., mint new NFTs via the Asset Transactor), let's discuss how new tokens get their IDs.

The UniqueInstancesAdapter uses the most trivial ID assignment approach for creating tokens — PredefinedId, a special case of a more general DeriveAndReportId. PredefinedId trivially "derives" the ID. It just takes the ID as the parameter.

However, depending on the task we are solving and on a given NFT solution present in a chain, we might need a more general ID-deriving approach.

For example, ORML/Acala and Unique Network don't support creating NFTs with a predefined ID. Their NFT IDs are derived based on the collection ID by assigning incrementable in-collection IDs to new tokens.
Aventus ID derivation is even more general. It uses a contract address and a global incrementable ID as derivation parameters and forms a hash, which is used as the NFT ID.

Note: Aventus NFTs are an example of a situation when the NFT ID doesn't include the collection ID (contract address). The types of these two IDs are entirely different. This is a good thing to support since different teams may implement NFTs differently. Another notable example of unusual implementation is Core Time NFTs, which don't belong to any collection at all.

Therefore, this PR provides the UniqueInstancesDepositAdapter, which takes the ID derivation parameters type and requires the Matcher to return them. The matched ID parameters are supplied to the NFT engine to create a new token.

pub struct UniqueInstancesDepositAdapter<
	AccountId,
	AccountIdConverter,
	DeriveIdParams,
	Matcher,
	InstanceCreateOp,
>(PhantomData<...>);

We will use this general utility to create new NFT derivatives within pallet-nfts.

Registering derivative NFTs

Remember, because of how our task is formulated in this example, we can't make any assumptions about foreign NFT IDs. So, we can't directly convert foreign NFT's XCM IDs into our local NFT IDs.
This matters especially when a derivative NFT has yet to be created, so we don't have the needed mapping in the Registry to use in the Matcher.

Thus, we need a way to dynamically derive NFT IDs based on the chain's internal state.
We can utilize incrementable IDs for derivative tokens.

However, since pallet-nfts places NFTs in collections, our incrementable NFT ID should be derived from collection ID (it is our derivation parameter).

Creating derivative collections is out of the scope of an Asset Transactor. Like with fungibles, registering AssetIds must be handled separately.
pallet-xnft implements a second DerivativesRegistry that maps AssetIds to DerivativeIdParams supplied via pallet-xnft's Config. In our case, DerivativeIdParams is CollectionId.
This second Registry can be used like this: DerivativeIdParamsRegistry<Xnft>.

If a chain already has a means to register foreign assets (some "Asset Manager" pallet or a contract), the registration of derivative NFT collections can be easily integrated into it using the pallet-xnft's DerivativeIdParamsRegistry.

Let's introduce derivative ID parameters Registry into our code:

type ParamsRegitry = DerivativeIdParamsRegistry<Xnft>;

Now, we can use the UniqueInstancesDepositAdapter from the section above:

type NftDerivativesRegistrar = UniqueInstancesDepositAdapter<
	AccountId,
	LocationToAccountId,
	DerivativeRegisterParams<CollectionId>,
	MatchDerivativeRegisterParams<ParamsRegitry>,
	RegisterOnCreate<DerivativeNftsRegitry, ConcatIncrementableIdOnCreate<Xnft, Nfts>>,
>;

This Asset Transactor uses the RegisterOnCreate utility to bind a newly created derivative NFT with the foreign NFT identified by its XCM ID.
It implements the creation operation that uses DerivativeRegisterParams as parameters for DeriveAndReportId:

/// Parameters for registering a new derivative instance.
pub struct DerivativeRegisterParams<DerivativeIdParams> {
	/// The XCM identified of the original unique instance.
	pub foreign_nonfungible: NonFungibleAsset,

	/// The derive ID parameters for the [`DeriveAndReportId`]
	/// to create a new derivative instance.
	pub derivative_id_params: DerivativeIdParams,
}

The MatchDerivativeRegisterParams matches AssetId and maps it to the CollectionId using the ParamsRegitry and wraps it into the DerivativeRegisterParams.

The RegisterOnCreate creates a new NFT using the supplied NFT Engine (ConcatIncrementableIdOnCreate<Xnft, Nfts>) and binds it to the foreign_nonfungible in the DerivativeNftsRegitry.

Conclusion

This example describes a possible NFT derivatives solution for pallet-nfts. Its goal is to show the flexibility of the new XCM utility types, how they can be mixed, and how they work with the new granular NFT traits.
These types can be used with any NFT solution (ORML/Acala, Unique Network, Aventus, EVM-based chains like Moonbeam, etc.), and they are general enough to cover the differences in approaches between the solutions (both present and future).

We didn't consider de-registration in the example, yet it can be implemented since the DerivativesRegistry trait provides methods for that. Also, this PR provides the DeregisterOnDestroy type. However, the de-registration of NFT derivatives entirely depends on the internals of the NFT solution used on a given chain, so it is up to its developers to implement a concrete mechanism of sparing the actively used storage for derivatives.

@mrshiposha
Copy link
Contributor Author

CC @franciscoaguirre

@mrshiposha
Copy link
Contributor Author

mrshiposha commented Jul 2, 2024

@franciscoaguirre @xlc This PR adds new granular NFT traits to frame-support and provides new XCM types, giving us the instruments to implement XCM NFT in any chain regardless of differences in NFT solutions used in the ecosystem (different pallets, which represent NFTs differently or incompatible with each other, or smart contract-based solutions).

This PR could undoubtedly be divided into at least two. However, I decided to provide all the pieces at once so we can discuss the complete picture. I can divide the PR if needed.

@mrshiposha mrshiposha changed the title Feature/asset ops traits Granular NFT traits and new XCM NFT types Jul 2, 2024
@mrshiposha mrshiposha marked this pull request as ready for review July 2, 2024 17:57
@mrshiposha mrshiposha requested review from a team as code owners July 2, 2024 17:57
@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/6615539

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6615523

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

Did a shallow pass on the adapter, ops and strategies, I'll make more review passes


#[pallet::storage]
#[pallet::getter(fn foreign_asset_to_derivative_id_params)]
pub type ForeignAssetToDerivativeIdParams<T: Config<I>, I: 'static = ()> =
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to map AssetId to a DerivativeId. Is it possible to just use AssetId? As we do with ForeignAssets in asset hub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not strictly necessary. In fact, chains like AssetHub can use AssetId everywhere.

However, I introduced this optional pallet that can map AssetId to a different DerivativeId to support the use case of mapping foreign NFTs to the already existing NFT engine on a given chain. For instance, we at Unique want a unified interface for working with all kinds of NFTs on the chain, both ours and foreign ones. E.g., we want to use the existing methods for nesting and fractionalization.

This way, our clients can think about a single interface without worrying about the kind of token they are working with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the pallet's name should be refined to reflect its optionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good showcase of what can be done, I don't think it belongs in this repository then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it belongs in this repository then

Providing support for derivatives of a general form seems like a good thing. Also, the pallet is minimal, so maintaining it will be easy.

///
/// The implemented [`Destroy`] operation can be used in the
/// [`UniqueInstancesAdapter`](super::UniqueInstancesAdapter) via the [`UniqueInstancesOps`].
pub struct StashOnDestroy<InstanceOps>(PhantomData<InstanceOps>);
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of implementing destroy using a stash instead of burning the asset. However, should stash be its own operation? I also have a hard time wrapping my head around strategies, some seem to make sense, but some seem like they're more implementation details

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the Stash / Restore operations could be reasonably frequent. For instance, the fractionalization operation implies "stashing" the NFT and depositing associated fungibles (and symmetrically, burning fungibles and "restoring" the NFT).

The non-fungibles could be stashed during other, more high-level operations (like fractionalization). For instance, an NFT could be used as an entry ticket to a dedicated on-chain logic, and this ticket is considered "active" when the NFT is stashed. I feel that more use cases could be derived around stashing/restoring.

Nonetheless, I agree that these two strategies are more semantically complex than Transfer or Destroy.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that sense, isn't Stash be some sort of lock? Like how with fungibles you lock and you gain a benefit, it could be the same with an nft

Copy link
Contributor Author

@mrshiposha mrshiposha Aug 21, 2024

Choose a reason for hiding this comment

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

It could be. To me, the Stash seems more general than a hypothetical Lock. Maybe I'm wrong, but it looks like the Lock should preserve ownership (the Unlock should return the token to the before-lock-owner). Stash isn't obliged to provide such guarantees; hence, it is more general.

}

pub trait MatchesInstance<Id> {
fn matches_instance(a: &Asset) -> result::Result<Id, Error>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting addition. Some NFTs do indeed come without a collection, like coretime regions. Do they use the same AssetId and an index or do they use different AssetIds and the Undefined instance, which is supposed to be used for NFTs with only one instance?

Copy link
Contributor Author

@mrshiposha mrshiposha Jul 26, 2024

Choose a reason for hiding this comment

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

I think it depends on the chain that provides such NFTs :)

Yet, I'd like to see the following principle. Suppose we have collection-less NFTs on a chain. In that case, in XCM, they should be identified by the same AssetId and varying AssetInstance if these NFTs are different instances of the same "type" (where "type" is defined by the operations you can do with its objects).

So, applying this principle, the combination of AssetId and AssetInstance::Undefined should identify a singleton object with associated operations unique to this particular asset.

In short, I'd say this:

  • AssetId should identify the NFT engine (and possibly some interior "group" of NFTs in that engine, like a collection)
  • AssetInstance should identify the particular NFT instance inside the specified NFT engine.

Note: I assume the chain can host multiple NFT engines that provide different NFT-associated operations.

Self(PhantomData)
}
}
impl<Owner> MetadataInspectStrategy for Ownership<Owner> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see what you mean by "strategy" here, it's more of a view inside the metadata. You specify some values that are reasonable for the metadata to have, and a catch-all bytes one for custom stuff. It's maybe more of a Field?

Copy link
Contributor Author

@mrshiposha mrshiposha Jul 26, 2024

Choose a reason for hiding this comment

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

I agree. It indeed feels like a Field or something. Maybe rename it to MetadataField?

@acatangiu acatangiu requested a review from a team July 26, 2024 10:21
@mrshiposha mrshiposha mentioned this pull request Aug 26, 2024
12 tasks
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/persistent-parachains-for-a-long-term-testnet-on-v1-14/9886/1

@mrshiposha
Copy link
Contributor Author

@franciscoaguirre, I opened a separate PR #5620 containing only the new NFT traits.

Later, I will refactor the current PR #4300 description and code to make it solely XCM NFT-focused.

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