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.

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

Counters library for incremental token id #507

Closed
remiroyc opened this issue Oct 31, 2022 · 4 comments
Closed

Counters library for incremental token id #507

remiroyc opened this issue Oct 31, 2022 · 4 comments

Comments

@remiroyc
Copy link

remiroyc commented Oct 31, 2022

Motivation

The mint function of the actual ERC721 contract requires a token id:

func _mint{pedersen_ptr: HashBuiltin*, syscall_ptr: felt*, range_check_ptr}(to: felt, token_id: Uint256) { }

In most cases, users who want to port their nft collections from mainnet to starknet will need incremental token ids.

Details

Token ids should not be assumed to follow any pattern as described by EIP72.
That's why I propose to introduce a Counters library and a new erc721 preset with updated documentation as it's done for the solidity version.

@martriay
Copy link
Contributor

Hey there! I don't think a counter library would add much value given its simplicity. I would support though, given its widespread use, the implementation of an auto-incremental ERC721 preset contract along with some docs to guide users into using it.

@andrew-fleming
Copy link
Collaborator

I agree that it's quite simple. From a convenience standpoint, however, I wouldn't hate having a Counter lib. Plus, the ReentrancyMock is already using one: https://github.com/OpenZeppelin/cairo-contracts/blob/main/tests/mocks/ReentrancyMock.cairo

Either way, +1 to an auto-incremental ERC721 preset.

@ggonzalez94
Copy link
Collaborator

The counter library was removed in Solidity, and we haven't seen any demand to implement this in Cairo so I'm closing this issue.
@andrew-fleming do you think we should still implement an auto-incremental ERC721 preset?

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Resolved in Contracts for Cairo Nov 25, 2024
@andrew-fleming
Copy link
Collaborator

andrew-fleming commented Nov 25, 2024

@ggonzalez94 not anymore with how we define presets. It might be worth adding the option to wizard with mintable like in the sol contracts wizard (#1230)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Resolved
Development

No branches or pull requests

4 participants