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

draft new ERC for contract working with ts #430

Draft
wants to merge 785 commits into
base: main
Choose a base branch
from
Draft

draft new ERC for contract working with ts #430

wants to merge 785 commits into from

Conversation

SmartLayer
Copy link

@SmartLayer SmartLayer commented May 9, 2022

rosieprom and others added 30 commits December 4, 2019 11:35
added redeemed text, compressed images
WIP add basic edcon draft TokenScript
correct contracts

swap
TS Template es.html front-end for 6/11 tickets WIP
This may break the older build but since the only known tokenscript file that uses multiple views are UEFI, we might control the release schedule so those users who would hold such tokens gets new build.
@jot2re
Copy link
Collaborator

jot2re commented May 25, 2022

@weiwu-zhang , @jot2re I have few comments for the ERCs

  1. for ERC5XX0 - better work with string[] , not struct and have option to upload/replace/add every single script or all-in-one-request. Besause in currebt structure if we have defined 100 scripts and admin wants to remove first script then all scripts will be shifted left and in blockchaine storage lot of updates can appear, and it can be very GAS-expencive. If we can remove single script then we can move last script to the slot of removed script and it can be much cheaper.
  2. better have "smart contract deployment key" as fallback option, but add variable "scriptSigner", to have option to transfer right to sign scripts/certficates to another wallet.

Regarding 1. I actually deliberately didn't specify this flexibility since how I understood @weiwu-zhang an ERC like this should only specify the smallest amount of constraints to make things work (since apparently ERC implementor very quickly lose attention 😅). @weiwu-zhang what do you think? Should we update the ERC to allow for the more flexibility as @oleggrib suggest? (BTW I would agree with Oleg, that this provides the most complete standard).
In any case, I will remove the struct and let is just be string[].

Regarding 2. I am not sure I completely understand what you suggest. You mean adding the option of allowing another key to control the smart contract? I think this is a bit out of the scope of ERC5XX0, but instead handled by ERC5XX1. Or maybe I am misunderstanding?

@SmartLayer
Copy link
Author

SmartLayer commented May 25, 2022

Options (I'd go for 2 and find an ERC to refer to, or an openzepplin convention).

  1. Deployment key issues certificates, whose subject is the script signing key;
  2. Admin key issues certificates, whose subject is the script signing key; (practical reason: deployment is done by a different team specialised at deployment and has no authority);
  3. Admin key updates smart contract which allows a new script signing key issued; no certificates;
  4. Admin key updates smart contract returns new certificate issuing key, which issues certificates whose subject is the script signing key;

Tore Kasper Frederiksen and others added 6 commits May 25, 2022 13:44
…2nd ERC and it made the first ERC too wordy
1. Is this scriptURI() unique in one smart contract?
   For example, in NFT smart contract can we define one scriptURI() per one tokenId?
   ** something like mapping(tokenId => scriptURI)

2. Regarding scriptURI struct, can we store hash of uri for transparency? It's some thing like below
   struct ScriptURI {
      string uri;
      bytes32 scriptHash // for Authenticity // user can know if he executes correct script
   }

3. In variables
   script: The script supplying additional functionality to the tokens.
   ??? is it different from scriptURI?

4. In IERC 5XX0 interface
   interface IERC5XX0 {
      /// @dev Mee This event emits when the scriptURI is updated,
      /// so wallets implementing this interface can update a cached script
      event ScriptUpdate(string[] memory newScriptURI);

      /// @notice Get the scriptURI for the contract
      /// @return The scriptURI
      function scriptURI() external view returns(string[] memory);

      /// @notice Update the scriptURI
      /// emits event ScriptUpdate(scriptURI memory newScriptURI);
      function setScriptURI(string[] memory newScriptURI, bytes memory newSigScriptURI) external;
   }

   function setScriptURI(string[] memory newScriptURI, bytes memory newSigScriptURI) external;
   == What is newSigScriptURI?

5. The smart contract implementing IERC5XX0 MUST set owner=msg.sender in its constructor.
   == Does it means ownership is immutable in this smart contract? why only msg.sender?
Tore Kasper Frederiksen and others added 10 commits June 16, 2022 18:29
    
    1. the URI can be interpreted as the URI of the server API access point, if a server is involved in the client script. Change URI to downlaod-URI to avoid this.
    
    2. added a point in Security Considerations that server API is not in the scope.
    
    3. Explain why the client script need to be linked to a contract.
Removed these text
(may add it later to a medium article


The `scriptURI` points to decentralized immutable locations *or* the location itself contains information that can validate the script's authenticity. For example, the URI may point to a location on a blockchain, or, URI may contain a hash digest of the resource, such as with IPFS. In the first situation, the client (wallet) should assume it is authentic. In the latter case, the client must also validate that hash digest.

If the `scriptURI` is a download link to a server, it may not contain a digest in the URI. In such a case, it may be desirable to return the digest in a struct together with the URI. This allows the caller to assert the client script is authentic after downloading it. However, igital Signature is a much better way to solve this problem.
@jot2re
Copy link
Collaborator

jot2re commented Jun 30, 2022

@JamesSmartCell @weiwu-zhang what is the status on ERC 5XX0 and 5XX1? Are we continuing to keep this branch open and working on this until they are either accepted or rejected?

@JamesSmartCell
Copy link
Contributor

@jot2re Yes it's still open

EIP-5169: ethereum/EIPs#5169
EIP-5170: ethereum/EIPs#5170

It looks like 5169 is pretty much in order now, after a bit of to-and-fro, once (if) it is accepted then 5170 would follow, since it depends on 5169 to make sense.

@SmartLayer SmartLayer force-pushed the main branch 2 times, most recently from e54d791 to 1368554 Compare June 30, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.