Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Introduce ERC1155 totalSupply() and exists() functions #2593

Merged
merged 8 commits into from
May 20, 2021
Merged

Introduce ERC1155 totalSupply() and exists() functions #2593

merged 8 commits into from
May 20, 2021

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Mar 15, 2021 •
edited by frangio
Loading

Followup to #2185

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@Amxx Amxx force-pushed the feature/ERC1155exists branch from 62ee599 to 76f54ee Compare March 15, 2021 10:27
@frangio
Copy link
Contributor

frangio commented Mar 16, 2021

Do we know if there is precedent for these totalSupply and exists functions? I'd like to check that we aren't making them incompatible with something that's already out there.

@frangio
Copy link
Contributor

frangio commented Mar 16, 2021

The code in this PR needs to remove SafeMath.

@Amxx
Copy link
Collaborator Author

Amxx commented Mar 16, 2021

The code in this PR needs to remove SafeMath.

done.

@Amxx
Copy link
Collaborator Author

Amxx commented Mar 16, 2021

Do we know if there is precedent for these totalSupply and exists functions? I'd like to check that we aren't making them incompatible with something that's already out there.

Not 100% sure. I've discussed usecase where 1155 could be used for voting, and total supply was useful ... but that is everything

@frangio
Copy link
Contributor

frangio commented Mar 17, 2021

totalSupply in OpenSea: https://github.com/ProjectOpenSea/opensea-erc1155/blob/baa203757e33fc41e5465e3c2e6b0de82391c7fd/contracts/ERC1155Tradable.sol#L73-L77

They also have an internal _exists function which has different semantics than the exists in this PR, but it could be overriden or extended through inheritance.

@Amxx Amxx force-pushed the feature/ERC1155exists branch from 20e55a1 to d241577 Compare March 23, 2021 15:22
@Amxx Amxx force-pushed the feature/ERC1155exists branch from d241577 to 628a697 Compare March 23, 2021 15:29
@frangio frangio changed the title Introduce ERC1155 totalSupply() and _exists() functions Introduce ERC1155 totalSupply() and exists() functions May 20, 2021
@frangio frangio self-requested a review May 20, 2021 21:39
@frangio frangio marked this pull request as ready for review May 20, 2021 21:41
@frangio frangio enabled auto-merge (squash) May 20, 2021 21:41
@frangio frangio merged commit 406c836 into OpenZeppelin:master May 20, 2021
@conorbros
Copy link
Contributor

When will these changes be released?

@frangio
Copy link
Contributor

frangio commented May 27, 2021

We're aiming for a release candidate on June 3rd.

@Parth2404
Copy link

Is this release yet or not??

@Amxx Amxx deleted the feature/ERC1155exists branch March 15, 2022 10:13
@Amxx
Copy link
Collaborator Author

Amxx commented Mar 15, 2022

Is this release yet or not??

Its been released in 4.2.0 (2021-06-30)

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

@frangio frangio frangio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Amxx @frangio @conorbros @Parth2404