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

Hybrid ERC721 Minting #56

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Hybrid ERC721 Minting #56

wants to merge 5 commits into from

Conversation

alex-connolly
Copy link
Contributor

No description provided.


import { AccessControlEnumerable } from "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

abstract contract ERC721AccessControl is AccessControlEnumerable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we want to refer to this as ERC721AccessControl as then it implies in implements ERC721 interfaces. Maybe we should rename it to just ImmutableAccessControl something else?

All other ERC721 functions are supported, with routing logic depending on the tokenId.
*/

abstract contract ERC721HybridMinting is ERC721PsiBurnable, ERC721 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have this extend ERC721Royalty instead of ERC721 since ERC721Royalty already extends ERC721?


//
function _safeMint(address to, uint256 quantity) internal virtual override(ERC721, ERC721Psi) {
return super._safeMint(to, quantity);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if it makes sense to explicitly override and call ERC721Psi._safeMint(to, quantity) here since thats what is actually being called. Same for the two overrides below

@drinkcoffee
Copy link
Contributor

@alex-connolly this PR appears to be stale, and superseded by newer work. Should this PR be abandoned?

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

Successfully merging this pull request may close these issues.

3 participants