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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate ERC721 Enumerate extension #722

Open
Bal7hazar opened this issue Aug 31, 2023 · 6 comments 路 May be fixed by #983
Open

Migrate ERC721 Enumerate extension #722

Bal7hazar opened this issue Aug 31, 2023 · 6 comments 路 May be fixed by #983
Assignees
Milestone

Comments

@Bal7hazar
Copy link
Contributor

馃 Motivation

To be feature partiy with the Cairo 0 version, having a ERC721 Enumerable extension proposition would be interesting.

馃摑 Details

Here are 2 repo which did an implementation:

@Bal7hazar
Copy link
Contributor Author

Bal7hazar commented Aug 31, 2023

Is there any concern to not have done it yet (maybe this issue #207)?

Also are you comfortable to split the current ERC721 implementation between ERC721 and ERC721Metadata (in order to have ERC721Metadata defined as an extension separated from the core ERC721)?

@martriay
Copy link
Contributor

martriay commented Sep 5, 2023

@Bal7hazar i think the best is to wait for components to streamline this eventual separation.
although not a blocker, another feature that'd help here is supertraits (or similar) to extend a trait with another i.e. extend ERC721 by just defining the metadata-specific fields on top. I think there might be a way of emulating it now though.

@Bal7hazar
Copy link
Contributor Author

@martriay thanks for sharing your thoughts, just to ensure I understand well, do you think it's risky for the community if OZ repo have a pre-component implementation right now and make another other one after the component feature is released (with the risk of changing the signature or storage making the upgrade not possible easily) ?

I understand that you could have other priority, my point is if I (or someone else) make a PR with a proposition right now, would you accept to merge it in the repo? (Assuming it passes the merging process ofc).

@martriay
Copy link
Contributor

@Bal7hazar sorry for the long silence. are you still willing to work on this?

@martriay martriay modified the milestones: after, next, later Nov 24, 2023
@Bal7hazar
Copy link
Contributor Author

Hey @martriay actually there is already a component version made by Seraph-Labs:

https://github.com/Seraph-Labs/Cairo-Contracts/blob/main/src/tokens/erc721/extensions/enumerable.cairo

@martriay
Copy link
Contributor

martriay commented Nov 24, 2023

Great! That'd be a useful reference. We're still going to add this to our library, I expect us to work tackle it sometime during Q1 2024. Anyone interested in tackling it before will be welcomed :)

@martriay martriay modified the milestones: later, after Dec 10, 2023
@martriay martriay modified the milestones: after, next Jan 26, 2024
@martriay martriay modified the milestones: next, current Mar 4, 2024
@martriay martriay modified the milestones: next, current, after Mar 4, 2024
@andrew-fleming andrew-fleming self-assigned this Apr 27, 2024
@andrew-fleming andrew-fleming modified the milestones: 2. next, 1. current Apr 30, 2024
@andrew-fleming andrew-fleming linked a pull request Apr 30, 2024 that will close this issue
4 tasks
@ericnordelo ericnordelo modified the milestones: 1. current, 2. next May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 馃彈 In progress
Development

Successfully merging a pull request may close this issue.

4 participants