Contract Module to support ERC5169 (TokenScript)

[oleggrib]

Hello
This PR created to support ERC5169.
Basically its option to add list of URLs to the contract.

We have to save URLs to contracts because URLs will be links to the TokenScript files, which are XML bundles with code , which describes interface and code to interact with contract.

Tokenscript is a framework designed to make contract interaction easy.
https://www.tokenscript.org/docs/TokenScript-Component.html

A popular example of TokenScript usage is https://www.smartlayer.network/smartcat

SmartCat is NFT token, rated # 21 on OpenSea. Users can do actions with their tokens using SmartCat TokenScript file

Users can use TokenScript viewer to open their own SmartCat NFTs and interact with the contract with TokenScript UI. UI and code embedded in the TokenScript file, thereby eliminating the need to create/host webpage or app to help user interact with the contract.
TokenScript can automate most of contract interactions with and require a bit of coding on Svelte/React/VanillaJS

We are working with multiple projects which will support TokenScript and going to allow everyone to code TokenScript interfaces to their contracts. This is the reason, why we are asking to add this ERC5169 to the OpenZeppelin repo - let users add support of ERC5169 using only OpenZeppelin repo.

Currently developers have to use https://www.npmjs.com/package/stl-contracts to add support of ERC5169, but it can be more easy to use OpenZeppelin repo only.

Developer docs - https://launchpad-doc.vercel.app/

TokenScript Examples - https://github.com/SmartTokenLabs/TokenScript-Templates

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

🦋 Changeset detected

Latest commit: 60dd533

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openzeppelin-solidity	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Amxx]

Hello @oleggrib

Thank you for raising attention to this ERC.

I'm honestly not a fan of it, and I'm worried it got finalize in the current format. In particular my concerns are:

• I don't understand the rational for having a list of string instead of a unique string. A unique string to point to a folder (on IPFS?) with multiple files/subfolders. Having to support a list of string increasses the complexity of the bytecode, and the cost for running it.
• EIPs like this one usually include a getter and an event but no setter. What if you don't want the value to be updated after construction ? Having a useless setter has a cost? Also what if you want an internal setter that is triggered by some process. We usually implement internal setters, and devs are free to expose them externally or not, with the access control they want.
• In the case of ERC-721 and ERC-1155 there is already a tokenURI that usually points to a JSON. If a script is needed, I would expect it to be in that JSON, not in a separate getter.
• Running arbitrary code from an address that is published (even onchain) is usually a bad ting to do for security. This ERC feels like it could be leveraged to perform attacks. An admin could change the script to inject nasty code, and holder of the tokens that run this updated script may fall victim for that. Its not something I feel we should encourage.

Overall I'm very sceptical, and I don't think we should implement that unless there is "significant" request from the community. By significant, I mean I would expect at least 2 or 3 independant projects (not related to the EIP authors or to one another) to plan on using this ERC, and to ask for us to provide the implementation.

[Amxx]

I'd love to hear @ernestognw and @frangio 's opinions.

[oleggrib]

Hi, @Amxx , thank you for your response.

I don't understand the rational for having a list of string instead of a unique string. A unique string to point to a folder (on IPFS?) with multiple files/subfolders. Having to support a list of string increasses the complexity of the bytecode, and the cost for running it.

Actions to set/update script list very rare and in most cases it will contain single sring, so code can be simple and stay as is.

EIPs like this one usually include a getter and an event but no setter. What if you don't want the value to be updated after construction ? Having a useless setter has a cost? Also what if you want an internal setter that is triggered by some process.
We usually implement internal setters, and devs are free to expose them externally or not, with the access control they want.

it makes sense

In the case of ERC-721 and ERC-1155 there is already a tokenURI that usually points to a JSON. If a script is needed, I would expect it to be in that JSON, not in a separate getter.

TokenScript file related to whole contract, so we can use contractURI, not tokenURI
but contractURI points to some file, which can be additional failure point. for example when hosting broken
but in case if we read strings directy from contract then we can add both - HTTPS URL and IPFS string location, TokenScript viewer will have backup script location, it can be useful.

Running arbitrary code from an address that is published (even onchain) is usually a bad ting to do for security. This ERC feels like it could be leveraged to perform attacks. An admin could change the script to inject nasty code, and holder of the tokens that run this updated script may fall victim for that. Its not something I feel we should encourage.

I dont think its a problem, because any web-app , which interacts with contracts can have same problems.
In our case its more secure than WebApp, because TokenScript file developer can sign TokenScript with contracts Owner() key, its additional level of security, because signature verified and user can see checkmark that TokenScript file created by some reliable person.
User can check TokenScript code and easy read it
TokenScript can be cached on user device and work even whem hosting broken
Anyway - TokenScript makes user frindly UI to the contract, its doesn't add any additional rights.

[frangio]

I agree with @Amxx on not accepting this ERC implementation for the moment. It seems experimental, it might turn out to be a good idea but at this early stage it should be tried and iterated on by other projects, which merging into OpenZeppelin Contracts is not a prerequisite for.

[Amxx]

It seems experimental, it might turn out to be a good idea but at this early stage it should be tried and iterated on by other projects

The ERC is marked as final. Iterations (if any) will have to go through a new ERC...