Openzeppelin-contracts: clarify that exists() is an extension to ERC-721

[fulldecent]

This statement is incorrect:

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/token/ERC721/DeprecatedERC721.sol#L9

And this is a proprietary extension to ERC-721.

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/token/ERC721/ERC721BasicToken.sol#L69

Please add a note that the exists function is an extension and is not part of the standard. This will help avoid confusion.

[nventuro]

Hi @fulldecent!

The sentence on DeprecatedERC721 is actually correct: it indicates that ERC721 is the interface to use when interacting with standard-compliant ERC721 contracts (as opposed to DeprecatedERC721, which implements an older version of the standard). You can find our 1.12 ERC721 interface here. Please let us know if you find any differences between that contract and the standard!

From what I can tell, you seem to be right about the exists function not being part of the standard (at least according to this document, which I believe is the latest and finalized version of it). Thanks for reporting this!

I see two main courses of action that we can take regarding said function:

1. Add a comment (as suggested by @fulldecent)
2. Make exists internal (it is used in some require statements)

I'm leaning towards 2, since we're doing some API breakage in 2.0 anyway, and I'd rather the stable interface be as minimal as possible allowing us to add on top of it, rather than having to remove stuff in the future. @shrugs @frangio thoughts?

[shrugs]

We do actually make it clear that exists() is non-standard; we have a 165 identifier for it: https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/token/ERC721/ERC721Basic.sol#L26

Probably the best method is to create an ERC721Exists implementation that does add that function as public and have it internal within the basic implementation.

[nventuro]

Hmm, I missed exists being in ERC721Basic, and therefore also in ERC721. We definitely should remove it from there. This also means @fulldecent's first comment is spot-on.

Is that method useful enough to justify providing a derived contract with that single addition? I'd prefer keeping it internal, giving our users the choice to expose it, instead of 'suggesting' said usage by having it be a thing.