rmrk-substrate icon indicating copy to clipboard operation
rmrk-substrate copied to clipboard

Add Mutability to Properties

Open HashWarlock opened this issue 3 years ago • 3 comments

Currently only the issuer can set a Property & if a NFT is locked then the Property cannot be changed or removed. Based on the spec setproperty.md, the user should be able to customize these properties if they are mutable. We have also run into this problem with flexibility for a project on Phala utilizing RMRK NFTs.

If we take a look at the logic here, the only way to update a Property for a NFT is if the Collection issuer is the sender AND the issuer owns the NFT. https://github.com/rmrk-team/rmrk-substrate/blob/a3abdac27660099baeb50b3db91fadbf66b7e7ea/pallets/rmrk-core/src/functions.rs#L48-L69

HashWarlock avatar Jul 14 '22 20:07 HashWarlock

This can also start the discussion on Logic for properties.

HashWarlock avatar Jul 14 '22 20:07 HashWarlock

Removing the Locked error sounds like a good idea.

So it seems the Property should map to an object that is {isMutable: bool, value: something} and just do a check on its existence or mutability?

I assume the issuer should be able to update the properties if the property is mutable but the issuer is not the owner, correct?

bmacer avatar Sep 12 '22 13:09 bmacer

initial attempts to implement this failed, running into an issue refactoring because of query_properties.

https://github.com/rmrk-team/rmrk-substrate/blob/d2d508dda2b19c76c9042163356938d5f58c1e29/pallets/rmrk-core/src/functions.rs#L753-L764

tried changing the .map line to .map(|(key, mutable, value)| PropertyInfoOf::<T> { key, mutable, value })

after changing PropertyInfo to

pub struct PropertyInfo<BoundedValue> {
	/// Key of the property
	// #[cfg_attr(feature = "std", serde(with = "serialize::vec"))]
	pub key: BoundedKey,

	/// Value of the property
	// #[cfg_attr(feature = "std", serde(with = "serialize::bool"))]
	pub mutable: bool,

	/// Value of the property
	#[cfg_attr(feature = "std", serde(with = "serialize::vec"))]
	pub value: BoundedValue,
}

but there's some type mismatch happening here that I can't figure out.

bmacer avatar Sep 12 '22 14:09 bmacer