transmissions11/solmate

solmate's ERC721 implementation of `ownerOf` does not comply with the EIP-721 standard

Gilgames000 opened this issue · 5 comments

Gilgames000 commented

As per EIP-721, queries about NFTs whose owner is the zero address should throw

    /// @notice Find the owner of an NFT
    /// @dev NFTs assigned to zero address are considered invalid, and queries
    ///  about them do throw.
    /// @param _tokenId The identifier for an NFT
    /// @return The address of the owner of the NFT
    function ownerOf(uint256 _tokenId) external view returns (address);

but the current implementation does not comply with that.

transmissions11 commented

man thats dumb

transmissions11 commented

but hm unlike balanceOf needing to revert if the input is addresss(0), this does seem like behavior someone could rely on

Gilgames000 commented

man thats dumb

ikr lol it's not a good idea to have a standard like that when the language has no way to enforce it because it defeats the purpose of having a standard in the first place since you cannot programmatically assess its compliance

but hm unlike balanceOf needing to revert if the input is addresss(0), this does seem like behavior someone could rely on

yeah balanceOf should throw too lmao, the problem is that I noticed it because I stumbled upon some code relying on this, and some other code not relying on this at all, so I looked at openzeppelin implementation and I was like what why do they throw and so then I checked the EIP...

transmissions11 commented

yeah balanceOf should throw too lmao

yeah i made the deliberate choice not to cuz i couldnt see how any reasonable person would depend on that behavior but it totally makes sense for someone to rely on ownerOf behavior.... ok hm ill fix

transmissions11 commented

fixed, thank you for raising this @Gilgames000

  • 👍1