search

mudgen / diamond-1

EIP-2535 Diamonds reference implementation.
MIT License
30 stars 12 forks source link

Move non-standard functionality to mocks folder #1

Open fulldecent opened 4 years ago

fulldecent commented 4 years ago

This issue applies to diamond-1 and all other implementations referenced in the standard.

Expected

  1. Features which are not specified in the standard are not included in the reference implementation.

Actual

Some non-standard features are in the reference implementation, including ownership standard: https://github.com/mudgen/diamond-1/blob/master/contracts/Diamond.sol#L14

Background

This repository is advertised as a "reference implementation".

Source: https://eips.ethereum.org/EIPS/eip-2535

See the reference implementation to see an example of how this can be implemented.

Source: https://github.com/mudgen/Diamond

Links to diamond reference implementation repositories: diamond-1 diamond-2 diamond-3

Discussion

Currently, this implementation depends on ERC-173: Contract Ownership Standard. Such a dependency is not specified in ERC-2535: Diamond Standard [DRAFT].

Please consider the ERC-721 reference implementation as an example of how to properly include non-standard implementation details in an ERC reference implementation.

The ERC-721 standard does not specify how to mint tokens. So the reference implementation does not include a public function for minting tokens. Of course, many people want to have more than zero tokens, so there is included a "Mock" which provides such useful, non-standard features.

You can similarly have a proper reference implementation by extracting all non-standard features to a "mock" folder.

mudgen commented 4 years ago

Thanks @fulldecent Good point. Will consider this.