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

RIP-0014 Add RESMODIFY and RESREMOVE interactions #44

Open
SupremaLex opened this issue Feb 14, 2022 · 16 comments
Open

RIP-0014 Add RESMODIFY and RESREMOVE interactions #44

SupremaLex opened this issue Feb 14, 2022 · 16 comments
Labels
RIP RMRK Improvement Proposal

Comments

@SupremaLex
Copy link

  • author(s): Heorhii
  • contact: @SupremaLex
  • RIP type: upgrade

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).

@SupremaLex SupremaLex added the RIP RMRK Improvement Proposal label Feb 14, 2022
@SupremaLex SupremaLex changed the title RIP-0014 RIP-0014 Add RESMODIFY and RESREMOVE interactions Feb 14, 2022
@Swader
Copy link
Contributor

Swader commented Feb 17, 2022

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
Copy link
Author

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
Copy link
Contributor

Swader commented Feb 17, 2022

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

@Yuripetusko
Copy link
Member

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

No this was never implemented

@Swader
Copy link
Contributor

Swader commented Feb 17, 2022

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
Copy link
Author

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

@Swader
Copy link
Contributor

Swader commented Feb 17, 2022

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
Copy link
Member

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
Copy link
Contributor

Swader commented Feb 17, 2022

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
Copy link
Author

SupremaLex commented Feb 17, 2022

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
Copy link
Contributor

Swader commented Feb 17, 2022

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
Copy link
Author

Can we make this feature Substrate/EVM version specific?

@Swader
Copy link
Contributor

Swader commented Feb 17, 2022

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
Copy link
Author

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
Copy link
Contributor

Swader commented Feb 17, 2022

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

@SupremaLex
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RIP RMRK Improvement Proposal
Projects
None yet
Development

No branches or pull requests

3 participants