Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove check for address zero in ERC1155.balanceOf #4259

Closed
frangio opened this issue May 16, 2023 · 2 comments · Fixed by #4263
Closed

Remove check for address zero in ERC1155.balanceOf #4259

frangio opened this issue May 16, 2023 · 2 comments · Fixed by #4263
Milestone

Comments

@frangio
Copy link
Contributor

frangio commented May 16, 2023

We have a require statement in this function that doesn't seem to be mandated ERC-1155, or at least I can't find it in the spec. It looks like it should be removed.

function balanceOf(address account, uint256 id) public view virtual override returns (uint256) {
require(account != address(0), "ERC1155: address zero is not a valid owner");

@frangio frangio added this to the 5.0 milestone May 16, 2023
@Amxx
Copy link
Collaborator

Amxx commented May 19, 2023

This might have been imported from the ERC-721 standard into the ERC-1155 implementation (for consistency)

/// @dev NFTs assigned to the zero address are considered invalid, and this
/// function throws for queries about the zero address.

(IMO, the part of ERC-721 is really bad)

@balajipachai
Copy link
Contributor

In that case it's better we remove it, and update the tests wherever required.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants