Skip to content

Improved ERC721 granularity - #1304

Merged
frangio merged 5 commits into
OpenZeppelin:masterfrom
nventuro:erc721-granularity
Sep 7, 2018
Merged

Improved ERC721 granularity#1304
frangio merged 5 commits into
OpenZeppelin:masterfrom
nventuro:erc721-granularity

Conversation

@nventuro

@nventuro nventuro commented Sep 7, 2018

Copy link
Copy Markdown
Contributor

ERC721 is the basic contract
Enumerable and Metadata are extensions
Full has both extensions

@nventuro nventuro added contracts Smart contract code. breaking change Changes that break backwards compatibility of the public API. labels Sep 7, 2018
@nventuro nventuro added this to the v2.0 milestone Sep 7, 2018
@nventuro nventuro self-assigned this Sep 7, 2018
@nventuro
nventuro requested a review from frangio September 7, 2018 14:02

@frangio frangio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but please update for new underscore convention for variables. #1282

* @title ERC-721 Non-Fungible Token Standard, optional enumeration extension
* @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
*/
contract IERC721Enumerable is IERC721Basic {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make this an interface?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interfaces cannot inherit, sadly.

Comment thread contracts/token/ERC721/ERC721.sol Outdated
// register the supported interfaces to conform to ERC721 via ERC165
_registerInterface(InterfaceId_ERC721Enumerable);
_registerInterface(InterfaceId_ERC721Metadata);
contract ERC721 is ERC721Basic, ERC721Enumerable, ERC721Metadata {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add is IERC721 also.

@frangio frangio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nventuro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Changes that break backwards compatibility of the public API. contracts Smart contract code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants