rmrk-team / rmrk-spec

The RMRK NFT Specification
GNU General Public License v3.0
94 stars 35 forks source link

RIP-0014 Add RESMODIFY and RESREMOVE interactions #44

Open SupremaLex opened 2 years ago

SupremaLex commented 2 years ago

RIP Summary

Add RESMODIFY and RESREMOVE interaction, which NFT's owner should accept.

RIP Details

I propose to add two new interactions to NFT's resource system. The RESREMOVE interaction allows the issuer of a collection to remove a resource from an NFT in that collection. If the issuer is also the owner of this NFT, this interaction also counts as a ACCEPT automatically.

This is useful if some resource becomes deprecated and if you don't want to use the PRIORITY system to simulate resources mutability. Also, this is useful if we assume that BASE resource can change over time, i.e., new fixed parts or slots are added, and we don't want to store all previous versions of this BASE resource.

The RESMODIFY interaction allows the issuer of a collection to modify a resource of an NFT in that collection. If the issuer is also the owner of this NFT, this interaction also counts as a ACCEPT automatically.

This also helps to keep Storage clean, i.e., don't store old versions of BASE.

Examples

Let's assume that we have a base1:

  "symbol": "kanaria_superbird",
  "type": "svg",
  "parts": [
    {
        "id": "left_wing_front",
        "src": "ipfs://ipfs/hash",
        "metadata": "ipfs://ipfs/hash"
    },
    {
        "id": "left_wing_back",
        "src": "ipfs://ipfs/hash",
        "metadata": "ipfs://ipfs/hash2"
    }
  ]

and some NFT, which Resource's array looks like

"resources": [
  {
      "id": "V1StG",
      "base": "base1",
      "parts": ["left_wing_front", "left_wing_back"]
  }
]

Now, we want to add "right_wing_front" and "right_wing_back" (or remove some already existed, but deprecated parts). We can do it via RESADD interaction, but in case we want to keep consistency and keep resources logic relation, we may want to allow base1 to support these two new parts, i.e.

"symbol": "kanaria_superbird",
"type": "svg",
"parts": [
  {
      "id": "left_wing_front",
      "src": "ipfs://ipfs/hash",
      "metadata": "ipfs://ipfs/hash"
  },
  {
      "id": "left_wing_back",
      "src": "ipfs://ipfs/hash",
      "metadata": "ipfs://ipfs/hash2"
  },
  {
      "id": "right_wing_front",
      "src": "ipfs://ipfs/hash",
      "metadata": "ipfs://ipfs/hash"
  },
  {
      "id": "right_wing_back",
      "src": "ipfs://ipfs/hash",
      "metadata": "ipfs://ipfs/hash2"
  }
]

We can create base1_version2 with the data mentioned above and RESADD it. Then we will achieve:

"resources": [
  {
      "id": "V1StG",
      "base": "base1",
      "parts": ["left_wing_front", "left_wing_back"]
  },
  {
      "id": "V1FsU",
      "base": "base1_version2",
      "parts": ["left_wing_front", "left_wing_back", "right_wing_front", "right_wing_back"]
  }
]

or we can have (if we have RESREMOVE & RESMODIFY):

"resources": [
  {
      "id": "V1StG",
      "base": "base1",
      "parts": ["left_wing_front", "left_wing_back", "right_wing_front", "right_wing_back"]
  }
]

or

"resources": [
  {
      "id": "V1FsU", // - new id, because of new resource
      "base": "base1", // but the name is the same
      "parts": ["left_wing_front", "left_wing_back", "right_wing_front", "right_wing_back"]
  }
]

Such an approach will add more flexibility to RMRK NFT's resource system. Both remove and modify requests should be accepted by NFT's owner, so decentralization is kept.

Open Questions

Does the community interested in this?

Impact

I can't comment on Solidity implementation, but in the case of Substrate, we need to modify rmrk-traits(RESREMOVE, RESADD signatures) rmrk-core pallet (RESREMOVE, RESADD implementations, Storage to track requested remove/change).

Swader commented 2 years ago

Why not use RESADD with the replace param to just replace an outdated resource with a new one? Solves both cases and is already implemented

SupremaLex commented 2 years ago

Sorry for my blindness, but I don't see this "replace" param here https://github.com/rmrk-team/rmrk-spec/blob/master/standards/rmrk2.0.0/interactions/resadd.md, is it outdated? Also, I can't find such functionality in the rmrk-core pallet on the master branch. I would be glad if you pointed right piece of docs or code. Thank you.

Swader commented 2 years ago

Ooof sorry, looks like we did not publish it. @Yuripetusko is this in tools?

Yuripetusko commented 2 years ago

Ooof sorry, looks like we did not publish it. @Yuripetusko is this in tools?

No this was never implemented

Swader commented 2 years ago

Ok, added as requirement to both pallets and EVM. We should add it to spec and add it to tools as well. Sorry about that @SupremaLex I thought this was in.

SupremaLex commented 2 years ago

No problems, do you mean to add [replace] param to RESADD?

Swader commented 2 years ago

Yes, so RESADD will get a replace param which is the ID of an already existing resource. If provided, it will overwrite the existing one, marking the existing one as deprecated (UIs should not render it)

Yuripetusko commented 2 years ago

hm you think mutating existing Resource is a no go? In pallets a lot of entities are mutable, attributes, metadata etc. I don't see a harm in making resources mutable too

Swader commented 2 years ago

Mutating is a permissioned operation which has many problems attached to art rugpulls and centralization and needs an ACL layer to match which parts are mutable and when a resource can be mutated. Would an owner need to accept mutations? If not, ruggable. If yes, nightmare in UI. Much easier to just accept a new resource and get the same result

SupremaLex commented 2 years ago

What do you mean by problems with UI? We already need to accept a new Resource and this looks fine, accepting mutation looks similar. Also, we already need to accept/reject NFT in the context of nft_send.

Swader commented 2 years ago

Accepting a mutation needs a proposing a mutation flow and UI too, which is just more burden, esp since we want to feature-freeze kusama implementation of RMRK and focus on contracts and pallets

SupremaLex commented 2 years ago

Can we make this feature Substrate/EVM version specific?

Swader commented 2 years ago

Yes, the idea is to feature freeze the remark implementation and upgrade only EVM and pallets going forward. I have already added this as a requirement to both.

SupremaLex commented 2 years ago

The idea is good, but I still can't understand what a requirement that you added is: RESADD with [replace], or RESMODIFY/RESREMOVE as I suggested, or just "Resource mutability".

Swader commented 2 years ago

I suggested upgrading RESADD with an optional ID target to overwrite an existing resource

SupremaLex commented 2 years ago

So, now we can overwrite resources, but we cannot remove them. @Swader, can you clarify whether or not resource removal functionality will be added? It is selfish, but our project depends on RMRK, so I am interested in the best way of leveraging RMRK. And resource removal functionality is crucial for us if we want to use the RMRK resource model.