oceanprotocol / contracts

🐙 Smart contracts for Ocean Protocol
https://oceanprotocol.com
77 stars 61 forks source link

ERC20Template4 - Confidential EVM #870

Closed alexcos20 closed 2 months ago

alexcos20 commented 3 months ago

ERC20TemplateSapphire (asset files object stored in contract)

- follows same functions and principles as ERC20TemplateEnterprise, with the following additions
- should be deployed only on Oasis Sapphire, and all transactions should be encrypted, otherwise security is compromised
- on initialize, files object(asset URLs) is stored in the contract
- owner can change files object anytime, calling setFilesObject
- for every order, consumer and serviceId are added to a mapping
- has a list of allowed/denied providers (using standard ERC721.balanceOf). Contracts addresses can be changed by owner
- when a provider tries to fetch the files object (by calling getFilesObject), the following conditions are checked:
    - provider address has to be in allow list and not on deny list (given those lists are not address_zero)
    - consumer has to have a valid order
openzeppelin-code[bot] commented 3 months ago

Feature/template_sapphire

Generated at commit: 2ed0099a60357a11cf13368f69424d2a1b669999

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
1
0
10
39
52
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

trentmc commented 3 months ago

Hey @alexcos20 I recommend to also update the README that describes the templates: https://github.com/oceanprotocol/contracts/tree/main/contracts/templates.

trentmc commented 3 months ago

I recommend renaming it from "ERC20TemplateSapphire.sol" --> "ERC20Template4.sol".

Here's why:

First, calling it "ERC20TemplateSapphire.sol" is a misnomer. Since template 3 is also Sapphire-only. And it's tuned for Predictoor, of course.

Second, remember the problem with the name "ERC20TemplateEnterprise.sol": it was a template useful beyond enterprises too. However the "enterprise" portion always made people think it was just for enterprises.

Third: there's a good chance that we deploy >=1 new variant of template 3 for Predictoor. Eg for continuous-valued predictions, or other new features into the contract. Similarly, there's a good chance we deploy variants of the other templates too (including this new one). It starts to get messy to manage if the names are misleading. Hence why there's a simple name, plus a table to show what characteristics each template has.

If these weren't smart contracts, because of the issues above, we probably would have refactored the names into "ERC20Template1.sol", "ERC20Template2.sol", and "ERC20Template3.sol" long ago. But we only realized them when getting into template 3, and that's why it's already named ERC20Template3.sol". Let's keep this trend going: I recommend that this this new template, call it "ERC20Template4.sol".

alexcos20 commented 3 months ago

Hey @alexcos20 I recommend to also update the README that describes the templates: https://github.com/oceanprotocol/contracts/tree/main/contracts/templates.

Updated in https://github.com/oceanprotocol/contracts/pull/870/files#diff-e3d04a7f6d6d8ac50c0603b74dee727caa6d28993abdaeef48552bd29ac83dcc

alexcos20 commented 3 months ago

@trentmc - renamed to ERC20Template4 in https://github.com/oceanprotocol/contracts/pull/870/commits/80e682a3160282fae3bbf2ef6dd99fb6311d6565