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

Some base functions are not virtual and thus can't be overridden by default. #358

Closed
sdocquir opened this issue Jul 8, 2022 · 5 comments

Comments

@sdocquir
Copy link

sdocquir commented Jul 8, 2022

Some base functions are not virtual and thus can't be overridden by default.
For example, ownerOf(), balanceOf(), etc.
Is there any reason behind that design decision ? Openzeppelin has theirs all virtual to allow the most amount of extensions/customizations.

I have a use case for an extension to ERC721A that would involve modifying the logic post-mint. I'm fine with simply changing those functions to virtual in my source code, but was also wondering if there's more to that design decision that I might be missing.

@Vectorized
Copy link
Collaborator

We can change them to virtual.

@Vectorized
Copy link
Collaborator

Btw, curious, what is your use case for overriding these functions?

@sdocquir
Copy link
Author

sdocquir commented Jul 8, 2022

Thanks, that'd be great.
Not able to disclose that just now, but my team will have an extension proposal in the next couple weeks !

@Vectorized
Copy link
Collaborator

Just a disclaimer, we will very most likely reject extension proposals though.

@sdocquir
Copy link
Author

sdocquir commented Jul 8, 2022

We're writing this extension for our project.
I understand modifying those base functions could be considered to be outside the purview of an extension and might warrant maintaining a separate standard altogether, but I figured it wouldn't hurt to start with a proposal and get community feedback.

No worries if/when it gets rejected.

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

No branches or pull requests

2 participants