enjin / erc-1155

ERC-1155: Smart Contract Sample Implementation
Apache License 2.0
420 stars 208 forks source link

Rename ERC1155Mintable.sol #20

Open fulldecent opened 5 years ago

fulldecent commented 5 years ago

Minting functionality is not standardized in ERC-1155.

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

Code for minting is included in this repository as a demonstration (that's good) and it will be helpful to people studying this code as a reference implementation (also good). Currently this code is included in a file named ERC1155Mintable.sol (that's bad).

Because the minting functionality is not standardized, I recommend that any interfaces (interface) or implementations (contract) which implement minting should be in files that are not named including "1155". At current, somebody referencing this repository may mistakingly assume that the provided minting interface is standardized.

I recommend that these example extensions should be renamed as a "mock" or "example" or other. A reference for best practice in this aspect is the file organization presented at

https://github.com/0xcert/ethereum-erc721/tree/master/src/contracts/tokens


Similarly, this issue also applies to other non-standard implementations provided in this project.

AC0DEM0NK3Y commented 5 years ago

Technically minting is standardized:

When minting/creating tokens, the _from argument MUST be set to 0x0 (i.e. zero address). See “Minting/creating and burning/destroying rules”.

Also see "Minting/creating and burning/destroying rules:".

How you write your implementation around those rules is not standardized but it shouldn't be given it's an API.

fulldecent commented 5 years ago

Yes, there are rules that apply to minting.

The function signature:

function create(uint256 _initialSupply, string calldata _uri) external returns(uint256 _id);

is not standardized.

And seeing the function in a file named ERC1155... may lead somebody to think it is.

fulldecent commented 5 years ago

Also please email me, I lost your email

AC0DEM0NK3Y commented 5 years ago

I can't quite agree on this one, there is stuff in there that is standardized and things that aren't, so there are things that have to be followed.

If people think the create function is part of the standard, they didn't read the standard.

We could add comments to such things to say "this is a non-standard function".