rmrk-team / rmrk-substrate

[WIP] RMRK Substrate pallets
https://rmrk.app
Other
74 stars 36 forks source link

Add Mutability to Properties #198

Open HashWarlock opened 2 years ago

HashWarlock commented 2 years ago

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 commented 2 years ago

This can also start the discussion on Logic for properties.

bmacer commented 2 years ago

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 commented 2 years ago

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.